Skip to content

Conversation

fetchfern
Copy link
Contributor

@fetchfern fetchfern commented Sep 10, 2025

Integrates with Anrok.

While at it this also massively cleans up the code handling PaymentIntents, and fixes a bug where the intent metadata wasn't being updated when updating the intent via /_internal/billing/payment.

Merging of this is blocked by #4338, so we can implement email notifications before charging people taxes.

Still left to do:

  • Actually use the PaymentIntent functionality in the payments module everywhere
  • Alert emails
  • Pre-fill the database with values for products_tax_identifiers
  • Tests

@fetchfern fetchfern marked this pull request as ready for review September 16, 2025 15:54
@fetchfern
Copy link
Contributor Author

May need to do some tweaks to the approach to price change notifications w.r.t taxes + tax calculation pending some clarifications from product but it's 90% of the way there—ready for review.

It's not the cleanest implementation I'll admit but it works and needs to be shipped.

Copy link
Member

@AlexTMjugador AlexTMjugador left a comment

Choose a reason for hiding this comment

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

In general this looks sensible enough to try out. I couldn't review or test this too deeply, but I suppose it's better than nothing.

@fetchfern
Copy link
Contributor Author

There was an issue with my approach to notify users about price changes (wouldn't notify users if the sales tax rate in their jurisdiction changes) so I ended up modifying this PR quite a bit and fixed some of the implementation issues with it:

  • Generate Anrok transactions through a queue worker rather than within the webhook directly (no more tokio::spawn in an impl Drop!)
  • Continuously update the tax amounts rather than just when the tax amount is initially zero to account for sales tax changes, and insert the related notification in the same task rather than a separate one. This removes the need for the user_aware_of_tax_changes column so I removed it.
  • More variables available to the tax change notification template.
  • Store product names in the DB rather than hardcoding them.
  • Retry logic for Anrok API operations with random exponential backoff.

Still waiting on the tax-notification email on prod. Requesting review again since the changes are significant.

Copy link
Member

@AlexTMjugador AlexTMjugador left a comment

Choose a reason for hiding this comment

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

Nice, this looks noticeably better now! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants