From cbc0144309e5af5f0dbf4ecb191ff7d316980d99 Mon Sep 17 00:00:00 2001 From: shamoon <4887959+shamoon@users.noreply.github.com> Date: Sat, 14 Nov 2026 00:51:44 -0800 Subject: [PATCH] We need a tasks notification service --- .../app-frame/app-frame.component.spec.ts | 45 ++++-- .../app-frame/app-frame.component.ts | 20 +-- .../common/toast/toast.component.scss | 8 + .../common/toast/toast.component.spec.ts | 30 ++++ .../common/toast/toast.component.ts | 10 +- .../common/toasts/toasts.component.spec.ts | 4 +- .../common/toasts/toasts.component.ts | 9 +- .../document-detail.component.spec.ts | 14 ++ .../document-detail.component.ts | 48 +++--- ...ocument-version-dropdown.component.spec.ts | 3 + .../document-version-dropdown.component.ts | 30 +--- .../bulk-editor/bulk-editor.component.spec.ts | 7 + .../bulk-editor/bulk-editor.component.ts | 42 +++-- .../task-notification.service.spec.ts | 145 ++++++++++++++++++ .../app/services/task-notification.service.ts | 110 +++++++++++++ src-ui/src/app/services/toast.service.spec.ts | 23 +++ src-ui/src/app/services/toast.service.ts | 9 +- src-ui/src/app/services/trash.service.spec.ts | 38 ++++- src-ui/src/app/services/trash.service.ts | 65 +++----- 19 files changed, 498 insertions(+), 162 deletions(-) create mode 100644 src-ui/src/app/services/task-notification.service.spec.ts create mode 100644 src-ui/src/app/services/task-notification.service.ts diff --git a/src-ui/src/app/components/app-frame/app-frame.component.spec.ts b/src-ui/src/app/components/app-frame/app-frame.component.spec.ts index 80da6375e3..e70442c913 100644 --- a/src-ui/src/app/components/app-frame/app-frame.component.spec.ts +++ b/src-ui/src/app/components/app-frame/app-frame.component.spec.ts @@ -33,9 +33,9 @@ import { RemoteVersionService } from 'src/app/services/rest/remote-version.servi import { SavedViewService } from 'src/app/services/rest/saved-view.service' import { SearchService } from 'src/app/services/rest/search.service' import { SettingsService } from 'src/app/services/settings.service' +import { TaskNotificationService } from 'src/app/services/task-notification.service' import { TasksService } from 'src/app/services/tasks.service' import { ToastService } from 'src/app/services/toast.service' -import { TrashService } from 'src/app/services/trash.service' import { UploadDocumentsService } from 'src/app/services/upload-documents.service' import { WebsocketStatusService } from 'src/app/services/websocket-status.service' import { environment } from 'src/environments/environment' @@ -199,14 +199,14 @@ describe('AppFrameComponent', () => { fixture.detectChanges() }) - it('refreshes and notifies on completion without client-side task tracking', () => { + it('refreshes and updates the task notification on completion', () => { const list = TestBed.inject(DocumentListViewService) const reload = jest.spyOn(list, 'reload').mockImplementation(() => {}) jest.spyOn(list, 'reduceSelectionToFilter').mockImplementation(() => {}) const tasksReload = jest .spyOn(tasksService, 'reload') .mockImplementation(() => {}) - const success = jest.spyOn(toastService, 'showSuccess') + const success = jest.spyOn(toastService, 'show') const refresh = jest .spyOn(openDocumentsService, 'refreshDocument') .mockImplementation(() => {}) @@ -231,7 +231,12 @@ describe('AppFrameComponent', () => { expect(refresh).toHaveBeenCalledWith(document.id) expect(maybeRefreshSpy).toHaveBeenCalledTimes(1) expect(tasksReload).toHaveBeenCalledTimes(1) - expect(success).toHaveBeenCalledWith('Operation completed.') + expect(success).toHaveBeenCalledWith( + expect.objectContaining({ + id: 'task-bulk-task', + content: 'Operation completed.', + }) + ) fixture.destroy() bulkEvents.next({ task_id: 'bulk-task', @@ -246,14 +251,14 @@ describe('AppFrameComponent', () => { const reload = jest.spyOn(list, 'reload').mockImplementation(() => {}) jest.spyOn(list, 'reduceSelectionToFilter').mockImplementation(() => {}) jest.spyOn(tasksService, 'reload').mockImplementation(() => {}) - const error = jest.spyOn(toastService, 'showError') + const error = jest.spyOn(toastService, 'show') const success = jest.spyOn(toastService, 'showSuccess') for (const status of [ PaperlessTaskStatus.Failure, PaperlessTaskStatus.Revoked, ]) { bulkEvents.next({ - task_id: 'bulk-task', + task_id: `${status}-task`, operation: 'set_permissions', status, error: @@ -264,29 +269,35 @@ describe('AppFrameComponent', () => { } expect(reload).toHaveBeenCalledTimes(2) expect(error).toHaveBeenCalledWith( - 'Operation failed. Some changes may already have been applied.', - 'Could not update' + expect.objectContaining({ + content: + 'Operation failed. Some changes may already have been applied.', + error: 'Could not update', + }) ) expect(error).toHaveBeenCalledWith( - 'Operation revoked.', - 'Task expired before execution.' + expect.objectContaining({ + content: 'Operation revoked.', + error: 'Task expired before execution.', + }) ) expect(success).not.toHaveBeenCalled() }) - it('defers the delete success toast to TrashService', () => { + it('passes delete completion to the shared task notification handler', () => { const success = jest.spyOn(toastService, 'showSuccess') - const trashService = TestBed.inject(TrashService) - const handleStatus = jest.spyOn(trashService, 'handleDeleteTaskStatus') + const notifications = TestBed.inject(TaskNotificationService) + const handleStatus = jest.spyOn(notifications, 'complete') bulkEvents.next({ task_id: 'delete-1', operation: 'delete', status: PaperlessTaskStatus.Success, }) - expect(handleStatus).toHaveBeenCalledWith( - 'delete-1', - PaperlessTaskStatus.Success - ) + expect(handleStatus).toHaveBeenCalledWith({ + task_id: 'delete-1', + operation: 'delete', + status: PaperlessTaskStatus.Success, + }) expect(success).not.toHaveBeenCalled() }) diff --git a/src-ui/src/app/components/app-frame/app-frame.component.ts b/src-ui/src/app/components/app-frame/app-frame.component.ts index abb454afe9..0e672db328 100644 --- a/src-ui/src/app/components/app-frame/app-frame.component.ts +++ b/src-ui/src/app/components/app-frame/app-frame.component.ts @@ -57,9 +57,9 @@ import { } from 'src/app/services/rest/remote-version.service' import { SavedViewService } from 'src/app/services/rest/saved-view.service' import { SettingsService } from 'src/app/services/settings.service' +import { TaskNotificationService } from 'src/app/services/task-notification.service' import { TasksService } from 'src/app/services/tasks.service' import { ToastService } from 'src/app/services/toast.service' -import { TrashService } from 'src/app/services/trash.service' import { UploadDocumentsService } from 'src/app/services/upload-documents.service' import { WebsocketStatusService } from 'src/app/services/websocket-status.service' import { environment } from 'src/environments/environment' @@ -116,7 +116,7 @@ export class AppFrameComponent settingsService = inject(SettingsService) tasksService = inject(TasksService) private readonly toastService = inject(ToastService) - private readonly trashService = inject(TrashService) + private readonly taskNotifications = inject(TaskNotificationService) private modalService = inject(NgbModal) permissionsService = inject(PermissionsService) private djangoMessagesService = inject(DjangoMessagesService) @@ -184,7 +184,7 @@ export class AppFrameComponent ), takeUntilDestroyed(this.destroyRef) ) - .subscribe(({ task_id, operation, status, error }) => { + .subscribe((result) => { this.documentList.reload(null, false) this.documentList.reduceSelectionToFilter() // maybe someday we can optimize to only refresh affected documents @@ -200,19 +200,7 @@ export class AppFrameComponent ) { this.tasksService.reload() } - if (operation === 'delete') { - this.trashService.handleDeleteTaskStatus(task_id, status) - } - if (status === PaperlessTaskStatus.Success && operation !== 'delete') { - this.toastService.showSuccess($localize`Operation completed.`) - } else if (status === PaperlessTaskStatus.Revoked) { - this.toastService.showError($localize`Operation revoked.`, error) - } else if (status === PaperlessTaskStatus.Failure) { - this.toastService.showError( - $localize`Operation failed. Some changes may already have been applied.`, - error - ) - } + this.taskNotifications.complete(result) }) this.lastScrollY = window.scrollY diff --git a/src-ui/src/app/components/common/toast/toast.component.scss b/src-ui/src/app/components/common/toast/toast.component.scss index 47df1a1583..7ae26a5cae 100644 --- a/src-ui/src/app/components/common/toast/toast.component.scss +++ b/src-ui/src/app/components/common/toast/toast.component.scss @@ -7,6 +7,7 @@ background-color: var(--pngx-toast-bg); color: var(--bs-body-color); overflow: hidden; + transition: background-color .18s ease, border-color .18s ease; &.toast-success { --pngx-toast-accent: var(--pngx-toast-success); @@ -23,6 +24,13 @@ .toast-icon { color: var(--pngx-toast-accent); + transition: color .18s ease; +} + +@media (prefers-reduced-motion: reduce) { + .toast, .toast-icon { + transition: none; + } } .toast-content { diff --git a/src-ui/src/app/components/common/toast/toast.component.spec.ts b/src-ui/src/app/components/common/toast/toast.component.spec.ts index 63e58b0293..10e3573ad9 100644 --- a/src-ui/src/app/components/common/toast/toast.component.spec.ts +++ b/src-ui/src/app/components/common/toast/toast.component.spec.ts @@ -65,6 +65,36 @@ describe('ToastComponent', () => { expect(component.delayRemaining()).toEqual(3000) }) + it('updates its color and icon in place and restarts the countdown', () => { + jest.useFakeTimers() + fixture.componentRef.setInput('toast', { + id: 'task-1', + content: 'Queued', + delay: 5000, + }) + fixture.detectChanges() + component.onShown(component.toast) + jest.advanceTimersByTime(4500) + const element = fixture.nativeElement.querySelector('ngb-toast') + fixture.componentRef.setInput('toast', { + id: 'task-1', + content: 'Completed', + delay: 5000, + level: ToastLevel.Success, + }) + fixture.detectChanges() + expect(fixture.nativeElement.querySelector('ngb-toast')).toBe(element) + expect(element.classList).toContain('toast-success') + expect(element.querySelector('[name="check-circle"]')).not.toBeNull() + expect(component.delayRemaining()).toBe(5000) + const hide = jest.spyOn(component.ngbToast, 'hide') + jest.advanceTimersByTime(1000) + expect(hide).not.toHaveBeenCalled() + jest.advanceTimersByTime(4200) + expect(hide).toHaveBeenCalled() + jest.useRealTimers() + }) + it('should hide the toast when the countdown ends', () => { jest.useFakeTimers() const toast = { content: 'Info', delay: 3000 } diff --git a/src-ui/src/app/components/common/toast/toast.component.ts b/src-ui/src/app/components/common/toast/toast.component.ts index 27bee856cf..6f251f6277 100644 --- a/src-ui/src/app/components/common/toast/toast.component.ts +++ b/src-ui/src/app/components/common/toast/toast.component.ts @@ -5,8 +5,10 @@ import { DestroyRef, EventEmitter, Input, + OnChanges, OnInit, Output, + SimpleChanges, ViewChild, inject, signal, @@ -34,7 +36,7 @@ import { Toast, ToastLevel } from 'src/app/services/toast.service' styleUrl: './toast.component.scss', providers: [DatePipe, CustomDatePipe], }) -export class ToastComponent implements OnInit { +export class ToastComponent implements OnInit, OnChanges { private clipboard = inject(Clipboard) private customDatePipe = inject(CustomDatePipe) private destroyRef = inject(DestroyRef) @@ -69,6 +71,12 @@ export class ToastComponent implements OnInit { if (this.autohide) this.delayRemaining.set(this.toast.delay) } + ngOnChanges(changes: SimpleChanges): void { + if (changes.toast && !changes.toast.firstChange && this.autohide) { + this.onShown(this.toast) + } + } + get relativeTimestamp(): string { return this.customDatePipe.transform(this.toast.timestamp, 'relative') } diff --git a/src-ui/src/app/components/common/toasts/toasts.component.spec.ts b/src-ui/src/app/components/common/toasts/toasts.component.spec.ts index baa1fe15f4..8c034a51f0 100644 --- a/src-ui/src/app/components/common/toasts/toasts.component.spec.ts +++ b/src-ui/src/app/components/common/toasts/toasts.component.spec.ts @@ -68,9 +68,9 @@ describe('ToastsComponent', () => { toasts.forEach((t) => toastSubject.next(t)) expect(component.toasts().map((t) => t.id)).toEqual(['3', '2', '1']) - // showing a visible toast again moves it to the top without duplicating + // Updating a visible toast preserves its position without duplicating it. toastSubject.next(toasts[1]) - expect(component.toasts().map((t) => t.id)).toEqual(['1', '3', '2']) + expect(component.toasts().map((t) => t.id)).toEqual(['3', '2', '1']) }) it('should remove a popup when it hides without closing it', () => { diff --git a/src-ui/src/app/components/common/toasts/toasts.component.ts b/src-ui/src/app/components/common/toasts/toasts.component.ts index de383ca790..8496f75665 100644 --- a/src-ui/src/app/components/common/toasts/toasts.component.ts +++ b/src-ui/src/app/components/common/toasts/toasts.component.ts @@ -32,10 +32,11 @@ export class ToastsComponent { return } this.toasts.update((toasts) => - [toast, ...toasts.filter((t) => t.id !== toast.id)].slice( - 0, - MAX_VISIBLE_TOASTS - ) + toasts.some((existing) => existing.id === toast.id) + ? toasts.map((existing) => + existing.id === toast.id ? toast : existing + ) + : [toast, ...toasts].slice(0, MAX_VISIBLE_TOASTS) ) }) } diff --git a/src-ui/src/app/components/document-detail/document-detail.component.spec.ts b/src-ui/src/app/components/document-detail/document-detail.component.spec.ts index 19beede47e..784a100e1b 100644 --- a/src-ui/src/app/components/document-detail/document-detail.component.spec.ts +++ b/src-ui/src/app/components/document-detail/document-detail.component.spec.ts @@ -1014,6 +1014,7 @@ describe('DocumentDetailComponent', () => { }) it('should queue reprocess, show the task link and close the modal', () => { + jest.useFakeTimers() initNormally() const reprocessSpy = jest.spyOn(documentService, 'reprocessDocuments') reprocessSpy.mockReturnValue(of({ result: 'OK', task_id: 'reprocess-1' })) @@ -1026,6 +1027,9 @@ describe('DocumentDetailComponent', () => { openModal.componentInstance.confirmClicked.next() expect(reprocessSpy).toHaveBeenCalledWith({ documents: [doc.id] }, false) expect(modalSpy).toHaveBeenCalled() + expect(toastSpy).not.toHaveBeenCalled() + expect(modalCloseSpy).toHaveBeenCalled() + jest.advanceTimersByTime(400) expect(toastSpy).toHaveBeenCalledWith( expect.objectContaining({ content: 'Document reprocessing queued.', @@ -1037,9 +1041,11 @@ describe('DocumentDetailComponent', () => { queryParams: { task_id: 'reprocess-1' }, }) expect(modalCloseSpy).toHaveBeenCalled() + jest.useRealTimers() }) it('should run a workflow on the document, confirm and close modal', () => { + jest.useFakeTimers() initNormally() const bulkEditSpy = jest.spyOn(documentService, 'bulkEdit') bulkEditSpy.mockReturnValue(of({ result: 'OK', task_id: 'workflow-1' })) @@ -1057,10 +1063,12 @@ describe('DocumentDetailComponent', () => { 'run_workflow', { workflow: 5, filter_documents: false } ) + jest.advanceTimersByTime(400) expect(toastSpy).toHaveBeenCalledWith( expect.objectContaining({ content: 'Workflow queued.' }) ) expect(modalCloseSpy).toHaveBeenCalled() + jest.useRealTimers() }) it('should apply workflow filters when the single-document checkbox is cleared', () => { @@ -2012,6 +2020,7 @@ describe('DocumentDetailComponent', () => { }) it('should support pdf editor, handle error', () => { + jest.useFakeTimers() let modal: NgbModalRef modalService.activeInstances.subscribe((m) => (modal = m[0])) const closeSpy = jest.spyOn(openDocumentsService, 'closeDocument') @@ -2049,13 +2058,16 @@ describe('DocumentDetailComponent', () => { `${environment.apiBaseUrl}documents/edit_pdf/` ) req.flush({ result: 'OK', task_id: 'edit-pdf-1' }) + jest.advanceTimersByTime(400) expect(closeSpy).not.toHaveBeenCalled() expect(queuedToastSpy).toHaveBeenCalledWith( expect.objectContaining({ content: 'PDF edit queued.' }) ) + jest.useRealTimers() }) it('should support removing password protection from pdfs', () => { + jest.useFakeTimers() let modal: NgbModalRef modalService.activeInstances.subscribe((m) => (modal = m[0])) const closeSpy = jest.spyOn(openDocumentsService, 'closeDocument') @@ -2083,10 +2095,12 @@ describe('DocumentDetailComponent', () => { from_webui: true, }) req.flush({ result: 'OK', task_id: 'remove-password-1' }) + jest.advanceTimersByTime(400) expect(closeSpy).not.toHaveBeenCalled() expect(queuedToastSpy).toHaveBeenCalledWith( expect.objectContaining({ content: 'Password removal queued.' }) ) + jest.useRealTimers() }) it('should require the current password before removing it', () => { diff --git a/src-ui/src/app/components/document-detail/document-detail.component.ts b/src-ui/src/app/components/document-detail/document-detail.component.ts index 8f897d7acb..f6aa0052d8 100644 --- a/src-ui/src/app/components/document-detail/document-detail.component.ts +++ b/src-ui/src/app/components/document-detail/document-detail.component.ts @@ -97,6 +97,7 @@ import { TagService } from 'src/app/services/rest/tag.service' import { UserService } from 'src/app/services/rest/user.service' import { WorkflowService } from 'src/app/services/rest/workflow.service' import { SettingsService } from 'src/app/services/settings.service' +import { TaskNotificationService } from 'src/app/services/task-notification.service' import { ToastService } from 'src/app/services/toast.service' import { TrashService } from 'src/app/services/trash.service' import { WebsocketStatusService } from 'src/app/services/websocket-status.service' @@ -226,6 +227,7 @@ export class DocumentDetailComponent private documentListViewService = inject(DocumentListViewService) private documentTitlePipe = inject(DocumentTitlePipe) private toastService = inject(ToastService) + private taskNotifications = inject(TaskNotificationService) private trashService = inject(TrashService) private settings = inject(SettingsService) private storagePathService = inject(StoragePathService) @@ -1559,9 +1561,10 @@ export class DocumentDetailComponent ) .subscribe({ next: ({ task_id }) => { - this.notifyTaskQueued( + this.taskNotifications.showQueued( task_id, - $localize`Document reprocessing queued.` + $localize`Document reprocessing queued.`, + { content: $localize`Document reprocessing completed.` } ) if (modal) { modal.close() @@ -1599,7 +1602,13 @@ export class DocumentDetailComponent }) .subscribe({ next: ({ task_id }) => { - this.notifyTaskQueued(task_id, $localize`Workflow queued.`) + this.taskNotifications.showQueued( + task_id, + $localize`Workflow queued.`, + { + content: $localize`Workflow completed.`, + } + ) modal.close() }, error: (error) => { @@ -1613,26 +1622,6 @@ export class DocumentDetailComponent }) } - private notifyTaskQueued(taskId: string, content: string) { - const canViewTasks = this.permissionsService.currentUserCan( - PermissionAction.View, - PermissionType.PaperlessTask - ) - this.toastService.show({ - content, - delay: 5000, - ...(canViewTasks - ? { - actionName: $localize`View task`, - action: () => - this.router.navigate(['/tasks'], { - queryParams: { task_id: taskId }, - }), - } - : {}), - }) - } - private getSelectedNonLatestVersionId(): number | null { const versions = this.document()?.versions ?? [] if (!versions.length || !this.selectedVersionId()) { @@ -2009,7 +1998,13 @@ export class DocumentDetailComponent .pipe(first(), takeUntil(this.unsubscribeNotifier)) .subscribe({ next: ({ task_id }) => { - this.notifyTaskQueued(task_id, $localize`PDF edit queued.`) + this.taskNotifications.showQueued( + task_id, + $localize`PDF edit queued.`, + { + content: $localize`PDF edit completed.`, + } + ) modal.close() }, error: (error) => { @@ -2061,9 +2056,10 @@ export class DocumentDetailComponent .pipe(first(), takeUntil(this.unsubscribeNotifier)) .subscribe({ next: ({ task_id }) => { - this.notifyTaskQueued( + this.taskNotifications.showQueued( task_id, - $localize`Password removal queued.` + $localize`Password removal queued.`, + { content: $localize`Password removal completed.` } ) this.networkActive.set(false) modal.close() diff --git a/src-ui/src/app/components/document-detail/document-version-dropdown/document-version-dropdown.component.spec.ts b/src-ui/src/app/components/document-detail/document-version-dropdown/document-version-dropdown.component.spec.ts index a5a9b37836..88b9faa590 100644 --- a/src-ui/src/app/components/document-detail/document-version-dropdown/document-version-dropdown.component.spec.ts +++ b/src-ui/src/app/components/document-detail/document-version-dropdown/document-version-dropdown.component.spec.ts @@ -384,6 +384,7 @@ describe('DocumentVersionDropdownComponent', () => { }) it('addExistingDocumentAsVersion should queue with a label and wait to refresh versions', () => { + jest.useFakeTimers() const confirmClicked = new Subject() const modal = { componentInstance: { @@ -416,11 +417,13 @@ describe('DocumentVersionDropdownComponent', () => { expect(selectedEmitSpy).not.toHaveBeenCalled() expect(component.newVersionLabel).toEqual('') expect(modal.close).toHaveBeenCalled() + jest.advanceTimersByTime(400) expect(toastService.show).toHaveBeenCalledWith( expect.objectContaining({ content: 'Existing document queued to be added as a version.', actionName: 'View task', }) ) + jest.useRealTimers() }) }) diff --git a/src-ui/src/app/components/document-detail/document-version-dropdown/document-version-dropdown.component.ts b/src-ui/src/app/components/document-detail/document-version-dropdown/document-version-dropdown.component.ts index 27507ef7e4..c6d578c9d6 100644 --- a/src-ui/src/app/components/document-detail/document-version-dropdown/document-version-dropdown.component.ts +++ b/src-ui/src/app/components/document-detail/document-version-dropdown/document-version-dropdown.component.ts @@ -11,7 +11,6 @@ import { SimpleChanges, } from '@angular/core' import { FormsModule } from '@angular/forms' -import { Router } from '@angular/router' import { NgbDropdownModule, NgbModal } from '@ng-bootstrap/ng-bootstrap' import { merge, of, Subject } from 'rxjs' import { @@ -28,12 +27,8 @@ import { BootstrapIconComponent } from 'src/app/components/common/bootstrap-icon import { DocumentVersionInfo } from 'src/app/data/document' import { IfPermissionsDirective } from 'src/app/directives/if-permissions.directive' import { CustomDatePipe } from 'src/app/pipes/custom-date.pipe' -import { - PermissionAction, - PermissionsService, - PermissionType, -} from 'src/app/services/permissions.service' import { DocumentService } from 'src/app/services/rest/document.service' +import { TaskNotificationService } from 'src/app/services/task-notification.service' import { ToastService } from 'src/app/services/toast.service' import { UploadState, @@ -83,8 +78,7 @@ export class DocumentVersionDropdownComponent private readonly toastService = inject(ToastService) private readonly websocketStatusService = inject(WebsocketStatusService) private readonly modalService = inject(NgbModal) - private readonly router = inject(Router) - private readonly permissionsService = inject(PermissionsService) + private readonly taskNotifications = inject(TaskNotificationService) private readonly destroy$ = new Subject() private readonly documentChange$ = new Subject() @@ -322,23 +316,11 @@ export class DocumentVersionDropdownComponent next: ({ task_id }) => { this.newVersionLabel = '' modal.close() - const canViewTasks = this.permissionsService.currentUserCan( - PermissionAction.View, - PermissionType.PaperlessTask + this.taskNotifications.showQueued( + task_id, + $localize`Existing document queued to be added as a version.`, + { content: $localize`Existing document added as a version.` } ) - this.toastService.show({ - content: $localize`Existing document queued to be added as a version.`, - delay: 5000, - ...(canViewTasks - ? { - actionName: $localize`View task`, - action: () => - this.router.navigate(['/tasks'], { - queryParams: { task_id }, - }), - } - : {}), - }) }, error: (error) => { this.toastService.showError( diff --git a/src-ui/src/app/components/document-list/bulk-editor/bulk-editor.component.spec.ts b/src-ui/src/app/components/document-list/bulk-editor/bulk-editor.component.spec.ts index d39705c416..e1198faed3 100644 --- a/src-ui/src/app/components/document-list/bulk-editor/bulk-editor.component.spec.ts +++ b/src-ui/src/app/components/document-list/bulk-editor/bulk-editor.component.spec.ts @@ -1482,6 +1482,7 @@ describe('BulkEditorComponent', () => { }) it('should support merging documents as versions', () => { + jest.useFakeTimers() let modal: NgbModalRef modalService.activeInstances.subscribe((m) => (modal = m[0])) jest.spyOn(permissionsService, 'currentUserCan').mockReturnValue(true) @@ -1526,9 +1527,11 @@ describe('BulkEditorComponent', () => { `${environment.apiBaseUrl}documents/?page=1&page_size=100000&fields=id` ) expect(documentListViewService.selected.size).toEqual(0) + jest.advanceTimersByTime(400) expect(toastSpy).toHaveBeenCalledWith( expect.objectContaining({ content: 'Bulk operation queued.' }) ) + jest.useRealTimers() }) it('should not report success when merging documents as versions fails', () => { @@ -1622,6 +1625,7 @@ describe('BulkEditorComponent', () => { it.each([true, false])( 'shows queued feedback and gates the task link on permission (%s)', (canViewTasks) => { + jest.useFakeTimers() jest .spyOn(permissionsService, 'currentUserCan') .mockReturnValue(canViewTasks) @@ -1642,6 +1646,8 @@ describe('BulkEditorComponent', () => { ) req.flush({ result: 'OK', task_id: 'queued-operation' }) expect(close).toHaveBeenCalledTimes(1) + expect(queued).not.toHaveBeenCalled() + jest.advanceTimersByTime(400) const notification = queued.mock.calls.at(-1)[0] expect(notification.content).toBe('Bulk operation queued.') if (canViewTasks) { @@ -1657,6 +1663,7 @@ describe('BulkEditorComponent', () => { expect(reload).not.toHaveBeenCalled() expect(documentListViewService.selected.size).toBe(0) expect(documentListViewService.allSelected).toBe(false) + jest.useRealTimers() } ) diff --git a/src-ui/src/app/components/document-list/bulk-editor/bulk-editor.component.ts b/src-ui/src/app/components/document-list/bulk-editor/bulk-editor.component.ts index 3eea261f20..3f6f66876f 100644 --- a/src-ui/src/app/components/document-list/bulk-editor/bulk-editor.component.ts +++ b/src-ui/src/app/components/document-list/bulk-editor/bulk-editor.component.ts @@ -20,14 +20,15 @@ import { } from '@ng-bootstrap/ng-bootstrap' import { saveAs } from 'file-saver' import { first, map, Observable, Subject, switchMap, takeUntil } from 'rxjs' -import { BulkOperationResponse } from 'src/app/data/bulk-operation' import { BootstrapIconComponent } from 'src/app/components/common/bootstrap-icon/bootstrap-icon.component' import { ConfirmDialogComponent } from 'src/app/components/common/confirm-dialog/confirm-dialog.component' +import { BulkOperationResponse } from 'src/app/data/bulk-operation' import { CustomField } from 'src/app/data/custom-field' import { MatchingModel } from 'src/app/data/matching-model' import { SelectionDataItem } from 'src/app/data/results' import { SETTINGS_KEYS } from 'src/app/data/ui-settings' import { Workflow } from 'src/app/data/workflow' +import { WorkflowTriggerType } from 'src/app/data/workflow-trigger' import { IfPermissionsDirective } from 'src/app/directives/if-permissions.directive' import { DocumentListViewService } from 'src/app/services/document-list-view.service' import { @@ -45,10 +46,11 @@ import { MergeDocumentsRequest, } from 'src/app/services/rest/document.service' import { ShareLinkBundleService } from 'src/app/services/rest/share-link-bundle.service' -import { WorkflowService } from 'src/app/services/rest/workflow.service' import { StoragePathService } from 'src/app/services/rest/storage-path.service' import { TagService } from 'src/app/services/rest/tag.service' +import { WorkflowService } from 'src/app/services/rest/workflow.service' import { SettingsService } from 'src/app/services/settings.service' +import { TaskNotificationService } from 'src/app/services/task-notification.service' import { ToastService } from 'src/app/services/toast.service' import { TrashService } from 'src/app/services/trash.service' import { flattenTags } from 'src/app/utils/flatten-tags' @@ -75,7 +77,6 @@ import { PermissionsDialogComponent } from '../../common/permissions-dialog/perm import { ShareLinkBundleDialogComponent } from '../../common/share-link-bundle-dialog/share-link-bundle-dialog.component' import { ComponentWithPermissions } from '../../with-permissions/with-permissions.component' import { CustomFieldsBulkEditDialogComponent } from './custom-fields-bulk-edit-dialog/custom-fields-bulk-edit-dialog.component' -import { WorkflowTriggerType } from 'src/app/data/workflow-trigger' @Component({ selector: 'pngx-bulk-editor', @@ -103,6 +104,7 @@ export class BulkEditorComponent private modalService = inject(NgbModal) private settings = inject(SettingsService) private toastService = inject(ToastService) + private taskNotifications = inject(TaskNotificationService) private storagePathService = inject(StoragePathService) private customFieldService = inject(CustomFieldsService) public readonly permissionService = inject(PermissionsService) @@ -336,26 +338,21 @@ export class BulkEditorComponent }) } - private handleOperationQueued(modal: NgbModalRef, taskId: string) { + private handleOperationQueued( + modal: NgbModalRef, + taskId: string, + onQueued?: (taskId: string) => void + ) { modal?.close() this.list.selectNone() - const canViewTasks = this.permissionService.currentUserCan( - PermissionAction.View, - PermissionType.PaperlessTask - ) - this.toastService.show({ - content: $localize`Bulk operation queued.`, - delay: 5000, - ...(canViewTasks - ? { - actionName: $localize`View task`, - action: () => - this.router.navigate(['/tasks'], { - queryParams: { task_id: taskId }, - }), - } - : {}), - }) + if (onQueued) { + onQueued(taskId) + } else { + this.taskNotifications.showQueued( + taskId, + $localize`Bulk operation queued.` + ) + } } private executeDocumentAction( @@ -368,8 +365,7 @@ export class BulkEditorComponent } request.pipe(first()).subscribe({ next: ({ task_id }) => { - this.handleOperationQueued(modal, task_id) - onQueued?.(task_id) + this.handleOperationQueued(modal, task_id, onQueued) }, error: (error) => this.handleOperationError(modal, error), }) diff --git a/src-ui/src/app/services/task-notification.service.spec.ts b/src-ui/src/app/services/task-notification.service.spec.ts new file mode 100644 index 0000000000..2cd5b1c700 --- /dev/null +++ b/src-ui/src/app/services/task-notification.service.spec.ts @@ -0,0 +1,145 @@ +import { TestBed } from '@angular/core/testing' +import { Router } from '@angular/router' +import { BulkOperationStatus } from '../data/bulk-operation' +import { PaperlessTaskStatus } from '../data/paperless-task' +import { PermissionsService } from './permissions.service' +import { TaskNotificationService } from './task-notification.service' +import { Toast, ToastLevel, ToastService } from './toast.service' + +describe('TaskNotificationService', () => { + let service: TaskNotificationService + let toastService: ToastService + let popups: Toast[] + let notifications: Toast[] + const navigate = jest.fn() + const currentUserCan = jest.fn() + const result = ( + status = PaperlessTaskStatus.Success + ): BulkOperationStatus => ({ + task_id: 'task-1', + operation: 'run_workflow', + status, + }) + + beforeEach(() => { + jest.useFakeTimers() + currentUserCan.mockReturnValue(true) + TestBed.configureTestingModule({ + providers: [ + { provide: Router, useValue: { navigate } }, + { provide: PermissionsService, useValue: { currentUserCan } }, + ], + }) + service = TestBed.inject(TaskNotificationService) + toastService = TestBed.inject(ToastService) + popups = [] + notifications = [] + toastService.showToast.subscribe((toast) => popups.push(toast)) + toastService + .getToasts() + .subscribe((toasts) => (notifications = [...toasts])) + }) + + afterEach(() => { + jest.clearAllTimers() + jest.useRealTimers() + }) + + it('replaces a queued notification with its result, retaining the task link', () => { + service.showQueued('task-1', 'Workflow queued.', { + content: 'Workflow completed.', + }) + jest.advanceTimersByTime(399) + expect(popups).toHaveLength(0) + jest.advanceTimersByTime(1) + expect(popups[0].content).toBe('Workflow queued.') + service.complete(result()) + expect(notifications).toHaveLength(1) + expect(popups[1]).toEqual( + expect.objectContaining({ + id: popups[0].id, + content: 'Workflow completed.', + level: ToastLevel.Success, + actionName: 'View task', + }) + ) + popups[1].action() + expect(navigate).toHaveBeenCalledWith(['/tasks'], { + queryParams: { task_id: 'task-1' }, + }) + }) + + it('shows only the result when a task completes during the grace period', () => { + service.showQueued('task-1', 'Queued') + jest.advanceTimersByTime(100) + service.complete(result()) + jest.advanceTimersByTime(1000) + expect(popups.map((toast) => toast.content)).toEqual([ + 'Operation completed.', + ]) + }) + + it('does not show a stale queued state when completion beats the HTTP response', () => { + service.complete(result()) + service.showQueued('task-1', 'Workflow queued.', { + content: 'Workflow completed.', + }) + jest.advanceTimersByTime(1000) + expect(popups.every((toast) => toast.level === ToastLevel.Success)).toBe( + true + ) + expect(notifications).toHaveLength(1) + expect(notifications[0].content).toBe('Workflow completed.') + }) + + it.each([PaperlessTaskStatus.Failure, PaperlessTaskStatus.Revoked])( + 'replaces the queued state with an error for %s', + (status) => { + service.showQueued('task-1', 'Queued', { + content: 'Success', + actionName: 'Undo', + }) + jest.advanceTimersByTime(400) + service.complete({ ...result(status), error: 'Task failed' }) + expect(notifications).toHaveLength(1) + expect(notifications[0]).toEqual( + expect.objectContaining({ + level: ToastLevel.Error, + error: 'Task failed', + actionName: 'View task', + }) + ) + } + ) + + it('ignores nonterminal and duplicate outcomes', () => { + service.showQueued('task-1', 'Queued') + service.complete(result(PaperlessTaskStatus.Started)) + expect(popups).toHaveLength(0) + service.complete(result()) + service.complete(result()) + expect(popups).toHaveLength(1) + }) + + it('keeps concurrent task notifications separate', () => { + service.showQueued('task-1', 'First queued') + service.showQueued('task-2', 'Second queued') + jest.advanceTimersByTime(400) + service.complete(result()) + expect(notifications).toHaveLength(2) + expect(notifications.map((toast) => toast.content)).toEqual([ + 'Second queued', + 'Operation completed.', + ]) + }) + + it('omits task links without task viewing permission', () => { + currentUserCan.mockReturnValue(false) + service.showQueued('task-1', 'Queued') + jest.advanceTimersByTime(400) + service.complete(result()) + expect(popups.every((toast) => !toast.action && !toast.actionName)).toBe( + true + ) + }) +}) diff --git a/src-ui/src/app/services/task-notification.service.ts b/src-ui/src/app/services/task-notification.service.ts new file mode 100644 index 0000000000..65eab3ba8b --- /dev/null +++ b/src-ui/src/app/services/task-notification.service.ts @@ -0,0 +1,110 @@ +import { inject, Injectable, OnDestroy } from '@angular/core' +import { Router } from '@angular/router' +import { BulkOperationStatus } from '../data/bulk-operation' +import { PaperlessTaskStatus } from '../data/paperless-task' +import { + PermissionAction, + PermissionsService, + PermissionType, +} from './permissions.service' +import { Toast, ToastLevel, ToastService } from './toast.service' + +interface PendingNotification { + success: Partial + timer: ReturnType +} + +@Injectable({ providedIn: 'root' }) +export class TaskNotificationService implements OnDestroy { + private toastService = inject(ToastService) + private router = inject(Router) + private permissionsService = inject(PermissionsService) + private pending = new Map() + private completed = new Map() + + ngOnDestroy() { + this.pending.forEach(({ timer }) => clearTimeout(timer)) + } + + showQueued(taskId: string, content: string, success: Partial = {}) { + const completed = this.completed.get(taskId) + if (completed) { + this.showResult(completed, success) + return + } + if (this.pending.has(taskId)) return + this.pending.set(taskId, { + success, + // short delay in case the task finishes super fast + timer: setTimeout(() => this.show(taskId, { content, delay: 5000 }), 400), + }) + } + + complete(result: BulkOperationStatus) { + if ( + ![ + PaperlessTaskStatus.Success, + PaperlessTaskStatus.Failure, + PaperlessTaskStatus.Revoked, + ].includes(result.status) || + this.completed.has(result.task_id) + ) + return + + this.completed.set(result.task_id, result) + // Retain recent outcomes in case task finishes quickly + if (this.completed.size > 100) { + this.completed.delete(this.completed.keys().next().value) + } + const pending = this.pending.get(result.task_id) + if (pending) { + clearTimeout(pending.timer) + this.pending.delete(result.task_id) + } + // Trash needs the caller's document IDs to offer Undo on success. + if ( + pending || + result.operation !== 'delete' || + result.status !== PaperlessTaskStatus.Success + ) { + this.showResult(result, pending?.success) + } + } + + private showResult( + result: BulkOperationStatus, + success: Partial = {} + ) { + const succeeded = result.status === PaperlessTaskStatus.Success + this.show(result.task_id, { + content: succeeded + ? $localize`Operation completed.` + : result.status === PaperlessTaskStatus.Revoked + ? $localize`Operation revoked.` + : $localize`Operation failed. Some changes may already have been applied.`, + delay: succeeded ? 5000 : 10000, + level: succeeded ? ToastLevel.Success : ToastLevel.Error, + ...(succeeded ? success : { error: result.error }), + }) + } + + private show(taskId: string, toast: Toast) { + const canViewTasks = this.permissionsService.currentUserCan( + PermissionAction.View, + PermissionType.PaperlessTask + ) + this.toastService.show({ + id: `task-${taskId}`, + ...(canViewTasks + ? { + actionName: $localize`View task`, + action: () => + this.router.navigate(['/tasks'], { + queryParams: { task_id: taskId }, + }), + } + : {}), + ...toast, + }) + } +} diff --git a/src-ui/src/app/services/toast.service.spec.ts b/src-ui/src/app/services/toast.service.spec.ts index 9ddb856203..10d8356a2a 100644 --- a/src-ui/src/app/services/toast.service.spec.ts +++ b/src-ui/src/app/services/toast.service.spec.ts @@ -38,6 +38,29 @@ describe('ToastService', () => { }) }) + it('updates a notification by ID without duplicating or moving it', () => { + let notifications: Toast[] = [] + toastService + .getToasts() + .subscribe((toasts) => (notifications = [...toasts])) + toastService.show({ id: 'task-1', content: 'Queued', delay: 5000 }) + toastService.show({ + id: 'other', + content: 'Another notification', + delay: 5000, + }) + toastService.show({ + id: 'task-1', + content: 'Completed', + delay: 5000, + level: ToastLevel.Success, + }) + expect(notifications.map((toast) => toast.content)).toEqual([ + 'Another notification', + 'Completed', + ]) + }) + it('parses error string to object on show', () => { const toast = { title: 'Title', diff --git a/src-ui/src/app/services/toast.service.ts b/src-ui/src/app/services/toast.service.ts index 6d9916143b..8d7eb8f86b 100644 --- a/src-ui/src/app/services/toast.service.ts +++ b/src-ui/src/app/services/toast.service.ts @@ -75,7 +75,14 @@ export class ToastService { if (persistent) { // the dropdown is open, so it's seen as it arrives toast.read = !!this._suppressPopupToasts - this.toasts.unshift(toast) + const index = this.toasts.findIndex( + (existing) => existing.id === toast.id + ) + if (index === -1) { + this.toasts.unshift(toast) + } else { + this.toasts[index] = toast + } this.toastsSubject.next(this.toasts) } // Popups are suppressed while the dropdown is open diff --git a/src-ui/src/app/services/trash.service.spec.ts b/src-ui/src/app/services/trash.service.spec.ts index f65372d2f9..33c3f8097f 100644 --- a/src-ui/src/app/services/trash.service.spec.ts +++ b/src-ui/src/app/services/trash.service.spec.ts @@ -7,7 +7,8 @@ import { import { environment } from 'src/environments/environment' import { PaperlessTaskStatus } from '../data/paperless-task' import { DocumentListViewService } from './document-list-view.service' -import { Toast, ToastService } from './toast.service' +import { TaskNotificationService } from './task-notification.service' +import { Toast, ToastLevel, ToastService } from './toast.service' import { TrashService } from './trash.service' describe('TrashService', () => { @@ -112,21 +113,46 @@ describe('TrashService', () => { toastService.getToasts().subscribe((t) => (toasts = [...t])) service.trackMovedToTrash('delete-1', 2, [1, 2]) expect(toasts).toHaveLength(0) - service.handleDeleteTaskStatus('other-delete', PaperlessTaskStatus.Success) + const notifications = TestBed.inject(TaskNotificationService) + notifications.complete({ + task_id: 'other-delete', + operation: 'delete', + status: PaperlessTaskStatus.Success, + }) expect(toasts).toHaveLength(0) - service.handleDeleteTaskStatus('delete-1', PaperlessTaskStatus.Success) + notifications.complete({ + task_id: 'delete-1', + operation: 'delete', + status: PaperlessTaskStatus.Success, + }) expect(toasts).toHaveLength(1) expect(toasts[0].actionName).toBe('Undo') + toasts[0].action() + const req = httpTestingController.expectOne( + `${environment.apiBaseUrl}trash/` + ) + expect(req.request.body).toEqual({ action: 'restore', documents: [1, 2] }) }) it('handles completion before the queue response and does not offer Undo on failure', () => { let toasts: Toast[] = [] toastService.getToasts().subscribe((t) => (toasts = [...t])) - service.handleDeleteTaskStatus('delete-1', PaperlessTaskStatus.Success) + const notifications = TestBed.inject(TaskNotificationService) + notifications.complete({ + task_id: 'delete-1', + operation: 'delete', + status: PaperlessTaskStatus.Success, + }) service.trackMovedToTrash('delete-1', 1, [1]) expect(toasts[0].actionName).toBe('Undo') service.trackMovedToTrash('delete-2', 1, [2]) - service.handleDeleteTaskStatus('delete-2', PaperlessTaskStatus.Failure) - expect(toasts).toHaveLength(1) + notifications.complete({ + task_id: 'delete-2', + operation: 'delete', + status: PaperlessTaskStatus.Failure, + }) + expect(toasts).toHaveLength(2) + expect(toasts[0].level).toBe(ToastLevel.Error) + expect(toasts[0].actionName).not.toBe('Undo') }) }) diff --git a/src-ui/src/app/services/trash.service.ts b/src-ui/src/app/services/trash.service.ts index f66a8c51be..93a0a36369 100644 --- a/src-ui/src/app/services/trash.service.ts +++ b/src-ui/src/app/services/trash.service.ts @@ -3,10 +3,10 @@ import { Injectable, inject } from '@angular/core' import { Observable } from 'rxjs' import { environment } from 'src/environments/environment' import { Document } from '../data/document' -import { PaperlessTaskStatus } from '../data/paperless-task' import { Results } from '../data/results' import { DocumentListViewService } from './document-list-view.service' -import { ToastLevel, ToastService } from './toast.service' +import { TaskNotificationService } from './task-notification.service' +import { Toast, ToastLevel, ToastService } from './toast.service' @Injectable({ providedIn: 'root', @@ -15,48 +15,21 @@ export class TrashService { private http = inject(HttpClient) private toastService = inject(ToastService) private documentListViewService = inject(DocumentListViewService) - private pendingMoves = new Map< - string, - { count: number; documents?: number[] } - >() - private completedMoves = new Map() + private taskNotifications = inject(TaskNotificationService) public trackMovedToTrash( taskId: string, count: number, documents?: number[] ) { - const status = this.completedMoves.get(taskId) - if (status !== undefined) { - this.completedMoves.delete(taskId) - if (status === PaperlessTaskStatus.Success) { - this.notifyMovedToTrash( - $localize`Moved ${count} document(s) to the trash`, - documents - ) - } - } else { - this.pendingMoves.set(taskId, { count, documents }) - } - } - - public handleDeleteTaskStatus(taskId: string, status: PaperlessTaskStatus) { - const move = this.pendingMoves.get(taskId) - if (move) { - this.pendingMoves.delete(taskId) - if (status === PaperlessTaskStatus.Success) { - this.notifyMovedToTrash( - $localize`Moved ${move.count} document(s) to the trash`, - move.documents - ) - } - } else { - // in case the task finishes fast - this.completedMoves.set(taskId, status) - if (this.completedMoves.size > 100) { - this.completedMoves.delete(this.completedMoves.keys().next().value) - } - } + this.taskNotifications.showQueued( + taskId, + $localize`Bulk operation queued.`, + this.movedToTrashToast( + $localize`Moved ${count} document(s) to the trash`, + documents + ) + ) } public getTrash(page: number = 1): Observable> { @@ -87,14 +60,22 @@ export class TrashService { * Shows a toast with an Undo action when possible */ public notifyMovedToTrash(content: string, documents?: number[]) { + this.toastService.show(this.movedToTrashToast(content, documents)) + } + + private movedToTrashToast(content: string, documents?: number[]): Toast { const canUndo = documents?.length > 0 - this.toastService.show({ + return { content, delay: 10000, level: ToastLevel.Success, - actionName: canUndo ? $localize`Undo` : undefined, - action: canUndo ? () => this.undoMoveToTrash(documents) : undefined, - }) + ...(canUndo + ? { + actionName: $localize`Undo`, + action: () => this.undoMoveToTrash(documents), + } + : {}), + } } private undoMoveToTrash(documents: number[]) {