From 73ef14f37a6b24877fe3f2df4796206364bd9be7 Mon Sep 17 00:00:00 2001 From: shamoon <4887959+shamoon@users.noreply.github.com> Date: Tue, 1 Sep 2026 15:05:58 -0700 Subject: [PATCH] Fix/chore: refactor some signal-backed conversion technical debt (#13902) --- .../components/admin/trash/trash.component.ts | 5 +- .../app-frame/app-frame.component.spec.ts | 17 ++ .../app-frame/app-frame.component.ts | 59 +++-- .../global-search/global-search.component.ts | 10 +- .../workflow-edit-dialog.component.spec.ts | 32 ++- .../workflow-edit-dialog.component.ts | 14 +- .../filterable-dropdown.component.spec.ts | 14 +- .../filterable-dropdown.component.ts | 229 ++++++++++-------- ...permissions-filter-dropdown.component.html | 19 +- ...missions-filter-dropdown.component.spec.ts | 69 +++--- .../permissions-filter-dropdown.component.ts | 106 ++++---- .../document-detail.component.spec.ts | 63 ++++- .../document-detail.component.ts | 29 ++- .../document-list/document-list.component.ts | 5 +- .../filter-editor.component.spec.ts | 106 ++++++-- .../filter-editor/filter-editor.component.ts | 65 +++-- .../src/app/services/settings.service.spec.ts | 42 ++++ src-ui/src/app/services/settings.service.ts | 20 +- 18 files changed, 580 insertions(+), 324 deletions(-) diff --git a/src-ui/src/app/components/admin/trash/trash.component.ts b/src-ui/src/app/components/admin/trash/trash.component.ts index b64f3a88f..2f8fbe841 100644 --- a/src-ui/src/app/components/admin/trash/trash.component.ts +++ b/src-ui/src/app/components/admin/trash/trash.component.ts @@ -41,6 +41,8 @@ export class TrashComponent private modalService = inject(NgbModal) private settingsService = inject(SettingsService) private router = inject(Router) + private readonly emptyTrashDelaySetting = + this.settingsService.getSignal(SETTINGS_KEYS.EMPTY_TRASH_DELAY) readonly documentsInTrash = signal([]) readonly selectedDocuments = signal>(new Set()) @@ -200,8 +202,7 @@ export class TrashComponent } getDaysRemaining(document: Document): number { - this.settingsService.trackChanges() - const delay = this.settingsService.get(SETTINGS_KEYS.EMPTY_TRASH_DELAY) + const delay = this.emptyTrashDelaySetting() const diff = new Date().getTime() - new Date(document.deleted_at).getTime() const days = Math.ceil(diff / (1000 * 3600 * 24)) return delay - days diff --git a/src-ui/src/app/components/app-frame/app-frame.component.spec.ts b/src-ui/src/app/components/app-frame/app-frame.component.spec.ts index 306671817..d9b1acda4 100644 --- a/src-ui/src/app/components/app-frame/app-frame.component.spec.ts +++ b/src-ui/src/app/components/app-frame/app-frame.component.spec.ts @@ -193,6 +193,23 @@ describe('AppFrameComponent', () => { expect(savedViewSpy).toHaveBeenCalled() }) + it('should update reinitialized signal-backed settings without manual change detection', async () => { + settingsService.initializeSettings().subscribe() + httpTestingController + .expectOne(`${environment.apiBaseUrl}ui_settings/`) + .flush({ + settings: { app_title: 'Reactive title' }, + user: {}, + permissions: [], + }) + + await fixture.whenStable() + + expect( + fixture.nativeElement.querySelector('.brand-title').textContent + ).toBe('Reactive title') + }) + it('should check for update if enabled', () => { const updateCheckSpy = jest.spyOn(remoteVersionService, 'checkForUpdates') updateCheckSpy.mockImplementation(() => { diff --git a/src-ui/src/app/components/app-frame/app-frame.component.ts b/src-ui/src/app/components/app-frame/app-frame.component.ts index a495741f6..59a2ba18b 100644 --- a/src-ui/src/app/components/app-frame/app-frame.component.ts +++ b/src-ui/src/app/components/app-frame/app-frame.component.ts @@ -98,6 +98,29 @@ export class AppFrameComponent readonly isMenuCollapsed = signal(true) readonly slimSidebarAnimating = signal(false) readonly mobileSearchHidden = signal(false) + private readonly versionSetting = this.settingsService.getSignal( + SETTINGS_KEYS.VERSION + ) + private readonly appTitleSetting = this.settingsService.getSignal( + SETTINGS_KEYS.APP_TITLE + ) + private readonly appLogoSetting = this.settingsService.getSignal( + SETTINGS_KEYS.APP_LOGO + ) + private readonly slimSidebarSetting = this.settingsService.getSignal( + SETTINGS_KEYS.SLIM_SIDEBAR + ) + private readonly attributesSectionsCollapsedSetting = + this.settingsService.getSignal( + SETTINGS_KEYS.ATTRIBUTES_SECTIONS_COLLAPSED + ) + private readonly aiEnabledSetting = this.settingsService.getSignal( + SETTINGS_KEYS.AI_ENABLED + ) + private readonly sidebarViewsShowCountSetting = + this.settingsService.getSignal( + SETTINGS_KEYS.SIDEBAR_VIEWS_SHOW_COUNT + ) private lastScrollY: number = 0 constructor() { @@ -191,33 +214,23 @@ export class AppFrameComponent } get versionString(): string { - this.settingsService.trackChanges() - return `${environment.appTitle} v${this.settingsService.get(SETTINGS_KEYS.VERSION)}${environment.tag === 'prod' ? '' : ` #${environment.tag}`}` + return `${environment.appTitle} v${this.versionSetting()}${environment.tag === 'prod' ? '' : ` #${environment.tag}`}` } get appTitle(): string { - this.settingsService.trackChanges() - return ( - this.settingsService.get(SETTINGS_KEYS.APP_TITLE) || environment.appTitle - ) + return this.appTitleSetting() || environment.appTitle } get customAppTitle(): string { - this.settingsService.trackChanges() - return this.settingsService.get(SETTINGS_KEYS.APP_TITLE) + return this.appTitleSetting() } get hasCustomBranding(): boolean { - this.settingsService.trackChanges() - return !!( - this.settingsService.get(SETTINGS_KEYS.APP_TITLE)?.length || - this.settingsService.get(SETTINGS_KEYS.APP_LOGO)?.length - ) + return !!(this.appTitleSetting()?.length || this.appLogoSetting()?.length) } get customAppLogo(): string { - this.settingsService.trackChanges() - const logo = this.settingsService.get(SETTINGS_KEYS.APP_LOGO) + const logo = this.appLogoSetting() return logo?.length ? environment.apiBaseUrl.replace(/\/api\/$/, logo) : null @@ -262,8 +275,7 @@ export class AppFrameComponent } get slimSidebarEnabled(): boolean { - this.settingsService.trackChanges() - return this.settingsService.get(SETTINGS_KEYS.SLIM_SIDEBAR) + return this.slimSidebarSetting() } set slimSidebarEnabled(enabled: boolean) { @@ -286,10 +298,9 @@ export class AppFrameComponent } get attributesSectionsCollapsed(): boolean { - this.settingsService.trackChanges() - return this.settingsService - .get(SETTINGS_KEYS.ATTRIBUTES_SECTIONS_COLLAPSED) - ?.includes(CollapsibleSection.ATTRIBUTES) + return this.attributesSectionsCollapsedSetting()?.includes( + CollapsibleSection.ATTRIBUTES + ) } set attributesSectionsCollapsed(collapsed: boolean) { @@ -312,8 +323,7 @@ export class AppFrameComponent } get aiEnabled(): boolean { - this.settingsService.trackChanges() - return this.settingsService.get(SETTINGS_KEYS.AI_ENABLED) + return this.aiEnabledSetting() } @HostListener('window:resize') @@ -480,9 +490,8 @@ export class AppFrameComponent } get showSidebarCounts(): boolean { - this.settingsService.trackChanges() return ( - this.settingsService.get(SETTINGS_KEYS.SIDEBAR_VIEWS_SHOW_COUNT) && + this.sidebarViewsShowCountSetting() && !this.settingsService.organizingSidebarSavedViews() ) } diff --git a/src-ui/src/app/components/app-frame/global-search/global-search.component.ts b/src-ui/src/app/components/app-frame/global-search/global-search.component.ts index 685810c42..9a70189a6 100644 --- a/src-ui/src/app/components/app-frame/global-search/global-search.component.ts +++ b/src-ui/src/app/components/app-frame/global-search/global-search.component.ts @@ -81,6 +81,10 @@ export class GlobalSearchComponent implements OnInit { private hotkeyService = inject(HotKeyService) private settingsService = inject(SettingsService) private locationStrategy = inject(LocationStrategy) + private readonly searchFullTypeSetting = + this.settingsService.getSignal( + SETTINGS_KEYS.SEARCH_FULL_TYPE + ) public DataType = DataType readonly query = signal(null) @@ -97,11 +101,7 @@ export class GlobalSearchComponent implements OnInit { @ViewChildren('secondaryButton') secondaryButtons: QueryList get useAdvancedForFullSearch(): boolean { - this.settingsService.trackChanges() - return ( - this.settingsService.get(SETTINGS_KEYS.SEARCH_FULL_TYPE) === - GlobalSearchType.ADVANCED - ) + return this.searchFullTypeSetting() === GlobalSearchType.ADVANCED } constructor() { diff --git a/src-ui/src/app/components/common/edit-dialog/workflow-edit-dialog/workflow-edit-dialog.component.spec.ts b/src-ui/src/app/components/common/edit-dialog/workflow-edit-dialog/workflow-edit-dialog.component.spec.ts index d9c365673..3c4d8f790 100644 --- a/src-ui/src/app/components/common/edit-dialog/workflow-edit-dialog/workflow-edit-dialog.component.spec.ts +++ b/src-ui/src/app/components/common/edit-dialog/workflow-edit-dialog/workflow-edit-dialog.component.spec.ts @@ -196,6 +196,16 @@ describe('WorkflowEditDialogComponent', () => { fixture.detectChanges() }) + function setActionSettings({ + email = true, + remoteOcr = true, + ai = true, + } = {}) { + settingsService.set(SETTINGS_KEYS.EMAIL_ENABLED, email) + settingsService.set(SETTINGS_KEYS.REMOTE_OCR_CONFIGURED, remoteOcr) + settingsService.set(SETTINGS_KEYS.AI_ENABLED, ai) + } + it('should support create and edit modes, support adding triggers and actions on new workflow', () => { component.dialogMode.set(EditDialogMode.CREATE) const createTitleSpy = jest.spyOn(component, 'getCreateTitle') @@ -218,7 +228,7 @@ describe('WorkflowEditDialogComponent', () => { }) it('should return source options, type options, type name, schedule date field options', () => { - jest.spyOn(settingsService, 'get').mockReturnValue(true) + setActionSettings() component.ngOnInit() expect(component.sourceOptions).toEqual(DOCUMENT_SOURCE_OPTIONS) expect(component.triggerTypeOptions).toEqual(WORKFLOW_TYPE_OPTIONS) @@ -242,7 +252,7 @@ describe('WorkflowEditDialogComponent', () => { ) // Email, remote OCR and AI all disabled - jest.spyOn(settingsService, 'get').mockReturnValue(false) + setActionSettings({ email: false, remoteOcr: false, ai: false }) component.ngOnInit() expect(component.actionTypeOptions).toEqual( WORKFLOW_ACTION_OPTIONS.filter( @@ -255,7 +265,7 @@ describe('WorkflowEditDialogComponent', () => { }) it('should offer remote OCR only for consumption workflows', () => { - jest.spyOn(settingsService, 'get').mockReturnValue(true) + setActionSettings() // A consumption trigger makes the action reachable component.object = { @@ -285,7 +295,7 @@ describe('WorkflowEditDialogComponent', () => { }) it('should offer remote OCR on a trigger added to a new workflow', () => { - jest.spyOn(settingsService, 'get').mockReturnValue(true) + setActionSettings() component.ngOnInit() // Nothing for the action to apply to yet @@ -311,7 +321,7 @@ describe('WorkflowEditDialogComponent', () => { }) it('should keep remote OCR listed when an action already uses it', () => { - jest.spyOn(settingsService, 'get').mockReturnValue(true) + setActionSettings() // Otherwise changing the trigger would silently blank the selection component.object = { @@ -329,9 +339,7 @@ describe('WorkflowEditDialogComponent', () => { }) it('should not offer remote OCR when no engine is configured', () => { - jest - .spyOn(settingsService, 'get') - .mockImplementation((key) => key !== SETTINGS_KEYS.REMOTE_OCR_CONFIGURED) + setActionSettings({ remoteOcr: false }) component.object = { name: 'Workflow 1', @@ -348,7 +356,7 @@ describe('WorkflowEditDialogComponent', () => { }) it('should offer apply AI suggestions unless every trigger is consumption', () => { - jest.spyOn(settingsService, 'get').mockReturnValue(true) + setActionSettings() // Consumption runs before the document has been parsed, so there would be // no content to make suggestions from @@ -382,7 +390,7 @@ describe('WorkflowEditDialogComponent', () => { }) it('should keep apply AI suggestions listed when an action already uses it', () => { - jest.spyOn(settingsService, 'get').mockReturnValue(true) + setActionSettings() // Otherwise changing the trigger would silently blank the selection component.object = { @@ -400,9 +408,7 @@ describe('WorkflowEditDialogComponent', () => { }) it('should not offer apply AI suggestions when AI is disabled', () => { - jest - .spyOn(settingsService, 'get') - .mockImplementation((key) => key !== SETTINGS_KEYS.AI_ENABLED) + setActionSettings({ ai: false }) component.object = { name: 'Workflow 1', diff --git a/src-ui/src/app/components/common/edit-dialog/workflow-edit-dialog/workflow-edit-dialog.component.ts b/src-ui/src/app/components/common/edit-dialog/workflow-edit-dialog/workflow-edit-dialog.component.ts index bb8cc58fa..76f9ac2af 100644 --- a/src-ui/src/app/components/common/edit-dialog/workflow-edit-dialog/workflow-edit-dialog.component.ts +++ b/src-ui/src/app/components/common/edit-dialog/workflow-edit-dialog/workflow-edit-dialog.component.ts @@ -537,6 +537,13 @@ export class WorkflowEditDialogComponent readonly dateCustomFields = computed(() => this.customFields()?.filter((f) => f.data_type === CustomFieldDataType.Date) ) + private readonly emailEnabledSetting = + this.settingsService.getSignal(SETTINGS_KEYS.EMAIL_ENABLED) + private readonly remoteOcrConfiguredSetting = + this.settingsService.getSignal(SETTINGS_KEYS.REMOTE_OCR_CONFIGURED) + private readonly aiEnabledSetting = this.settingsService.getSignal( + SETTINGS_KEYS.AI_ENABLED + ) expandedItem: number = null @@ -589,7 +596,7 @@ export class WorkflowEditDialogComponent private getAllowedActionTypes() { let allowed = WORKFLOW_ACTION_OPTIONS - if (!this.settingsService.get(SETTINGS_KEYS.EMAIL_ENABLED)) { + if (!this.emailEnabledSetting()) { allowed = allowed.filter((a) => a.id !== WorkflowActionType.Email) } @@ -597,7 +604,7 @@ export class WorkflowEditDialogComponent // offered for workflows that run at consumption. const formWorkflow: Workflow = this.objectForm?.value const remoteOcrUsable = - this.settingsService.get(SETTINGS_KEYS.REMOTE_OCR_CONFIGURED) && + this.remoteOcrConfiguredSetting() && (formWorkflow?.triggers?.some( (trigger) => trigger.type === WorkflowTriggerType.Consumption ) || @@ -612,7 +619,7 @@ export class WorkflowEditDialogComponent // once every trigger is consumption, so it stays offered on a workflow // that has no triggers yet. const aiSuggestionsUsable = - this.settingsService.get(SETTINGS_KEYS.AI_ENABLED) && + this.aiEnabledSetting() && (!formWorkflow?.triggers?.length || formWorkflow.triggers.some( (trigger) => trigger.type !== WorkflowTriggerType.Consumption @@ -1362,7 +1369,6 @@ export class WorkflowEditDialogComponent } get actionTypeOptions() { - this.settingsService.trackChanges() // Computed on read rather than cached return this.getAllowedActionTypes() } diff --git a/src-ui/src/app/components/common/filterable-dropdown/filterable-dropdown.component.spec.ts b/src-ui/src/app/components/common/filterable-dropdown/filterable-dropdown.component.spec.ts index e6f2205f0..c58244018 100644 --- a/src-ui/src/app/components/common/filterable-dropdown/filterable-dropdown.component.spec.ts +++ b/src-ui/src/app/components/common/filterable-dropdown/filterable-dropdown.component.spec.ts @@ -839,7 +839,9 @@ describe('FilterableDropdownComponent & FilterableDropdownSelectionModel', () => selectionModel.items = [memoRoot] selectionModel.documentCounts = [{ id: memoRoot.id, document_count: 9 }] - const getRootDocCount = (selectionModel as any).createRootDocCounter() + const getRootDocCount = (selectionModel as any).createRootDocCounter( + selectionModel.items + ) expect(getRootDocCount(memoRoot.id)).toEqual(9) selectionModel.documentCounts = [] @@ -855,7 +857,9 @@ describe('FilterableDropdownComponent & FilterableDropdownSelectionModel', () => selectionModel.items = [rootWithoutSelection] selectionModel.documentCounts = [] - const getRootDocCount = (selectionModel as any).createRootDocCounter() + const getRootDocCount = (selectionModel as any).createRootDocCounter( + selectionModel.items + ) expect(getRootDocCount(rootWithoutSelection.id)).toEqual(4) }) @@ -865,7 +869,9 @@ describe('FilterableDropdownComponent & FilterableDropdownSelectionModel', () => selectionModel.items = [rootWithoutCounts] selectionModel.documentCounts = [] - const getRootDocCount = (selectionModel as any).createRootDocCounter() + const getRootDocCount = (selectionModel as any).createRootDocCounter( + selectionModel.items + ) expect(getRootDocCount(rootWithoutCounts.id)).toEqual(0) }) @@ -966,7 +972,7 @@ describe('FilterableDropdownComponent & FilterableDropdownSelectionModel', () => component.selectionModel['temporarySelectionStates'].set(id, state) const changedSpy = jest.spyOn(component.selectionModel.changed, 'next') component.selectionModel.exclude(id) - expect(component.selectionModel.temporaryLogicalOperator).toBe( + expect(component.selectionModel.temporaryLogicalOperator()).toBe( LogicalOperator.And ) expect(component.selectionModel['temporarySelectionStates'].get(id)).toBe( diff --git a/src-ui/src/app/components/common/filterable-dropdown/filterable-dropdown.component.ts b/src-ui/src/app/components/common/filterable-dropdown/filterable-dropdown.component.ts index ca5c38e9a..abd2241eb 100644 --- a/src-ui/src/app/components/common/filterable-dropdown/filterable-dropdown.component.ts +++ b/src-ui/src/app/components/common/filterable-dropdown/filterable-dropdown.component.ts @@ -64,43 +64,56 @@ export class FilterableDropdownSelectionModel { manyToOne = false singleSelect = false - private _logicalOperator: LogicalOperator = LogicalOperator.And - temporaryLogicalOperator: LogicalOperator = this._logicalOperator - private _intersection: Intersection = Intersection.Include - temporaryIntersection: Intersection = this._intersection - private _documentCounts: SelectionDataItem[] = [] + private readonly _logicalOperator = signal(LogicalOperator.And) + readonly temporaryLogicalOperator = signal(LogicalOperator.And) + private readonly _intersection = signal(Intersection.Include) + readonly temporaryIntersection = signal(Intersection.Include) + private readonly _documentCounts = signal([]) + private readonly _items = signal([]) + private readonly _selectionStates = signal( + new Map() + ) + private readonly _temporarySelectionStates = signal( + new Map() + ) + public documentCountSortingEnabled = false + private get selectionStates(): ReadonlyMap { + return this._selectionStates() + } + + private get temporarySelectionStates(): ReadonlyMap< + number, + ToggleableItemState + > { + return this._temporarySelectionStates() + } + public set documentCounts(counts: SelectionDataItem[]) { - this._documentCounts = counts + this._documentCounts.set(counts) if (this.documentCountSortingEnabled) { - this.sortItems() + this._items.set(this.sortItems(this.items)) } } - private _items: MatchingModel[] = [] get items(): MatchingModel[] { - return this._items + return this._items() } set items(items: MatchingModel[]) { if (items) { - this._items = Array.from(items) - this.sortItems() - this.setNullItem() + this._items.set(this.withNullItem(this.sortItems(Array.from(items)))) } } - private setNullItem() { + private withNullItem(items: MatchingModel[]): MatchingModel[] { if (this.manyToOne && this.logicalOperator === LogicalOperator.Or) { - if (this._items[0]?.id === null) { - this._items.shift() - } - return + return items[0]?.id === null ? items.slice(1) : items } - const item = { + const nullItem = { name: $localize`:Filter drop down element to filter for documents with no correspondent/type/tag assigned:Not assigned`, id: this.manyToOne || this.intersection === Intersection.Include @@ -108,22 +121,17 @@ export class FilterableDropdownSelectionModel { : NEGATIVE_NULL_FILTER_VALUE, } - if ( - this._items[0]?.id === null || - this._items[0]?.id === NEGATIVE_NULL_FILTER_VALUE - ) { - this._items[0] = item - } else if (this._items) { - this._items.unshift(item) - } + return items[0]?.id === null || items[0]?.id === NEGATIVE_NULL_FILTER_VALUE + ? [nullItem, ...items.slice(1)] + : [nullItem, ...items] } constructor(manyToOne: boolean = false) { this.manyToOne = manyToOne } - private sortItems() { - this._items.sort((a, b) => { + private sortItems(items: MatchingModel[]): MatchingModel[] { + const sorted = [...items].sort((a, b) => { if ( (a.id == null && b.id != null) || (a.id == NEGATIVE_NULL_FILTER_VALUE && @@ -154,13 +162,13 @@ export class FilterableDropdownSelectionModel { ) { return -1 } else if ( - this._documentCounts.length && + this._documentCounts().length && this.getDocumentCount(b.id) === 0 && this.getDocumentCount(a.id) > this.getDocumentCount(b.id) ) { return -1 } else if ( - this._documentCounts.length && + this._documentCounts().length && this.getDocumentCount(a.id) === 0 && this.getDocumentCount(a.id) < this.getDocumentCount(b.id) ) { @@ -170,15 +178,11 @@ export class FilterableDropdownSelectionModel { } }) - if (this._documentCounts.length) { - this.promoteBranchesWithDocumentCounts() - } + return this._documentCounts().length + ? this.promoteBranchesWithDocumentCounts(sorted) + : sorted } - private selectionStates = new Map() - - private temporarySelectionStates = new Map() - getSelectedItems() { return this.items.filter( (i) => @@ -194,30 +198,33 @@ export class FilterableDropdownSelectionModel { } set(id: number, state: ToggleableItemState, fireEvent = true) { + const states = new Map(this.temporarySelectionStates) if (state == ToggleableItemState.NotSelected) { - this.temporarySelectionStates.delete(id) + states.delete(id) } else { - this.temporarySelectionStates.set(id, state) + states.set(id, state) } + this._temporarySelectionStates.set(states) if (fireEvent) { this.changed.next(this) } } toggle(id: number, fireEvent = true) { - let state = this.temporarySelectionStates.get(id) + const states = new Map(this.temporarySelectionStates) + let state = states.get(id) if ( state == undefined || (state != ToggleableItemState.Selected && state != ToggleableItemState.Excluded) ) { if (this.manyToOne || this.singleSelect) { - this.temporarySelectionStates.set(id, ToggleableItemState.Selected) + states.set(id, ToggleableItemState.Selected) if (this.singleSelect) { - for (let key of this.temporarySelectionStates.keys()) { + for (let key of states.keys()) { if (key != id) { - this.temporarySelectionStates.delete(key) + states.delete(key) } } } @@ -233,25 +240,26 @@ export class FilterableDropdownSelectionModel { ) { newState = ToggleableItemState.NotSelected } - this.temporarySelectionStates.set(id, newState) + states.set(id, newState) } } else if ( state == ToggleableItemState.Selected || state == ToggleableItemState.Excluded ) { - this.temporarySelectionStates.delete(id) - this.clearDescendantSelections(id) + states.delete(id) + this.clearDescendantSelections(states, id) } if (!id) { - for (let key of this.temporarySelectionStates.keys()) { + for (let key of states.keys()) { if (key) { - this.temporarySelectionStates.delete(key) + states.delete(key) } } } else { - this.temporarySelectionStates.delete(null) + states.delete(null) } + this._temporarySelectionStates.set(states) if (fireEvent) { this.changed.next(this) @@ -259,20 +267,21 @@ export class FilterableDropdownSelectionModel { } exclude(id: number, fireEvent: boolean = true) { - let state = this.temporarySelectionStates.get(id) + const states = new Map(this.temporarySelectionStates) + let state = states.get(id) if (id && (state == null || state != ToggleableItemState.Excluded)) { - this.temporaryLogicalOperator = this._logicalOperator = this.manyToOne - ? LogicalOperator.And - : LogicalOperator.Or + const operator = this.manyToOne ? LogicalOperator.And : LogicalOperator.Or + this.temporaryLogicalOperator.set(operator) + this._logicalOperator.set(operator) if (this.manyToOne || this.singleSelect) { - this.temporarySelectionStates.set(id, ToggleableItemState.Excluded) - this.clearDescendantSelections(id) + states.set(id, ToggleableItemState.Excluded) + this.clearDescendantSelections(states, id) if (this.singleSelect) { - for (let key of this.temporarySelectionStates.keys()) { + for (let key of states.keys()) { if (key != id) { - this.temporarySelectionStates.delete(key) + states.delete(key) } } } @@ -287,17 +296,18 @@ export class FilterableDropdownSelectionModel { ) { newState = ToggleableItemState.NotSelected } - this.temporarySelectionStates.set(id, newState) + states.set(id, newState) if (newState == ToggleableItemState.Excluded) { - this.clearDescendantSelections(id) + this.clearDescendantSelections(states, id) } } } else if (!id || state == ToggleableItemState.Excluded) { - this.temporarySelectionStates.delete(id) + states.delete(id) if (id) { - this.clearDescendantSelections(id) + this.clearDescendantSelections(states, id) } } + this._temporarySelectionStates.set(states) if (fireEvent) { this.changed.next(this) @@ -308,9 +318,12 @@ export class FilterableDropdownSelectionModel { return this.selectionStates.get(id) || ToggleableItemState.NotSelected } - private clearDescendantSelections(id: number) { + private clearDescendantSelections( + states: Map, + id: number + ) { for (const descendantID of this.getDescendantIDs(id)) { - this.temporarySelectionStates.delete(descendantID) + states.delete(descendantID) } } @@ -320,7 +333,7 @@ export class FilterableDropdownSelectionModel { while (queue.length) { const parentID = queue.shift() - for (const item of this._items) { + for (const item of this.items) { if ( typeof item?.id === 'number' && typeof (item as any)['parent'] === 'number' && @@ -336,12 +349,12 @@ export class FilterableDropdownSelectionModel { } get logicalOperator(): LogicalOperator { - return this.temporaryLogicalOperator + return this.temporaryLogicalOperator() } set logicalOperator(operator: LogicalOperator) { - this.temporaryLogicalOperator = operator - this.setNullItem() + this.temporaryLogicalOperator.set(operator) + this._items.set(this.withNullItem(this.items)) } toggleOperator() { @@ -349,12 +362,12 @@ export class FilterableDropdownSelectionModel { } get intersection(): Intersection { - return this.temporaryIntersection + return this.temporaryIntersection() } set intersection(intersection: Intersection) { - this.temporaryIntersection = intersection - this.setNullItem() + this.temporaryIntersection.set(intersection) + this._items.set(this.withNullItem(this.items)) } toggleIntersection() { @@ -364,18 +377,20 @@ export class FilterableDropdownSelectionModel { ? ToggleableItemState.Selected : ToggleableItemState.Excluded - this.temporarySelectionStates.forEach((state, key) => { + const states = new Map(this.temporarySelectionStates) + states.forEach((state, key) => { if (key === null && this.intersection === Intersection.Exclude) { - this.temporarySelectionStates.set(NEGATIVE_NULL_FILTER_VALUE, newState) + states.set(NEGATIVE_NULL_FILTER_VALUE, newState) } else if ( key === NEGATIVE_NULL_FILTER_VALUE && this.intersection === Intersection.Include ) { - this.temporarySelectionStates.set(null, newState) + states.set(null, newState) } else { - this.temporarySelectionStates.set(key, newState) + states.set(key, newState) } }) + this._temporarySelectionStates.set(states) this.changed.next(this) } @@ -395,10 +410,12 @@ export class FilterableDropdownSelectionModel { } clear(fireEvent = true) { - this.temporarySelectionStates.clear() - this.temporaryLogicalOperator = this._logicalOperator = LogicalOperator.And - this.temporaryIntersection = this._intersection = Intersection.Include - this.setNullItem() + this._temporarySelectionStates.set(new Map()) + this.temporaryLogicalOperator.set(LogicalOperator.And) + this._logicalOperator.set(LogicalOperator.And) + this.temporaryIntersection.set(Intersection.Include) + this._intersection.set(Intersection.Include) + this._items.set(this.withNullItem(this.items)) if (fireEvent) { this.changed.next(this) } @@ -419,9 +436,9 @@ export class FilterableDropdownSelectionModel { ) ) { return true - } else if (this.temporaryLogicalOperator !== this._logicalOperator) { + } else if (this.temporaryLogicalOperator() !== this._logicalOperator()) { return true - } else if (this.temporaryIntersection !== this._intersection) { + } else if (this.temporaryIntersection() !== this._intersection()) { return true } else { return false @@ -438,23 +455,29 @@ export class FilterableDropdownSelectionModel { } getDocumentCount(id: number) { - return this._documentCounts.find((c) => c.id === id)?.document_count + return this._documentCounts().find((c) => c.id === id)?.document_count } - private promoteBranchesWithDocumentCounts() { - const parentById = this.buildParentById() + private promoteBranchesWithDocumentCounts( + items: MatchingModel[] + ): MatchingModel[] { + const parentById = this.buildParentById(items) const findRootId = this.createRootFinder(parentById) - const getRootDocCount = this.createRootDocCounter() - const summaries = this.buildBranchSummaries(findRootId, getRootDocCount) + const getRootDocCount = this.createRootDocCounter(items) + const summaries = this.buildBranchSummaries( + items, + findRootId, + getRootDocCount + ) const orderedBranches = this.orderBranchesByPriority(summaries) - this._items = orderedBranches.flatMap((summary) => summary.items) + return orderedBranches.flatMap((summary) => summary.items) } - private buildParentById(): Map { + private buildParentById(items: MatchingModel[]): Map { const parentById = new Map() - for (const item of this._items) { + for (const item of items) { if (typeof item?.id === 'number') { const parentValue = (item as any)['parent'] parentById.set( @@ -492,7 +515,9 @@ export class FilterableDropdownSelectionModel { return findRootId } - private createRootDocCounter(): (rootId: number) => number { + private createRootDocCounter( + items: MatchingModel[] + ): (rootId: number) => number { const docCountMemo = new Map() return (rootId: number): number => { @@ -507,7 +532,7 @@ export class FilterableDropdownSelectionModel { return explicit } - const rootItem = this._items.find((i) => i.id === rootId) + const rootItem = items.find((i) => i.id === rootId) const fallback = typeof (rootItem as any)?.['document_count'] === 'number' ? (rootItem as any)['document_count'] @@ -519,12 +544,13 @@ export class FilterableDropdownSelectionModel { } private buildBranchSummaries( + items: MatchingModel[], findRootId: (id: number) => number, getRootDocCount: (rootId: number) => number ): Map { const summaries = new Map() - for (const [index, item] of this._items.entries()) { + for (const [index, item] of items.entries()) { const { key, special, rootId } = this.describeBranchItem( item, index, @@ -616,28 +642,23 @@ export class FilterableDropdownSelectionModel { } init(map: Map) { - this.temporarySelectionStates = map + this._temporarySelectionStates.set(new Map(map)) this.apply() } apply() { - this.selectionStates.clear() - this.temporarySelectionStates.forEach((value, key) => { - this.selectionStates.set(key, value) - }) - this._logicalOperator = this.temporaryLogicalOperator - this._intersection = this.temporaryIntersection - this.sortItems() + this._selectionStates.set(new Map(this.temporarySelectionStates)) + this._logicalOperator.set(this.temporaryLogicalOperator()) + this._intersection.set(this.temporaryIntersection()) + this._items.set(this.sortItems(this.items)) } reset(complete: boolean = false) { - this.temporarySelectionStates.clear() if (complete) { - this.selectionStates.clear() + this._selectionStates.set(new Map()) + this._temporarySelectionStates.set(new Map()) } else { - this.selectionStates.forEach((value, key) => { - this.temporarySelectionStates.set(key, value) - }) + this._temporarySelectionStates.set(new Map(this.selectionStates)) } } diff --git a/src-ui/src/app/components/common/permissions-filter-dropdown/permissions-filter-dropdown.component.html b/src-ui/src/app/components/common/permissions-filter-dropdown/permissions-filter-dropdown.component.html index 0c54e8991..ab34c1817 100644 --- a/src-ui/src/app/components/common/permissions-filter-dropdown/permissions-filter-dropdown.component.html +++ b/src-ui/src/app/components/common/permissions-filter-dropdown/permissions-filter-dropdown.component.html @@ -7,7 +7,7 @@
- @if (selectionModel.ownerFilter === OwnerFilterType.NONE || selectionModel.ownerFilter === OwnerFilterType.NOT_SELF) { + @if (selectionModel.ownerFilter() === OwnerFilterType.NONE || selectionModel.ownerFilter() === OwnerFilterType.NOT_SELF) {
- +
diff --git a/src-ui/src/app/components/common/permissions-filter-dropdown/permissions-filter-dropdown.component.spec.ts b/src-ui/src/app/components/common/permissions-filter-dropdown/permissions-filter-dropdown.component.spec.ts index de5449a3e..f352355c5 100644 --- a/src-ui/src/app/components/common/permissions-filter-dropdown/permissions-filter-dropdown.component.spec.ts +++ b/src-ui/src/app/components/common/permissions-filter-dropdown/permissions-filter-dropdown.component.spec.ts @@ -90,56 +90,56 @@ describe('PermissionsFilterDropdownComponent', () => { component.setFilter(OwnerFilterType.OTHERS) expect(component.isActive).toBeTruthy() component.setFilter(OwnerFilterType.NONE) - component.selectionModel.hideUnowned = true + component.selectionModel.hideUnowned.set(true) expect(component.isActive).toBeTruthy() }) it('should describe concrete user filters honestly', () => { - component.selectionModel.ownerFilter = OwnerFilterType.SELF - component.selectionModel.userID = 1 + component.selectionModel.ownerFilter.set(OwnerFilterType.SELF) + component.selectionModel.userID.set(1) expect(component.ownerFilterLabel).toEqual('Owned by user1') - component.selectionModel.ownerFilter = OwnerFilterType.NOT_SELF - component.selectionModel.excludeUsers = [1] + component.selectionModel.ownerFilter.set(OwnerFilterType.NOT_SELF) + component.selectionModel.excludeUsers.set([1]) expect(component.ownerExclusionFilterLabel).toEqual('Not owned by user1') - component.selectionModel.ownerFilter = OwnerFilterType.SHARED_BY_ME - component.selectionModel.userID = 1 + component.selectionModel.ownerFilter.set(OwnerFilterType.SHARED_BY_ME) + component.selectionModel.userID.set(1) expect(component.sharedByFilterLabel).toEqual('Shared by user1') }) it('should describe concrete filters when usernames are unavailable', () => { - component.selectionModel.ownerFilter = OwnerFilterType.SELF - component.selectionModel.userID = 99 + component.selectionModel.ownerFilter.set(OwnerFilterType.SELF) + component.selectionModel.userID.set(99) expect(component.ownerFilterLabel).toEqual('Owned by another user') - component.selectionModel.ownerFilter = OwnerFilterType.NOT_SELF - component.selectionModel.excludeUsers = [99] + component.selectionModel.ownerFilter.set(OwnerFilterType.NOT_SELF) + component.selectionModel.excludeUsers.set([99]) expect(component.ownerExclusionFilterLabel).toEqual( 'Not owned by another user' ) - component.selectionModel.excludeUsers = [98, 99] + component.selectionModel.excludeUsers.set([98, 99]) expect(component.ownerExclusionFilterLabel).toEqual( 'Not owned by selected users' ) - component.selectionModel.ownerFilter = OwnerFilterType.SHARED_BY_ME - component.selectionModel.userID = 99 + component.selectionModel.ownerFilter.set(OwnerFilterType.SHARED_BY_ME) + component.selectionModel.userID.set(99) expect(component.sharedByFilterLabel).toEqual('Shared by another user') }) it('should retain relative labels for filters bound to the current user', () => { - component.selectionModel.userID = currentUserID + component.selectionModel.userID.set(currentUserID) expect(component.ownerFilterLabel).toEqual('My documents') expect(component.sharedByFilterLabel).toEqual('Shared by me') - component.selectionModel.excludeUsers = [currentUserID] + component.selectionModel.excludeUsers.set([currentUserID]) expect(component.ownerExclusionFilterLabel).toEqual('Shared with me') }) it('should retain relative labels for inactive filter choices', () => { - component.selectionModel.ownerFilter = OwnerFilterType.NONE + component.selectionModel.ownerFilter.set(OwnerFilterType.NONE) expect(component.ownerFilterLabel).toEqual('My documents') expect(component.ownerExclusionFilterLabel).toEqual('Shared with me') @@ -148,32 +148,41 @@ describe('PermissionsFilterDropdownComponent', () => { it('should support reset', () => { component.setFilter(OwnerFilterType.OTHERS) - expect(component.selectionModel.ownerFilter).not.toEqual( + expect(component.selectionModel.ownerFilter()).not.toEqual( OwnerFilterType.NONE ) component.reset() - expect(component.selectionModel.ownerFilter).toEqual(OwnerFilterType.NONE) + expect(component.selectionModel.ownerFilter()).toEqual(OwnerFilterType.NONE) }) it('should toggle owner filter type when users selected', () => { - component.selectionModel.ownerFilter = OwnerFilterType.NONE + component.selectionModel.ownerFilter.set(OwnerFilterType.NONE) // this would normally be done by select component - component.selectionModel.includeUsers = [12] + component.selectionModel.includeUsers.set([12]) component.onUserSelect() - expect(component.selectionModel.ownerFilter).toEqual(OwnerFilterType.OTHERS) + expect(component.selectionModel.ownerFilter()).toEqual( + OwnerFilterType.OTHERS + ) // this would normally be done by select component - component.selectionModel.includeUsers = null + component.selectionModel.includeUsers.set(null) component.onUserSelect() - expect(component.selectionModel.ownerFilter).toEqual(OwnerFilterType.NONE) + expect(component.selectionModel.ownerFilter()).toEqual(OwnerFilterType.NONE) }) it('should emit a selection model depending on the type of owner filter set', () => { - component.selectionModel.ownerFilter = OwnerFilterType.NONE + const emitted = () => ({ + excludeUsers: ownerFilterSetResult.excludeUsers(), + hideUnowned: ownerFilterSetResult.hideUnowned(), + includeUsers: ownerFilterSetResult.includeUsers(), + ownerFilter: ownerFilterSetResult.ownerFilter(), + userID: ownerFilterSetResult.userID(), + }) + component.selectionModel.ownerFilter.set(OwnerFilterType.NONE) component.setFilter(OwnerFilterType.SELF) - expect(ownerFilterSetResult).toEqual({ + expect(emitted()).toEqual({ excludeUsers: [], hideUnowned: false, includeUsers: [], @@ -182,7 +191,7 @@ describe('PermissionsFilterDropdownComponent', () => { }) component.setFilter(OwnerFilterType.NOT_SELF) - expect(ownerFilterSetResult).toEqual({ + expect(emitted()).toEqual({ excludeUsers: [currentUserID], hideUnowned: false, includeUsers: [], @@ -191,7 +200,7 @@ describe('PermissionsFilterDropdownComponent', () => { }) component.setFilter(OwnerFilterType.NONE) - expect(ownerFilterSetResult).toEqual({ + expect(emitted()).toEqual({ excludeUsers: [], hideUnowned: false, includeUsers: [], @@ -200,7 +209,7 @@ describe('PermissionsFilterDropdownComponent', () => { }) component.setFilter(OwnerFilterType.SHARED_BY_ME) - expect(ownerFilterSetResult).toEqual({ + expect(emitted()).toEqual({ excludeUsers: [], hideUnowned: false, includeUsers: [], @@ -209,7 +218,7 @@ describe('PermissionsFilterDropdownComponent', () => { }) component.setFilter(OwnerFilterType.UNOWNED) - expect(ownerFilterSetResult).toEqual({ + expect(emitted()).toEqual({ excludeUsers: [], hideUnowned: false, includeUsers: [], diff --git a/src-ui/src/app/components/common/permissions-filter-dropdown/permissions-filter-dropdown.component.ts b/src-ui/src/app/components/common/permissions-filter-dropdown/permissions-filter-dropdown.component.ts index 2ae8c2523..afa390948 100644 --- a/src-ui/src/app/components/common/permissions-filter-dropdown/permissions-filter-dropdown.component.ts +++ b/src-ui/src/app/components/common/permissions-filter-dropdown/permissions-filter-dropdown.component.ts @@ -25,18 +25,18 @@ import { ComponentWithPermissions } from '../../with-permissions/with-permission import { ClearableBadgeComponent } from '../clearable-badge/clearable-badge.component' export class PermissionsSelectionModel { - ownerFilter: OwnerFilterType - hideUnowned: boolean - userID: number - includeUsers: number[] - excludeUsers: number[] + readonly ownerFilter = signal(OwnerFilterType.NONE) + readonly hideUnowned = signal(false) + readonly userID = signal(null) + readonly includeUsers = signal([]) + readonly excludeUsers = signal([]) clear() { - this.ownerFilter = OwnerFilterType.NONE - this.userID = null - this.hideUnowned = false - this.includeUsers = [] - this.excludeUsers = [] + this.ownerFilter.set(OwnerFilterType.NONE) + this.userID.set(null) + this.hideUnowned.set(false) + this.includeUsers.set([]) + this.excludeUsers.set([]) } } @@ -84,33 +84,31 @@ export class PermissionsFilterDropdownComponent extends ComponentWithPermissions readonly users = signal([]) - hideUnowned: boolean - get isActive(): boolean { return ( - this.selectionModel.ownerFilter !== OwnerFilterType.NONE || - this.selectionModel.hideUnowned + this.selectionModel.ownerFilter() !== OwnerFilterType.NONE || + this.selectionModel.hideUnowned() ) } get ownerFilterLabel(): string { if ( - this.selectionModel?.ownerFilter !== OwnerFilterType.SELF || - this.selectionModel?.userID === this.settingsService.currentUser()?.id + this.selectionModel?.ownerFilter() !== OwnerFilterType.SELF || + this.selectionModel?.userID() === this.settingsService.currentUser()?.id ) { return $localize`My documents` } - const username = this.getUsername(this.selectionModel?.userID) + const username = this.getUsername(this.selectionModel?.userID()) return username ? $localize`Owned by ${username}` : $localize`Owned by another user` } get ownerExclusionFilterLabel(): string { - const excludedUsers = this.selectionModel?.excludeUsers ?? [] + const excludedUsers = this.selectionModel?.excludeUsers() ?? [] if ( - this.selectionModel?.ownerFilter !== OwnerFilterType.NOT_SELF || + this.selectionModel?.ownerFilter() !== OwnerFilterType.NOT_SELF || (excludedUsers.length === 1 && excludedUsers[0] === this.settingsService.currentUser()?.id) ) { @@ -130,13 +128,13 @@ export class PermissionsFilterDropdownComponent extends ComponentWithPermissions get sharedByFilterLabel(): string { if ( - this.selectionModel?.ownerFilter !== OwnerFilterType.SHARED_BY_ME || - this.selectionModel?.userID === this.settingsService.currentUser()?.id + this.selectionModel?.ownerFilter() !== OwnerFilterType.SHARED_BY_ME || + this.selectionModel?.userID() === this.settingsService.currentUser()?.id ) { return $localize`Shared by me` } - const username = this.getUsername(this.selectionModel?.userID) + const username = this.getUsername(this.selectionModel?.userID()) return username ? $localize`Shared by ${username}` : $localize`Shared by another user` @@ -169,34 +167,36 @@ export class PermissionsFilterDropdownComponent extends ComponentWithPermissions } setFilter(type: OwnerFilterType) { - this.selectionModel.ownerFilter = type - if (this.selectionModel.ownerFilter === OwnerFilterType.SELF) { - this.selectionModel.includeUsers = [] - this.selectionModel.excludeUsers = [] - this.selectionModel.userID = this.settingsService.currentUser().id - this.selectionModel.hideUnowned = false - } else if (this.selectionModel.ownerFilter === OwnerFilterType.NOT_SELF) { - this.selectionModel.userID = null - this.selectionModel.includeUsers = [] - this.selectionModel.excludeUsers = [this.settingsService.currentUser().id] - this.selectionModel.hideUnowned = false - } else if (this.selectionModel.ownerFilter === OwnerFilterType.NONE) { - this.selectionModel.userID = null - this.selectionModel.includeUsers = [] - this.selectionModel.excludeUsers = [] - this.selectionModel.hideUnowned = false + this.selectionModel.ownerFilter.set(type) + if (this.selectionModel.ownerFilter() === OwnerFilterType.SELF) { + this.selectionModel.includeUsers.set([]) + this.selectionModel.excludeUsers.set([]) + this.selectionModel.userID.set(this.settingsService.currentUser().id) + this.selectionModel.hideUnowned.set(false) + } else if (this.selectionModel.ownerFilter() === OwnerFilterType.NOT_SELF) { + this.selectionModel.userID.set(null) + this.selectionModel.includeUsers.set([]) + this.selectionModel.excludeUsers.set([ + this.settingsService.currentUser().id, + ]) + this.selectionModel.hideUnowned.set(false) + } else if (this.selectionModel.ownerFilter() === OwnerFilterType.NONE) { + this.selectionModel.userID.set(null) + this.selectionModel.includeUsers.set([]) + this.selectionModel.excludeUsers.set([]) + this.selectionModel.hideUnowned.set(false) } else if ( - this.selectionModel.ownerFilter === OwnerFilterType.SHARED_BY_ME + this.selectionModel.ownerFilter() === OwnerFilterType.SHARED_BY_ME ) { - this.selectionModel.userID = this.settingsService.currentUser()?.id - this.selectionModel.includeUsers = [] - this.selectionModel.excludeUsers = [] - this.selectionModel.hideUnowned = false - } else if (this.selectionModel.ownerFilter === OwnerFilterType.UNOWNED) { - this.selectionModel.userID = null - this.selectionModel.includeUsers = [] - this.selectionModel.excludeUsers = [] - this.selectionModel.hideUnowned = false + this.selectionModel.userID.set(this.settingsService.currentUser()?.id) + this.selectionModel.includeUsers.set([]) + this.selectionModel.excludeUsers.set([]) + this.selectionModel.hideUnowned.set(false) + } else if (this.selectionModel.ownerFilter() === OwnerFilterType.UNOWNED) { + this.selectionModel.userID.set(null) + this.selectionModel.includeUsers.set([]) + this.selectionModel.excludeUsers.set([]) + this.selectionModel.hideUnowned.set(false) } this.onChange() } @@ -206,11 +206,11 @@ export class PermissionsFilterDropdownComponent extends ComponentWithPermissions } onUserSelect() { - if (this.selectionModel.includeUsers?.length) { - this.selectionModel.ownerFilter = OwnerFilterType.OTHERS - } else { - this.selectionModel.ownerFilter = OwnerFilterType.NONE - } + this.selectionModel.ownerFilter.set( + this.selectionModel.includeUsers()?.length + ? OwnerFilterType.OTHERS + : OwnerFilterType.NONE + ) this.onChange() } diff --git a/src-ui/src/app/components/document-detail/document-detail.component.spec.ts b/src-ui/src/app/components/document-detail/document-detail.component.spec.ts index 471e65487..bb75f8d07 100644 --- a/src-ui/src/app/components/document-detail/document-detail.component.spec.ts +++ b/src-ui/src/app/components/document-detail/document-detail.component.spec.ts @@ -1209,24 +1209,53 @@ describe('DocumentDetailComponent', () => { expect(fixture.debugElement.queryAll(By.css('textarea.rtl'))).not.toBeNull() }) - it('should display built-in pdf viewer if not disabled', () => { + it('should display built-in pdf viewer if not disabled', async () => { initNormally() - component.document().archived_file_name = 'file.pdf' + component.document.update((document) => ({ + ...document, + archived_file_name: 'file.pdf', + })) settingsService.set(SETTINGS_KEYS.USE_NATIVE_PDF_VIEWER, false) expect(component.useNativePdfViewer).toBeFalsy() - fixture.detectChanges() + await fixture.whenStable() expect(fixture.debugElement.query(By.css('pngx-pdf-viewer'))).not.toBeNull() }) it('should display native pdf viewer if enabled', () => { initNormally() - component.document().archived_file_name = 'file.pdf' + component.document.update((document) => ({ + ...document, + archived_file_name: 'file.pdf', + })) settingsService.set(SETTINGS_KEYS.USE_NATIVE_PDF_VIEWER, true) expect(component.useNativePdfViewer).toBeTruthy() fixture.detectChanges() expect(fixture.debugElement.query(By.css('object'))).not.toBeNull() }) + it('should reflect signal-backed document detail display settings', () => { + settingsService.set(SETTINGS_KEYS.DOCUMENT_EDITING_OVERLAY_THUMBNAIL, false) + settingsService.set(SETTINGS_KEYS.DOCUMENT_DETAILS_HIDDEN_FIELDS, [ + component.DocumentDetailFieldID.Correspondent, + ]) + + expect(component.showThumbnailOverlay).toBeFalsy() + expect( + component.isFieldHidden(component.DocumentDetailFieldID.Correspondent) + ).toBeTruthy() + expect( + component.isFieldHidden(component.DocumentDetailFieldID.DocumentType) + ).toBeFalsy() + + settingsService.set(SETTINGS_KEYS.DOCUMENT_EDITING_OVERLAY_THUMBNAIL, true) + settingsService.set(SETTINGS_KEYS.DOCUMENT_DETAILS_HIDDEN_FIELDS, []) + + expect(component.showThumbnailOverlay).toBeTruthy() + expect( + component.isFieldHidden(component.DocumentDetailFieldID.Correspondent) + ).toBeFalsy() + }) + it('should attempt to retrieve metadata', () => { const metadataSpy = jest.spyOn(documentService, 'getMetadata') metadataSpy.mockReturnValue(of({ has_archive_version: true })) @@ -1685,7 +1714,10 @@ describe('DocumentDetailComponent', () => { it('should change preview element by render type', () => { initNormally() - component.document().archived_file_name = 'file.pdf' + component.document.update((document) => ({ + ...document, + archived_file_name: 'file.pdf', + })) fixture.detectChanges() expect(component.archiveContentRenderType).toEqual( component.ContentRenderType.PDF @@ -1694,8 +1726,11 @@ describe('DocumentDetailComponent', () => { fixture.debugElement.query(By.css('pdf-viewer-container')) ).not.toBeUndefined() - component.document().archived_file_name = undefined - component.document().mime_type = 'text/plain' + component.document.update((document) => ({ + ...document, + archived_file_name: undefined, + mime_type: 'text/plain', + })) fixture.detectChanges() expect(component.archiveContentRenderType).toEqual( component.ContentRenderType.Text @@ -1704,7 +1739,10 @@ describe('DocumentDetailComponent', () => { fixture.debugElement.query(By.css('div.preview-sticky')) ).not.toBeUndefined() - component.document().mime_type = 'image/jpeg' + component.document.update((document) => ({ + ...document, + mime_type: 'image/jpeg', + })) fixture.detectChanges() expect(component.archiveContentRenderType).toEqual( component.ContentRenderType.Image @@ -1712,9 +1750,12 @@ describe('DocumentDetailComponent', () => { expect( fixture.debugElement.query(By.css('.preview-sticky img')) ).not.toBeUndefined() - ;((component.document().mime_type = - 'application/vnd.openxmlformats-officedocument.wordprocessingml.document'), - fixture.detectChanges()) + component.document.update((document) => ({ + ...document, + mime_type: + 'application/vnd.openxmlformats-officedocument.wordprocessingml.document', + })) + fixture.detectChanges() expect(component.archiveContentRenderType).toEqual( component.ContentRenderType.Other ) diff --git a/src-ui/src/app/components/document-detail/document-detail.component.ts b/src-ui/src/app/components/document-detail/document-detail.component.ts index d58d7ad1e..18478ea40 100644 --- a/src-ui/src/app/components/document-detail/document-detail.component.ts +++ b/src-ui/src/app/components/document-detail/document-detail.component.ts @@ -227,6 +227,19 @@ export class DocumentDetailComponent private deviceDetectorService = inject(DeviceDetectorService) private savedViewService = inject(SavedViewService) private readonly websocketStatusService = inject(WebsocketStatusService) + private readonly useNativePdfViewerSetting = this.settings.getSignal( + SETTINGS_KEYS.USE_NATIVE_PDF_VIEWER + ) + private readonly aiEnabledSetting = this.settings.getSignal( + SETTINGS_KEYS.AI_ENABLED + ) + private readonly showThumbnailOverlaySetting = + this.settings.getSignal( + SETTINGS_KEYS.DOCUMENT_EDITING_OVERLAY_THUMBNAIL + ) + private readonly hiddenFieldsSetting = this.settings.getSignal< + DocumentDetailFieldID[] + >(SETTINGS_KEYS.DOCUMENT_DETAILS_HIDDEN_FIELDS) @ViewChild('inputTitle') titleInput: TextComponent @@ -333,8 +346,7 @@ export class DocumentDetailComponent } get useNativePdfViewer(): boolean { - this.settings.trackChanges() - return this.settings.get(SETTINGS_KEYS.USE_NATIVE_PDF_VIEWER) + return this.useNativePdfViewerSetting() } get isMobile(): boolean { @@ -342,12 +354,10 @@ export class DocumentDetailComponent } get aiEnabled(): boolean { - this.settings.trackChanges() - return this.settings.get(SETTINGS_KEYS.AI_ENABLED) + return this.aiEnabledSetting() } get archiveContentRenderType(): ContentRenderType { - this.settings.trackChanges() const hasArchiveVersion = this.metadata()?.has_archive_version ?? !!this.document()?.archived_file_name @@ -359,22 +369,17 @@ export class DocumentDetailComponent } get originalContentRenderType(): ContentRenderType { - this.settings.trackChanges() return this.getRenderType( this.metadata()?.original_mime_type || this.document()?.mime_type ) } get showThumbnailOverlay(): boolean { - this.settings.trackChanges() - return this.settings.get(SETTINGS_KEYS.DOCUMENT_EDITING_OVERLAY_THUMBNAIL) + return this.showThumbnailOverlaySetting() } isFieldHidden(fieldId: DocumentDetailFieldID): boolean { - this.settings.trackChanges() - return this.settings - .get(SETTINGS_KEYS.DOCUMENT_DETAILS_HIDDEN_FIELDS) - .includes(fieldId) + return this.hiddenFieldsSetting().includes(fieldId) } private getRenderType(mimeType: string): ContentRenderType { diff --git a/src-ui/src/app/components/document-list/document-list.component.ts b/src-ui/src/app/components/document-list/document-list.component.ts index ff44cfec1..358d94547 100644 --- a/src-ui/src/app/components/document-list/document-list.component.ts +++ b/src-ui/src/app/components/document-list/document-list.component.ts @@ -121,6 +121,8 @@ export class DocumentListComponent settingsService = inject(SettingsService) private hotKeyService = inject(HotKeyService) permissionService = inject(PermissionsService) + private readonly notesEnabledSetting = + this.settingsService.getSignal(SETTINGS_KEYS.NOTES_ENABLED) DisplayField = DisplayField DisplayMode = DisplayMode @@ -574,8 +576,7 @@ export class DocumentListComponent } get notesEnabled(): boolean { - this.settingsService.trackChanges() - return this.settingsService.get(SETTINGS_KEYS.NOTES_ENABLED) + return this.notesEnabledSetting() } resetFilters() { diff --git a/src-ui/src/app/components/document-list/filter-editor/filter-editor.component.spec.ts b/src-ui/src/app/components/document-list/filter-editor/filter-editor.component.spec.ts index af47edf5a..36a016670 100644 --- a/src-ui/src/app/components/document-list/filter-editor/filter-editor.component.spec.ts +++ b/src-ui/src/app/components/document-list/filter-editor/filter-editor.component.spec.ts @@ -621,6 +621,43 @@ describe('FilterEditorComponent', () => { component.toggleTag(2) // coverage }) + it('should reflect ingested tag filter rules in the dropdown toggle', () => { + const dropdown = fixture.debugElement.query( + By.css('pngx-filterable-dropdown') + ) + const toggle = dropdown.nativeElement.querySelector('#dropdown_tags') + expect(toggle.classList.contains('btn-primary')).toBeFalsy() + expect( + dropdown.nativeElement.querySelector('pngx-clearable-badge') + ).toBeNull() + + // switching to a view with a tag filter + component.filterRules = [ + { + rule_type: FILTER_HAS_TAGS_ALL, + value: '2', + }, + ] + fixture.detectChanges() + expect(toggle.classList.contains('btn-primary')).toBeTruthy() + expect( + dropdown.nativeElement.querySelector('pngx-clearable-badge') + ).not.toBeNull() + + // and back to a view without one + component.filterRules = [ + { + rule_type: FILTER_HAS_CORRESPONDENT_ANY, + value: '12', + }, + ] + fixture.detectChanges() + expect(toggle.classList.contains('btn-primary')).toBeFalsy() + expect( + dropdown.nativeElement.querySelector('pngx-clearable-badge') + ).toBeNull() + }) + it('should ingest filter rules for has any tags', () => { expect(component.tagSelectionModel.getSelectedItems()).toHaveLength(0) component.filterRules = [ @@ -1078,7 +1115,7 @@ describe('FilterEditorComponent', () => { }) it('should ingest filter rules for owner', () => { - expect(component.permissionsSelectionModel.ownerFilter).toEqual( + expect(component.permissionsSelectionModel.ownerFilter()).toEqual( OwnerFilterType.NONE ) component.filterRules = [ @@ -1087,15 +1124,38 @@ describe('FilterEditorComponent', () => { value: '100', }, ] - expect(component.permissionsSelectionModel.ownerFilter).toEqual( + expect(component.permissionsSelectionModel.ownerFilter()).toEqual( OwnerFilterType.SELF ) - expect(component.permissionsSelectionModel.hideUnowned).toBeFalsy() - expect(component.permissionsSelectionModel.userID).toEqual(100) + expect(component.permissionsSelectionModel.hideUnowned()).toBeFalsy() + expect(component.permissionsSelectionModel.userID()).toEqual(100) + }) + + it('should reflect ingested owner filter rules in the dropdown toggle', () => { + const dropdown = fixture.debugElement.query( + By.css('pngx-permissions-filter-dropdown') + ) + const toggle = dropdown.nativeElement.querySelector('button') + expect(toggle.classList.contains('btn-primary')).toBeFalsy() + + // switching to a view with an owner filter + component.filterRules = [ + { + rule_type: FILTER_OWNER, + value: '100', + }, + ] + fixture.detectChanges() + expect(toggle.classList.contains('btn-primary')).toBeTruthy() + + // and back to a view without one + component.filterRules = [] + fixture.detectChanges() + expect(toggle.classList.contains('btn-primary')).toBeFalsy() }) it('should ingest filter rules for owner is others', () => { - expect(component.permissionsSelectionModel.ownerFilter).toEqual( + expect(component.permissionsSelectionModel.ownerFilter()).toEqual( OwnerFilterType.NONE ) component.filterRules = [ @@ -1104,14 +1164,14 @@ describe('FilterEditorComponent', () => { value: '50', }, ] - expect(component.permissionsSelectionModel.ownerFilter).toEqual( + expect(component.permissionsSelectionModel.ownerFilter()).toEqual( OwnerFilterType.OTHERS ) - expect(component.permissionsSelectionModel.includeUsers).toContain(50) + expect(component.permissionsSelectionModel.includeUsers()).toContain(50) }) it('should ingest filter rules for owner does not include others', () => { - expect(component.permissionsSelectionModel.ownerFilter).toEqual( + expect(component.permissionsSelectionModel.ownerFilter()).toEqual( OwnerFilterType.NONE ) component.filterRules = [ @@ -1120,14 +1180,14 @@ describe('FilterEditorComponent', () => { value: '50', }, ] - expect(component.permissionsSelectionModel.ownerFilter).toEqual( + expect(component.permissionsSelectionModel.ownerFilter()).toEqual( OwnerFilterType.NOT_SELF ) - expect(component.permissionsSelectionModel.excludeUsers).toContain(50) + expect(component.permissionsSelectionModel.excludeUsers()).toContain(50) }) it('should ingest filter rules for owner is null', () => { - expect(component.permissionsSelectionModel.ownerFilter).toEqual( + expect(component.permissionsSelectionModel.ownerFilter()).toEqual( OwnerFilterType.NONE ) component.filterRules = [ @@ -1136,10 +1196,10 @@ describe('FilterEditorComponent', () => { value: 'true', }, ] - expect(component.permissionsSelectionModel.ownerFilter).toEqual( + expect(component.permissionsSelectionModel.ownerFilter()).toEqual( OwnerFilterType.UNOWNED ) - expect(component.permissionsSelectionModel.hideUnowned).toBeFalsy() + expect(component.permissionsSelectionModel.hideUnowned()).toBeFalsy() }) it('should ingest filter rules for owner is not null', () => { @@ -1149,14 +1209,14 @@ describe('FilterEditorComponent', () => { value: 'false', }, ] - expect(component.permissionsSelectionModel.hideUnowned).toBeTruthy() + expect(component.permissionsSelectionModel.hideUnowned()).toBeTruthy() component.filterRules = [ { rule_type: FILTER_OWNER_ISNULL, value: '0', }, ] - expect(component.permissionsSelectionModel.hideUnowned).toBeTruthy() + expect(component.permissionsSelectionModel.hideUnowned()).toBeTruthy() }) it('should ingest filter rules for shared by me', () => { @@ -1166,7 +1226,7 @@ describe('FilterEditorComponent', () => { value: '2', }, ] - expect(component.permissionsSelectionModel.userID).toEqual(2) + expect(component.permissionsSelectionModel.userID()).toEqual(2) }) // GET filterRules @@ -1932,7 +1992,10 @@ describe('FilterEditorComponent', () => { value: '1', }, ]) - component.permissionsSelectionModel.excludeUsers.push(2) + component.permissionsSelectionModel.excludeUsers.update((users) => [ + ...users, + 2, + ]) fixture.detectChanges() expect(component.filterRules).toEqual([ { @@ -1982,8 +2045,11 @@ describe('FilterEditorComponent', () => { // TODO: mock input in code // userSelect.query(By.css('input')).nativeElement.value = '3' // userSelect.triggerEventHandler('change') - component.permissionsSelectionModel.ownerFilter = OwnerFilterType.OTHERS - component.permissionsSelectionModel.includeUsers.push(3) + component.permissionsSelectionModel.ownerFilter.set(OwnerFilterType.OTHERS) + component.permissionsSelectionModel.includeUsers.update((users) => [ + ...users, + 3, + ]) fixture.detectChanges() expect(component.filterRules).toEqual([ { @@ -2003,7 +2069,7 @@ describe('FilterEditorComponent', () => { ownerToggle.nativeElement.checked = true // ownerToggle.triggerEventHandler('change') // TODO: ngModel isn't doing this here - component.permissionsSelectionModel.hideUnowned = true + component.permissionsSelectionModel.hideUnowned.set(true) fixture.detectChanges() expect(component.filterRules).toEqual([ { diff --git a/src-ui/src/app/components/document-list/filter-editor/filter-editor.component.ts b/src-ui/src/app/components/document-list/filter-editor/filter-editor.component.ts index 3d002b076..16bf3cccb 100644 --- a/src-ui/src/app/components/document-list/filter-editor/filter-editor.component.ts +++ b/src-ui/src/app/components/document-list/filter-editor/filter-editor.component.ts @@ -735,38 +735,50 @@ export class FilterEditorComponent this._textFilter = rule.value break case FILTER_OWNER: - this.permissionsSelectionModel.ownerFilter = OwnerFilterType.SELF - this.permissionsSelectionModel.hideUnowned = false + this.permissionsSelectionModel.ownerFilter.set(OwnerFilterType.SELF) + this.permissionsSelectionModel.hideUnowned.set(false) if (rule.value) - this.permissionsSelectionModel.userID = parseInt(rule.value, 10) + this.permissionsSelectionModel.userID.set( + Number.parseInt(rule.value, 10) + ) break case FILTER_OWNER_ANY: - this.permissionsSelectionModel.ownerFilter = OwnerFilterType.OTHERS + this.permissionsSelectionModel.ownerFilter.set(OwnerFilterType.OTHERS) if (rule.value) - this.permissionsSelectionModel.includeUsers.push( - parseInt(rule.value, 10) - ) + this.permissionsSelectionModel.includeUsers.update((users) => [ + ...users, + Number.parseInt(rule.value, 10), + ]) break case FILTER_OWNER_DOES_NOT_INCLUDE: - this.permissionsSelectionModel.ownerFilter = OwnerFilterType.NOT_SELF + this.permissionsSelectionModel.ownerFilter.set( + OwnerFilterType.NOT_SELF + ) if (rule.value) - this.permissionsSelectionModel.excludeUsers.push( - parseInt(rule.value, 10) - ) + this.permissionsSelectionModel.excludeUsers.update((users) => [ + ...users, + Number.parseInt(rule.value, 10), + ]) break case FILTER_SHARED_BY_USER: - this.permissionsSelectionModel.ownerFilter = + this.permissionsSelectionModel.ownerFilter.set( OwnerFilterType.SHARED_BY_ME + ) if (rule.value) - this.permissionsSelectionModel.userID = parseInt(rule.value, 10) + this.permissionsSelectionModel.userID.set( + Number.parseInt(rule.value, 10) + ) break case FILTER_OWNER_ISNULL: if (rule.value === 'true' || rule.value === '1') { - this.permissionsSelectionModel.hideUnowned = false - this.permissionsSelectionModel.ownerFilter = OwnerFilterType.UNOWNED + this.permissionsSelectionModel.hideUnowned.set(false) + this.permissionsSelectionModel.ownerFilter.set( + OwnerFilterType.UNOWNED + ) } else { - this.permissionsSelectionModel.hideUnowned = + this.permissionsSelectionModel.hideUnowned.set( rule.value === 'false' || rule.value === '0' + ) break } } @@ -1074,34 +1086,35 @@ export class FilterEditorComponent }) } } - if (this.permissionsSelectionModel.ownerFilter == OwnerFilterType.SELF) { + if (this.permissionsSelectionModel.ownerFilter() == OwnerFilterType.SELF) { filterRules.push({ rule_type: FILTER_OWNER, - value: this.permissionsSelectionModel.userID.toString(), + value: this.permissionsSelectionModel.userID().toString(), }) } else if ( - this.permissionsSelectionModel.ownerFilter == OwnerFilterType.NOT_SELF + this.permissionsSelectionModel.ownerFilter() == OwnerFilterType.NOT_SELF ) { filterRules.push({ rule_type: FILTER_OWNER_DOES_NOT_INCLUDE, - value: this.permissionsSelectionModel.excludeUsers?.join(','), + value: this.permissionsSelectionModel.excludeUsers()?.join(','), }) } else if ( - this.permissionsSelectionModel.ownerFilter == OwnerFilterType.OTHERS + this.permissionsSelectionModel.ownerFilter() == OwnerFilterType.OTHERS ) { filterRules.push({ rule_type: FILTER_OWNER_ANY, - value: this.permissionsSelectionModel.includeUsers?.join(','), + value: this.permissionsSelectionModel.includeUsers()?.join(','), }) } else if ( - this.permissionsSelectionModel.ownerFilter == OwnerFilterType.SHARED_BY_ME + this.permissionsSelectionModel.ownerFilter() == + OwnerFilterType.SHARED_BY_ME ) { filterRules.push({ rule_type: FILTER_SHARED_BY_USER, - value: this.permissionsSelectionModel.userID.toString(), + value: this.permissionsSelectionModel.userID().toString(), }) } else if ( - this.permissionsSelectionModel.ownerFilter == OwnerFilterType.UNOWNED + this.permissionsSelectionModel.ownerFilter() == OwnerFilterType.UNOWNED ) { filterRules.push({ rule_type: FILTER_OWNER_ISNULL, @@ -1109,7 +1122,7 @@ export class FilterEditorComponent }) } - if (this.permissionsSelectionModel.hideUnowned) { + if (this.permissionsSelectionModel.hideUnowned()) { filterRules.push({ rule_type: FILTER_OWNER_ISNULL, value: 'false', diff --git a/src-ui/src/app/services/settings.service.spec.ts b/src-ui/src/app/services/settings.service.spec.ts index 510f1857f..b89a3554f 100644 --- a/src-ui/src/app/services/settings.service.spec.ts +++ b/src-ui/src/app/services/settings.service.spec.ts @@ -210,6 +210,48 @@ describe('SettingsService', () => { expect(settingsService.get(SETTINGS_KEYS.THEME_COLOR)).toEqual('#000000') }) + it('provides stable signals that update when settings change', () => { + const req = httpTestingController.expectOne( + `${environment.apiBaseUrl}ui_settings/` + ) + req.flush(ui_settings) + + const notesEnabled = settingsService.getSignal( + SETTINGS_KEYS.NOTES_ENABLED + ) + + expect(notesEnabled()).toBeTruthy() + expect( + settingsService.getSignal(SETTINGS_KEYS.NOTES_ENABLED) + ).toBe(notesEnabled) + + settingsService.set(SETTINGS_KEYS.NOTES_ENABLED, false) + + expect(notesEnabled()).toBeFalsy() + }) + + it('updates setting signals when settings are reinitialized', () => { + let req = httpTestingController.expectOne( + `${environment.apiBaseUrl}ui_settings/` + ) + req.flush(ui_settings) + const appTitle = settingsService.getSignal(SETTINGS_KEYS.APP_TITLE) + + settingsService.initializeSettings().subscribe() + req = httpTestingController.expectOne( + `${environment.apiBaseUrl}ui_settings/` + ) + req.flush({ + ...ui_settings, + settings: { + ...ui_settings.settings, + app_title: 'Updated title', + }, + }) + + expect(appTitle()).toBe('Updated title') + }) + it('sets django cookie for languages', () => { httpTestingController .expectOne(`${environment.apiBaseUrl}ui_settings/`) diff --git a/src-ui/src/app/services/settings.service.ts b/src-ui/src/app/services/settings.service.ts index 850b4d4e8..c6fd5c8f3 100644 --- a/src-ui/src/app/services/settings.service.ts +++ b/src-ui/src/app/services/settings.service.ts @@ -2,6 +2,8 @@ import { HttpClient } from '@angular/common/http' import { DOCUMENT, EventEmitter, + Signal, + computed, inject, Injectable, LOCALE_ID, @@ -297,6 +299,7 @@ export class SettingsService { private settings: Record = {} private readonly settingsVersion = signal(0) + private readonly settingSignals = new Map>() readonly currentUser = signal(undefined) public settingsSaved: EventEmitter = new EventEmitter() @@ -326,10 +329,6 @@ export class SettingsService { return !UNSAFE_OBJECT_KEYS.has(key) } - public trackChanges(): void { - this.settingsVersion() - } - private assignSafeSettings(source: Record) { if (!source || typeof source !== 'object' || Array.isArray(source)) { return @@ -339,6 +338,7 @@ export class SettingsService { if (!this.isSafeObjectKey(key)) continue this.settings[key] = source[key] } + this.settingsVersion.update((version) => version + 1) } // this is called by the app initializer in app.module @@ -594,6 +594,18 @@ export class SettingsService { } } + getSignal(key: string): Signal { + let settingSignal = this.settingSignals.get(key) + if (!settingSignal) { + settingSignal = computed(() => { + this.settingsVersion() + return this.get(key) + }) + this.settingSignals.set(key, settingSignal) + } + return settingSignal as Signal + } + set(key: string, value: any) { // parse key:key:key into nested object let settingObj = this.settings