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 d6e8a5072..9694ed1e9 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 @@ -191,6 +191,14 @@ describe('BulkEditorComponent', () => { }) afterEach(async () => { + // A filter_selection_data request now fires concurrently with every + // non-search reload(), independent of whether a given test flushes or + // even inspects the primary list response. Drain any left unclaimed. + httpTestingController.match( + (request) => + request.url === + `${environment.apiBaseUrl}documents/filter_selection_data/` + ) httpTestingController.verify() }) 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 8ef096303..7c478de97 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 @@ -137,6 +137,16 @@ describe('DocumentListViewService', () => { afterEach(() => { documentListViewService.cancelPending() + // A filter_selection_data request now fires concurrently with every + // non-search reload(), independent of whether the test cares about or + // flushes the primary list response. Drain any that a test didn't + // explicitly claim via flushSelectionDataRequest, so unrelated tests + // don't have to know about this follow-up request to pass verify(). + httpTestingController.match( + (request) => + request.url === + `${environment.apiBaseUrl}documents/filter_selection_data/` + ) httpTestingController.verify() sessionStorage.clear() localStorage.clear() @@ -434,7 +444,10 @@ describe('DocumentListViewService', () => { count: 3, results: documents.slice(0, 3), }) - flushSelectionDataRequest(httpTestingController) + // two reload()s ran above (setFilterRules, then pageSize), each firing + // its own concurrent filter_selection_data request with an identical + // (unfiltered) URL; this test doesn't assert on selectionData, so let + // afterEach's drain step clean both up rather than disambiguating here. expect(documentListViewService.hasNext(documents[0].id)).toBeTruthy() expect(documentListViewService.hasPrevious(documents[0].id)).toBeFalsy() documentListViewService.getNext(documents[0].id).subscribe((docId) => { 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 6fb1c98dc..1b7c3ac7a 100644 --- a/src-ui/src/app/services/document-list-view.service.ts +++ b/src-ui/src/app/services/document-list-view.service.ts @@ -314,6 +314,22 @@ export class DocumentListViewService { } } + private loadFilterSelectionData(filterRules: FilterRule[]) { + this.documentService + .getFilterSelectionData(filterRules) + .pipe(takeUntil(this.unsubscribeNotifier)) + .subscribe({ + next: (selectionData) => { + this.selectionData = selectionData + this.markChanged() + }, + error: () => { + this.selectionData = null + this.markChanged() + }, + }) + } + reload(onFinish?, updateQueryParams: boolean = true) { this.cancelPending() this.isReloading = true @@ -323,10 +339,14 @@ export class DocumentListViewService { // 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. + // data is fetched separately -- concurrently with the list itself, rather + // than blocking or waiting on it. const isFullTextSearch = isFullTextFilterRule( activeListViewState.filterRules ) + if (!isFullTextSearch) { + this.loadFilterSelectionData(activeListViewState.filterRules) + } this.documentService .listFiltered( activeListViewState.currentPage, @@ -342,31 +362,17 @@ export class DocumentListViewService { .pipe(takeUntil(this.unsubscribeNotifier)) .subscribe({ next: (result) => { - const resultWithSelectionData = result as DocumentResults this.initialized = true this.isReloading = false activeListViewState.collectionSize = result.count activeListViewState.documents = result.results - this.selectionData = resultWithSelectionData.selection_data ?? null + if (isFullTextSearch) { + this.selectionData = + (result as DocumentResults).selection_data ?? null + } 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, { @@ -402,6 +408,9 @@ export class DocumentListViewService { // e.g. field was deleted this.sortField = 'created' } else { + // cancel the concurrently-fired selection-data request too, so it + // can't resolve afterward and clobber this reset with stale data + this.cancelPending() this.selectionData = null let errorMessage if ( diff --git a/src-ui/src/app/services/rest/abstract-paperless-service.ts b/src-ui/src/app/services/rest/abstract-paperless-service.ts index d293d6453..8b7633256 100644 --- a/src-ui/src/app/services/rest/abstract-paperless-service.ts +++ b/src-ui/src/app/services/rest/abstract-paperless-service.ts @@ -41,6 +41,24 @@ export abstract class AbstractPaperlessService { } } + /** + * Merges a plain params object into an HttpParams instance, skipping + * null/undefined values so they aren't serialized as literal "null" / + * "undefined" query string entries. + */ + protected withParams( + params, + base: HttpParams = new HttpParams() + ): HttpParams { + let httpParams = base + for (let key in params) { + if (params[key] != null) { + httpParams = httpParams.set(key, params[key]) + } + } + return httpParams + } + list( page?: number, pageSize?: number, @@ -60,11 +78,7 @@ export abstract class AbstractPaperlessService { if (ordering) { httpParams = httpParams.set('ordering', ordering) } - for (let extraParamKey in extraParams) { - if (extraParams[extraParamKey] != null) { - httpParams = httpParams.set(extraParamKey, extraParams[extraParamKey]) - } - } + httpParams = this.withParams(extraParams, httpParams) return this.http .get>(this.getResourceUrl(), { params: httpParams, @@ -113,11 +127,7 @@ export abstract class AbstractPaperlessService { httpParams = httpParams.set('id__in', ids.join(',')) httpParams = httpParams.set('ordering', '-id') httpParams = httpParams.set('page_size', 1000) - for (let extraParamKey in extraParams) { - if (extraParams[extraParamKey] != null) { - httpParams = httpParams.set(extraParamKey, extraParams[extraParamKey]) - } - } + httpParams = this.withParams(extraParams, httpParams) return this.http .get>(this.getResourceUrl(), { params: httpParams, diff --git a/src-ui/src/app/services/rest/document.service.ts b/src-ui/src/app/services/rest/document.service.ts index c6a94c7b7..0a117ee14 100644 --- a/src-ui/src/app/services/rest/document.service.ts +++ b/src-ui/src/app/services/rest/document.service.ts @@ -1,4 +1,3 @@ -import { HttpParams } from '@angular/common/http' import { Injectable, inject } from '@angular/core' import { Observable } from 'rxjs' import { map } from 'rxjs/operators' @@ -400,16 +399,9 @@ 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 } + { params: this.withParams(queryParamsFromFilterRules(filterRules)) } ) } diff --git a/src/documents/views.py b/src/documents/views.py index 7731e752b..d99109844 100644 --- a/src/documents/views.py +++ b/src/documents/views.py @@ -1034,7 +1034,11 @@ class DocumentViewSet( ], } - def get_queryset(self): + def _base_document_queryset(self): + # Root documents only, with the annotations that filter_backends rely + # on (effective_content for SearchFilter, num_notes for ordering) -- + # but no select_related/prefetch_related, since those only matter for + # serializing documents, not for filtering, ordering, or aggregating. latest_version_content = Subquery( Document.objects.filter(root_document=OuterRef("pk")) .order_by("-id") @@ -1046,6 +1050,11 @@ class DocumentViewSet( .order_by("-created", "-id") .annotate(effective_content=Coalesce(latest_version_content, F("content"))) .annotate(num_notes=Count("notes")) + ) + + def get_queryset(self): + return ( + self._base_document_queryset() .select_related("correspondent", "storage_path", "document_type", "owner") .prefetch_related( Prefetch( @@ -1197,7 +1206,7 @@ class DocumentViewSet( ) @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()) + queryset = self.filter_queryset(self._base_document_queryset()) return Response(self._get_selection_data_for_queryset(queryset)) def destroy(self, request, *args, **kwargs):