Fix: move re-index after workflow runs on doc edit

---------

Co-authored-by: shamoon <4887959+shamoon@users.noreply.github.com>

src-ui/src/app/components/admin/tasks/tasks.component.spec.ts

@@ -19,7 +19,6 @@ import {
   PaperlessTaskTriggerSource,
   PaperlessTaskType,
 } from 'src/app/data/paperless-task'
-import { Results } from 'src/app/data/results'
 import { IfPermissionsDirective } from 'src/app/directives/if-permissions.directive'
 import { PermissionsGuard } from 'src/app/guards/permissions.guard'
 import { CustomDatePipe } from 'src/app/pipes/custom-date.pipe'
@@ -153,11 +152,6 @@ const tasks: PaperlessTask[] = [
   },
 ]
 
-const paginatedTasks: Results<PaperlessTask> = {
-  count: tasks.length,
-  results: tasks,
-}
-
 const sectionCountResponse = {
   all: 7,
   needs_attention: 2,
@@ -204,7 +198,7 @@ describe('TasksComponent', () => {
     }).compileComponents()
 
     tasksService = TestBed.inject(TasksService)
-    reloadSpy = jest.spyOn(tasksService, 'reload')
+    reloadSpy = jest.spyOn(tasksService, 'reload').mockImplementation(() => {})
     httpTestingController = TestBed.inject(HttpTestingController)
     modalService = TestBed.inject(NgbModal)
     router = TestBed.inject(Router)
@@ -214,15 +208,6 @@ describe('TasksComponent', () => {
     jest.useFakeTimers()
     fixture.detectChanges()
 
-    httpTestingController
-      .expectOne(
-        (req) =>
-          req.url === `${environment.apiBaseUrl}tasks/` &&
-          req.params.get('acknowledged') === 'false' &&
-          req.params.get('page_size') === '1000'
-      )
-      .flush(paginatedTasks)
-
     httpTestingController
       .expectOne(
         (req) =>

src-ui/src/app/components/app-frame/app-frame.component.html

@@ -344,11 +344,11 @@
                 ngbPopover="Tasks" i18n-ngbPopover [disablePopover]="!slimSidebarPopoversEnabled" placement="end"
                 container="body" triggers="mouseenter:mouseleave" popoverClass="popover-slim">
                 <i-bs class="me-2" name="list-task"></i-bs><span class="nav-link-label"><ng-container i18n>Tasks</ng-container></span>
-                @if (tasksService.needsAttentionTasks.length > 0 && !slimSidebarEnabled) {
-                  <span class="badge bg-danger ms-2 d-inline flex-shrink-0">{{tasksService.needsAttentionTasks.length}}</span>
+                @if (tasksService.needsAttentionTasksCount > 0 && !slimSidebarEnabled) {
+                  <span class="badge bg-danger ms-2 d-inline flex-shrink-0">{{tasksService.needsAttentionTasksCount}}</span>
                 }
-                @if (tasksService.needsAttentionTasks.length > 0 && slimSidebarEnabled) {
-                  <span class="badge bg-danger position-absolute top-0 end-0 d-none d-md-block">{{tasksService.needsAttentionTasks.length}}</span>
+                @if (tasksService.needsAttentionTasksCount > 0 && slimSidebarEnabled) {
+                  <span class="badge bg-danger position-absolute top-0 end-0 d-none d-md-block">{{tasksService.needsAttentionTasksCount}}</span>
                 }
               </a>
             </li>

src-ui/src/app/components/app-frame/app-frame.component.spec.ts

@@ -588,8 +588,8 @@ describe('AppFrameComponent', () => {
 
   it('should show tasks badge for needs-attention tasks', () => {
     jest
-      .spyOn(tasksService, 'needsAttentionTasks', 'get')
-      .mockReturnValue([{} as any, {} as any])
+      .spyOn(tasksService, 'needsAttentionTasksCount', 'get')
+      .mockReturnValue(2)
 
     fixture.detectChanges()
 

src-ui/src/app/services/tasks.service.spec.ts

@@ -9,11 +9,7 @@ import {
 } from '@angular/common/http/testing'
 import { TestBed } from '@angular/core/testing'
 import { environment } from 'src/environments/environment'
-import {
-  PaperlessTaskStatus,
-  PaperlessTaskTriggerSource,
-  PaperlessTaskType,
-} from '../data/paperless-task'
+import { PaperlessTaskType } from '../data/paperless-task'
 import { TasksService } from './tasks.service'
 
 describe('TasksService', () => {
@@ -38,23 +34,23 @@ describe('TasksService', () => {
     httpTestingController.verify()
   })
 
-  it('calls tasks api endpoint on reload', () => {
+  it('calls tasks status counts api endpoint on reload', () => {
     tasksService.reload()
     const req = httpTestingController.expectOne(
       (req: HttpRequest<unknown>) =>
-        req.url === `${environment.apiBaseUrl}tasks/` &&
-        req.params.get('acknowledged') === 'false' &&
-        req.params.get('page_size') === '1000'
+        req.url === `${environment.apiBaseUrl}tasks/status_counts/` &&
+        req.params.get('acknowledged') === 'false'
     )
     expect(req.request.method).toEqual('GET')
-    req.flush({ count: 0, results: [] })
+    req.flush({ all: 5, needs_attention: 3, in_progress: 1, completed: 1 })
+    expect(tasksService.needsAttentionTasksCount).toEqual(3)
   })
 
   it('cancels an in-progress reload when reloading again', () => {
     tasksService.reload()
     const staleReload = httpTestingController.expectOne(
       (req: HttpRequest<unknown>) =>
-        req.url === `${environment.apiBaseUrl}tasks/`
+        req.url === `${environment.apiBaseUrl}tasks/status_counts/`
     )
     tasksService.reload()
 
@@ -62,9 +58,9 @@ describe('TasksService', () => {
     httpTestingController
       .expectOne(
         (req: HttpRequest<unknown>) =>
-          req.url === `${environment.apiBaseUrl}tasks/`
+          req.url === `${environment.apiBaseUrl}tasks/status_counts/`
       )
-      .flush({ count: 0, results: [] })
+      .flush({ all: 0, needs_attention: 0, in_progress: 0, completed: 0 })
   })
 
   it('continues reloading after a reload request fails', () => {
@@ -72,7 +68,7 @@ describe('TasksService', () => {
     httpTestingController
       .expectOne(
         (req: HttpRequest<unknown>) =>
-          req.url === `${environment.apiBaseUrl}tasks/`
+          req.url === `${environment.apiBaseUrl}tasks/status_counts/`
       )
       .flush('error', { status: 500, statusText: 'error' })
 
@@ -82,16 +78,16 @@ describe('TasksService', () => {
     httpTestingController
       .expectOne(
         (req: HttpRequest<unknown>) =>
-          req.url === `${environment.apiBaseUrl}tasks/`
+          req.url === `${environment.apiBaseUrl}tasks/status_counts/`
       )
-      .flush({ count: 0, results: [] })
+      .flush({ all: 0, needs_attention: 0, in_progress: 0, completed: 0 })
   })
 
   it('reloads after dismissing a task while a reload is already in progress', () => {
     tasksService.reload()
     const staleReload = httpTestingController.expectOne(
       (req: HttpRequest<unknown>) =>
-        req.url === `${environment.apiBaseUrl}tasks/` &&
+        req.url === `${environment.apiBaseUrl}tasks/status_counts/` &&
         req.params.get('acknowledged') === 'false'
     )
 
@@ -104,12 +100,12 @@ describe('TasksService', () => {
     httpTestingController
       .expectOne(
         (req: HttpRequest<unknown>) =>
-          req.url === `${environment.apiBaseUrl}tasks/` &&
+          req.url === `${environment.apiBaseUrl}tasks/status_counts/` &&
           req.params.get('acknowledged') === 'false'
       )
-      .flush({ count: 0, results: [] })
+      .flush({ all: 50, needs_attention: 42, in_progress: 3, completed: 5 })
 
-    expect(tasksService.needsAttentionTasks).toHaveLength(0)
+    expect(tasksService.needsAttentionTasksCount).toEqual(42)
   })
 
   it('calls acknowledge_tasks api endpoint on dismiss and reloads', () => {
@@ -126,11 +122,10 @@ describe('TasksService', () => {
     httpTestingController
       .expectOne(
         (req: HttpRequest<unknown>) =>
-          req.url === `${environment.apiBaseUrl}tasks/` &&
-          req.params.get('acknowledged') === 'false' &&
-          req.params.get('page_size') === '1000'
+          req.url === `${environment.apiBaseUrl}tasks/status_counts/` &&
+          req.params.get('acknowledged') === 'false'
       )
-      .flush({ count: 0, results: [] })
+      .flush({ all: 0, needs_attention: 0, in_progress: 0, completed: 0 })
   })
 
   it('calls acknowledge_tasks api endpoint on dismiss all and reloads', () => {
@@ -147,65 +142,10 @@ describe('TasksService', () => {
     httpTestingController
       .expectOne(
         (req: HttpRequest<unknown>) =>
-          req.url === `${environment.apiBaseUrl}tasks/` &&
-          req.params.get('acknowledged') === 'false' &&
-          req.params.get('page_size') === '1000'
+          req.url === `${environment.apiBaseUrl}tasks/status_counts/` &&
+          req.params.get('acknowledged') === 'false'
       )
-      .flush({ count: 0, results: [] })
-  })
-
-  it('includes revoked tasks in needs attention', () => {
-    const mockTasks = [
-      {
-        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.Revoked,
-        acknowledged: false,
-        task_id: '1236',
-        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<unknown>) =>
-        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.needsAttentionTasks).toHaveLength(2)
-    expect(tasksService.needsAttentionTasks.map((task) => task.status)).toEqual(
-      expect.arrayContaining([
-        PaperlessTaskStatus.Failure,
-        PaperlessTaskStatus.Revoked,
-      ])
-    )
+      .flush({ all: 0, needs_attention: 0, in_progress: 0, completed: 0 })
   })
 
   it('supports running tasks', () => {

src-ui/src/app/services/tasks.service.ts

@@ -5,14 +5,12 @@ import {
   catchError,
   finalize,
   first,
-  map,
   switchMap,
   takeUntil,
   tap,
 } from 'rxjs/operators'
 import {
   PaperlessTask,
-  PaperlessTaskStatus,
   PaperlessTaskStatusCounts,
   PaperlessTaskType,
 } from 'src/app/data/paperless-task'
@@ -27,50 +25,42 @@ export class TasksService {
 
   private baseUrl: string = environment.apiBaseUrl
   private endpoint: string = 'tasks'
-  private readonly defaultReloadPageSize = 1000
 
   public loading: boolean = false
 
-  private readonly tasks = signal<PaperlessTask[]>([])
+  private readonly tasksStatusCounts = signal<PaperlessTaskStatusCounts>({
+    all: 0,
+    needs_attention: 0,
+    in_progress: 0,
+    completed: 0,
+  })
   private readonly reloadNotifier = new Subject<void>()
 
   private unsubscribeNotifer: Subject<any> = new Subject()
 
   constructor() {
     this.reloadNotifier
-      .pipe(
-        switchMap(() => {
-          this.loading = true
-          return this.http
-            .get<Results<PaperlessTask>>(`${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)
+      .pipe(switchMap(() => this.fetchUnacknowledgedTasksCounts()))
+      .subscribe((taskStatusCounts) => {
+        this.tasksStatusCounts.set(taskStatusCounts)
       })
   }
 
-  public get needsAttentionTasks(): PaperlessTask[] {
-    return this.tasks().filter((t) =>
-      [PaperlessTaskStatus.Failure, PaperlessTaskStatus.Revoked].includes(
-        t.status
-      )
+  private fetchUnacknowledgedTasksCounts(): Observable<PaperlessTaskStatusCounts> {
+    this.loading = true
+    return this.statusCounts({ acknowledged: false }).pipe(
+      takeUntil(this.unsubscribeNotifer),
+      catchError(() => EMPTY),
+      finalize(() => {
+        this.loading = false
+      })
     )
   }
 
+  public get needsAttentionTasksCount(): number {
+    return this.tasksStatusCounts().needs_attention
+  }
+
   public reload() {
     this.reloadNotifier.next()
   }
@@ -105,7 +95,7 @@ export class TasksService {
 
   public dismissTasks(task_ids: Set<number>): Observable<any> {
     return this.http
-      .post(`${this.baseUrl}tasks/acknowledge/`, {
+      .post(`${this.baseUrl}${this.endpoint}/acknowledge/`, {
         tasks: [...task_ids],
       })
       .pipe(
@@ -119,7 +109,7 @@ export class TasksService {
 
   public dismissAllTasks(): Observable<any> {
     return this.http
-      .post(`${this.baseUrl}tasks/acknowledge/`, {
+      .post(`${this.baseUrl}${this.endpoint}/acknowledge/`, {
         all: true,
       })
       .pipe(

src/documents/tests/test_api_documents.py

@@ -46,6 +46,7 @@ from documents.models import UiSettings
 from documents.models import Workflow
 from documents.models import WorkflowAction
 from documents.models import WorkflowTrigger
+from documents.search import get_backend
 from documents.signals.handlers import run_workflows
 from documents.tests.utils import ConsumeTaskMixin
 from paperless_testing.dirs import DirectoriesMixin
@@ -66,6 +67,70 @@ class TestDocumentApi(DirectoriesMixin, ConsumeTaskMixin, APITestCase):
         self.client.force_authenticate(user=self.user)
         cache.clear()
 
+    def test_detail_save_indexes_final_workflow_state(self) -> None:
+        viewer = UserFactory()
+        grant_global(viewer, "view_document")
+        inbox = TagFactory(is_inbox_tag=True)
+        doc = DocumentFactory(owner=self.user, content="distinctiveword")
+        doc.tags.add(inbox)
+        trigger = WorkflowTrigger.objects.create(
+            type=WorkflowTrigger.WorkflowTriggerType.DOCUMENT_UPDATED,
+        )
+        action = WorkflowAction.objects.create(assign_title="Workflow title")
+        action.assign_view_users.add(viewer)
+        action.remove_view_users.add(viewer)
+        workflow = Workflow.objects.create(name="Update document")
+        workflow.triggers.add(trigger)
+        workflow.actions.add(action)
+        backend = get_backend()
+        backend.add_or_update(doc)
+
+        for action_type in (
+            WorkflowAction.WorkflowActionType.ASSIGNMENT,
+            WorkflowAction.WorkflowActionType.REMOVAL,
+            WorkflowAction.WorkflowActionType.MOVE_TO_TRASH,
+        ):
+            with self.subTest(action_type=action_type):
+                grant = action_type == WorkflowAction.WorkflowActionType.ASSIGNMENT
+                action.type = action_type
+                action.save()
+                trigger.filter_has_not_tags.set([inbox] if grant else [])
+                trigger.filter_has_tags.set([] if grant else [inbox])
+                self.client.force_authenticate(self.user)
+                response = self.client.patch(
+                    f"/api/documents/{doc.pk}/",
+                    {"tags": [] if grant else [inbox.pk]},
+                    format="json",
+                )
+                self.assertEqual(response.status_code, status.HTTP_200_OK)
+                self.client.force_authenticate(viewer)
+                self.assertEqual(
+                    self.client.get(f"/api/documents/{doc.pk}/").status_code,
+                    status.HTTP_200_OK if grant else status.HTTP_404_NOT_FOUND,
+                )
+                # Check the index directly: the API's DB permission filtering
+                # would otherwise hide stale grants and trashed index entries.
+                self.assertEqual(
+                    list(backend.search_ids("distinctiveword", user=viewer)),
+                    [doc.pk] if grant else [],
+                )
+                if grant:
+                    response = self.client.get(
+                        "/api/documents/",
+                        {"query": "distinctiveword AND title:workflow"},
+                    )
+                    self.assertEqual(response.status_code, status.HTTP_200_OK)
+                    self.assertEqual(
+                        [result["id"] for result in response.data["results"]],
+                        [doc.pk],
+                    )
+                elif action_type == WorkflowAction.WorkflowActionType.MOVE_TO_TRASH:
+                    self.assertFalse(Document.objects.filter(pk=doc.pk).exists())
+                    self.assertEqual(
+                        list(backend.search_ids("distinctiveword", user=None)),
+                        [],
+                    )
+
     def testDocuments(self) -> None:
         response = self.client.get("/api/documents/").data
 

src/documents/views.py

@@ -1304,13 +1304,19 @@ class DocumentViewSet(
 
         from documents.search import get_backend
 
-        get_backend().add_or_update(refreshed_doc)
-
         document_updated.send(
             sender=self.__class__,
             document=refreshed_doc,
         )
 
+        # Index the final workflow state, including deletion.
+        backend = get_backend()
+        final_doc = Document.objects.filter(pk=root_doc.pk).first()
+        if final_doc is None:
+            backend.remove(root_doc.pk)
+        else:
+            backend.add_or_update(final_doc)
+
         return response
 
     def list(self, request, *args, **kwargs):