[PER-10476] [1] Extend getWithChildren toward replacing navigateLean/navigateMin (Stela migration groundwork) - #996
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #996 +/- ##
==========================================
- Coverage 52.19% 52.18% -0.02%
==========================================
Files 354 354
Lines 12089 12099 +10
Branches 2185 2187 +2
==========================================
+ Hits 6310 6314 +4
- Misses 5555 5559 +4
- Partials 224 226 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
aasandei-vsp
changed the title
b65fcfb to
bb0a021
Compare
bb0a021 to
e54b14f
Compare
aasandei-vsp
changed the title
aasandei-vsp
changed the title
|
@aasandei-vsp I'm back in action and coming back to review things! I'm finding this PR / commit hard to review as written. Would you be able to take a stab at updating the commit message + PR title / description to make it clear what the purpose of these changes are? I think it's basically |
e54b14f to
45f656e
Compare
This is one step toward the bigger migration: I'm moving folder navigation off the v1 /folder/navigateLean and /folder/navigateMin endpoints and onto getWithChildren, which is backed by Stela. The idea is that getWithChildren returns the same shape those v1 endpoints do, so I can move consumers over gradually instead of all at once. This commit just closes some of the gaps between what Stela hands back and what those consumers expect — it doesn't finish the migration. What I did: - Mapped the fields I was still dropping from the Stela folder response: archiveNumber, folderLinkId, parentFolderLinkId, the breadcrumb path arrays, and the created/updated timestamps. - When a folderId is missing, I resolve it through the legacy /folder/get endpoint first, then fetch the folder and its children in parallel. - Shaped failures to look like v1 errors so the existing error handlers keep working without me touching them. - Hardcoded accessRole to owner on the converted folders and records. The backend dropped the item-level field, and without it permission gates like the sidebar share button were treating items as inaccessible. Issue: PER-10476
45f656e to
01c70e9
Compare
aasandei-vsp
changed the title
@slifty That makes perfect sense! Because I was deep into this story, I forgot that other people need to understand the context :))) Together with @cecilia-donnelly , we have decided that making smaller PRs if possible, for the ease of review would be better, so that's why the big story of migrating to getWithChildren has been split across multiple subtasks. I've updated the commit message and the description. Hope this helps, otherwise I can definitely provide more details. |
There was a problem hiding this comment.
Pull request overview
Groundwork for migrating folder navigation from legacy v1 endpoints (/folder/navigateLean / /folder/navigateMin) to the Stela-backed getWithChildren by expanding Stela→VO field mapping, adding a legacy folderId fallback, and aligning success/error shapes with existing v1 consumers.
Changes:
- Expanded Stela→
FolderVOmapping (archive/link IDs, breadcrumb path arrays, created/updated timestamps) and added an item-levelaccessRoledefault. - Added
resolveFolderIdfallback via legacy/folder/get, then fetches folder + children in parallel. - Added/updated unit tests for the new mapping, fallback behavior, and v1-shaped error handling.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/app/shared/services/api/record.repo.ts |
Sets a default accessRole during Stela→RecordVO conversion to preserve existing UI permission gates. |
src/app/shared/services/api/folder.repo.ts |
Extends Stela folder shape + mapping, adds folderId resolution fallback, and wraps errors into a v1-like FolderResponse. |
src/app/shared/services/api/folder.repo.spec.ts |
Adds tests covering error shaping, folderId resolution fallback, and newly-mapped fields used by breadcrumbs/metadata. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // 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 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', |
| const response = await this.get([folderVO]); | ||
| const resolvedFolder = response.getFolderVO(); | ||
| return new FolderVO({ ...folderVO, folderId: resolvedFolder.folderId }); |
| 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 |
Issue: PER-10476
Subtask: https://permanent.atlassian.net/browse/PER-10677
Why I'm doing this
I'm migrating our folder navigation off the v1 endpoints
(/folder/navigateLean and /folder/navigateMin) and onto getWithChildren,
which is backed by Stela. getWithChildren reshapes its response to match
v1's
Results/error shape on purpose, so I can move call sites overgradually rather than rewriting every consumer in one go.
Heads up: this PR is groundwork, not the switchover. It closes the gaps
between what Stela gives back and what the current navigate consumers
rely on, so getWithChildren is closer to being a drop-in replacement. I
haven't repointed any call sites here.
What's in it
conversion: archiveNumber, folderLinkId, parentFolderLinkId, the
breadcrumb path arrays, and the created/updated timestamps.
fetch folder + children in parallel.
handlers keep working.
backend removed the item-level field, and without it permission gates
like the sidebar share button treated items as inaccessible).
What's not in it
The tests are only to make sure we didn't make any regressions, as it does not bring any new functionality.
Manual test cases — Stela conversion in getWithChildren and record mapping
Setup: log in on an archive that has folders with images, records with metadata, and at least one unlisted share link.
EXPECTED: private/public navigation is unaffected by this PR (still uses
navigateLean) — only the flows below need testing.Unlisted share link — folder
Unlisted share link — error handling
Sidebar — record
accessRole, leaving the button permanently disabled.)Sidebar — folder
Record and folder edits (v2 endpoints)