[MNT-25883] Fix: don't persist sorting key until document list load succeeds (#5389)

projects/aca-content/src/lib/directives/document-list.directive.spec.ts

@@ -22,7 +22,7 @@
  * from Hyland Software. If not, see <http://www.gnu.org/licenses/>.
  */
 
-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<any>(),
+    error: new Subject<HttpErrorResponse>(),
     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<SortingChangedEventDetail> =>
+      new CustomEvent<SortingChangedEventDetail>('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;

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<SortingChangedEventDetail>) {
     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`);