From 009b67a631695e3442523c627d029694ba3a109e Mon Sep 17 00:00:00 2001 From: Mykyta Maliarchuk <84377976+nikita-web-ua@users.noreply.github.com> Date: Wed, 16 Sep 2026 11:58:10 +0200 Subject: [PATCH] [MNT-25883] Fix: don't persist sorting key until document list load succeeds (#5389) --- .../document-list.directive.spec.ts | 87 ++++++++++++++++++- .../lib/directives/document-list.directive.ts | 29 ++++++- 2 files changed, 111 insertions(+), 5 deletions(-) diff --git a/projects/aca-content/src/lib/directives/document-list.directive.spec.ts b/projects/aca-content/src/lib/directives/document-list.directive.spec.ts index 0cee942d9..dd7a8d1a9 100644 --- a/projects/aca-content/src/lib/directives/document-list.directive.spec.ts +++ b/projects/aca-content/src/lib/directives/document-list.directive.spec.ts @@ -22,7 +22,7 @@ * from Hyland Software. If not, see . */ -import { DocumentListDirective } from './document-list.directive'; +import { DocumentListDirective, SortingChangedEventDetail } from './document-list.directive'; import { BehaviorSubject, Subject } from 'rxjs'; import { SetSelectedNodesAction } from '@alfresco/aca-shared/store'; import { fakeAsync, TestBed, tick } from '@angular/core/testing'; @@ -33,6 +33,7 @@ import { ActivatedRoute, Router } from '@angular/router'; import { ElementRef } from '@angular/core'; import { AppHookService } from '@alfresco/aca-shared'; import { NodeEntry } from '@alfresco/js-api'; +import { HttpErrorResponse } from '@angular/common/http'; describe('DocumentListDirective', () => { let documentListDirective: DocumentListDirective; @@ -52,6 +53,7 @@ describe('DocumentListDirective', () => { reload: jasmine.createSpy('reload'), resetSelection: jasmine.createSpy('resetSelection'), ready: new Subject(), + error: new Subject(), setColumnsWidths: {}, setColumnsVisibility: {}, setColumnsOrder: {} @@ -95,6 +97,10 @@ describe('DocumentListDirective', () => { }; beforeEach(() => { + userPreferencesServiceMock.set = jasmine.createSpy('set'); + userPreferencesServiceMock.get = jasmine.createSpy('get'); + userPreferencesServiceMock.hasItem = jasmine.createSpy('hasItem'); + TestBed.configureTestingModule({ imports: [DocumentListDirective], providers: [ @@ -117,6 +123,7 @@ describe('DocumentListDirective', () => { nodeToSelect$.next(null); documentListMock.preselectNodes = []; documentListMock.selection = []; + documentListMock.sortingMode = undefined; elementRefMock.nativeElement.querySelector.calls.reset(); mockSelectedElement.focus.calls.reset(); }); @@ -270,6 +277,84 @@ describe('DocumentListDirective', () => { }); }); + describe('onSortingChanged - persisting sorting', () => { + const sortingChangedEvent = (detail: SortingChangedEventDetail): CustomEvent => + new CustomEvent('sorting-changed', { detail }); + + beforeEach(() => { + mockRoute.snapshot.data.sortingPreferenceKey = preferenceKey; + documentListDirective.ngOnInit(); + }); + + it('should persist sorting immediately when sortingMode is `client`', () => { + documentListMock.sortingMode = 'client'; + documentListDirective.onSortingChanged(sortingChangedEvent({ key: 'name', sortingKey: 'name', direction: 'asc' })); + + expect(userPreferencesServiceMock.set).toHaveBeenCalledWith(`${preferenceKey}.sorting.key`, 'name'); + expect(userPreferencesServiceMock.set).toHaveBeenCalledWith(`${preferenceKey}.sorting.sortingKey`, 'name'); + expect(userPreferencesServiceMock.set).toHaveBeenCalledWith(`${preferenceKey}.sorting.direction`, 'asc'); + }); + + it('should NOT persist sorting immediately when sortingMode is `server` (deferred until success)', () => { + documentListMock.sortingMode = 'server'; + documentListDirective.onSortingChanged( + sortingChangedEvent({ key: 'properties.custom:prop', sortingKey: 'properties.custom:prop', direction: 'asc' }) + ); + + expect(userPreferencesServiceMock.set).not.toHaveBeenCalledWith(`${preferenceKey}.sorting.key`, jasmine.anything()); + }); + + it('should persist deferred `server` sorting once the document list emits `ready` (successful load)', () => { + documentListMock.sortingMode = 'server'; + documentListDirective.onSortingChanged( + sortingChangedEvent({ key: 'properties.custom:prop', sortingKey: 'properties.custom:prop', direction: 'asc' }) + ); + + expect(userPreferencesServiceMock.set).not.toHaveBeenCalledWith(`${preferenceKey}.sorting.key`, jasmine.anything()); + + documentListMock.ready.next({}); + + expect(userPreferencesServiceMock.set).toHaveBeenCalledWith(`${preferenceKey}.sorting.key`, 'properties.custom:prop'); + expect(userPreferencesServiceMock.set).toHaveBeenCalledWith(`${preferenceKey}.sorting.sortingKey`, 'properties.custom:prop'); + expect(userPreferencesServiceMock.set).toHaveBeenCalledWith(`${preferenceKey}.sorting.direction`, 'asc'); + }); + + it('should discard deferred `server` sorting and never persist it when the document list emits `error`', () => { + documentListMock.sortingMode = 'server'; + + documentListDirective.onSortingChanged( + sortingChangedEvent({ key: 'properties.custom:unsortable', sortingKey: 'properties.custom:unsortable', direction: 'asc' }) + ); + + documentListMock.error.next(new HttpErrorResponse({ status: 400 })); + + expect(userPreferencesServiceMock.set).not.toHaveBeenCalledWith(`${preferenceKey}.sorting.key`, jasmine.anything()); + + userPreferencesServiceMock.set.calls.reset(); + documentListMock.ready.next({}); + expect(userPreferencesServiceMock.set).not.toHaveBeenCalledWith(`${preferenceKey}.sorting.key`, jasmine.anything()); + }); + + it('should persist a subsequent valid sort even if the previous sort attempt failed', () => { + documentListMock.sortingMode = 'server'; + + documentListDirective.onSortingChanged( + sortingChangedEvent({ key: 'properties.custom:unsortable', sortingKey: 'properties.custom:unsortable', direction: 'asc' }) + ); + documentListMock.error.next(new HttpErrorResponse({ status: 400 })); + userPreferencesServiceMock.set.calls.reset(); + + documentListDirective.onSortingChanged(sortingChangedEvent({ key: 'name', sortingKey: 'name', direction: 'asc' })); + expect(userPreferencesServiceMock.set).not.toHaveBeenCalledWith(`${preferenceKey}.sorting.key`, jasmine.anything()); + + documentListMock.ready.next({}); + + expect(userPreferencesServiceMock.set).toHaveBeenCalledWith(`${preferenceKey}.sorting.key`, 'name'); + expect(userPreferencesServiceMock.set).toHaveBeenCalledWith(`${preferenceKey}.sorting.sortingKey`, 'name'); + expect(userPreferencesServiceMock.set).toHaveBeenCalledWith(`${preferenceKey}.sorting.direction`, 'asc'); + }); + }); + describe('onReady', () => { beforeEach(() => { mockRoute.snapshot.data.sortingPreferenceKey = preferenceKey; diff --git a/projects/aca-content/src/lib/directives/document-list.directive.ts b/projects/aca-content/src/lib/directives/document-list.directive.ts index 5f0d5bbd1..3aa338084 100644 --- a/projects/aca-content/src/lib/directives/document-list.directive.ts +++ b/projects/aca-content/src/lib/directives/document-list.directive.ts @@ -36,6 +36,12 @@ import { AppHookService } from '@alfresco/aca-shared'; const INCLUDE_FIELDS = ['isFavorite', 'aspectNames', 'definition', 'isLink']; const SEARCH_INCLUDE_FIELDS = ['aspectNames', 'isLink']; +export interface SortingChangedEventDetail { + key: string; + sortingKey: string; + direction: string; +} + @Directive({ standalone: true, selector: '[acaDocumentList]' @@ -52,6 +58,7 @@ export class DocumentListDirective implements OnInit { private isLibrary = false; private pendingNode: NodeEntry | null = null; + private pendingSorting: SortingChangedEventDetail | null = null; selectedNode: NodeEntry; @@ -104,6 +111,10 @@ export class DocumentListDirective implements OnInit { ) .subscribe(() => this.onReady()); + this.documentList.error.pipe(takeUntilDestroyed(this.destroyRef)).subscribe(() => { + this.pendingSorting = null; + }); + this.documentListService.reload$.pipe(takeUntilDestroyed(this.destroyRef)).subscribe(() => { this.reload(); }); @@ -124,14 +135,14 @@ export class DocumentListDirective implements OnInit { } @HostListener('sorting-changed', ['$event']) - onSortingChanged(event: CustomEvent) { + onSortingChanged(event: CustomEvent) { if (this.sortingPreferenceKey) { if (this.documentList.sortingMode === 'client') { this.storePreviousSorting(); + this.persistSorting(event.detail); + } else { + this.pendingSorting = event.detail; } - this.preferences.set(`${this.sortingPreferenceKey}.sorting.key`, event.detail.key); - this.preferences.set(`${this.sortingPreferenceKey}.sorting.sortingKey`, event.detail.sortingKey); - this.preferences.set(`${this.sortingPreferenceKey}.sorting.direction`, event.detail.direction); } } @@ -171,6 +182,10 @@ export class DocumentListDirective implements OnInit { onReady() { this.updateSelection(); + if (this.pendingSorting) { + this.persistSorting(this.pendingSorting); + this.pendingSorting = null; + } this.restoreSorting(); if (this.pendingNode) { const wasSelected = this.documentList.selection.some((node) => node.entry?.id === this.pendingNode.entry.id); @@ -206,6 +221,12 @@ export class DocumentListDirective implements OnInit { this.documentList.data.setSorting({ key, direction }); } + private persistSorting(detail: SortingChangedEventDetail) { + this.preferences.set(`${this.sortingPreferenceKey}.sorting.key`, detail.key); + this.preferences.set(`${this.sortingPreferenceKey}.sorting.sortingKey`, detail.sortingKey); + this.preferences.set(`${this.sortingPreferenceKey}.sorting.direction`, detail.direction); + } + private storePreviousSorting() { if (this.preferences.hasItem(`${this.sortingPreferenceKey}.sorting.key`)) { const keyToSave = this.preferences.get(`${this.sortingPreferenceKey}.sorting.key`);