Skip to content

Conversation

@aasandei-vsp
Copy link
Contributor

@aasandei-vsp aasandei-vsp commented Nov 4, 2025

STEPS TO TEST UNLISTED SHARES FLOW

Anonymous user:

  1. Create an account or use an existing one and log in

  2. Create or use a folder that contains other folders and files, inside the folders, create more folders, so you can have a tree structure like of folders and files.

  3. Create share link for the "root" folder, the one you created first and make sure the link type is "Anyone can view"

  4. Open a tab in incognito mode and paste the generated link

  5. Go through folders by single clicking them
    EXPECTED: Being able to navigate forward inside the folder structure.

  6. In a folder with multiple files, click on a file

  7. Use the arrows to go through all the files in the folder

  8. Download one of the files

  9. Click the back arrow
    EXPECTED: It should get back to the containing folder

  10. After you have clicked several folders, click a folder name in the breadcrumbs.
    EXPECTED: The breadcrumbs cut to that specific folder and you get the see the folder/files inside it

Signed in user:
Repeat the steps above, only 4 changes:
4. Open a new tab and log in with a different user and then paste the generated link

STEPS TO TEST EXISTING SHARES FLOW:
Anonymous user:

  1. Create an account or use an existing one and log in
  2. Create or use a folder that contains other folders and files, inside the folders, create more folders, so you can have a tree structure like of folders and files.
  3. Create share link for the "root" folder, the one you created first and make sure the link type is "Restricted"
  4. Open a tab in incognito mode and paste the generated link
    EXPECTED: "Create account" modal should appear

Signed in user:
Repeat steps 1 through 3, then:
4. Open a new tab and log in with a different user and then paste the generated link
EXPECTED: "Accept share" sticky footer should appear.

OTHER FLOWS THAT HAVE BEEN AFFECTED BY THIS CHANGE
Private files/Shared files/Public files/Public gallery

  1. Create an account or use an existing one and log in
  2. Create or use a folder(in one of the sections mentioned above) that contains other folders and files, inside the folders, create more folders, so you can have a tree structure like of folders and files.
  3. Click through the folder structure
  4. Click on files to preview, try to navigate using the arrows, edit details, download

@codecov
Copy link

codecov bot commented Nov 4, 2025

Codecov Report

❌ Patch coverage is 51.85185% with 39 lines in your changes missing coverage. Please review.
✅ Project coverage is 47.05%. Comparing base (348a823) to head (7339c56).
⚠️ Report is 21 commits behind head on main.

Files with missing lines Patch % Lines
...omponents/share-preview/share-preview.component.ts 40.90% 12 Missing and 1 partial ⚠️
...rowser/components/file-list/file-list.component.ts 58.62% 12 Missing ⚠️
...red/components/breadcrumbs/breadcrumb.component.ts 0.00% 5 Missing ⚠️
src/app/shared/services/data/data.service.ts 44.44% 5 Missing ⚠️
...ponents/file-list-item/file-list-item.component.ts 0.00% 3 Missing ⚠️
...ed/components/breadcrumbs/breadcrumbs.component.ts 90.90% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #805      +/-   ##
==========================================
+ Coverage   46.93%   47.05%   +0.11%     
==========================================
  Files         372      372              
  Lines       11444    11494      +50     
  Branches     1896     1907      +11     
==========================================
+ Hits         5371     5408      +37     
- Misses       5885     5898      +13     
  Partials      188      188              

☔ 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 aasandei-vsp self-assigned this Nov 5, 2025
@aasandei-vsp aasandei-vsp requested a review from slifty November 5, 2025 14:06
Copy link
Contributor

@slifty slifty left a comment

Choose a reason for hiding this comment

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

This does all look great and thank you for the easy to review commits!

Some notes about commit messages though, inspired by the great guidance of https://cbea.ms/git-commit/:

  1. These title lines are all are quite long (and wrap to the body) -- ideally subject is just 50 characters
  2. The issue number is in the subject, but we might want to consider putting this one line after the body <-- you aren't alone in doing this, but saves space for the subject line for content, while keeping the issue context in the commit overall.
  3. There is no body, for the most part (what goes in the body is a bit of an art, but generally explaining the "why does this commit exist" is the point.

So for instance you might consider a message like this

Add breadcrumbs for unlisted shares

Some unlisted shares have a folder tree structure.  This commit adds
support for handling that case and also creating the navigation logic
within that tree structure.

Issue PER-10314 { title of the issue }

Copy link
Member

@omnignorant omnignorant left a comment

Choose a reason for hiding this comment

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

First of all, the steps to test were excellent! I definitely tried stuff I wouldn't have otherwise. Very well done.

I think this is in pretty good shape, but I found a few small-ish issues while testing on Safari.

  1. Thumbnails for documents are not showing. Images are fine, but documents were not. Did not check other file types. Documents were fine on full screen view.
  2. Clicking on the browser back button after opening a download (e.g. previously clicking on download opens file in browser) returns to the login screen with an error.
  3. When viewing an unlisted share while signed in, the "view in archive" and "my archive" buttons do not seem to resolve to anything. Everything just greys out and becomes inoperable.
  4. When loading an unlisted share link, a preview with blurred photos loads first and then is replaced with the unlisted share preview.

I don't think any of these bugs are deal breakers that would prevent a release to prod with the feature flag so we can also test on prod but we can discuss. I will mark this as approved with the understanding that these other bugs might be separate tickets.

@aasandei-vsp
Copy link
Contributor Author

aasandei-vsp commented Nov 12, 2025

  1. Thumbnails for documents are not showing. Images are fine, but documents were not. Did not check other file types. Documents were fine on full screen view. --> https://permanent.atlassian.net/browse/PER-10350
  2. Clicking on the browser back button after opening a download (e.g. previously clicking on download opens file in browser) returns to the login screen with an error. --> https://permanent.atlassian.net/browse/PER-10351
  3. When viewing an unlisted share while signed in, the "view in archive" and "my archive" buttons do not seem to resolve to anything. Everything just greys out and becomes inoperable.
    --> "my archive" https://permanent.atlassian.net/browse/PER-10324 - IN PROGRESS and https://permanent.atlassian.net/browse/PER-10353
    --> "view in archive" where does it appear and what's the expected behavior?
  4. When loading an unlisted share link, a preview with blurred photos loads first and then is replaced with the unlisted share preview. --> https://permanent.atlassian.net/browse/PER-10352

@aasandei-vsp aasandei-vsp force-pushed the PER-10314-unlisted-share-preview-folder branch from 4fa3125 to 82154bf Compare November 12, 2025 10:39
…n async and actually it would remove items that were just registering again
The properties router and isUnlistedShare were private members of the file-list
component, but they are used in the template, so they needed to be made public.
The code works, as it's javascript, but taking into account that we are using
Typescript, we should be using the correct modifiers.

Issue PER-10314 Implement folder in folder share preview for unlisted shares
@aasandei-vsp aasandei-vsp force-pushed the PER-10314-unlisted-share-preview-folder branch from 82154bf to 7339c56 Compare November 12, 2025 15:14
@aasandei-vsp aasandei-vsp merged commit 06ab47b into main Nov 14, 2025
8 of 9 checks passed
@aasandei-vsp aasandei-vsp deleted the PER-10314-unlisted-share-preview-folder branch November 14, 2025 06:49
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.

4 participants