Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Reporting] Fix job completion notification to disappear after 24 hours #133381

Merged
merged 1 commit into from
Jun 2, 2022
Merged

[Reporting] Fix job completion notification to disappear after 24 hours #133381

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 40 additions & 29 deletions x-pack/plugins/reporting/public/lib/__snapshots__/stream_handler.test.ts.snap
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions x-pack/plugins/reporting/public/lib/stream_handler.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
* 2.0.
*/

import { omit } from 'lodash';
import sinon, { stub } from 'sinon';
import { NotificationsStart } from '@kbn/core/public';
import { coreMock, themeServiceMock, docLinksServiceMock } from '@kbn/core/public/mocks';
Expand Down Expand Up @@ -124,7 +123,7 @@ describe('stream handler', () => {
expect(mockShowDanger.callCount).toBe(0);
expect(mockShowSuccess.callCount).toBe(1);
expect(mockShowWarning.callCount).toBe(0);
expect(omit(mockShowSuccess.args[0][0], 'toastLifeTimeMs')).toMatchSnapshot();
expect(mockShowSuccess.args[0]).toMatchSnapshot();
done();
});
});
Expand Down
20 changes: 16 additions & 4 deletions x-pack/plugins/reporting/public/lib/stream_handler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,12 @@ import {
import { Job } from './job';
import { ReportingAPIClient } from './reporting_api_client';

/**
* @todo Replace with `Infinity` once elastic/eui#5945 is resolved.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this comment outdated? There is no Infinity in the lower code

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it isn't. I think we should replace this value with Infinity once it's supported.

* @see https://github.com/elastic/eui/issues/5945
*/
const COMPLETED_JOB_TOAST_TIMEOUT = 24 * 60 * 60 * 1000; // 24 hours

function updateStored(jobIds: JobId[]): void {
sessionStorage.setItem(JOB_COMPLETION_NOTIFICATIONS_SESSION_KEY, JSON.stringify(jobIds));
}
Expand Down Expand Up @@ -54,6 +60,8 @@ export class ReportingNotifierStreamHandler {
failed: failedJobs,
}: JobSummarySet): Rx.Observable<JobSummarySet> {
const showNotificationsAsync = async () => {
const completedOptions = { toastLifeTimeMs: COMPLETED_JOB_TOAST_TIMEOUT };

// notifications with download link
for (const job of completedJobs) {
if (job.csvContainsFormulas) {
Expand All @@ -63,7 +71,8 @@ export class ReportingNotifierStreamHandler {
this.apiClient.getManagementLink,
this.apiClient.getDownloadLink,
this.theme
)
),
completedOptions
);
} else if (job.maxSizeReached) {
this.notifications.toasts.addWarning(
Expand All @@ -72,7 +81,8 @@ export class ReportingNotifierStreamHandler {
this.apiClient.getManagementLink,
this.apiClient.getDownloadLink,
this.theme
)
),
completedOptions
);
} else if (job.status === JOB_STATUSES.WARNINGS) {
this.notifications.toasts.addWarning(
Expand All @@ -81,7 +91,8 @@ export class ReportingNotifierStreamHandler {
this.apiClient.getManagementLink,
this.apiClient.getDownloadLink,
this.theme
)
),
completedOptions
);
} else {
this.notifications.toasts.addSuccess(
Expand All @@ -90,7 +101,8 @@ export class ReportingNotifierStreamHandler {
this.apiClient.getManagementLink,
this.apiClient.getDownloadLink,
this.theme
)
),
completedOptions
);
}
}
Expand Down
7 changes: 0 additions & 7 deletions x-pack/plugins/reporting/public/notifier/job_success.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,5 @@ export const getSuccessToast = (
</>,
{ theme$: theme.theme$ }
),
/**
* If timeout is an Infinity value, a Not-a-Number (NaN) value, or negative, then timeout will be zero.
* And we cannot use `Number.MAX_SAFE_INTEGER` because EUI's Timer implementation
* subtracts it from the current time to evaluate the remainder.
* @see https://www.w3.org/TR/2011/WD-html5-20110525/timers.html
*/
toastLifeTimeMs: Number.MAX_SAFE_INTEGER - Date.now(),
'data-test-subj': 'completeReportSuccess',
});