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

fix: alpha fixes #2835

Merged
merged 5 commits into from
Jan 8, 2024
Merged

fix: alpha fixes #2835

merged 5 commits into from
Jan 8, 2024

Conversation

dkoo
Copy link
Contributor

@dkoo dkoo commented Jan 5, 2024 •
edited
Loading

All Submissions:

Changes proposed in this Pull Request:

Fixes some issues resulting from a bad hotfix merge.

How to test the changes in this Pull Request:

  1. Follow testing instructions in fix: use Woo's cart fee for covering transaction fees #2820 and make sure they work
  2. Follow testing instructions in feat(my-account): support edit address #2733 and make sure they work
  3. Re: this comment, enable some but not all billing address fields in Newspack > Reader Revenue > Donations.
  4. Complete a purchase of a shippable product, then log into/verify My Account, then visit Addresses.
  5. Confirm that when editing the billing address, only the fields enabled in Reader Revenue in step 3 are required, and that you can save with empty "optional" fields (but not with empty "required" fields).
  6. Confirm that when editing the shipping address, all default address fields are still required.

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 marked this pull request as ready for review January 5, 2024 03:05
@dkoo dkoo requested a review from a team as a code owner January 5, 2024 03:05
@dkoo dkoo self-assigned this Jan 5, 2024
@dkoo dkoo added the [Status] Needs Review The issue or pull request needs to be reviewed label Jan 5, 2024
adekbadek
adekbadek approved these changes Jan 5, 2024
View reviewed changes
Copy link
Member

@adekbadek adekbadek left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Address editing: I've just enabled two address fields for the checkout flow, but in the edit view all are displayed and required. Not sure if that's expected, description in #2733 does not mention such a case.

@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 Jan 5, 2024
@dkoo
Copy link
Contributor Author

dkoo commented Jan 5, 2024

Address editing: I've just enabled two address fields for the checkout flow, but in the edit view all are displayed and required. Not sure if that's expected, description in #2733 does not mention such a case.

Hmm, we're not filtering the billing fields shown here to match them up with the fields selected in Reader Revenue. Do you think that's a requirement?

@dkoo
Copy link
Contributor Author

dkoo commented Jan 5, 2024

@adekbadek 20cbded filters the address fields for the "edit billing address" page only, so that only fields enabled in Reader Revenue > Donations are required. Other address fields are still shown but are marked optional. Updated testing instructions, too.

@adekbadek
Copy link
Member

That works for address fields, but "Phone" is added later in WC. I've updated the filter used to make it filter the billing fields at a later stage, when all are added (f0b5490).

@leogermani leogermani merged commit 49efd8b into alpha Jan 8, 2024
1 check passed
@dkoo dkoo deleted the fix/alpha-fixes branch January 8, 2024 16:58
matticbot pushed a commit that referenced this pull request Jan 8, 2024
@semantic-release-bot
chore(release): 2.13.0 [skip ci]
04cd223 
# [2.13.0](v2.12.2...v2.13.0) (2024-01-08)

### Bug Fixes

* alpha fixes ([#2835](#2835)) ([49efd8b](49efd8b))
* newsletters Lists indentation ([#2797](#2797)) ([1af3c23](1af3c23))
* **ras:** remove modal checkout logic ([#2781](#2781)) ([6aad17d](6aad17d))
* use Woo's cart fee for covering transaction fees ([#2820](#2820)) ([fded027](fded027))

### Features

* add media partners module ([#2753](#2753)) ([70f7dcb](70f7dcb))
* don’t send OTP via preauth flow when signing up for newsletter ([#2795](#2795)) ([686af03](686af03))
* **donation:** additional receipt email template variables ([#2799](#2799)) ([0c9c373](0c9c373))
* force allow subscription switching ([#2784](#2784)) ([ae7523f](ae7523f))
* force option to enble retries of failed payments ([#2808](#2808)) ([f8d35ec](f8d35ec))
* give editors permission for restricted content ([#2806](#2806)) ([64d7817](64d7817))
* **my-account:** support edit address ([#2733](#2733)) ([92d5778](92d5778))
* remove commenting from engagement tab ([#2726](#2726)) ([f51c7bc](f51c7bc))
@matticbot
Copy link
Contributor

🎉 This PR is included in version 2.13.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@dkoo dkoo mentioned this pull request Sep 4, 2024
Merged
6 tasks
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

@dkoo dkoo

Labels
released [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.
4 participants
@dkoo @adekbadek @matticbot @leogermani