Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(ras-acc): add faq section to otp email #3034

Merged
merged 10 commits into from
Apr 18, 2024
Merged

feat(ras-acc): add faq section to otp email #3034

merged 10 commits into from
Apr 18, 2024

Conversation

chickenn00dle
Copy link
Contributor

@chickenn00dle chickenn00dle commented Apr 4, 2024 •
edited
Loading

All Submissions:

Changes proposed in this Pull Request:

PR 3 of 4

This PR adds onto the work in #3032 and #3033 by adding an FAQ section to the otp email:

Screenshot 2024-04-05 at 13 16 04

How to test the changes in this Pull Request:

  1. With these changes checked out, go to Newspack > Engagment > Show Advanced Settings and select the one-time password email under Transactional Email Content section
  2. Trash the post (so that we can force our updated template to be used on next otp sign in attempt)
  3. As a logged out user, select the sign in link and attempt to login via email to trigger an otp email
  4. Check the otp email and confirm the FAQ section is present and the link to set a new password works as expected

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully ran tests with your changes locally?

@chickenn00dle chickenn00dle changed the base branch from trunk to feat/update-transaction-emails-dynamic-colors April 4, 2024 22:29
@chickenn00dle chickenn00dle mentioned this pull request Apr 4, 2024
Merged
6 tasks
@chickenn00dle chickenn00dle force-pushed the feat/update-transactional-emails-faq-section branch from 82cc6b8 to 54f1301 Compare April 5, 2024 16:48
@chickenn00dle chickenn00dle marked this pull request as ready for review April 5, 2024 17:18
@chickenn00dle chickenn00dle requested a review from a team as a code owner April 5, 2024 17:18
@chickenn00dle chickenn00dle added the [Status] Needs Review The issue or pull request needs to be reviewed label Apr 5, 2024
@chickenn00dle chickenn00dle mentioned this pull request Apr 6, 2024
Closed
6 tasks
adekbadek
adekbadek requested changes Apr 8, 2024
View reviewed changes
Copy link
Member

@adekbadek adekbadek left a comment

Choose a reason for hiding this comment

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

The update logic discussion (#3032 (review)) applies here, too.

Also, I'm confused about structure of these PRs (PRs to other PR branches, instead of PRs to trunk). Should there be an epic branch?

@chickenn00dle chickenn00dle force-pushed the feat/update-transaction-emails-dynamic-colors branch from 0bce5a5 to 2b4f3d6 Compare April 9, 2024 15:32
@chickenn00dle
Copy link
Contributor Author

Apologies for the confusion @adekbadek!

I meant to update the descriptions for these email PRs, but that slipped my mind. I've broken the changes up into 4 different PRs, and since they all sort of rely on one another they are based off of the other (all going into epic/ras-acc branch). So there is an order which I've updated each of the PR descriptions with.

But to be clear, here is the order for these PRs:

PR 1 - Already approved
PR 2- #3033
PR 3 - This one
PR 4 - #3038

@chickenn00dle chickenn00dle force-pushed the feat/update-transactional-emails-faq-section branch from b495722 to 4838996 Compare April 9, 2024 16:42
@chickenn00dle chickenn00dle requested a review from adekbadek April 10, 2024 15:21
adekbadek
adekbadek approved these changes Apr 10, 2024
View reviewed changes
Copy link
Member

@adekbadek adekbadek left a comment

Choose a reason for hiding this comment

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

Pre-approving pending a small email content change.

includes/templates/reader-activation-emails/otp.php Outdated
<p><strong>' . __( 'What is a magic link?', 'newspack' ) . '</strong><br>' . __( 'It\’s a temporary, password-free link like the one above to help you quickly and securely verify your identity and sign in – all without needing to set a password.', 'newspack' ) . '</p>
<!-- /wp:paragraph -->
<!-- wp:paragraph -->
<p><strong>' . __( 'Can I still create a password?', 'newspack' ) . '</strong><br>' . sprintf( /* Translators: 1: opening HTML anchor tag 2: closing HTML anchor tag */ __( 'Yes. For security reasons, we recommend signing in with Google, Facebook, or Apple, or using a one-time code sent to your email, but you can set a password if you prefer. %1$sClick here%2$s to create a password.', 'newspack' ), '<a href="*SET_PASSWORD_LINK*" data-type="link" data-id="*SET_PASSWORD_LINK*">', '</a>' ) . '</p>
Copy link
Member

Choose a reason for hiding this comment

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

We don't support signing in with Facebook and Apple yet. This should only mention Google.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! Removed in 5101590

@github-actions github-actions bot added [Status] Approved The pull request has been reviewed and is ready to merge and removed [Status] Needs Review The issue or pull request needs to be reviewed labels Apr 10, 2024
@chickenn00dle chickenn00dle force-pushed the feat/update-transaction-emails-dynamic-colors branch from 0989b8a to e4c8473 Compare April 16, 2024 22:20
Base automatically changed from feat/update-transaction-emails-dynamic-colors to epic/ras-acc April 18, 2024 16:12
@chickenn00dle chickenn00dle force-pushed the feat/update-transactional-emails-faq-section branch from 4838996 to 5101590 Compare April 18, 2024 17:46
@chickenn00dle chickenn00dle merged commit bccb560 into epic/ras-acc Apr 18, 2024
7 checks passed
@matticbot
Copy link
Contributor

🎉 This PR is included in version 3.6.0-epic-ras-acc.9 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@chickenn00dle chickenn00dle mentioned this pull request Apr 20, 2024
Merged
6 tasks
@matticbot
Copy link
Contributor

🎉 This PR is included in version 3.9.0-epic-ras-acc.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@matticbot
Copy link
Contributor

🎉 This PR is included in version 4.1.0-epic-ras-acc.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@matticbot
Copy link
Contributor

🎉 This PR is included in version 4.2.0-epic-ras-acc.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@matticbot
Copy link
Contributor

🎉 This PR is included in version 4.4.0-epic-ras-acc.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

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

@adekbadek adekbadek adekbadek approved these changes

Assignees
No one assigned
Labels
released on @epic/ras-acc [Status] Approved The pull request has been reviewed and is ready to merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@chickenn00dle @matticbot @adekbadek