-
Notifications
You must be signed in to change notification settings - Fork 12
rcv: Make IVA fields optional in schemas and data models #941
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
Conversation
- Updated `iva_retenido_total`, `iva_retenido_parcial`, `iva_no_retenido`, `iva_propio`, `iva_terceros`. - Discovered that the fields `iva_retenido_total` and `iva_retenido_parcial`can be `null` in some cases, so all `iva*` fields are now nullable. Ref: https://app.shortcut.com/cordada/story/17456/
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #941 +/- ##
========================================
Coverage 88.79% 88.79%
========================================
Files 40 40
Lines 3767 3767
Branches 390 390
========================================
Hits 3345 3345
Misses 267 267
Partials 155 155 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Pull Request Overview
This PR makes five IVA-related fields optional in the RCV (Registro de Compras y Ventas - Chilean tax document) venta (sales) records parsing system. The changes were motivated by discovering that iva_retenido_total and iva_retenido_parcial can be null in real-world data.
Key Changes:
- Updated data model type annotations from
inttoOptional[int]for 5 IVA fields - Updated marshmallow schema fields from
required=Truetorequired=False, allow_none=True - Added test case demonstrating nullable behavior with
iva_retenido_parcial=Noneandiva_retenido_total=2020
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/cl_sii/rcv/data_models.py |
Updated type annotations to Optional[int] for all 5 IVA fields in RvDetalleEntry class |
src/cl_sii/rcv/parse_csv.py |
Updated marshmallow field definitions to required=False, allow_none=True for all 5 IVA fields in RcvVentaCsvRowSchema |
src/tests/test_rcv_parse_csv.py |
Updated test expectations to include a case with iva_retenido_total=2020 and iva_retenido_parcial=None |
src/tests/test_data/sii-rcv/RCV-venta-extra-empty-impuestos-rows.csv |
Added test data row with non-zero iva_retenido_total and empty iva_retenido_parcial field |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.



iva_retenido_total,iva_retenido_parcial,iva_no_retenido,iva_propio,iva_terceros.iva_retenido_totalandiva_retenido_parcialcan benullin some cases, so alliva*fields are now nullable.Ref: https://app.shortcut.com/cordada/story/17456/