Added lading support#5
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a typed Carta Porte (lading) model in Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Encontre muchas discrepancias entre los modelos del SDK y el contrato público de la API. Por favor utiliza como referencia la API publica (CartaPorteComplementRequest.cs en el backend) y sus clases anidadas para validar campo por campo o directamente reescribir los modelos equivalentes. Los problemas principales son: propiedades que la API expone pero el SDK no tiene (~20 en mercancias, 5 de seguros en autotransporte, guias de identificacion completas), campos marcados como opcionales en el SDK cuando la API los exige como obligatorios (8 en transporte maritimo, 4 en aereo, 3 en ferroviario, entre otros), campos marcados como obligatorios en el SDK cuando la API los tiene como opcionales (totalDistRec, pesoNetoTotal), y 5 campos en el SDK que no existen en la API (configMaritimaId, numCertITC, nombreEmbarCargador, nombreAgente en maritimo y rfcTransportista en aereo). Ademas los nombres de los campos del SDK no coinciden con los nombres que usa el backend. Esto agrega una carga cognitiva alta cuando se realiza trazabilidad entre el SDK y la API publica, o cuando toca debuggear por que un campo no llega como se esperaba. Te sugiero que uses los mismos nombres de propiedades que tiene CartaPorteComplementRequest.cs y sus clases relacionadas, manteniendo tipos de datos equivalentes y la misma obligatoriedad.
Otro comentario es que Falta interface LadingGuiaIdentificacion, no lo agregue en el código porque no existe tal código.
La API expone guiasIdentificacion como lista en Mercancia con 3 campos: numeroGuiaIdentificacion, descripGuiaIdentificacion, pesoGuiaIdentificacion. Crea la interface LadingGuiaIdentificacion con esos
campos, agrega la propiedad guiasIdentificacion en LadingMerchandise y exportala en index.ts.
Revise superficialmente el de Java y veo los mismos issues. Por favor haz ajustes en todo y avísame cuando esté listo para Code review por favor.
| /** | ||
| * Mercancía en carta porte | ||
| */ | ||
| export interface LadingMerchandise { |
There was a problem hiding this comment.
Faltan propiedades en LadingMerchandise que la API sí expone. Revisa este contrato en la API: CartaPorteMercanciaRequest
Las siguientes propiedades existen en el backend pero no en el SDK:
- claveSTCCId
- unidad
- dimensiones
- cveMaterialPeligrosoId
- embalajeId
- descripEmbalaje
- sectorCOFEPRISId
- nombreIngredienteActivo
- nomQuimico
- permisoImportacion
- folioImpoVUCEM
- numCAS
- razonSocialEmpImp
- numRegSanPlagCOFEPRIS
- datosFabricante
- datosFormulador
- datosMaquilador
- usoAutorizado
- uuidComercioExt
Agregar todas estas propiedades opcionales a LadingMerchandise.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/index.ts (1)
87-88: Public alias casing inconsistency:LadingRailcarvsLadingRailContainer.The two rail-related aliases use different compound-word casing on the public API surface. For consistency with
LadingRailContainer, considerLadingRailCar. Also note that the original namesLadingContainerCar/LadingCarare not re-exported, so once published consumers can only import the aliased names — confirm that's intentional (no code path outside the package needs the original names).♻️ Proposed casing alignment
- LadingContainerCar as LadingRailContainer, - LadingCar as LadingRailcar, + LadingContainerCar as LadingRailContainer, + LadingCar as LadingRailCar,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/index.ts` around lines 87 - 88, Public API aliases have inconsistent compound-word casing: `LadingContainerCar` -> exported as `LadingRailContainer` but `LadingCar` -> exported as `LadingRailcar`; align them by renaming the alias `LadingRailcar` to `LadingRailCar` so both use consistent CamelCase; update the export statement that maps `LadingCar as LadingRailcar` to `LadingCar as LadingRailCar` and verify no internal or external code relies on the old alias name (adjust any imports accordingly).src/models/invoice.ts (1)
469-534:LadingLocationAddressandLadingFigureAddressare structurally identical.Both interfaces declare the same 10 fields with the same optionality and the same required set (
estadoId,paisId,codigoPostalId). This is a DRY violation — consider collapsing them into a single sharedLadingAddress(or keeping both names as aliases) so future schema changes only need to be applied in one place.♻️ Proposed dedup via a shared base type
/** - * Domicilio de una ubicación en carta porte (CartaPorteUbicacionDomicilioRequest) + * Domicilio base para carta porte (ubicaciones y figuras). */ -export interface LadingLocationAddress { +export interface LadingAddress { /** Calle */ calle?: string; // ... /** Código postal. Catálogo SAT c_CodigoPostal */ codigoPostalId: string; } -/** - * Domicilio de una figura de transporte en carta porte (CartaPorteTiposFiguraDomicilioRequest) - */ -export interface LadingFigureAddress { - // ...duplicate fields... -} +/** Alias semánticos para mantener la intención del contrato */ +export type LadingLocationAddress = LadingAddress; +export type LadingFigureAddress = LadingAddress;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/models/invoice.ts` around lines 469 - 534, LadingLocationAddress and LadingFigureAddress are identical duplicates; consolidate them by creating a single exported interface (e.g., LadingAddress) that contains the shared fields (calle, numeroExterior, numeroInterior, coloniaId, localidadId, referencia, municipioId, estadoId, paisId, codigoPostalId) and then replace both LadingLocationAddress and LadingFigureAddress with type aliases to that interface (or export the single name and update usages to reference LadingAddress) so future schema changes are applied in one place; update any imports/usages of LadingLocationAddress and LadingFigureAddress to use the new LadingAddress or aliases.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/index.ts`:
- Around line 87-88: Public API aliases have inconsistent compound-word casing:
`LadingContainerCar` -> exported as `LadingRailContainer` but `LadingCar` ->
exported as `LadingRailcar`; align them by renaming the alias `LadingRailcar` to
`LadingRailCar` so both use consistent CamelCase; update the export statement
that maps `LadingCar as LadingRailcar` to `LadingCar as LadingRailCar` and
verify no internal or external code relies on the old alias name (adjust any
imports accordingly).
In `@src/models/invoice.ts`:
- Around line 469-534: LadingLocationAddress and LadingFigureAddress are
identical duplicates; consolidate them by creating a single exported interface
(e.g., LadingAddress) that contains the shared fields (calle, numeroExterior,
numeroInterior, coloniaId, localidadId, referencia, municipioId, estadoId,
paisId, codigoPostalId) and then replace both LadingLocationAddress and
LadingFigureAddress with type aliases to that interface (or export the single
name and update usages to reference LadingAddress) so future schema changes are
applied in one place; update any imports/usages of LadingLocationAddress and
LadingFigureAddress to use the new LadingAddress or aliases.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 015aeca9-1b6c-417a-af4f-2327e87935c2
📒 Files selected for processing (2)
src/index.tssrc/models/invoice.ts
Co-authored-by: Copilot <copilot@github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/models/invoice.ts (2)
539-578: TightentipoUbicacionto a string-literal union.
tipoUbicacionis a closed SAT enum (Origen/Destino). Typing it as a literal union gives consumers autocompletion and catches typos at compile time, which aligns with the strict TS posture in this repo.♻️ Suggested refactor
- /** Tipo de ubicación (Origen / Destino) */ - tipoUbicacion: string; + /** Tipo de ubicación (Origen / Destino) */ + tipoUbicacion: 'Origen' | 'Destino';As per coding guidelines: "Enable strict TypeScript mode with checks: noImplicitAny, strictNullChecks, noImplicitReturns, noUnusedParameters".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/models/invoice.ts` around lines 539 - 578, The tipoUbicacion field on the LadingLocation interface should be narrowed from string to the string-literal union 'Origen' | 'Destino' to provide autocomplete and catch typos; update the LadingLocation declaration to use tipoUbicacion: 'Origen' | 'Destino' and then adjust any call sites, parsers, validators, deserializers or tests that construct LadingLocation objects (or accept raw strings) to either produce one of those two literals or perform explicit validation/normalization so the code compiles under strict TypeScript.
651-777: BothmaterialPeligrosoIdandcveMaterialPeligrosoIdare correctly present; consider clarifying JSDoc comments to distinguish their roles.All 19 previously flagged properties are present. Usage across all examples confirms the intended distinction:
materialPeligrosoIdserves as a flag/indicator (consistently set to'No'in examples), whilecveMaterialPeligrosoIdis the catalog clave value. Improve clarity by distinguishing these roles more explicitly in the JSDoc comments—for example, specify thatmaterialPeligrosoIdis an inline indicator/flag andcveMaterialPeligrosoIdis the corresponding SAT c_MaterialPeligroso catalog code when applicable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/models/invoice.ts` around lines 651 - 777, Update the JSDoc for the LadingMerchandise interface to clearly distinguish the two related fields: change the comment for materialPeligrosoId to state it is an inline indicator/flag (e.g., "Indica si es material peligroso (flag/inline, e.g., 'Sí'/'No')") and change the comment for cveMaterialPeligrosoId to state it is the SAT catalog clave (e.g., "Clave del catálogo SAT c_MaterialPeligroso — usar cuando materialPeligrosoId indicates a hazardous material"). Ensure these edits are made on the LadingMerchandise interface fields materialPeligrosoId and cveMaterialPeligrosoId so their intended roles are explicit.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/models/invoice.ts`:
- Line 107: The public API renamed Complement.lading to Complement.cartaPorte
which breaks existing consumers; add a backwards-compatible deprecation alias or
document the break: either add a deprecated getter/optional property named
lading on the Complement type/class that forwards to cartaPorte (and mark it
with a deprecation comment/ts-expect-deprecation) so existing code compiles, and
update the exported types (e.g., LadingComplement) in index.ts to re-export the
alias, OR add a prominent "Breaking changes" entry in README/CHANGELOG
describing the rename (Complement.lading → Complement.cartaPorte) and include
migration guidance and timeline for removal.
---
Nitpick comments:
In `@src/models/invoice.ts`:
- Around line 539-578: The tipoUbicacion field on the LadingLocation interface
should be narrowed from string to the string-literal union 'Origen' | 'Destino'
to provide autocomplete and catch typos; update the LadingLocation declaration
to use tipoUbicacion: 'Origen' | 'Destino' and then adjust any call sites,
parsers, validators, deserializers or tests that construct LadingLocation
objects (or accept raw strings) to either produce one of those two literals or
perform explicit validation/normalization so the code compiles under strict
TypeScript.
- Around line 651-777: Update the JSDoc for the LadingMerchandise interface to
clearly distinguish the two related fields: change the comment for
materialPeligrosoId to state it is an inline indicator/flag (e.g., "Indica si es
material peligroso (flag/inline, e.g., 'Sí'/'No')") and change the comment for
cveMaterialPeligrosoId to state it is the SAT catalog clave (e.g., "Clave del
catálogo SAT c_MaterialPeligroso — usar cuando materialPeligrosoId indicates a
hazardous material"). Ensure these edits are made on the LadingMerchandise
interface fields materialPeligrosoId and cveMaterialPeligrosoId so their
intended roles are explicit.
🪄 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: 11046e96-cbd0-4c90-be86-89fe38e3dd76
📒 Files selected for processing (3)
examples/ejemplos-carta-porte-referencias.tsexamples/ejemplos-carta-porte-valores.tssrc/models/invoice.ts
|
|
||
| /** Complemento de carta porte */ | ||
| lading?: LadingComplement; | ||
| cartaPorte?: LadingComplement; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether any other source/tests still reference the old `lading` field on Complement,
# and whether CHANGELOG/README mention this rename.
rg -nP '\bcomplement\s*\.\s*lading\b' --type=ts -C2
rg -nP '\blading\?\s*:\s*LadingComplement\b' --type=ts -C2
fd -i 'changelog|readme' -t f -d 3 --exec rg -nP -H 'cartaPorte|lading' {} \;Repository: FiscalAPI/fiscalapi-node
Length of output: 50
🏁 Script executed:
#!/bin/bash
# Confirm the actual code at line 107 and surrounding context
echo "=== Line 107 context in src/models/invoice.ts ==="
sed -n '100,115p' src/models/invoice.ts
# Check for CHANGELOG and README files
echo -e "\n=== CHANGELOG/README files ==="
find . -type f \( -iname 'changelog*' -o -iname 'readme*' \) -not -path './node_modules/*' | head -10
# Check index.ts exports for Complement and LadingComplement
echo -e "\n=== Checking index.ts for exports ==="
fd -i 'index.ts' -t f | xargs rg -nP 'Complement|LadingComplement' -A1 -B1 | head -50Repository: FiscalAPI/fiscalapi-node
Length of output: 1011
🏁 Script executed:
#!/bin/bash
# Check README.md for breaking changes section and migration info
echo "=== README.md content (checking for breaking changes) ==="
cat README.md | head -100
# Also check if there are any comments/history in invoice.ts about the rename
echo -e "\n=== Checking invoice.ts for comments about lading/cartaPorte ==="
rg -nP 'lading|cartaPorte' src/models/invoice.ts -B2 -A2 | head -30Repository: FiscalAPI/fiscalapi-node
Length of output: 4510
Breaking rename: Complement.lading → Complement.cartaPorte requires migration path or release documentation.
This is a breaking change for SDK consumers already using complement.lading. The public API exports Complement and LadingComplement (index.ts), so any consumer code will encounter compilation errors.
No deprecation alias is present and no breaking changes section exists in README or CHANGELOG. Implement one of the following:
- Add a deprecated alias (backward compatible) to give consumers a migration window, or
- Document this breaking change prominently in CHANGELOG and README under a "Breaking changes" section for this release.
Example: Transitional deprecation alias
/** Complemento de carta porte */
cartaPorte?: LadingComplement;
+
+ /**
+ * `@deprecated` Use `cartaPorte` instead. Will be removed in a future release.
+ */
+ lading?: LadingComplement;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/models/invoice.ts` at line 107, The public API renamed Complement.lading
to Complement.cartaPorte which breaks existing consumers; add a
backwards-compatible deprecation alias or document the break: either add a
deprecated getter/optional property named lading on the Complement type/class
that forwards to cartaPorte (and mark it with a deprecation
comment/ts-expect-deprecation) so existing code compiles, and update the
exported types (e.g., LadingComplement) in index.ts to re-export the alias, OR
add a prominent "Breaking changes" entry in README/CHANGELOG describing the
rename (Complement.lading → Complement.cartaPorte) and include migration
guidance and timeline for removal.
2 participants
Summary by CodeRabbit
New Features
Chores