[PER-10476, PER-10735] Replace navigateMin with getWithChildren - #1132
[PER-10476, PER-10735] Replace navigateMin with getWithChildren#1132aasandei-vsp wants to merge 1 commit into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1132 +/- ##
==========================================
+ Coverage 52.30% 52.37% +0.06%
==========================================
Files 354 354
Lines 12091 12100 +9
Branches 2185 2189 +4
==========================================
+ Hits 6324 6337 +13
+ Misses 5543 5537 -6
- Partials 224 226 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
NavigateMin was used mainly by the folder picker modal, so this is a more encapsulated change. But because we needed to map some new properties in the folder object, there are side effects that need extensive testing and care to make sure we do not do any regression. This would happen because properties that up until now would be ignored suddenly do have a value and we need to make sure it is the correct one. It is worth mentioning that we also had a positive side effect. With this change, we fixed a bug related to the back button on the folder picker, where instead of creating a folder from its ID, we actually load the correct folder with all its information. Issues: PER-10476, PER-10735
bf77982 to
72c4bfc
Compare
There was a problem hiding this comment.
Pull request overview
This PR replaces the folder picker’s reliance on the legacy /folder/navigateMin flow with the Stela-backed getWithChildren path, while also expanding the Stela→FolderVO mapping (notably archiveNbr and folder_linkId) to prevent downstream regressions and fix Back-navigation thumbnail issues.
Changes:
- Map
archiveNumber → archiveNbrand coercefolderLinkId → folder_linkIdduring Stela folder conversion. - Remove
navigateMinusage from the folder picker by switching folder loads togetWithChildren(andgetRootfor the virtual root). - Add folder picker history replay for Back navigation and expand unit tests for the new behavior and mappings.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/app/shared/services/api/folder.repo.ts | Extends Stela folder mapping to include archiveNbr and numeric folder_linkId; removes legacy navigateMin method. |
| src/app/shared/services/api/folder.repo.spec.ts | Adds tests verifying the new Stela folder conversion behavior and thumbnail carry-through via getWithChildren. |
| src/app/core/components/folder-picker/folder-picker.component.ts | Switches folder loading to getWithChildren/getRoot, introduces visited-folder replay for Back, and updates error handling. |
| src/app/core/components/folder-picker/folder-picker.component.spec.ts | Updates and adds tests to cover the new loading paths, Back replay behavior, filtering, and error handling. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| const toFolderLinkId = (folderLinkId: string): number | undefined => { | ||
| if (folderLinkId === null || folderLinkId === undefined) { | ||
| return undefined; | ||
| } | ||
| const parsed = Number(folderLinkId); | ||
| return Number.isNaN(parsed) ? undefined : parsed; | ||
| }; |
| private async setFolderAndLoadChildData(folder: FolderVO) { | ||
| await this.setFolder(folder); | ||
| this.loadCurrentFolderChildData(); | ||
| } |
| if (this.currentFolder) { | ||
| this.visitedFolders.push(this.currentFolder); | ||
| } | ||
| await this.setFolderAndLoadChildData(folder); |
| const previousFolder = this.visitedFolders.pop(); | ||
|
|
||
| // Nothing to pop when the picker was opened directly on a workspace | ||
| // folder, as the record choosers do with My Files. Going up from there | ||
| // means the archive root. | ||
| return await this.setFolderAndLoadChildData( | ||
| previousFolder ?? new FolderVO({ type: 'type.folder.root.root' }), | ||
| ); |
NavigateMin was used mainly by the folder picker modal, so this is a more encapsulated change. But because we needed to map some new properties in the folder object, there are side effects that need extensive testing and care to make sure we do not do any regression. This would happen because properties that up until now would be ignored suddenly do have a value and we need to make sure it is the correct one.
It is worth mentioning that we also had a positive side effect. With this change, we fixed a bug related to the back button on the folder picker, where instead of creating a folder from its ID, we actually load the correct folder with all its information.
Issues: PER-10476, PER-10735
Manual test cases — Replace navigateMin with getWithChildren
There are a lot of tests we are running not for the code we intended to change, but for the side effects of having a very tight coupled architecture, that can yield in a lot of unexpected behaviours.
Setup: log in on an archive with nested folders, at least one subfolder two
levels deep, a folder containing images.
Move dialog
Select an item and choose Move.
EXPECTED: The picker opens at the root, listing My Files and Public, and the header shows the root folder name.
Click into My Files, then into a subfolder.
EXPECTED: Each folder's subfolders load and the header name updates.
Press Back once.
EXPECTED: You land on the folder you came from, one level up.
Press Back again to reach the top.
EXPECTED: The root lists My Files and Public.
At the root, check the header area.
EXPECTED: The Back link is gone and the confirm button is disabled.
Select a destination folder and confirm the Move.
EXPECTED: The item moves to the chosen folder and the dialog closes.
Copy dialog(I could not test this locally, as copy seems to return a warning from BE. Will test on dev --> works on DEV)
Select an item and choose Copy.
EXPECTED: The picker opens at the root with the same folders as above.
Navigate into a folder, select it, and confirm the Copy.
EXPECTED: The item is copied to the chosen folder and the dialog closes.
Open Copy again and navigate into a public folder, then confirm.
EXPECTED: The "this folder is publicly accessible" confirmation prompt appears before the copy runs.
Choose-file dialog
Open the image picker from the archive profile photo.
EXPECTED: Both folders and records are listed (records are not stripped).
Navigate into a folder containing images.
EXPECTED: Image records show their thumbnails.
Press Back to the parent folder.
EXPECTED: The parent's image records show their thumbnails. This is the case that was broken before — Back did not load child data, so every tile came back blank.
Click a record in that parent folder.
EXPECTED: The preview panel shows the image, not an empty panel.
Confirm the selection.
EXPECTED: The record is accepted and the profile photo updates.(This is a bug fixed in a different PR(#1113 ), refresh to make sure the picture updates)
Repeat steps 1–5 via Change Banner.
EXPECTED: Same behaviour in the banner flow.
Filtered move
Multi-select two folders and choose Move.
EXPECTED: The picker opens at the root.
Navigate into the folder that contains the two selected folders.
EXPECTED: Neither of the two folders being moved appears in the destination list.
Navigate one level deeper and back out.
EXPECTED: The two folders are still excluded after Back — the filter is not lost on re-entry.
Back navigation
Back now replays the folders you actually navigated through rather than deriving a
parent from ids, so these cases are about the trail being correct rather than just
non-empty.(This also fixes https://permanent.atlassian.net/browse/PER-10735?issueKey=PER-10735&subProduct=jira-software )
From the root, go four levels deep, noting each folder name.
EXPECTED: Each level loads its own subfolders.
Press Back four times, checking the header name each time.
EXPECTED: You retrace exactly the folders you came through, in reverse order, one press per level. You arrive at the root on the fourth press, not the fifth.
From the root, go into folder A, back out, then into a different folder B, then one level deeper, then Back twice.
EXPECTED: You return through B to the root. Folder A does not reappear anywhere in the trail.
Go three levels deep, then close the dialog with Cancel and reopen it.
EXPECTED: The picker opens at the root again. Pressing Back does nothing surprising — no jumping into folders from the previous session.
In the record chooser (profile photo), which opens directly on My Files, press Back straight away.
EXPECTED: You go up to the root listing. No error toast, no blank panel.
Select a record, then press Back.
EXPECTED: You return to the same listing with its thumbnails intact.
Field mapping side effects — outside the folder picker
The two fields this change adds to the Stela folder conversion —
archiveNbrandfolder_linkId— were previouslyundefinedon every Stela-sourced folder.These cases cover the minimum needed to confirm no regression in those flows.
Setup: the plain file browser, no dialogs open.
Folder navigation after a lean refresh
Highest-value case. Folder rows navigate using
[item.archiveNbr, item.folder_linkId],and both are now overwritten by the mapped Stela values. Previously they were left
untouched.
Open a folder containing subfolders and wait about ten seconds without clicking.
EXPECTED: No visible change to the list.
Click a subfolder row.
EXPECTED: It opens the correct folder. The URL contains the real archive number and folder link id — no
undefined, noNaN, and not another folder's id.Use the browser Back button, then click a different subfolder.
EXPECTED: The second folder opens correctly too.
Reload the page on the subfolder URL.
EXPECTED: The folder loads directly from the URL.
Distinct folder thumbnails in gallery
Open a folder containing at least three subfolders that each hold different images.
EXPECTED: Each folder tile shows a thumbnail taken from its own contents — no two folders showing the same image.
Navigate into one of them, then back out.
EXPECTED: The tiles still show their own correct thumbnails.
Revisiting a folder after its row was refreshed
Open a folder with subfolders, wait about ten seconds, then navigate into one that you know has contents.
EXPECTED: Its contents are listed. It does not show as empty or "no items".
Go back out and into the same folder a second time.
EXPECTED: Contents are listed again.
Timeline folder click
Switch to timeline view on a folder containing subfolders and click a folder immediately, before the list settles.
EXPECTED: It navigates into the folder. It does not hang on a loading state.
Repeat, this time waiting for the list to settle first.
EXPECTED: Same result.
Renaming a folder
Rename a folder from the file list and wait for the save to finish.
EXPECTED: The new name shows on the row.
Click into the renamed folder.
EXPECTED: It opens the correct folder with the correct contents — the rename did not disturb its identity.
Change the folder's date, then click into it again.
EXPECTED: Still opens correctly.
Drag and drop onto a folder
Drag a file onto a subfolder row and drop it.
EXPECTED: The file moves into that folder, not a different one.
Open the destination folder to confirm.
EXPECTED: The file is there.
Shares
The three share contexts are affected differently, so they are separated here.
Restricted shares are the most exposed: their initial load comes from the legacy
navigateLean, but their lean refreshes go throughgetWithChildrenand so pickup the new mapping. Unlisted shares skip lean refreshes entirely. The shares
workspace listing does not come from folder conversion at all.
Setup: a second account sharing at least one folder (containing subfolders and
images) with your archive, plus an unlisted share link to a folder in your own
archive.
Shares workspace listing
EXPECTED: The list of shared archives and items renders.
Click a shared item to open it.
EXPECTED: It opens. It does not sit doing nothing, and no unhandled rejection appears in the console.
Restricted share navigation
From the Shares workspace, open a folder shared with you by another archive.
EXPECTED: Its contents list correctly.
Wait about ten seconds, then click a subfolder inside it.
EXPECTED: The correct subfolder opens. The URL carries the owning archive's number, not yours, and no
undefinedorNaN.Use the browser Back button and open a different subfolder.
EXPECTED: That one opens correctly too.
Open an image inside the shared folder.
EXPECTED: The preview loads.
Unlisted share preview
Open the unlisted share link while logged out.
EXPECTED: The shared folder and its contents render, with thumbnails.
Navigate into a subfolder, then back out.
EXPECTED: Both levels render correctly, with the right contents at each level — not another folder's contents.
Open the same link while logged in as an account with no access to that archive.
EXPECTED: Same behaviour as logged out.