-
Notifications
You must be signed in to change notification settings - Fork 0
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
Update the logic to adapt for the new UK VAT endpoint #2
base: master
Are you sure you want to change the base?
Update the logic to adapt for the new UK VAT endpoint #2
Conversation
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.
PR Overview
This PR fixes the bug related to the new HMRC VAT validation endpoint and updates naming conventions from “United Kingdom” to “Great Britain”.
- Updated HMRCRegistry to use separate base URLs and added explicit authentication logic with OAuth.
- Renamed “GB” comments and docstrings in both the country definitions and VAT rules to reflect the updated naming.
- Reordered instantiation of HMRCRegistry in init.py for improved clarity.
Reviewed Changes
File | Description |
---|---|
pyvat/registries.py | Updated HMRCRegistry logic for new API endpoints and OAuth flow. |
pyvat/countries.py | Updated comment for 'GB' to “Great Britain”. |
pyvat/vat_rules.py | Updated VAT rules docstring to use “Great Britain” instead of “United Kingdom”. |
pyvat/init.py | Reordered instantiation of HMRCRegistry. |
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
url = "{0}/organisations/vat/check-vat-number/lookup/".format(base_url) | ||
headers = self._authentication_headers() | ||
response = requests.get( | ||
url + vat_number, |
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.
[nitpick] Consider using a URL joining function (such as urllib.parse.urljoin) for more robust URL construction and to avoid potential issues with missing or extra slashes.
url = "{0}/organisations/vat/check-vat-number/lookup/".format(base_url) | |
headers = self._authentication_headers() | |
response = requests.get( | |
url + vat_number, | |
url = urljoin(base_url, "/organisations/vat/check-vat-number/lookup/") | |
headers = self._authentication_headers() | |
response = requests.get( | |
urljoin(url, vat_number), |
Copilot is powered by AI, so mistakes are possible. Review output carefully before use.
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.
Approved (FYI this branch is on your personal GH account)
Can this be deployed somehow to Monies staging?
Can we change Monies requirements to use this branch https://github.com/wp-media/monies/blob/develop/requirements.txt
@nicomollet we need to add the env secrets into the k8 namespaces first let me handle that |
Description
Fixes #149
Fix the bug by using the new API from gov.uk where it returns a 404.
For that we had to upgrade the library to use the version 2 of the API with Oauth authentification.
Type of change
Detailed scenario
What was tested
Describe the scenarios that you tested, and specify if it is automated or manual. For manual scenarios, provide a screenshot of the results.
The new code was tested manually on the testing env using this script:
Then I used the keys to test both a valid VAT number and an invalid one in production and testing.
How to test
Describe how the PR can be tested so that the validator can be autonomous: environment, dependencies, specific setup, steps to perform, API requests, etc.
Use the script and set the env variables:
PYVAT_UK_CLIENT_ID=
PYVAT_UK_CLIENT_SECRET=
You need to find a client in WP Rocket with a valid VAT and use a fake VAT as such as
123456789
to make sure both cases are covered.Technical description
Documentation
Explain how this code works. Diagrams & drawings are welcome.
New dependencies
List any new dependencies that are required for this change.
Risks
List possible performance & security issues or risks, and explain how they have been mitigated.
Mandatory Checklist
Code validation
Code style
Unticked items justification
This is a library with few tests and a fork.
If some mandatory items are not relevant, explain why in this section.
Additional Checks