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

Add total tax amount to metadata #3726

Merged
merged 13 commits into from
Jan 29, 2025
Merged

Add total tax amount to metadata #3726

merged 13 commits into from
Jan 29, 2025

Conversation

hsingyuc
Copy link
Contributor

@hsingyuc hsingyuc commented Jan 14, 2025

Fixes #2103

Changes proposed in this Pull Request:

This PR adds the total tax amount to metadata because we only pass the order total to Stripe but not tax information.
Screenshot 2025-01-14 at 3 29 34 PM
Screenshot 2025-01-16 at 11 54 30 AM

I also considered adding individual product tax information in the Level 3 data and add into metadata, because here says we support sales tax in level 2 but I don't see it. I also learned that Stripe supports Level 3 data in their API on both Charge and PaymentIntent but gated, so I removed it.

Screenshot 2025-01-14 at 9 32 29 AM

Testing instructions

  1. Create a new taxable product.
  2. Navigate to WooCommerce > Settings > Taxes and set a tax rate.
  3. Ensure the WooCommerce Stripe Payment Gateway is installed and active.
  4. Make sure the Legacy checkout experience under Stripe settings is unchecked.
  5. Complete a purchase of the taxable product created in step 1.
  6. Navigate to the associated order on dashboard.stripe.com.
  7. Check the event data related to the purchase made in step 5.
  8. Should see tax_amount under metadata.
  9. Confirm that metadata is the right place to add tax amount, and this is where level 2 data should go.

  • Covered with tests (or have a good reason not to test in description ☝️)
  • Added changelog entry in both changelog.txt and readme.txt (or does not apply)
  • Tested on mobile (or does not apply)

Post merge

@hsingyuc hsingyuc requested review from a team and diegocurbelo and removed request for a team January 14, 2025 22:56
@@ -1769,6 +1769,7 @@ public function get_metadata_from_order( $order ) {
'order_key' => $order->get_order_key(),
'payment_type' => $payment_type,
'signature' => $this->get_order_signature( $order ),
'tax_amount' => WC_Stripe_Helper::get_stripe_amount( $order->get_total_tax(), strtolower( $order->get_currency() ) ),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've considered naming it total_tax, but since we use shipping_amount, I opted for tax_amount here.

Copy link
Member

@diegocurbelo diegocurbelo 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, and it tests well 🚢

@hsingyuc hsingyuc self-assigned this Jan 24, 2025
nbloomf and others added 7 commits January 27, 2025 11:19
* Lint fixes

---------

Co-authored-by: nbloomf <nathan.bloomfield@automattic.com>
Co-authored-by: Anne Mirasol <anne.mirasol@automattic.com>
* Fix duplicate emails when enabling the gateway

During payment gateway initialization, WC core adds hooks that will get
triggered when the main gateway is enabled. Using a centralized option_key for all
'child gateways' makes WC core issue separate email notifications for all of them. To sidestep this problem,
with this change, we use separate option_id for each child gateway but still use the
parent gateway settings when retrieving them.

Changes made:
- Update get_option_key() to return settings specific to the Stripe ID.
- Introduce get_option() method to fetch options from the main Stripe gateway.

---------

Co-authored-by: Mayisha <33387139+Mayisha@users.noreply.github.com>
Co-authored-by: Diego Curbelo <diego@curbelo.com>
@hsingyuc hsingyuc force-pushed the add/tax-info-to-metadata branch from d43a20f to d194aa2 Compare January 27, 2025 16:24
@hsingyuc hsingyuc merged commit 2a12b65 into develop Jan 29, 2025
37 checks passed
@hsingyuc hsingyuc deleted the add/tax-info-to-metadata branch January 29, 2025 18:53
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.

Tax information not passed to Stripe dashboard
6 participants