Skip to content

Conversation

@crisnicandrei
Copy link
Contributor

@crisnicandrei crisnicandrei commented Jun 3, 2025

How to test:

  1. Have account A invite account B to Permanent (Account B must have an account)
  2. Account B must receive an email whilst logged in and click the link
  3. Account B should be redirected to the pending archives modal

@codecov
Copy link

codecov bot commented Jun 3, 2025

Codecov Report

❌ Patch coverage is 66.66667% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 45.51%. Comparing base (308f258) to head (2528c26).
⚠️ Report is 131 commits behind head on main.

Files with missing lines Patch % Lines
src/app/core/core.routes.ts 0.00% 4 Missing ⚠️
...my-archives-dialog/my-archives-dialog.component.ts 72.72% 3 Missing ⚠️
...nts/account-dropdown/account-dropdown.component.ts 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #577      +/-   ##
==========================================
+ Coverage   45.17%   45.51%   +0.33%     
==========================================
  Files         370      370              
  Lines       11255    11272      +17     
  Branches     1855     1858       +3     
==========================================
+ Hits         5085     5130      +45     
+ Misses       6001     5974      -27     
+ Partials      169      168       -1     

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

@crisnicandrei crisnicandrei force-pushed the PER-10096-archive-invitation-email-redirect-to-pending-archives branch 3 times, most recently from 1946df0 to 33b173e Compare June 3, 2025 23:33
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.

Code looks good -- ready for QA testing!

@slifty slifty requested review from cecilia-donnelly and removed request for omnignorant July 7, 2025 16:14
@slifty slifty requested review from omnignorant and removed request for cecilia-donnelly July 17, 2025 16:36
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.

This does not appear to be working as described. I invited an existing account B to be a member of my archive from account A. I got an email from account A to account B with a link, then copy/pasted it to a browser window where account B was logged in. I was not directed to pending archives. See attached video:

I did not attempt to QA this for a member invite to a non-existant account since that should take the new account through onboarding to accept the invite.

Screen.Recording.2025-07-21.at.1.09.02.PM.mov

@crisnicandrei
Copy link
Contributor Author

crisnicandrei commented Jul 21, 2025

@omnignorant you are right, when I implemented this the email system was not working on my side. Should I implement this only on the frontend or should we change the backend aswell with the correct url taking you to the pending archive? The curent email url only has 2 parameters which maybe in the future could also have other purposes, and I think the frontend should not handle this on its own, but I might be wrong and I am open to suggestions.

CC: @slifty @liam-lloyd

@liam-lloyd
Copy link
Member

@omnignorant you are right, when I implemented this the email system was not working on my side. Should I implement this only on the frontend or should we change the backend aswell with the correct url taking you to the pending archive? The curent email url only has 2 parameters which maybe in the future could also have other purposes, and I think the frontend should not handle this on its own, but I might be wrong and I am open to suggestions.

CC: @slifty @liam-lloyd

I think it does make sense for the URL to specify which tab in the modal should be selected, and to update the link the backend includes in this email to reflect that.

@crisnicandrei
Copy link
Contributor Author

@liam-lloyd this is how the url looks like to get redirected to pending archives: https://local.permanent.org/app/(private/0004-0006/75//dialog:archives/pending)

@slifty
Copy link
Contributor

slifty commented Aug 21, 2025

Bumping this @crisnicandrei and @omnignorant -- it would be great to rebase this and get it over the edge if at all possible. It sounds like there may be open questions about desired functionality?

@slifty slifty self-requested a review August 22, 2025 15:08
@crisnicandrei crisnicandrei force-pushed the PER-10096-archive-invitation-email-redirect-to-pending-archives branch 2 times, most recently from 067c535 to 1a2411f Compare September 2, 2025 16:28
PER-10096-archive-invitations

-Create new route for archives dialog
-Redirect that new route based on the url to the pending archives dialog
@crisnicandrei crisnicandrei force-pushed the PER-10096-archive-invitation-email-redirect-to-pending-archives branch from 1a2411f to 2528c26 Compare September 2, 2025 16:35
@omnignorant omnignorant self-requested a review September 8, 2025 18:40
@slifty
Copy link
Contributor

slifty commented Sep 15, 2025

This is waiting on a backend fix, which @cecilia-donnelly is going to be taking on at some point.

@aasandei-vsp aasandei-vsp self-requested a review September 17, 2025 10:14
@aasandei-vsp aasandei-vsp marked this pull request as draft October 7, 2025 13:14
@aasandei-vsp aasandei-vsp changed the title PER-10096-archive-invitations PER-10096-archive-invitations[waiting on BE fix] Oct 8, 2025
@cecilia-donnelly
Copy link
Member

This is the backend ticket: https://permanent.atlassian.net/browse/PER-10291. We talked to @omnignorant about it and he'd like to write better requirements better we work on it (in particular drafting the email text).

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.

6 participants