diff --git a/tests/ui/perfherder/alerts-view/alert_status_countdown_test.jsx b/tests/ui/perfherder/alerts-view/alert_status_countdown_test.jsx index 2dd47412695..06990051d74 100644 --- a/tests/ui/perfherder/alerts-view/alert_status_countdown_test.jsx +++ b/tests/ui/perfherder/alerts-view/alert_status_countdown_test.jsx @@ -42,7 +42,7 @@ test('Alert is created on Monday, Triage countdown shows 3 working days', async const dueDateStatus = await waitFor(() => getByTestId('due-date-status')); const dueDateStatusText = dueDateStatus.querySelector('p').innerHTML; - expect(dueDateStatusText).toBe('Triage: 3 days left'); + expect(dueDateStatusText).toBe('Time left: 3 days left'); }); test('Alert is created on Monday, Triage countdown shows 2 working days', async () => { @@ -62,7 +62,7 @@ test('Alert is created on Monday, Triage countdown shows 2 working days', async const dueDateStatus = await waitFor(() => getByTestId('due-date-status')); const dueDateStatusText = dueDateStatus.querySelector('p').innerHTML; - expect(dueDateStatusText).toBe('Triage: 2 days left'); + expect(dueDateStatusText).toBe('Time left: 2 days left'); }); test('Alert is created on Monday, Triage countdown shows 1 working days', async () => { @@ -82,7 +82,7 @@ test('Alert is created on Monday, Triage countdown shows 1 working days', async const dueDateStatus = await waitFor(() => getByTestId('due-date-status')); const dueDateStatusText = dueDateStatus.querySelector('p').innerHTML; - expect(dueDateStatusText).toBe('Triage: 1 days left'); + expect(dueDateStatusText).toBe('Time left: 1 days left'); }); test('Alert is created on Monday, Triage countdown shows hours left', async () => { @@ -102,7 +102,7 @@ test('Alert is created on Monday, Triage countdown shows hours left', async () = const dueDateStatus = await waitFor(() => getByTestId('due-date-status')); const dueDateStatusText = dueDateStatus.querySelector('p').innerHTML; - expect(dueDateStatusText).toBe('Triage: 10 hours left'); + expect(dueDateStatusText).toBe('Time left: 10 hours left'); }); test('Alert is created on Monday, Triage countdown shows Overdue', async () => { @@ -122,7 +122,7 @@ test('Alert is created on Monday, Triage countdown shows Overdue', async () => { const dueDateStatus = await waitFor(() => getByTestId('due-date-status')); const dueDateStatusText = dueDateStatus.querySelector('p').innerHTML; - expect(dueDateStatusText).toBe('Triage: Overdue'); + expect(dueDateStatusText).toBe('Time left: Overdue'); }); test('Alert is created on Monday, Bug countdown shows 5 working days', async () => { @@ -143,7 +143,7 @@ test('Alert is created on Monday, Bug countdown shows 5 working days', async () const dueDateStatus = await waitFor(() => getByTestId('due-date-status')); const dueDateStatusText = dueDateStatus.querySelector('p').innerHTML; - expect(dueDateStatusText).toBe('Bug: 5 days left'); + expect(dueDateStatusText).toBe('Time left: 5 days left'); }); test('Alert is created on Monday, Bug countdown shows 4 working days', async () => { @@ -164,7 +164,7 @@ test('Alert is created on Monday, Bug countdown shows 4 working days', async () const dueDateStatus = await waitFor(() => getByTestId('due-date-status')); const dueDateStatusText = dueDateStatus.querySelector('p').innerHTML; - expect(dueDateStatusText).toBe('Bug: 4 days left'); + expect(dueDateStatusText).toBe('Time left: 4 days left'); }); test('Alert is created on Monday, Bug countdown shows 3 working days', async () => { @@ -185,7 +185,7 @@ test('Alert is created on Monday, Bug countdown shows 3 working days', async () const dueDateStatus = await waitFor(() => getByTestId('due-date-status')); const dueDateStatusText = dueDateStatus.querySelector('p').innerHTML; - expect(dueDateStatusText).toBe('Bug: 3 days left'); + expect(dueDateStatusText).toBe('Time left: 3 days left'); }); test('Alert is created on Monday, Bug countdown shows 2 working days', async () => { @@ -206,7 +206,7 @@ test('Alert is created on Monday, Bug countdown shows 2 working days', async () const dueDateStatus = await waitFor(() => getByTestId('due-date-status')); const dueDateStatusText = dueDateStatus.querySelector('p').innerHTML; - expect(dueDateStatusText).toBe('Bug: 2 days left'); + expect(dueDateStatusText).toBe('Time left: 2 days left'); }); test('Alert is created on Monday, Bug countdown shows 1 working days', async () => { @@ -227,7 +227,7 @@ test('Alert is created on Monday, Bug countdown shows 1 working days', async () const dueDateStatus = await waitFor(() => getByTestId('due-date-status')); const dueDateStatusText = dueDateStatus.querySelector('p').innerHTML; - expect(dueDateStatusText).toBe('Bug: 1 days left'); + expect(dueDateStatusText).toBe('Time left: 1 days left'); }); test('Alert is created on Monday, Bug countdown shows hours left', async () => { @@ -248,7 +248,7 @@ test('Alert is created on Monday, Bug countdown shows hours left', async () => { const dueDateStatus = await waitFor(() => getByTestId('due-date-status')); const dueDateStatusText = dueDateStatus.querySelector('p').innerHTML; - expect(dueDateStatusText).toBe('Bug: 23 hours left'); + expect(dueDateStatusText).toBe('Time left: 23 hours left'); }); test('Alert is created on Monday, Bug countdown shows Overdue', async () => { @@ -269,7 +269,7 @@ test('Alert is created on Monday, Bug countdown shows Overdue', async () => { const dueDateStatus = await waitFor(() => getByTestId('due-date-status')); const dueDateStatusText = dueDateStatus.querySelector('p').innerHTML; - expect(dueDateStatusText).toBe('Bug: Overdue'); + expect(dueDateStatusText).toBe('Time left: Overdue'); }); // testing what the tooltip shows for the cases when the alert is created either on Wednesday, Thursday, Friday or the weekend @@ -293,7 +293,7 @@ test('Alert is created on Wednesday, Triage countdown shows 3 working days', asy const dueDateStatus = await waitFor(() => getByTestId('due-date-status')); const dueDateStatusText = dueDateStatus.querySelector('p').innerHTML; - expect(dueDateStatusText).toBe('Triage: 3 days left'); + expect(dueDateStatusText).toBe('Time left: 3 days left'); }); test('Alert is created on Wednesday, Triage countdown shows 2 working days', async () => { @@ -314,7 +314,7 @@ test('Alert is created on Wednesday, Triage countdown shows 2 working days', asy const dueDateStatus = await waitFor(() => getByTestId('due-date-status')); const dueDateStatusText = dueDateStatus.querySelector('p').innerHTML; - expect(dueDateStatusText).toBe('Triage: 2 days left'); + expect(dueDateStatusText).toBe('Time left: 2 days left'); }); test('Alert is created on Wednesday, Triage countdown shows 1 working days', async () => { @@ -335,10 +335,10 @@ test('Alert is created on Wednesday, Triage countdown shows 1 working days', asy const dueDateStatus = await waitFor(() => getByTestId('due-date-status')); const dueDateStatusText = dueDateStatus.querySelector('p').innerHTML; - expect(dueDateStatusText).toBe('Triage: 1 days left'); + expect(dueDateStatusText).toBe('Time left: 1 days left'); }); -test('Alert is created on Wednesday, Triage countdown shows hours left', async () => { +test('Alert is created on Wednesday, Triage countdown shows Overdue when due date has been reached', async () => { const alert = testAlertSummaries[0]; // created date day is set to Wednesday @@ -356,7 +356,7 @@ test('Alert is created on Wednesday, Triage countdown shows hours left', async ( const dueDateStatus = await waitFor(() => getByTestId('due-date-status')); const dueDateStatusText = dueDateStatus.querySelector('p').innerHTML; - expect(dueDateStatusText).toBe('Triage: 0 hours left'); + expect(dueDateStatusText).toBe('Time left: Overdue'); }); test('Alert is created on Wednesday, Triage countdown shows Overdue', async () => { @@ -377,7 +377,7 @@ test('Alert is created on Wednesday, Triage countdown shows Overdue', async () = const dueDateStatus = await waitFor(() => getByTestId('due-date-status')); const dueDateStatusText = dueDateStatus.querySelector('p').innerHTML; - expect(dueDateStatusText).toBe('Triage: Overdue'); + expect(dueDateStatusText).toBe('Time left: Overdue'); }); test('Alert is created on Wednesday, Bug countdown shows 5 working days', async () => { @@ -398,7 +398,7 @@ test('Alert is created on Wednesday, Bug countdown shows 5 working days', async const dueDateStatus = await waitFor(() => getByTestId('due-date-status')); const dueDateStatusText = dueDateStatus.querySelector('p').innerHTML; - expect(dueDateStatusText).toBe('Bug: 5 days left'); + expect(dueDateStatusText).toBe('Time left: 5 days left'); }); test('Alert is created on Wednesday, Bug countdown shows 4 working days', async () => { @@ -419,7 +419,7 @@ test('Alert is created on Wednesday, Bug countdown shows 4 working days', async const dueDateStatus = await waitFor(() => getByTestId('due-date-status')); const dueDateStatusText = dueDateStatus.querySelector('p').innerHTML; - expect(dueDateStatusText).toBe('Bug: 4 days left'); + expect(dueDateStatusText).toBe('Time left: 4 days left'); }); test('Alert is created on Wednesday, Bug countdown shows 3 working days', async () => { @@ -440,7 +440,7 @@ test('Alert is created on Wednesday, Bug countdown shows 3 working days', async const dueDateStatus = await waitFor(() => getByTestId('due-date-status')); const dueDateStatusText = dueDateStatus.querySelector('p').innerHTML; - expect(dueDateStatusText).toBe('Bug: 3 days left'); + expect(dueDateStatusText).toBe('Time left: 3 days left'); }); test('Alert is created on Wednesday, Bug countdown shows 2 working days', async () => { @@ -461,7 +461,7 @@ test('Alert is created on Wednesday, Bug countdown shows 2 working days', async const dueDateStatus = await waitFor(() => getByTestId('due-date-status')); const dueDateStatusText = dueDateStatus.querySelector('p').innerHTML; - expect(dueDateStatusText).toBe('Bug: 2 days left'); + expect(dueDateStatusText).toBe('Time left: 2 days left'); }); test('Alert is created on Wednesday, Bug countdown shows 1 working days', async () => { @@ -482,7 +482,7 @@ test('Alert is created on Wednesday, Bug countdown shows 1 working days', async const dueDateStatus = await waitFor(() => getByTestId('due-date-status')); const dueDateStatusText = dueDateStatus.querySelector('p').innerHTML; - expect(dueDateStatusText).toBe('Bug: 1 days left'); + expect(dueDateStatusText).toBe('Time left: 1 days left'); }); test('Alert is created on Wednesday, Bug countdown shows hours left', async () => { @@ -491,10 +491,10 @@ test('Alert is created on Wednesday, Bug countdown shows hours left', async () = // created date day is set to Wednesday alert.created = '2022-02-09T11:41:31.419156'; alert.first_triaged = '2022-02-09T11:41:31.419156'; - alert.bug_due_date = '2022-02-16T10:41:31.419156'; + alert.bug_due_date = '2022-02-16T12:41:31.419156'; // current day is set to Wednesday - Date.now = jest.fn(() => Date.parse('2022-02-16T12:40:31.419156')); + Date.now = jest.fn(() => Date.parse('2022-02-16T10:41:31.419156')); const { getByTestId } = testStatusDropdown([], alert); const dueDateIcon = await waitFor(() => getByTestId('triage-clock-icon')); @@ -503,7 +503,7 @@ test('Alert is created on Wednesday, Bug countdown shows hours left', async () = const dueDateStatus = await waitFor(() => getByTestId('due-date-status')); const dueDateStatusText = dueDateStatus.querySelector('p').innerHTML; - expect(dueDateStatusText).toBe('Bug: 2 hours left'); + expect(dueDateStatusText).toBe('Time left: 2 hours left'); }); test('Alert is created on Wednesday, Bug countdown shows Overdue', async () => { @@ -524,7 +524,7 @@ test('Alert is created on Wednesday, Bug countdown shows Overdue', async () => { const dueDateStatus = await waitFor(() => getByTestId('due-date-status')); const dueDateStatusText = dueDateStatus.querySelector('p').innerHTML; - expect(dueDateStatusText).toBe('Bug: Overdue'); + expect(dueDateStatusText).toBe('Time left: Overdue'); }); test('Alert is ready, countdown shows Ready for acknowledge', async () => { diff --git a/ui/perfherder/alerts/AlertStatusCountdown.jsx b/ui/perfherder/alerts/AlertStatusCountdown.jsx index 6f677d70a4f..cad28bc2737 100644 --- a/ui/perfherder/alerts/AlertStatusCountdown.jsx +++ b/ui/perfherder/alerts/AlertStatusCountdown.jsx @@ -10,139 +10,110 @@ import { countdownClasses } from '../perf-helpers/constants'; import { alertIsLinkedToBug, alertIsTriaged, - getCountdownText, - getTimeDifference, + getCountdownLabel, isWeekend, } from '../perf-helpers/alertCountdownHelper'; -export default class AlertStatusCountdown extends React.Component { - constructor(props) { - super(props); - - this.showCountdownToTriageIcon = true; - } - - getDueDateCountdownsStatus() { - const { alertSummary } = this.props; - let { - triage_due_date: triageDueDate, - bug_due_date: bugDueDate, - } = alertSummary; - - const currentDate = new Date(Date.now()); - triageDueDate = new Date(triageDueDate); - bugDueDate = new Date(bugDueDate); - - const timeToTriageDifference = getTimeDifference( - currentDate, - triageDueDate, - ); - const timeToBugDifference = getTimeDifference(currentDate, bugDueDate); - - const countdowns = { - triage: '', - bug: '', - }; +// Before: defined inside render() on every call, allocating a new object each time. +// After: module-level constant, allocated once. +const DUE_DATE_FORMAT = { + day: 'numeric', + month: 'short', + hour: '2-digit', + minute: '2-digit', +}; - if (isWeekend()) { - this.showCountdownToTriageIcon = false; - return countdowns; - } - if (alertIsTriaged(alertSummary)) { - countdowns.bug = getCountdownText( - currentDate, - bugDueDate, - timeToBugDifference, - ); - } else { - countdowns.triage = getCountdownText( - currentDate, - triageDueDate, - timeToTriageDifference, - ); - countdowns.bug = getCountdownText( - currentDate, - bugDueDate, - timeToBugDifference, - ); - } +// Before: 'en-UK' is not a valid BCP-47 locale tag and silently falls back to the +// system locale, producing inconsistent date formats across machines. +// After: 'en-GB' is the correct tag for British English. +const formatDate = (dateStr) => + new Date(dateStr).toLocaleString('en-GB', DUE_DATE_FORMAT); - return countdowns; - } +const iconClassFor = (label) => { + if (label === 'Overdue') return countdownClasses.overdue; + if (label.includes('hours')) return countdownClasses.today; + return countdownClasses.ok; +}; - getCountdownClass(countdown) { - if (countdown === 'Overdue') { - return countdownClasses.overdue; - } - if (countdown.endsWith('hours left')) { - return countdownClasses.today; - } - return countdownClasses.ok; +// Before: class component with a getDueDateCountdownsStatus() method that mutated +// this.showCountdownToTriageIcon as a side effect, computed both triage and bug +// countdowns even when only one was shown, and used three boolean flags +// (showTriageCountdown, showBugCountdown, showReady) to control what to render. +// triageDueDate / bugDueDate were also shadowed in render(): first holding a raw +// ISO string from alertSummary, then overwritten with a formatted display string. +// +// After: function component with a linear three-branch state machine — each branch +// returns exactly what it renders, with no shared mutable state. +export default function AlertStatusCountdown({ alertSummary }) { + const alertStatus = getStatus(alertSummary.status); + + if (alertStatus !== 'untriaged' || isWeekend()) { + return
; } - render() { - const { alertSummary } = this.props; - - const alertStatus = getStatus(alertSummary.status); - const countdown = this.getDueDateCountdownsStatus(); - let countdownClass; - let showTriageCountdown; - let showBugCountdown; - let showReady; - - if (!alertIsTriaged(alertSummary)) { - countdownClass = this.getCountdownClass(countdown.triage); - showTriageCountdown = true; - } else if (!alertIsLinkedToBug(alertSummary)) { - countdownClass = this.getCountdownClass(countdown.bug); - showBugCountdown = true; - } else { - countdownClass = countdownClasses.ready; - showReady = true; - } - - const showCountdown = - alertStatus === 'untriaged' && this.showCountdownToTriageIcon; + const triaged = alertIsTriaged(alertSummary); + const hasBug = alertIsLinkedToBug(alertSummary); + if (triaged && hasBug) { return ( - -
- {showCountdown && ( -
-
- - } - tooltipText={ -
- {showTriageCountdown && ( - <> -
Due date:
-

Triage: {countdown.triage}

- - )} - {showBugCountdown && ( - <> -
Due date:
-

Bug: {countdown.bug}

- - )} - {showReady &&
Ready for acknowledge
} -
- } +
+
+
+ -
-
- )} + } + tooltipText={ +
+
Ready for acknowledge
+
+ } + /> +
- +
); } + + const dueDateStr = triaged + ? alertSummary.bug_due_date + : alertSummary.triage_due_date; + + const now = new Date(Date.now()); + const label = getCountdownLabel(now, new Date(dueDateStr)); + const heading = triaged ? 'Bug Due' : 'Triage Due'; + + return ( +
+
+
+ + } + tooltipText={ +
+
+
+ {heading}: {formatDate(dueDateStr)} +
+

Time left: {label}

+
+
+ } + /> +
+
+
+ ); } AlertStatusCountdown.propTypes = { diff --git a/ui/perfherder/perf-helpers/alertCountdownHelper.js b/ui/perfherder/perf-helpers/alertCountdownHelper.js index 5b5bcb87490..ac7574cba50 100644 --- a/ui/perfherder/perf-helpers/alertCountdownHelper.js +++ b/ui/perfherder/perf-helpers/alertCountdownHelper.js @@ -1,66 +1,47 @@ -import { weekdays } from './constants'; +const MS_PER_HOUR = 1000 * 60 * 60; +const MS_PER_DAY = MS_PER_HOUR * 24; export const isWeekend = () => { - const currentDate = new Date(Date.now()); - const currentDay = currentDate.getDay(); - - return currentDay === weekdays.saturday || currentDay === weekdays.sunday; + const day = new Date(Date.now()).getDay(); + return day === 0 || day === 6; }; -export const getTimeDifference = (currentDate, dueDate) => { - const timeDifference = Math.abs(dueDate - currentDate); - let differenceInDays = Math.ceil(timeDifference / (1000 * 60 * 60 * 24)); - // saturday and sunday are considered weekend days - const weekendDaysCount = 2; - // convert days to weeks and keeps the count of full weeks - const weeksCount = Math.trunc(differenceInDays / 7); - // count the total weekend days - const weekendDaysToSubstract = weekendDaysCount * weeksCount; - const currentDay = currentDate.getUTCDay(); - const dueDay = dueDate.getUTCDay(); - - // [1. Mon] [2. Tue] [3. Wed] [4. Thu] [5. Fri] [6. Sat] [7. Sun] - // currentDate -------- -------- ----->>> - // -------- -------> dueDate - if (currentDay > dueDay) { - // If weekday of startDate is bigger than endDate then it is a new week, - // and we have to subtract the weekend days - differenceInDays -= weekendDaysCount; - } else { - // substracts the total weekend days from the total days - differenceInDays -= weekendDaysToSubstract; +export const alertIsTriaged = (alertSummary) => !!alertSummary.first_triaged; +export const alertIsLinkedToBug = (alertSummary) => !!alertSummary.bug_number; + +// Before: two exported functions, getTimeDifference() and getCountdownText(), that +// callers had to invoke in sequence and pass the intermediate { hours, days } object +// between them. getTimeDifference used a week-counting heuristic (full weeks × 2 +// weekend days) that gave wrong results when the date range crossed a partial week. +// getCountdownText also checked "hours left" before "Overdue", so an alert that was +// overdue by less than 24 h would display "0 hours left" instead of "Overdue". +// +// After: workingMsBetween() is a private helper that steps day-by-day to count +// weekend days precisely, and getCountdownLabel() is the single public entry point +// that returns the final display string in one call. +const workingMsBetween = (now, dueDate) => { + const totalMs = dueDate.getTime() - now.getTime(); + if (totalMs <= 0) return 0; + + let weekendDayCount = 0; + let cursor = new Date(now.getTime()); + while (cursor < dueDate) { + const day = cursor.getUTCDay(); + if (day === 0 || day === 6) weekendDayCount++; + cursor.setTime(cursor.getTime() + MS_PER_DAY); } - let hoursDifference = Math.ceil(timeDifference / (1000 * 60 * 60)); - const shouldGetHoursLeft = - currentDay === weekdays.friday && dueDay === weekdays.monday; - // If due date is Monday and today is Friday and we have to show the hours left, - // we have to subtract the weekend hours from the difference - if (shouldGetHoursLeft) { - hoursDifference -= 2 * 24; - } - return { - hours: hoursDifference, - days: differenceInDays, - }; + return Math.max(0, totalMs - weekendDayCount * MS_PER_DAY); }; -export const getCountdownText = (now, dueDate, difference) => { - if (difference.hours < 24 && difference.hours >= 0) { - return `${difference.hours} hours left`; - } +export const getCountdownLabel = (now, dueDate) => { + // Overdue check must come first — the old code checked "hours left" before + // "Overdue", causing an alert past its due date by <24 h to show "0 hours left". + if (now >= dueDate) return 'Overdue'; - if (now.getTime() >= dueDate.getTime()) { - return `Overdue`; + const workingMs = workingMsBetween(now, dueDate); + if (workingMs < MS_PER_DAY) { + return `${Math.ceil(workingMs / MS_PER_HOUR)} hours left`; } - - return `${difference.days} days left`; -}; - -export const alertIsTriaged = (alertSummary) => { - return !!alertSummary.first_triaged; -}; - -export const alertIsLinkedToBug = (alertSummary) => { - return !!alertSummary.bug_number; + return `${Math.ceil(workingMs / MS_PER_DAY)} days left`; };