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.

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

fix(my-account): fixes for My Account #2904

Merged
merged 4 commits into from
Feb 8, 2024
Merged

Conversation

dkoo
Copy link
Contributor

@dkoo dkoo commented Feb 3, 2024

All Submissions:

Changes proposed in this Pull Request:

Fixes a few bugs affecting My Account. Replaces #2899. See also Automattic/newspack-blocks#1661 and Automattic/newspack-theme#2240

How to test the changes in this Pull Request:

Required billing fields for gift recipients

When the recipient of a gifted subscription logs into My Account for the first time, WooCommerce asks them to fill in some required billing information. This info should match the selected fields in Reader Revenue settings.

  1. install the Gift Subscriptions extension if you don't already have it.
  2. As a reader, purchase a subscription as a gift for another email address.
  3. On release, in a new session, log in as the gift recipient. Observe that you're asked for shipping address details in a strangely styled form:

Screenshot 2024-02-01 at 09-58-43 My account - Newspack

  1. Check out this branch (and fix(my-account): fixes for My Account #2904 and fix(my-account): fixes for My Account newspack-theme#2240) and the corresponding branch in the Theme and refresh. Confirm that the form now only asks for First Name and Last Name fields, if they're required in Reader Revenue settings.

Gift recipients can't see "Subscriptions" menu item in My Account

  1. After completing the form in the previous instructions, on release, observe that there's no Subscriptions menu item in the My Account navigation sidebar.
  2. Check out this branch (and fix(my-account): fixes for My Account #2904 and fix(my-account): fixes for My Account newspack-theme#2240), refresh, and confirm the Subscriptions menu item now shows up and that clicking it shows the subscription you were gifted.

Can't renew subscriptions from the Subscription page

  1. As a reader, complete a subscription donation and then log into My Account/verify as needed.
  2. On release, go to My Subscription and click "Renew Now". Observe that you get redirected with no feedback, and the renewal request is never processed.
  3. Check out this branch (and fix(my-account): fixes for My Account #2904 and fix(my-account): fixes for My Account newspack-theme#2240 and repeat. Now confirm that you clicking "Renew Now" brings you to a checkout form which you can complete to renew early.

Thank you page for renewals shows modal checkout thank you template despite not being modal checkout

#1573 added some additional hook callbacks to force modal checkout styles in the thank you page for express checkout methods (Google/Apple Pay), but #1654 implements a similar fix with a more reliable check for whether the express checkout request originated in a modal checkout flow, so these hooks are no longer needed.

  1. Check out this branch (and fix(my-account): fixes for My Account #2904 and fix(my-account): fixes for My Account newspack-theme#2240) and repeat the renewal process as described above.
  2. After completing the renewal, confirm that the thank you page shown is the standard WooCommerce thank you template, and not our minimalist modal version.

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?

@dkoo dkoo self-assigned this Feb 3, 2024
@dkoo dkoo added the [Status] Needs Review The issue or pull request needs to be reviewed label Feb 3, 2024
@dkoo dkoo marked this pull request as ready for review February 3, 2024 01:30
@dkoo dkoo requested a review from a team as a code owner February 3, 2024 01:30
@dkoo dkoo changed the base branch from release to epic/ras-acc February 6, 2024 00:51
@dkoo dkoo force-pushed the hotfix/my-account-fixes branch from 92ad6ec to 1230580 Compare February 6, 2024 00:51
@dkoo
Copy link
Contributor Author

dkoo commented Feb 6, 2024

Note: Rebased against epic/ras-acc as gift subscription functionality has been fixed in the epic, not in trunk.

Copy link
Contributor

@chickenn00dle chickenn00dle left a comment

Choose a reason for hiding this comment

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

Hey @dkoo,

I tried testing this but ran into a few issues, with the modal checkout. Not sure if some other changes to epic/ras-acc might be conflicting with those in the three PRs that make up these fixes, but am seeing:

  1. A fatal related to a missing method in blocks: fix(my-account): fixes for My Account newspack-blocks#1661 (comment)
  2. And some wonky behavior in the modal checkout preventing me from actually purchasing a gift sub:
Screen.Recording.2024-02-06.at.11.26.48.mov

@dkoo
Copy link
Contributor Author

dkoo commented Feb 6, 2024

@chickenn00dle looks like the branch needed a merge from the epic. Checkout and gifting should be working again

@dkoo dkoo requested a review from chickenn00dle February 6, 2024 18:04
Copy link
Contributor

@chickenn00dle chickenn00dle left a comment

Choose a reason for hiding this comment

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

Nice! Thanks for clearing that up. This is working as expected now 👍

@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 Feb 8, 2024
@dkoo dkoo merged commit 2c5f3bd into epic/ras-acc Feb 8, 2024
8 checks passed
@dkoo dkoo deleted the hotfix/my-account-fixes branch February 8, 2024 18:32
@matticbot
Copy link
Contributor

🎉 This PR is included in version 3.0.0-epic-ras-acc.3 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@matticbot
Copy link
Contributor

🎉 This PR is included in version 3.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 3.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 3.6.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 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
Labels
released on @epic/ras-acc [Status] Approved The pull request has been reviewed and is ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants