From eb5bf534764c69383313d45926ac51f54b40ad83 Mon Sep 17 00:00:00 2001 From: Trenton Holmes <797416+stumpylog@users.noreply.github.com> Date: Mon, 20 Jul 2026 09:14:58 -0700 Subject: [PATCH] Fix (beta): stop blocking the document list on selection-data aggregation The document overview sent include_selection_data=true on every list request (page/filter/sort change), computing 5 correlated Count(DISTINCT) aggregations over the full matching queryset inline before the list could render. At scale (hundreds of thousands of documents, unfiltered) this is the confirmed root cause of the document overview "eternally loading" in paperless-ngx/paperless-ngx#13161. Split the aggregation into its own endpoint, GET /api/documents/filter_selection_data/, filter-scoped the same way the list endpoint already resolves matches (no document ID enumeration needed). The frontend now fetches it as a separate, non-blocking request after the list has already rendered, for plain (non-search) browsing. Full-text search keeps computing it inline since results are already narrowed by the search backend first. include_selection_data never shipped in a stable release (introduced this beta cycle, not present on main), so the plain list endpoint simply stops acting on it rather than needing a deprecation path. Also closes a latent localStorage leak between document-list-view.service.spec.ts tests (filterRules persisted via localStorage were never cleared, only sessionStorage was) that this change's new URL-dependent follow-up request exposed, and fixes two tests that were unknowingly relying on that leaked state to pass. Co-Authored-By: Claude Sonnet 5 --- .../bulk-editor/bulk-editor.component.spec.ts | 38 ++--- .../document-list-view.service.spec.ts | 134 ++++++++++++------ .../services/document-list-view.service.ts | 28 +++- .../src/app/services/rest/document.service.ts | 15 ++ src/documents/tests/test_api_documents.py | 27 ++-- src/documents/views.py | 31 ++-- 6 files changed, 181 insertions(+), 92 deletions(-) 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 041eed976..d6e8a5072 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 @@ -386,7 +386,7 @@ describe('BulkEditorComponent', () => { parameters: { add_tags: [101], remove_tags: [] }, }) httpTestingController.match( - `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true` + `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false` ) // list reload httpTestingController.match( `${environment.apiBaseUrl}documents/?page=1&page_size=100000&fields=id` @@ -432,7 +432,7 @@ describe('BulkEditorComponent', () => { parameters: { add_tags: [101], remove_tags: [] }, }) httpTestingController.match( - `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true` + `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false` ) // list reload }) @@ -461,7 +461,7 @@ describe('BulkEditorComponent', () => { .expectOne(`${environment.apiBaseUrl}documents/bulk_edit/`) .flush(true) httpTestingController.match( - `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true` + `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false` ) // list reload httpTestingController.match( `${environment.apiBaseUrl}documents/?page=1&page_size=100000&fields=id` @@ -552,7 +552,7 @@ describe('BulkEditorComponent', () => { parameters: { correspondent: 101 }, }) httpTestingController.match( - `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true` + `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false` ) // list reload httpTestingController.match( `${environment.apiBaseUrl}documents/?page=1&page_size=100000&fields=id` @@ -584,7 +584,7 @@ describe('BulkEditorComponent', () => { .expectOne(`${environment.apiBaseUrl}documents/bulk_edit/`) .flush(true) httpTestingController.match( - `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true` + `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false` ) // list reload httpTestingController.match( `${environment.apiBaseUrl}documents/?page=1&page_size=100000&fields=id` @@ -650,7 +650,7 @@ describe('BulkEditorComponent', () => { parameters: { document_type: 101 }, }) httpTestingController.match( - `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true` + `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false` ) // list reload httpTestingController.match( `${environment.apiBaseUrl}documents/?page=1&page_size=100000&fields=id` @@ -682,7 +682,7 @@ describe('BulkEditorComponent', () => { .expectOne(`${environment.apiBaseUrl}documents/bulk_edit/`) .flush(true) httpTestingController.match( - `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true` + `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false` ) // list reload httpTestingController.match( `${environment.apiBaseUrl}documents/?page=1&page_size=100000&fields=id` @@ -748,7 +748,7 @@ describe('BulkEditorComponent', () => { parameters: { storage_path: 101 }, }) httpTestingController.match( - `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true` + `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false` ) // list reload httpTestingController.match( `${environment.apiBaseUrl}documents/?page=1&page_size=100000&fields=id` @@ -780,7 +780,7 @@ describe('BulkEditorComponent', () => { .expectOne(`${environment.apiBaseUrl}documents/bulk_edit/`) .flush(true) httpTestingController.match( - `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true` + `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false` ) // list reload httpTestingController.match( `${environment.apiBaseUrl}documents/?page=1&page_size=100000&fields=id` @@ -846,7 +846,7 @@ describe('BulkEditorComponent', () => { parameters: { add_custom_fields: [101], remove_custom_fields: [102] }, }) httpTestingController.match( - `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true` + `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false` ) // list reload httpTestingController.match( `${environment.apiBaseUrl}documents/?page=1&page_size=100000&fields=id` @@ -878,7 +878,7 @@ describe('BulkEditorComponent', () => { .expectOne(`${environment.apiBaseUrl}documents/bulk_edit/`) .flush(true) httpTestingController.match( - `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true` + `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false` ) // list reload httpTestingController.match( `${environment.apiBaseUrl}documents/?page=1&page_size=100000&fields=id` @@ -987,7 +987,7 @@ describe('BulkEditorComponent', () => { documents: [3, 4], }) httpTestingController.match( - `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true` + `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false` ) // list reload httpTestingController.match( `${environment.apiBaseUrl}documents/?page=1&page_size=100000&fields=id` @@ -1080,7 +1080,7 @@ describe('BulkEditorComponent', () => { documents: [3, 4], }) httpTestingController.match( - `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true` + `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false` ) // list reload httpTestingController.match( `${environment.apiBaseUrl}documents/?page=1&page_size=100000&fields=id` @@ -1115,7 +1115,7 @@ describe('BulkEditorComponent', () => { source_mode: 'latest_version', }) httpTestingController.match( - `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true` + `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false` ) // list reload httpTestingController.match( `${environment.apiBaseUrl}documents/?page=1&page_size=100000&fields=id` @@ -1156,7 +1156,7 @@ describe('BulkEditorComponent', () => { metadata_document_id: 3, }) httpTestingController.match( - `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true` + `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false` ) // list reload httpTestingController.match( `${environment.apiBaseUrl}documents/?page=1&page_size=100000&fields=id` @@ -1175,7 +1175,7 @@ describe('BulkEditorComponent', () => { delete_originals: true, }) httpTestingController.match( - `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true` + `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false` ) // list reload httpTestingController.match( `${environment.apiBaseUrl}documents/?page=1&page_size=100000&fields=id` @@ -1196,7 +1196,7 @@ describe('BulkEditorComponent', () => { archive_fallback: true, }) httpTestingController.match( - `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true` + `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false` ) // list reload httpTestingController.match( `${environment.apiBaseUrl}documents/?page=1&page_size=100000&fields=id` @@ -1299,7 +1299,7 @@ describe('BulkEditorComponent', () => { }, }) httpTestingController.match( - `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true` + `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false` ) // list reload httpTestingController.match( `${environment.apiBaseUrl}documents/?page=1&page_size=100000&fields=id` @@ -1607,7 +1607,7 @@ describe('BulkEditorComponent', () => { expect(toastServiceShowInfoSpy).toHaveBeenCalled() expect(listReloadSpy).toHaveBeenCalled() httpTestingController.match( - `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true` + `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false` ) // list reload httpTestingController.match( `${environment.apiBaseUrl}documents/?page=1&page_size=100000&fields=id` diff --git a/src-ui/src/app/services/document-list-view.service.spec.ts b/src-ui/src/app/services/document-list-view.service.spec.ts index 9447101ad..8ef096303 100644 --- a/src-ui/src/app/services/document-list-view.service.spec.ts +++ b/src-ui/src/app/services/document-list-view.service.spec.ts @@ -84,6 +84,28 @@ const view: SavedView = { filter_rules: filterRules, } +const emptySelectionData = { + selected_correspondents: [], + selected_tags: [], + selected_document_types: [], + selected_storage_paths: [], + selected_custom_fields: [], +} + +// A successful (non-search) list response now triggers a separate, +// non-blocking request for filter dropdown counts. Tests that flush a +// successful list response need to also flush this follow-up request. +function flushSelectionDataRequest( + httpTestingController: HttpTestingController, + querySuffix: string = '' +) { + const req = httpTestingController.expectOne( + `${environment.apiBaseUrl}documents/filter_selection_data/${querySuffix}` + ) + expect(req.request.method).toEqual('GET') + req.flush(emptySelectionData) +} + describe('DocumentListViewService', () => { let httpTestingController: HttpTestingController let documentListViewService: DocumentListViewService @@ -105,6 +127,7 @@ describe('DocumentListViewService', () => { }) sessionStorage.clear() + localStorage.clear() httpTestingController = TestBed.inject(HttpTestingController) documentListViewService = TestBed.inject(DocumentListViewService) settingsService = TestBed.inject(SettingsService) @@ -116,6 +139,7 @@ describe('DocumentListViewService', () => { documentListViewService.cancelPending() httpTestingController.verify() sessionStorage.clear() + localStorage.clear() }) afterAll(() => { @@ -128,10 +152,11 @@ describe('DocumentListViewService', () => { expect(documentListViewService.currentPage).toEqual(1) documentListViewService.reload() const req = httpTestingController.expectOne( - `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true` + `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false` ) expect(req.request.method).toEqual('GET') req.flush(full_results) + flushSelectionDataRequest(httpTestingController) expect(req.request.method).toEqual('GET') expect(documentListViewService.isReloading).toBeFalsy() expect(documentListViewService.activeSavedViewId).toBeNull() @@ -143,12 +168,12 @@ describe('DocumentListViewService', () => { it('should handle error on page request out of range', () => { documentListViewService.currentPage = 50 let req = httpTestingController.expectOne( - `${environment.apiBaseUrl}documents/?page=50&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true` + `${environment.apiBaseUrl}documents/?page=50&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false` ) expect(req.request.method).toEqual('GET') req.flush([], { status: 404, statusText: 'Unexpected error' }) req = httpTestingController.expectOne( - `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true` + `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false` ) expect(req.request.method).toEqual('GET') expect(documentListViewService.currentPage).toEqual(1) @@ -165,21 +190,20 @@ describe('DocumentListViewService', () => { ] documentListViewService.setFilterRules(filterRulesAny) let req = httpTestingController.expectOne( - `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true&tags__id__in=${tags__id__in}` + `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false&tags__id__in=${tags__id__in}` ) expect(req.request.method).toEqual('GET') req.flush( { archive_serial_number: 'hello' }, { status: 404, statusText: 'Unexpected error' } ) - req = httpTestingController.expectOne( - `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true` - ) - expect(req.request.method).toEqual('GET') + // the error is a plain field error (not a page-out-of-range or deleted + // custom-field-sort case), so no automatic retry request is sent here + expect(documentListViewService.error).toBeTruthy() // reset the list documentListViewService.setFilterRules([]) req = httpTestingController.expectOne( - `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true` + `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false` ) }) @@ -187,7 +211,7 @@ describe('DocumentListViewService', () => { documentListViewService.currentPage = 1 documentListViewService.sortField = 'custom_field_999' let req = httpTestingController.expectOne( - `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-custom_field_999&truncate_content=true&include_selection_data=true` + `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-custom_field_999&truncate_content=true&include_selection_data=false` ) expect(req.request.method).toEqual('GET') req.flush( @@ -196,7 +220,7 @@ describe('DocumentListViewService', () => { ) // resets itself req = httpTestingController.expectOne( - `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true` + `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false` ) }) @@ -211,7 +235,7 @@ describe('DocumentListViewService', () => { ] documentListViewService.setFilterRules(filterRulesAny) let req = httpTestingController.expectOne( - `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true&tags__id__in=${tags__id__in}` + `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false&tags__id__in=${tags__id__in}` ) expect(req.request.method).toEqual('GET') req.flush('Generic error', { status: 404, statusText: 'Unexpected error' }) @@ -219,7 +243,7 @@ describe('DocumentListViewService', () => { // reset the list documentListViewService.setFilterRules([]) req = httpTestingController.expectOne( - `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true` + `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false` ) }) @@ -228,7 +252,7 @@ describe('DocumentListViewService', () => { expect(documentListViewService.sortReverse).toBeTruthy() documentListViewService.setSort('added', false) let req = httpTestingController.expectOne( - `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=added&truncate_content=true&include_selection_data=true` + `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=added&truncate_content=true&include_selection_data=false` ) expect(req.request.method).toEqual('GET') expect(documentListViewService.sortField).toEqual('added') @@ -236,12 +260,12 @@ describe('DocumentListViewService', () => { documentListViewService.sortField = 'created' req = httpTestingController.expectOne( - `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=created&truncate_content=true&include_selection_data=true` + `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=created&truncate_content=true&include_selection_data=false` ) expect(documentListViewService.sortField).toEqual('created') documentListViewService.sortReverse = true req = httpTestingController.expectOne( - `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true` + `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false` ) expect(req.request.method).toEqual('GET') expect(documentListViewService.sortReverse).toBeTruthy() @@ -284,7 +308,7 @@ describe('DocumentListViewService', () => { const req = httpTestingController.expectOne( `${environment.apiBaseUrl}documents/?page=${page}&page_size=${ documentListViewService.pageSize - }&ordering=${reverse ? '-' : ''}${sort}&truncate_content=true&include_selection_data=true` + }&ordering=${reverse ? '-' : ''}${sort}&truncate_content=true&include_selection_data=false` ) expect(req.request.method).toEqual('GET') expect(documentListViewService.currentPage).toEqual(page) @@ -301,7 +325,7 @@ describe('DocumentListViewService', () => { } documentListViewService.loadFromQueryParams(convertToParamMap(params)) let req = httpTestingController.expectOne( - `${environment.apiBaseUrl}documents/?page=${documentListViewService.currentPage}&page_size=${documentListViewService.pageSize}&ordering=-added&truncate_content=true&include_selection_data=true&tags__id__all=${tags__id__all}` + `${environment.apiBaseUrl}documents/?page=${documentListViewService.currentPage}&page_size=${documentListViewService.pageSize}&ordering=-added&truncate_content=true&include_selection_data=false&tags__id__all=${tags__id__all}` ) expect(req.request.method).toEqual('GET') expect(documentListViewService.filterRules).toEqual([ @@ -311,12 +335,16 @@ describe('DocumentListViewService', () => { }, ]) req.flush(full_results) + flushSelectionDataRequest( + httpTestingController, + `?tags__id__all=${tags__id__all}` + ) }) it('should use filter rules to update query params', () => { documentListViewService.setFilterRules(filterRules) const req = httpTestingController.expectOne( - `${environment.apiBaseUrl}documents/?page=${documentListViewService.currentPage}&page_size=${documentListViewService.pageSize}&ordering=-created&truncate_content=true&include_selection_data=true&tags__id__all=${tags__id__all}` + `${environment.apiBaseUrl}documents/?page=${documentListViewService.currentPage}&page_size=${documentListViewService.pageSize}&ordering=-created&truncate_content=true&include_selection_data=false&tags__id__all=${tags__id__all}` ) expect(req.request.method).toEqual('GET') }) @@ -325,26 +353,31 @@ describe('DocumentListViewService', () => { documentListViewService.currentPage = 2 let req = httpTestingController.expectOne((request) => request.urlWithParams.startsWith( - `${environment.apiBaseUrl}documents/?page=2&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true` + `${environment.apiBaseUrl}documents/?page=2&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false` ) ) expect(req.request.method).toEqual('GET') req.flush(full_results) + flushSelectionDataRequest(httpTestingController) documentListViewService.setFilterRules(filterRules, true) const filteredReqs = httpTestingController.match( - `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true&tags__id__all=${tags__id__all}` + `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false&tags__id__all=${tags__id__all}` ) expect(filteredReqs).toHaveLength(1) filteredReqs[0].flush(full_results) + flushSelectionDataRequest( + httpTestingController, + `?tags__id__all=${tags__id__all}` + ) expect(documentListViewService.currentPage).toEqual(1) }) it('should support quick filter', () => { documentListViewService.quickFilter(filterRules) const req = httpTestingController.expectOne( - `${environment.apiBaseUrl}documents/?page=${documentListViewService.currentPage}&page_size=${documentListViewService.pageSize}&ordering=-created&truncate_content=true&include_selection_data=true&tags__id__all=${tags__id__all}` + `${environment.apiBaseUrl}documents/?page=${documentListViewService.currentPage}&page_size=${documentListViewService.pageSize}&ordering=-created&truncate_content=true&include_selection_data=false&tags__id__all=${tags__id__all}` ) expect(req.request.method).toEqual('GET') }) @@ -367,21 +400,21 @@ describe('DocumentListViewService', () => { convertToParamMap(params) ) let req = httpTestingController.expectOne( - `${environment.apiBaseUrl}documents/?page=${page}&page_size=${documentListViewService.pageSize}&ordering=-added&truncate_content=true&include_selection_data=true&tags__id__all=${tags__id__all}` + `${environment.apiBaseUrl}documents/?page=${page}&page_size=${documentListViewService.pageSize}&ordering=-added&truncate_content=true&include_selection_data=false&tags__id__all=${tags__id__all}` ) expect(req.request.method).toEqual('GET') // reset the list documentListViewService.currentPage = 1 req = httpTestingController.expectOne( - `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-added&truncate_content=true&include_selection_data=true&tags__id__all=9` + `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-added&truncate_content=true&include_selection_data=false&tags__id__all=9` ) documentListViewService.setFilterRules([]) req = httpTestingController.expectOne( - `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-added&truncate_content=true&include_selection_data=true` + `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-added&truncate_content=true&include_selection_data=false` ) documentListViewService.sortField = 'created' req = httpTestingController.expectOne( - `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true` + `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false` ) documentListViewService.activateSavedView(null) }) @@ -389,18 +422,19 @@ describe('DocumentListViewService', () => { it('should support navigating next / previous', () => { documentListViewService.setFilterRules([]) let req = httpTestingController.expectOne( - `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true` + `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false` ) expect(documentListViewService.currentPage).toEqual(1) documentListViewService.pageSize = 3 req = httpTestingController.expectOne( - `${environment.apiBaseUrl}documents/?page=1&page_size=3&ordering=-created&truncate_content=true&include_selection_data=true` + `${environment.apiBaseUrl}documents/?page=1&page_size=3&ordering=-created&truncate_content=true&include_selection_data=false` ) expect(req.request.method).toEqual('GET') req.flush({ count: 3, results: documents.slice(0, 3), }) + flushSelectionDataRequest(httpTestingController) expect(documentListViewService.hasNext(documents[0].id)).toBeTruthy() expect(documentListViewService.hasPrevious(documents[0].id)).toBeFalsy() documentListViewService.getNext(documents[0].id).subscribe((docId) => { @@ -447,7 +481,7 @@ describe('DocumentListViewService', () => { expect(documentListViewService.currentPage).toEqual(1) documentListViewService.pageSize = 3 httpTestingController.match( - `${environment.apiBaseUrl}documents/?page=1&page_size=3&ordering=-created&truncate_content=true&include_selection_data=true` + `${environment.apiBaseUrl}documents/?page=1&page_size=3&ordering=-created&truncate_content=true&include_selection_data=false` ) jest .spyOn(documentListViewService, 'getLastPage') @@ -462,7 +496,7 @@ describe('DocumentListViewService', () => { expect(reloadSpy).toHaveBeenCalled() expect(documentListViewService.currentPage).toEqual(2) const reqs = httpTestingController.match( - `${environment.apiBaseUrl}documents/?page=2&page_size=3&ordering=-created&truncate_content=true&include_selection_data=true` + `${environment.apiBaseUrl}documents/?page=2&page_size=3&ordering=-created&truncate_content=true&include_selection_data=false` ) expect(reqs.length).toBeGreaterThan(0) }) @@ -497,11 +531,11 @@ describe('DocumentListViewService', () => { .mockReturnValue(documents) documentListViewService.currentPage = 2 httpTestingController.match( - `${environment.apiBaseUrl}documents/?page=2&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true` + `${environment.apiBaseUrl}documents/?page=2&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false` ) documentListViewService.pageSize = 3 httpTestingController.match( - `${environment.apiBaseUrl}documents/?page=2&page_size=3&ordering=-created&truncate_content=true&include_selection_data=true` + `${environment.apiBaseUrl}documents/?page=2&page_size=3&ordering=-created&truncate_content=true&include_selection_data=false` ) const reloadSpy = jest.spyOn(documentListViewService, 'reload') documentListViewService.getPrevious(1).subscribe({ @@ -511,7 +545,7 @@ describe('DocumentListViewService', () => { expect(reloadSpy).toHaveBeenCalled() expect(documentListViewService.currentPage).toEqual(1) const reqs = httpTestingController.match( - `${environment.apiBaseUrl}documents/?page=1&page_size=3&ordering=-created&truncate_content=true&include_selection_data=true` + `${environment.apiBaseUrl}documents/?page=1&page_size=3&ordering=-created&truncate_content=true&include_selection_data=false` ) expect(reqs.length).toBeGreaterThan(0) }) @@ -524,10 +558,11 @@ describe('DocumentListViewService', () => { it('should support select a document', () => { documentListViewService.reload() const req = httpTestingController.expectOne( - `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true` + `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false` ) expect(req.request.method).toEqual('GET') req.flush(full_results) + flushSelectionDataRequest(httpTestingController) documentListViewService.toggleSelected(documents[0]) expect(documentListViewService.isSelected(documents[0])).toBeTruthy() documentListViewService.toggleSelected(documents[0]) @@ -537,10 +572,11 @@ describe('DocumentListViewService', () => { it('should support select all', () => { documentListViewService.reload() const reloadReq = httpTestingController.expectOne( - `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true` + `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false` ) expect(reloadReq.request.method).toEqual('GET') reloadReq.flush(full_results) + flushSelectionDataRequest(httpTestingController) documentListViewService.selectAll() expect(documentListViewService.allSelected).toBeTruthy() @@ -553,13 +589,14 @@ describe('DocumentListViewService', () => { it('should support select page', () => { documentListViewService.pageSize = 3 const req = httpTestingController.expectOne( - `${environment.apiBaseUrl}documents/?page=1&page_size=3&ordering=-created&truncate_content=true&include_selection_data=true` + `${environment.apiBaseUrl}documents/?page=1&page_size=3&ordering=-created&truncate_content=true&include_selection_data=false` ) expect(req.request.method).toEqual('GET') req.flush({ count: 3, results: documents.slice(0, 3), }) + flushSelectionDataRequest(httpTestingController) documentListViewService.selectPage() expect(documentListViewService.selected.size).toEqual(3) expect(documentListViewService.isSelected(documents[5])).toBeFalsy() @@ -568,10 +605,11 @@ describe('DocumentListViewService', () => { it('should support select range', () => { documentListViewService.reload() const req = httpTestingController.expectOne( - `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true` + `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false` ) expect(req.request.method).toEqual('GET') req.flush(full_results) + flushSelectionDataRequest(httpTestingController) documentListViewService.toggleSelected(documents[0]) expect(documentListViewService.isSelected(documents[0])).toBeTruthy() documentListViewService.selectRangeTo(documents[2]) @@ -583,9 +621,10 @@ describe('DocumentListViewService', () => { it('should clear all-selected mode when toggling a single document', () => { documentListViewService.reload() const req = httpTestingController.expectOne( - `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true` + `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false` ) req.flush(full_results) + flushSelectionDataRequest(httpTestingController) documentListViewService.selectAll() expect(documentListViewService.allSelected).toBeTruthy() @@ -599,9 +638,10 @@ describe('DocumentListViewService', () => { it('should clear all-selected mode when selecting a range', () => { documentListViewService.reload() const req = httpTestingController.expectOne( - `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true` + `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false` ) req.flush(full_results) + flushSelectionDataRequest(httpTestingController) documentListViewService.selectAll() documentListViewService.toggleSelected(documents[1]) @@ -619,22 +659,24 @@ describe('DocumentListViewService', () => { it('should support selection range reduction', () => { documentListViewService.reload() let req = httpTestingController.expectOne( - `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true` + `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false` ) expect(req.request.method).toEqual('GET') req.flush(full_results) + flushSelectionDataRequest(httpTestingController) documentListViewService.selectAll() expect(documentListViewService.selected.size).toEqual(6) documentListViewService.setFilterRules(filterRules) req = httpTestingController.expectOne( - `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true&tags__id__all=9` + `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false&tags__id__all=9` ) req.flush({ count: 3, results: documents.slice(0, 3), }) + flushSelectionDataRequest(httpTestingController, '?tags__id__all=9') expect(documentListViewService.allSelected).toBeTruthy() expect(documentListViewService.selected.size).toEqual(3) }) @@ -643,7 +685,7 @@ describe('DocumentListViewService', () => { const cancelSpy = jest.spyOn(documentListViewService, 'cancelPending') documentListViewService.reload() httpTestingController.expectOne( - `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true&tags__id__all=9` + `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false` ) expect(cancelSpy).toHaveBeenCalled() }) @@ -662,7 +704,7 @@ describe('DocumentListViewService', () => { documentListViewService.setFilterRules([]) expect(documentListViewService.sortField).toEqual('created') httpTestingController.expectOne( - `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true` + `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false` ) }) @@ -689,11 +731,11 @@ describe('DocumentListViewService', () => { expect(localStorageSpy).toHaveBeenCalled() // reload triggered httpTestingController.match( - `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true` + `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false` ) documentListViewService.displayFields = null httpTestingController.match( - `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true` + `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false` ) expect(documentListViewService.displayFields).toEqual( DEFAULT_DISPLAY_FIELDS.filter((f) => f.id !== DisplayField.ADDED).map( @@ -738,7 +780,7 @@ describe('DocumentListViewService', () => { it('should generate quick filter URL preserving default state', () => { documentListViewService.reload() httpTestingController.expectOne( - `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true` + `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false` ) const urlTree = documentListViewService.getQuickFilterUrl(filterRules) expect(urlTree).toBeDefined() diff --git a/src-ui/src/app/services/document-list-view.service.ts b/src-ui/src/app/services/document-list-view.service.ts index de1db2be6..6fb1c98dc 100644 --- a/src-ui/src/app/services/document-list-view.service.ts +++ b/src-ui/src/app/services/document-list-view.service.ts @@ -320,6 +320,13 @@ export class DocumentListViewService { this.error = null this.markChanged() let activeListViewState = this.activeListViewState + // Full-text search results are already narrowed by the search backend, so + // computing selection data inline there is cheap. A plain (unfiltered or + // ORM-filtered) browse can span the entire document set, so its selection + // data is fetched separately below instead of blocking the list response. + const isFullTextSearch = isFullTextFilterRule( + activeListViewState.filterRules + ) this.documentService .listFiltered( activeListViewState.currentPage, @@ -327,7 +334,10 @@ export class DocumentListViewService { activeListViewState.sortField, activeListViewState.sortReverse, activeListViewState.filterRules, - { truncate_content: true, include_selection_data: true } + { + truncate_content: true, + include_selection_data: isFullTextSearch, + } ) .pipe(takeUntil(this.unsubscribeNotifier)) .subscribe({ @@ -341,6 +351,22 @@ export class DocumentListViewService { this.syncSelectedToCurrentPage() this.markChanged() + if (!isFullTextSearch) { + this.documentService + .getFilterSelectionData(activeListViewState.filterRules) + .pipe(takeUntil(this.unsubscribeNotifier)) + .subscribe({ + next: (selectionData) => { + this.selectionData = selectionData + this.markChanged() + }, + error: () => { + this.selectionData = null + this.markChanged() + }, + }) + } + if (updateQueryParams && !this._activeSavedViewId) { let base = ['/documents'] this.router.navigate(base, { diff --git a/src-ui/src/app/services/rest/document.service.ts b/src-ui/src/app/services/rest/document.service.ts index bc87cb1fb..c6a94c7b7 100644 --- a/src-ui/src/app/services/rest/document.service.ts +++ b/src-ui/src/app/services/rest/document.service.ts @@ -1,3 +1,4 @@ +import { HttpParams } from '@angular/common/http' import { Injectable, inject } from '@angular/core' import { Observable } from 'rxjs' import { map } from 'rxjs/operators' @@ -398,6 +399,20 @@ export class DocumentService extends AbstractPaperlessService { ) } + getFilterSelectionData(filterRules: FilterRule[]): Observable { + let httpParams = new HttpParams() + const filterParams = queryParamsFromFilterRules(filterRules) + for (let key in filterParams) { + if (filterParams[key] != null) { + httpParams = httpParams.set(key, filterParams[key]) + } + } + return this.http.get( + this.getResourceUrl(null, 'filter_selection_data'), + { params: httpParams } + ) + } + getSuggestions(id: number): Observable { return this.http.get( this.getResourceUrl(id, 'suggestions') diff --git a/src/documents/tests/test_api_documents.py b/src/documents/tests/test_api_documents.py index b0ec51d68..21d6bdf2c 100644 --- a/src/documents/tests/test_api_documents.py +++ b/src/documents/tests/test_api_documents.py @@ -1241,7 +1241,7 @@ class TestDocumentApi(DirectoriesMixin, ConsumeTaskMixin, APITestCase): ], ) - def test_list_with_include_selection_data(self) -> None: + def test_selection_data_endpoint(self) -> None: correspondent = Correspondent.objects.create(name="c1") doc_type = DocumentType.objects.create(name="dt1") storage_path = StoragePath.objects.create(name="sp1") @@ -1259,30 +1259,28 @@ class TestDocumentApi(DirectoriesMixin, ConsumeTaskMixin, APITestCase): non_matching_doc.tags.add(Tag.objects.create(name="other")) response = self.client.get( - f"/api/documents/?tags__id__in={tag.id}&include_selection_data=true", + f"/api/documents/filter_selection_data/?tags__id__in={tag.id}", ) self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertIn("selection_data", response.data) + self.assertNotIn("results", response.data) selected_correspondent = next( item - for item in response.data["selection_data"]["selected_correspondents"] + for item in response.data["selected_correspondents"] if item["id"] == correspondent.id ) selected_tag = next( - item - for item in response.data["selection_data"]["selected_tags"] - if item["id"] == tag.id + item for item in response.data["selected_tags"] if item["id"] == tag.id ) selected_type = next( item - for item in response.data["selection_data"]["selected_document_types"] + for item in response.data["selected_document_types"] if item["id"] == doc_type.id ) selected_storage_path = next( item - for item in response.data["selection_data"]["selected_storage_paths"] + for item in response.data["selected_storage_paths"] if item["id"] == storage_path.id ) @@ -1291,6 +1289,17 @@ class TestDocumentApi(DirectoriesMixin, ConsumeTaskMixin, APITestCase): self.assertEqual(selected_type["document_count"], 1) self.assertEqual(selected_storage_path["document_count"], 1) + def test_list_no_longer_supports_include_selection_data(self) -> None: + """ + include_selection_data was never part of a stable release (beta-only, + introduced and removed within the 3.0.0-beta cycle) -- the plain list + endpoint should just ignore the param now rather than compute it inline. + """ + response = self.client.get("/api/documents/?include_selection_data=true") + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertNotIn("selection_data", response.data) + def test_statistics(self) -> None: doc1 = Document.objects.create( title="none1", diff --git a/src/documents/views.py b/src/documents/views.py index 20dbe9247..7731e752b 100644 --- a/src/documents/views.py +++ b/src/documents/views.py @@ -1184,24 +1184,21 @@ class DocumentViewSet( return response - def list(self, request, *args, **kwargs): - if not get_boolean( - str(request.query_params.get("include_selection_data", "false")), - ): - return super().list(request, *args, **kwargs) - + @extend_schema( + operation_id="documents_filter_selection_data", + description=( + "Returns per-tag/correspondent/document-type/storage-path/custom-field " + "document counts for the current filter, without paginating or " + "serializing the matching documents themselves. Split out from the " + "plain document list so that browsing the (potentially huge) unfiltered " + "document list doesn't pay for this aggregation on every request." + ), + responses={200: inline_serializer(name="SelectionData", fields={})}, + ) + @action(detail=False, methods=["get"], url_path="filter_selection_data") + def filter_selection_data(self, request, *args, **kwargs): queryset = self.filter_queryset(self.get_queryset()) - selection_data = self._get_selection_data_for_queryset(queryset) - - page = self.paginate_queryset(queryset) - if page is not None: - serializer = self.get_serializer(page, many=True) - response = self.get_paginated_response(serializer.data) - response.data["selection_data"] = selection_data - return response - - serializer = self.get_serializer(queryset, many=True) - return Response({"results": serializer.data, "selection_data": selection_data}) + return Response(self._get_selection_data_for_queryset(queryset)) def destroy(self, request, *args, **kwargs): from documents.search import get_backend