From 03e6d58f8645af9820b07602531e90ba64714c33 Mon Sep 17 00:00:00 2001 From: shamoon <4887959+shamoon@users.noreply.github.com> Date: Tue, 28 Oct 2025 19:54:39 -0700 Subject: [PATCH] Fix: refactor nested sorting in filterable dropdowns (#11214) --- .../filterable-dropdown.component.spec.ts | 161 ++++++++++++++ .../filterable-dropdown.component.ts | 200 +++++++++++++++++- 2 files changed, 354 insertions(+), 7 deletions(-) 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 06bc18b0cf..5b643dc9ff 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 @@ -564,6 +564,167 @@ describe('FilterableDropdownComponent & FilterableDropdownSelectionModel', () => ]) }) + it('keeps children with their parent when parent has document count', () => { + const parent: Tag = { + id: 10, + name: 'Parent Tag', + orderIndex: 0, + document_count: 2, + } + const child: Tag = { + id: 11, + name: 'Child Tag', + parent: parent.id, + orderIndex: 1, + document_count: 0, + } + const otherRoot: Tag = { + id: 20, + name: 'Other Tag', + orderIndex: 2, + document_count: 0, + } + + component.selectionModel.items = [parent, child, otherRoot] + component.selectionModel = selectionModel + component.documentCounts = [ + { id: parent.id, document_count: 2 }, + { id: otherRoot.id, document_count: 0 }, + ] + selectionModel.apply() + + expect(component.selectionModel.items).toEqual([ + nullItem, + parent, + child, + otherRoot, + ]) + }) + + it('keeps selected branches ahead of document-based ordering', () => { + const selectedRoot: Tag = { + id: 30, + name: 'Selected Root', + orderIndex: 0, + document_count: 0, + } + const otherRoot: Tag = { + id: 40, + name: 'Other Root', + orderIndex: 1, + document_count: 2, + } + + component.selectionModel.items = [selectedRoot, otherRoot] + component.selectionModel = selectionModel + selectionModel.set(selectedRoot.id, ToggleableItemState.Selected) + component.documentCounts = [ + { id: selectedRoot.id, document_count: 0 }, + { id: otherRoot.id, document_count: 2 }, + ] + selectionModel.apply() + + expect(component.selectionModel.items).toEqual([ + nullItem, + selectedRoot, + otherRoot, + ]) + }) + + it('uses fallback document counts when selection data is missing', () => { + const fallbackRoot: Tag = { + id: 50, + name: 'Fallback Root', + orderIndex: 0, + document_count: 3, + } + const fallbackChild: Tag = { + id: 51, + name: 'Fallback Child', + parent: fallbackRoot.id, + orderIndex: 1, + document_count: 0, + } + const otherRoot: Tag = { + id: 60, + name: 'Other Root', + orderIndex: 2, + document_count: 0, + } + + component.selectionModel = selectionModel + selectionModel.items = [fallbackRoot, fallbackChild, otherRoot] + component.documentCounts = [{ id: otherRoot.id, document_count: 0 }] + + selectionModel.apply() + + expect(selectionModel.items).toEqual([ + nullItem, + fallbackRoot, + fallbackChild, + otherRoot, + ]) + }) + + it('handles special and non-numeric ids when promoting branches', () => { + const rootWithDocs: Tag = { + id: 70, + name: 'Root With Docs', + orderIndex: 0, + document_count: 1, + } + const miscItem: any = { id: 'misc', name: 'Misc Item' } + + component.selectionModel = selectionModel + selectionModel.intersection = Intersection.Exclude + selectionModel.items = [rootWithDocs, miscItem as any] + component.documentCounts = [{ id: rootWithDocs.id, document_count: 1 }] + + selectionModel.apply() + + expect(selectionModel.items.map((item) => item.id)).toEqual([ + NEGATIVE_NULL_FILTER_VALUE, + rootWithDocs.id, + 'misc', + ]) + }) + + it('memoizes root document counts between lookups', () => { + const memoRoot: Tag = { id: 80, name: 'Memo Root' } + selectionModel.items = [memoRoot] + selectionModel.documentCounts = [{ id: memoRoot.id, document_count: 9 }] + + const getRootDocCount = (selectionModel as any).createRootDocCounter() + + expect(getRootDocCount(memoRoot.id)).toEqual(9) + selectionModel.documentCounts = [] + expect(getRootDocCount(memoRoot.id)).toEqual(9) + }) + + it('falls back to model stored document counts if selection data missing entry', () => { + const rootWithoutSelection: Tag = { + id: 90, + name: 'Fallback Root', + document_count: 4, + } + selectionModel.items = [rootWithoutSelection] + selectionModel.documentCounts = [] + + const getRootDocCount = (selectionModel as any).createRootDocCounter() + + expect(getRootDocCount(rootWithoutSelection.id)).toEqual(4) + }) + + it('defaults to zero document count when neither selection nor model provide it', () => { + const rootWithoutCounts: Tag = { id: 91, name: 'Fallback Zero Root' } + selectionModel.items = [rootWithoutCounts] + selectionModel.documentCounts = [] + + const getRootDocCount = (selectionModel as any).createRootDocCounter() + + expect(getRootDocCount(rootWithoutCounts.id)).toEqual(0) + }) + it('should set support create, keep open model and call createRef method', fakeAsync(() => { component.selectionModel.items = items component.icon = 'tag-fill' 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 3ca6edc5c8..1e15930d1c 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 @@ -32,6 +32,14 @@ export interface ChangedItems { itemsToRemove: MatchingModel[] } +type BranchSummary = { + items: MatchingModel[] + firstIndex: number + special: boolean + selected: boolean + hasDocs: boolean +} + export enum LogicalOperator { And = 'and', Or = 'or', @@ -114,6 +122,13 @@ export class FilterableDropdownSelectionModel { b.id == NEGATIVE_NULL_FILTER_VALUE) ) { return 1 + } + + // Preserve hierarchical order when provided (e.g., Tags) + const ao = (a as any)['orderIndex'] + const bo = (b as any)['orderIndex'] + if (ao !== undefined && bo !== undefined) { + return ao - bo } else if ( this.getNonTemporary(a.id) == ToggleableItemState.NotSelected && this.getNonTemporary(b.id) != ToggleableItemState.NotSelected @@ -136,17 +151,14 @@ export class FilterableDropdownSelectionModel { this.getDocumentCount(a.id) < this.getDocumentCount(b.id) ) { return 1 - } - - // Preserve hierarchical order when provided (e.g., Tags) - const ao = (a as any)['orderIndex'] - const bo = (b as any)['orderIndex'] - if (ao !== undefined && bo !== undefined) { - return ao - bo } else { return a.name.localeCompare(b.name) } }) + + if (this._documentCounts.length) { + this.promoteBranchesWithDocumentCounts() + } } private selectionStates = new Map() @@ -380,6 +392,180 @@ export class FilterableDropdownSelectionModel { return this._documentCounts.find((c) => c.id === id)?.document_count } + private promoteBranchesWithDocumentCounts() { + const parentById = this.buildParentById() + const findRootId = this.createRootFinder(parentById) + const getRootDocCount = this.createRootDocCounter() + const summaries = this.buildBranchSummaries(findRootId, getRootDocCount) + const orderedBranches = this.orderBranchesByPriority(summaries) + + this._items = orderedBranches.flatMap((summary) => summary.items) + } + + private buildParentById(): Map { + const parentById = new Map() + + for (const item of this._items) { + if (typeof item?.id === 'number') { + const parentValue = (item as any)['parent'] + parentById.set( + item.id, + typeof parentValue === 'number' ? parentValue : null + ) + } + } + + return parentById + } + + private createRootFinder( + parentById: Map + ): (id: number) => number { + const rootMemo = new Map() + + const findRootId = (id: number): number => { + const cached = rootMemo.get(id) + if (cached !== undefined) { + return cached + } + + const parentId = parentById.get(id) + if (parentId === undefined || parentId === null) { + rootMemo.set(id, id) + return id + } + + const rootId = findRootId(parentId) + rootMemo.set(id, rootId) + return rootId + } + + return findRootId + } + + private createRootDocCounter(): (rootId: number) => number { + const docCountMemo = new Map() + + return (rootId: number): number => { + const cached = docCountMemo.get(rootId) + if (cached !== undefined) { + return cached + } + + const explicit = this.getDocumentCount(rootId) + if (typeof explicit === 'number') { + docCountMemo.set(rootId, explicit) + return explicit + } + + const rootItem = this._items.find((i) => i.id === rootId) + const fallback = + typeof (rootItem as any)?.['document_count'] === 'number' + ? (rootItem as any)['document_count'] + : 0 + + docCountMemo.set(rootId, fallback) + return fallback + } + } + + private buildBranchSummaries( + findRootId: (id: number) => number, + getRootDocCount: (rootId: number) => number + ): Map { + const summaries = new Map() + + for (const [index, item] of this._items.entries()) { + const { key, special, rootId } = this.describeBranchItem( + item, + index, + findRootId + ) + + let summary = summaries.get(key) + if (!summary) { + summary = { + items: [], + firstIndex: index, + special, + selected: false, + hasDocs: + special || rootId === null ? false : getRootDocCount(rootId) > 0, + } + summaries.set(key, summary) + } + + summary.items.push(item) + + if (this.shouldMarkSummarySelected(summary, item)) { + summary.selected = true + } + } + + return summaries + } + + private describeBranchItem( + item: MatchingModel, + index: number, + findRootId: (id: number) => number + ): { key: string; special: boolean; rootId: number | null } { + if (item?.id === null) { + return { key: 'null', special: true, rootId: null } + } + + if (item?.id === NEGATIVE_NULL_FILTER_VALUE) { + return { key: 'neg-null', special: true, rootId: null } + } + + if (typeof item?.id === 'number') { + const rootId = findRootId(item.id) + return { key: `root-${rootId}`, special: false, rootId } + } + + return { key: `misc-${index}`, special: false, rootId: null } + } + + private shouldMarkSummarySelected( + summary: BranchSummary, + item: MatchingModel + ): boolean { + if (summary.special) { + return false + } + + if (typeof item?.id !== 'number') { + return false + } + + return this.getNonTemporary(item.id) !== ToggleableItemState.NotSelected + } + + private orderBranchesByPriority( + summaries: Map + ): BranchSummary[] { + return Array.from(summaries.values()).sort((a, b) => { + const rankDiff = this.branchRank(a) - this.branchRank(b) + if (rankDiff !== 0) { + return rankDiff + } + if (a.hasDocs !== b.hasDocs) { + return a.hasDocs ? -1 : 1 + } + return a.firstIndex - b.firstIndex + }) + } + + private branchRank(summary: BranchSummary): number { + if (summary.special) { + return -1 + } + if (summary.selected) { + return 0 + } + return 1 + } + init(map: Map) { this.temporarySelectionStates = map this.apply() -- 2.47.3