Skip to content

Enhancement/11 move vat rates#12

Merged
mostafa-hisham merged 41 commits into
masterfrom
enhancement/11-move-vat-rates
Dec 19, 2025
Merged

Enhancement/11 move vat rates#12
mostafa-hisham merged 41 commits into
masterfrom
enhancement/11-move-vat-rates

Conversation

@mostafa-hisham
Copy link
Copy Markdown

Description

Fixes #11

Type of change

  • New feature (non-breaking change which adds functionality).
  • Bug fix (non-breaking change which fixes an issue).
  • Enhancement (non-breaking change which improves an existing functionality).
  • Breaking change (fix or feature that would cause existing functionality to not work as before).
  • Sub-task of #(issue number)
  • Chore
  • Release

Detailed scenario

What was tested

unit tests

How to test

  • ci i should run unit test
  • follow readme and run tests

Technical description

Documentation

no

New dependencies

no

Risks

no

Mandatory Checklist

Code validation

  • I validated all the Acceptance Criteria. If possible, provide screenshots or videos.
  • I triggered all changed lines of code at least once without new errors/warnings/notices.
  • I implemented built-in tests to cover the new/changed code.

Code style

  • I wrote a self-explanatory code about what it does.
  • I protected entry points against unexpected inputs.
  • I did not introduce unnecessary complexity.
  • Output messages (errors, notices, logs) are explicit enough for users to understand the issue and are actionnable.

Unticked items justification

If some mandatory items are not relevant, explain why in this section.

Additional Checks

  • In the case of complex code, I wrote comments to explain it.
  • When possible, I prepared ways to observe the implemented system (logs, data, etc.).
  • I added error handling logic when using functions that could throw errors (HTTP/API request, filesystem, etc.)

Comment thread pyvat/vat_rules.py Outdated
'EG': EgVatRules(),
'CH': ChVatRules(),
'CA': CaVatRules(),
'NO': NoVatRules(),
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I am not confortable with the NoVat naming.
Can we name the rules with the countries explicitely named: Norway, Canada, FranceDom, Egypt, Switzerland?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@mostafa-hisham Please rename the shortnames to country:

  • CaVatRules > CanadaVatRues
  • NoVatRules > NorwayVatRules

So that we can avoid the misleading NoVatRules

Comment thread pyvat/__init__.py
@nicomollet
Copy link
Copy Markdown

@mostafa-hisham

Monaco and 3 Doms can be considered differently.
Monaco is exactly like France, so it can be added added to the EuropeVAT rules and have same rules as France 20%.

But the 3 Doms are not Europe VAT zone (and have different VAT rates).

image

Comment thread pyvat/vat_rules.py Outdated
# French VAT zone: seller in France/territories selling to buyer in France/territories
# For B2B or consumers after 2015-01-01, charge buyer's country VAT
if seller_in_french_zone and buyer_in_french_zone:
if buyer.is_business or date >= JANUARY_1_2015:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think we can remove this 2015 condition everywhere

Comment thread pyvat/vat_rules.py Outdated
Copy link
Copy Markdown

@nicomollet nicomollet left a comment

Choose a reason for hiding this comment

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

Check comments:

  • code consistency
  • behavior of B2B exemptions for NON EU

Comment thread pyvat/vat_rules.py Outdated
'EG': EgVatRules(),
'CH': ChVatRules(),
'CA': CaVatRules(),
'NO': NoVatRules(),
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@mostafa-hisham Please rename the shortnames to country:

  • CaVatRules > CanadaVatRues
  • NoVatRules > NorwayVatRules

So that we can avoid the misleading NoVatRules

Comment thread tests/test_sale_vat_charge.py Outdated
Comment thread tests/test_sale_vat_charge.py Outdated
Comment thread tests/test_new_countries.py
Comment thread tests/test_new_countries.py
Comment thread pyvat/registries.py
Comment thread pyvat/vat_rules.py Outdated
Comment thread pyvat/vat_rules.py Outdated
@mostafa-hisham
Copy link
Copy Markdown
Author

mostafa-hisham commented Dec 15, 2025

@nicomollet if we are ok with this PR we should release it

Copy link
Copy Markdown

@nicomollet nicomollet left a comment

Choose a reason for hiding this comment

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

Approving @mostafa-hisham

@nicomollet
Copy link
Copy Markdown

nicomollet commented Dec 15, 2025

@mostafa-hisham Please check this message from Mathieu:

Egypt: VAT (14%) must be applied to both B2C & B2B sales

https://group-onecom.slack.com/archives/C08EFCVF459/p1765807325784649

This issue will be closed:

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 6 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/test_new_countries.py Outdated
Comment thread README.rst Outdated
Comment thread tests/test_new_countries.py Outdated
Comment thread tests/test_new_countries.py Outdated
Comment on lines +100 to +106
ItemType.generic_physical_good: Decimal(25.5),
ItemType.generic_electronic_service: Decimal(25.5),
ItemType.generic_telecommunications_service: Decimal(25.5),
ItemType.generic_broadcasting_service: Decimal(25.5),
ItemType.prepaid_broadcasting_service: Decimal(25.5),
ItemType.ebook: Decimal(10),
ItemType.enewspaper: Decimal(24),
ItemType.enewspaper: Decimal(25.5),
Copy link

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

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

VAT rate update for Finland. The test expectations show Finland's VAT rate increasing from 24% to 25.5%, which is reflected in the updated test on lines 100-106. However, ensure this VAT rate change (from 24% to 25.5%) is accurate and matches official Finnish tax authority announcements. The comment in vat_rules.py indicates "VAT rates updated July 1st 2025", so verify this is the correct effective date for Finland's rate change.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Author

@mostafa-hisham mostafa-hisham Dec 15, 2025

Choose a reason for hiding this comment

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

@nicomollet, are we good with this?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Let's ignore this type of product

Comment thread tests/test_validators.py Outdated
Copy link
Copy Markdown

@nicomollet nicomollet left a comment

Choose a reason for hiding this comment

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

Approved @mostafa-hisham

@mostafa-hisham
Copy link
Copy Markdown
Author

QA on monies staging ✅

https://github.com/wp-media/monies/pull/230#issue-3733560193

@mostafa-hisham mostafa-hisham merged commit 51e59f8 into master Dec 19, 2025
1 check passed
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.

Move VAT rates from Monies to Pyvat

3 participants