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

feat: add cli command to fix stripe covered fees #2848

Merged
merged 4 commits into from
Jan 15, 2024

Conversation

leogermani
Copy link
Contributor

All Submissions:

Changes proposed in this Pull Request:

Since #2820 we use an actual woocommerce Fee to add the Stripe fee to the order. Before that, we would manually add the fee to the order total if a metadata was present.

We didn't take into account the need to update existing subscriptions so that renewal orders will also get the fee added. (we assumed they would copy from the Sub's parent order, but they don't).

This PR adds a CLI command to migrate the subscriptions that had fees added the old way to the new way.

It's a CLI command because it should affect few subscriptions in a few sites.

How to test the changes in this Pull Request:

  1. Checkout the plugin in the version prior to fix: use Woo's cart fee for covering transaction fees #2820 git checkout v2.12.0
  2. Make sure the option for the donor to cover fees is enabled
  3. Make a recurring donation covering Stripe fees
  4. Checkout master or this branch
  5. Force the subscription to renewal and verify the total of the renewal order does not include the fee
  6. Run the commands in this PR to and confirm the subscription is listed to be fixed, and that the fix is applied
  7. wp newspack-woocommerce list_subscriptions_missing_fee should list the subscription
  8. wp newspack-woocommerce fix_subscriptions_missing_fee --dry-run should output what the fix will look like
  9. wp newspack-woocommerce fix_subscriptions_missing_fee should fix it
  10. Go back to the admin, see the subscription, verify that the fee is there and the total is updated
  11. Force a renewal and confirm the renewal order has the fee
  12. Also test a new recurring subscription on this branch with fees covered, and its renewal orders, to make sure we didn't break anything

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?

@leogermani leogermani added the [Status] Needs Review The issue or pull request needs to be reviewed label Jan 12, 2024
@leogermani leogermani self-assigned this Jan 12, 2024
@leogermani leogermani requested a review from a team as a code owner January 12, 2024 18:19
Copy link
Member

@miguelpeixe miguelpeixe left a comment

Choose a reason for hiding this comment

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

Working flawlessly.

NIT suggestion: maybe add a meta to the subscription so it's easier to track which of them got migrated? Just in case we need to revisit them. The order note works too, but it's more of a UI feature.

@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 12, 2024
@leogermani
Copy link
Contributor Author

Thank you! Added post mea in 8a4bd00 - tested again and it's all fine

@leogermani leogermani merged commit 3f5874f into release Jan 15, 2024
@leogermani leogermani deleted the hotfix/fix-stripe-cover-fees branch January 15, 2024 12:40
matticbot pushed a commit that referenced this pull request Jan 15, 2024
# [2.14.0](v2.13.0...v2.14.0) (2024-01-15)

### Features

* add cli command to fix stripe covered fees ([#2848](#2848)) ([3f5874f](3f5874f))
@matticbot
Copy link
Contributor

🎉 This PR is included in version 2.14.0 🎉

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 [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