Conversation
45cc3fb to
b21f44a
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds test coverage for query parameter handling in invoice retrieval operations. The tests verify that the retrieve() and all() methods correctly accept and pass query parameters like validation_type, include_edit_histories, and with_disabled to the API.
Key Changes
- Added tests for single query parameter (
validation_type) handling in bothretrieve()andall()methods - Added comprehensive tests verifying multiple query parameters are correctly passed in API requests
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…bled_by to Invoice
loomchild
left a comment
There was a problem hiding this comment.
Code looks OK, I have some minor comments.
But I don't know how to test it and whether adding extra fields was intentional.
pkopac
left a comment
There was a problem hiding this comment.
Please address @loomchild 's comments.
| disabled = fields.Boolean(allow_none=True) | ||
| disabled_at = fields.DateTime(allow_none=True) | ||
| disabled_by = fields.String(allow_none=True) | ||
| edit_history_summary = fields.Dict(allow_none=True) |
There was a problem hiding this comment.
[major] When trying to retrieve this field from few random invoices in production, I get:
AttributeError: 'Invoice' object has no attribute 'edit_history_summary'
Could you confirm that it works for you in production using any invoice?
|
I was able to find an invalid invoice, and the |
|
@loomchild thank you for the test, it helped me discover the issue. It is the same that I had with my previous PR related to listing and retrieving DataSources 😅. I added the generic boolean query params handler to the Resource to address this in the latest commit - please take another look. You can call Invoices / Disable an Invoice public API endpoint on any of your invoices: it produces both an |
Testing instructions: