From 3eb85d243e6f97239e724b34a26ae2367f533c89 Mon Sep 17 00:00:00 2001 From: aasandei-vsp Date: Fri, 21 Aug 2026 13:19:20 +0300 Subject: [PATCH 1/3] Add missing fields to stela objects and extra lookout for folder In order for the migration of the timeline to be complete, we needed to map some extra fields to stela: folderLinkIds, archiveNumbers, accessRole. And for deep links, we needed to be able to get folders using the properties mentioned above, so we needed an extra call to the old API, because if the user is deep in a folder and uses the browser refresh, the only way to identity the folder you were in, would be using the archiveNbr and the folder_linkId, which is not possible using the stela endpoints. Issue: PER-10677 --- src/app/models/access-role.spec.ts | 69 +++++++ src/app/models/access-role.ts | 55 ++++++ .../shared/services/api/folder.repo.spec.ts | 169 +++++++++++++++++- src/app/shared/services/api/folder.repo.ts | 37 +++- .../shared/services/api/record.repo.spec.ts | 60 +++++++ src/app/shared/services/api/record.repo.ts | 20 ++- 6 files changed, 402 insertions(+), 8 deletions(-) create mode 100644 src/app/models/access-role.spec.ts diff --git a/src/app/models/access-role.spec.ts b/src/app/models/access-role.spec.ts new file mode 100644 index 000000000..107a7311c --- /dev/null +++ b/src/app/models/access-role.spec.ts @@ -0,0 +1,69 @@ +import { + ARCHIVE_MEMBERSHIP_ROLE_TO_ACCESS_ROLE, + getAccessRoleFromArchiveMembershipRole, + getAccessAsEnum, + type ArchiveMembershipRoleType, +} from './access-role'; + +describe('getAccessRoleFromArchiveMembershipRole', () => { + const expectedTranslations: Array<[ArchiveMembershipRoleType, string]> = [ + ['contributor', 'access.role.contributor'], + ['curator', 'access.role.curator'], + ['editor', 'access.role.editor'], + ['manager', 'access.role.manager'], + ['owner', 'access.role.owner'], + ['viewer', 'access.role.viewer'], + ]; + + expectedTranslations.forEach( + ([archiveMembershipRole, expectedAccessRole]) => { + it(`should translate ${archiveMembershipRole}`, () => { + expect( + getAccessRoleFromArchiveMembershipRole(archiveMembershipRole), + ).toBe(expectedAccessRole); + }); + }, + ); + + it('should cover every role Stela can send', () => { + expect(Object.keys(ARCHIVE_MEMBERSHIP_ROLE_TO_ACCESS_ROLE).length).toBe( + expectedTranslations.length, + ); + }); + + it('should produce roles that getAccessAsEnum understands', () => { + expectedTranslations.forEach(([archiveMembershipRole]) => { + const accessRole = getAccessRoleFromArchiveMembershipRole( + archiveMembershipRole, + ); + + expect(getAccessAsEnum(accessRole)).toBeDefined(); + }); + }); + + it('should translate manager to manager, not the curator that PERMISSIONS_LEVEL_TO_ACCESS_ROLE maps it to', () => { + expect(getAccessRoleFromArchiveMembershipRole('manager')).toBe( + 'access.role.manager', + ); + }); + + it('should return undefined rather than guess at an unknown role', () => { + expect( + getAccessRoleFromArchiveMembershipRole( + 'wizard' as ArchiveMembershipRoleType, + ), + ).toBeUndefined(); + }); + + it('should return undefined when no role is given', () => { + expect(getAccessRoleFromArchiveMembershipRole(undefined)).toBeUndefined(); + }); + + it('should return undefined for an inherited object property name', () => { + expect( + getAccessRoleFromArchiveMembershipRole( + 'toString' as ArchiveMembershipRoleType, + ), + ).toBeUndefined(); + }); +}); diff --git a/src/app/models/access-role.ts b/src/app/models/access-role.ts index 4f06f63b0..1e68dac0b 100644 --- a/src/app/models/access-role.ts +++ b/src/app/models/access-role.ts @@ -22,6 +22,61 @@ export type PermissionsLevel = | 'owner' | 'viewer'; +/** Mirrors Stela's ArchiveMembershipRole enum */ +export type ArchiveMembershipRoleType = + | 'contributor' + | 'curator' + | 'editor' + | 'manager' + | 'owner' + | 'viewer'; + +export const ARCHIVE_MEMBERSHIP_ROLE_TO_ACCESS_ROLE: Record< + ArchiveMembershipRoleType, + AccessRoleType +> = { + contributor: 'access.role.contributor', + curator: 'access.role.curator', + editor: 'access.role.editor', + manager: 'access.role.manager', + owner: 'access.role.owner', + viewer: 'access.role.viewer', +}; + +function isArchiveMembershipRole( + value: unknown, +): value is ArchiveMembershipRoleType { + return ( + typeof value === 'string' && + Object.hasOwn(ARCHIVE_MEMBERSHIP_ROLE_TO_ACCESS_ROLE, value) + ); +} + +export function getAccessRoleFromArchiveMembershipRole( + archiveMembershipRole: ArchiveMembershipRoleType | undefined, +): AccessRoleType | undefined { + if (!isArchiveMembershipRole(archiveMembershipRole)) { + return undefined; + } + + return ARCHIVE_MEMBERSHIP_ROLE_TO_ACCESS_ROLE[archiveMembershipRole]; +} + +/** + * Spread into VO data so that a role Stela did not send -- or one we cannot + * translate -- leaves no accessRole field behind at all, letting permission + * checks keep using the role the v1 endpoints supplied. + */ +export function getOptionalAccessRoleField( + archiveMembershipRole: ArchiveMembershipRoleType | undefined, +): { accessRole?: AccessRoleType } { + const accessRole = getAccessRoleFromArchiveMembershipRole( + archiveMembershipRole, + ); + + return accessRole ? { accessRole } : {}; +} + // Mapping for share link permissions. Note the stela share link API // mistakenly returns "manager" where it should use "curator" -- see // https://github.com/PermanentOrg/stela/issues/540 diff --git a/src/app/shared/services/api/folder.repo.spec.ts b/src/app/shared/services/api/folder.repo.spec.ts index e97fe0412..1fc8a8045 100644 --- a/src/app/shared/services/api/folder.repo.spec.ts +++ b/src/app/shared/services/api/folder.repo.spec.ts @@ -4,7 +4,7 @@ import { of } from 'rxjs'; import { ShareLink } from '@root/app/share-links/models/share-link'; import { HttpV2Service } from '../http-v2/http-v2.service'; import { HttpService } from '../http/http.service'; -import { FolderRepo } from './folder.repo'; +import { FolderRepo, FolderResponse } from './folder.repo'; const emptyResponse = { items: [] }; const fakeFolderResponse = { @@ -29,7 +29,12 @@ const mockStelaFolder = { displayName: 'Test Folder', downloadName: 'test-folder', imageRatio: 1.5, - paths: { names: ['path1', 'path2'] }, + paths: { + names: ['path1', 'path2'], + folderLinkIds: ['11', '22'], + archiveNumbers: ['0001-0000', '0002-0000'], + }, + accessRole: 'owner', publicAt: null, sort: 'name', thumbnailUrls: { @@ -516,5 +521,165 @@ describe('Folder repo', () => { expect(folder.folder_linkId).toBeUndefined(); }); + + it('should map the breadcrumb archive numbers', async () => { + const folder = await convertFolder({ + paths: { + names: ['My Files', 'Photos'], + folderLinkIds: ['11', '22'], + archiveNumbers: ['0001-0000', '0002-0000'], + }, + }); + + expect(folder.pathAsArchiveNbr).toEqual(['0001-0000', '0002-0000']); + }); + + it('should map the breadcrumb link ids as numbers', async () => { + const folder = await convertFolder({ + paths: { + names: ['My Files', 'Photos'], + folderLinkIds: ['11', '22'], + archiveNumbers: ['0001-0000', '0002-0000'], + }, + }); + + expect(folder.pathAsFolder_linkId).toEqual([11, 22]); + }); + }); + + describe('getWithChildrenByIdentifier', () => { + it('should go straight to Stela when the folder already has an id', async () => { + httpV2Spy.get.and.returnValues( + of([{ items: [mockStelaFolder] }]), + of([{ items: [] }]), + ); + + await folderRepo.getWithChildrenByIdentifier( + new FolderVO({ folderId: 123 }), + ); + + expect(httpSpy.sendRequestPromise).not.toHaveBeenCalled(); + expect(httpV2Spy.get).toHaveBeenCalled(); + }); + + it('should resolve the id through v1 when the folder has none', async () => { + httpSpy.sendRequestPromise.and.resolveTo( + new FolderResponse({ + isSuccessful: true, + Results: [{ data: [{ FolderVO: { folderId: '123' } }] }], + }), + ); + httpV2Spy.get.and.returnValues( + of([{ items: [mockStelaFolder] }]), + of([{ items: [] }]), + ); + + const result = await folderRepo.getWithChildrenByIdentifier( + new FolderVO({ archiveNbr: '0001-0002', folder_linkId: 55 }), + ); + + expect(httpSpy.sendRequestPromise).toHaveBeenCalled(); + expect(httpV2Spy.get).toHaveBeenCalledWith('v2/folder', { + folderIds: ['123'], + }); + + expect(result.isSuccessful).toBeTrue(); + }); + + it('should throw the v1 response when the id cannot be resolved', async () => { + httpSpy.sendRequestPromise.and.resolveTo( + new FolderResponse({ isSuccessful: false }), + ); + + await expectAsync( + folderRepo.getWithChildrenByIdentifier( + new FolderVO({ archiveNbr: '0001-0002', folder_linkId: 55 }), + ), + ).toBeRejected(); + }); + }); + + describe('access role translation', () => { + const convertFolder = async (overrides: Record) => { + httpV2Spy.get.and.returnValue( + of([{ items: [{ ...mockStelaFolder, ...overrides }] }]), + ); + const result = await folderRepo.getStelaFolderVOs([ + new FolderVO({ folderId: 123 }), + ]); + return result.getFolderVOs()[0]; + }; + + it("should translate Stela's role into ours", async () => { + const folder = await convertFolder({ accessRole: 'owner' }); + + expect(folder.accessRole).toBe('access.role.owner'); + }); + + it('should translate manager to manager, not curator', async () => { + const folder = await convertFolder({ accessRole: 'manager' }); + + expect(folder.accessRole).toBe('access.role.manager'); + }); + + it('should add no role at all when Stela sends nothing', async () => { + const folder = await convertFolder({ accessRole: undefined }); + + expect(Object.hasOwn(folder, 'accessRole')).toBeFalse(); + }); + + it('should add no role at all when Stela sends null', async () => { + const folder = await convertFolder({ accessRole: null }); + + expect(Object.hasOwn(folder, 'accessRole')).toBeFalse(); + }); + + it('should add no role at all when Stela sends one we cannot translate', async () => { + const folder = await convertFolder({ accessRole: 'archivist' }); + + expect(Object.hasOwn(folder, 'accessRole')).toBeFalse(); + }); + + it('should merge onto an existing folder without breaking its role', async () => { + const existingFolder = new FolderVO({ + folderId: '123', + accessRole: 'access.role.owner', + }); + + existingFolder.update(await convertFolder({ accessRole: 'owner' })); + + expect(existingFolder.accessRole).toBe('access.role.owner'); + }); + + it('should leave an existing role alone when Stela sends none', async () => { + const existingFolder = new FolderVO({ + folderId: '123', + accessRole: 'access.role.owner', + }); + + existingFolder.update(await convertFolder({ accessRole: undefined })); + + expect(existingFolder.accessRole).toBe('access.role.owner'); + }); + + it('should translate the role on child folders too', async () => { + httpV2Spy.get.and.returnValues( + of([{ items: [mockStelaFolder] }]), + of([ + { + items: [ + { ...mockStelaFolder, folderId: '999', accessRole: 'viewer' }, + ], + }, + ]), + ); + + const result = await folderRepo.getWithChildren([ + new FolderVO({ folderId: 123 }), + ]); + const child = result.getFolderVO(true).ChildItemVOs[0]; + + expect(child.accessRole).toBe('access.role.viewer'); + }); }); }); diff --git a/src/app/shared/services/api/folder.repo.ts b/src/app/shared/services/api/folder.repo.ts index e2ecf9a53..667eff963 100644 --- a/src/app/shared/services/api/folder.repo.ts +++ b/src/app/shared/services/api/folder.repo.ts @@ -2,6 +2,10 @@ import { FolderVO, FolderVOData, ItemVO } from '@root/app/models'; import { BaseResponse, BaseRepo } from '@shared/services/api/base'; import { firstValueFrom, Observable } from 'rxjs'; import { DataStatus } from '@models/data-status.enum'; +import { + getOptionalAccessRoleField, + type ArchiveMembershipRoleType, +} from '@models/access-role'; import { ShareLink } from '@root/app/share-links/models/share-link'; import { convertStelaLocationToLocnVOData, @@ -70,7 +74,10 @@ interface StelaFolder { imageRatio: number; paths: { names: string[]; + folderLinkIds: string[]; + archiveNumbers: Array; }; + accessRole?: ArchiveMembershipRoleType; publicAt: string; sort: string; thumbnailUrls?: { @@ -123,8 +130,10 @@ const convertStelaFolderToFolderVO = (stelaFolder: StelaFolder): FolderVO => { const childRecordVOs = stelaFolder.children .filter(isStelaRecord) .map(convertStelaRecordToRecordVO); + const { accessRole: stelaAccessRole, ...stelaFolderWithoutAccessRole } = + stelaFolder; return new FolderVO({ - ...stelaFolder, + ...stelaFolderWithoutAccessRole, folderId: stelaFolder.folderId, archiveId: stelaFolder.archive?.id, archiveNbr: stelaFolder.archiveNumber, @@ -151,6 +160,7 @@ const convertStelaFolderToFolderVO = (stelaFolder: StelaFolder): FolderVO => { view: stelaFolder.view, imageRatio: stelaFolder.imageRatio, type: stelaFolder.type, + ...getOptionalAccessRoleField(stelaAccessRole), thumbStatus: stelaFolder.status, thumbURL200: stelaFolder.thumbnailUrls?.['200'], thumbURL500: stelaFolder.thumbnailUrls?.['500'], @@ -163,6 +173,10 @@ const convertStelaFolderToFolderVO = (stelaFolder: StelaFolder): FolderVO => { publicDT: stelaFolder.publicAt, parentFolderId: stelaFolder.parentFolder?.id, pathAsText: stelaFolder.paths?.names, + pathAsArchiveNbr: stelaFolder.paths?.archiveNumbers, + pathAsFolder_linkId: stelaFolder.paths?.folderLinkIds?.map((folderLinkId) => + toFolderLinkId(folderLinkId), + ), ParentFolderVOs: [new FolderVO({ folderId: stelaFolder.parentFolder?.id })], ChildFolderVOs: childFolderVOs, RecordVOs: childRecordVOs, @@ -390,6 +404,27 @@ export class FolderRepo extends BaseRepo { return folderResponse; } + /** + * Stela can only look a folder up by numeric folderId, but our routes and + * breadcrumbs address folders by archiveNbr + folder_linkId, so the id is + * resolved through the v1 endpoint first. Separate from getWithChildren + * because that v1 lookup needs an auth token a share-token visitor lacks. + */ + public async getWithChildrenByIdentifier( + folderVO: FolderVO, + ): Promise { + if (folderVO.folderId) { + return await this.getWithChildren([folderVO]); + } + + const identityResponse = await this.get([folderVO]); + if (!identityResponse.isSuccessful) { + throw identityResponse; + } + + return await this.getWithChildren([identityResponse.getFolderVO()]); + } + public navigateLean(folderVO: FolderVO): Observable { const data = [ { diff --git a/src/app/shared/services/api/record.repo.spec.ts b/src/app/shared/services/api/record.repo.spec.ts index c0403ec8e..3cd8feceb 100644 --- a/src/app/shared/services/api/record.repo.spec.ts +++ b/src/app/shared/services/api/record.repo.spec.ts @@ -466,5 +466,65 @@ describe('RecordRepo', () => { expect(record.thumbURL2000).toBe('https://example.com/2000'); expect(record.thumbnail256).toBe('https://example.com/256'); }); + + it("should translate Stela's role into ours", () => { + const record = convertStelaRecordToRecordVO({ + ...baseStelaRecord, + accessRole: 'viewer', + } as any); + + expect(record.accessRole).toBe('access.role.viewer'); + }); + + it('should translate manager to manager, not curator', () => { + const record = convertStelaRecordToRecordVO({ + ...baseStelaRecord, + accessRole: 'manager', + } as any); + + expect(record.accessRole).toBe('access.role.manager'); + }); + + it('should add no role at all when Stela sends nothing', () => { + const record = convertStelaRecordToRecordVO({ + ...baseStelaRecord, + } as any); + + expect(Object.hasOwn(record, 'accessRole')).toBeFalse(); + }); + + it('should add no role at all when Stela sends null', () => { + const record = convertStelaRecordToRecordVO({ + ...baseStelaRecord, + accessRole: null, + } as any); + + expect(Object.hasOwn(record, 'accessRole')).toBeFalse(); + }); + + it('should add no role at all when Stela sends one we cannot translate', () => { + const record = convertStelaRecordToRecordVO({ + ...baseStelaRecord, + accessRole: 'archivist', + } as any); + + expect(Object.hasOwn(record, 'accessRole')).toBeFalse(); + }); + + it('should merge onto an existing record without breaking its role', () => { + const existingRecord = new RecordVO({ + recordId: 42, + accessRole: 'access.role.curator', + }); + + existingRecord.update( + convertStelaRecordToRecordVO({ + ...baseStelaRecord, + accessRole: 'owner', + } as any), + ); + + expect(existingRecord.accessRole).toBe('access.role.owner'); + }); }); }); diff --git a/src/app/shared/services/api/record.repo.ts b/src/app/shared/services/api/record.repo.ts index b49354431..5bd28a213 100644 --- a/src/app/shared/services/api/record.repo.ts +++ b/src/app/shared/services/api/record.repo.ts @@ -18,7 +18,11 @@ import { ThumbnailCache } from '@shared/utilities/thumbnail-cache/thumbnail-cach import { firstValueFrom } from 'rxjs'; import { FileFormat, PermanentFile } from '@models/file-vo'; import { ShareStatus } from '@models/share-vo'; -import { AccessRoleType } from '@models/access-role'; +import { + AccessRoleType, + getOptionalAccessRoleField, + type ArchiveMembershipRoleType, +} from '@models/access-role'; import { ShareLink } from '@root/app/share-links/models/share-link'; import { getFirst } from '../http-v2/http-v2.service'; import { CENTRAL_TIMEZONE_VO } from './folder.repo'; @@ -106,8 +110,9 @@ export interface StelaShare { thumbURL200: string; }; } -export type StelaRecord = Omit & { +export type StelaRecord = Omit & { tags: Array | null; + accessRole?: ArchiveMembershipRoleType; archiveNumber: string; displayDate: string; displayTime?: string; @@ -199,9 +204,12 @@ export const convertStelaLocationToLocnVOData = ( export const convertStelaRecordToRecordVO = ( stelaRecord: StelaRecord, -): RecordVO => - new RecordVO({ - ...stelaRecord, +): RecordVO => { + const { accessRole: stelaAccessRole, ...stelaRecordWithoutAccessRole } = + stelaRecord; + + return new RecordVO({ + ...stelaRecordWithoutAccessRole, thumbURL200: stelaRecord.thumbnailUrls?.['200'] ?? stelaRecord.thumbURL200, thumbURL500: stelaRecord.thumbnailUrls?.['500'] ?? stelaRecord.thumbURL500, thumbURL1000: @@ -214,6 +222,7 @@ export const convertStelaRecordToRecordVO = ( convertStelaTagToTagVO(stelaTag, stelaRecord.archiveId), ), archiveNbr: stelaRecord.archiveNumber, + ...getOptionalAccessRoleField(stelaAccessRole), displayDT: stelaRecord.displayDate, displayTime: stelaRecord.displayTime, folder_linkId: Number.parseInt(stelaRecord.folderLinkId, 10), @@ -230,6 +239,7 @@ export const convertStelaRecordToRecordVO = ( TimezoneVO: CENTRAL_TIMEZONE_VO, ShareVOs: (stelaRecord.shares ?? []).map(convertStelaSharetoShareVO), }); +}; export class RecordRepo extends BaseRepo { private async getRecordIdByArchiveNbr(archiveNbr: string): Promise { From b317c4a1b07c64cdcb1455c5d67ca0d522d7a495 Mon Sep 17 00:00:00 2001 From: aasandei-vsp Date: Fri, 21 Aug 2026 13:29:28 +0300 Subject: [PATCH 2/3] Update the lean folder response with getWithChildren and handle error Lean folder response uses now getWithChildren instead of navigateLean and returns consistently a promise, no observable. Stela rejects with a raw http error, which is different from the old backend, so a specific error message handler was needed. Issue: PER-10677 --- .../lean-folder-resolve.service.spec.ts | 211 ++++++++++++++++++ .../resolves/lean-folder-resolve.service.ts | 69 +++--- .../shared/utilities/folder-error-message.ts | 15 ++ 3 files changed, 258 insertions(+), 37 deletions(-) create mode 100644 src/app/core/resolves/lean-folder-resolve.service.spec.ts create mode 100644 src/app/shared/utilities/folder-error-message.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 new file mode 100644 index 000000000..726a795a9 --- /dev/null +++ b/src/app/core/resolves/lean-folder-resolve.service.spec.ts @@ -0,0 +1,211 @@ +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' as any); + 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 index 72bbb904a..38c5e0f68 100644 --- a/src/app/core/resolves/lean-folder-resolve.service.ts +++ b/src/app/core/resolves/lean-folder-resolve.service.ts @@ -4,14 +4,12 @@ import { RouterStateSnapshot, Router, } from '@angular/router'; -import { Observable } from 'rxjs'; -import { map } from 'rxjs/operators'; 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 { FolderResponse } from '@shared/services/api/index.repo'; +import { getFolderErrorMessage } from '@shared/utilities/folder-error-message'; import { FolderVO } from '@root/app/models'; @@ -24,10 +22,10 @@ export class LeanFolderResolveService { private router: Router, ) {} - resolve( + async resolve( route: ActivatedRouteSnapshot, state: RouterStateSnapshot, - ): Observable | Promise { + ): Promise { let targetFolder; if (route.params.archiveNbr && route.params.folderLinkId) { @@ -51,7 +49,7 @@ export class LeanFolderResolveService { folder.pathAsText.unshift('Shares', 'Record'); folder.pathAsFolder_linkId.unshift(0, 0); folder.ChildItemVOs = [sharedRecord]; - return Promise.resolve(folder); + return folder; } } else { const myFiles = find(this.accountService.getRootFolder().ChildItemVOs, { @@ -60,38 +58,35 @@ export class LeanFolderResolveService { targetFolder = new FolderVO(myFiles); } - return this.api.folder - .navigateLean(targetFolder) - .pipe( - map((response: FolderResponse) => { - if (!response.isSuccessful) { - throw response; - } + try { + const folderResponse = + await this.api.folder.getWithChildrenByIdentifier(targetFolder); - return response.getFolderVO(true); - }), - ) - .toPromise() - .catch(async (response: FolderResponse) => { - this.message.showError({ - message: response.getMessage(), - 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); + 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/shared/utilities/folder-error-message.ts b/src/app/shared/utilities/folder-error-message.ts new file mode 100644 index 000000000..c631937f3 --- /dev/null +++ b/src/app/shared/utilities/folder-error-message.ts @@ -0,0 +1,15 @@ +import { FolderResponse } from '@shared/services/api/index.repo'; + +export const GENERIC_FOLDER_ERROR_MESSAGE = 'error.generic.internal'; + +/** + * Legacy endpoints failed with a FolderResponse carrying a translatable message, + * while Stela rejects with the raw HTTP error, which has none. + */ +export function getFolderErrorMessage(error: unknown): string { + if (error instanceof FolderResponse) { + return error.getMessage() ?? GENERIC_FOLDER_ERROR_MESSAGE; + } + + return GENERIC_FOLDER_ERROR_MESSAGE; +} From 3bae6b8694bbde603cf52e9dc1c52f577718b24d Mon Sep 17 00:00:00 2001 From: aasandei-vsp Date: Fri, 21 Aug 2026 13:41:32 +0300 Subject: [PATCH 3/3] Update the timeline navigation to getWithChildren and the unit testing The timeline folder navigation, going into a folder used the navigateLean endpoint, so we replaced it with getWithChildren. The fact that we are making a BE call from a component that otherwise does not have this kind of logic is counterintuitive, but the needed refactoring to extract this logic is out of scope for the current work and it would need testing many other places. Issue: PER-10677 --- .../timeline-breadcrumbs.component.spec.ts | 91 ++++++++--- .../timeline-view.component.spec.ts | 151 +++++++++++++++--- .../timeline-view/timeline-view.component.ts | 25 ++- 3 files changed, 212 insertions(+), 55 deletions(-) diff --git a/src/app/views/components/timeline-view/timeline-breadcrumbs/timeline-breadcrumbs.component.spec.ts b/src/app/views/components/timeline-view/timeline-breadcrumbs/timeline-breadcrumbs.component.spec.ts index 6f63e12f9..b027cac3b 100644 --- a/src/app/views/components/timeline-view/timeline-breadcrumbs/timeline-breadcrumbs.component.spec.ts +++ b/src/app/views/components/timeline-view/timeline-breadcrumbs/timeline-breadcrumbs.component.spec.ts @@ -1,25 +1,66 @@ -// import { async, ComponentFixture, TestBed } from '@angular/core/testing'; - -// import { TimelineBreadcrumbsComponent } from './timeline-breadcrumbs.component'; - -// describe('TimelineBreadcrumbsComponent', () => { -// let component: TimelineBreadcrumbsComponent; -// let fixture: ComponentFixture; - -// beforeEach(async(() => { -// TestBed.configureTestingModule({ -// declarations: [ TimelineBreadcrumbsComponent ] -// }) -// .compileComponents(); -// })); - -// beforeEach(() => { -// fixture = TestBed.createComponent(TimelineBreadcrumbsComponent); -// component = fixture.componentInstance; -// fixture.detectChanges(); -// }); - -// it('should create', () => { -// expect(component).toBeTruthy(); -// }); -// }); +import { ComponentFixture, TestBed } from '@angular/core/testing'; +import * as Testing from '@root/test/testbedConfig'; +import { cloneDeep } from 'lodash'; + +import { DataService } from '@shared/services/data/data.service'; +import { FolderVO } from '@root/app/models'; +import { TimelineBreadcrumbsComponent } from './timeline-breadcrumbs.component'; + +describe('TimelineBreadcrumbsComponent', () => { + let component: TimelineBreadcrumbsComponent; + let fixture: ComponentFixture; + let dataService: DataService; + + beforeEach(async () => { + const config = cloneDeep(Testing.BASE_TEST_CONFIG); + + config.declarations.push(TimelineBreadcrumbsComponent); + config.providers.push(DataService); + + TestBed.configureTestingModule(config).compileComponents(); + + fixture = TestBed.createComponent(TimelineBreadcrumbsComponent); + component = fixture.componentInstance; + dataService = TestBed.inject(DataService); + }); + + it('should create', () => { + expect(component).toBeTruthy(); + }); + + it('should build a crumb per path entry from a converted folder', () => { + dataService.currentFolder = new FolderVO({ + pathAsText: ['My Files', 'Photos'], + pathAsArchiveNbr: ['0001-0000', '0002-0000'], + pathAsFolder_linkId: [11, 22], + }); + + component.setFolderBreadcrumbs(); + + expect(component.breadcrumbs.length).toBe(2); + expect(component.breadcrumbs[0]).toEqual( + jasmine.objectContaining({ + type: 'folder', + text: 'My Files', + archiveNbr: '0001-0000', + folder_linkId: 11, + }), + ); + + expect(component.breadcrumbs[1]).toEqual( + jasmine.objectContaining({ + text: 'Photos', + archiveNbr: '0002-0000', + folder_linkId: 22, + }), + ); + }); + + it('should build no crumbs without a current folder', () => { + dataService.currentFolder = undefined; + + component.setFolderBreadcrumbs(); + + expect(component.breadcrumbs).toEqual([]); + }); +}); diff --git a/src/app/views/components/timeline-view/timeline-view.component.spec.ts b/src/app/views/components/timeline-view/timeline-view.component.spec.ts index 5ef60737c..82ac09efd 100644 --- a/src/app/views/components/timeline-view/timeline-view.component.spec.ts +++ b/src/app/views/components/timeline-view/timeline-view.component.spec.ts @@ -1,25 +1,126 @@ -// import { async, ComponentFixture, TestBed } from '@angular/core/testing'; - -// import { TimelineViewComponent } from './timeline-view.component'; - -// describe('TimelineViewComponent', () => { -// let component: TimelineViewComponent; -// let fixture: ComponentFixture; - -// beforeEach(async(() => { -// TestBed.configureTestingModule({ -// declarations: [ TimelineViewComponent ] -// }) -// .compileComponents(); -// })); - -// beforeEach(() => { -// fixture = TestBed.createComponent(TimelineViewComponent); -// component = fixture.componentInstance; -// fixture.detectChanges(); -// }); - -// it('should create', () => { -// expect(component).toBeTruthy(); -// }); -// }); +import { ComponentFixture, TestBed } from '@angular/core/testing'; +import * as Testing from '@root/test/testbedConfig'; +import { cloneDeep } from 'lodash'; +import { ActivatedRoute } from '@angular/router'; +import { Subscription } from 'rxjs'; + +import { ApiService } from '@shared/services/api/api.service'; +import { DataService } from '@shared/services/data/data.service'; +import { MessageService } from '@shared/services/message/message.service'; +import { FolderResponse } from '@shared/services/api/index.repo'; +import { FolderVO } from '@root/app/models'; +import { TimelineViewComponent } from './timeline-view.component'; + +const buildFolderResponse = (displayName: string) => + new FolderResponse({ + isSuccessful: true, + Results: [{ data: [{ FolderVO: { displayName, ChildItemVOs: [] } }] }], + }); + +describe('TimelineViewComponent', () => { + let component: TimelineViewComponent; + let fixture: ComponentFixture; + let api: ApiService; + let dataService: DataService; + let message: MessageService; + + beforeEach(async () => { + const config = cloneDeep(Testing.BASE_TEST_CONFIG); + + config.declarations.push(TimelineViewComponent); + config.providers.push(DataService); + config.providers.push(ApiService); + config.providers.push({ + provide: ActivatedRoute, + useValue: { + snapshot: { data: { currentFolder: new FolderVO({}) }, params: {} }, + }, + }); + + TestBed.configureTestingModule(config).compileComponents(); + + // Deliberately no detectChanges: ngOnInit stands up vis-timeline against a + // real canvas, and none of that is under test here. + fixture = TestBed.createComponent(TimelineViewComponent); + component = fixture.componentInstance; + + api = TestBed.inject(ApiService); + dataService = TestBed.inject(DataService); + message = TestBed.inject(MessageService); + + // ngOnDestroy still runs when the fixture is torn down, and it tears down + // the two things the skipped lifecycle hooks would have created. + component.timeline = { destroy: () => {} } as any; + (component as any).dataServiceSubscription = new Subscription(); + }); + + it('should create', () => { + expect(component).toBeTruthy(); + }); + + describe('onFolderClick', () => { + it('should publish the loaded folder as the current folder', async () => { + spyOn(api.folder, 'getWithChildrenByIdentifier').and.resolveTo( + buildFolderResponse('Photos'), + ); + const setCurrentFolderSpy = spyOn(dataService, 'setCurrentFolder'); + + await component.onFolderClick(new FolderVO({ folderId: '77' })); + + expect(setCurrentFolderSpy).toHaveBeenCalled(); + expect(setCurrentFolderSpy.calls.mostRecent().args[0].displayName).toBe( + 'Photos', + ); + + expect(component.isNavigating).toBeFalse(); + }); + + it('should pass a breadcrumb folder through unchanged', async () => { + const getSpy = spyOn( + api.folder, + 'getWithChildrenByIdentifier', + ).and.resolveTo(buildFolderResponse('Ancestor')); + spyOn(dataService, 'setCurrentFolder'); + + await component.onFolderClick( + new FolderVO({ archiveNbr: '0001-0005', folder_linkId: 99 }), + ); + + const requestedFolder = getSpy.calls.mostRecent().args[0]; + + expect(requestedFolder.archiveNbr).toBe('0001-0005'); + expect(requestedFolder.folder_linkId).toBe(99); + expect(requestedFolder.folderId).toBeUndefined(); + }); + + it('should show an error instead of rejecting when the load fails', async () => { + spyOn(api.folder, 'getWithChildrenByIdentifier').and.rejectWith( + new Error('Network down'), + ); + const setCurrentFolderSpy = spyOn(dataService, 'setCurrentFolder'); + const showErrorSpy = spyOn(message, 'showError'); + + await expectAsync( + component.onFolderClick(new FolderVO({ folderId: '77' })), + ).toBeResolved(); + + expect(showErrorSpy).toHaveBeenCalledWith({ + message: 'error.generic.internal', + translate: true, + }); + + expect(setCurrentFolderSpy).not.toHaveBeenCalled(); + }); + + it('should stop navigating even when the load fails', async () => { + spyOn(api.folder, 'getWithChildrenByIdentifier').and.rejectWith( + new Error('Network down'), + ); + spyOn(message, 'showError'); + + await component.onFolderClick(new FolderVO({ folderId: '77' })); + + expect(component.isNavigating).toBeFalse(); + }); + }); +}); diff --git a/src/app/views/components/timeline-view/timeline-view.component.ts b/src/app/views/components/timeline-view/timeline-view.component.ts index c67f46154..bcea37f3f 100644 --- a/src/app/views/components/timeline-view/timeline-view.component.ts +++ b/src/app/views/components/timeline-view/timeline-view.component.ts @@ -27,6 +27,8 @@ import { find, throttle, maxBy, debounce, countBy } from 'lodash'; import { Subscription } from 'rxjs'; import { FolderViewService } from '@shared/services/folder-view/folder-view.service'; import { DeviceService } from '@shared/services/device/device.service'; +import { MessageService } from '@shared/services/message/message.service'; +import { getFolderErrorMessage } from '@shared/utilities/folder-error-message'; import { slideUpAnimation } from '@shared/animations'; import { TimelineBreadcrumbsComponent, @@ -148,6 +150,7 @@ export class TimelineViewComponent implements OnInit, AfterViewInit, OnDestroy { private elementRef: ElementRef, private fvService: FolderViewService, private device: DeviceService, + private message: MessageService, ) { this.currentTimespan = TimelineGroupTimespan.Year; this.dataService.showBreadcrumbs = false; @@ -485,11 +488,23 @@ export class TimelineViewComponent implements OnInit, AfterViewInit, OnDestroy { if (folder.isFetching) { await folder.fetched; } - const folderResponse = await this.api.folder - .navigateLean(folder) - .toPromise(); - this.dataService.setCurrentFolder(folderResponse.getFolderVO(true)); - this.isNavigating = false; + try { + const folderResponse = + await this.api.folder.getWithChildrenByIdentifier(folder); + + if (!folderResponse.isSuccessful) { + throw folderResponse; + } + + this.dataService.setCurrentFolder(folderResponse.getFolderVO(true)); + } catch (error) { + this.message.showError({ + message: getFolderErrorMessage(error), + translate: true, + }); + } finally { + this.isNavigating = false; + } } async onRecordClick(record: RecordVO) {