From de9f520143aac757b1f232d39ae75ab630a003b4 Mon Sep 17 00:00:00 2001 From: shamoon <4887959+shamoon@users.noreply.github.com> Date: Tue, 28 Jul 2026 00:34:30 -0700 Subject: [PATCH] Chore/fix: refactor frontend task service (#13365) --- .../admin/tasks/tasks.component.html | 2 +- src-ui/src/app/services/tasks.service.spec.ts | 132 ++++++++---------- src-ui/src/app/services/tasks.service.ts | 89 ++++++------ 3 files changed, 96 insertions(+), 127 deletions(-) diff --git a/src-ui/src/app/components/admin/tasks/tasks.component.html b/src-ui/src/app/components/admin/tasks/tasks.component.html index 7fae03a7f..f1220e4b5 100644 --- a/src-ui/src/app/components/admin/tasks/tasks.component.html +++ b/src-ui/src/app/components/admin/tasks/tasks.component.html @@ -21,7 +21,7 @@ -@if (!tasksService.completedFileTasks && tasksService.loading) { +@if (loading() && pagedTasks().length === 0) { } diff --git a/src-ui/src/app/services/tasks.service.spec.ts b/src-ui/src/app/services/tasks.service.spec.ts index 3412ae2ce..c8dca41fb 100644 --- a/src-ui/src/app/services/tasks.service.spec.ts +++ b/src-ui/src/app/services/tasks.service.spec.ts @@ -50,13 +50,66 @@ describe('TasksService', () => { req.flush({ count: 0, results: [] }) }) - it('does not call tasks api endpoint on reload if already loading', () => { - tasksService.loading = true + it('cancels an in-progress reload when reloading again', () => { tasksService.reload() - httpTestingController.expectNone( + const staleReload = httpTestingController.expectOne( (req: HttpRequest) => req.url === `${environment.apiBaseUrl}tasks/` ) + tasksService.reload() + + expect(staleReload.cancelled).toBe(true) + httpTestingController + .expectOne( + (req: HttpRequest) => + req.url === `${environment.apiBaseUrl}tasks/` + ) + .flush({ count: 0, results: [] }) + }) + + it('continues reloading after a reload request fails', () => { + tasksService.reload() + httpTestingController + .expectOne( + (req: HttpRequest) => + req.url === `${environment.apiBaseUrl}tasks/` + ) + .flush('error', { status: 500, statusText: 'error' }) + + expect(tasksService.loading).toBe(false) + + tasksService.reload() + httpTestingController + .expectOne( + (req: HttpRequest) => + req.url === `${environment.apiBaseUrl}tasks/` + ) + .flush({ count: 0, results: [] }) + }) + + it('reloads after dismissing a task while a reload is already in progress', () => { + tasksService.reload() + const staleReload = httpTestingController.expectOne( + (req: HttpRequest) => + req.url === `${environment.apiBaseUrl}tasks/` && + req.params.get('acknowledged') === 'false' + ) + + tasksService.dismissTasks(new Set([1])).subscribe() + httpTestingController + .expectOne(`${environment.apiBaseUrl}tasks/acknowledge/`) + .flush([]) + + expect(staleReload.cancelled).toBe(true) + httpTestingController + .expectOne( + (req: HttpRequest) => + req.url === `${environment.apiBaseUrl}tasks/` && + req.params.get('acknowledged') === 'false' + ) + .flush({ count: 0, results: [] }) + + expect(tasksService.needsAttentionTasks).toHaveLength(0) }) it('calls acknowledge_tasks api endpoint on dismiss and reloads', () => { @@ -101,79 +154,6 @@ describe('TasksService', () => { .flush({ count: 0, results: [] }) }) - it('groups mixed task types by status when reloading', () => { - expect(tasksService.total).toEqual(0) - const mockTasks = [ - { - task_type: PaperlessTaskType.ConsumeFile, - trigger_source: PaperlessTaskTriggerSource.FolderConsume, - status: PaperlessTaskStatus.Success, - acknowledged: false, - task_id: '1234', - input_data: { filename: 'file1.pdf' }, - date_created: new Date(), - related_document_ids: [], - }, - { - task_type: PaperlessTaskType.SanityCheck, - trigger_source: PaperlessTaskTriggerSource.System, - status: PaperlessTaskStatus.Failure, - acknowledged: false, - task_id: '1235', - input_data: {}, - date_created: new Date(), - related_document_ids: [], - }, - { - task_type: PaperlessTaskType.MailFetch, - trigger_source: PaperlessTaskTriggerSource.Scheduled, - status: PaperlessTaskStatus.Pending, - acknowledged: false, - task_id: '1236', - input_data: {}, - date_created: new Date(), - related_document_ids: [], - }, - { - task_type: PaperlessTaskType.LlmIndex, - trigger_source: PaperlessTaskTriggerSource.WebUI, - status: PaperlessTaskStatus.Started, - acknowledged: false, - task_id: '1237', - input_data: {}, - date_created: new Date(), - related_document_ids: [], - }, - { - task_type: PaperlessTaskType.EmptyTrash, - trigger_source: PaperlessTaskTriggerSource.Manual, - status: PaperlessTaskStatus.Success, - acknowledged: false, - task_id: '1238', - input_data: {}, - date_created: new Date(), - related_document_ids: [], - }, - ] - - tasksService.reload() - - const req = httpTestingController.expectOne( - (req: HttpRequest) => - req.url === `${environment.apiBaseUrl}tasks/` && - req.params.get('acknowledged') === 'false' && - req.params.get('page_size') === '1000' - ) - - req.flush({ count: mockTasks.length, results: mockTasks }) - - expect(tasksService.allFileTasks).toHaveLength(5) - expect(tasksService.completedFileTasks).toHaveLength(2) - expect(tasksService.failedFileTasks).toHaveLength(1) - expect(tasksService.queuedFileTasks).toHaveLength(1) - expect(tasksService.startedFileTasks).toHaveLength(1) - }) - it('includes revoked tasks in needs attention', () => { const mockTasks = [ { diff --git a/src-ui/src/app/services/tasks.service.ts b/src-ui/src/app/services/tasks.service.ts index 8b71c6923..189d3ac62 100644 --- a/src-ui/src/app/services/tasks.service.ts +++ b/src-ui/src/app/services/tasks.service.ts @@ -1,7 +1,15 @@ import { HttpClient } from '@angular/common/http' import { Injectable, inject, signal } from '@angular/core' -import { Observable, Subject } from 'rxjs' -import { first, map, takeUntil, tap } from 'rxjs/operators' +import { EMPTY, Observable, Subject } from 'rxjs' +import { + catchError, + finalize, + first, + map, + switchMap, + takeUntil, + tap, +} from 'rxjs/operators' import { PaperlessTask, PaperlessTaskStatus, @@ -23,44 +31,40 @@ export class TasksService { public loading: boolean = false - private readonly fileTasks = signal([]) + private readonly tasks = signal([]) + private readonly reloadNotifier = new Subject() private unsubscribeNotifer: Subject = new Subject() - public get total(): number { - return this.fileTasks().length - } - - public get allFileTasks(): PaperlessTask[] { - return this.fileTasks().slice(0) - } - - public get queuedFileTasks(): PaperlessTask[] { - return this.fileTasks().filter( - (t) => t.status === PaperlessTaskStatus.Pending - ) - } - - public get startedFileTasks(): PaperlessTask[] { - return this.fileTasks().filter( - (t) => t.status === PaperlessTaskStatus.Started - ) - } - - public get completedFileTasks(): PaperlessTask[] { - return this.fileTasks().filter( - (t) => t.status === PaperlessTaskStatus.Success - ) - } - - public get failedFileTasks(): PaperlessTask[] { - return this.fileTasks().filter( - (t) => t.status === PaperlessTaskStatus.Failure - ) + constructor() { + this.reloadNotifier + .pipe( + switchMap(() => { + this.loading = true + return this.http + .get>(`${this.baseUrl}${this.endpoint}/`, { + params: { + acknowledged: 'false', + page_size: this.defaultReloadPageSize, + }, + }) + .pipe( + map((response) => response.results), + takeUntil(this.unsubscribeNotifer), + catchError(() => EMPTY), + finalize(() => { + this.loading = false + }) + ) + }) + ) + .subscribe((tasks) => { + this.tasks.set(tasks) + }) } public get needsAttentionTasks(): PaperlessTask[] { - return this.fileTasks().filter((t) => + return this.tasks().filter((t) => [PaperlessTaskStatus.Failure, PaperlessTaskStatus.Revoked].includes( t.status ) @@ -68,22 +72,7 @@ export class TasksService { } public reload() { - if (this.loading) return - this.loading = true - - this.http - .get>(`${this.baseUrl}${this.endpoint}/`, { - params: { - acknowledged: 'false', - page_size: this.defaultReloadPageSize, - }, - }) - .pipe(map((r) => r.results)) - .pipe(takeUntil(this.unsubscribeNotifer), first()) - .subscribe((r) => { - this.fileTasks.set(r) - this.loading = false - }) + this.reloadNotifier.next() } public list(