From 4fb950230868152311c881adfecba122a8a2e5b5 Mon Sep 17 00:00:00 2001 From: aasandei-vsp Date: Wed, 9 Sep 2026 14:32:19 +0300 Subject: [PATCH 01/13] Keep the order of folders/records stela sends In the new converter from stela records/folders, when the items were coming as children of a folder, they would be separated into folders and records for mapping, so the sort order the backend sent was lost. We do not do any kind of sorting on the FE, so this actually fixes the scrambled sort issue we were having before. Issue: PER-10678 --- .../shared/services/api/folder.repo.spec.ts | 57 +++++++++++++++++++ src/app/shared/services/api/folder.repo.ts | 24 +++++--- 2 files changed, 73 insertions(+), 8 deletions(-) diff --git a/src/app/shared/services/api/folder.repo.spec.ts b/src/app/shared/services/api/folder.repo.spec.ts index e9ac56675..32bc25ad0 100644 --- a/src/app/shared/services/api/folder.repo.spec.ts +++ b/src/app/shared/services/api/folder.repo.spec.ts @@ -508,6 +508,63 @@ describe('Folder repo', () => { expect(child.thumbnail256).toBe('thumb256'); }); + // Stela's children endpoint ranks folders and records together by the + // parent folder's sort setting, so the response order is the render order. + it('should keep children in the order the response sent them', async () => { + httpV2Spy.get.and.returnValues( + of([{ items: [mockStelaFolder] }]), + of([ + { + items: [ + { recordId: '77', displayName: 'Apple', folderLinkId: '900' }, + { folderId: '78', displayName: 'Banana', folderLinkId: '901' }, + { recordId: '79', displayName: 'Cherry', folderLinkId: '902' }, + { folderId: '80', displayName: 'Date', folderLinkId: '903' }, + ], + }, + ]), + ); + + const result = await folderRepo.getWithChildren([ + new FolderVO({ folderId: 123 }), + ]); + const childNames = result + .getFolderVO(true) + .ChildItemVOs.map((child) => child.displayName); + + expect(childNames).toEqual(['Apple', 'Banana', 'Cherry', 'Date']); + }); + + it('should still expose children split by kind, each in response order', async () => { + httpV2Spy.get.and.returnValues( + of([{ items: [mockStelaFolder] }]), + of([ + { + items: [ + { recordId: '77', displayName: 'Apple', folderLinkId: '900' }, + { folderId: '78', displayName: 'Banana', folderLinkId: '901' }, + { recordId: '79', displayName: 'Cherry', folderLinkId: '902' }, + { folderId: '80', displayName: 'Date', folderLinkId: '903' }, + ], + }, + ]), + ); + + const folder = ( + await folderRepo.getWithChildren([new FolderVO({ folderId: 123 })]) + ).getFolderVO(); + + expect(folder.ChildFolderVOs.map((child) => child.displayName)).toEqual([ + 'Banana', + 'Date', + ]); + + expect(folder.RecordVOs.map((child) => child.displayName)).toEqual([ + 'Apple', + 'Cherry', + ]); + }); + it('should leave link ids undefined rather than NaN when absent', async () => { const folder = await convertFolder({ folderLinkId: undefined }); diff --git a/src/app/shared/services/api/folder.repo.ts b/src/app/shared/services/api/folder.repo.ts index b2571f56f..6e073d8b3 100644 --- a/src/app/shared/services/api/folder.repo.ts +++ b/src/app/shared/services/api/folder.repo.ts @@ -1,4 +1,4 @@ -import { FolderVO, FolderVOData, ItemVO } from '@root/app/models'; +import { FolderVO, FolderVOData, ItemVO, RecordVO } from '@root/app/models'; import { BaseResponse, BaseRepo } from '@shared/services/api/base'; import { firstValueFrom, Observable } from 'rxjs'; import { DataStatus } from '@models/data-status.enum'; @@ -155,12 +155,20 @@ const convertStelaPathsToBreadcrumbPaths = ( const convertStelaFolderToFolderVO = (stelaFolder: StelaFolder): FolderVO => { stelaFolder.children ??= []; - const childFolderVOs = stelaFolder.children - .filter((child): child is StelaFolder => !isStelaRecord(child)) - .map(convertStelaFolderToFolderVO); - const childRecordVOs = stelaFolder.children - .filter(isStelaRecord) - .map(convertStelaRecordToRecordVO); + // Stela's children endpoint already ranks folders and records together by the + // parent folder's sort setting, so the incoming order is the order to render. + // Splitting the children by kind and concatenating them would discard it. + const childItemVOs: ItemVO[] = stelaFolder.children.map((child) => + isStelaRecord(child) + ? convertStelaRecordToRecordVO(child) + : convertStelaFolderToFolderVO(child), + ); + const childFolderVOs = childItemVOs.filter( + (childItemVO): childItemVO is FolderVO => childItemVO instanceof FolderVO, + ); + const childRecordVOs = childItemVOs.filter( + (childItemVO): childItemVO is RecordVO => childItemVO instanceof RecordVO, + ); const { accessRole: stelaAccessRole, ...stelaFolderWithoutAccessRole } = stelaFolder; return new FolderVO({ @@ -212,7 +220,7 @@ const convertStelaFolderToFolderVO = (stelaFolder: StelaFolder): FolderVO => { TagVOs: (stelaFolder.tags ?? []).map((stelaTag) => convertStelaTagToTagVO(stelaTag, stelaFolder.archive?.id), ), - ChildItemVOs: [...childRecordVOs, ...childFolderVOs], + ChildItemVOs: childItemVOs, ShareVOs: (stelaFolder.shares ?? []).map(convertStelaSharetoShareVO), isFolder: true, }); From f5889ecddff6e6bfec2fdaa46ee447f3b25fe592 Mon Sep 17 00:00:00 2001 From: aasandei-vsp Date: Wed, 9 Sep 2026 14:37:36 +0300 Subject: [PATCH 02/13] Make the error handler more generic to also accept stela errors The folder resolver's catch block would expect a FolderResponse type of error, which we are trying to move away from. So instead of converting the stela error to a FolderResponse, we will just make the expected error type more generic. Issue: PER-10678 --- .../resolves/folder-resolve.service.spec.ts | 281 ++++++++++++++++++ .../core/resolves/folder-resolve.service.ts | 12 +- 2 files changed, 287 insertions(+), 6 deletions(-) diff --git a/src/app/core/resolves/folder-resolve.service.spec.ts b/src/app/core/resolves/folder-resolve.service.spec.ts index abd82f723..1b05893cc 100644 --- a/src/app/core/resolves/folder-resolve.service.spec.ts +++ b/src/app/core/resolves/folder-resolve.service.spec.ts @@ -1,20 +1,301 @@ import { TestBed } from '@angular/core/testing'; import * as Testing from '@root/test/testbedConfig'; import { cloneDeep } from 'lodash'; +import { Router } from '@angular/router'; import { FolderResolveService } from '@core/resolves/folder-resolve.service'; +import { AccountService } from '@shared/services/account/account.service'; +import { FilesystemService } from '@root/app/filesystem/filesystem.service'; +import { FolderResponse } from '@shared/services/api/folder.repo'; +import { FolderVO } from '@models/index'; +import { + MessageDisplayOptions, + MessageService, +} from '@shared/services/message/message.service'; + +const buildFolder = (folderData: Record) => + new FolderVO({ + ChildItemVOs: [], + type: 'type.folder.private', + view: 'folder.view.grid', + ...folderData, + }); describe('FolderResolveService', () => { let service: FolderResolveService; + let accountService: AccountService; + let filesystem: FilesystemService; + let message: MessageService; + let router: Router; beforeEach(() => { const config = cloneDeep(Testing.BASE_TEST_CONFIG); config.providers.push(FolderResolveService); TestBed.configureTestingModule(config); + service = TestBed.inject(FolderResolveService); + accountService = TestBed.inject(AccountService); + filesystem = TestBed.inject(FilesystemService); + message = TestBed.inject(MessageService); + router = TestBed.inject(Router); + + spyOn(accountService, 'getRootFolder').and.returnValue( + new FolderVO({ + ChildItemVOs: [ + new FolderVO({ + folderId: '11', + type: 'type.folder.root.private', + archiveNbr: '0001-0001', + }), + new FolderVO({ + folderId: '22', + type: 'type.folder.root.app', + archiveNbr: '0001-0002', + }), + new FolderVO({ + folderId: '33', + type: 'type.folder.root.public', + archiveNbr: '0001-0003', + }), + ], + }), + ); }); it('should be created', () => { expect(service).toBeTruthy(); }); + + it('should load My Files by default', async () => { + const getFolderSpy = spyOn(filesystem, 'getFolder').and.resolveTo( + buildFolder({ + displayName: 'My Files', + type: 'type.folder.root.private', + }), + ); + + const result = await service.resolve( + { params: {}, data: {} } as any, + { url: '/private' } as any, + ); + + const requestedFolder = getFolderSpy.calls.mostRecent().args[0] as FolderVO; + + expect(getFolderSpy).toHaveBeenCalled(); + expect(requestedFolder.folderId).toBe('11'); + expect(result.displayName).toBe('My Files'); + }); + + it('should load the apps folder on /apps', async () => { + const getFolderSpy = spyOn(filesystem, 'getFolder').and.resolveTo( + buildFolder({ displayName: 'Apps', type: 'type.folder.root.app' }), + ); + + await service.resolve( + { params: {}, data: {} } as any, + { url: '/apps' } as any, + ); + + expect((getFolderSpy.calls.mostRecent().args[0] as FolderVO).folderId).toBe( + '22', + ); + }); + + it('should load the public root on /public', async () => { + const getFolderSpy = spyOn(filesystem, 'getFolder').and.resolveTo( + buildFolder({ displayName: 'Public', type: 'type.folder.root.public' }), + ); + + await service.resolve( + { params: {}, data: {} } as any, + { url: '/public' } as any, + ); + + expect((getFolderSpy.calls.mostRecent().args[0] as FolderVO).folderId).toBe( + '33', + ); + }); + + it('should pass the route identifiers through for a deep link, coercing the link id', async () => { + const getFolderSpy = spyOn(filesystem, 'getFolder').and.resolveTo( + buildFolder({ displayName: 'Deep Linked' }), + ); + + const result = await service.resolve( + { + params: { archiveNbr: '0001-0005', folderLinkId: '99' }, + data: {}, + } as any, + { url: '/private/0001-0005/99' } as any, + ); + + const requestedFolder = getFolderSpy.calls.mostRecent().args[0] as FolderVO; + + expect(requestedFolder.archiveNbr).toBe('0001-0005'); + expect(requestedFolder.folder_linkId).toBe(99); + expect(requestedFolder.folderId).toBeUndefined(); + expect(result.displayName).toBe('Deep Linked'); + }); + + it('should splice share crumbs onto a shared record without loading a folder', async () => { + const getFolderSpy = spyOn(filesystem, 'getFolder'); + const sharedRecord = { displayName: 'A shared photo' }; + + const result = await service.resolve( + { + params: {}, + data: {}, + parent: { + data: { + sharePreviewVO: { FolderVO: null, RecordVO: sharedRecord }, + currentFolder: new FolderVO({ + pathAsText: ['My Files'], + pathAsArchiveNbr: ['0001-0001'], + pathAsFolder_linkId: [11], + }), + }, + }, + } as any, + { url: '/share/abc123' } as any, + ); + + expect(getFolderSpy).not.toHaveBeenCalled(); + expect(result.pathAsText).toEqual(['Shares', 'Record', 'My Files']); + expect(result.pathAsArchiveNbr).toEqual([ + '0000-0000', + '0000-0000', + '0001-0001', + ]); + + expect(result.pathAsFolder_linkId).toEqual([0, 0, 11]); + expect(result.ChildItemVOs).toEqual([sharedRecord] as any); + }); + + it('should redirect a timeline folder to the public timeline route', async () => { + spyOn(filesystem, 'getFolder').and.resolveTo( + buildFolder({ displayName: 'Trip', view: 'folder.view.timeline' }), + ); + const navigateSpy = spyOn(router, 'navigate'); + + await service.resolve( + { + params: { + archiveNbr: '0001-0005', + folderLinkId: '99', + publicArchiveNbr: '0002-0000', + }, + data: {}, + } as any, + { url: '/p/archive/0002-0000/0001-0005/99' } as any, + ); + + expect(navigateSpy).toHaveBeenCalledWith([ + 'p', + 'archive', + '0002-0000', + 'view', + 'timeline', + '0001-0005', + '99', + ]); + }); + + it('should not redirect when the route already declares a folder view', async () => { + spyOn(filesystem, 'getFolder').and.resolveTo( + buildFolder({ displayName: 'Trip', view: 'folder.view.timeline' }), + ); + const navigateSpy = spyOn(router, 'navigate'); + + const result = await service.resolve( + { + params: { + archiveNbr: '0001-0005', + folderLinkId: '99', + publicArchiveNbr: '0002-0000', + }, + data: { folderView: 'folder.view.timeline' }, + } as any, + { url: '/p/archive/0002-0000/view/timeline/0001-0005/99' } as any, + ); + + expect(navigateSpy).not.toHaveBeenCalled(); + expect(result.displayName).toBe('Trip'); + }); + + it('should surface the server message when the load fails', async () => { + spyOn(filesystem, 'getFolder').and.rejectWith( + new FolderResponse({ + isSuccessful: false, + Results: [{ message: ['Test Error'] }], + }), + ); + spyOn(accountService, 'logOut').and.resolveTo(null); + spyOn(router, 'navigate'); + let displayedErrorMessage: string; + spyOn(message, 'showError').and.callFake((data: MessageDisplayOptions) => { + displayedErrorMessage = data.message; + }); + + await expectAsync( + service.resolve( + { params: {}, data: {} } as any, + { url: '/private' } as any, + ), + ).toBeRejected(); + + expect(displayedErrorMessage).toBe('Test Error'); + }); + + it('should fall back to a generic message for a raw error', async () => { + spyOn(filesystem, 'getFolder').and.rejectWith(new Error('Network down')); + spyOn(accountService, 'logOut').and.resolveTo(null); + spyOn(router, 'navigate'); + let displayedErrorMessage: string; + spyOn(message, 'showError').and.callFake((data: MessageDisplayOptions) => { + displayedErrorMessage = data.message; + }); + + await expectAsync( + service.resolve( + { params: {}, data: {} } as any, + { url: '/private' } as any, + ), + ).toBeRejected(); + + expect(displayedErrorMessage).toBe('error.generic.internal'); + }); + + it('should log out when a root folder fails to load', async () => { + spyOn(filesystem, 'getFolder').and.rejectWith(new Error('Network down')); + const logOutSpy = spyOn(accountService, 'logOut').and.resolveTo(null); + spyOn(router, 'navigate'); + spyOn(message, 'showError'); + + await expectAsync( + service.resolve( + { params: {}, data: {} } as any, + { url: '/private' } as any, + ), + ).toBeRejected(); + + expect(logOutSpy).toHaveBeenCalled(); + }); + + it('should redirect rather than throw when a deep link fails', async () => { + spyOn(filesystem, 'getFolder').and.rejectWith(new Error('Network down')); + const navigateSpy = spyOn(router, 'navigate'); + spyOn(message, 'showError'); + + await expectAsync( + service.resolve( + { + params: { archiveNbr: '0001-0005', folderLinkId: '99' }, + data: {}, + } as any, + { url: '/private/0001-0005/99' } as any, + ), + ).toBeRejectedWith(false); + + expect(navigateSpy).toHaveBeenCalledWith(['/private']); + }); }); diff --git a/src/app/core/resolves/folder-resolve.service.ts b/src/app/core/resolves/folder-resolve.service.ts index ab54c126b..991e09600 100644 --- a/src/app/core/resolves/folder-resolve.service.ts +++ b/src/app/core/resolves/folder-resolve.service.ts @@ -9,11 +9,11 @@ import { find, cloneDeep } from 'lodash'; import { AccountService } from '@shared/services/account/account.service'; import { MessageService } from '@shared/services/message/message.service'; -import { FolderResponse } from '@shared/services/api/index.repo'; - import { FolderVO } from '@root/app/models'; import { FolderView } from '@shared/services/folder-view/folder-view.enum'; import { findRouteData } from '@shared/utilities/router'; +import { getFolderErrorMessage } from '@shared/utilities/folder-error-message'; +import { toFolderLinkId } from '@shared/services/api/folder.repo'; import { FilesystemService } from '@root/app/filesystem/filesystem.service'; @Injectable() @@ -34,7 +34,7 @@ export class FolderResolveService { if (route.params.archiveNbr && route.params.folderLinkId) { targetFolder = new FolderVO({ archiveNbr: route.params.archiveNbr, - folder_linkId: route.params.folderLinkId, + folder_linkId: toFolderLinkId(route.params.folderLinkId), }); } else if (state.url === '/apps') { const apps = find(this.accountService.getRootFolder().ChildItemVOs, { @@ -92,12 +92,12 @@ export class FolderResolveService { } return folder; }) - .catch(async (response: FolderResponse) => { + .catch(async (error: unknown) => { this.message.showError({ - message: response.getMessage(), + message: getFolderErrorMessage(error), translate: true, }); - if (targetFolder.type.includes('root')) { + if (targetFolder.type?.includes('root')) { this.accountService .logOut() .then(() => { From 535abd6dad5ade546a449cdf7bc31fcb3596ddb2 Mon Sep 17 00:00:00 2001 From: aasandei-vsp Date: Wed, 9 Sep 2026 14:40:58 +0300 Subject: [PATCH 03/13] Replace navigateLean with getWithChildren for the fileSystem service The fileSystem service, that is used by the folder resolver has quite a wide reach, so this will instantly replace a lot of the navigateLean calls from route level. Issue: PER-10678 --- .../filesystem/filesystem-api.service.spec.ts | 40 ++++++++++++++----- src/app/filesystem/filesystem-api.service.ts | 8 ++-- 2 files changed, 35 insertions(+), 13 deletions(-) diff --git a/src/app/filesystem/filesystem-api.service.spec.ts b/src/app/filesystem/filesystem-api.service.spec.ts index 1716643bf..24181419d 100644 --- a/src/app/filesystem/filesystem-api.service.spec.ts +++ b/src/app/filesystem/filesystem-api.service.spec.ts @@ -3,7 +3,6 @@ import { FolderResponse } from '@shared/services/api/folder.repo'; import { FolderVO } from '@models/index'; import { DataStatus } from '@models/data-status.enum'; import { ApiService } from '@shared/services/api/api.service'; -import { of } from 'rxjs'; import { ShareLinksService } from '../share-links/services/share-links.service'; import { FilesystemApiService } from './filesystem-api.service'; @@ -46,9 +45,9 @@ describe('FilesystemApiService', () => { getWithChildren: jasmine .createSpy('getWithChildren') .and.returnValue(Promise.resolve(mockSuccessResponse)), - navigateLean: jasmine - .createSpy('navigateLean') - .and.returnValue(of(mockSuccessResponse)), + getWithChildrenByIdentifier: jasmine + .createSpy('getWithChildrenByIdentifier') + .and.returnValue(Promise.resolve(mockSuccessResponse)), }, }; @@ -72,20 +71,32 @@ describe('FilesystemApiService', () => { expect(service).toBeTruthy(); }); - it('should navigate using navigateLean', async () => { + it('should navigate using getWithChildrenByIdentifier', async () => { shareLinksServiceSpy.isUnlistedShare.and.resolveTo(false); const folder = await service.navigate({ folderId }); - expect(mockApiService.folder.navigateLean).toHaveBeenCalledWith( - jasmine.any(FolderVO), - ); + expect( + mockApiService.folder.getWithChildrenByIdentifier, + ).toHaveBeenCalledWith(jasmine.any(FolderVO)); expect(folder.folderId).toBe(folderId); expect(folder.displayName).toBe('Unlisted Folder'); expect(folder.dataStatus).toBe(DataStatus.Lean); }); + it('should navigate by archiveNbr and folder_linkId without a folder id', async () => { + shareLinksServiceSpy.isUnlistedShare.and.resolveTo(false); + + await service.navigate({ archiveNbr: '0001-0000' }); + + const [requestedFolder] = + mockApiService.folder.getWithChildrenByIdentifier.calls.mostRecent().args; + + expect(requestedFolder.archiveNbr).toBe('0001-0000'); + expect(requestedFolder.folderId).toBeUndefined(); + }); + it('should navigate using getWithChildren when in unlisted share', async () => { shareLinksServiceSpy.isUnlistedShare.and.resolveTo(true); shareLinksServiceSpy.currentShareToken = 'mock-token'; @@ -104,8 +115,8 @@ describe('FilesystemApiService', () => { it('should throw FolderResponse error if response is unsuccessful', async () => { shareLinksServiceSpy.isUnlistedShare.and.resolveTo(false); - mockApiService.folder.navigateLean.and.returnValue( - of(mockUnsuccessfulResponse), + mockApiService.folder.getWithChildrenByIdentifier.and.resolveTo( + mockUnsuccessfulResponse, ); try { @@ -115,4 +126,13 @@ describe('FilesystemApiService', () => { expect(error).toBeDefined(); } }); + + it('should surface a rejection from getWithChildrenByIdentifier', async () => { + shareLinksServiceSpy.isUnlistedShare.and.resolveTo(false); + mockApiService.folder.getWithChildrenByIdentifier.and.rejectWith( + new Error('500 Internal Server Error'), + ); + + await expectAsync(service.navigate({ folderId })).toBeRejected(); + }); }); diff --git a/src/app/filesystem/filesystem-api.service.ts b/src/app/filesystem/filesystem-api.service.ts index 2026554df..8e1ff5fed 100644 --- a/src/app/filesystem/filesystem-api.service.ts +++ b/src/app/filesystem/filesystem-api.service.ts @@ -1,5 +1,4 @@ import { Injectable } from '@angular/core'; -import { firstValueFrom } from 'rxjs'; import { FolderVO, RecordVO } from '@models/index'; import { ApiService } from '@shared/services/api/api.service'; @@ -26,13 +25,16 @@ export class FilesystemApiService implements FilesystemApi { const isUnlistedShare = await this.shareLinksService.isUnlistedShare(); let response: FolderResponse = null; if (isUnlistedShare) { + // A share-token visitor has no auth token, so the folder id cannot be + // resolved through the v1 endpoint the way getWithChildrenByIdentifier + // does -- these routes always carry a real folder id already. response = await this.api.folder.getWithChildren( [new FolderVO(folder)], this.shareLinksService.currentShareToken, ); } else { - response = await firstValueFrom( - this.api.folder.navigateLean(new FolderVO(folder)), + response = await this.api.folder.getWithChildrenByIdentifier( + new FolderVO(folder), ); } if (!response.isSuccessful) { From 8e01b3022c8b5e5bdd1b1a55fdd97cd13bd35af4 Mon Sep 17 00:00:00 2001 From: aasandei-vsp Date: Wed, 9 Sep 2026 14:46:18 +0300 Subject: [PATCH 04/13] Replace lean folder resolve with the updated folder resolve Folder resolve is already using getWithChildren and it serves the same data the lean folder resolve did, so keeping it became redundant. Issue: PER-10678 --- .../lean-folder-resolve.service.spec.ts | 211 ------------------ .../resolves/lean-folder-resolve.service.ts | 93 -------- src/app/views/views.routes.ts | 15 +- 3 files changed, 3 insertions(+), 316 deletions(-) delete mode 100644 src/app/core/resolves/lean-folder-resolve.service.spec.ts delete mode 100644 src/app/core/resolves/lean-folder-resolve.service.ts diff --git a/src/app/core/resolves/lean-folder-resolve.service.spec.ts b/src/app/core/resolves/lean-folder-resolve.service.spec.ts deleted file mode 100644 index dee541767..000000000 --- a/src/app/core/resolves/lean-folder-resolve.service.spec.ts +++ /dev/null @@ -1,211 +0,0 @@ -import { TestBed } from '@angular/core/testing'; -import * as Testing from '@root/test/testbedConfig'; -import { cloneDeep } from 'lodash'; -import { Router } from '@angular/router'; - -import { LeanFolderResolveService } from '@core/resolves/lean-folder-resolve.service'; -import { ApiService } from '@shared/services/api/api.service'; -import { AccountService } from '@shared/services/account/account.service'; -import { FolderResponse } from '@shared/services/api/folder.repo'; -import { FolderVO } from '@models/index'; -import { - MessageDisplayOptions, - MessageService, -} from '@shared/services/message/message.service'; - -const buildFolderResponse = (folderData: Record) => - new FolderResponse({ - isSuccessful: true, - Results: [{ data: [{ FolderVO: { ChildItemVOs: [], ...folderData } }] }], - }); - -describe('LeanFolderResolveService', () => { - let service: LeanFolderResolveService; - let api: ApiService; - let accountService: AccountService; - let message: MessageService; - let router: Router; - - beforeEach(() => { - const config = cloneDeep(Testing.BASE_TEST_CONFIG); - config.providers.push(LeanFolderResolveService); - TestBed.configureTestingModule(config); - - service = TestBed.inject(LeanFolderResolveService); - api = TestBed.inject(ApiService); - accountService = TestBed.inject(AccountService); - message = TestBed.inject(MessageService); - router = TestBed.inject(Router); - - spyOn(accountService, 'getRootFolder').and.returnValue( - new FolderVO({ - ChildItemVOs: [ - new FolderVO({ - folderId: '11', - type: 'type.folder.root.private', - archiveNbr: '0001-0001', - }), - new FolderVO({ - folderId: '22', - type: 'type.folder.root.app', - archiveNbr: '0001-0002', - }), - ], - }), - ); - }); - - it('should be created', () => { - expect(service).toBeTruthy(); - }); - - it('should load My Files by default', async () => { - const getSpy = spyOn( - api.folder, - 'getWithChildrenByIdentifier', - ).and.resolveTo(buildFolderResponse({ displayName: 'My Files' })); - - const result = await service.resolve( - { params: {} } as any, - { url: '/private' } as any, - ); - - expect(getSpy).toHaveBeenCalled(); - expect(getSpy.calls.mostRecent().args[0].folderId).toBe('11'); - expect(result.displayName).toBe('My Files'); - }); - - it('should load the apps folder on /apps', async () => { - const getSpy = spyOn( - api.folder, - 'getWithChildrenByIdentifier', - ).and.resolveTo(buildFolderResponse({ displayName: 'Apps' })); - - await service.resolve({ params: {} } as any, { url: '/apps' } as any); - - expect(getSpy.calls.mostRecent().args[0].folderId).toBe('22'); - }); - - it('should pass the route identifiers through for a deep link', async () => { - const getSpy = spyOn( - api.folder, - 'getWithChildrenByIdentifier', - ).and.resolveTo(buildFolderResponse({ displayName: 'Deep Linked' })); - - const result = await service.resolve( - { params: { archiveNbr: '0001-0005', folderLinkId: '99' } } as any, - { url: '/view/timeline/0001-0005/99' } as any, - ); - - const requestedFolder = getSpy.calls.mostRecent().args[0]; - - expect(requestedFolder.archiveNbr).toBe('0001-0005'); - expect(requestedFolder.folder_linkId).toBe(99); - expect(requestedFolder.folderId).toBeUndefined(); - expect(result.displayName).toBe('Deep Linked'); - }); - - it('should splice share crumbs onto a shared record without calling the API', async () => { - const getSpy = spyOn(api.folder, 'getWithChildrenByIdentifier'); - const sharedRecord = { displayName: 'A shared photo' }; - - const result = await service.resolve( - { - params: {}, - parent: { - data: { - sharePreviewVO: { FolderVO: null, RecordVO: sharedRecord }, - currentFolder: new FolderVO({ - pathAsText: ['My Files'], - pathAsArchiveNbr: ['0001-0001'], - pathAsFolder_linkId: [11], - }), - }, - }, - } as any, - { url: '/share/abc123/view/timeline' } as any, - ); - - expect(getSpy).not.toHaveBeenCalled(); - expect(result.pathAsText).toEqual(['Shares', 'Record', 'My Files']); - expect(result.pathAsArchiveNbr).toEqual([ - '0000-0000', - '0000-0000', - '0001-0001', - ]); - - expect(result.pathAsFolder_linkId).toEqual([0, 0, 11]); - expect(result.ChildItemVOs).toEqual([sharedRecord] as any); - }); - - it('should surface the server message when the load fails', async () => { - spyOn(api.folder, 'getWithChildrenByIdentifier').and.rejectWith( - new FolderResponse({ - isSuccessful: false, - Results: [{ message: ['Test Error'] }], - }), - ); - spyOn(accountService, 'logOut').and.resolveTo(null); - spyOn(router, 'navigate'); - let displayedErrorMessage: string; - spyOn(message, 'showError').and.callFake((data: MessageDisplayOptions) => { - displayedErrorMessage = data.message; - }); - - await expectAsync( - service.resolve({ params: {} } as any, { url: '/private' } as any), - ).toBeRejected(); - - expect(displayedErrorMessage).toBe('Test Error'); - }); - - it('should log out when a root folder fails to load', async () => { - spyOn(api.folder, 'getWithChildrenByIdentifier').and.rejectWith( - new Error('Network down'), - ); - const logOutSpy = spyOn(accountService, 'logOut').and.resolveTo(null); - spyOn(router, 'navigate'); - spyOn(message, 'showError'); - - await expectAsync( - service.resolve({ params: {} } as any, { url: '/private' } as any), - ).toBeRejected(); - - expect(logOutSpy).toHaveBeenCalled(); - }); - - it('should fall back to a generic message for a raw error', async () => { - spyOn(api.folder, 'getWithChildrenByIdentifier').and.rejectWith( - new Error('Network down'), - ); - spyOn(accountService, 'logOut').and.resolveTo(null); - spyOn(router, 'navigate'); - let displayedErrorMessage: string; - spyOn(message, 'showError').and.callFake((data: MessageDisplayOptions) => { - displayedErrorMessage = data.message; - }); - - await expectAsync( - service.resolve({ params: {} } as any, { url: '/private' } as any), - ).toBeRejected(); - - expect(displayedErrorMessage).toBe('error.generic.internal'); - }); - - it('should redirect rather than throw when a deep link fails', async () => { - spyOn(api.folder, 'getWithChildrenByIdentifier').and.rejectWith( - new Error('Network down'), - ); - const navigateSpy = spyOn(router, 'navigate'); - spyOn(message, 'showError'); - - await expectAsync( - service.resolve( - { params: { archiveNbr: '0001-0005', folderLinkId: '99' } } as any, - { url: '/view/timeline/0001-0005/99' } as any, - ), - ).toBeRejectedWith(false); - - expect(navigateSpy).toHaveBeenCalledWith(['/private']); - }); -}); diff --git a/src/app/core/resolves/lean-folder-resolve.service.ts b/src/app/core/resolves/lean-folder-resolve.service.ts deleted file mode 100644 index ef6b4b601..000000000 --- a/src/app/core/resolves/lean-folder-resolve.service.ts +++ /dev/null @@ -1,93 +0,0 @@ -import { Injectable } from '@angular/core'; -import { - ActivatedRouteSnapshot, - RouterStateSnapshot, - Router, -} from '@angular/router'; -import { find, cloneDeep } from 'lodash'; -import { ApiService } from '@shared/services/api/api.service'; -import { AccountService } from '@shared/services/account/account.service'; -import { MessageService } from '@shared/services/message/message.service'; - -import { getFolderErrorMessage } from '@shared/utilities/folder-error-message'; - -import { FolderVO } from '@root/app/models'; -import { toFolderLinkId } from '@shared/services/api/folder.repo'; - -@Injectable() -export class LeanFolderResolveService { - constructor( - private api: ApiService, - private accountService: AccountService, - private message: MessageService, - private router: Router, - ) {} - - async resolve( - route: ActivatedRouteSnapshot, - state: RouterStateSnapshot, - ): Promise { - let targetFolder; - - if (route.params.archiveNbr && route.params.folderLinkId) { - targetFolder = new FolderVO({ - archiveNbr: route.params.archiveNbr, - folder_linkId: toFolderLinkId(route.params.folderLinkId), - }); - } else if (state.url === '/apps') { - const apps = find(this.accountService.getRootFolder().ChildItemVOs, { - type: 'type.folder.root.app', - }); - targetFolder = new FolderVO(apps); - } else if (state.url.includes('/share/')) { - const sharedFolder = route.parent.data.sharePreviewVO.FolderVO; - const sharedRecord = route.parent.data.sharePreviewVO.RecordVO; - if (sharedFolder) { - targetFolder = new FolderVO(sharedFolder); - } else { - const folder = new FolderVO(cloneDeep(route.parent.data.currentFolder)); - folder.pathAsArchiveNbr.unshift('0000-0000', '0000-0000'); - folder.pathAsText.unshift('Shares', 'Record'); - folder.pathAsFolder_linkId.unshift(0, 0); - folder.ChildItemVOs = [sharedRecord]; - return folder; - } - } else { - const myFiles = find(this.accountService.getRootFolder().ChildItemVOs, { - type: 'type.folder.root.private', - }); - targetFolder = new FolderVO(myFiles); - } - - try { - const folderResponse = - await this.api.folder.getWithChildrenByIdentifier(targetFolder); - - if (!folderResponse.isSuccessful) { - throw folderResponse; - } - - return folderResponse.getFolderVO(true); - } catch (error) { - this.message.showError({ - message: getFolderErrorMessage(error), - translate: true, - }); - if (targetFolder.type?.includes('root')) { - this.accountService - .logOut() - .then(() => { - this.router.navigate(['/login']); - }) - .catch(() => { - this.router.navigate(['/login']); - }); - } else if (state.url.includes('apps')) { - this.router.navigate(['/apps']); - } else { - this.router.navigate(['/private']); - } - return await Promise.reject(false); - } - } -} diff --git a/src/app/views/views.routes.ts b/src/app/views/views.routes.ts index e935a98bc..aed86bafb 100644 --- a/src/app/views/views.routes.ts +++ b/src/app/views/views.routes.ts @@ -1,7 +1,6 @@ import { NgModule } from '@angular/core'; import { RouterModule } from '@angular/router'; import { FileViewerComponent } from '@fileBrowser/components/file-viewer/file-viewer.component'; -import { LeanFolderResolveService } from '@core/resolves/lean-folder-resolve.service'; import { RecordResolveService } from '@core/resolves/record-resolve.service'; import { FileBrowserComponentsModule } from '@fileBrowser/file-browser-components.module'; import { fileListChildRoutes } from '@fileBrowser/file-browser.routes'; @@ -16,10 +15,6 @@ const folderResolve = { currentFolder: FolderResolveService, }; -const leanFolderResolve = { - currentFolder: LeanFolderResolveService, -}; - const recordResolve = { currentRecord: RecordResolveService, }; @@ -34,13 +29,13 @@ export const routes: RoutesWithData = [ { path: '', component: TimelineViewComponent, - resolve: leanFolderResolve, + resolve: folderResolve, children: fileListChildRoutes, }, { path: ':archiveNbr/:folderLinkId', component: TimelineViewComponent, - resolve: leanFolderResolve, + resolve: folderResolve, children: [ { path: 'record/:recArchiveNbr', @@ -79,11 +74,7 @@ export const routes: RoutesWithData = [ FileBrowserComponentsModule, ], exports: [], - providers: [ - LeanFolderResolveService, - RecordResolveService, - FolderResolveService, - ], + providers: [RecordResolveService, FolderResolveService], declarations: [], }) export class ViewsRoutingModule {} From 0634c0b155ff5842795efc639011c544dc25c6bd Mon Sep 17 00:00:00 2001 From: aasandei-vsp Date: Wed, 9 Sep 2026 15:43:22 +0300 Subject: [PATCH 05/13] Make sure the shares workspace starts with a Shares breadcrumb Stela sends the real sharers hierarchy of folders, so when we build the breadcrumbs, the root will be set as Private instead of Shares. In order to bypass that, we'll check if the route we are is shared and show the correct route. Issue: PER-10678 --- .../breadcrumbs/breadcrumbs.component.spec.ts | 16 ++++++++++++++++ .../breadcrumbs/breadcrumbs.component.ts | 15 ++++++++++++--- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/src/app/shared/components/breadcrumbs/breadcrumbs.component.spec.ts b/src/app/shared/components/breadcrumbs/breadcrumbs.component.spec.ts index 140a2264b..ff1002807 100644 --- a/src/app/shared/components/breadcrumbs/breadcrumbs.component.spec.ts +++ b/src/app/shared/components/breadcrumbs/breadcrumbs.component.spec.ts @@ -127,4 +127,20 @@ describe('BreadcrumbsComponent', () => { expect(component.breadcrumbs[0].text).toEqual('Shares'); expect(component.breadcrumbs[1].routerPath).toContain('/shares/test2'); }); + + it('should keep the Shares root even when the folder path starts in the sharing archive', async () => { + await init('/shares/test2/2'); + const sharedFolderFromAnotherArchive = new FolderVO({ + pathAsArchiveNbr: ['test1', 'test2', 'test3'], + pathAsText: ['My Files', 'shared folder', 'shared inner folder'], + pathAsFolder_linkId: [1, 2, 3], + }); + TestBed.inject(DataService).setCurrentFolder( + sharedFolderFromAnotherArchive, + ); + + expect(component.breadcrumbs[0].text).toEqual('Shares'); + expect(component.breadcrumbs[0].routerPath).toEqual('/shares'); + expect(component.breadcrumbs[1].routerPath).toEqual('/shares/test2/2'); + }); }); diff --git a/src/app/shared/components/breadcrumbs/breadcrumbs.component.ts b/src/app/shared/components/breadcrumbs/breadcrumbs.component.ts index 8598bbbce..2c7172140 100644 --- a/src/app/shared/components/breadcrumbs/breadcrumbs.component.ts +++ b/src/app/shared/components/breadcrumbs/breadcrumbs.component.ts @@ -186,9 +186,18 @@ export class BreadcrumbsComponent implements OnInit, OnDestroy { } if (showRootBreadcrumb) { - this.breadcrumbs.push(new Breadcrumb(0, rootUrl, folder.pathAsText[0])); - if (this.breadcrumbs[0].routerPath === '/private') - this.breadcrumbs[0].text = 'Private'; + // A shared folder's path is rooted in the sharing archive's own tree, + // so its first path name ("My Files") would link back to this user's + // private workspace. The root crumb must be the workspace, not the path. + if (rootUrl === '/shares') { + this.breadcrumbs.push( + new Breadcrumb(0, rootUrl, 'Shares', null, null, true), + ); + } else { + this.breadcrumbs.push(new Breadcrumb(0, rootUrl, folder.pathAsText[0])); + if (this.breadcrumbs[0].routerPath === '/private') + this.breadcrumbs[0].text = 'Private'; + } } if (isInPublicArchive) { From 9c9475205f330166a67cfb5275b3bb25dcc7b160 Mon Sep 17 00:00:00 2001 From: aasandei-vsp Date: Wed, 9 Sep 2026 17:13:02 +0300 Subject: [PATCH 06/13] Add a guard for showing the Publish dialog title In the publish dialog, the title was decided by a property from the folder or record, so if that property would ever be undefined, the dialog would crash. Moved the property check in the component instead of the view and also check if the item includes a type, which is a stela property. Issue: PER-10678 --- .../components/publish/publish.component.html | 6 +-- .../publish/publish.component.spec.ts | 48 ++++++++++++++++++- .../components/publish/publish.component.ts | 10 +++- 3 files changed, 56 insertions(+), 8 deletions(-) diff --git a/src/app/file-browser/components/publish/publish.component.html b/src/app/file-browser/components/publish/publish.component.html index 18d18fce7..b8c2c7065 100644 --- a/src/app/file-browser/components/publish/publish.component.html +++ b/src/app/file-browser/components/publish/publish.component.html @@ -1,11 +1,7 @@
- {{ - this.sourceItem.folder_linkType.includes('public') - ? 'Get public link for' - : 'Publish' - }} + {{ isPublicSourceItem ? 'Get public link for' : 'Publish' }} {{ sourceItem.displayName }}
@if (publicLink) { diff --git a/src/app/file-browser/components/publish/publish.component.spec.ts b/src/app/file-browser/components/publish/publish.component.spec.ts index 7bf8703f6..0139bb720 100644 --- a/src/app/file-browser/components/publish/publish.component.spec.ts +++ b/src/app/file-browser/components/publish/publish.component.spec.ts @@ -85,6 +85,11 @@ describe('PublishComponent', () => { showErrorSpy = jasmine.createSpy('showError'); + await init({ folder_linkType: 'linkType' }); + }); + + async function init(dialogItem: unknown) { + TestBed.resetTestingModule(); await TestBed.configureTestingModule({ declarations: [PublishComponent], providers: [ @@ -93,7 +98,7 @@ describe('PublishComponent', () => { { provide: DIALOG_DATA, useValue: { - item: { folder_linkType: 'linkType' }, + item: dialogItem, }, }, { provide: DialogRef, useClass: MockDialogRef }, @@ -113,12 +118,51 @@ describe('PublishComponent', () => { fixture = TestBed.createComponent(PublishComponent); component = fixture.componentInstance; fixture.detectChanges(); - }); + } it('should create', () => { expect(component).toBeTruthy(); }); + describe('Stela-shaped items with no folder_linkType', () => { + it('should render the Publish title for a private folder without throwing', async () => { + await init( + new FolderVO({ + folderId: '900', + archiveNbr: '0002-0001', + folder_linkId: 12, + displayName: 'Trip to Iceland', + type: 'type.folder.private', + }), + ); + + expect(component.isPublicSourceItem).toBeFalse(); + + const pageTitle = fixture.nativeElement.querySelector('.page-title'); + + expect(pageTitle.textContent).toContain('Publish'); + expect(pageTitle.textContent).not.toContain('Get public link for'); + }); + + it('should treat a public folder as already published', async () => { + await init( + new FolderVO({ + folderId: '901', + archiveNbr: '0001-0002', + folder_linkId: 71, + displayName: 'Trip to Iceland', + type: 'type.folder.public', + }), + ); + + expect(component.isPublicSourceItem).toBeTrue(); + expect(component.publicItem).toBe(component.sourceItem); + expect(component.publicLink).toContain( + '/p/archive/0001-0000/0001-0002/71', + ); + }); + }); + it('should disable the public to internet archive button if the user does not have the correct access role', () => { mockAccountService.getArchive = () => new ArchiveVO({ accessRole: 'access.role.viewer' }); diff --git a/src/app/file-browser/components/publish/publish.component.ts b/src/app/file-browser/components/publish/publish.component.ts index 1c3b4aec1..a8b6e2bf7 100644 --- a/src/app/file-browser/components/publish/publish.component.ts +++ b/src/app/file-browser/components/publish/publish.component.ts @@ -32,6 +32,7 @@ export class PublishComponent { public linkCopied = false; public iaLinkCopied = false; public isAtleastManager = false; + public isPublicSourceItem = false; @ViewChild('publicLinkInput', { static: false }) publicLinkInput: ElementRef; @ViewChild('iaLinkInput', { static: false }) iaLinkInput: ElementRef; @@ -54,7 +55,14 @@ export class PublishComponent { this.isAtleastManager = this.getRole().includes('manager') || this.getRole().includes('owner'); - if (this.sourceItem?.folder_linkType?.includes('public')) { + // Stela folders carry no folder_linkType, so public-ness also has to be + // read off the folder type, the same way the sidebar decides it. + this.isPublicSourceItem = !!( + this.sourceItem?.folder_linkType?.includes('public') || + this.sourceItem?.type?.includes('public') + ); + + if (this.isPublicSourceItem) { this.publicItem = this.sourceItem; this.publicLink = this.linkPipe.transform(this.publicItem); this.checkInternetArchiveLink(); From c2bbfa804cf2298209d9270d827afa1d13526664 Mon Sep 17 00:00:00 2001 From: aasandei-vsp Date: Thu, 10 Sep 2026 13:57:03 +0300 Subject: [PATCH 07/13] 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 83910520c226aa7b1183cf3caddad787853840f9 Mon Sep 17 00:00:00 2001 From: aasandei-vsp Date: Thu, 10 Sep 2026 14:15:16 +0300 Subject: [PATCH 08/13] 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 57f1dd3a84efe5758cd2c9a7530c4f58adc250f5 Mon Sep 17 00:00:00 2001 From: aasandei-vsp Date: Thu, 10 Sep 2026 14:38:44 +0300 Subject: [PATCH 09/13] 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 8e30cd65c8ea36b0cd40ad50eeab9bd80a7d9ba9 Mon Sep 17 00:00:00 2001 From: aasandei-vsp Date: Wed, 16 Sep 2026 10:33:51 +0300 Subject: [PATCH 10/13] Create a edtf display service for formatting the edtf date in a user friendly text The EDTF date comes as a string from stela in the displayTime property. Its representation is very technical, so this service will do the heavy lifting of transforming any edtf date string into a readable text, for both the list and the sidebar, with the possibility of extension, in case we need to show the edtf date in other places, with specific formatting. Issue: PER-10655 --- .../sidebar-date-picker.component.ts | 10 +- .../edtf-service/edtf-display.service.spec.ts | 357 ++++++++++++++ .../edtf-service/edtf-display.service.ts | 466 ++++++++++++++++++ .../services/edtf-service/edtf.service.ts | 2 +- 4 files changed, 831 insertions(+), 4 deletions(-) create mode 100644 src/app/shared/services/edtf-service/edtf-display.service.spec.ts create mode 100644 src/app/shared/services/edtf-service/edtf-display.service.ts diff --git a/src/app/file-browser/components/sidebar-date-picker/sidebar-date-picker.component.ts b/src/app/file-browser/components/sidebar-date-picker/sidebar-date-picker.component.ts index 64c1666bf..78525a9f5 100644 --- a/src/app/file-browser/components/sidebar-date-picker/sidebar-date-picker.component.ts +++ b/src/app/file-browser/components/sidebar-date-picker/sidebar-date-picker.component.ts @@ -22,6 +22,7 @@ import { DateQualifierFlags, DEFAULT_DATE_QUALIFIERS, } from '@shared/services/edtf-service/edtf.service'; +import { EdtfDisplayService } from '@shared/services/edtf-service/edtf-display.service'; import { DatepickerInputComponent } from '@shared/components/datepicker-input/datepicker-input.component'; import { TimepickerInputComponent } from '@shared/components/timepicker-input/timepicker-input.component'; @@ -61,7 +62,10 @@ export class SidebarDatePickerComponent implements OnInit, OnChanges { @ViewChild('sidebarDatePickerContainer') container?: ElementRef; - constructor(private readonly edtfService: EdtfService) {} + constructor( + private readonly edtfService: EdtfService, + private readonly edtfDisplayService: EdtfDisplayService, + ) {} isDropdownOpen = signal(false); @@ -94,7 +98,7 @@ export class SidebarDatePickerComponent implements OnInit, OnChanges { formattedStartDate = computed(() => { if (this._qualifiers().unknown) return 'Unknown'; if (this._isOpenStart()) return '..'; - return this.formatDate(this._date()); + return this.edtfDisplayService.formatDateForDisplay(this._date()); }); formattedStartTime = computed(() => this.formatTime(this._time())); @@ -119,7 +123,7 @@ export class SidebarDatePickerComponent implements OnInit, OnChanges { formattedEndDate = computed(() => { if (this._endQualifiers().unknown) return 'Unknown'; if (this._isOpenEnd()) return '..'; - return this.formatDate(this._endDate()); + return this.edtfDisplayService.formatDateForDisplay(this._endDate()); }); formattedEndTime = computed(() => this.formatTime(this._endTime())); diff --git a/src/app/shared/services/edtf-service/edtf-display.service.spec.ts b/src/app/shared/services/edtf-service/edtf-display.service.spec.ts new file mode 100644 index 000000000..3ec5409df --- /dev/null +++ b/src/app/shared/services/edtf-service/edtf-display.service.spec.ts @@ -0,0 +1,357 @@ +import { TestBed } from '@angular/core/testing'; + +import { + EdtfDisplayService, + FILE_LIST_DATE_OPTIONS, +} from './edtf-display.service'; + +describe('EdtfDisplayService', () => { + let service: EdtfDisplayService; + + beforeEach(() => { + TestBed.configureTestingModule({}); + service = TestBed.inject(EdtfDisplayService); + }); + + const dateTextOf = (displayTime: string): string => + service.formatToPlainText(displayTime); + + const dateEndTextOf = (displayTime: string): string => + service + .formatForDisplay(displayTime) + .dateEnd.map((segment) => segment.text) + .join(''); + + const timeTextOf = (displayTime: string): string => + service + .formatForDisplay(displayTime) + .time.map((segment) => segment.text) + .join(''); + + it('should be created', () => { + expect(service).toBeTruthy(); + }); + + describe('every value in the supported-values sheet', () => { + // Ranks are the EDTF_sort_ranking sheet's, so a failure names its row. + const supportedValues: [ + rank: number, + storedValue: string, + expectedDate: string, + expectedTime: string, + ][] = [ + [1, '../1985-04-12', 'Before Apr. 12, 1985', ''], + [2, '/1985-04-12', 'Before Apr. 12, 1985', ''], + [3, '0000', '0000', ''], + [4, 'XXXX-XX-XX', 'Unknown', ''], + [5, '1000', '1000', ''], + [6, '15XX-12-25', 'Dec. 25, 15XX', ''], + [7, '156X-12-25', 'Dec. 25, 156X', ''], + [8, '1900-01-01', 'Jan. 1, 1900', ''], + [9, '1964/2008', '1964 — 2008', ''], + [11, '1984%', '1984 %', ''], + [12, '1984?', '1984 ?', ''], + [13, '1984~', '1984 ~', ''], + [14, '1985', '1985', ''], + [15, '1985-XX-03', '1985-XX-03', ''], + [16, '1985-04', 'April 1985', ''], + [17, '1985-04-12', 'Apr. 12, 1985', ''], + [18, '1985-04-12T23:20:30', 'Apr. 12, 1985', '11:20 PM'], + [19, '1985-04-12T23:20:30-04:00', 'Apr. 12, 1985', '11:20 PM'], + [20, '1985-04-12/1985-06-10', 'Apr. 12 — Jun. 10, 1985', ''], + [21, '1985-04-12/', 'After Apr. 12, 1985', ''], + [22, '1985-04-12/..', 'After Apr. 12, 1985', ''], + ]; + + supportedValues.forEach( + ([rank, storedValue, expectedDate, expectedTime]) => { + it(`should render rank ${rank}, ${storedValue}, as "${expectedDate}"`, () => { + expect(dateTextOf(storedValue)).toBe(expectedDate); + expect(timeTextOf(storedValue)).toBe(expectedTime); + }); + }, + ); + }); + + describe('the three shapes the sheet does not list', () => { + it('should keep both times for a range inside one day', () => { + const sameDayRange = '2001-09-24T09:45:00Z/2001-09-24T16:00:00Z'; + + expect(dateTextOf(sameDayRange)).toBe('Sep. 24, 2001'); + expect(timeTextOf(sameDayRange)).toBe('9:45 AM — 4:00 PM'); + }); + + it('should print the month once for a range inside one month', () => { + expect(dateTextOf('2001-09-24/2001-09-30')).toBe('Sep. 24 — 30, 2001'); + }); + + it('should render a range with both sides unknown', () => { + expect(dateTextOf('/')).toBe('Unknown — Unknown'); + }); + }); + + describe('a range splits so its end can wrap', () => { + it('should keep the dash with the start', () => { + expect( + service + .formatForDisplay('1985-04-12/1985-06-10') + .date.map((segment) => segment.text) + .join(''), + ).toBe('Apr. 12 \u2014'); + + expect(dateEndTextOf('1985-04-12/1985-06-10')).toBe('Jun. 10, 1985'); + }); + + it('should split a same-month range after the dash', () => { + expect(dateEndTextOf('2001-09-24/2001-09-30')).toBe('30, 2001'); + }); + + it('should split a year-only range', () => { + expect(dateEndTextOf('1964/2008')).toBe('2008'); + }); + + it('should split a range with both sides unknown', () => { + expect(dateEndTextOf('/')).toBe('Unknown'); + }); + + it('should not split a value that is not a two-sided range', () => { + expect(dateEndTextOf('1985-04-12')).toBe(''); + expect(dateEndTextOf('1985-04-12/..')).toBe(''); + expect(dateEndTextOf('../1985-04-12')).toBe(''); + expect(dateEndTextOf('2001-09-24T09:45:00Z/2001-09-24T16:00:00Z')).toBe( + '', + ); + }); + }); + + describe('qualifiers', () => { + it('should append the stored symbol rather than an abbreviation', () => { + expect(dateTextOf('1962-03~')).toBe('March 1962 ~'); + expect(dateTextOf('1962-03~')).not.toContain('c.'); + }); + + it('should keep a qualifier on the side that carries it', () => { + expect(dateTextOf('1984?/1990~')).toBe('1984 ? — 1990 ~'); + }); + + it('should not stop a qualified side from merging', () => { + expect(dateTextOf('2001-09-24?/2001-09-30')).toBe('Sep. 24 ? — 30, 2001'); + }); + + it('should mark a qualifier as an annotation so it can be muted', () => { + const segments = service.formatForDisplay('1984~').date; + + expect(segments).toEqual([ + { text: '1984', isAnnotation: false }, + { text: ' ~', isAnnotation: true }, + ]); + }); + }); + + describe('annotations', () => { + it('should mark Unknown as an annotation', () => { + expect(service.formatForDisplay('XXXX-XX-XX').date).toEqual([ + { text: 'Unknown', isAnnotation: true }, + ]); + }); + + it('should mark the open-interval prefix as an annotation', () => { + expect(service.formatForDisplay('1985-04-12/..').date).toEqual([ + { text: 'After ', isAnnotation: true }, + { text: 'Apr. 12, 1985', isAnnotation: false }, + ]); + }); + }); + + describe('values that carry a time', () => { + it('should drop the seconds', () => { + expect(timeTextOf('1985-04-12T23:20:59')).toBe('11:20 PM'); + }); + + it('should not show a timezone next to the time', () => { + const rendered = service.formatForDisplay('1985-04-12T23:20:30-04:00'); + const allText = [...rendered.date, ...rendered.time] + .map((segment) => segment.text) + .join(''); + + expect(allText).not.toMatch(/C[SD]T|UTC|GMT|[+-]\d{2}:\d{2}/); + }); + + it('should drop the time on an interval that is not inside one day', () => { + expect(timeTextOf('1985-04-12T09:00:00Z/1985-06-10T17:00:00Z')).toBe(''); + }); + + it('should drop the time on an open-ended interval', () => { + expect(timeTextOf('1985-04-12T09:00:00Z/..')).toBe(''); + }); + }); + + describe('values it cannot parse', () => { + it('should show the stored string rather than an error', () => { + expect(dateTextOf('not-a-date')).toBe('not-a-date'); + }); + + it('should show a qualifier on unspecified digits verbatim', () => { + // Not a supported value: the edtf grammar rejects a qualifier sitting + // on X digits, so it falls through to the stored string. + expect(dateTextOf('198X?')).toBe('198X?'); + }); + + it('should render nothing for an absent value', () => { + const nothing = { date: [], dateEnd: [], time: [] }; + + expect(service.formatForDisplay('')).toEqual(nothing); + expect(service.formatForDisplay(null)).toEqual(nothing); + expect(service.formatForDisplay(undefined)).toEqual(nothing); + }); + }); + + it('should never render the strings the moment pipe produces today', () => { + const everyStoredValue = [ + '../1985-04-12', + '/1985-04-12', + '0000', + 'XXXX-XX-XX', + '1000', + '15XX-12-25', + '156X-12-25', + '1900-01-01', + '1964/2008', + '198X?', + '1984%', + '1984?', + '1984~', + '1985', + '1985-XX-03', + '1985-04', + '1985-04-12', + '1985-04-12T23:20:30', + '1985-04-12T23:20:30-04:00', + '1985-04-12/1985-06-10', + '1985-04-12/', + '1985-04-12/..', + '/', + '2001-09-24/2001-09-30', + '1985-04-XX', + ]; + + everyStoredValue.forEach((storedValue) => { + const rendered = dateTextOf(storedValue); + + expect(rendered).not.toContain('Invalid'); + expect(rendered).not.toContain('NaN'); + expect(rendered).toBeTruthy(); + }); + }); + + describe('formatForTooltip', () => { + it('should carry the times a range drops', () => { + expect( + service.formatForTooltip('1985-04-12T09:00:00Z/1985-06-10T17:00:00Z'), + ).toEqual({ + from: 'Apr. 12, 1985 \u2022 9:00:00 AM', + to: 'Jun. 10, 1985 \u2022 5:00:00 PM', + }); + }); + + it('should say nothing for anything that is not a range hiding a time', () => { + // a single value always shows its own time, seconds included or not + expect(service.formatForTooltip('1985-04-12T23:20:30')).toBeNull(); + expect(service.formatForTooltip('1985-04-12T23:20:00')).toBeNull(); + expect(service.formatForTooltip('1985-04-12')).toBeNull(); + // a range carrying no time has nothing to reveal + expect(service.formatForTooltip('1985-04-12/1985-06-10')).toBeNull(); + expect(service.formatForTooltip('XXXX-XX-XX')).toBeNull(); + expect(service.formatForTooltip('')).toBeNull(); + }); + + it('should not claim a same-day range hides its times', () => { + expect( + service.formatForTooltip('2001-09-24T09:45:00Z/2001-09-24T16:00:00Z'), + ).toBeNull(); + }); + + it('should name an unknown side rather than leaving it blank', () => { + expect(service.formatForTooltip('1985-04-12T09:00:00Z/')).toEqual({ + from: 'Apr. 12, 1985 \u2022 9:00:00 AM', + to: 'Unknown', + }); + }); + }); + + describe('formatDateForDisplay', () => { + it('should read a year as a year without going through a number', () => { + expect(service.formatDateForDisplay({ year: '0000' })).toBe('0000'); + }); + + it('should pad unspecified year digits with X', () => { + expect(service.formatDateForDisplay({ year: '198' })).toBe('198X'); + }); + + it('should abbreviate the month only when a day is present', () => { + expect( + service.formatDateForDisplay( + { year: '1985', month: '04', day: '12' }, + FILE_LIST_DATE_OPTIONS, + ), + ).toBe('Apr. 12, 1985'); + + expect( + service.formatDateForDisplay( + { year: '1985', month: '04' }, + FILE_LIST_DATE_OPTIONS, + ), + ).toBe('April 1985'); + }); + + it('should keep the sidebar picker on full month names and padded days', () => { + expect( + service.formatDateForDisplay({ + year: '1985', + month: '05', + day: '2', + }), + ).toBe('May 02, 1985'); + }); + + it('should drop the leading zero on a day for the file list', () => { + expect( + service.formatDateForDisplay( + { year: '1900', month: '01', day: '01' }, + FILE_LIST_DATE_OPTIONS, + ), + ).toBe('Jan. 1, 1900'); + }); + + it('should keep the numeric form zero-padded when the month is unspecified', () => { + expect( + service.formatDateForDisplay( + { year: '1985', month: '', day: '03' }, + FILE_LIST_DATE_OPTIONS, + ), + ).toBe('1985-XX-03'); + }); + + it('should put a period only on a month that is actually shortened', () => { + expect( + service.formatDateForDisplay( + { year: '2001', month: '05', day: '24' }, + FILE_LIST_DATE_OPTIONS, + ), + ).toBe('May 24, 2001'); + + expect( + service.formatDateForDisplay( + { year: '2001', month: '09', day: '24' }, + FILE_LIST_DATE_OPTIONS, + ), + ).toBe('Sep. 24, 2001'); + }); + + it('should render nothing for an empty date', () => { + expect( + service.formatDateForDisplay({ year: '', month: '', day: '' }), + ).toBe(''); + }); + }); +}); diff --git a/src/app/shared/services/edtf-service/edtf-display.service.ts b/src/app/shared/services/edtf-service/edtf-display.service.ts new file mode 100644 index 000000000..22a751357 --- /dev/null +++ b/src/app/shared/services/edtf-service/edtf-display.service.ts @@ -0,0 +1,466 @@ +import { Injectable, inject } from '@angular/core'; +import { format } from 'date-fns'; +import { + DateModel, + DateQualifierFlags, + DateTimeModel, + EdtfService, + TIME_FORMAT_LABEL, + TimeModel, +} from './edtf.service'; + +export interface EdtfDisplaySegment { + text: string; + isAnnotation: boolean; +} + +export interface EdtfDisplayText { + /** The value, or the start of a range together with its dash. */ + date: EdtfDisplaySegment[]; + /** The end of a range, so it can wrap onto its own line. Empty otherwise. */ + dateEnd: EdtfDisplaySegment[]; + time: EdtfDisplaySegment[]; +} + +export interface EdtfTooltipText { + from: string; + to: string; +} + +export interface DateDisplayOptions { + abbreviateMonthWithDay: boolean; + padSingleDigitDay: boolean; +} + +export const SIDEBAR_PICKER_DATE_OPTIONS: DateDisplayOptions = { + abbreviateMonthWithDay: false, + padSingleDigitDay: true, +}; + +export const FILE_LIST_DATE_OPTIONS: DateDisplayOptions = { + abbreviateMonthWithDay: true, + padSingleDigitDay: false, +}; + +const MONTHS_IN_YEAR = 12; + +const UNKNOWN_LABEL = 'Unknown'; +const OPEN_START_PREFIX = 'Before '; +const OPEN_END_PREFIX = 'After '; +const RANGE_SEPARATOR = ' —'; +const NO_MONTH_INDEX = -1; +const SEPARATOR = ' \u2022'; + +const EMPTY_DISPLAY_TEXT: EdtfDisplayText = { date: [], dateEnd: [], time: [] }; + +type IntervalMergeLevel = 'day' | 'month' | 'year' | 'none'; + +interface EdtfIntervalSide { + date: DateModel; + dateText: string; + timeText: string; + timeTextWithSeconds: string; + qualifierSymbol: string; + isEmpty: boolean; +} + +interface EdtfIntervalSides { + isInterval: boolean; + start: EdtfIntervalSide; + end: EdtfIntervalSide | null; +} + +function content(text: string): EdtfDisplaySegment { + return { text, isAnnotation: false }; +} + +function annotation(text: string): EdtfDisplaySegment { + return { text, isAnnotation: true }; +} + +function joinSegments(segments: EdtfDisplaySegment[]): EdtfDisplaySegment[] { + return segments.reduce((joined, segment) => { + if (!segment.text) { + return joined; + } + const previous = joined[joined.length - 1]; + if (previous && previous.isAnnotation === segment.isAnnotation) { + previous.text += segment.text; + return joined; + } + joined.push({ ...segment }); + return joined; + }, []); +} + +@Injectable({ + providedIn: 'root', +}) +export class EdtfDisplayService { + private readonly edtfService = inject(EdtfService); + + formatDateForDisplay( + date: DateModel, + options: DateDisplayOptions = SIDEBAR_PICKER_DATE_OPTIONS, + ): string { + const yearValue = date?.year ?? ''; + const monthValue = date?.month ?? ''; + const dayValue = date?.day ?? ''; + + const hasYear = !!yearValue; + const hasMonth = !!monthValue; + // A lone '0' day is an unfinished value ('05' minus a keystroke), so it + // is treated as absent rather than guessed at. + const hasDay = !!dayValue && parseInt(dayValue, 10) !== 0; + + if (!hasYear && !hasMonth && !hasDay) { + return ''; + } + + const yearDisplay = this.edtfService.padWithX(yearValue, 4); + const monthIndex = this.toMonthIndex(monthValue); + + if (monthIndex !== NO_MONTH_INDEX && hasDay) { + const monthName = options.abbreviateMonthWithDay + ? this.abbreviatedMonthName(monthIndex) + : this.fullMonthName(monthIndex); + const dayDisplay = options.padSingleDigitDay + ? dayValue.padStart(2, '0') + : this.withoutLeadingZeros(dayValue); + return `${monthName} ${dayDisplay}, ${yearDisplay}`; + } + + if (monthIndex !== NO_MONTH_INDEX) { + return `${this.fullMonthName(monthIndex)} ${yearDisplay}`; + } + + if (!hasMonth && !hasDay) { + return yearDisplay; + } + + const parts = [ + yearDisplay, + hasMonth ? this.edtfService.padWithX(monthValue, 2) : 'XX', + ]; + if (hasDay) { + parts.push(dayValue.padStart(2, '0')); + } + return parts.join('-'); + } + + formatForDisplay(displayTime: string | null | undefined): EdtfDisplayText { + if (!displayTime) { + return EMPTY_DISPLAY_TEXT; + } + + const sides = this.toIntervalSides(displayTime); + + if (!sides) { + return { ...EMPTY_DISPLAY_TEXT, date: [content(displayTime)] }; + } + + return sides.end + ? this.buildInterval(sides.start, sides.end) + : this.buildSingleValue(sides.start); + } + + /** + * The full value for a range whose times the row does not show, or null for + * anything else — a single value shows its own time, and a range inside one + * day shows both of them. + */ + formatForTooltip( + displayTime: string | null | undefined, + ): EdtfTooltipText | null { + const sides = this.toIntervalSides(displayTime); + if (!sides) { + return null; + } + + const { start, end } = sides; + const showsItsTimes = + !end || this.intervalMergeLevel(start.date, end.date) === 'day'; + + if (showsItsTimes || !(start.timeText || end.timeText)) { + return null; + } + + return { from: this.toFullText(start), to: this.toFullText(end) }; + } + + formatToPlainText(displayTime: string | null | undefined): string { + const rendered = this.formatForDisplay(displayTime); + return [...rendered.date, ...rendered.dateEnd] + .map((segment) => segment.text) + .join(' ') + .replace(/\s+/g, ' ') + .trim(); + } + + private toIntervalSides( + displayTime: string | null | undefined, + ): EdtfIntervalSides | null { + const model = this.parseOrNull(displayTime); + + if (!model) { + return null; + } + + const isInterval = !!displayTime?.includes('/'); + + return { + isInterval, + start: this.toIntervalSide(model.qualifiers, model.date, model.time), + end: isInterval + ? this.toIntervalSide(model.endQualifiers, model.endDate, model.endTime) + : null, + }; + } + + private toFullText(side: EdtfIntervalSide): string { + if (side.isEmpty) { + return UNKNOWN_LABEL; + } + const dateText = `${side.dateText}${side.qualifierSymbol ? ` ${side.qualifierSymbol}` : ''}`; + return side.timeTextWithSeconds + ? `${dateText}${SEPARATOR} ${side.timeTextWithSeconds}` + : dateText; + } + + private parseOrNull( + edtfString: string | null | undefined, + ): DateTimeModel | null { + try { + return this.edtfService.toDateTimeModel(edtfString ?? ''); + } catch { + return null; + } + } + + private toQualifierSymbol( + qualifiers: DateQualifierFlags | undefined, + ): string { + if (qualifiers?.approximate && qualifiers?.uncertain) return '%'; + if (qualifiers?.approximate) return '~'; + if (qualifiers?.uncertain) return '?'; + return ''; + } + + private toIntervalSide( + qualifiers: DateQualifierFlags | undefined, + date: DateModel | undefined, + time: TimeModel | undefined, + ): EdtfIntervalSide { + const hasAnyDatePart = !!(date?.year || date?.month || date?.day); + const isEmpty = !!qualifiers?.unknown || !hasAnyDatePart; + + return { + date: date ?? { year: '' }, + dateText: isEmpty + ? '' + : this.formatDateForDisplay(date, FILE_LIST_DATE_OPTIONS), + timeText: this.formatTimeForDisplay(time), + timeTextWithSeconds: this.formatTimeForDisplay(time, true), + qualifierSymbol: this.toQualifierSymbol(qualifiers), + isEmpty, + }; + } + + private buildSingleValue(side: EdtfIntervalSide): EdtfDisplayText { + if (side.isEmpty) { + return { ...EMPTY_DISPLAY_TEXT, date: [annotation(UNKNOWN_LABEL)] }; + } + + return { + ...EMPTY_DISPLAY_TEXT, + date: joinSegments([ + content(side.dateText), + ...this.qualifierSegments(side), + ]), + time: side.timeText ? [content(side.timeText)] : [], + }; + } + + private buildInterval( + start: EdtfIntervalSide, + end: EdtfIntervalSide, + ): EdtfDisplayText { + if (start.isEmpty && end.isEmpty) { + return { + ...EMPTY_DISPLAY_TEXT, + date: [annotation(UNKNOWN_LABEL), content(RANGE_SEPARATOR)], + dateEnd: [annotation(UNKNOWN_LABEL)], + }; + } + + if (start.isEmpty) { + return { + ...EMPTY_DISPLAY_TEXT, + date: joinSegments([ + annotation(OPEN_START_PREFIX), + content(end.dateText), + ...this.qualifierSegments(end), + ]), + }; + } + + if (end.isEmpty) { + return { + ...EMPTY_DISPLAY_TEXT, + date: joinSegments([ + annotation(OPEN_END_PREFIX), + content(start.dateText), + ...this.qualifierSegments(start), + ]), + }; + } + + return this.buildClosedInterval(start, end); + } + + private buildClosedInterval( + start: EdtfIntervalSide, + end: EdtfIntervalSide, + ): EdtfDisplayText { + const mergeLevel = this.intervalMergeLevel(start.date, end.date); + + if (mergeLevel === 'day') { + return { + ...EMPTY_DISPLAY_TEXT, + date: joinSegments([ + content(start.dateText), + ...this.qualifierSegments(start), + ...this.qualifierSegments(end), + ]), + time: this.buildTimeRange(start, end), + }; + } + + if (mergeLevel === 'none') { + return { + ...EMPTY_DISPLAY_TEXT, + date: joinSegments([ + content(start.dateText), + ...this.qualifierSegments(start), + content(RANGE_SEPARATOR), + ]), + dateEnd: joinSegments([ + content(end.dateText), + ...this.qualifierSegments(end), + ]), + }; + } + + const endLabel = + mergeLevel === 'month' + ? this.withoutLeadingZeros(end.date.day) + : this.monthAndDayLabel(end.date); + + return { + ...EMPTY_DISPLAY_TEXT, + date: joinSegments([ + content(this.monthAndDayLabel(start.date)), + ...this.qualifierSegments(start), + content(RANGE_SEPARATOR), + ]), + dateEnd: joinSegments([ + content(endLabel), + ...this.qualifierSegments(end), + content(`, ${this.edtfService.padWithX(start.date.year, 4)}`), + ]), + }; + } + + private intervalMergeLevel( + start: DateModel, + end: DateModel, + ): IntervalMergeLevel { + if (!this.isMergeable(start) || !this.isMergeable(end)) { + return 'none'; + } + if (start.year !== end.year) { + return 'none'; + } + if (start.month !== end.month) { + return 'year'; + } + if (start.day !== end.day) { + return 'month'; + } + return 'day'; + } + + private isMergeable(date: DateModel): boolean { + return ( + !!date.year && + !!date.day && + this.toMonthIndex(date.month ?? '') !== NO_MONTH_INDEX + ); + } + + private buildTimeRange( + start: EdtfIntervalSide, + end: EdtfIntervalSide, + ): EdtfDisplaySegment[] { + if (start.timeText && end.timeText) { + return [content(`${start.timeText}${RANGE_SEPARATOR} ${end.timeText}`)]; + } + const singleTime = start.timeText || end.timeText; + return singleTime ? [content(singleTime)] : []; + } + + private qualifierSegments(side: EdtfIntervalSide): EdtfDisplaySegment[] { + return side.qualifierSymbol ? [annotation(` ${side.qualifierSymbol}`)] : []; + } + + private monthAndDayLabel(date: DateModel): string { + const monthName = this.abbreviatedMonthName(this.toMonthIndex(date.month)); + return `${monthName} ${this.withoutLeadingZeros(date.day)}`; + } + + private formatTimeForDisplay( + time: TimeModel | undefined, + withSeconds = false, + ): string { + const hours = time?.hours ?? ''; + if (!hours) { + return ''; + } + + const minutes = (time.minutes || '00').padStart(2, '0'); + const seconds = withSeconds + ? `:${(time.seconds || '00').padStart(2, '0')}` + : ''; + + if (time.format === 'h24') { + return `${hours.padStart(2, '0')}:${minutes}${seconds}`; + } + + return `${this.withoutLeadingZeros(hours)}:${minutes}${seconds} ${TIME_FORMAT_LABEL[time.format]}`; + } + + private toMonthIndex(month: string | undefined): number { + if (!/^\d{2}$/.test(month ?? '')) { + return NO_MONTH_INDEX; + } + const monthIndex = parseInt(month, 10) - 1; + return monthIndex >= 0 && monthIndex < MONTHS_IN_YEAR + ? monthIndex + : NO_MONTH_INDEX; + } + + private fullMonthName(monthIndex: number): string { + return format(new Date(2000, monthIndex), 'MMMM'); + } + + private abbreviatedMonthName(monthIndex: number): string { + const abbreviated = format(new Date(2000, monthIndex), 'MMM'); + return abbreviated === this.fullMonthName(monthIndex) + ? abbreviated + : `${abbreviated}.`; + } + + private withoutLeadingZeros(value: string | undefined): string { + return (value ?? '').replace(/^0+(?=\d)/, ''); + } +} diff --git a/src/app/shared/services/edtf-service/edtf.service.ts b/src/app/shared/services/edtf-service/edtf.service.ts index 300743f44..7236d1109 100644 --- a/src/app/shared/services/edtf-service/edtf.service.ts +++ b/src/app/shared/services/edtf-service/edtf.service.ts @@ -371,7 +371,7 @@ export class EdtfService { return this.padWithX(value, 2); } - private padWithX(value: string, width: number): string { + padWithX(value: string, width: number): string { const v = value ?? ''; return v.length >= width ? v : v + 'X'.repeat(width - v.length); } From 182178151434ef9dcd07f4ce33ca3127445de40e Mon Sep 17 00:00:00 2001 From: aasandei-vsp Date: Wed, 16 Sep 2026 10:54:20 +0300 Subject: [PATCH 11/13] Create a presentational component to display the user friendly formatted edtf date The edtf display component uses the edtf display service to format the edtf date that comes from stela on the displayTime property. It adds styling and behaviour so it fits correctly in the file list and also show more data when it is needed via a tooltip. Issue: PER-10655 --- .../file-browser-components.module.ts | 2 + .../edtf-date-display.component.html | 49 +++++++++++++++++++ .../edtf-date-display.component.scss | 37 ++++++++++++++ .../edtf-date-display.component.ts | 40 +++++++++++++++ src/styles/_ng-bootstrap.scss | 23 +++++++++ src/styles/_variables.scss | 30 +++++++++--- 6 files changed, 174 insertions(+), 7 deletions(-) create mode 100644 src/app/shared/components/edtf-date-display/edtf-date-display.component.html create mode 100644 src/app/shared/components/edtf-date-display/edtf-date-display.component.scss create mode 100644 src/app/shared/components/edtf-date-display/edtf-date-display.component.ts diff --git a/src/app/file-browser/file-browser-components.module.ts b/src/app/file-browser/file-browser-components.module.ts index 16921f1b4..d1edc30eb 100644 --- a/src/app/file-browser/file-browser-components.module.ts +++ b/src/app/file-browser/file-browser-components.module.ts @@ -16,6 +16,7 @@ import { FaIconLibrary, } from '@fortawesome/angular-fontawesome'; import { faFileArchive } from '@fortawesome/free-solid-svg-icons'; +import { EdtfDateDisplayComponent } from '@shared/components/edtf-date-display/edtf-date-display.component'; import { FolderViewComponent } from './components/folder-view/folder-view.component'; import { PublishComponent } from './components/publish/publish.component'; import { FolderDescriptionComponent } from './components/folder-description/folder-description.component'; @@ -40,6 +41,7 @@ import { SidebarLocationComponent } from './components/sidebar-location/sidebar- FontAwesomeModule, SidebarDatePickerComponent, SidebarLocationComponent, + EdtfDateDisplayComponent, ], exports: [ FileListComponent, diff --git a/src/app/shared/components/edtf-date-display/edtf-date-display.component.html b/src/app/shared/components/edtf-date-display/edtf-date-display.component.html new file mode 100644 index 000000000..2a7424f7a --- /dev/null +++ b/src/app/shared/components/edtf-date-display/edtf-date-display.component.html @@ -0,0 +1,49 @@ + + + @for (segment of dateSegments(); track $index) { + {{ segment.text }} + } + @if (hasTime()) { + • + } + @if (tooltip() && !dateEndSegments().length) { + info + } + + @if (dateEndSegments().length) { + + @for (segment of dateEndSegments(); track $index) { + {{ + segment.text + }} + } + @if (tooltip()) { + info + } + + } + @if (hasTime()) { + + @for (segment of timeSegments(); track $index) { + {{ + segment.text + }} + } + + } + + + + @if (tooltip(); as lines) { +
+ From{{ lines.from }} +
+
To{{ lines.to }}
+ } + diff --git a/src/app/shared/components/edtf-date-display/edtf-date-display.component.scss b/src/app/shared/components/edtf-date-display/edtf-date-display.component.scss new file mode 100644 index 000000000..14a74358a --- /dev/null +++ b/src/app/shared/components/edtf-date-display/edtf-date-display.component.scss @@ -0,0 +1,37 @@ +@import 'variables'; + +:host { + display: block; + min-width: 0; +} + +// Each part is unbreakable and the row wraps between them, so a value too wide +// for its column moves its end onto a second line instead of being cut off. +.value { + display: flex; + flex-wrap: wrap; + align-items: baseline; + column-gap: 0.35em; +} + +.part { + white-space: nowrap; +} + +.separator { + margin-left: 0.35em; +} + +// Opacity rather than a fixed colour so an annotation reads as lighter both in +// the Date column and in the already-muted row under the item name. +.annotation { + opacity: 0.65; +} + +.more-info { + font-size: 1.1em; + line-height: 1; + vertical-align: text-bottom; + margin-left: 0.4em; + opacity: 0.65; +} diff --git a/src/app/shared/components/edtf-date-display/edtf-date-display.component.ts b/src/app/shared/components/edtf-date-display/edtf-date-display.component.ts new file mode 100644 index 000000000..8ca514ff0 --- /dev/null +++ b/src/app/shared/components/edtf-date-display/edtf-date-display.component.ts @@ -0,0 +1,40 @@ +import { + ChangeDetectionStrategy, + Component, + computed, + inject, + input, +} from '@angular/core'; +import { NgbTooltipModule } from '@ng-bootstrap/ng-bootstrap'; +import { ngIfFadeInAnimation } from '@shared/animations'; +import { EdtfDisplayService } from '@shared/services/edtf-service/edtf-display.service'; + +@Component({ + selector: 'pr-edtf-date-display', + standalone: true, + imports: [NgbTooltipModule], + templateUrl: './edtf-date-display.component.html', + styleUrls: ['./edtf-date-display.component.scss'], + changeDetection: ChangeDetectionStrategy.OnPush, + animations: [ngIfFadeInAnimation], +}) +export class EdtfDateDisplayComponent { + readonly displayTime = input(); + readonly showTime = input(true); + + private readonly edtfDisplayService = inject(EdtfDisplayService); + + private readonly displayText = computed(() => + this.edtfDisplayService.formatForDisplay(this.displayTime()), + ); + + readonly dateSegments = computed(() => this.displayText().date); + readonly dateEndSegments = computed(() => this.displayText().dateEnd); + readonly timeSegments = computed(() => this.displayText().time); + readonly hasTime = computed( + () => this.showTime() && this.timeSegments().length > 0, + ); + readonly tooltip = computed(() => + this.edtfDisplayService.formatForTooltip(this.displayTime()), + ); +} diff --git a/src/styles/_ng-bootstrap.scss b/src/styles/_ng-bootstrap.scss index 5ddc6aa44..02640f80c 100644 --- a/src/styles/_ng-bootstrap.scss +++ b/src/styles/_ng-bootstrap.scss @@ -87,3 +87,26 @@ ngb-timepicker { justify-content: center; } } + +// The tooltip window is appended to the body, outside this component's styles, +// so its overrides have to be global. Bootstrap caps the window at 200px, which +// is narrower than a full date and time and leaves the value running past the +// background. +.edtf-date-tooltip { + .tooltip-inner { + max-width: none; + padding: $grid-unit * 0.5 $grid-unit * 0.75; + text-align: left; + } + + .tooltip-line { + white-space: nowrap; + line-height: 1.5; + + .label { + display: inline-block; + width: 3.2em; + opacity: 0.65; + } + } +} diff --git a/src/styles/_variables.scss b/src/styles/_variables.scss index 461efc5b2..9f5e3d01a 100644 --- a/src/styles/_variables.scss +++ b/src/styles/_variables.scss @@ -61,7 +61,8 @@ $file-list-controls-height: 4 * $grid-unit; $file-list-thumb-col-width: $file-list-row-height; $file-list-type-col-width: 8rem; $file-list-access-col-width: $file-list-type-col-width; -$file-list-date-col-width: 8rem; +$file-list-date-col-width: 11.5rem; +$file-list-date-col-width-wide: 14rem; $file-list-name-col-width: 30rem; $file-list-shared-by-col-width: 20rem; $file-list-shared-icon-col-width: 2rem; @@ -71,6 +72,7 @@ $phone: 450px; $tablet: 768px; $tablet-horizontal: 900px; $desktop: 1200px; +$desktop-wide: 1440px; $scrollbar-margin: pxToGrid(20px); @@ -109,6 +111,15 @@ $linear-gradient-background: linear-gradient(90deg, #131b4a 0%, #364493 100%); margin-right: auto; } +@function file-list-name-basis($date-col-width) { + $ui-width-px: $left-menu-width + $sidebar-width - (4 * $grid-unit); + $list-items-width-px: $scrollbar-margin + $file-list-row-height + $grid-unit; + $list-items-width-em: $file-list-type-col-width + $date-col-width; + @return calc( + 100vw - #{$ui-width-px + $list-items-width-px} - #{$list-items-width-em} + ); +} + @mixin has-breadcrumbs { // padding-top: 34px; display: block; @@ -120,12 +131,11 @@ $linear-gradient-background: linear-gradient(90deg, #131b4a 0%, #364493 100%); @include after($desktop) { padding-right: 0.5rem; flex-grow: 0; - $ui-width-px: $left-menu-width + $sidebar-width - (4 * $grid-unit); - $list-items-width-em: $file-list-type-col-width + $file-list-date-col-width; - $list-items-width-px: $scrollbar-margin + $file-list-row-height + $grid-unit; - flex-basis: calc( - 100vw - #{$ui-width-px + $list-items-width-px} - #{$list-items-width-em} - ); + flex-basis: file-list-name-basis($file-list-date-col-width); + } + + @include after($desktop-wide) { + flex-basis: file-list-name-basis($file-list-date-col-width-wide); } } @@ -150,6 +160,12 @@ $linear-gradient-background: linear-gradient(90deg, #131b4a 0%, #364493 100%); @mixin file-list-col-date { width: $file-list-date-col-width; flex: 0 0 $file-list-date-col-width; + padding-right: $grid-unit * 0.25; + + @include after($desktop-wide) { + width: $file-list-date-col-width-wide; + flex-basis: $file-list-date-col-width-wide; + } @include until($desktop) { display: none; From a987a00a6b929e2c9dc5f360b875ebe36125d7a5 Mon Sep 17 00:00:00 2001 From: aasandei-vsp Date: Wed, 16 Sep 2026 10:59:08 +0300 Subject: [PATCH 12/13] Integrate the new formatted edtf date into the file list component Now the file list component shows the edtf date using the display edtf date component and dropped the prDate pipe, as it could not support the formatting of the edtf date and the newly added behaviour. The prDate pipe has not been removed, because it is used in other places for formatting plain dates and times (like the creation date or the upload date). Issue: PER-10655 --- .../file-list-item.component.html | 50 ++++++---- .../file-list-item.component.spec.ts | 91 +++++++++++++++---- .../file-list-item.component.ts | 26 ++++-- src/styles/_fileList.scss | 8 +- 4 files changed, 131 insertions(+), 44 deletions(-) diff --git a/src/app/file-browser/components/file-list-item/file-list-item.component.html b/src/app/file-browser/components/file-list-item/file-list-item.component.html index 4e0d8e311..90ace9253 100644 --- a/src/app/file-browser/components/file-list-item/file-list-item.component.html +++ b/src/app/file-browser/components/file-list-item/file-list-item.component.html @@ -89,14 +89,21 @@
@if (!showAccess || !item.ShareArchiveVO) { - - {{ startDisplayTime | prDate: item.TimezoneVO : 'date' }} - @if (item.dataStatus > 0) { - {{ - startDisplayTime | prDate: item.TimezoneVO : 'time' - }} - } - + @if (showEdtfDate) { + + } @else { + + {{ startDisplayTime | prDate: item.TimezoneVO : 'date' }} + @if (item.dataStatus > 0) { + {{ + startDisplayTime | prDate: item.TimezoneVO : 'time' + }} + } + + } } @if (showAccess || item.ShareArchiveVO) { Shared by The {{ item.ShareArchiveVO?.fullName }} Archive @@ -110,15 +117,24 @@ } @if (!showAccess) {
- @if (item.dataStatus > 0) { -
- {{ startDisplayTime | prDate: item.TimezoneVO : 'date' }} -
- } - @if (item.dataStatus > 0) { -
- {{ startDisplayTime | prDate: item.TimezoneVO : 'time' }} -
+ @if (showEdtfDate) { + @if (item.dataStatus > 0) { + + } + } @else { + @if (item.dataStatus > 0) { +
+ {{ startDisplayTime | prDate: item.TimezoneVO : 'date' }} +
+ } + @if (item.dataStatus > 0) { +
+ {{ startDisplayTime | prDate: item.TimezoneVO : 'time' }} +
+ } }
} diff --git a/src/app/file-browser/components/file-list-item/file-list-item.component.spec.ts b/src/app/file-browser/components/file-list-item/file-list-item.component.spec.ts index da9e3f0f6..507ea56e1 100644 --- a/src/app/file-browser/components/file-list-item/file-list-item.component.spec.ts +++ b/src/app/file-browser/components/file-list-item/file-list-item.component.spec.ts @@ -15,6 +15,7 @@ import { EditService } from '@core/services/edit/edit.service'; import { DeviceService } from '@shared/services/device/device.service'; import { provideNoopAnimations } from '@angular/platform-browser/animations'; import { GetThumbnailPipe } from '@shared/pipes/get-thumbnail.pipe'; +import { EdtfDateDisplayComponent } from '@shared/components/edtf-date-display/edtf-date-display.component'; import { FileListItemComponent } from './file-list-item.component'; @Pipe({ name: 'itemTypeIcon' }) @@ -38,6 +39,21 @@ export class MockPrConstantsPipe implements PipeTransform { } } +const buildTestItem = (): any => + ({ + displayDT: new Date().toISOString(), + displayName: 'Test Item', + archiveNbr: '123', + folder_linkId: '456', + type: '', + isFolder: false, + isRecord: false, + dataStatus: 0, + isFetching: false, + update: jasmine.createSpy(), + fetched: Promise.resolve(true), + }) as any; + describe('FileListItemComponent', () => { let component: FileListItemComponent; let fixture: ComponentFixture; @@ -78,7 +94,12 @@ describe('FileListItemComponent', () => { mockFeatureFlagService.isEnabled.and.returnValue(false); await TestBed.configureTestingModule({ - imports: [MockItemTypeIconPipe, MockPrDatePipe, MockPrConstantsPipe], + imports: [ + MockItemTypeIconPipe, + MockPrDatePipe, + MockPrConstantsPipe, + EdtfDateDisplayComponent, + ], declarations: [FileListItemComponent, GetThumbnailPipe], providers: [ provideNoopAnimations(), @@ -150,19 +171,7 @@ describe('FileListItemComponent', () => { component = fixture.componentInstance; editService = TestBed.inject(EditService); - component.item = { - displayDT: new Date().toISOString(), - displayName: 'Test Item', - archiveNbr: '123', - folder_linkId: '456', - type: '', - isFolder: false, - isRecord: false, - dataStatus: 0, - isFetching: false, - update: jasmine.createSpy(), - fetched: Promise.resolve(true), - } as any; + component.item = buildTestItem(); component.folderView = '' as any; fixture.detectChanges(); @@ -581,12 +590,17 @@ describe('FileListItemComponent', () => { mockFeatureFlagService.isEnabled.and.callFake( (flag: string) => flag === 'edtf-date', ); + // The flag is read in the constructor, so the fixture has to be + // built again for the new value to take. + fixture = TestBed.createComponent(FileListItemComponent); + component = fixture.componentInstance; + component.item = buildTestItem(); + component.folderView = '' as any; }); it('should not fall back to displayDT when displayTime is missing', () => { component.item.displayTime = undefined; component.item.displayDT = '2023-01-01T00:00:00.000Z'; - fixture.detectChanges(); expect(component.startDisplayTime).toBe(''); }); @@ -594,7 +608,6 @@ describe('FileListItemComponent', () => { it('should show nothing when displayTime was explicitly cleared', () => { component.item.displayTime = null; component.item.displayDT = '2023-01-01T00:00:00.000Z'; - fixture.detectChanges(); expect(component.startDisplayTime).toBe(''); }); @@ -602,9 +615,53 @@ describe('FileListItemComponent', () => { it('should still show the displayTime start date', () => { component.item.displayTime = '2020-06-10/2026-06-15'; component.item.displayDT = '2023-01-01T00:00:00.000Z'; - fixture.detectChanges(); expect(component.startDisplayTime).toBe('2020-06-10'); }); + + it('should render the public-archive date from the EDTF value', async () => { + component.item.displayTime = '1985-04'; + component.item.displayDT = '2023-01-01T00:00:00.000Z'; + + await component.ngOnInit(); + + expect(component.date).toBe('April 1985'); + }); + + it('should leave the public-archive date empty when there is no EDTF value', async () => { + component.item.displayTime = undefined; + component.item.displayDT = '2023-01-01T00:00:00.000Z'; + + await component.ngOnInit(); + + expect(component.date).toBe(''); + }); + + it('should never put an unreadable value in the public-archive date', async () => { + component.item.displayTime = 'XXXX-XX-XX'; + + await component.ngOnInit(); + + expect(component.date).toBe('Unknown'); + expect(component.date).not.toContain('Invalid'); + expect(component.date).not.toContain('NaN'); + }); + }); + + describe('with the edtf-date feature flag disabled', () => { + it('should keep falling back to displayDT', () => { + component.item.displayTime = undefined; + component.item.displayDT = '2023-01-01T00:00:00.000Z'; + + expect(component.startDisplayTime).toBe('2023-01-01T00:00:00.000Z'); + }); + + it('should not print Invalid Date for an EDTF value it cannot read', async () => { + component.item.displayTime = '198X'; + + await component.ngOnInit(); + + expect(component.date).toBe(''); + }); }); }); diff --git a/src/app/file-browser/components/file-list-item/file-list-item.component.ts b/src/app/file-browser/components/file-list-item/file-list-item.component.ts index 376bf65a5..90f750201 100644 --- a/src/app/file-browser/components/file-list-item/file-list-item.component.ts +++ b/src/app/file-browser/components/file-list-item/file-list-item.component.ts @@ -34,6 +34,7 @@ import { import { DataStatus } from '@models/data-status.enum'; import { EditService } from '@core/services/edit/edit.service'; import { EdtfService } from '@shared/services/edtf-service/edtf.service'; +import { EdtfDisplayService } from '@shared/services/edtf-service/edtf-display.service'; import { FeatureFlagService } from '@root/app/feature-flag/services/feature-flag.service'; import { RecordResponse, @@ -206,6 +207,7 @@ export class FileListItemComponent public canEdit = true; public isZip = false; public date: string = ''; + public showEdtfDate = false; public isUnlistedShare = false; public recordThumbnailUrl: string | undefined; @@ -251,29 +253,39 @@ export class FileListItemComponent @Inject(DOCUMENT) private document: Document, private shareLinksService: ShareLinksService, private edtfService: EdtfService, + private edtfDisplayService: EdtfDisplayService, private featureFlagService: FeatureFlagService, - ) {} + ) { + this.showEdtfDate = this.featureFlagService.isEnabled('edtf-date'); + } get startDisplayTime(): string { const edtfStartDate = this.edtfService.getEdtfIntervalStartDate( this.item.displayTime, ); - // Once the edtf-date UI ships, displayTime is authoritative (a null - // value means the user cleared the date, so nothing is shown). Until - // then, items may only have displayDT populated, so keep the fallback. - if (this.featureFlagService.isEnabled('edtf-date')) { + if (this.showEdtfDate) { return edtfStartDate; } return edtfStartDate || this.item.displayDT; } + private getPublicArchiveDate(): string { + if (this.showEdtfDate) { + return this.edtfDisplayService.formatToPlainText(this.item.displayTime); + } + + const legacyDate = new Date(this.startDisplayTime); + return Number.isNaN(legacyDate.getTime()) + ? '' + : getFormattedDate(legacyDate); + } + async ngOnInit() { this.isInSharePreview = this.router.routerState.snapshot.url.includes('/share/'); - const date = new Date(this.startDisplayTime); - this.date = getFormattedDate(date); + this.date = this.getPublicArchiveDate(); // Only a share preview can be an unlisted share: the token that decides it // is set by SharePreviewComponent and cleared when it is destroyed, so diff --git a/src/styles/_fileList.scss b/src/styles/_fileList.scss index e54041c57..edfea2a62 100644 --- a/src/styles/_fileList.scss +++ b/src/styles/_fileList.scss @@ -7,7 +7,11 @@ pr-shares { display: block; @include after($tablet-horizontal) { &.show-sidebar { - width: calc(100% - #{$sidebar-width}); + // The sidebar is fixed to the viewport edge, so it sits outside + // .main-content's right padding. Reaching back across that padding is + // what stops the rows ending short of it. + width: calc(100% - #{$sidebar-width} + #{$grid-unit}); + margin-right: -$grid-unit; height: calc(100vh - #{$navbar-total-height-desktop}); pr-sidebar { @@ -25,8 +29,6 @@ pr-shares { max-height: calc( 100vh - #{$navbar-total-height-desktop + $file-list-controls-height} ); - margin-right: -$grid-unit; - padding-right: $grid-unit; padding-bottom: $file-list-row-height * 1; } } From 600dd7bc7ace41bf02100ab2029249c949b319a9 Mon Sep 17 00:00:00 2001 From: aasandei-vsp Date: Fri, 18 Sep 2026 11:26:32 +0300 Subject: [PATCH 13/13] Fix month input for january in the display service When the user would input 1 for the month, this is considered January, which is correct. The display service would actually buffer that date to 1X, adding unspecified digits, which is incorrect. Issue: PER-10655 --- .../services/edtf-service/edtf-display.service.spec.ts | 7 +------ .../services/edtf-service/edtf-display.service.ts | 10 +++++----- src/app/shared/services/edtf-service/edtf.service.ts | 2 +- 3 files changed, 7 insertions(+), 12 deletions(-) diff --git a/src/app/shared/services/edtf-service/edtf-display.service.spec.ts b/src/app/shared/services/edtf-service/edtf-display.service.spec.ts index 3ec5409df..ea266d700 100644 --- a/src/app/shared/services/edtf-service/edtf-display.service.spec.ts +++ b/src/app/shared/services/edtf-service/edtf-display.service.spec.ts @@ -49,6 +49,7 @@ describe('EdtfDisplayService', () => { [7, '156X-12-25', 'Dec. 25, 156X', ''], [8, '1900-01-01', 'Jan. 1, 1900', ''], [9, '1964/2008', '1964 — 2008', ''], + [10, '198X?', '198X ?', ''], [11, '1984%', '1984 %', ''], [12, '1984?', '1984 ?', ''], [13, '1984~', '1984 ~', ''], @@ -191,12 +192,6 @@ describe('EdtfDisplayService', () => { expect(dateTextOf('not-a-date')).toBe('not-a-date'); }); - it('should show a qualifier on unspecified digits verbatim', () => { - // Not a supported value: the edtf grammar rejects a qualifier sitting - // on X digits, so it falls through to the stored string. - expect(dateTextOf('198X?')).toBe('198X?'); - }); - it('should render nothing for an absent value', () => { const nothing = { date: [], dateEnd: [], time: [] }; diff --git a/src/app/shared/services/edtf-service/edtf-display.service.ts b/src/app/shared/services/edtf-service/edtf-display.service.ts index 22a751357..85f38a2ac 100644 --- a/src/app/shared/services/edtf-service/edtf-display.service.ts +++ b/src/app/shared/services/edtf-service/edtf-display.service.ts @@ -118,7 +118,10 @@ export class EdtfDisplayService { } const yearDisplay = this.edtfService.padWithX(yearValue, 4); - const monthIndex = this.toMonthIndex(monthValue); + const monthDisplay = hasMonth + ? this.edtfService.padMonthOrDay(monthValue) + : 'XX'; + const monthIndex = this.toMonthIndex(monthDisplay); if (monthIndex !== NO_MONTH_INDEX && hasDay) { const monthName = options.abbreviateMonthWithDay @@ -138,10 +141,7 @@ export class EdtfDisplayService { return yearDisplay; } - const parts = [ - yearDisplay, - hasMonth ? this.edtfService.padWithX(monthValue, 2) : 'XX', - ]; + const parts = [yearDisplay, monthDisplay]; if (hasDay) { parts.push(dayValue.padStart(2, '0')); } diff --git a/src/app/shared/services/edtf-service/edtf.service.ts b/src/app/shared/services/edtf-service/edtf.service.ts index 7236d1109..6cd3c8d7e 100644 --- a/src/app/shared/services/edtf-service/edtf.service.ts +++ b/src/app/shared/services/edtf-service/edtf.service.ts @@ -362,7 +362,7 @@ export class EdtfService { // Shared by serialization (toEdtfDate) and display (formatDateForDisplay) // so a saved value always reads back the way the preview rendered it. - private padMonthOrDay(value: string): string { + padMonthOrDay(value: string): string { // A single digit is zero-padded ('1' → '01', i.e. January / the 1st). // '1' could in principle be the start of '10'–'12', but this runs on a // finished value, not mid-keystroke, and the digits-only inputs give no