From 341815cc032b6fe05d3245b8fda16e1eab9d60ad Mon Sep 17 00:00:00 2001 From: shamoon <4887959+shamoon@users.noreply.github.com> Date: Sun, 17 Dec 2023 16:39:45 -0800 Subject: [PATCH] Enhancement: document link field fixes (#5020) * Implement more efficient getFew for document retrieval * Filter out parent document ID & already-selected documents * Clip very long document titles --- .../document-link.component.scss | 12 ++-- .../document-link.component.spec.ts | 58 ++++++++++++++++--- .../document-link/document-link.component.ts | 17 ++++-- .../document-detail.component.html | 2 +- .../rest/abstract-paperless-service.spec.ts | 15 +++++ .../rest/abstract-paperless-service.ts | 13 +++++ 6 files changed, 99 insertions(+), 18 deletions(-) diff --git a/src-ui/src/app/components/common/input/document-link/document-link.component.scss b/src-ui/src/app/components/common/input/document-link/document-link.component.scss index bcaa4e849..51f6fa055 100644 --- a/src-ui/src/app/components/common/input/document-link/document-link.component.scss +++ b/src-ui/src/app/components/common/input/document-link/document-link.component.scss @@ -1,6 +1,10 @@ -::ng-deep .ng-select-container .ng-value-container .ng-value { - background-color: transparent !important; - border-color: transparent; +::ng-deep .ng-select-container .ng-value-container { + overflow: hidden; + + .ng-value { + background-color: transparent !important; + border-color: transparent; + } } .sidebaricon { @@ -9,6 +13,4 @@ .badge { font-size: .75rem; - // --bs-primary: var(--pngx-bg-alt); - // color: var(--pngx-primary-text-contrast); } diff --git a/src-ui/src/app/components/common/input/document-link/document-link.component.spec.ts b/src-ui/src/app/components/common/input/document-link/document-link.component.spec.ts index d1af7ab2f..e00460ec5 100644 --- a/src-ui/src/app/components/common/input/document-link/document-link.component.spec.ts +++ b/src-ui/src/app/components/common/input/document-link/document-link.component.spec.ts @@ -20,6 +20,10 @@ const documents = [ id: 12, title: 'Document 12 bar', }, + { + id: 16, + title: 'Document 16 bar', + }, { id: 23, title: 'Document 23 bar', @@ -48,10 +52,15 @@ describe('DocumentLinkComponent', () => { fixture.detectChanges() }) - it('should retrieve selected documents from APIs', () => { - const getSpy = jest.spyOn(documentService, 'getCachedMany') + it('should retrieve selected documents from API', () => { + const getSpy = jest.spyOn(documentService, 'getFew') getSpy.mockImplementation((ids) => { - return of(documents.filter((d) => ids.includes(d.id))) + const docs = documents.filter((d) => ids.includes(d.id)) + return of({ + count: docs.length, + all: docs.map((d) => d.id), + results: docs, + }) }) component.writeValue([1]) expect(getSpy).toHaveBeenCalled() @@ -85,12 +94,18 @@ describe('DocumentLinkComponent', () => { }) it('should load values correctly', () => { - jest.spyOn(documentService, 'getCachedMany').mockImplementation((ids) => { - return of(documents.filter((d) => ids.includes(d.id))) + const getSpy = jest.spyOn(documentService, 'getFew') + getSpy.mockImplementation((ids) => { + const docs = documents.filter((d) => ids.includes(d.id)) + return of({ + count: docs.length, + all: docs.map((d) => d.id), + results: docs, + }) }) component.writeValue([12, 23]) expect(component.value).toEqual([12, 23]) - expect(component.selectedDocuments).toEqual([documents[1], documents[2]]) + expect(component.selectedDocuments).toEqual([documents[1], documents[3]]) component.writeValue(null) expect(component.value).toEqual([]) expect(component.selectedDocuments).toEqual([]) @@ -100,9 +115,14 @@ describe('DocumentLinkComponent', () => { }) it('should support unselect', () => { - const getSpy = jest.spyOn(documentService, 'getCachedMany') + const getSpy = jest.spyOn(documentService, 'getFew') getSpy.mockImplementation((ids) => { - return of(documents.filter((d) => ids.includes(d.id))) + const docs = documents.filter((d) => ids.includes(d.id)) + return of({ + count: docs.length, + all: docs.map((d) => d.id), + results: docs, + }) }) component.writeValue([12, 23]) component.unselect({ id: 23 }) @@ -115,4 +135,26 @@ describe('DocumentLinkComponent', () => { expect(component.compareDocuments(documents[0], { id: 2 })).toBeFalsy() expect(component.trackByFn(documents[1])).toEqual(12) }) + + it('should not include the current document or already selected documents in results', () => { + let foundDocs + component.foundDocuments$.subscribe((found) => (foundDocs = found)) + component.parentDocumentID = 23 + component.selectedDocuments = [documents[2]] + const listSpy = jest.spyOn(documentService, 'listFiltered') + listSpy.mockImplementation( + (page, pageSize, sortField, sortReverse, filterRules, extraParams) => { + const docs = documents.filter((d) => + d.title.includes(filterRules[0].value) + ) + return of({ + count: docs.length, + results: docs, + all: docs.map((d) => d.id), + }) + } + ) + component.documentsInput$.next('bar') + expect(foundDocs).toEqual([documents[1]]) + }) }) diff --git a/src-ui/src/app/components/common/input/document-link/document-link.component.ts b/src-ui/src/app/components/common/input/document-link/document-link.component.ts index dd7118074..77a0fb99a 100644 --- a/src-ui/src/app/components/common/input/document-link/document-link.component.ts +++ b/src-ui/src/app/components/common/input/document-link/document-link.component.ts @@ -43,6 +43,9 @@ export class DocumentLinkComponent @Input() notFoundText: string = $localize`No documents found` + @Input() + parentDocumentID: number + constructor(private documentsService: DocumentService) { super() } @@ -58,11 +61,11 @@ export class DocumentLinkComponent } else { this.loading = true this.documentsService - .getCachedMany(documentIDs) + .getFew(documentIDs, { fields: 'id,title' }) .pipe(takeUntil(this.unsubscribeNotifier)) - .subscribe((documents) => { + .subscribe((documentResults) => { this.loading = false - this.selectedDocuments = documents + this.selectedDocuments = documentResults.results super.writeValue(documentIDs) }) } @@ -86,7 +89,13 @@ export class DocumentLinkComponent { truncate_content: true } ) .pipe( - map((results) => results.results), + map((results) => + results.results.filter( + (d) => + d.id !== this.parentDocumentID && + !this.selectedDocuments.find((sd) => sd.id === d.id) + ) + ), catchError(() => of([])), // empty on error tap(() => (this.loading = false)) ) diff --git a/src-ui/src/app/components/document-detail/document-detail.component.html b/src-ui/src/app/components/document-detail/document-detail.component.html index 192680e57..3211e60ed 100644 --- a/src-ui/src/app/components/document-detail/document-detail.component.html +++ b/src-ui/src/app/components/document-detail/document-detail.component.html @@ -124,7 +124,7 @@ - + diff --git a/src-ui/src/app/services/rest/abstract-paperless-service.spec.ts b/src-ui/src/app/services/rest/abstract-paperless-service.spec.ts index 92ff923f4..7b5254bfd 100644 --- a/src-ui/src/app/services/rest/abstract-paperless-service.spec.ts +++ b/src-ui/src/app/services/rest/abstract-paperless-service.spec.ts @@ -96,6 +96,21 @@ export const commonAbstractPaperlessServiceTests = (endpoint, ServiceClass) => { expect(req.request.method).toEqual('PATCH') req.flush([]) }) + + test('should call appropriate api endpoint for get a few objects', () => { + subscription = service.getFew([1, 2, 3]).subscribe() + const req = httpTestingController.expectOne( + `${environment.apiBaseUrl}${endpoint}/?id__in=1,2,3` + ) + expect(req.request.method).toEqual('GET') + req.flush([]) + subscription = service.getFew([4, 5, 6], { foo: 'bar' }).subscribe() + const req2 = httpTestingController.expectOne( + `${environment.apiBaseUrl}${endpoint}/?id__in=4,5,6&foo=bar` + ) + expect(req2.request.method).toEqual('GET') + req2.flush([]) + }) }) beforeEach(() => { 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 14735b1ad..9305ed8b6 100644 --- a/src-ui/src/app/services/rest/abstract-paperless-service.ts +++ b/src-ui/src/app/services/rest/abstract-paperless-service.ts @@ -91,6 +91,19 @@ export abstract class AbstractPaperlessService { ) } + getFew(ids: number[], extraParams?): Observable> { + let httpParams = new HttpParams() + httpParams = httpParams.set('id__in', ids.join(',')) + for (let extraParamKey in extraParams) { + if (extraParams[extraParamKey] != null) { + httpParams = httpParams.set(extraParamKey, extraParams[extraParamKey]) + } + } + return this.http.get>(this.getResourceUrl(), { + params: httpParams, + }) + } + clearCache() { this._listAll = null }