[PER-10107] Make preview show for unlisted shares #727

aasandei-vsp · 2025-09-18T12:11:11Z

This is the main story tackled here: https://permanent.atlassian.net/browse/PER-10107
This story was more of a collateral need and has been done in this PR as well: https://permanent.atlassian.net/browse/PER-10306

STEPS TO TEST
OBSERVATION: Unlisted share links will be provided, because as of now we do not have a way of creating unlisted shares from the UI.
Record as unlisted share:

Create a new account or log in to existing account
Paste in the browser the link: https://dev.permanent.org/share/1e258bb5a7570930cb3f6bbd033db44a19aca23aefdddd5d9d3120498e9fe161
The preview of the record will appear
Click on the record thumbnail
The modal with details will appear, click on the download button/resize buttons
Click the back button

Video for reference(can only be viewed in Chrome):
record-unlisted-share-preview-on.webm

Folder as unlisted share:

Create a new account or log in to existing account
Paste in the browser the link: https://dev.permanent.org/share/79b30dfc326662ebba22146b131b4dc7769cc7629344fe248113c53d844dc4e9
The preview of the folder will appear, thumbnails for all the records
Click on the first record thumbnail
The modal with details will appear, click on the download button/resize buttons
Click on the forward and back arrows from the image in order to navigate through all the files in the folder
Click the back button

Video for reference(can only be viewed in Chrome):
folder-unlisted-share-preview-on.webm

Expired unlisted share:

Create a new account or log in to existing account
Paste in the browser the link: https://dev.permanent.org/share/a99ca24389c92c7379e78ab7e2318845a0e4bd42b1995e78f74ac37b79dbfd0f
A screen with the error message will appear stating that the link is wrong or expired

Video for reference(can only be viewed in Chrome):
expired-link.webm

EXTRA TESTS FOR EXISTING SHARE LINK BEHAVIOUR
This tests would be useful to make sure that while developing this new feature, we did not introduce any new bugs or created regressions.

Auto-approve share link:
1 .Create account or sign in to account A
2. Upload a file
3. Create a share link for the uploaded file
4. Copy the link
5. Open a new browser instance
6. Create or sign in to a different account B
8. Paste the link in the browser where the account B is signed in
9. The share link preview will appear, with the "Accept Share" button at the bottom of the screen

Video for reference(can only be viewed in Chrome):
auto-approve-share-link.webm

Request access share link:
1 .Create account or sign in to account A
2. Upload a file
3. Create a share link for the uploaded file
4. Copy the link
5. Open a new browser instance
6. Create or sign in to a different account B
8. Paste the link in the browser where the account B is signed in
9. The share link preview will appear, with the "Request Access" button at the bottom of the screen

Video for reference(can only be viewed in Chrome):
request-access-share-link.webm

Try to access share link with no account:
1 .Create account or sign in to account A
2. Upload a file
3. Create a share link for the uploaded file
4. Copy the link
5. Open a new browser instance in incognito mode
8. Paste the link in the browser
9. The share link preview will appear, with an overlay and a modal asking you to create an account or log in

Video for reference(can only be viewed in Chrome):
no-account-share-link.webm

slifty · 2025-09-24T21:09:43Z

Should this use the get record (by stela) that we recently added here: #669

aasandei-vsp · 2025-09-29T14:46:12Z

This story has the following dependencies, these 2 changes need to go before the preview of unlisted share:

Creation of unlisted share(maybe hide it under the feature flag and activate it when the preview will also be available)
GitHub - PermanentOrg/web-app at PER-10298-create-unlisted-share

Getting record/folder information using the share token for authentication
Update getWithChildren to use stela by slifty · Pull Request #720 · PermanentOrg/web-app

codecov · 2025-10-01T10:41:40Z

Codecov Report

❌ Patch coverage is 60.75949% with 31 lines in your changes missing coverage. Please review.
✅ Project coverage is 46.46%. Comparing base (1be2606) to head (1e5da12).
⚠️ Report is 3 commits behind head on main.

Files with missing lines	Patch %	Lines
src/app/shared/services/api/folder.repo.ts	0.00%	12 Missing ⚠️
...er/components/file-viewer/file-viewer.component.ts	57.89%	6 Missing and 2 partials ⚠️
...ponents/file-list-item/file-list-item.component.ts	50.00%	6 Missing ⚠️
...omponents/share-preview/share-preview.component.ts	66.66%	3 Missing ⚠️
src/app/shared/services/api/record.repo.ts	50.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #727      +/-   ##
==========================================
+ Coverage   45.12%   46.46%   +1.33%     
==========================================
  Files         370      371       +1     
  Lines       11314    11372      +58     
  Branches     1867     1885      +18     
==========================================
+ Hits         5106     5284     +178     
+ Misses       6033     5910     -123     
- Partials      175      178       +3

☔ 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.

aasandei-vsp · 2025-10-01T10:44:48Z

Please ignore files:

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

src/app/shared/services/api/record.repo.ts

They are being thoroughly reviewed in PR: #720, my changes are minimal. Not the best choice, maybe I should have rebased on that branch instead of copy/pasting the files, but I wanted to make sure I'm up to date with main.

@slifty @cecilia-donnelly @liam-lloyd Just FYI.

cecilia-donnelly

This is exciting! A few thoughts:

Can you write an additional comment (for QA) that describes what should change as the result of this PR? It might also be good for us to create a few unlisted shares (one expired, say, and one with preview on, one with preview off...that kind of thing) on dev.permanent.org for QA purposes.
General note: we generally don't have a separate "linting fixes" commit. I like how you've separated out the steps here! In the future when you're creating your commits, try to have each one pass linting separately (that lets reviewers look at them one by one).
I don't think this should need to wait for unlisted share creation, since it would only be visible if someone did create an unlisted share. Am I missing something? I'd like to get it merged if we can.

This is a big change! When I first went through I was surprised how many pieces were involved. I think I understand it more now - it seems like a lot of it is for an unlisted share of a folder specifically. Is that right? If so, we should include that in the test plan (mainly what I've referenced in (1), above).

src/app/share-links/services/share-links-api.service.ts

src/app/filesystem/filesystem-api.service.spec.ts

cecilia-donnelly · 2025-10-03T19:13:12Z

src/app/file-browser/components/file-list/file-list.component.ts

+		const isUnlistedShare = await this.shareLinksService.isUnlistedShare();
+		if (isUnlistedShare) {
+			itemsToFetch = [];
+		}


I'm not entirely sure why we need to check for this in this component, though that's probably my lack of knowledge of the web-app. I expected this component to only be used inside a workspace (that is, the part of the app that a member sees when logged in.) Can you explain?

This is a great catch! We do not need this code anymore, it was for testing purposes. Before using stela to get records, I had to bypass this call in order to test the preview for unlisted shares.

In all honesty, it might actually be redundant in general to make this BE call here, but I did not investigate this enough to make sure that removing it wouldn't break other flows in the app.

cecilia-donnelly · 2025-10-03T19:15:24Z

src/app/file-browser/components/file-viewer/file-viewer.component.ts

-				this.currentRecord.accessRole,
-				AccessRole.Editor,
-			) && !route.snapshot.data?.isPublicArchive;
+		this.canEdit = this.isUnlistedShare


canEdit might be misleading here, but viewers of an unlisted share can't edit, so I wouldn't expect to have to check that value for editing at all.

This file viewer component is the one that loads when you double-click on a record(the big picture with the details and download button appears). So I have reused it for the unlisted share as well. When you double click on a record thumbnail, the same view loads, but the difference is you cannot edit the details of it, you can only download if you want.

This is the component, when we use it in a private folder to view our records, which we can edit(the link in the browser shows it is from private):

