-
Notifications
You must be signed in to change notification settings - Fork 0
Enhancement/11 move vat rates #12
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
Changes from 13 commits
3d1e48d
f45ef1d
8b3adc7
9696462
1cb9f87
0dcd614
12ad19a
ca9ce85
d631d34
7df0c08
c9d6052
106b503
900a182
662a6a7
95f3550
acb0fdb
aa85a94
81e7ba7
2cd45ca
3718f20
370809d
35f537e
2ce4ef7
26c8bd5
1cdf199
9d2431b
4601aa6
6f53399
084ced6
4035704
6aadd1a
edcc349
bae70a3
528f133
5373fd5
682258f
7013fda
5f7f3bd
3fb7220
bf3947a
eef3a89
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| name: Run Tests | ||
|
|
||
| on: | ||
| pull_request: | ||
| branches: | ||
| - master | ||
| pull_request_target: | ||
| branches: | ||
| - master | ||
| push: | ||
| branches: | ||
| - master | ||
|
|
||
| permissions: | ||
| contents: read | ||
| pull-requests: read | ||
|
|
||
| jobs: | ||
| test: | ||
| runs-on: ubuntu-latest | ||
| strategy: | ||
| matrix: | ||
| python-version: ['3.10'] | ||
|
|
||
| steps: | ||
| - name: Checkout code | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| ref: ${{ github.event.pull_request.head.sha || github.sha }} | ||
|
|
||
| - name: Set up Python ${{ matrix.python-version }} | ||
| uses: actions/setup-python@v5 | ||
| with: | ||
| python-version: ${{ matrix.python-version }} | ||
|
|
||
| - name: Install dependencies | ||
| run: | | ||
| python -m pip install --upgrade pip | ||
| pip install -e . | ||
| pip install pytest | ||
|
|
||
| - name: Run tests | ||
| run: | | ||
| python -m pytest tests/ -v | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -67,3 +67,69 @@ Usage | |||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
| For more detailed documentation, see the `full pyvat documentation <http://pyvat.readthedocs.org/>`_. | ||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
| Running Tests | ||||||||||||||||
| ------------- | ||||||||||||||||
|
|
||||||||||||||||
| ``pyvat`` uses `pytest <https://pytest.org/>`_ for testing. To run the test suite: | ||||||||||||||||
|
|
||||||||||||||||
| **Install test dependencies:** | ||||||||||||||||
|
|
||||||||||||||||
| .. code-block:: bash | ||||||||||||||||
|
|
||||||||||||||||
| $ pip install pytest | ||||||||||||||||
|
|
||||||||||||||||
| **Run all tests:** | ||||||||||||||||
|
|
||||||||||||||||
| .. code-block:: bash | ||||||||||||||||
|
|
||||||||||||||||
| $ python -m pytest tests/ -v | ||||||||||||||||
|
|
||||||||||||||||
| **Run specific test files:** | ||||||||||||||||
|
|
||||||||||||||||
| .. code-block:: bash | ||||||||||||||||
|
|
||||||||||||||||
| # Test new countries implementation | ||||||||||||||||
| $ python -m pytest tests/test_new_countries.py -v | ||||||||||||||||
|
|
||||||||||||||||
| # Test VAT charge calculations | ||||||||||||||||
| $ python -m pytest tests/test_sale_vat_charge.py -v | ||||||||||||||||
|
|
||||||||||||||||
| # Test VAT number validators | ||||||||||||||||
| $ python -m pytest tests/test_validators.py -v | ||||||||||||||||
|
|
||||||||||||||||
| **Run with coverage:** | ||||||||||||||||
|
|
||||||||||||||||
| .. code-block:: bash | ||||||||||||||||
|
|
||||||||||||||||
| $ pip install pytest-cov | ||||||||||||||||
| $ python -m pytest tests/ --cov=pyvat --cov-report=html | ||||||||||||||||
|
|
||||||||||||||||
| **Test Results:** | ||||||||||||||||
|
|
||||||||||||||||
| The test suite includes: | ||||||||||||||||
|
|
||||||||||||||||
| * **VAT Rules Tests**: Verify correct VAT rates for all supported countries | ||||||||||||||||
| * **Registry Tests**: Test B2B exemption logic for different countries | ||||||||||||||||
| * **VAT Charge Tests**: Ensure proper VAT calculation for cross-border sales | ||||||||||||||||
| * **Validator Tests**: Check VAT number format validation | ||||||||||||||||
|
|
||||||||||||||||
| *Note: Some validator tests that call external VIES API are currently skipped and will be refactored with mocks.* | ||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
| Supported Countries | ||||||||||||||||
| ------------------- | ||||||||||||||||
|
|
||||||||||||||||
| **EU Countries**: All EU member states with full VIES integration | ||||||||||||||||
|
|
||||||||||||||||
| **Non-EU Countries**: | ||||||||||||||||
|
|
||||||||||||||||
| * Egypt (EG) - 14% VAT, B2B exempt | ||||||||||||||||
| * Switzerland (CH) - 8.1% VAT, B2B not exempt | ||||||||||||||||
| * Canada (CA) - 0% VAT, B2B exempt | ||||||||||||||||
| * Norway (NO) - 25% VAT, B2B not exempt | ||||||||||||||||
| * Monaco (MC) - 20% VAT, B2B not exempt | ||||||||||||||||
| * French Overseas Departments (RE, GP, MQ) - 8.5% VAT, B2B not exempt | ||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
|
||||||||||||||||
| * Monaco (MC) - 20% VAT, B2B not exempt | |
| * French Overseas Departments (RE, GP, MQ) - 8.5% VAT, B2B not exempt | |
| * French Overseas Departments (RE, GP, MQ) - 8.5% VAT, B2B not exempt | |
| **French VAT Zone Countries**: | |
| * Monaco (MC) - 20% VAT, B2B not exempt (Treated as part of the French VAT zone; VAT numbers are validated using the French VIES registry. Monaco is not considered a non-EU country for VAT purposes.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -4,7 +4,7 @@ | |||||
|
|
||||||
| from .item_type import ItemType | ||||||
| from .party import Party | ||||||
| from .registries import ViesRegistry, HMRCRegistry, EgyptRegistry | ||||||
| from .registries import ViesRegistry, HMRCRegistry, EgyptRegistry, SwitzerlandRegistry, CanadaRegistry, NorwayRegistry | ||||||
|
|
||||||
| from .result import VatNumberCheckResult | ||||||
| from .vat_charge import VatCharge, VatChargeAction | ||||||
|
|
@@ -50,7 +50,6 @@ | |||||
| "SE": re.compile(r"^\d{12}$"), | ||||||
| "SI": re.compile(r"^\d{8}$"), | ||||||
| "SK": re.compile(r"^\d{10}$"), | ||||||
| 'EG': re.compile(r'^\d{9}$'), | ||||||
|
|
||||||
| } | ||||||
| """VAT number expressions. | ||||||
|
|
@@ -75,6 +74,18 @@ | |||||
| """Egypt Registry instance. | ||||||
|
||||||
| """Egypt Registry instance. | |
| """Egypt Registry instance. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Format
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -28,14 +28,49 @@ def check_vat_number(self, vat_number, country_code, test): | |||
|
|
||||
| class EgyptRegistry(Registry): | ||||
| """ | ||||
| Egyptian registry refusing all VAT numbers. | ||||
| Egyptian registry accepting all VAT numbers for B2B exemption. | ||||
| """ | ||||
|
|
||||
| def check_vat_number(self, vat_number, country_code, test): | ||||
| result = VatNumberCheckResult() | ||||
| result.is_valid = True | ||||
|
nicomollet marked this conversation as resolved.
|
||||
| return result | ||||
|
|
||||
|
|
||||
| class SwitzerlandRegistry(Registry): | ||||
| """ | ||||
| Switzerland registry refusing all VAT numbers (B2B will not be exempt). | ||||
| """ | ||||
|
|
||||
| def check_vat_number(self, vat_number, country_code, test): | ||||
| result = VatNumberCheckResult() | ||||
| result.is_valid = False | ||||
| return result | ||||
|
|
||||
|
|
||||
| class CanadaRegistry(Registry): | ||||
| """ | ||||
| Canadian registry accepting all VAT numbers for B2B exemption. | ||||
| """ | ||||
|
|
||||
| def check_vat_number(self, vat_number, country_code, test): | ||||
| result = VatNumberCheckResult() | ||||
| result.is_valid = True | ||||
| return result | ||||
|
|
||||
|
|
||||
| class NorwayRegistry(Registry): | ||||
| """ | ||||
| Norwegian registry refusing all VAT numbers (B2B will not be exempt). | ||||
| """ | ||||
|
|
||||
| def check_vat_number(self, vat_number, country_code, test): | ||||
| result = VatNumberCheckResult() | ||||
| result.is_valid = False | ||||
| return result | ||||
|
|
||||
|
|
||||
|
|
||||
|
||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Format
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -96,12 +96,28 @@ def get_sale_to_country_vat_charge(self, | |||||||||
| 'non-business sellers are currently not supported' | ||||||||||
| ) | ||||||||||
|
|
||||||||||
| # French territories (MC, RE, GP, MQ) are treated as part of France for VAT purposes | ||||||||||
| french_territories = {'FR', 'MC', 'RE', 'GP', 'MQ'} | ||||||||||
| seller_in_french_zone = seller.country_code in french_territories | ||||||||||
| buyer_in_french_zone = buyer.country_code in french_territories | ||||||||||
|
|
||||||||||
| # If the seller resides in the same country as the buyer, we charge | ||||||||||
| # VAT regardless of whether the buyer is a business or not. Similarly, | ||||||||||
| # if the buyer is a consumer, we must charge VAT in the buyer's country | ||||||||||
| # of residence. | ||||||||||
| if seller.country_code == buyer.country_code or \ | ||||||||||
| (not buyer.is_business and date >= JANUARY_1_2015): | ||||||||||
| # VAT regardless of whether the buyer is a business or not. | ||||||||||
| if seller.country_code == buyer.country_code: | ||||||||||
| return VatCharge(VatChargeAction.charge, | ||||||||||
| buyer.country_code, | ||||||||||
| self.get_vat_rate(item_type, postal_code)) | ||||||||||
|
|
||||||||||
| # 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 | ||||||||||
|
nicomollet marked this conversation as resolved.
Outdated
|
||||||||||
| if seller_in_french_zone and buyer_in_french_zone: | ||||||||||
| if buyer.is_business or date >= JANUARY_1_2015: | ||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we can remove this 2015 condition everywhere |
||||||||||
| return VatCharge(VatChargeAction.charge, | ||||||||||
| buyer.country_code, | ||||||||||
| self.get_vat_rate(item_type, postal_code)) | ||||||||||
|
|
||||||||||
| # Consumers in other EU countries after 2015-01-01 are charged in their country | ||||||||||
|
mostafa-hisham marked this conversation as resolved.
Outdated
|
||||||||||
| if not buyer.is_business and date >= JANUARY_1_2015: | ||||||||||
| return VatCharge(VatChargeAction.charge, | ||||||||||
| buyer.country_code, | ||||||||||
| self.get_vat_rate(item_type, postal_code)) | ||||||||||
|
||||||||||
| if not buyer.is_business and date >= JANUARY_1_2015: | |
| return VatCharge(VatChargeAction.charge, | |
| buyer.country_code, | |
| self.get_vat_rate(item_type, postal_code)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Optimize condition
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using both 'pull_request' and 'pull_request_target' triggers can lead to security concerns. The 'pull_request_target' event runs in the context of the base repository and has access to secrets, which can be dangerous when combined with 'pull_request'. Unless there's a specific need for 'pull_request_target', it should be removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would apply suggestion