From e54b14f0516f35a6cb7cea3bc6371c31edeac16e Mon Sep 17 00:00:00 2001 From: aasandei-vsp Date: Thu, 16 Jul 2026 11:47:25 +0300 Subject: [PATCH 1/4] Extend Stela conversion in getWithChildren and record mapping Map archiveNumber, folderLinkId, parentFolderLinkId, the breadcrumb path arrays, and created/updated timestamps from the Stela folder response. Resolve missing folderIds through the legacy /folder/get endpoint, fetch the folder and its children in parallel, and convert failures into v1-shaped error responses so existing error handlers keep working. Hardcode accessRole to owner on both converted folders and records, since the backend removed the item-level field; without it, permission gates like the sidebar share button treated records as inaccessible. Issue: PER-10476 --- .../shared/services/api/folder.repo.spec.ts | 191 +++++++++++++++++- src/app/shared/services/api/folder.repo.ts | 116 +++++++---- .../shared/services/api/record.repo.spec.ts | 11 + src/app/shared/services/api/record.repo.ts | 4 + 4 files changed, 280 insertions(+), 42 deletions(-) diff --git a/src/app/shared/services/api/folder.repo.spec.ts b/src/app/shared/services/api/folder.repo.spec.ts index bea2d65f3..13f57ed26 100644 --- a/src/app/shared/services/api/folder.repo.spec.ts +++ b/src/app/shared/services/api/folder.repo.spec.ts @@ -1,10 +1,10 @@ import { TestBed } from '@angular/core/testing'; import { FolderVO } from '@models/index'; -import { of } from 'rxjs'; +import { Observable, 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 = { @@ -55,6 +55,42 @@ const fakeChildrenResponse = { ], }; +const buildStelaFolderResponse = (overrides: Record = {}) => ({ + items: [ + { + folderId: '42', + archiveNumber: 'ARCH-001', + archive: { id: 'arch-id', name: 'Test Archive' }, + folderLinkId: 100, + createdAt: '2024-01-01T00:00:00Z', + updatedAt: '2024-06-01T00:00:00Z', + description: 'Test', + displayTimestamp: '2024-01-01T00:00:00Z', + displayEndTimestamp: null, + displayName: 'Test Folder', + downloadName: 'Test Folder', + imageRatio: 1, + paths: { + names: ['My Files', 'Test Folder'], + folderLinkIds: ['55', '100'], + archiveNumbers: ['ARCH-000', 'ARCH-001'], + }, + publicAt: null, + sort: null, + thumbnailUrls: null, + type: 'type.folder.generic', + status: 'status.generic.ok', + view: 'grid', + size: 0, + location: null, + parentFolder: { id: 'parent-id', parentFolderLinkId: 55 }, + shares: null, + tags: null, + ...overrides, + }, + ], +}); + describe('Folder repo', () => { let folderRepo: FolderRepo; let httpSpy: jasmine.SpyObj; @@ -158,10 +194,13 @@ describe('Folder repo', () => { it('should get folder with children using fallback to auth token', async () => { const mockFolderVO = { folderId: 42 } as FolderVO; + // The folder and children requests run in parallel, so the call order + // is: folder (share token), children (share token), then the + // auth-token fallbacks in the same order. httpV2Spy.get.and.returnValues( of([emptyResponse]), + of([{}]), of([fakeFolderResponse]), - of([emptyResponse]), of([fakeChildrenResponse]), ); @@ -184,6 +223,152 @@ describe('Folder repo', () => { expect(result.Results[0].data[0].FolderVO).toBeDefined(); }); + describe('getWithChildren error handling', () => { + it('should return a FolderResponse with isSuccessful falsy when the Stela API throws', async () => { + const folderVO = new FolderVO({ folderId: 42 }); + const apiError = { error: { error: 'Internal server error' } }; + + httpV2Spy.get.and.returnValue( + new Observable((subscriber) => subscriber.error(apiError)), + ); + + const result = await folderRepo.getWithChildren([folderVO]); + + expect(result.isSuccessful).toBeFalsy(); + }); + + it('should surface the error message from err.error.error via getMessage()', async () => { + const folderVO = new FolderVO({ folderId: 42 }); + const apiError = { error: { error: 'Folder not found' } }; + + httpV2Spy.get.and.returnValue( + new Observable((subscriber) => subscriber.error(apiError)), + ); + + const result = await folderRepo.getWithChildren([folderVO]); + + expect(result.getMessage()).toBe('Folder not found'); + }); + + it('should return an empty error message when err.error.error is absent', async () => { + const folderVO = new FolderVO({ folderId: 42 }); + + httpV2Spy.get.and.returnValue( + new Observable((subscriber) => subscriber.error({})), + ); + + const result = await folderRepo.getWithChildren([folderVO]); + + expect(result.getMessage()).toBeUndefined(); + }); + + it('should surface the message of internally thrown errors via getMessage()', async () => { + const folderVO = new FolderVO({ folderId: 42 }); + + // Both the folder and children endpoints return empty results, + // so getWithChildren throws its internal "no folder" Error. + httpV2Spy.get.and.returnValue(of([{ items: [] }])); + + const result = await folderRepo.getWithChildren([folderVO]); + + expect(result.getMessage()).toBe( + 'No folder returned from getStelaFolders', + ); + }); + }); + + describe('resolveFolderId', () => { + it('should not call the legacy /folder/get endpoint when folderId is already present', async () => { + const folderVO = new FolderVO({ folderId: 42 }); + + httpV2Spy.get.and.returnValues( + of([buildStelaFolderResponse()]), + of([{ items: [] }]), + ); + + await folderRepo.getWithChildren([folderVO]); + + expect(httpSpy.sendRequestPromise).not.toHaveBeenCalled(); + }); + + it('should call legacy /folder/get to resolve folderId when it is missing', async () => { + const folderVO = new FolderVO({ + archiveNbr: '0001-0001', + folder_linkId: 123, + }); + + const resolvedFolderResponse = new FolderResponse({ + isSuccessful: true, + Results: [ + { + data: [{ FolderVO: { folderId: 99 } }], + status: true, + message: ['OK'], + resultDT: new Date().toISOString(), + createdDT: null, + updatedDT: null, + }, + ], + }); + + httpSpy.sendRequestPromise.and.resolveTo(resolvedFolderResponse); + httpV2Spy.get.and.returnValues( + of([buildStelaFolderResponse({ folderId: '99' })]), + of([{ items: [] }]), + ); + + await folderRepo.getWithChildren([folderVO]); + + expect(httpSpy.sendRequestPromise).toHaveBeenCalledWith( + '/folder/get', + jasmine.any(Array), + jasmine.any(Object), + ); + + expect(httpV2Spy.get).toHaveBeenCalledWith('v2/folder', { + folderIds: [99], + }); + }); + }); + + describe('convertStelaFolderToFolderVO mapping', () => { + const getConvertedFolder = async () => { + const folderVO = new FolderVO({ folderId: 42 }); + httpV2Spy.get.and.returnValues( + of([buildStelaFolderResponse()]), + of([{ items: [] }]), + ); + const result = await folderRepo.getWithChildren([folderVO]); + return result.getFolderVO(true); + }; + + it('should map parentFolder_linkId from the parentFolder object', async () => { + const folder = await getConvertedFolder(); + + expect(folder.parentFolder_linkId).toBe(55); + }); + + it('should map the folder path arrays for breadcrumbs', async () => { + const folder = await getConvertedFolder(); + + expect(folder.pathAsText).toEqual(['My Files', 'Test Folder']); + expect(folder.pathAsFolder_linkId).toEqual([55, 100]); + expect(folder.pathAsArchiveNbr).toEqual(['ARCH-000', 'ARCH-001']); + }); + + it('should leave folder_linkType undefined since the backend omits it', async () => { + const folder = await getConvertedFolder(); + + expect(folder.folder_linkType).toBeUndefined(); + }); + + it('should hardcode accessRole to owner', async () => { + const folder = await getConvertedFolder(); + + expect(folder.accessRole).toBe('access.role.owner'); + }); + }); + describe('getFolderShareLink', () => { const mockShareLink: ShareLink = { id: 'link1', diff --git a/src/app/shared/services/api/folder.repo.ts b/src/app/shared/services/api/folder.repo.ts index 87ffa4ae3..5d9662eb7 100644 --- a/src/app/shared/services/api/folder.repo.ts +++ b/src/app/shared/services/api/folder.repo.ts @@ -48,6 +48,7 @@ interface StelaFolder { location: StelaLocation; parentFolder: { id: string; + parentFolderLinkId: number; }; shares: Array; tags: Array; @@ -55,6 +56,8 @@ interface StelaFolder { id: string; name: string; }; + archiveNumber: string; + folderLinkId: number; createdAt: string; updatedAt: string; description: string; @@ -66,6 +69,8 @@ interface StelaFolder { imageRatio: number; paths: { names: string[]; + folderLinkIds: string[]; + archiveNumbers: string[]; }; publicAt: string; sort: string; @@ -103,6 +108,9 @@ const convertStelaFolderToFolderVO = (stelaFolder: StelaFolder): FolderVO => { ...stelaFolder, folderId: stelaFolder.folderId, archiveId: stelaFolder.archive?.id, + archiveNbr: stelaFolder.archiveNumber, + folder_linkId: stelaFolder.folderLinkId, + parentFolder_linkId: stelaFolder.parentFolder?.parentFolderLinkId, displayName: stelaFolder.displayName, displayDT: stelaFolder.displayTimestamp, displayEndDT: stelaFolder.displayEndTimestamp, @@ -126,9 +134,13 @@ const convertStelaFolderToFolderVO = (stelaFolder: StelaFolder): FolderVO => { thumbnail256: stelaFolder.thumbnailUrls?.['256'], thumbnail256CloudPath: stelaFolder.thumbnailUrls?.['256'], status: stelaFolder.status, + createdDT: stelaFolder.createdAt, + updatedDT: stelaFolder.updatedAt, publicDT: stelaFolder.publicAt, parentFolderId: stelaFolder.parentFolder?.id, pathAsText: stelaFolder.paths?.names, + pathAsFolder_linkId: stelaFolder.paths?.folderLinkIds?.map(Number), + pathAsArchiveNbr: stelaFolder.paths?.archiveNumbers, ParentFolderVOs: [new FolderVO({ folderId: stelaFolder.parentFolder?.id })], ChildFolderVOs: childFolderVOs, RecordVOs: childRecordVOs, @@ -139,6 +151,9 @@ const convertStelaFolderToFolderVO = (stelaFolder: StelaFolder): FolderVO => { ), ChildItemVOs: [...childRecordVOs, ...childFolderVOs], ShareVOs: (stelaFolder.shares ?? []).map(convertStelaSharetoShareVO), + // accessRole is intentionally always owner: the backend removed item-level accessRole + // because all non-owner values were deprecated in 2020. Real access is on ShareVOs. + accessRole: 'access.role.owner', isFolder: true, }); }; @@ -303,51 +318,74 @@ export class FolderRepo extends BaseRepo { return response[0].items; } + private async resolveFolderId(folderVO: FolderVO): Promise { + if (folderVO.folderId) { + return folderVO; + } + const response = await this.get([folderVO]); + const resolvedFolder = response.getFolderVO(); + return new FolderVO({ ...folderVO, folderId: resolvedFolder.folderId }); + } + public async getWithChildren( folderVOs: FolderVO[], shareToken: string = null, ): Promise { - // Stela has two separate endpoints -- one for loading the folder, one for loading the children. - const requests = folderVOs.map(async (folderVO) => { - const stelaFolders = await this.getStelaFolders([folderVO], shareToken); - const stelaFolderChildren = await this.getStelaFolderChildren( - folderVO, - shareToken, + try { + // Stela has two separate endpoints -- one for loading the folder, one for loading the children. + const requests = folderVOs.map(async (folderVO) => { + const resolvedFolderVO = await this.resolveFolderId(folderVO); + const [stelaFolders, stelaFolderChildren] = await Promise.all([ + this.getStelaFolders([resolvedFolderVO], shareToken), + this.getStelaFolderChildren(resolvedFolderVO, shareToken), + ]); + const stelaFolder = stelaFolders[0]; + if (!stelaFolder) { + throw new Error('No folder returned from getStelaFolders'); + } + return { + ...stelaFolder, + children: stelaFolderChildren, + }; + }); + + const stelaFolders = (await Promise.all(requests)).flat(); + + // We need the `Results` to look the way v1 results look, for now. + const simulatedV1FolderResponseResults = stelaFolders.map( + (stelaFolder) => ({ + data: [ + { + FolderVO: convertStelaFolderToFolderVO(stelaFolder), + }, + ], + message: ['Folder retrieved'], + status: true, + resultDT: new Date().toISOString(), + createdDT: null, + updatedDT: null, + }), ); - const stelaFolder = stelaFolders[0]; - if (!stelaFolder) { - throw new Error('No folder returned from getStelaFolders'); - } - return { - ...stelaFolder, - children: stelaFolderChildren, - }; - }); - - const stelaFolders = (await Promise.all(requests)).flat(); - - // We need the `Results` to look the way v1 results look, for now. - const simulatedV1FolderResponseResults = stelaFolders.map( - (stelaFolder) => ({ - data: [ - { - FolderVO: convertStelaFolderToFolderVO(stelaFolder), - }, - ], - message: ['Folder retrieved'], - status: true, - resultDT: new Date().toISOString(), - createdDT: null, - updatedDT: null, - }), - ); - const folderResponse = new FolderResponse({ - isSuccessful: true, - isSystemUp: true, - Results: simulatedV1FolderResponseResults, - }); - return folderResponse; + const folderResponse = new FolderResponse({ + isSuccessful: true, + isSystemUp: true, + Results: simulatedV1FolderResponseResults, + }); + return folderResponse; + } catch (err) { + // We need the error to look the way v1 errors look too, + // Changing all the error handlers would be errror prone + const errorFolderResponse = new FolderResponse(); + errorFolderResponse.Results = [ + { + // Stela API errors carry the message in err.error.error; + // internally thrown Errors carry it in err.message. + message: [err?.error?.error ?? err?.message], + }, + ]; + return errorFolderResponse; + } } public navigate(folderVO: FolderVO): Observable { diff --git a/src/app/shared/services/api/record.repo.spec.ts b/src/app/shared/services/api/record.repo.spec.ts index 4cb1ebedc..c0e71e63a 100644 --- a/src/app/shared/services/api/record.repo.spec.ts +++ b/src/app/shared/services/api/record.repo.spec.ts @@ -447,5 +447,16 @@ describe('RecordRepo', () => { expect(record.displayTime).toBeUndefined(); }); + + it('should hardcode accessRole to owner, matching the folder conversion', () => { + // The backend omits item-level accessRole; UI permission gates + // (e.g. the sidebar share button) read it and treat a missing + // value as no access. + const record = convertStelaRecordToRecordVO({ + ...baseStelaRecord, + } as any); + + expect(record.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 d132d7121..cf5864d53 100644 --- a/src/app/shared/services/api/record.repo.ts +++ b/src/app/shared/services/api/record.repo.ts @@ -222,6 +222,10 @@ export const convertStelaRecordToRecordVO = ( timeZoneId: CENTRAL_TIMEZONE_VO.timeZoneId, TimezoneVO: CENTRAL_TIMEZONE_VO, ShareVOs: (stelaRecord.shares ?? []).map(convertStelaSharetoShareVO), + // accessRole is intentionally always owner, matching the folder + // conversion: the backend removed item-level accessRole because all + // non-owner values were deprecated in 2020. Real access is on ShareVOs. + accessRole: 'access.role.owner', }); export class RecordRepo extends BaseRepo { From 5bbdd9fd1b30e6b380068e7d5b85842a182c5116 Mon Sep 17 00:00:00 2001 From: aasandei-vsp Date: Thu, 16 Jul 2026 14:44:02 +0300 Subject: [PATCH 2/4] Migrate navigation consumers to getWithChildren Replace navigateLean with getWithChildren in the lean folder resolver, the filesystem API service, and the timeline view. The resolver detects the error-shaped response getWithChildren returns on failure and keeps its logout/redirect handling; the filesystem service now uses a single code path for regular and unlisted-share navigation. Issue: PER-10476 --- .../lean-folder-resolve.service.spec.ts | 287 ++++++++++++++++++ .../resolves/lean-folder-resolve.service.ts | 71 +++-- .../filesystem/filesystem-api.service.spec.ts | 34 +-- src/app/filesystem/filesystem-api.service.ts | 19 +- .../timeline-view/timeline-view.component.ts | 4 +- 5 files changed, 345 insertions(+), 70 deletions(-) create mode 100644 src/app/core/resolves/lean-folder-resolve.service.spec.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..8972838e5 --- /dev/null +++ b/src/app/core/resolves/lean-folder-resolve.service.spec.ts @@ -0,0 +1,287 @@ +import { TestBed } from '@angular/core/testing'; +import { Router } from '@angular/router'; +import { ActivatedRouteSnapshot, RouterStateSnapshot } from '@angular/router'; +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 { FolderVO } from '@root/app/models'; +import { LeanFolderResolveService } from './lean-folder-resolve.service'; + +const buildMockFolderResponse = (folderVO: Partial = {}) => + new FolderResponse({ + isSuccessful: true, + Results: [ + { + data: [ + { + FolderVO: new FolderVO({ + folderId: 'folder-1', + type: 'type.folder.generic', + ChildItemVOs: [], + ...folderVO, + }), + }, + ], + status: true, + message: ['OK'], + resultDT: new Date().toISOString(), + createdDT: null, + updatedDT: null, + }, + ], + }); + +const buildRoute = ( + params: Record = {}, + parentData: Record = {}, +): ActivatedRouteSnapshot => + ({ + params, + parent: { data: parentData }, + }) as any; + +const buildState = (url: string): RouterStateSnapshot => ({ url }) as any; + +describe('LeanFolderResolveService', () => { + let service: LeanFolderResolveService; + let getWithChildrenSpy: jasmine.Spy; + let accountService: { getRootFolder: jasmine.Spy; logOut: jasmine.Spy }; + let messageService: { showError: jasmine.Spy }; + let router: { navigate: jasmine.Spy }; + + const privateFolder = new FolderVO({ + folderId: 'private-root', + folder_linkId: 1, + archiveNbr: '0001-0001', + type: 'type.folder.root.private', + }); + + const appsFolder = new FolderVO({ + folderId: 'apps-root', + folder_linkId: 2, + archiveNbr: '0002-0001', + type: 'type.folder.root.app', + }); + + beforeEach(() => { + getWithChildrenSpy = jasmine + .createSpy('getWithChildren') + .and.resolveTo(buildMockFolderResponse()); + + accountService = { + getRootFolder: jasmine.createSpy('getRootFolder').and.returnValue( + new FolderVO({ + ChildItemVOs: [privateFolder, appsFolder], + }), + ), + logOut: jasmine.createSpy('logOut').and.resolveTo(undefined), + }; + + messageService = { showError: jasmine.createSpy('showError') }; + router = { navigate: jasmine.createSpy('navigate') }; + + TestBed.configureTestingModule({ + providers: [ + LeanFolderResolveService, + { + provide: ApiService, + useValue: { folder: { getWithChildren: getWithChildrenSpy } }, + }, + { provide: AccountService, useValue: accountService }, + { provide: MessageService, useValue: messageService }, + { provide: Router, useValue: router }, + ], + }); + + service = TestBed.inject(LeanFolderResolveService); + }); + + describe('route branches', () => { + it('should call getWithChildren with a FolderVO built from URL params', async () => { + const route = buildRoute({ + archiveNbr: '0001-0001', + folderLinkId: '123', + }); + const state = buildState('/private/0001-0001/123'); + + await service.resolve(route, state); + + const calledWith = getWithChildrenSpy.calls.mostRecent() + .args[0][0] as FolderVO; + + expect(calledWith.archiveNbr).toBe('0001-0001'); + expect(String(calledWith.folder_linkId)).toBe('123'); + }); + + it('should call getWithChildren with the apps folder when url is /apps', async () => { + const route = buildRoute({}); + const state = buildState('/apps'); + + await service.resolve(route, state); + + const calledWith = getWithChildrenSpy.calls.mostRecent() + .args[0][0] as FolderVO; + + expect(calledWith.type).toBe('type.folder.root.app'); + }); + + it('should call getWithChildren with the shared folder when in a /share/ route with a folder', async () => { + const sharedFolder = new FolderVO({ + folderId: 'shared-folder', + type: 'type.folder.generic', + }); + const route = buildRoute( + {}, + { sharePreviewVO: { FolderVO: sharedFolder, RecordVO: null } }, + ); + const state = buildState('/share/token123'); + + await service.resolve(route, state); + + const calledWith = getWithChildrenSpy.calls.mostRecent() + .args[0][0] as FolderVO; + + expect(calledWith.folderId).toBe('shared-folder'); + }); + + it('should return the folder directly without an API call when in a /share/ route with a record', async () => { + const currentFolder = new FolderVO({ + folderId: 'current', + pathAsArchiveNbr: ['root'], + pathAsText: ['My Files'], + pathAsFolder_linkId: [0], + ChildItemVOs: [], + }); + const sharedRecord = { recordId: 999, isRecord: true }; + const route = buildRoute( + {}, + { + sharePreviewVO: { FolderVO: null, RecordVO: sharedRecord }, + currentFolder, + }, + ); + const state = buildState('/share/token123'); + + const result = await service.resolve(route, state); + + expect(getWithChildrenSpy).not.toHaveBeenCalled(); + expect(result.ChildItemVOs).toContain(sharedRecord as any); + }); + + it('should call getWithChildren with the private root folder for the default route', async () => { + const route = buildRoute({}); + const state = buildState('/private'); + + await service.resolve(route, state); + + const calledWith = getWithChildrenSpy.calls.mostRecent() + .args[0][0] as FolderVO; + + expect(calledWith.type).toBe('type.folder.root.private'); + }); + + it('should return the FolderVO from the response', async () => { + getWithChildrenSpy.and.resolveTo( + buildMockFolderResponse({ folderId: 'resolved' }), + ); + + const route = buildRoute({}); + const state = buildState('/private'); + + const result = await service.resolve(route, state); + + expect(result.folderId).toBe('resolved'); + }); + }); + + describe('error handling', () => { + // getWithChildren never rejects: on failure it resolves with an + // error-shaped FolderResponse (isSuccessful falsy, no data, message + // from the API error). This mirrors that shape. + const buildErrorFolderResponse = (errorMessage: string) => { + const errorResponse = new FolderResponse(); + errorResponse.Results = [{ message: [errorMessage] }]; + return errorResponse; + }; + + it('should show an error message when getWithChildren returns an unsuccessful response', async () => { + getWithChildrenSpy.and.resolveTo( + buildErrorFolderResponse('Folder not found'), + ); + + const route = buildRoute({}); + const state = buildState('/private'); + + await service.resolve(route, state).catch(() => {}); + + expect(messageService.showError).toHaveBeenCalledWith({ + message: 'Folder not found', + translate: true, + }); + }); + + it('should log out and navigate to /login when a root folder fails', async () => { + // Default branch → privateFolder with type 'type.folder.root.private' + // which includes 'root' → logOut is called + getWithChildrenSpy.and.resolveTo( + buildErrorFolderResponse('Folder not found'), + ); + + const route = buildRoute({}); + const state = buildState('/private'); + + await service.resolve(route, state).catch(() => {}); + + expect(accountService.logOut).toHaveBeenCalled(); + }); + + it('should log out and navigate to /login when the apps root folder fails', async () => { + // Apps branch → appsFolder with type 'type.folder.root.app' + // which also includes 'root' → logOut is called, not navigate(['/apps']) + getWithChildrenSpy.and.resolveTo( + buildErrorFolderResponse('Folder not found'), + ); + + const route = buildRoute({}); + const state = buildState('/apps'); + + await service.resolve(route, state).catch(() => {}); + + expect(accountService.logOut).toHaveBeenCalled(); + }); + + it('should navigate to /private when a non-root shared folder fails', async () => { + // Share branch → sharedFolder with type 'type.folder.generic' (no 'root') + // state.url does not include 'apps' → navigate(['/private']) + getWithChildrenSpy.and.resolveTo( + buildErrorFolderResponse('Folder not found'), + ); + + const sharedFolder = new FolderVO({ + folderId: 'shared-folder', + type: 'type.folder.generic', + }); + const route = buildRoute( + {}, + { sharePreviewVO: { FolderVO: sharedFolder, RecordVO: null } }, + ); + const state = buildState('/share/token123'); + + await service.resolve(route, state).catch(() => {}); + + expect(router.navigate).toHaveBeenCalledWith(['/private']); + }); + + it('should return a rejected promise when the API fails', async () => { + getWithChildrenSpy.and.resolveTo( + buildErrorFolderResponse('Folder not found'), + ); + + const route = buildRoute({}); + const state = buildState('/private'); + + await expectAsync(service.resolve(route, state)).toBeRejected(); + }); + }); +}); diff --git a/src/app/core/resolves/lean-folder-resolve.service.ts b/src/app/core/resolves/lean-folder-resolve.service.ts index 72bbb904a..e2ac13d32 100644 --- a/src/app/core/resolves/lean-folder-resolve.service.ts +++ b/src/app/core/resolves/lean-folder-resolve.service.ts @@ -4,8 +4,6 @@ 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'; @@ -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,39 @@ export class LeanFolderResolveService { targetFolder = new FolderVO(myFiles); } - return this.api.folder - .navigateLean(targetFolder) - .pipe( - map((response: FolderResponse) => { - if (!response.isSuccessful) { - throw response; - } + try { + const response: FolderResponse = await this.api.folder.getWithChildren([ + 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); + // getWithChildren resolves with an error-shaped FolderResponse instead + // of rejecting, so failures must be detected here and thrown to reach + // the catch block below. + if (!response.isSuccessful) { + throw response; + } + + return response.getFolderVO(true); + } catch (response) { + 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); + } } } diff --git a/src/app/filesystem/filesystem-api.service.spec.ts b/src/app/filesystem/filesystem-api.service.spec.ts index 1716643bf..070d3e8de 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'; @@ -11,7 +10,7 @@ const folderId = 42; const mockFolderVO = { folderId, - displayName: 'Unlisted Folder', + displayName: 'Test Folder', ChildItemVOs: [], dataStatus: DataStatus.Lean, }; @@ -20,11 +19,12 @@ const mockSuccessResponse = new FolderResponse({ isSuccessful: true, Results: [ { - data: [ - { - FolderVO: mockFolderVO, - }, - ], + data: [{ FolderVO: mockFolderVO }], + status: true, + message: ['OK'], + resultDT: new Date().toISOString(), + createdDT: null, + updatedDT: null, }, ], }); @@ -46,9 +46,6 @@ describe('FilesystemApiService', () => { getWithChildren: jasmine .createSpy('getWithChildren') .and.returnValue(Promise.resolve(mockSuccessResponse)), - navigateLean: jasmine - .createSpy('navigateLean') - .and.returnValue(of(mockSuccessResponse)), }, }; @@ -72,21 +69,21 @@ describe('FilesystemApiService', () => { expect(service).toBeTruthy(); }); - it('should navigate using navigateLean', async () => { + it('should navigate using getWithChildren with null shareToken when not in an unlisted share', async () => { shareLinksServiceSpy.isUnlistedShare.and.resolveTo(false); const folder = await service.navigate({ folderId }); - expect(mockApiService.folder.navigateLean).toHaveBeenCalledWith( - jasmine.any(FolderVO), + expect(mockApiService.folder.getWithChildren).toHaveBeenCalledWith( + [jasmine.any(FolderVO)], + null, ); expect(folder.folderId).toBe(folderId); - expect(folder.displayName).toBe('Unlisted Folder'); expect(folder.dataStatus).toBe(DataStatus.Lean); }); - it('should navigate using getWithChildren when in unlisted share', async () => { + it('should navigate using getWithChildren with shareToken when in an unlisted share', async () => { shareLinksServiceSpy.isUnlistedShare.and.resolveTo(true); shareLinksServiceSpy.currentShareToken = 'mock-token'; @@ -98,14 +95,13 @@ describe('FilesystemApiService', () => { ); expect(folder.folderId).toBe(folderId); - expect(folder.displayName).toBe('Unlisted Folder'); expect(folder.dataStatus).toBe(DataStatus.Lean); }); - it('should throw FolderResponse error if response is unsuccessful', async () => { + it('should throw when the response is unsuccessful', async () => { shareLinksServiceSpy.isUnlistedShare.and.resolveTo(false); - mockApiService.folder.navigateLean.and.returnValue( - of(mockUnsuccessfulResponse), + mockApiService.folder.getWithChildren.and.returnValue( + Promise.resolve(mockUnsuccessfulResponse), ); try { diff --git a/src/app/filesystem/filesystem-api.service.ts b/src/app/filesystem/filesystem-api.service.ts index 2026554df..c384702e9 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'; @@ -24,17 +23,13 @@ export class FilesystemApiService implements FilesystemApi { public async navigate(folder: FolderIdentifier): Promise { const isUnlistedShare = await this.shareLinksService.isUnlistedShare(); - let response: FolderResponse = null; - if (isUnlistedShare) { - response = await this.api.folder.getWithChildren( - [new FolderVO(folder)], - this.shareLinksService.currentShareToken, - ); - } else { - response = await firstValueFrom( - this.api.folder.navigateLean(new FolderVO(folder)), - ); - } + const shareToken = isUnlistedShare + ? this.shareLinksService.currentShareToken + : null; + const response: FolderResponse = await this.api.folder.getWithChildren( + [new FolderVO(folder)], + shareToken, + ); if (!response.isSuccessful) { throw response; } 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..4ae0c9f03 100644 --- a/src/app/views/components/timeline-view/timeline-view.component.ts +++ b/src/app/views/components/timeline-view/timeline-view.component.ts @@ -485,9 +485,7 @@ export class TimelineViewComponent implements OnInit, AfterViewInit, OnDestroy { if (folder.isFetching) { await folder.fetched; } - const folderResponse = await this.api.folder - .navigateLean(folder) - .toPromise(); + const folderResponse = await this.api.folder.getWithChildren([folder]); this.dataService.setCurrentFolder(folderResponse.getFolderVO(true)); this.isNavigating = false; } From 045c6b429c5f59499341b4363ca0f86eefa5478f Mon Sep 17 00:00:00 2001 From: aasandei-vsp Date: Thu, 16 Jul 2026 15:16:22 +0300 Subject: [PATCH 3/4] Migrate publish dialog and fix title crash for Stela folders Replace navigateLean with getWithChildren in the publish polling loop. Compute the dialog title from a component property that also checks the item type, since folders loaded through the Stela API have no folder_linkType and the raw template access threw a TypeError on every change detection cycle. Issue: PER-10476 --- .../components/publish/publish.component.html | 6 +----- .../components/publish/publish.component.spec.ts | 15 +++++++++++---- .../components/publish/publish.component.ts | 14 +++++++++----- 3 files changed, 21 insertions(+), 14 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 8319e2f87..f05fe903a 100644 --- a/src/app/file-browser/components/publish/publish.component.spec.ts +++ b/src/app/file-browser/components/publish/publish.component.spec.ts @@ -3,7 +3,6 @@ import { ComponentFixture, TestBed } from '@angular/core/testing'; import { AccountService } from '@shared/services/account/account.service'; import { FolderVO, RecordVO } from '@models/index'; import { FolderResponse } from '@shared/services/api/folder.repo'; -import { Observable } from 'rxjs'; import { MessageService } from '@shared/services/message/message.service'; import { EventService } from '@shared/services/event/event.service'; import { DIALOG_DATA, DialogRef } from '@angular/cdk/dialog'; @@ -33,8 +32,6 @@ const mockApiService = { folderVOs: FolderVO[], destination: FolderVO, ): Promise => await Promise.resolve(new FolderResponse({})), - navigateLean: (folder: FolderVO): Observable => - new Observable(), }, publish: { getInternetArchiveLink: async () => ({ @@ -64,7 +61,9 @@ describe('PublishComponent', () => { { provide: DIALOG_DATA, useValue: { - item: { folder_linkType: 'linkType' }, + // No folder_linkType, like items loaded through the Stela + // API, which omits it. + item: { type: 'type.folder.generic', displayName: 'Test Item' }, }, }, { provide: DialogRef, useClass: MockDialogRef }, @@ -94,6 +93,14 @@ describe('PublishComponent', () => { expect(component).toBeTruthy(); }); + it('should show the publish title when the item has no folder_linkType', () => { + const title = fixture.nativeElement.querySelector('.page-title'); + + expect(component.isPublicSourceItem).toBeFalse(); + expect(title.textContent).toContain('Publish'); + expect(title.textContent).toContain('Test Item'); + }); + it('should disaple the public to internet archive button if the user does not have the correct access role', () => { component.publicItem = new RecordVO({ recordId: 1 }); component.publishIa = null; diff --git a/src/app/file-browser/components/publish/publish.component.ts b/src/app/file-browser/components/publish/publish.component.ts index 6c418e652..346821bdf 100644 --- a/src/app/file-browser/components/publish/publish.component.ts +++ b/src/app/file-browser/components/publish/publish.component.ts @@ -8,7 +8,6 @@ import { PublicLinkPipe } from '@shared/pipes/public-link.pipe'; import { AccountService } from '@shared/services/account/account.service'; import { GoogleAnalyticsService } from '@shared/services/google-analytics/google-analytics.service'; import { EVENTS } from '@shared/services/google-analytics/events'; -import { FolderResponse } from '@shared/services/api/index.repo'; import { PublicRoutePipe } from '@shared/pipes/public-route.pipe'; import { Router } from '@angular/router'; import { PublishIaData } from '@models/publish-ia-vo'; @@ -33,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; @@ -55,7 +55,11 @@ export class PublishComponent { this.isAtleastManager = this.getRole().includes('manager') || this.getRole().includes('owner'); - if (this.sourceItem?.folder_linkType?.includes('public')) { + this.isPublicSourceItem = + !!this.sourceItem?.type?.includes('public') || + !!this.sourceItem?.folder_linkType?.includes('public'); + + if (this.isPublicSourceItem) { this.publicItem = this.sourceItem; this.publicLink = this.linkPipe.transform(this.publicItem); this.checkInternetArchiveLink(); @@ -80,9 +84,9 @@ export class PublishComponent { let tries = 0; while (!this.publicItem && tries < 10) { tries += 1; - const publicRootResponse = (await this.api.folder - .navigateLean(publicRoot) - .toPromise()) as FolderResponse; + const publicRootResponse = await this.api.folder.getWithChildren([ + publicRoot, + ]); const publicRootFull = publicRootResponse.getFolderVO(true); const publicFolders: FolderVO[] = publicRootFull.ChildItemVOs.filter( (i) => i instanceof FolderVO, From fa06911d4ac8672df73e14d842de48829ba17a4b Mon Sep 17 00:00:00 2001 From: aasandei-vsp Date: Mon, 3 Aug 2026 10:57:37 +0300 Subject: [PATCH 4/4] Derive access roles and shares breadcrumbs for Stela navigation Stela doesn't return the caller's access role or shares-aware paths, so instead of hardcoding accessRole to owner I now derive it from the item's shares, with children inheriting the parent's role via an in-memory cache across navigation requests. Shared folders also get a rebuilt v1-style 'Shares' breadcrumb path instead of the owner's private path. Fixed two crashes in the folder resolvers' error handling that froze navigation. Issue: PER-10476 --- .../core/resolves/folder-resolve.service.ts | 5 +- .../lean-folder-resolve.service.spec.ts | 19 ++ .../resolves/lean-folder-resolve.service.ts | 5 +- .../shared/services/api/folder.repo.spec.ts | 263 +++++++++++++++++- src/app/shared/services/api/folder.repo.ts | 191 ++++++++++++- .../shared/services/api/record.repo.spec.ts | 59 +++- src/app/shared/services/api/record.repo.ts | 173 +++++++++++- 7 files changed, 684 insertions(+), 31 deletions(-) diff --git a/src/app/core/resolves/folder-resolve.service.ts b/src/app/core/resolves/folder-resolve.service.ts index ab54c126b..7c9023361 100644 --- a/src/app/core/resolves/folder-resolve.service.ts +++ b/src/app/core/resolves/folder-resolve.service.ts @@ -97,7 +97,10 @@ export class FolderResolveService { message: response.getMessage(), translate: true, }); - if (targetFolder.type.includes('root')) { + // targetFolder built from route params has no type; without the + // optional chain a failed navigation crashes here instead of + // redirecting, leaving the app stuck on the old route. + if (targetFolder.type?.includes('root')) { this.accountService .logOut() .then(() => { diff --git a/src/app/core/resolves/lean-folder-resolve.service.spec.ts b/src/app/core/resolves/lean-folder-resolve.service.spec.ts index 8972838e5..c952cde73 100644 --- a/src/app/core/resolves/lean-folder-resolve.service.spec.ts +++ b/src/app/core/resolves/lean-folder-resolve.service.spec.ts @@ -205,6 +205,25 @@ describe('LeanFolderResolveService', () => { return errorResponse; }; + it('should navigate to /private when a folder built from URL params fails', async () => { + // A targetFolder built from route params has no type; the error + // handler must not crash on the root-type check and must still + // redirect instead of leaving navigation stuck. + getWithChildrenSpy.and.resolveTo( + buildErrorFolderResponse('Folder not found'), + ); + + const route = buildRoute({ + archiveNbr: '0001-0001', + folderLinkId: '123', + }); + const state = buildState('/shares/0001-0001/123'); + + await service.resolve(route, state).catch(() => {}); + + expect(router.navigate).toHaveBeenCalledWith(['/private']); + }); + it('should show an error message when getWithChildren returns an unsuccessful response', async () => { getWithChildrenSpy.and.resolveTo( buildErrorFolderResponse('Folder not found'), diff --git a/src/app/core/resolves/lean-folder-resolve.service.ts b/src/app/core/resolves/lean-folder-resolve.service.ts index e2ac13d32..1fd78303d 100644 --- a/src/app/core/resolves/lean-folder-resolve.service.ts +++ b/src/app/core/resolves/lean-folder-resolve.service.ts @@ -76,7 +76,10 @@ export class LeanFolderResolveService { message: response.getMessage(), translate: true, }); - if (targetFolder.type.includes('root')) { + // targetFolder built from route params has no type; without the + // optional chain a failed navigation crashes here instead of + // redirecting, leaving the app stuck on the old route. + if (targetFolder.type?.includes('root')) { this.accountService .logOut() .then(() => { diff --git a/src/app/shared/services/api/folder.repo.spec.ts b/src/app/shared/services/api/folder.repo.spec.ts index 13f57ed26..a5853c525 100644 --- a/src/app/shared/services/api/folder.repo.spec.ts +++ b/src/app/shared/services/api/folder.repo.spec.ts @@ -2,9 +2,15 @@ import { TestBed } from '@angular/core/testing'; import { FolderVO } from '@models/index'; import { Observable, of } from 'rxjs'; import { ShareLink } from '@root/app/share-links/models/share-link'; +import { StorageService } from '@shared/services/storage/storage.service'; import { HttpV2Service } from '../http-v2/http-v2.service'; import { HttpService } from '../http/http.service'; -import { FolderRepo, FolderResponse } from './folder.repo'; +import { clearDerivedStelaAccessRoleCache } from './record.repo'; +import { + clearSharesBreadcrumbPathCache, + FolderRepo, + FolderResponse, +} from './folder.repo'; const emptyResponse = { items: [] }; const fakeFolderResponse = { @@ -250,7 +256,9 @@ describe('Folder repo', () => { expect(result.getMessage()).toBe('Folder not found'); }); - it('should return an empty error message when err.error.error is absent', async () => { + it('should return an empty string message when err.error.error is absent', async () => { + // Must be a string, not undefined: error handlers pass the message + // through PrConstantsService.translate, which crashes on undefined. const folderVO = new FolderVO({ folderId: 42 }); httpV2Spy.get.and.returnValue( @@ -259,7 +267,7 @@ describe('Folder repo', () => { const result = await folderRepo.getWithChildren([folderVO]); - expect(result.getMessage()).toBeUndefined(); + expect(result.getMessage()).toBe(''); }); it('should surface the message of internally thrown errors via getMessage()', async () => { @@ -361,12 +369,257 @@ describe('Folder repo', () => { expect(folder.folder_linkType).toBeUndefined(); }); + }); - it('should hardcode accessRole to owner', async () => { - const folder = await getConvertedFolder(); + describe('accessRole derivation', () => { + const CURRENT_ARCHIVE_ID = 77; + const storage = new StorageService(); + + const okShareWithCurrentArchive = (accessRole: string) => ({ + id: 'share-1', + status: 'status.generic.ok', + accessRole, + archive: { + id: String(CURRENT_ARCHIVE_ID), + name: 'My Archive', + thumbURL200: '', + }, + }); + + const getConvertedFolder = async ( + folderOverrides: Record, + children: unknown[] = [], + ) => { + httpV2Spy.get.and.returnValues( + of([buildStelaFolderResponse(folderOverrides)]), + of([{ items: children }]), + ); + const result = await folderRepo.getWithChildren([ + new FolderVO({ folderId: 42 }), + ]); + return result.getFolderVO(true); + }; + + beforeEach(() => { + clearDerivedStelaAccessRoleCache(); + clearSharesBreadcrumbPathCache(); + storage.local.set('archive', { archiveId: CURRENT_ARCHIVE_ID }); + }); + + afterEach(() => { + clearDerivedStelaAccessRoleCache(); + clearSharesBreadcrumbPathCache(); + storage.local.delete('archive'); + storage.session.delete('archive'); + }); + + it('derives owner for folders belonging to the current archive', async () => { + const folder = await getConvertedFolder({ + archive: { id: String(CURRENT_ARCHIVE_ID), name: 'My Archive' }, + }); + + expect(folder.accessRole).toBe('access.role.owner'); + }); + + it('derives the share role for folders shared with the current archive', async () => { + const folder = await getConvertedFolder({ + archive: { id: 'other-archive', name: 'Other Archive' }, + shares: [okShareWithCurrentArchive('access.role.viewer')], + }); + + expect(folder.accessRole).toBe('access.role.viewer'); + }); + + it('ignores shares with the current archive that are not status-ok', async () => { + const folder = await getConvertedFolder({ + archive: { id: 'other-archive', name: 'Other Archive' }, + shares: [ + { + ...okShareWithCurrentArchive('access.role.viewer'), + status: 'status.generic.pending', + }, + ], + }); + + expect(folder.accessRole).toBe('access.role.owner'); + }); + + it('passes a shared folder role down to children without their own shares', async () => { + const childStelaFolder = { + folderId: 'child-folder-1', + archive: { id: 'other-archive', name: 'Other Archive' }, + shares: null, + }; + const childStelaRecord = { + recordId: 'child-record-1', + archive: { id: 'other-archive', name: 'Other Archive' }, + shares: null, + files: [], + tags: null, + location: null, + }; + + const folder = await getConvertedFolder( + { + archive: { id: 'other-archive', name: 'Other Archive' }, + shares: [okShareWithCurrentArchive('access.role.viewer')], + }, + [childStelaFolder, childStelaRecord], + ); + + expect(folder.ChildItemVOs.length).toBe(2); + folder.ChildItemVOs.forEach((childItem) => { + expect(childItem.accessRole).toBe('access.role.viewer'); + }); + }); + + it('prefers a child item direct share over the inherited parent role', async () => { + const childStelaRecord = { + recordId: 'child-record-1', + archive: { id: 'other-archive', name: 'Other Archive' }, + shares: [okShareWithCurrentArchive('access.role.editor')], + files: [], + tags: null, + location: null, + }; + + const folder = await getConvertedFolder( + { + archive: { id: 'other-archive', name: 'Other Archive' }, + shares: [okShareWithCurrentArchive('access.role.viewer')], + }, + [childStelaRecord], + ); + + expect(folder.ChildItemVOs[0].accessRole).toBe('access.role.editor'); + }); + + it('keeps the shared role when navigating deeper into a shared subfolder tree', async () => { + const subfolderStelaFolder = { + folderId: 'subfolder-1', + archive: { id: 'other-archive', name: 'Other Archive' }, + shares: null, + }; + + // First navigation: the directly-shared folder, whose children + // include the subfolder. Stela puts the share only on the folder + // that was shared, so the subfolder itself carries no share. + await getConvertedFolder( + { + archive: { id: 'other-archive', name: 'Other Archive' }, + shares: [okShareWithCurrentArchive('access.role.viewer')], + }, + [subfolderStelaFolder], + ); + + // Second navigation: into the subfolder, a separate request with + // no share data and no parent context of its own. + httpV2Spy.get.and.returnValues( + of([ + buildStelaFolderResponse({ + folderId: 'subfolder-1', + archive: { id: 'other-archive', name: 'Other Archive' }, + shares: null, + }), + ]), + of([ + { + items: [ + { + recordId: 'nested-record-1', + files: [], + tags: null, + location: null, + archive: { id: 'other-archive', name: 'Other Archive' }, + shares: null, + }, + ], + }, + ]), + ); + const result = await folderRepo.getWithChildren([ + new FolderVO({ folderId: 'subfolder-1' }), + ]); + const subfolder = result.getFolderVO(true); + + expect(subfolder.accessRole).toBe('access.role.viewer'); + expect(subfolder.ChildItemVOs[0].accessRole).toBe('access.role.viewer'); + }); + + it('falls back to owner when no current archive is cached', async () => { + storage.local.delete('archive'); + + const folder = await getConvertedFolder({ + archive: { id: 'other-archive', name: 'Other Archive' }, + shares: [okShareWithCurrentArchive('access.role.viewer')], + }); expect(folder.accessRole).toBe('access.role.owner'); }); + + it('rebuilds the breadcrumb path for folders shared with the current archive', async () => { + const folder = await getConvertedFolder({ + archive: { id: 'other-archive', name: 'Other Archive' }, + shares: [okShareWithCurrentArchive('access.role.viewer')], + }); + + expect(folder.pathAsText).toEqual(['Shares', 'Test Folder']); + expect(folder.pathAsFolder_linkId).toEqual([0, 100]); + expect(folder.pathAsArchiveNbr).toEqual(['0000-0000', 'ARCH-001']); + }); + + it('keeps the owner path for folders in the current archive', async () => { + const folder = await getConvertedFolder({ + archive: { id: String(CURRENT_ARCHIVE_ID), name: 'My Archive' }, + }); + + expect(folder.pathAsText).toEqual(['My Files', 'Test Folder']); + }); + + it('extends the shares breadcrumb path when navigating into a subfolder', async () => { + const subfolderStelaFolder = { + folderId: 'subfolder-1', + displayName: 'Subfolder', + archiveNumber: 'ARCH-002', + folderLinkId: 200, + archive: { id: 'other-archive', name: 'Other Archive' }, + shares: null, + }; + + await getConvertedFolder( + { + archive: { id: 'other-archive', name: 'Other Archive' }, + shares: [okShareWithCurrentArchive('access.role.viewer')], + }, + [subfolderStelaFolder], + ); + + httpV2Spy.get.and.returnValues( + of([ + buildStelaFolderResponse({ + folderId: 'subfolder-1', + displayName: 'Subfolder', + archiveNumber: 'ARCH-002', + folderLinkId: 200, + archive: { id: 'other-archive', name: 'Other Archive' }, + shares: null, + }), + ]), + of([{ items: [] }]), + ); + const result = await folderRepo.getWithChildren([ + new FolderVO({ folderId: 'subfolder-1' }), + ]); + const subfolder = result.getFolderVO(true); + + expect(subfolder.pathAsText).toEqual([ + 'Shares', + 'Test Folder', + 'Subfolder', + ]); + + expect(subfolder.pathAsFolder_linkId).toEqual([0, 100, 200]); + }); }); describe('getFolderShareLink', () => { diff --git a/src/app/shared/services/api/folder.repo.ts b/src/app/shared/services/api/folder.repo.ts index 5d9662eb7..695700d62 100644 --- a/src/app/shared/services/api/folder.repo.ts +++ b/src/app/shared/services/api/folder.repo.ts @@ -3,11 +3,17 @@ import { BaseResponse, BaseRepo } from '@shared/services/api/base'; import { firstValueFrom, Observable } from 'rxjs'; import { DataStatus } from '@models/data-status.enum'; import { ShareLink } from '@root/app/share-links/models/share-link'; +import { AccessRoleType } from '@models/access-role'; import { convertStelaLocationToLocnVOData, convertStelaRecordToRecordVO, convertStelaSharetoShareVO, convertStelaTagToTagVO, + deriveStelaAccessRole, + findStelaShareWithCurrentArchive, + getCachedCurrentArchiveId, + recallDerivedStelaAccessRole, + rememberDerivedStelaAccessRole, StelaLocation, StelaShare, StelaTag, @@ -96,14 +102,173 @@ type StelaFolderChild = StelaFolder | StelaRecord; const isStelaRecord = (child: StelaFolderChild): child is StelaRecord => child && 'recordId' in child; -const convertStelaFolderToFolderVO = (stelaFolder: StelaFolder): FolderVO => { +interface SharesBreadcrumbPath { + pathAsText: string[]; + pathAsArchiveNbr: string[]; + pathAsFolder_linkId: number[]; +} + +// For folders reached through a share, v1 never returned the owning archive's +// real ancestry: it synthesized breadcrumbs in the PHP session +// (SessionBO::HandlePath) as a fake 'Shares' root, the owning archive's name, +// then only the folders the caller navigated through inside the share. Stela's +// paths field is instead the owning archive's full private ancestry, which the +// caller can neither see nor navigate — using it flips the breadcrumb root to +// Private. Rebuild the v1 shape client-side, and carry it across navigation +// requests the same way derived access roles are carried, since each request +// loads a single folder level. In-memory only: a page reload or a new tab +// loses it, and deep links into a shared subtree fall back to the owner path +// until the backend exposes share-aware paths. +const sharesBreadcrumbPathCache = new Map(); + +const buildSharesBreadcrumbPathCacheKey = ( + folderId: string | number, + currentArchiveId: string | number, +): string => `${String(currentArchiveId)}:${String(folderId)}`; + +const rememberSharesBreadcrumbPath = ( + folderId: string | number | null | undefined, + currentArchiveId: string | number | null | undefined, + sharesBreadcrumbPath: SharesBreadcrumbPath, +): void => { + if ( + folderId === null || + folderId === undefined || + currentArchiveId === null || + currentArchiveId === undefined + ) { + return; + } + sharesBreadcrumbPathCache.set( + buildSharesBreadcrumbPathCacheKey(folderId, currentArchiveId), + sharesBreadcrumbPath, + ); +}; + +const recallSharesBreadcrumbPath = ( + folderId: string | number | null | undefined, + currentArchiveId: string | number | null | undefined, +): SharesBreadcrumbPath | undefined => { + if ( + folderId === null || + folderId === undefined || + currentArchiveId === null || + currentArchiveId === undefined + ) { + return undefined; + } + return sharesBreadcrumbPathCache.get( + buildSharesBreadcrumbPathCacheKey(folderId, currentArchiveId), + ); +}; + +export const clearSharesBreadcrumbPathCache = (): void => { + sharesBreadcrumbPathCache.clear(); +}; + +const appendFolderToSharesBreadcrumbPath = ( + sharesBreadcrumbPath: SharesBreadcrumbPath, + stelaFolder: StelaFolder, +): SharesBreadcrumbPath => ({ + pathAsText: [...sharesBreadcrumbPath.pathAsText, stelaFolder.displayName], + pathAsArchiveNbr: [ + ...sharesBreadcrumbPath.pathAsArchiveNbr, + stelaFolder.archiveNumber, + ], + pathAsFolder_linkId: [ + ...sharesBreadcrumbPath.pathAsFolder_linkId, + stelaFolder.folderLinkId, + ], +}); + +const deriveSharesBreadcrumbPath = ( + stelaFolder: StelaFolder, + currentArchiveId: string | number | null | undefined, + parentSharesBreadcrumbPath?: SharesBreadcrumbPath, +): SharesBreadcrumbPath | undefined => { + let sharesBreadcrumbPath: SharesBreadcrumbPath | undefined; + if (parentSharesBreadcrumbPath) { + sharesBreadcrumbPath = appendFolderToSharesBreadcrumbPath( + parentSharesBreadcrumbPath, + stelaFolder, + ); + } else if ( + findStelaShareWithCurrentArchive(stelaFolder.shares, currentArchiveId) + ) { + // The share entry point anchors the path. The 'Shares' entry is + // synthetic (no real folder_link, so it must never render as a + // navigable folder crumb), like what FolderResolveService fabricates + // for shared records. Unlike v1, no archive-name element is inserted: + // it had no navigable target and clicking it broke navigation. + sharesBreadcrumbPath = { + pathAsText: ['Shares', stelaFolder.displayName], + pathAsArchiveNbr: ['0000-0000', stelaFolder.archiveNumber], + pathAsFolder_linkId: [0, stelaFolder.folderLinkId], + }; + } else { + sharesBreadcrumbPath = recallSharesBreadcrumbPath( + stelaFolder.folderId, + currentArchiveId, + ); + } + if (sharesBreadcrumbPath) { + rememberSharesBreadcrumbPath( + stelaFolder.folderId, + currentArchiveId, + sharesBreadcrumbPath, + ); + } + return sharesBreadcrumbPath; +}; + +const convertStelaFolderToFolderVO = ( + stelaFolder: StelaFolder, + currentArchiveId: string | number | null = getCachedCurrentArchiveId(), + parentFolderAccessRole?: AccessRoleType, + parentSharesBreadcrumbPath?: SharesBreadcrumbPath, +): FolderVO => { stelaFolder.children ??= []; + const accessRole = deriveStelaAccessRole( + stelaFolder.shares, + stelaFolder.archive?.id, + currentArchiveId, + parentFolderAccessRole ?? + recallDerivedStelaAccessRole( + 'folder', + stelaFolder.folderId, + currentArchiveId, + ), + ); + rememberDerivedStelaAccessRole( + 'folder', + stelaFolder.folderId, + currentArchiveId, + accessRole, + ); + const sharesBreadcrumbPath = deriveSharesBreadcrumbPath( + stelaFolder, + currentArchiveId, + parentSharesBreadcrumbPath, + ); const childFolderVOs = stelaFolder.children .filter((child): child is StelaFolder => !isStelaRecord(child)) - .map(convertStelaFolderToFolderVO); + .map((childStelaFolder) => + convertStelaFolderToFolderVO( + childStelaFolder, + currentArchiveId, + accessRole, + sharesBreadcrumbPath, + ), + ); const childRecordVOs = stelaFolder.children .filter(isStelaRecord) - .map(convertStelaRecordToRecordVO); + .map((childStelaRecord) => + convertStelaRecordToRecordVO( + childStelaRecord, + currentArchiveId, + accessRole, + ), + ); return new FolderVO({ ...stelaFolder, folderId: stelaFolder.folderId, @@ -138,9 +303,13 @@ const convertStelaFolderToFolderVO = (stelaFolder: StelaFolder): FolderVO => { updatedDT: stelaFolder.updatedAt, publicDT: stelaFolder.publicAt, parentFolderId: stelaFolder.parentFolder?.id, - pathAsText: stelaFolder.paths?.names, - pathAsFolder_linkId: stelaFolder.paths?.folderLinkIds?.map(Number), - pathAsArchiveNbr: stelaFolder.paths?.archiveNumbers, + pathAsText: sharesBreadcrumbPath?.pathAsText ?? stelaFolder.paths?.names, + pathAsFolder_linkId: + sharesBreadcrumbPath?.pathAsFolder_linkId ?? + stelaFolder.paths?.folderLinkIds?.map(Number), + pathAsArchiveNbr: + sharesBreadcrumbPath?.pathAsArchiveNbr ?? + stelaFolder.paths?.archiveNumbers, ParentFolderVOs: [new FolderVO({ folderId: stelaFolder.parentFolder?.id })], ChildFolderVOs: childFolderVOs, RecordVOs: childRecordVOs, @@ -151,9 +320,7 @@ const convertStelaFolderToFolderVO = (stelaFolder: StelaFolder): FolderVO => { ), ChildItemVOs: [...childRecordVOs, ...childFolderVOs], ShareVOs: (stelaFolder.shares ?? []).map(convertStelaSharetoShareVO), - // accessRole is intentionally always owner: the backend removed item-level accessRole - // because all non-owner values were deprecated in 2020. Real access is on ShareVOs. - accessRole: 'access.role.owner', + accessRole, isFolder: true, }); }; @@ -380,8 +547,10 @@ export class FolderRepo extends BaseRepo { errorFolderResponse.Results = [ { // Stela API errors carry the message in err.error.error; - // internally thrown Errors carry it in err.message. - message: [err?.error?.error ?? err?.message], + // internally thrown Errors carry it in err.message. The final + // fallback must be a string: error handlers pass this through + // PrConstantsService.translate, which crashes on undefined. + message: [err?.error?.error ?? err?.message ?? ''], }, ]; return errorFolderResponse; diff --git a/src/app/shared/services/api/record.repo.spec.ts b/src/app/shared/services/api/record.repo.spec.ts index c0e71e63a..544c1a4ad 100644 --- a/src/app/shared/services/api/record.repo.spec.ts +++ b/src/app/shared/services/api/record.repo.spec.ts @@ -7,6 +7,7 @@ import { of } from 'rxjs'; import { environment } from '@root/environments/environment'; import { HttpService } from '@shared/services/http/http.service'; import { + clearDerivedStelaAccessRoleCache, convertStelaRecordToRecordVO, RecordRepo, RecordResponse, @@ -410,6 +411,14 @@ describe('RecordRepo', () => { }); describe('convertStelaRecordToRecordVO', () => { + beforeEach(() => { + clearDerivedStelaAccessRoleCache(); + }); + + afterEach(() => { + clearDerivedStelaAccessRoleCache(); + }); + const baseStelaRecord = { recordId: 42, displayName: 'Test Record', @@ -448,13 +457,49 @@ describe('RecordRepo', () => { expect(record.displayTime).toBeUndefined(); }); - it('should hardcode accessRole to owner, matching the folder conversion', () => { - // The backend omits item-level accessRole; UI permission gates - // (e.g. the sidebar share button) read it and treat a missing - // value as no access. - const record = convertStelaRecordToRecordVO({ - ...baseStelaRecord, - } as any); + it('derives owner accessRole for records belonging to the current archive', () => { + const record = convertStelaRecordToRecordVO( + { ...baseStelaRecord } as any, + '1', + ); + + expect(record.accessRole).toBe('access.role.owner'); + }); + + it('derives the share accessRole for records shared with the current archive', () => { + const record = convertStelaRecordToRecordVO( + { + ...baseStelaRecord, + shares: [ + { + id: 'share-1', + status: 'status.generic.ok', + accessRole: 'access.role.editor', + archive: { id: '9', name: 'Recipient Archive', thumbURL200: '' }, + }, + ], + } as any, + 9, + ); + + expect(record.accessRole).toBe('access.role.editor'); + }); + + it('inherits the parent folder accessRole for foreign records without their own share', () => { + const record = convertStelaRecordToRecordVO( + { ...baseStelaRecord } as any, + '9', + 'access.role.viewer', + ); + + expect(record.accessRole).toBe('access.role.viewer'); + }); + + it('falls back to owner accessRole without a current archive context', () => { + const record = convertStelaRecordToRecordVO( + { ...baseStelaRecord } as any, + null, + ); expect(record.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 cf5864d53..21b73afb0 100644 --- a/src/app/shared/services/api/record.repo.ts +++ b/src/app/shared/services/api/record.repo.ts @@ -166,6 +166,150 @@ export const convertStelaSharetoShareVO = (stelaShare: StelaShare): ShareVO => }, }); +// AccountService caches the current archive under this key (see +// AccountService.setArchive). The repos cannot inject AccountService to read +// it directly: that would create a circular dependency through ApiService. +const CURRENT_ARCHIVE_STORAGE_KEY = 'archive'; + +export const getCachedCurrentArchiveId = (): + | string + | number + | null + | undefined => { + const storage = new StorageService(); + const cachedCurrentArchive = + storage.local.get<{ archiveId?: string | number }>( + CURRENT_ARCHIVE_STORAGE_KEY, + ) || + storage.session.get<{ archiveId?: string | number }>( + CURRENT_ARCHIVE_STORAGE_KEY, + ); + return cachedCurrentArchive?.archiveId; +}; + +// Stela ids arrive as strings while the cached archive id is a number. +const normalizeArchiveId = ( + archiveId: string | number | null | undefined, +): string | null => + archiveId === null || archiveId === undefined ? null : String(archiveId); + +// Each navigation request loads a single folder level, and Stela only returns +// a share on the item that was shared directly — so when navigating deeper +// into a shared folder tree, the role learned at a higher level must be +// carried across requests. Derived roles are remembered per (archive, item) +// as the client-side equivalent of v1 materializing a share's role down the +// whole subtree. Entries are only ever re-derived from stronger evidence +// (a direct share match or an inherited parent role win before the recall +// fallback), so a remembered role is never downgraded by a bare fallback. +// In-memory only: a page reload or a new tab loses it, and deep links into a +// shared subtree fall back to owner until the backend exposes the caller's +// effective role. +const derivedStelaAccessRoleCache = new Map(); + +type StelaItemType = 'folder' | 'record'; + +const buildDerivedAccessRoleCacheKey = ( + itemType: StelaItemType, + itemId: string | number, + currentArchiveId: string | number, +): string => `${String(currentArchiveId)}:${itemType}:${String(itemId)}`; + +export const rememberDerivedStelaAccessRole = ( + itemType: StelaItemType, + itemId: string | number | null | undefined, + currentArchiveId: string | number | null | undefined, + accessRole: AccessRoleType, +): void => { + if ( + itemId === null || + itemId === undefined || + currentArchiveId === null || + currentArchiveId === undefined + ) { + return; + } + derivedStelaAccessRoleCache.set( + buildDerivedAccessRoleCacheKey(itemType, itemId, currentArchiveId), + accessRole, + ); +}; + +export const recallDerivedStelaAccessRole = ( + itemType: StelaItemType, + itemId: string | number | null | undefined, + currentArchiveId: string | number | null | undefined, +): AccessRoleType | undefined => { + if ( + itemId === null || + itemId === undefined || + currentArchiveId === null || + currentArchiveId === undefined + ) { + return undefined; + } + return derivedStelaAccessRoleCache.get( + buildDerivedAccessRoleCacheKey(itemType, itemId, currentArchiveId), + ); +}; + +export const clearDerivedStelaAccessRoleCache = (): void => { + derivedStelaAccessRoleCache.clear(); +}; + +// Stela responses carry no field for the caller's own access role on an item, +// so derive it to match what the legacy v1 API returned (its rule was +// COALESCE(access.accessRole, folder_link.accessRole)): +// 1. a status-ok share granting the current archive access to the item wins; +// 2. items belonging to the current archive report owner — the caller's +// archive-level membership role is applied separately by +// AccountService.checkMinimumAccess, never baked into the item; +// 3. items inside a shared folder inherit the parent's derived role, since v1 +// materialized a share's role down the whole subtree — within one response +// via the parentFolderAccessRole argument, and across navigation requests +// via the derived-role cache the converters consult before falling back; +// 4. otherwise fall back to owner, matching v1's fallback for public and +// share-token browsing, where visibility is gated elsewhere. This branch is +// also hit when deep-linking into an unshared descendant of a shared folder +// that was never navigated in this session (no parent context to inherit); +// fixing that requires Stela to expose the caller's effective role. +export const findStelaShareWithCurrentArchive = ( + stelaShares: StelaShare[] | null | undefined, + currentArchiveId: string | number | null | undefined, +): StelaShare | undefined => { + const normalizedCurrentArchiveId = normalizeArchiveId(currentArchiveId); + if (normalizedCurrentArchiveId === null) { + return undefined; + } + return (stelaShares ?? []).find( + (stelaShare) => + stelaShare.status === 'status.generic.ok' && + normalizeArchiveId(stelaShare.archive?.id) === normalizedCurrentArchiveId, + ); +}; + +export const deriveStelaAccessRole = ( + stelaShares: StelaShare[] | null | undefined, + itemArchiveId: string | number | null | undefined, + currentArchiveId: string | number | null | undefined, + parentFolderAccessRole?: AccessRoleType, +): AccessRoleType => { + const shareWithCurrentArchive = findStelaShareWithCurrentArchive( + stelaShares, + currentArchiveId, + ); + if (shareWithCurrentArchive) { + return shareWithCurrentArchive.accessRole; + } + const normalizedCurrentArchiveId = normalizeArchiveId(currentArchiveId); + if ( + normalizedCurrentArchiveId !== null && + normalizeArchiveId(itemArchiveId) === normalizedCurrentArchiveId + ) { + return 'access.role.owner'; + } + return parentFolderAccessRole ?? 'access.role.owner'; +}; + export const convertStelaLocationToLocnVOData = ( stelaLocation: StelaLocation | null | undefined, ): LocnVOData | null => { @@ -196,8 +340,27 @@ export const convertStelaLocationToLocnVOData = ( export const convertStelaRecordToRecordVO = ( stelaRecord: StelaRecord, -): RecordVO => - new RecordVO({ + currentArchiveId: string | number | null = getCachedCurrentArchiveId(), + parentFolderAccessRole?: AccessRoleType, +): RecordVO => { + const accessRole = deriveStelaAccessRole( + stelaRecord.shares, + stelaRecord.archive?.id ?? stelaRecord.archiveId, + currentArchiveId, + parentFolderAccessRole ?? + recallDerivedStelaAccessRole( + 'record', + stelaRecord.recordId, + currentArchiveId, + ), + ); + rememberDerivedStelaAccessRole( + 'record', + stelaRecord.recordId, + currentArchiveId, + accessRole, + ); + return new RecordVO({ ...stelaRecord, thumbURL200: stelaRecord.thumbUrl200, thumbURL500: stelaRecord.thumbUrl500, @@ -222,11 +385,9 @@ export const convertStelaRecordToRecordVO = ( timeZoneId: CENTRAL_TIMEZONE_VO.timeZoneId, TimezoneVO: CENTRAL_TIMEZONE_VO, ShareVOs: (stelaRecord.shares ?? []).map(convertStelaSharetoShareVO), - // accessRole is intentionally always owner, matching the folder - // conversion: the backend removed item-level accessRole because all - // non-owner values were deprecated in 2020. Real access is on ShareVOs. - accessRole: 'access.role.owner', + accessRole, }); +}; export class RecordRepo extends BaseRepo { private async getRecordIdByArchiveNbr(archiveNbr: string): Promise {