And this is the same component, when we view an unlisted share, but we cannot click on anything but the download button(the link in the browser shows it's a share):

So I do believe the canEdit property makes sense, but let me know your opinion on it.

cecilia-donnelly · 2025-10-03T19:44:24Z

One specific aspect of "preview" is whether this change should include being able to view a record full size in an unlisted share. That is something we need but might be coming in a future PR (for me, the record is still thumbnail size when I open an unlisted share on this branch).

aasandei-vsp · 2025-10-06T11:51:59Z

One specific aspect of "preview" is whether this change should include being able to view a record full size in an unlisted share. That is something we need but might be coming in a future PR (for me, the record is still thumbnail size when I open an unlisted share on this branch).

Maybe I didn't understand the question right or maybe there are some issues locally, but this is the behavior I implemented(just to mention, the details preview opens up at double click):

Single record unlisted share:

record-details-preview.webm

Folder with multiple files unlisted share
folder-details-preview.webm
Folder with multiple files and folders unlisted share
WORK IN PROGRESS

aasandei-vsp · 2025-10-06T12:35:42Z

This is exciting! A few thoughts:

Can you write an additional comment (for QA) that describes what should change as the result of this PR? It might also be good for us to create a few unlisted shares (one expired, say, and one with preview on, one with preview off...that kind of thing) on dev.permanent.org for QA purposes.

General note: we generally don't have a separate "linting fixes" commit. I like how you've separated out the steps here! In the future when you're creating your commits, try to have each one pass linting separately (that lets reviewers look at them one by one).

I don't think this should need to wait for unlisted share creation, since it would only be visible if someone did create an unlisted share. Am I missing something? I'd like to get it merged if we can.

This is a big change! When I first went through I was surprised how many pieces were involved. I think I understand it more now - it seems like a lot of it is for an unlisted share of a folder specifically. Is that right? If so, we should include that in the test plan (mainly what I've referenced in (1), above).

Sure, I'll add a different comment focused on the details of what should be tested. I think adding some unlisted shares on dev is a very good idea, but I think I need help with that. Maybe you @cecilia-donnelly or @slifty could help me with it?
Sorry for those commits with only linting errors, that was my mistake. But I think I started to get used to running the scripts before every commit and I'll definitely pay more attention to it. In my opinion, reviewing commits one by one might be a bit difficult, because at least in my case, sometimes work is not perfectly separated from one commit to another and things can get a bit confusing. Especially because every step of the way I get to see things from a broader perspective and I try to refactor and improve as I go.
We could merge, but I think there are two things that are a must in case we want to do that:

This PR needs to be merged, as the detailed preview depends on data from those stela endpoints: Update getWithChildren to use stela #720
I don't think merging this to production without testing locally the whole flow, would be the ideal way to do it, but we could just hide it under the unlisted-share feature flag. Would that work?

The change is for both records and folders as unlisted shares, but the code might be a bit misleading, because a lot of components are tied together and contain more logic than they should.
For example, how the existing code works for shares:

in order to show just one record, you still have to put that record into a list, go through the file-list-component, get to the file-list-item component that will finally render it.
the same components are used to show the list of records that are rendered as rows(e.g. private folder) and as thumbnails(e.g. share link preview). Sometimes this could be useful, but in this case, it creates a lot of ifs and cumbersome css that would be better separated in different components.

cecilia-donnelly · 2025-10-07T21:02:05Z

Thank you so much @aasandei-vsp! I was able to see that this is working for me locally - I just didn't double click initially, silly. @omnignorant, should we be able to view a record at full size with a single click or is double click fine?

Two requests:

Now that the folder PR is merged, can you rebase?
Unlisted shares don't have options for preview and auto approve, so we don't need to specifically test the preview for an unlisted share.

Here are the links for QA on dev:

Record unlisted share: https://dev.permanent.org/share/1e258bb5a7570930cb3f6bbd033db44a19aca23aefdddd5d9d3120498e9fe161
Folder unlisted share: https://dev.permanent.org/share/79b30dfc326662ebba22146b131b4dc7769cc7629344fe248113c53d844dc4e9
Expired unlisted share: https://dev.permanent.org/share/a99ca24389c92c7379e78ab7e2318845a0e4bd42b1995e78f74ac37b79dbfd0f

cecilia-donnelly

This seems right to me! I would appreciate an additional review from @slifty since you've been deep in the folder/record world lately.

Thank you for your hard work, @aasandei-vsp ! One note for QA-ers is that this worked for me in a private (Firefox) window but not my main window (even when logged out) so there might be some caching/data weirdness to look out for.

cecilia-donnelly · 2025-10-08T21:06:21Z

src/app/file-browser/components/file-list-item/file-list-item.component.html

 <div
 	class="file-list-item"
 	(click)="onItemClick($event)"
+	(dblclick)="showUnlistedPreview($event)"


I would prefer if a single click opened the item full size, but maybe we can't do that because we're already using click for an event here. How hard would that be?

It was a very easy change, so I've done it and rebased. Ready for review and test again!

omnignorant

Everything checks out for me! The unlisted share links are working and so are the restricted ones!

aasandei-vsp self-assigned this Sep 18, 2025

aasandei-vsp marked this pull request as draft September 18, 2025 12:11

aasandei-vsp force-pushed the PER-10107-implement-unlisted-share-page-ui branch 2 times, most recently from d8cb7d9 to d12d5b3 Compare October 1, 2025 10:39

aasandei-vsp requested review from slifty, liam-lloyd and cecilia-donnelly October 1, 2025 10:47

aasandei-vsp marked this pull request as ready for review October 1, 2025 10:47

aasandei-vsp mentioned this pull request Oct 2, 2025

[PER-10311] Implement unlisted share page UI for users without an account #741

Draft

cecilia-donnelly reviewed Oct 3, 2025

View reviewed changes

aasandei-vsp force-pushed the PER-10107-implement-unlisted-share-page-ui branch from 9868552 to 248aade Compare October 6, 2025 09:11

cecilia-donnelly requested a review from omnignorant October 7, 2025 21:03

aasandei-vsp force-pushed the PER-10107-implement-unlisted-share-page-ui branch from 95a3e19 to bf311b1 Compare October 8, 2025 13:25

cecilia-donnelly approved these changes Oct 8, 2025

View reviewed changes

cecilia-donnelly removed the request for review from liam-lloyd October 8, 2025 21:16

aasandei-vsp added 7 commits October 9, 2025 11:59

[PER-10107] Make preview show for unlisted shares

de5b386

[PER-10107] Call get share links by token

a40592b

[PER-10107] Add call to get record stela

8358187

[PER-10107] Add details for preview record

a2aedea

[PER-10107] Fix linting and prettier

eef28e0

[PER-10107] Add/Update unit tests

2794f2e

[PER-10107] Disabled detailed preview when preview toggle is off

9b358de

aasandei-vsp added 6 commits October 9, 2025 11:59

[PER-10107] Fix test for navigate method in filesystem-api service

81fb81e

[PER-10107] Fix flaky tests and remove unused dependencies

4f9da40

[PER-10107] Fix racing overriding condition

b6bdbe1

[PER-10107] Remove unused code

9cdf8d1

[PER-10107] Fixed record call to get with token

4d08006

[PER-10107] Open preview on one click

1e5da12

aasandei-vsp force-pushed the PER-10107-implement-unlisted-share-page-ui branch from 8b8dc4a to 1e5da12 Compare October 9, 2025 09:03

omnignorant approved these changes Oct 9, 2025

View reviewed changes

[PER-10107] Make preview show for unlisted shares #727

Are you sure you want to change the base?

[PER-10107] Make preview show for unlisted shares #727

Uh oh!

Conversation

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

Uh oh!

Uh oh!

slifty commented Sep 24, 2025

Uh oh!

aasandei-vsp commented Sep 29, 2025

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

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

Uh oh!

Uh oh!

cecilia-donnelly left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cecilia-donnelly Oct 3, 2025

Choose a reason for hiding this comment

Uh oh!

aasandei-vsp Oct 8, 2025

Choose a reason for hiding this comment

Uh oh!

cecilia-donnelly Oct 3, 2025

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cecilia-donnelly commented Oct 3, 2025

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

cecilia-donnelly commented Oct 7, 2025

Uh oh!

cecilia-donnelly left a comment

Choose a reason for hiding this comment

Uh oh!

cecilia-donnelly Oct 8, 2025

Choose a reason for hiding this comment

Uh oh!

aasandei-vsp Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

omnignorant left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

aasandei-vsp commented Sep 18, 2025 •

edited

Loading

codecov bot commented Oct 1, 2025 •

edited

Loading

aasandei-vsp commented Oct 1, 2025 •

edited

Loading

aasandei-vsp Oct 6, 2025 •

edited

Loading

aasandei-vsp commented Oct 6, 2025 •

edited

Loading

aasandei-vsp commented Oct 6, 2025 •

edited

Loading