From 0b834a3411ac924814ed3e54c83de08a5ab38bed Mon Sep 17 00:00:00 2001 From: aasandei-vsp Date: Thu, 10 Sep 2026 13:57:03 +0300 Subject: [PATCH 1/4] Replace navigateLean with getWithChildren in the data service The data service is the core of refetch for all actions that happen on records and folders.(copy, delete, upload, create) While changing the endpoint, the way sorting happens also changes, so we had to create a small sort helper that will preserve the sorting for newly created folder, so there is no scrambling effect anymore when we delete a record/folder.(PER-10682) PER-10680 --- .../file-list-controls.component.spec.ts | 48 +++- .../file-list-controls.component.ts | 12 +- .../shared/services/data/data.service.spec.ts | 242 ++++++++++++++++-- src/app/shared/services/data/data.service.ts | 155 +++++------ .../shared/utilities/sort-child-items.spec.ts | 107 ++++++++ src/app/shared/utilities/sort-child-items.ts | 99 +++++++ 6 files changed, 557 insertions(+), 106 deletions(-) create mode 100644 src/app/shared/utilities/sort-child-items.spec.ts create mode 100644 src/app/shared/utilities/sort-child-items.ts diff --git a/src/app/file-browser/components/file-list-controls/file-list-controls.component.spec.ts b/src/app/file-browser/components/file-list-controls/file-list-controls.component.spec.ts index f80568ee2..6babcd25c 100644 --- a/src/app/file-browser/components/file-list-controls/file-list-controls.component.spec.ts +++ b/src/app/file-browser/components/file-list-controls/file-list-controls.component.spec.ts @@ -24,9 +24,11 @@ describe('FileListControlsComponent', () => { accessRole: AccessRole.Manager, }, selectedItems$: () => of([]), - refreshCurrentFolder: jasmine - .createSpy('refreshCurrentFolder') - .and.returnValue(Promise.resolve(true)), + sortCurrentFolder: jasmine + .createSpy('sortCurrentFolder') + .and.callFake((sort: string) => { + dataServiceMock.currentFolder.sort = sort; + }), }; const editServiceMock = { @@ -77,6 +79,10 @@ describe('FileListControlsComponent', () => { }; beforeEach(async () => { + dataServiceMock.currentFolder.sort = 'sort.alphabetical_asc'; + dataServiceMock.sortCurrentFolder.calls.reset(); + apiServiceMock.folder.sort.calls.reset(); + await TestBed.configureTestingModule({ declarations: [FileListControlsComponent, TooltipsPipe], providers: [ @@ -103,4 +109,40 @@ describe('FileListControlsComponent', () => { it('should create the component', () => { expect(component).toBeTruthy(); }); + + it('should preview a sort through the data service without a request', () => { + component.onSortClick('date'); + + expect(dataServiceMock.sortCurrentFolder).toHaveBeenCalledWith( + 'sort.display_date_asc', + ); + + expect(apiServiceMock.folder.sort).not.toHaveBeenCalled(); + expect(component.currentSort).toBe('date'); + expect(component.sortDesc).toBeFalse(); + }); + + it('should flip the direction when the active column is clicked again', () => { + component.onSortClick('name'); + + expect(dataServiceMock.sortCurrentFolder).toHaveBeenCalledWith( + 'sort.alphabetical_desc', + ); + + expect(component.sortDesc).toBeTrue(); + }); + + it('should mark the sort as changed until it is saved', async () => { + component.onSortClick('type'); + + expect(component.isSortChanged()).toBeTrue(); + + await component.saveSort(); + + expect(apiServiceMock.folder.sort).toHaveBeenCalledWith([ + dataServiceMock.currentFolder, + ]); + + expect(component.isSortChanged()).toBeFalse(); + }); }); diff --git a/src/app/file-browser/components/file-list-controls/file-list-controls.component.ts b/src/app/file-browser/components/file-list-controls/file-list-controls.component.ts index 80549e2d2..02f59a88d 100644 --- a/src/app/file-browser/components/file-list-controls/file-list-controls.component.ts +++ b/src/app/file-browser/components/file-list-controls/file-list-controls.component.ts @@ -279,22 +279,16 @@ export class FileListControlsComponent implements OnDestroy, HasSubscriptions { return this.initialSortType !== this.data.currentFolder?.sort; } - async setSort(sort: SortType) { + setSort(sort: SortType) { if (this.isSorting) { return; } this.isSorting = true; - const originalSort = this.data.currentFolder.sort; - this.data.currentFolder.update({ sort }); - this.getSortFromCurrentFolder(); + this.isSorting$.next(true); try { - this.isSorting$.next(true); - await this.data.refreshCurrentFolder(true); - } catch (err) { - this.data.currentFolder.update({ sort: originalSort }); + this.data.sortCurrentFolder(sort); this.getSortFromCurrentFolder(); - throw err; } finally { this.isSorting = false; this.isSorting$.next(false); diff --git a/src/app/shared/services/data/data.service.spec.ts b/src/app/shared/services/data/data.service.spec.ts index 20991ddaa..cfb7b47ef 100644 --- a/src/app/shared/services/data/data.service.spec.ts +++ b/src/app/shared/services/data/data.service.spec.ts @@ -4,11 +4,9 @@ import { cloneDeep } from 'lodash'; import { HttpV2Service } from '@shared/services/http-v2/http-v2.service'; import { DataService } from '@shared/services/data/data.service'; -import { FolderVO, RecordVO } from '@root/app/models'; +import { FolderVO, FolderVOData, RecordVO } from '@root/app/models'; import { FolderResponse } from '@shared/services/api/index.repo'; import { of } from 'rxjs'; -import { HttpTestingController } from '@angular/common/http/testing'; -import { environment } from '@root/environments/environment'; import { DataStatus } from '@models/data-status.enum'; import { NgbTooltipModule } from '@ng-bootstrap/ng-bootstrap'; @@ -236,26 +234,230 @@ describe('DataService', () => { await service.fetchFullItems([]); }); - it('should refresh the current folder with latest data', (done) => { - const service = TestBed.inject(DataService); - const httpMock = TestBed.inject(HttpTestingController); - const navigateResponse = new FolderResponse(navigateMinData); - const currentFolder = navigateResponse.getFolderVO(true) as FolderVO; - const childItemCount = currentFolder.ChildItemVOs.length; + describe('refreshCurrentFolder', () => { + const berlinFolderData = { + folderId: '1', + folder_linkId: 1, + displayName: 'Berlin', + updatedDT: '2024-01-01T00:00:00.000Z', + }; + const amsterdamRecordData = { + recordId: '2', + folder_linkId: 2, + displayName: 'Amsterdam', + updatedDT: '2024-01-01T00:00:00.000Z', + }; + const cairoRecordData = { + recordId: '3', + folder_linkId: 3, + displayName: 'Cairo', + updatedDT: '2024-01-01T00:00:00.000Z', + }; + + const buildFolderResponse = (folderData: FolderVOData) => + new FolderResponse({ + isSuccessful: true, + Results: [{ data: [{ FolderVO: folderData }] }], + }); - currentFolder.ChildItemVOs = []; - service.setCurrentFolder(currentFolder); + const namesOf = (folder: FolderVO) => + folder.ChildItemVOs.map((item) => item.displayName); - service - .refreshCurrentFolder() - .then(() => { - expect(currentFolder.ChildItemVOs.length).toBe(childItemCount); - done(); - }) - .catch(done.fail); + let service: DataService; + let getWithChildrenByIdentifier: jasmine.Spy; + let folderUpdate: jasmine.Spy; + let currentFolder: FolderVO; + + beforeEach(() => { + service = TestBed.inject(DataService); + const api = TestBed.inject(ApiService); + getWithChildrenByIdentifier = spyOn( + api.folder, + 'getWithChildrenByIdentifier', + ); + folderUpdate = jasmine.createSpy('folderUpdate'); + service.folderUpdate.subscribe(folderUpdate); + currentFolder = new FolderVO( + { + folderId: '10', + folder_linkId: 100, + sort: 'sort.alphabetical_asc', + ChildItemVOs: [berlinFolderData, amsterdamRecordData], + }, + true, + ); + service.setCurrentFolder(currentFolder); + }); + + it('should fetch the current folder through getWithChildrenByIdentifier', async () => { + getWithChildrenByIdentifier.and.resolveTo( + buildFolderResponse({ + folderId: '10', + sort: 'sort.alphabetical_asc', + ChildItemVOs: [amsterdamRecordData, berlinFolderData], + }), + ); + + await service.refreshCurrentFolder(); + + expect(getWithChildrenByIdentifier).toHaveBeenCalledWith(currentFolder); + expect(folderUpdate).toHaveBeenCalledWith(currentFolder); + }); + + it('should keep existing children by reference and merge their updated timestamp', async () => { + const [berlin, amsterdam] = currentFolder.ChildItemVOs; + getWithChildrenByIdentifier.and.resolveTo( + buildFolderResponse({ + folderId: '10', + sort: 'sort.alphabetical_asc', + ChildItemVOs: [ + { ...amsterdamRecordData, updatedDT: '2024-06-01T00:00:00.000Z' }, + berlinFolderData, + ], + }), + ); + + await service.refreshCurrentFolder(); + + expect(currentFolder.ChildItemVOs[0]).toBe(amsterdam); + expect(currentFolder.ChildItemVOs[1]).toBe(berlin); + expect(amsterdam.updatedDT).toBe('2024-06-01T00:00:00.000Z'); + expect(amsterdam.isNewlyCreated).toBeFalse(); + }); + + it('should append children new since the last refresh in the server order and flag them', async () => { + getWithChildrenByIdentifier.and.resolveTo( + buildFolderResponse({ + folderId: '10', + sort: 'sort.alphabetical_asc', + ChildItemVOs: [ + amsterdamRecordData, + berlinFolderData, + cairoRecordData, + ], + }), + ); + + await service.refreshCurrentFolder(); + + expect(namesOf(currentFolder)).toEqual(['Amsterdam', 'Berlin', 'Cairo']); + expect(currentFolder.ChildItemVOs[2].isNewlyCreated).toBeTrue(); + expect(currentFolder.ChildItemVOs[2].isRecord).toBeTrue(); + }); + + it('should drop children the server no longer returns and deselect them', async () => { + const [berlin, amsterdam] = currentFolder.ChildItemVOs; + service.clickItemSingle(berlin); + getWithChildrenByIdentifier.and.resolveTo( + buildFolderResponse({ + folderId: '10', + sort: 'sort.alphabetical_asc', + ChildItemVOs: [amsterdamRecordData], + }), + ); - const req = httpMock.expectOne(`${environment.apiUrl}/folder/navigateLean`); - req.flush(navigateMinData); + await service.refreshCurrentFolder(); + + expect(currentFolder.ChildItemVOs).toEqual([amsterdam]); + expect(service.getSelectedItems().has(berlin)).toBeFalse(); + }); + + it('should match children whose link ids differ in type', async () => { + const [berlin] = currentFolder.ChildItemVOs; + getWithChildrenByIdentifier.and.resolveTo( + buildFolderResponse({ + folderId: '10', + sort: 'sort.alphabetical_asc', + ChildItemVOs: [ + { ...amsterdamRecordData, folder_linkId: '2' }, + { ...berlinFolderData, folder_linkId: '1' }, + ], + }), + ); + + await service.refreshCurrentFolder(); + + expect(currentFolder.ChildItemVOs[1]).toBe(berlin); + expect(berlin.isNewlyCreated).toBeFalse(); + }); + + it('should re-apply a previewed sort that is not saved on the folder yet', async () => { + currentFolder.update({ sort: 'sort.alphabetical_desc' }); + getWithChildrenByIdentifier.and.resolveTo( + buildFolderResponse({ + folderId: '10', + sort: 'sort.alphabetical_asc', + ChildItemVOs: [ + amsterdamRecordData, + berlinFolderData, + cairoRecordData, + ], + }), + ); + + await service.refreshCurrentFolder(); + + expect(namesOf(currentFolder)).toEqual(['Cairo', 'Berlin', 'Amsterdam']); + expect(currentFolder.sort).toBe('sort.alphabetical_desc'); + }); + + it('should keep the server order when the current sort is the saved one', async () => { + getWithChildrenByIdentifier.and.resolveTo( + buildFolderResponse({ + folderId: '10', + sort: 'sort.alphabetical_asc', + ChildItemVOs: [ + cairoRecordData, + berlinFolderData, + amsterdamRecordData, + ], + }), + ); + + await service.refreshCurrentFolder(); + + expect(namesOf(currentFolder)).toEqual(['Cairo', 'Berlin', 'Amsterdam']); + }); + + it('should reject with the raw error and leave the children alone when the fetch fails', async () => { + const stelaError = new Error('stela is down'); + getWithChildrenByIdentifier.and.rejectWith(stelaError); + + await expectAsync(service.refreshCurrentFolder()).toBeRejectedWith( + stelaError, + ); + + expect(namesOf(currentFolder)).toEqual(['Berlin', 'Amsterdam']); + expect(folderUpdate).not.toHaveBeenCalled(); + }); + }); + + describe('sortCurrentFolder', () => { + it('should reorder the children in place, store the sort and announce the update', () => { + const service = TestBed.inject(DataService); + const currentFolder = new FolderVO( + { + folderId: '10', + sort: 'sort.alphabetical_asc', + ChildItemVOs: [ + { recordId: '2', folder_linkId: 2, displayName: 'Amsterdam' }, + { folderId: '1', folder_linkId: 1, displayName: 'Berlin' }, + ], + }, + true, + ); + const [amsterdam, berlin] = currentFolder.ChildItemVOs; + service.setCurrentFolder(currentFolder); + const folderUpdate = jasmine.createSpy('folderUpdate'); + service.folderUpdate.subscribe(folderUpdate); + + service.sortCurrentFolder('sort.alphabetical_desc'); + + expect(currentFolder.sort).toBe('sort.alphabetical_desc'); + expect(currentFolder.ChildItemVOs[0]).toBe(berlin); + expect(currentFolder.ChildItemVOs[1]).toBe(amsterdam); + expect(folderUpdate).toHaveBeenCalledWith(currentFolder); + }); }); it('should add items to thumbRefreshQueue that meet the criteria', (done) => { diff --git a/src/app/shared/services/data/data.service.ts b/src/app/shared/services/data/data.service.ts index 15c855512..51b5a7b62 100644 --- a/src/app/shared/services/data/data.service.ts +++ b/src/app/shared/services/data/data.service.ts @@ -1,5 +1,4 @@ import { Injectable, EventEmitter } from '@angular/core'; -import { map } from 'rxjs/operators'; import { remove, find, findIndex, noop } from 'lodash'; import { ApiService } from '@shared/services/api/api.service'; @@ -20,12 +19,15 @@ import { Subject, BehaviorSubject, Observable } from 'rxjs'; import debug from 'debug'; import { debugSubscribable } from '@shared/utilities/debug'; import { TagsService } from '@core/services/tags/tags.service'; +import { SortType } from '@models/vo-types'; +import { sortChildItems } from '@shared/utilities/sort-child-items'; const THUMBNAIL_REFRESH_INTERVAL = 3000; // Identifiers reach us as numbers from the PHP API and as strings from stela, and -// a single item can carry both over its lifetime: a record loaded via navigateLean -// has a numeric parentFolderId until update() overwrites it with stela's string. +// a single item can carry both over its lifetime: a folder created through the +// PHP API has a numeric parentFolderId until a stela fetch overwrites it with a +// string. // Compare them as strings so the source of the id does not change the answer. // Null and undefined never match, including each other. type ItemId = string | number | null | undefined; @@ -450,97 +452,102 @@ export class DataService { }); } - public async refreshCurrentFolder(sortOnly = false) { - this.debug('refreshCurrentFolder (sortOnly = %o)', sortOnly); + public async refreshCurrentFolder(): Promise { + this.debug('refreshCurrentFolder'); - return await this.api.folder - .navigateLean(this.currentFolder) - .pipe( - map((response: FolderResponse) => { - this.debug('refreshCurrentFolder data fetched', sortOnly); + const response = await this.api.folder.getWithChildrenByIdentifier( + this.currentFolder, + ); + this.debug('refreshCurrentFolder data fetched'); - if (!response.isSuccessful) { - throw response; - } + if (!response.isSuccessful) { + throw response; + } - return response.getFolderVO(true); - }), - ) - .toPromise() - .then((updatedFolder: FolderVO) => { - this.updateChildItems(this.currentFolder, updatedFolder, sortOnly); - this.hideItemsInCurrentFolder(); - this.debug('refreshCurrentFolder done', sortOnly); - this.folderUpdate.emit(this.currentFolder); - this.currentHiddenItems = []; - }); + const updatedFolder = response.getFolderVO(true); + this.updateChildItems(this.currentFolder, updatedFolder); + this.reapplyUnsavedSort(updatedFolder.sort); + this.hideItemsInCurrentFolder(); + this.debug('refreshCurrentFolder done'); + this.folderUpdate.emit(this.currentFolder); + this.currentHiddenItems = []; } - public updateChildItems( - folder1: FolderVO, - folder2: FolderVO, - sortOnly = false, - ) { - this.debug('updateChildItems (sortOnly = %o)', sortOnly); + public sortCurrentFolder(sort: SortType): void { + this.debug('sortCurrentFolder %s', sort); + + this.currentFolder.update({ sort }); + this.currentFolder.ChildItemVOs = sortChildItems( + this.currentFolder.ChildItemVOs, + sort, + ); + this.folderUpdate.emit(this.currentFolder); + } + + // Stela orders children by the sort saved on the folder, while a sort picked in + // the list header lives only on the current folder until it is saved. Without + // this, every refresh would snap a previewed sort back to the saved order. + private reapplyUnsavedSort(savedSort: SortType | undefined): void { + const previewedSort = this.currentFolder.sort; + if (!previewedSort || previewedSort === savedSort) { + return; + } + + this.currentFolder.ChildItemVOs = sortChildItems( + this.currentFolder.ChildItemVOs, + previewedSort, + ); + } + + public updateChildItems(folder1: FolderVO, folder2: FolderVO): void { + this.debug('updateChildItems'); if (!folder2.ChildItemVOs || !folder2.ChildItemVOs.length) { folder1.ChildItemVOs = folder2.ChildItemVOs; - this.debug('updateChildItems done no child items', sortOnly); + this.debug('updateChildItems done no child items'); return; } const original = folder1.ChildItemVOs as ItemVO[]; const updated = folder2.ChildItemVOs as ItemVO[]; - const originalItemsById = new Map(); - const updatedItemsById = new Map(); - - const updatedOrderedIds: number[] = []; - - if (sortOnly) { - for (const item of original) { - originalItemsById.set(item.folder_linkId, item); - } + // Keyed by stringified folder_linkId because it arrives as a number from + // the PHP API and as a string from some stela responses. + const originalItemsById = new Map(); + const updatedItemsById = new Map(); + const updatedOrderedIds: string[] = []; - const sortedItems: ItemVO[] = updated.map((item) => - originalItemsById.get(item.folder_linkId), - ); + for (const item of updated) { + updatedItemsById.set(String(item.folder_linkId), item); + updatedOrderedIds.push(String(item.folder_linkId)); + } - folder1.ChildItemVOs = sortedItems; - } else { - for (const item of updated) { - updatedItemsById.set(item.folder_linkId, item); - updatedOrderedIds.push(item.folder_linkId); + for (const item of original) { + const itemId = String(item.folder_linkId); + originalItemsById.set(itemId, item); + + if (updatedItemsById.has(itemId)) { + const updatedItem = updatedItemsById.get(itemId); + const dataToUpdate: FolderVOData | RecordVOData = { + updatedDT: updatedItem.updatedDT, + }; + item.update(dataToUpdate); + } else if (this.selectedItems.has(item)) { + this.selectedItems.delete(item); + this.selectedItemsSubject.next(this.selectedItems); } + } - for (const item of original) { - originalItemsById.set(item.folder_linkId, item); - - if (updatedItemsById.has(item.folder_linkId)) { - const updatedItem = updatedItemsById.get(item.folder_linkId); - const dataToUpdate: FolderVOData | RecordVOData = { - updatedDT: updatedItem.updatedDT, - }; - item.update(dataToUpdate); - } else if (this.selectedItems.has(item)) { - this.selectedItems.delete(item); - this.selectedItemsSubject.next(this.selectedItems); - } + folder1.ChildItemVOs = updatedOrderedIds.map((itemId) => { + const existingItem = originalItemsById.get(itemId); + if (existingItem) { + return existingItem; } - const finalUpdatedItems: ItemVO[] = updatedOrderedIds.map((id) => { - const isNew = !originalItemsById.has(id); - const item = isNew - ? updatedItemsById.get(id) - : originalItemsById.get(id); - if (isNew) { - item.isNewlyCreated = true; - } - return item; - }); - - folder1.ChildItemVOs = finalUpdatedItems; - } + const newItem = updatedItemsById.get(itemId); + newItem.isNewlyCreated = true; + return newItem; + }); this.debug('updateChildItems done %d items', folder1.ChildItemVOs.length); } diff --git a/src/app/shared/utilities/sort-child-items.spec.ts b/src/app/shared/utilities/sort-child-items.spec.ts new file mode 100644 index 000000000..e85aff61c --- /dev/null +++ b/src/app/shared/utilities/sort-child-items.spec.ts @@ -0,0 +1,107 @@ +import { FolderVO, ItemVO, RecordVO } from '@root/app/models'; +import { sortChildItems } from './sort-child-items'; + +describe('sortChildItems', () => { + const folderBerlin = new FolderVO({ + folderId: 1, + folder_linkId: 1, + displayName: 'Berlin', + displayDT: '2021-05-01T00:00:00.000Z', + type: 'type.folder.private', + }); + const recordAmsterdam = new RecordVO({ + recordId: 2, + folder_linkId: 2, + displayName: 'Amsterdam', + displayDT: '2023-01-01T00:00:00.000Z', + type: 'type.record.image', + }); + const recordCairo = new RecordVO({ + recordId: 3, + folder_linkId: 3, + displayName: 'Cairo', + displayDT: '2019-12-31T00:00:00.000Z', + type: 'type.record.document', + }); + const recordUndated = new RecordVO({ + recordId: 4, + folder_linkId: 4, + displayName: 'Undated', + type: 'type.record.image', + }); + + const items: ItemVO[] = [ + folderBerlin, + recordAmsterdam, + recordCairo, + recordUndated, + ]; + const namesOf = (sortedItems: ItemVO[]) => + sortedItems.map((item) => item.displayName); + + it('should interleave folders and records by display name ascending', () => { + expect(namesOf(sortChildItems(items, 'sort.alphabetical_asc'))).toEqual([ + 'Amsterdam', + 'Berlin', + 'Cairo', + 'Undated', + ]); + }); + + it('should order by display name descending', () => { + expect(namesOf(sortChildItems(items, 'sort.alphabetical_desc'))).toEqual([ + 'Undated', + 'Cairo', + 'Berlin', + 'Amsterdam', + ]); + }); + + it('should order by display date ascending with undated items last', () => { + expect(namesOf(sortChildItems(items, 'sort.display_date_asc'))).toEqual([ + 'Cairo', + 'Berlin', + 'Amsterdam', + 'Undated', + ]); + }); + + it('should order by display date descending with undated items first', () => { + expect(namesOf(sortChildItems(items, 'sort.display_date_desc'))).toEqual([ + 'Undated', + 'Amsterdam', + 'Berlin', + 'Cairo', + ]); + }); + + it('should order by type ascending and break ties on display name', () => { + expect(namesOf(sortChildItems(items, 'sort.type_asc'))).toEqual([ + 'Berlin', + 'Cairo', + 'Amsterdam', + 'Undated', + ]); + }); + + it('should order by type descending and still break ties on display name ascending', () => { + expect(namesOf(sortChildItems(items, 'sort.type_desc'))).toEqual([ + 'Amsterdam', + 'Undated', + 'Cairo', + 'Berlin', + ]); + }); + + it('should return a new array holding the same item references', () => { + const sortedItems = sortChildItems(items, 'sort.alphabetical_asc'); + + expect(sortedItems).not.toBe(items); + expect(sortedItems[0]).toBe(recordAmsterdam); + expect(items[0]).toBe(folderBerlin); + }); + + it('should keep the order for an unknown sort', () => { + expect(sortChildItems(items, undefined)).toEqual(items); + }); +}); diff --git a/src/app/shared/utilities/sort-child-items.ts b/src/app/shared/utilities/sort-child-items.ts new file mode 100644 index 000000000..7c2e5ba22 --- /dev/null +++ b/src/app/shared/utilities/sort-child-items.ts @@ -0,0 +1,99 @@ +import { ItemVO } from '@root/app/models'; +import { SortType } from '@models/vo-types'; + +type CompareItems = (firstItem: ItemVO, secondItem: ItemVO) => number; + +interface SortRule { + compare: CompareItems; + descending: boolean; + tiebreak?: CompareItems; +} + +const compareDisplayNames: CompareItems = (firstItem, secondItem) => + String(firstItem.displayName ?? '').localeCompare( + String(secondItem.displayName ?? ''), + ); + +const compareTypes: CompareItems = (firstItem, secondItem) => + String(firstItem.type ?? '').localeCompare(String(secondItem.type ?? '')); + +// Postgres sorts NULL after every non-null value, so an undated item lands last +// ascending and first descending. Parsing a missing date to +Infinity gives the +// same result once the direction is applied. +const toSortableTime = (displayDT: unknown): number => { + const parsedTime = + typeof displayDT === 'string' ? Date.parse(displayDT) : Number.NaN; + + return Number.isNaN(parsedTime) ? Number.POSITIVE_INFINITY : parsedTime; +}; + +const compareDisplayDates: CompareItems = (firstItem, secondItem) => { + const firstTime = toSortableTime(firstItem.displayDT); + const secondTime = toSortableTime(secondItem.displayDT); + if (firstTime === secondTime) { + return 0; + } + + return firstTime < secondTime ? -1 : 1; +}; + +const SORT_RULES = new Map([ + [ + 'sort.alphabetical_asc', + { compare: compareDisplayNames, descending: false }, + ], + [ + 'sort.alphabetical_desc', + { compare: compareDisplayNames, descending: true }, + ], + [ + 'sort.display_date_asc', + { + compare: compareDisplayDates, + descending: false, + tiebreak: compareDisplayNames, + }, + ], + [ + 'sort.display_date_desc', + { + compare: compareDisplayDates, + descending: true, + tiebreak: compareDisplayNames, + }, + ], + [ + 'sort.type_asc', + { compare: compareTypes, descending: false, tiebreak: compareDisplayNames }, + ], + [ + 'sort.type_desc', + { compare: compareTypes, descending: true, tiebreak: compareDisplayNames }, + ], +]); + +/** + * Orders child items the way stela's children endpoint does for a sort type, + * so a sort can be previewed before it is saved on the folder. Returns a new + * array holding the same item references; an unknown sort keeps the order. + */ +export const sortChildItems = ( + items: ItemVO[], + sort: SortType | undefined, +): ItemVO[] => { + const sortRule = sort ? SORT_RULES.get(sort) : undefined; + if (!sortRule) { + return [...items]; + } + + const direction = sortRule.descending ? -1 : 1; + + return [...items].sort((firstItem, secondItem) => { + const primaryOrder = sortRule.compare(firstItem, secondItem) * direction; + if (primaryOrder !== 0 || !sortRule.tiebreak) { + return primaryOrder; + } + + return sortRule.tiebreak(firstItem, secondItem); + }); +}; From 34a843db2d9783c03e3999b03ad3f40d659b459c Mon Sep 17 00:00:00 2001 From: aasandei-vsp Date: Thu, 10 Sep 2026 14:15:16 +0300 Subject: [PATCH 2/4] Make the error message more generic for the create folder catch block If a folder creation would fail, the catch block would expect the error message in the old format, but stela is sending a more raw HTTP error message style, so we will make the error type more generic and extract the error message from it. Issue: PER-10680 --- .../right-menu/right-menu.component.ts | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/src/app/core/components/right-menu/right-menu.component.ts b/src/app/core/components/right-menu/right-menu.component.ts index bbc447c0a..b08da3548 100644 --- a/src/app/core/components/right-menu/right-menu.component.ts +++ b/src/app/core/components/right-menu/right-menu.component.ts @@ -15,7 +15,7 @@ import { FolderViewService } from '@shared/services/folder-view/folder-view.serv import { AccountService } from '@shared/services/account/account.service'; import { checkMinimumAccess, AccessRole } from '@models/access-role'; import { Subscription } from 'rxjs'; -import { BaseResponse } from '@shared/services/api/base'; +import { getFolderErrorMessage } from '@shared/utilities/folder-error-message'; @Component({ selector: 'pr-right-menu', @@ -177,16 +177,12 @@ export class RightMenuComponent implements OnInit { createResolve(); this.dataService.showItem(folder); }) - .catch((err) => { - if (err instanceof BaseResponse) { - this.message.showError({ - message: err.getMessage(), - translate: true, - }); - createReject(); - } else { - throw err; - } + .catch((err: unknown) => { + this.message.showError({ + message: getFolderErrorMessage(err), + translate: true, + }); + createReject(); }); }); } From 12b5d5a919cc13eabb45a2cc1a6a0cf5f30c9536 Mon Sep 17 00:00:00 2001 From: aasandei-vsp Date: Thu, 10 Sep 2026 14:38:44 +0300 Subject: [PATCH 3/4] Remove navigateLean from the folder repo, as it is not used anymore. All the usages of the navigateLean in the codebase have been sequencially removed, so there is no more need to keep this method. Issue: PER-10680 --- src/app/shared/services/api/folder.repo.ts | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/src/app/shared/services/api/folder.repo.ts b/src/app/shared/services/api/folder.repo.ts index 6e073d8b3..293a2ffeb 100644 --- a/src/app/shared/services/api/folder.repo.ts +++ b/src/app/shared/services/api/folder.repo.ts @@ -1,6 +1,6 @@ import { FolderVO, FolderVOData, ItemVO, RecordVO } from '@root/app/models'; import { BaseResponse, BaseRepo } from '@shared/services/api/base'; -import { firstValueFrom, Observable } from 'rxjs'; +import { firstValueFrom } from 'rxjs'; import { DataStatus } from '@models/data-status.enum'; import { getOptionalAccessRoleField, @@ -460,18 +460,6 @@ export class FolderRepo extends BaseRepo { return await this.getWithChildren([identityResponse.getFolderVO()]); } - public navigateLean(folderVO: FolderVO): Observable { - const data = [ - { - FolderVO: new FolderVO(folderVO), - }, - ]; - - return this.http.sendRequest('/folder/navigateLean', data, { - ResponseClass: FolderResponse, - }); - } - public async post(folderVOs: FolderVO[]): Promise { const data = folderVOs.map((folderVO) => ({ FolderVO: new FolderVO(folderVO), From d88916a1061b3f0ec2c529a22346ff591515b505 Mon Sep 17 00:00:00 2001 From: aasandei-vsp Date: Wed, 7 Oct 2026 13:02:27 +0300 Subject: [PATCH 4/4] Cover the stela refresh and create folder edge cases The code coverage was failing because a few paths were not tested: - refreshCurrentFolder for reject - createNewFolder fallback - sortChildItems edge case handleling Issue: PER-10680 --- .../right-menu/right-menu.component.spec.ts | 68 +++++++++++++++++++ .../shared/services/data/data.service.spec.ts | 30 ++++++++ .../shared/utilities/sort-child-items.spec.ts | 45 ++++++++++++ 3 files changed, 143 insertions(+) diff --git a/src/app/core/components/right-menu/right-menu.component.spec.ts b/src/app/core/components/right-menu/right-menu.component.spec.ts index c6e307638..29a9ea41c 100644 --- a/src/app/core/components/right-menu/right-menu.component.spec.ts +++ b/src/app/core/components/right-menu/right-menu.component.spec.ts @@ -6,6 +6,10 @@ import { RightMenuComponent } from '@core/components/right-menu/right-menu.compo import { FolderVO, ArchiveVO } from '@models'; import { DataService } from '@shared/services/data/data.service'; import { AccountService } from '@shared/services/account/account.service'; +import { PromptService } from '@shared/services/prompt/prompt.service'; +import { EditService } from '@core/services/edit/edit.service'; +import { MessageService } from '@shared/services/message/message.service'; +import { GENERIC_FOLDER_ERROR_MESSAGE } from '@shared/utilities/folder-error-message'; describe('RightMenuComponent', () => { let component: RightMenuComponent; @@ -126,4 +130,68 @@ describe('RightMenuComponent', () => { expect(component.hasAllowedActions).toBeFalsy(); expect(component.allowedActions.createFolder).toBeFalsy(); }); + + describe('createNewFolder', () => { + const newFolderName = 'Holidays'; + let createFolder: jasmine.Spy; + let messageService: MessageService; + let folderCreationPromise: Promise; + + beforeEach(() => { + dataService.setCurrentFolder( + new FolderVO({ + type: 'type.folder.private', + accessRole: 'access.role.owner', + }), + ); + spyOn(TestBed.inject(PromptService), 'prompt').and.callFake( + async (fields, title, savePromise) => { + folderCreationPromise = savePromise; + return { folderName: newFolderName }; + }, + ); + createFolder = spyOn(TestBed.inject(EditService), 'createFolder'); + messageService = TestBed.inject(MessageService); + spyOn(messageService, 'showMessage'); + spyOn(messageService, 'showError'); + }); + + it('should create the folder in the current folder, refresh it and show the new folder', async () => { + const createdFolder = new FolderVO({ displayName: newFolderName }); + createFolder.and.resolveTo(createdFolder); + const refreshCurrentFolder = spyOn( + dataService, + 'refreshCurrentFolder', + ).and.resolveTo(); + const showItem = spyOn(dataService, 'showItem'); + + await component.createNewFolder(); + await expectAsync(folderCreationPromise).toBeResolved(); + + expect(createFolder).toHaveBeenCalledWith( + newFolderName, + component.currentFolder, + ); + + expect(messageService.showMessage).toHaveBeenCalledWith({ + message: `Folder "${newFolderName}" has been created`, + style: 'success', + }); + + expect(refreshCurrentFolder).toHaveBeenCalled(); + expect(showItem).toHaveBeenCalledWith(createdFolder); + }); + + it('should show the generic folder error and reject the prompt when creating the folder fails', async () => { + createFolder.and.rejectWith(new Error('stela is down')); + + await component.createNewFolder(); + await expectAsync(folderCreationPromise).toBeRejected(); + + expect(messageService.showError).toHaveBeenCalledWith({ + message: GENERIC_FOLDER_ERROR_MESSAGE, + translate: true, + }); + }); + }); }); diff --git a/src/app/shared/services/data/data.service.spec.ts b/src/app/shared/services/data/data.service.spec.ts index cfb7b47ef..e72431ed9 100644 --- a/src/app/shared/services/data/data.service.spec.ts +++ b/src/app/shared/services/data/data.service.spec.ts @@ -430,6 +430,36 @@ describe('DataService', () => { expect(namesOf(currentFolder)).toEqual(['Berlin', 'Amsterdam']); expect(folderUpdate).not.toHaveBeenCalled(); }); + + it('should reject with the response and leave the children alone when it is unsuccessful', async () => { + const unsuccessfulResponse = new FolderResponse({ + isSuccessful: false, + Results: [], + }); + getWithChildrenByIdentifier.and.resolveTo(unsuccessfulResponse); + + await expectAsync(service.refreshCurrentFolder()).toBeRejectedWith( + unsuccessfulResponse, + ); + + expect(namesOf(currentFolder)).toEqual(['Berlin', 'Amsterdam']); + expect(folderUpdate).not.toHaveBeenCalled(); + }); + + it('should empty the children when the server returns none', async () => { + getWithChildrenByIdentifier.and.resolveTo( + buildFolderResponse({ + folderId: '10', + sort: 'sort.alphabetical_asc', + ChildItemVOs: [], + }), + ); + + await service.refreshCurrentFolder(); + + expect(currentFolder.ChildItemVOs).toEqual([]); + expect(folderUpdate).toHaveBeenCalledWith(currentFolder); + }); }); describe('sortCurrentFolder', () => { diff --git a/src/app/shared/utilities/sort-child-items.spec.ts b/src/app/shared/utilities/sort-child-items.spec.ts index e85aff61c..bc603d671 100644 --- a/src/app/shared/utilities/sort-child-items.spec.ts +++ b/src/app/shared/utilities/sort-child-items.spec.ts @@ -93,6 +93,51 @@ describe('sortChildItems', () => { ]); }); + it('should break display date ties on display name ascending', () => { + const recordZurich = new RecordVO({ + recordId: 5, + folder_linkId: 5, + displayName: 'Zurich', + displayDT: '2021-05-01T00:00:00.000Z', + type: 'type.record.image', + }); + + expect( + namesOf( + sortChildItems([recordZurich, folderBerlin], 'sort.display_date_desc'), + ), + ).toEqual(['Berlin', 'Zurich']); + }); + + it('should order items without a display name or type first ascending, keeping their relative order', () => { + const firstRecordWithoutNameOrType = new RecordVO({ + recordId: 6, + folder_linkId: 6, + }); + const secondRecordWithoutNameOrType = new RecordVO({ + recordId: 7, + folder_linkId: 7, + }); + const itemsWithMissingFields = [ + recordCairo, + firstRecordWithoutNameOrType, + secondRecordWithoutNameOrType, + ]; + const expectedOrder = [ + firstRecordWithoutNameOrType, + secondRecordWithoutNameOrType, + recordCairo, + ]; + + expect( + sortChildItems(itemsWithMissingFields, 'sort.alphabetical_asc'), + ).toEqual(expectedOrder); + + expect(sortChildItems(itemsWithMissingFields, 'sort.type_asc')).toEqual( + expectedOrder, + ); + }); + it('should return a new array holding the same item references', () => { const sortedItems = sortChildItems(items, 'sort.alphabetical_asc');