Skip to content

[PER-10476] [1] Add missing properties and create fallbacks to enable migration from navigateLean to getWithChildren - #996

Open
aasandei-vsp wants to merge 1 commit into
mainfrom
PER-10476-replace-navigate-lean
Open

[PER-10476] [1] Add missing properties and create fallbacks to enable migration from navigateLean to getWithChildren#996
aasandei-vsp wants to merge 1 commit into
mainfrom
PER-10476-replace-navigate-lean

Conversation

@aasandei-vsp

@aasandei-vsp aasandei-vsp commented Apr 21, 2026

Copy link
Copy Markdown
Contributor

Issue: PER-10476
Subtask: https://permanent.atlassian.net/browse/PER-10677

The tests are only to make sure we didn't make any regressions, as it does not bring any new functionality.

Manual test cases

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

  1. Open an unlisted share link to a folder in an incognito window.
    • EXPECTED: The folder contents load correctly.
  2. Navigate into a subfolder, then go back up using the breadcrumbs.
    • EXPECTED: Breadcrumb labels are correct and each breadcrumb link navigates to the right folder.
  3. Open the same share link while logged in.
    • EXPECTED: The folder contents load correctly.

Unlisted share link — error handling

  1. Open an invalid or revoked unlisted share link.
    • EXPECTED: A graceful error message is shown — no blank page, no errors in the console.

Sidebar — record

  1. Select a record and wait for the sidebar to finish loading.
    • EXPECTED: Full metadata is shown (dates, description, tags, location).
  2. Click the Share button.
    • EXPECTED: The button is enabled and the sharing dialog opens.

Sidebar — folder

  1. Select a folder and wait for the sidebar to finish loading.
    • EXPECTED: Full metadata is shown and the Share button is enabled.

Record and folder edits (v2 endpoints)

  1. Edit a record's date/time via the edit modal.
    • EXPECTED: The value saves and redisplays correctly after the sidebar refreshes.
  2. Edit a folder's date.
    • EXPECTED: The value saves and redisplays correctly.

Record and folder shared with me

  • EXPECTED: The shared workspace has the top buttons disabled(Delete, Copy, Share...)
  1. Select a record that has been shared with me as viewer
    • EXPECTED: The sidebar metadata is not editable
  2. Select a folder that has been shared with me and navigate through it
    • EXPECTED: All folders and files inside it have the same accessRole as the parent.

@aasandei-vsp aasandei-vsp self-assigned this Apr 21, 2026
@aasandei-vsp
aasandei-vsp marked this pull request as draft April 21, 2026 13:13
@codecov

codecov Bot commented Apr 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 52.39%. Comparing base (8c2bb85) to head (a366ced).
⚠️ Report is 10 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #996      +/-   ##
==========================================
+ Coverage   52.26%   52.39%   +0.13%     
==========================================
  Files         354      354              
  Lines       12093    12105      +12     
  Branches     2186     2189       +3     
==========================================
+ Hits         6320     6343      +23     
+ Misses       5550     5536      -14     
- Partials      223      226       +3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@aasandei-vsp aasandei-vsp changed the title [WIP] [PER-10476] Replace navigateLean with getWithChildren [WIP] [PER-10476] [BLOCKED BY BE] Replace navigateLean with getWithChildren Jun 4, 2026
@aasandei-vsp
aasandei-vsp force-pushed the PER-10476-replace-navigate-lean branch from b65fcfb to bb0a021 Compare July 8, 2026 13:30
@aasandei-vsp
aasandei-vsp force-pushed the PER-10476-replace-navigate-lean branch from bb0a021 to e54b14f Compare July 16, 2026 08:56
@aasandei-vsp aasandei-vsp changed the title [WIP] [PER-10476] [BLOCKED BY BE] Replace navigateLean with getWithChildren [PER-10476] Extend Stela conversion in getWithChildren and record mapping Jul 16, 2026
@aasandei-vsp
aasandei-vsp marked this pull request as ready for review July 16, 2026 09:08
@aasandei-vsp aasandei-vsp changed the title [PER-10476] Extend Stela conversion in getWithChildren and record mapping [PER-10476] [1] Extend Stela conversion in getWithChildren and record mapping Jul 23, 2026
@slifty

slifty commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

@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 Migrate record lookup to use stela, right? Or maybe it's just a partial implementation of that overall epic?

@aasandei-vsp
aasandei-vsp force-pushed the PER-10476-replace-navigate-lean branch 2 times, most recently from 45f656e to 01c70e9 Compare July 28, 2026 10:55
@aasandei-vsp aasandei-vsp changed the title [PER-10476] [1] Extend Stela conversion in getWithChildren and record mapping [PER-10476] [1] Extend getWithChildren toward replacing navigateLean/navigateMin (Stela migration groundwork) Jul 28, 2026
@aasandei-vsp

Copy link
Copy Markdown
Contributor Author

@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 Migrate record lookup to use stela, right? Or maybe it's just a partial implementation of that overall epic?

@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.
TL;DR Basically there were some properties missing from the stela record and folder that were stopping the full migration to getWithChildren. After @liam-lloyd added them on BE, I've also made the mapping on the FE.

Hope this helps, otherwise I can definitely provide more details.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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→FolderVO mapping (archive/link IDs, breadcrumb path arrays, created/updated timestamps) and added an item-level accessRole default.
  • Added resolveFolderId fallback 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.

Comment thread src/app/shared/services/api/record.repo.ts Outdated
Comment on lines +154 to +156
// 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',
Comment thread src/app/shared/services/api/folder.repo.ts
Comment thread src/app/shared/services/api/folder.repo.ts Outdated
@aasandei-vsp
aasandei-vsp force-pushed the PER-10476-replace-navigate-lean branch from 01c70e9 to 65dc646 Compare August 4, 2026 08:44
@aasandei-vsp aasandei-vsp changed the title [PER-10476] [1] Extend getWithChildren toward replacing navigateLean/navigateMin (Stela migration groundwork) [PER-10476] [1] Add missing properties to enable migration from navigateLean to getWithChildren Aug 4, 2026
@aasandei-vsp aasandei-vsp changed the title [PER-10476] [1] Add missing properties to enable migration from navigateLean to getWithChildren [PER-10476] [1] Add missing properties and create fallbacks to enable migration from navigateLean to getWithChildren Aug 4, 2026
@aasandei-vsp
aasandei-vsp force-pushed the PER-10476-replace-navigate-lean branch from 65dc646 to b13e8fb Compare August 4, 2026 09:36
Added accessRole, archiveNumber, folderLinkId, parentFolder.parentFolderLinkId,
createdAt, updatedAt, paths.folderLinkIds, paths.archiveNumbers in StelaFolder
and StelaRecord so navigateLean and navigateMin can be fully replaced by
getWithChildren.

Created fallback for missing folderId, call folder and children in parallel
for optimization and convert v2 call failure to v1 unsuccessful response.

Issue: PER-10476
@aasandei-vsp
aasandei-vsp force-pushed the PER-10476-replace-navigate-lean branch from b13e8fb to a366ced Compare August 4, 2026 09:51
@aasandei-vsp

Copy link
Copy Markdown
Contributor Author

@slifty I am pausing the work on this to pick up some more urgent tasks. I will let you know when it's ready for review again. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants