Skip to content

Conversation

@pieterbeulque
Copy link
Contributor

VAT is calculated based on the billing address at order creation time. Changing the country/state after an order has been paid would result in an inconsistent invoice where the address doesn't match the VAT charged.

…ated

VAT is calculated based on the billing address at order creation time.
Changing the country/state after an order has been paid would result in
an inconsistent invoice where the address doesn't match the VAT charged.

Changes:
- Backend: Add InvoiceBillingAddressUpdateError that is raised when
  attempting to change country or state on a paid order
- Frontend: Disable country and state picker fields in the Edit Invoice
  modal when the order has been paid
- Add disabled prop support to CountryPicker and CountryStatePicker
  components
@vercel
Copy link

vercel bot commented Dec 26, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments
Project Deployment Review Updated (UTC)
polar Ignored Ignored Preview Dec 26, 2025 2:31pm
polar-sandbox Ignored Ignored Preview Dec 26, 2025 2:31pm

@github-actions
Copy link
Contributor

📦 Next.js Bundle Analysis for web

This analysis was generated by the Next.js Bundle Analysis action. 🤖

This PR introduced no changes to the JavaScript bundle! 🙌

Copy link
Member

@frankie567 frankie567 left a comment

Choose a reason for hiding this comment

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

Good catch, just a few changes needed on the im plementation itself

existing_state = existing_address.get("state") if existing_address else None

if new_country != existing_country or new_state != existing_state:
raise InvoiceBillingAddressUpdateError(order)
Copy link
Member

Choose a reason for hiding this comment

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

Rather than raising a custom error, we should directly raise a PolarRequestValidationError, like here for example:

raise PolarRequestValidationError(
[
{
"type": "value_error",
"loc": ("body", "benefits", order),
"msg": "Benefit does not exist.",
"input": benefit_id,
}
]
)

@github-actions
Copy link
Contributor

📦 Next.js Bundle Analysis for web

This analysis was generated by the Next.js Bundle Analysis action. 🤖

This PR introduced no changes to the JavaScript bundle! 🙌

@pieterbeulque pieterbeulque force-pushed the claude/fix-vat-invoice-address-FLi5k branch from 06b9748 to 68ac948 Compare December 26, 2025 14:10
@github-actions
Copy link
Contributor

📦 Next.js Bundle Analysis for web

This analysis was generated by the Next.js Bundle Analysis action. 🤖

This PR introduced no changes to the JavaScript bundle! 🙌

@pieterbeulque pieterbeulque force-pushed the claude/fix-vat-invoice-address-FLi5k branch from 68ac948 to 5d6f49f Compare December 26, 2025 14:28
@github-actions
Copy link
Contributor

📦 Next.js Bundle Analysis for web

This analysis was generated by the Next.js Bundle Analysis action. 🤖

This PR introduced no changes to the JavaScript bundle! 🙌

Replace custom InvoiceBillingAddressUpdateError with standard
PolarRequestValidationError for better consistency with the codebase.
@pieterbeulque pieterbeulque force-pushed the claude/fix-vat-invoice-address-FLi5k branch from 5d6f49f to 355c6ad Compare December 26, 2025 14:31
@github-actions
Copy link
Contributor

📦 Next.js Bundle Analysis for web

This analysis was generated by the Next.js Bundle Analysis action. 🤖

This PR introduced no changes to the JavaScript bundle! 🙌

@pieterbeulque pieterbeulque marked this pull request as ready for review January 5, 2026 15:20
@pieterbeulque
Copy link
Contributor Author

Superseded by #8892

@frankie567
Copy link
Member

Wow sorry, @pieterbeulque, completely forgot about this one 🙈

@pieterbeulque
Copy link
Contributor Author

hehe no all good 🫶 this had merge conflicts anyway so better to do it from scratch

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.

4 participants