[PER-10357] Add fallback for stela folder calls #836

aasandei-vsp · 2025-11-24T13:08:08Z

Calls for folders and records on stela can be made with two types of authentication:

auth token
share token

When making a call with the share token, if that folder/record is not unrestricted, the endpoint will return an empty list. When this happens, if the user is logged in, we try to make the call for the folder/record again, using the auth token, because maybe the user does have access to it and we can still get it.

Issue: https://permanent.atlassian.net/browse/PER-10357

Linked issues:

HOW TO TEST:

Test for folder fallback: "My Archive" button

Create an account or use an existing one
Create an unlisted share
Open a new window and log in to a different account
Paste the unlisted share link
Click the "My Archive" button
EXPECTATION: Should be redirected to the logged in user's current archive

Test for properties check bug: "View in my archive" button

Create an account or use an existing one
Create a restricted share
Open a new window and log in to a different account
Paste the restricted share link
Accept the share
Paste the restricted share link again in the window
Click the "View in my archive" button
EXPECTATION: Should be redirected to the logged in user's current archive

Test for record fallback:
I did not find a place to test this, so I just faked an empty response from the code to make sure it works.

codecov · 2025-11-24T13:09:51Z

Codecov Report

❌ Patch coverage is 70.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 47.10%. Comparing base (518b4c5) to head (ada626c).
⚠️ Report is 5 commits behind head on main.

Files with missing lines	Patch %	Lines
src/app/shared/services/api/record.repo.ts	25.00%	1 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #836      +/-   ##
==========================================
+ Coverage   47.02%   47.10%   +0.07%     
==========================================
  Files         372      372              
  Lines       11495    11496       +1     
  Branches     1909     1912       +3     
==========================================
+ Hits         5406     5415       +9     
+ Misses       5900     5898       -2     
+ Partials      189      183       -6

☔ View full report in Codecov by Sentry.
📢 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.

cecilia-donnelly

I don't think we should make properties optional to make tests pass - it seems to me that the test setup should match reality. It feels much better to me to include the location and files in the fakeRecordVO` than to adjust the record model to not always have those properties.

One housekeeping note is that the first two commit messages are the same, which confused me a little. I love the overall organization though!

src/app/shared/services/api/base.ts

src/app/shared/services/api/folder.repo.spec.ts

aasandei-vsp · 2025-11-27T12:41:10Z

I don't think we should make properties optional to make tests pass - it seems to me that the test setup should match reality. It feels much better to me to include the location and files in the fakeRecordVO` than to adjust the record model to not always have those properties.

That is indeed a very good point, properties should not be optional just to make a test pass. The reason I added some checks in the mappings for location, files, thumbnailUrls and paths(convertStelaFolderToFolderVO, convertStelaRecordToRecordVO, convertStelaLocationToLocnVOData) is to make sure that even if they do not come from BE, we do not block the whole flow with FE errors. The reasons the buttons "My Archive" and "View in my archive" were not working were:

We did not have the fallback, so we would get an empty object for folder or record when we tried to use the share token for authentication.
We were trying to map an empty folder/record, so accessing properties from null/undefined threw errors that were blocking the FE.

Even though we should make sure those properties are defined from the BE, if it happens that they are not there for some reason, I believe it is better to show an empty field or some inconsistent value than blocking the whole interface.

One housekeeping note is that the first two commit messages are the same, which confused me a little. I love the overall organization though!

They are the same, indeed 🫣 the only difference is that one says folder, the other one record. I chose to do this because it adds the same fallback functionality in both places😅. I am very glad that you like the organization, I also believe it looks so much cleaner and structured having distinctive commits with good descriptions. Big thanks to @slifty for being patient and helping me get here with it!

cecilia-donnelly · 2025-12-02T18:52:33Z

I believe it is better to show an empty field or some inconsistent value than blocking the whole interface.

This is a good point! I worry a bit about unintended/unexpected behavior if we don't have, say, a file. I can imagine what would happen with no thumbnails or location, though.

the only difference is that one says folder, the other one record

I missed that! 🤦‍♀️ Thanks :)

cecilia-donnelly

This might be something local for me, but I didn't get the result I expected from the first test case.

I did the following:

Create an unrestricted share link with Account A
Log in to Account B in a private window
Open the link as Account B
Click the archive icon at the top right.

I expected to go to Account B's archive, but nothing happened.

aasandei-vsp · 2025-12-03T12:49:40Z

I expected to go to Account B's archive, but nothing happened.

I have debugged the code and it seems the implementation and functionality are a bit different than expected. I have made a video to showcase it:

https://www.loom.com/share/c99b6b7fbde44627be56a7592012510b

But there is indeed a bug. If we only have one archive, we do not open the modal, so the user might be confused on how this icon click actually works, because it behaves inconsistently. Maybe open a ticket to make this flow more user friendly?

omnignorant

huzzah! this appears to work correctly now based on the test cases provided.

Calls for folders on stela can be made with two types of authentication: - auth token - share token When making a call with the share token, if that folder is not unrestricted, the endpoint will return an empty list. When this happens, if the user is logged in, we try to make the call for the folder again, using the auth token, because maybe the user does have access to it and we can still get it. Issue: PER-10357 Fallback for share token

Calls for records on stela can be made with two types of authentication: - auth token - share token When making a call with the share token, if that record is not unrestricted, the endpoint will return an empty list. When this happens, if the user is logged in, we try to make the call for the record again, using the auth token, because maybe the user does have access to it and we can still get it. Issue: PER-10357 Fallback for share token

The method fetchFullItems uses both the record.repo and the folder.repo and the data service test suite does not create mocks for them. Because the methods api.folder.getWithChildren and api.record.get have changed, the test for the fetchFullItems fails, even with the timeout. At the moment, the best solution would be to disable this test, to avoid it from failing on future changes in the dependencies. Issue: PER-10357 Fallback for share token

The properties from stela for record(location, files) and for folder (location, thumbnailUrls, paths) did not have a check for null or undefined and if those properties are not present for some reason in the response, an error is generated that will block the whole flow in the browser. Issue: PER-10357 Fallback for share token

aasandei-vsp self-assigned this Nov 24, 2025

aasandei-vsp marked this pull request as draft November 24, 2025 13:09

aasandei-vsp force-pushed the PER-10357-fallback-for-share-token branch from 8ce47af to 8120567 Compare November 25, 2025 17:10

aasandei-vsp marked this pull request as ready for review November 25, 2025 17:10

aasandei-vsp requested review from cecilia-donnelly and slifty November 25, 2025 17:14

cecilia-donnelly requested changes Nov 26, 2025

View reviewed changes

src/app/shared/services/api/base.ts Show resolved Hide resolved

src/app/shared/services/api/folder.repo.spec.ts Show resolved Hide resolved

cecilia-donnelly self-requested a review December 2, 2025 17:28

cecilia-donnelly requested changes Dec 2, 2025

View reviewed changes

aasandei-vsp force-pushed the PER-10357-fallback-for-share-token branch from 8120567 to 6204ad0 Compare December 3, 2025 13:00

cecilia-donnelly self-requested a review December 8, 2025 20:34

cecilia-donnelly approved these changes Dec 9, 2025

View reviewed changes

aasandei-vsp requested a review from omnignorant December 11, 2025 06:47

This was referenced Dec 11, 2025

[PER-10371] Unlisted share record preview fixes #855

Open

[PER-10370] Archive icon should redirect to account workspace #858

Open

cecilia-donnelly force-pushed the PER-10357-fallback-for-share-token branch from 6204ad0 to ab652c2 Compare December 12, 2025 20:09

omnignorant approved these changes Dec 12, 2025

View reviewed changes

aasandei-vsp added 4 commits December 15, 2025 11:15

aasandei-vsp force-pushed the PER-10357-fallback-for-share-token branch from ab652c2 to ada626c Compare December 15, 2025 09:56

aasandei-vsp merged commit c59e5ca into main Dec 15, 2025
8 checks passed

aasandei-vsp deleted the PER-10357-fallback-for-share-token branch December 15, 2025 10:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[PER-10357] Add fallback for stela folder calls #836

[PER-10357] Add fallback for stela folder calls #836

Uh oh!

aasandei-vsp commented Nov 24, 2025 •

edited

Loading

Uh oh!

codecov bot commented Nov 24, 2025 •

edited

Loading

Uh oh!

cecilia-donnelly left a comment

Uh oh!

Uh oh!

Uh oh!

aasandei-vsp commented Nov 27, 2025

Uh oh!

cecilia-donnelly commented Dec 2, 2025

Uh oh!

cecilia-donnelly left a comment

Uh oh!

aasandei-vsp commented Dec 3, 2025 •

edited

Loading

Uh oh!

omnignorant left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

[PER-10357] Add fallback for stela folder calls #836

[PER-10357] Add fallback for stela folder calls #836

Uh oh!

Conversation

aasandei-vsp commented Nov 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Nov 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

cecilia-donnelly left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

aasandei-vsp commented Nov 27, 2025

Uh oh!

cecilia-donnelly commented Dec 2, 2025

Uh oh!

cecilia-donnelly left a comment

Choose a reason for hiding this comment

Uh oh!

aasandei-vsp commented Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

omnignorant left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

aasandei-vsp commented Nov 24, 2025 •

edited

Loading

codecov bot commented Nov 24, 2025 •

edited

Loading

aasandei-vsp commented Dec 3, 2025 •

edited

Loading