Added lading support#5
Conversation
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. 💤 Files selected but had no reviewable changes (2)
⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds a new Pydantic module modelling CFDI 4.0 Carta Porte (Bill of Lading) entities, re-exports those models at package and models package levels, updates InvoiceComplement to reference the new LadingComplement with alias "cartaPorte", and bumps the package version. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
fiscalapi/__init__.py (1)
92-92:⚠️ Potential issue | 🟡 MinorDuplicate import:
LadingComplementis already imported frombill_of_lading_modelsat line 45.🧹 Proposed fix: Remove duplicate import
PayrollDeduction, PayrollDisability, PayrollComplement, - LadingComplement, InvoiceComplement,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fiscalapi/__init__.py` at line 92, Remove the duplicate import of LadingComplement from bill_of_lading_models: locate the second occurrence of "LadingComplement" in the import list (the one referenced in the diff) and delete it so only the original import (already present around the earlier import block) remains; ensure no other symbols or commas are accidentally removed when editing the import list.fiscalapi/models/__init__.py (1)
77-77:⚠️ Potential issue | 🟡 MinorDuplicate import:
LadingComplementis already imported frombill_of_lading_modelsat line 32.Since
fiscalapi_modelsimportsLadingComplementfrombill_of_lading_modelsand re-exports it, this import is redundant.🧹 Proposed fix: Remove duplicate import
PayrollDeduction, PayrollDisability, PayrollComplement, - LadingComplement, InvoiceComplement,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fiscalapi/models/__init__.py` at line 77, Remove the duplicate import of LadingComplement (already imported from bill_of_lading_models earlier) by deleting the redundant "LadingComplement" entry in the import/export list so that LadingComplement is only imported/re-exported once; ensure the remaining import from bill_of_lading_models (the original LadingComplement reference) is kept and run tests/type checks to verify no other duplicate entries remain.
🧹 Nitpick comments (1)
fiscalapi/models/fiscalapi_models.py (1)
531-531: Consider consistency:LadingComplementextendsBaseModelwhile sibling complements extendBaseDto.All complement types in
InvoiceComplement(LocalTaxesComplement,PaymentComplement,PayrollComplement) inherit fromBaseDto, which providesid,created_at, andupdated_atfields.LadingComplementextendsBaseModeldirectly, creating a structural inconsistency.If the Carta Porte API doesn't return those base fields, this is intentional and acceptable. Otherwise, align
LadingComplementto extendBaseDtofor consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fiscalapi/models/fiscalapi_models.py` at line 531, The LadingComplement type currently extends BaseModel while its sibling complements used in InvoiceComplement (e.g., LocalTaxesComplement, PaymentComplement, PayrollComplement) extend BaseDto; update LadingComplement to inherit from BaseDto instead of BaseModel so it includes the common id/created_at/updated_at fields and matches the structure of other complements (change the class signature for LadingComplement to extend BaseDto and adjust any imports/refs accordingly), unless the Carta Porte API definitively does not return those base fields — in that case add a clarifying comment explaining the intentional omission.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@fiscalapi/__init__.py`:
- Line 196: The __all__ list in fiscalapi.__init__ contains a duplicate export
"LadingComplement"; remove the redundant "LadingComplement" entry so each public
symbol appears only once—update the __all__ definition where "LadingComplement"
is listed (ensure the remaining single occurrence stays) to eliminate the
duplicate export.
In `@fiscalapi/models/__init__.py`:
- Line 136: The __all__ list in fiscalapi.models contains a duplicated export
name "LadingComplement"; locate the __all__ definition in models/__init__.py and
remove the redundant "LadingComplement" entry so each exported symbol appears
only once (keep one "LadingComplement" and ensure the list remains syntactically
valid).
---
Outside diff comments:
In `@fiscalapi/__init__.py`:
- Line 92: Remove the duplicate import of LadingComplement from
bill_of_lading_models: locate the second occurrence of "LadingComplement" in the
import list (the one referenced in the diff) and delete it so only the original
import (already present around the earlier import block) remains; ensure no
other symbols or commas are accidentally removed when editing the import list.
In `@fiscalapi/models/__init__.py`:
- Line 77: Remove the duplicate import of LadingComplement (already imported
from bill_of_lading_models earlier) by deleting the redundant "LadingComplement"
entry in the import/export list so that LadingComplement is only
imported/re-exported once; ensure the remaining import from
bill_of_lading_models (the original LadingComplement reference) is kept and run
tests/type checks to verify no other duplicate entries remain.
---
Nitpick comments:
In `@fiscalapi/models/fiscalapi_models.py`:
- Line 531: The LadingComplement type currently extends BaseModel while its
sibling complements used in InvoiceComplement (e.g., LocalTaxesComplement,
PaymentComplement, PayrollComplement) extend BaseDto; update LadingComplement to
inherit from BaseDto instead of BaseModel so it includes the common
id/created_at/updated_at fields and matches the structure of other complements
(change the class signature for LadingComplement to extend BaseDto and adjust
any imports/refs accordingly), unless the Carta Porte API definitively does not
return those base fields — in that case add a clarifying comment explaining the
intentional omission.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 59d998d3-3e3a-4e77-bbd7-be5e3fec2675
📒 Files selected for processing (7)
examples/ejemplos-factura-carta-porte-referencias.pyexamples/ejemplos-factura-carta-porte-valores.pyfiscalapi/__init__.pyfiscalapi/models/__init__.pyfiscalapi/models/bill_of_lading_models.pyfiscalapi/models/fiscalapi_models.pysetup.py
| @@ -171,6 +195,27 @@ | |||
| "PayrollComplement", | |||
| "LadingComplement", | |||
There was a problem hiding this comment.
Duplicate entry: "LadingComplement" appears twice in __all__ (lines 196 and 218).
🧹 Proposed fix: Remove duplicate from line 218
"TipoFigura",
- "LadingComplement",
# Invoice modelsAlso applies to: 218-218
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@fiscalapi/__init__.py` at line 196, The __all__ list in fiscalapi.__init__
contains a duplicate export "LadingComplement"; remove the redundant
"LadingComplement" entry so each public symbol appears only once—update the
__all__ definition where "LadingComplement" is listed (ensure the remaining
single occurrence stays) to eliminate the duplicate export.
| "TransporteFerroviario", | ||
| "ParteTransporte", | ||
| "TipoFigura", | ||
| "LadingComplement", |
There was a problem hiding this comment.
Duplicate entry: "LadingComplement" appears twice in __all__ (lines 136 and 184).
Remove one of the duplicate entries.
🧹 Proposed fix: Remove duplicate from line 184
"PayrollComplement",
- "LadingComplement",
"InvoiceComplement",Also applies to: 184-184
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@fiscalapi/models/__init__.py` at line 136, The __all__ list in
fiscalapi.models contains a duplicated export name "LadingComplement"; locate
the __all__ definition in models/__init__.py and remove the redundant
"LadingComplement" entry so each exported symbol appears only once (keep one
"LadingComplement" and ensure the list remains syntactically valid).
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@fiscalapi/models/__init__.py`:
- Line 18: The __all__ export list is missing GuiaIdentificacion, causing
wildcard imports to omit this new model; update the module-level __all__ list(s)
to include "GuiaIdentificacion" (add the exact string to the existing tuple/list
alongside the other model names) so that GuiaIdentificacion is exported when
using from fiscalapi.models import *; ensure you add it in every __all__
occurrence where the other Carta Porte models are listed (the same block that
contains the other model names).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 33534b50-a2db-49f9-8dd4-42d518f54cdd
📒 Files selected for processing (4)
fiscalapi/__init__.pyfiscalapi/models/__init__.pyfiscalapi/models/bill_of_lading_models.pyfiscalapi/models/fiscalapi_models.py
🚧 Files skipped from review as they are similar to previous changes (3)
- fiscalapi/models/fiscalapi_models.py
- fiscalapi/init.py
- fiscalapi/models/bill_of_lading_models.py
| RegimenAduanero, | ||
| Ubicacion, | ||
| DocumentoAduanero, | ||
| GuiaIdentificacion, |
There was a problem hiding this comment.
Add GuiaIdentificacion to __all__.
Line 18 imports GuiaIdentificacion, and fiscalapi/__init__.py also exposes it, but fiscalapi.models.__all__ omits it. This makes wildcard exports incomplete for the new Carta Porte model set.
Proposed fix
"Ubicacion",
"DocumentoAduanero",
+ "GuiaIdentificacion",
"CantidadTransporta",Also applies to: 118-139
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@fiscalapi/models/__init__.py` at line 18, The __all__ export list is missing
GuiaIdentificacion, causing wildcard imports to omit this new model; update the
module-level __all__ list(s) to include "GuiaIdentificacion" (add the exact
string to the existing tuple/list alongside the other model names) so that
GuiaIdentificacion is exported when using from fiscalapi.models import *; ensure
you add it in every __all__ occurrence where the other Carta Porte models are
listed (the same block that contains the other model names).
Co-authored-by: Copilot <copilot@github.com>
1 participant
Summary by CodeRabbit
New Features
Chores