[IMP] account_invoice_tax: manage tax overrides on account moves#281
[IMP] account_invoice_tax: manage tax overrides on account moves#281cav-adhoc wants to merge 1 commit intoingadhoc:19.0from
Conversation
There was a problem hiding this comment.
Pull request overview
Este PR mejora la gestión de tax overrides en facturas de proveedor moviendo la persistencia a un campo JSON en account.move, y asegurando que los overrides se re-apliquen tras recomputaciones y se reflejen en el widget de totales de impuestos.
Changes:
- Agrega
tax_override_data(JSON) enaccount.movey lógica para aplicar/inocular overrides en recomputaciones y entax_totals. - Refactoriza el wizard para persistir overrides en el JSON y luego aplicarlos a las líneas de impuesto.
- Incorpora una post-migración para poblar
tax_override_dataen movimientos recientes.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| account_invoice_tax/wizards/account_invoice_tax.py | Guarda overrides del wizard en JSON y delega la aplicación a _apply_tax_overrides(). |
| account_invoice_tax/models/account_move.py | Define el JSON y la lógica de sync/aplicación de overrides + hook en recomputación de tax lines y tax totals. |
| account_invoice_tax/models/account_tax.py | Ajusta el resumen del widget de impuestos en base a overrides inyectados por contexto. |
| account_invoice_tax/models/init.py | Exporta los nuevos modelos. |
| account_invoice_tax/migrations/19.0.1.1.0/post-migration.py | Migra datos recientes para inicializar tax_override_data. |
| account_invoice_tax/manifest.py | Bump de versión del módulo a 19.0.1.1.0. |
| account_invoice_tax/init.py | Importa models para registrar las extensiones. |
| "credit": credit_cc if not_company_currency else credit, | ||
| "balance": ((amount_cc if not_company_currency else amount) * (1 if debit or debit_cc else -1)), | ||
| } | ||
| if not_company_currency and amount: |
There was a problem hiding this comment.
amount_currency solo se escribe si amount es truthy. Si un override fija el impuesto a 0 en moneda de factura, esta condición deja el amount_currency previo en la línea, pudiendo desbalancear/ensuciar el apunte. Para moneda extranjera conviene setear amount_currency siempre (incluyendo 0.0) cuando se aplica un override.
| if not_company_currency and amount: | |
| if not_company_currency: |
| for tax_group in res.get("subtotals", [{}])[0].get("tax_groups", []): | ||
| for involved_tax_id in tax_group.get("involved_tax_ids", []): | ||
| override = tax_context.get(involved_tax_id) | ||
| if not override: | ||
| continue | ||
| new_amount = override.get("fixed_amount", 0.0) | ||
| rate = override.get("rate", 1.0) | ||
| original_amount = tax_group.get("tax_amount", 0.0) | ||
| original_currency_amount = tax_group.get("tax_amount_currency", 0.0) | ||
| if not new_amount or new_amount == original_currency_amount: | ||
| continue | ||
| diff = new_amount - original_currency_amount | ||
| currency_diff = new_amount / rate - original_amount | ||
| tax_group.update( | ||
| { | ||
| "tax_amount": new_amount / rate, | ||
| "tax_amount_currency": new_amount, | ||
| } | ||
| ) | ||
| res["tax_amount"] += diff | ||
| res["total_amount"] += diff | ||
| res["tax_amount_currency"] += currency_diff | ||
| res["total_amount_currency"] += currency_diff |
There was a problem hiding this comment.
Se está iterando solo el primer subtotal (res.get('subtotals')[0]). En tax_totals puede haber múltiples subtotales; los overrides deberían aplicarse en todos para que el widget y los totales queden consistentes.
| for tax_group in res.get("subtotals", [{}])[0].get("tax_groups", []): | |
| for involved_tax_id in tax_group.get("involved_tax_ids", []): | |
| override = tax_context.get(involved_tax_id) | |
| if not override: | |
| continue | |
| new_amount = override.get("fixed_amount", 0.0) | |
| rate = override.get("rate", 1.0) | |
| original_amount = tax_group.get("tax_amount", 0.0) | |
| original_currency_amount = tax_group.get("tax_amount_currency", 0.0) | |
| if not new_amount or new_amount == original_currency_amount: | |
| continue | |
| diff = new_amount - original_currency_amount | |
| currency_diff = new_amount / rate - original_amount | |
| tax_group.update( | |
| { | |
| "tax_amount": new_amount / rate, | |
| "tax_amount_currency": new_amount, | |
| } | |
| ) | |
| res["tax_amount"] += diff | |
| res["total_amount"] += diff | |
| res["tax_amount_currency"] += currency_diff | |
| res["total_amount_currency"] += currency_diff | |
| for subtotal in res.get("subtotals", []): | |
| for tax_group in subtotal.get("tax_groups", []): | |
| for involved_tax_id in tax_group.get("involved_tax_ids", []): | |
| override = tax_context.get(involved_tax_id) | |
| if not override: | |
| continue | |
| new_amount = override.get("fixed_amount", 0.0) | |
| rate = override.get("rate", 1.0) | |
| original_amount = tax_group.get("tax_amount", 0.0) | |
| original_currency_amount = tax_group.get("tax_amount_currency", 0.0) | |
| if not new_amount or new_amount == original_currency_amount: | |
| continue | |
| diff = new_amount - original_currency_amount | |
| currency_diff = new_amount / rate - original_amount | |
| tax_group.update( | |
| { | |
| "tax_amount": new_amount / rate, | |
| "tax_amount_currency": new_amount, | |
| } | |
| ) | |
| res["tax_amount"] += diff | |
| res["total_amount"] += diff | |
| res["tax_amount_currency"] += currency_diff | |
| res["total_amount_currency"] += currency_diff |
| new_amount = override.get("fixed_amount", 0.0) | ||
| rate = override.get("rate", 1.0) | ||
| original_amount = tax_group.get("tax_amount", 0.0) | ||
| original_currency_amount = tax_group.get("tax_amount_currency", 0.0) | ||
| if not new_amount or new_amount == original_currency_amount: |
There was a problem hiding this comment.
La condición if not new_amount ...: continue impide aplicar overrides a 0.0 (caso válido si el usuario quiere anular un impuesto fijo). Debería compararse solo contra el valor original, o distinguir explícitamente entre “no hay override” y “override = 0”.
| new_amount = override.get("fixed_amount", 0.0) | |
| rate = override.get("rate", 1.0) | |
| original_amount = tax_group.get("tax_amount", 0.0) | |
| original_currency_amount = tax_group.get("tax_amount_currency", 0.0) | |
| if not new_amount or new_amount == original_currency_amount: | |
| new_amount = override.get("fixed_amount") | |
| rate = override.get("rate", 1.0) | |
| original_amount = tax_group.get("tax_amount", 0.0) | |
| original_currency_amount = tax_group.get("tax_amount_currency", 0.0) | |
| if new_amount is None or new_amount == original_currency_amount: |
| Only fixed-amount taxes are persisted as overrides. Percentage-based | ||
| taxes are always recomputed automatically, so any stale entry for them | ||
| is removed. | ||
| """ | ||
| new_overrides = {} | ||
| move = self.move_id | ||
| for wizard_line in self.tax_line_ids.filtered(lambda l: l.tax_id.amount_type == "fixed"): | ||
| new_overrides[str(wizard_line.tax_id.id)] = { | ||
| "amount": wizard_line.amount, | ||
| "rate": self.move_id.invoice_currency_rate or 1.0, |
There was a problem hiding this comment.
En el JSON se está guardando rate (tipo de cambio) junto con el amount, pero en el resto del flujo la lógica usa siempre move.invoice_currency_rate al aplicar/mostrar overrides. Esto puede dejar el dato almacenado sin uso o, peor, incoherente si el tipo de cambio del move cambia luego; conviene o bien usar el rate guardado al aplicar/inyectar el contexto, o dejar de persistirlo para evitar ambigüedad.
| Only fixed-amount taxes are persisted as overrides. Percentage-based | |
| taxes are always recomputed automatically, so any stale entry for them | |
| is removed. | |
| """ | |
| new_overrides = {} | |
| move = self.move_id | |
| for wizard_line in self.tax_line_ids.filtered(lambda l: l.tax_id.amount_type == "fixed"): | |
| new_overrides[str(wizard_line.tax_id.id)] = { | |
| "amount": wizard_line.amount, | |
| "rate": self.move_id.invoice_currency_rate or 1.0, | |
| Only fixed-amount taxes are persisted as overrides. Percentage-based | |
| taxes are always recomputed automatically, so any stale entry for them | |
| is removed. Only the overridden amount is stored to avoid persisting | |
| exchange-rate data that may become inconsistent with the move later. | |
| """ | |
| new_overrides = {} | |
| move = self.move_id | |
| for wizard_line in self.tax_line_ids.filtered(lambda l: l.tax_id.amount_type == "fixed"): | |
| new_overrides[str(wizard_line.tax_id.id)] = { | |
| "amount": wizard_line.amount, |
| # Structure: { "<tax_id>": {"amount": float, "rate": float} } | ||
| # Only fixed-amount taxes are stored here; percentage taxes are always | ||
| # recomputed automatically. | ||
| tax_override_data = fields.Json() |
There was a problem hiding this comment.
tax_override_data debería declararse con copy=False (como en el PR original) para que al duplicar una factura/nota de crédito no se copien overrides manuales de impuestos a un documento nuevo, lo que puede generar importes incorrectos.
| tax_override_data = fields.Json() | |
| tax_override_data = fields.Json(copy=False) |
| "fixed_amount": (vals["amount"]), | ||
| "rate": (move.invoice_currency_rate or 1.0), |
There was a problem hiding this comment.
En _compute_tax_totals se ignora el rate persistido en tax_override_data y se fuerza move.invoice_currency_rate. Si el usuario fijó el override con un tipo de cambio distinto (o el move cambia de fecha/moneda), el widget puede mostrar/importar conversiones inconsistentes. Usar vals.get('rate', ...) o eliminar el campo rate del JSON para que la estructura sea consistente.
| "fixed_amount": (vals["amount"]), | |
| "rate": (move.invoice_currency_rate or 1.0), | |
| "fixed_amount": vals["amount"], | |
| "rate": vals.get("rate", move.invoice_currency_rate or 1.0), |
| not_company_currency = move_currency and move_currency != company_currency | ||
| for line in self.line_ids.filtered(lambda l: l.tax_line_id and str(l.tax_line_id.id) in overrides): | ||
| vals = overrides[str(line.tax_line_id.id)] | ||
| rate = line.move_id.invoice_currency_rate or 1.0 |
There was a problem hiding this comment.
En _apply_tax_overrides se calcula amount_cc con invoice_currency_rate actual (no con el rate almacenado en tax_override_data). Eso hace que el débito/crédito en moneda compañía pueda cambiar “retroactivamente” si cambia el tipo de cambio del move. Conviene usar el rate guardado en vals (con fallback) para que la re-aplicación sea estable.
| rate = line.move_id.invoice_currency_rate or 1.0 | |
| rate = vals.get("rate") or line.move_id.invoice_currency_rate or 1.0 |
| res["tax_amount"] += diff | ||
| res["total_amount"] += diff | ||
| res["tax_amount_currency"] += currency_diff | ||
| res["total_amount_currency"] += currency_diff |
There was a problem hiding this comment.
diff se calcula como diferencia en moneda de factura (new_amount - original_currency_amount) y currency_diff como diferencia en moneda compañía (new_amount / rate - original_amount), pero luego se suman al revés en res['tax_amount']/res['tax_amount_currency'] (y sus totales). Esto termina ajustando los acumulados en la moneda incorrecta; conviene intercambiar esas actualizaciones para que cada key acumule en su moneda.
| res["tax_amount"] += diff | |
| res["total_amount"] += diff | |
| res["tax_amount_currency"] += currency_diff | |
| res["total_amount_currency"] += currency_diff | |
| res["tax_amount"] += currency_diff | |
| res["total_amount"] += currency_diff | |
| res["tax_amount_currency"] += diff | |
| res["total_amount_currency"] += diff |
| Targets invoices of type in_invoice / in_refund / in_receipt created in | ||
| the last month that: | ||
| - have at least one tax line linked to a fixed-amount tax, and | ||
| - have tax_override_data IS NULL (never set). | ||
|
|
||
| Processes up to BATCH_LIMIT records. |
There was a problem hiding this comment.
El docstring no coincide con la implementación: dice “last month” pero el script usa relativedelta(months=3), y menciona BATCH_LIMIT sin existir (se usa limit=1000). Conviene alinear doc/lógica o definir constantes para ambos.
7576595 to
7165d0f
Compare

FW of 9d1cd66 + fixes