[WIP] #851

jjscarafia · 2025-12-16T21:02:10Z

No description provided.

roboadhoc · 2025-12-16T21:02:13Z

Copilot

Pull request overview

Este PR añade funcionalidad para controlar la posición fiscal en el wizard de diferencias de cambio. Se introduce un nuevo mecanismo para prorratear diferencias de cambio entre líneas de factura agrupadas por impuestos, mejorando el tratamiento contable en contextos multi-impuesto (especialmente para localizaciones como Argentina).

Añade campos fiscal_position y fiscal_position_id para permitir selección automática o manual de posición fiscal
Modifica _prepare_debit_credit_note para aceptar exch_moves como parámetro y aplicar la posición fiscal seleccionada
Implementa nuevo método _prepare_invoice_lines_by_taxes que agrupa y prorratea diferencias de cambio por combinación de impuestos

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 8 comments.

File	Description
account_exchange_difference_invoice/wizards/exchange_difference_wizard.py	Añade campos de posición fiscal al wizard, implementa lógica de prorrateo de diferencias de cambio por grupo de impuestos, y modifica la creación de notas de débito/crédito para aplicar posición fiscal manual
account_exchange_difference_invoice/wizards/exchange_difference_wizard_views.xml	Añade campos de UI para seleccionar modo de posición fiscal (automático/manual) y campo condicional para seleccionar posición fiscal específica

Copilot · 2025-12-16T21:16:59Z

account_exchange_difference_invoice/wizards/exchange_difference_wizard.py

        }

+        if self.wizard_id.fiscal_position == "manual" and self.wizard_id.fiscal_position_id:
+            invoice_vals["fiscal_position_id"] = self.wizard_id.fiscal_position_id.id


Cuando fiscal_position == "automatic" (el valor por defecto), no se establece explícitamente fiscal_position_id en invoice_vals. Esto significa que Odoo aplicará su lógica automática de detección de posición fiscal. Sin embargo, podría ser más claro añadir un comentario explicando este comportamiento o considerar establecer explícitamente fiscal_position_id como False cuando sea "automatic" para mayor claridad del código.

Suggested change

invoice_vals["fiscal_position_id"] = self.wizard_id.fiscal_position_id.id

invoice_vals["fiscal_position_id"] = self.wizard_id.fiscal_position_id.id

elif self.wizard_id.fiscal_position == "automatic":

# Si la posición fiscal es automática, establecemos explícitamente fiscal_position_id en False

# para dejar claro que Odoo aplicará su lógica automática de detección de posición fiscal.

invoice_vals["fiscal_position_id"] = False

Copilot · 2025-12-16T21:16:59Z

account_exchange_difference_invoice/wizards/exchange_difference_wizard.py

+        invoice_line_vals = []
+        for exchange_move in exch_moves:
+            partial_reconcile = self.env["account.partial.reconcile"].search(
+                [("exchange_move_id", "=", exchange_move.ids)]


El operador de comparación está incorrecto. Se utiliza "=" (igualdad) cuando debería usarse "in" (pertenencia) ya que exchange_move.ids es una lista. Esto causará un error en tiempo de ejecución porque el dominio esperaría comparar un campo con un valor escalar, no con una lista.

Suggested change

[("exchange_move_id", "=", exchange_move.ids)]

[("exchange_move_id", "in", exchange_move.ids)]

Copilot · 2025-12-16T21:17:00Z

account_exchange_difference_invoice/wizards/exchange_difference_wizard.py

+        # approach si hacemos una sola linea
+        # partial_reconciles = self.env["account.partial.reconcile"].search(
+        #     [("exchange_move_id", "in", exch_moves.ids)]
+        # )
+        # invoice_lines = (partial_reconciles.mapped("debit_move_id") + partial_reconciles.mapped("credit_move_id")).mapped("move_id").filtered(lambda move: move.is_sale_document()).mapped("invoice_line_ids")
+        # invoice_line_vals = self._prepare_invoice_lines_by_taxes(invoice_lines, account, self.balance)


El código comentado debe eliminarse antes del merge. Si estos enfoques alternativos son necesarios para referencia futura, deberían documentarse en el historial de commits o en un comentario más conciso que explique la decisión de diseño, no dejando código completo comentado.

Copilot · 2025-12-16T21:17:00Z

account_exchange_difference_invoice/wizards/exchange_difference_wizard.py

+
+        # Calcular el total de subtotales para el prorrateo
+        total_subtotal = sum(group["subtotal"] for group in tax_groups.values())
+        # total_subtotal = sum(invoice_lines.mapped("price_total"))


Este código comentado debe eliminarse antes del merge. Si se consideró este cálculo alternativo, la decisión de usar el enfoque actual debería estar documentada en un comentario conciso o en el historial de commits.

Suggested change

# total_subtotal = sum(invoice_lines.mapped("price_total"))

Copilot · 2025-12-16T21:17:00Z

account_exchange_difference_invoice/wizards/exchange_difference_wizard.py

+        # invoice_lines = (partial_reconciles.mapped("debit_move_id") + partial_reconciles.mapped("credit_move_id")).mapped("move_id").filtered(lambda move: move.is_sale_document()).mapped("invoice_line_ids")
+        # invoice_line_vals = self._prepare_invoice_lines_by_taxes(invoice_lines, account, self.balance)
+
+        # en este approach iteramos sobre cada exhcange diff y generamos lineas para cada grupo de impuestos


Error ortográfico en el comentario: "exhcange" debería ser "exchange".

Suggested change

# en este approach iteramos sobre cada exhcange diff y generamos lineas para cada grupo de impuestos

# en este approach iteramos sobre cada exchange diff y generamos lineas para cada grupo de impuestos

Copilot · 2025-12-16T21:17:00Z

account_exchange_difference_invoice/wizards/exchange_difference_wizard.py

+        for exchange_move in exch_moves:
+            partial_reconcile = self.env["account.partial.reconcile"].search(
+                [("exchange_move_id", "=", exchange_move.ids)]
+            )


Se está ejecutando una búsqueda (search) dentro de un bucle for, lo cual es ineficiente. Esto genera N consultas a la base de datos (una por cada exchange_move). Sería más eficiente realizar una única búsqueda con el dominio [("exchange_move_id", "in", exch_moves.ids)] antes del bucle y luego iterar sobre los resultados agrupados, o usar un grouped/mapped si es posible.

Suggested change

for exchange_move in exch_moves:

partial_reconcile = self.env["account.partial.reconcile"].search(

[("exchange_move_id", "=", exchange_move.ids)]

)

# Buscar todos los partial_reconcile de una sola vez

all_partial_reconciles = self.env["account.partial.reconcile"].search(

[("exchange_move_id", "in", exch_moves.ids)]

)

# Agrupar por exchange_move_id

partials_by_exchange = {}

for pr in all_partial_reconciles:

exchange_id = pr.exchange_move_id.id

partials_by_exchange.setdefault(exchange_id, self.env["account.partial.reconcile"].browse()). |= pr

for exchange_move in exch_moves:

partial_reconcile = partials_by_exchange.get(exchange_move.id, self.env["account.partial.reconcile"].browse())

Copilot · 2025-12-16T21:17:01Z

account_exchange_difference_invoice/wizards/exchange_difference_wizard_views.xml

            <form>
                <group>
                    <field name="journal_id"/>
+                    <field name="fiscal_position"/>


El campo fiscal_position debería incluir el atributo string para definir la etiqueta del campo en la interfaz de usuario. Esto mejora la claridad y mantiene la consistencia con otros campos del formulario.

Copilot · 2025-12-16T21:17:01Z

account_exchange_difference_invoice/wizards/exchange_difference_wizard.py

+        help="If automatic, fiscal position will be auto detected for each customer, if manual you can force one or none fiscal position.",
+    )
+    fiscal_position_id = fields.Many2one(
+        "account.fiscal.position", string="Fiscal Position", help="Fiscal position to use for all the customers."


El campo fiscal_position_id de tipo Many2one hacia account.fiscal.position debería incluir el atributo check_company=True para garantizar que la posición fiscal seleccionada pertenezca a la misma compañía del wizard. Esto es especialmente importante dado que ExchangeDifferenceWizard usa _check_company_auto = True.

Suggested change

"account.fiscal.position", string="Fiscal Position", help="Fiscal position to use for all the customers."

"account.fiscal.position", string="Fiscal Position", help="Fiscal position to use for all the customers.", check_company=True

…osition support This commit improves the exchange difference wizard to properly handle taxes and fiscal positions when creating debit/credit notes. Main changes: - Add fiscal_position field to wizard (automatic/manual selection) - Add fiscal_position_id field for manual fiscal position assignment - Implement tax proration logic in _prepare_invoice_lines_by_taxes(): * Groups invoice lines by tax combination * For Argentina: only considers VAT taxes (with l10n_ar_vat_afip_code) * Prorates exchange difference amount across tax groups based on their weight * Handles rounding differences by adjusting the last line - Refactor _prepare_debit_credit_note() to: * Accept exch_moves parameter to retrieve related invoice lines * Generate separate lines for each tax combination * Apply manual fiscal position when configured - Update wizard view to show fiscal position fields This ensures exchange difference debit/credit notes have the same tax structure as the original invoices, maintaining proper tax tracking and compliance.

…osition When calculating tax groups for proration in the exchange difference wizard, we need to ensure that if the invoice has a fiscal position, the perceptions and other tax amounts should not be included in the subtotal used for proration. This case only applies if the user selects automatic fiscal position application in the wizard. Also, if user selects manual and not fiscal position id, the fiscal position will be cleaned from the invoice to avoid re-calculation taxes when adding the date.

cav-adhoc · 2026-01-05T14:32:43Z

@roboadhoc r+

roboadhoc · 2026-01-05T14:32:53Z

@jjscarafia @cav-adhoc because this PR has multiple commits, I need to know how to merge it:

merge to merge directly, using the PR as merge commit message
rebase-merge to rebase and merge, using the PR as merge commit message
rebase-ff to rebase and fast-forward

cav-adhoc · 2026-01-05T14:35:04Z

@roboadhoc rebase-ff

…osition support This commit improves the exchange difference wizard to properly handle taxes and fiscal positions when creating debit/credit notes. Main changes: - Add fiscal_position field to wizard (automatic/manual selection) - Add fiscal_position_id field for manual fiscal position assignment - Implement tax proration logic in _prepare_invoice_lines_by_taxes(): * Groups invoice lines by tax combination * For Argentina: only considers VAT taxes (with l10n_ar_vat_afip_code) * Prorates exchange difference amount across tax groups based on their weight * Handles rounding differences by adjusting the last line - Refactor _prepare_debit_credit_note() to: * Accept exch_moves parameter to retrieve related invoice lines * Generate separate lines for each tax combination * Apply manual fiscal position when configured - Update wizard view to show fiscal position fields This ensures exchange difference debit/credit notes have the same tax structure as the original invoices, maintaining proper tax tracking and compliance. Part-of: #851 Signed-off-by: Camila Vives <[email protected]>

…osition When calculating tax groups for proration in the exchange difference wizard, we need to ensure that if the invoice has a fiscal position, the perceptions and other tax amounts should not be included in the subtotal used for proration. This case only applies if the user selects automatic fiscal position application in the wizard. Also, if user selects manual and not fiscal position id, the fiscal position will be cleaned from the invoice to avoid re-calculation taxes when adding the date. closes #851 Signed-off-by: Camila Vives <[email protected]>

roboadhoc · 2026-01-05T14:36:35Z

Merge method set to rebase and fast-forward.

Copilot AI review requested due to automatic review settings December 16, 2025 21:02

Copilot started reviewing on behalf of jjscarafia December 16, 2025 21:02 View session

Copilot AI reviewed Dec 16, 2025

View reviewed changes

cav-adhoc force-pushed the 18.0-t-61301-jjs branch 3 times, most recently from 568d47c to 0c75b1f Compare December 26, 2025 15:37

cav-adhoc force-pushed the 18.0-t-61301-jjs branch from 0c75b1f to 82de292 Compare January 5, 2026 14:28

cav-adhoc force-pushed the 18.0-t-61301-jjs branch from 82de292 to f008011 Compare January 5, 2026 14:29

roboadhoc temporarily deployed to merge January 5, 2026 14:36 Inactive

roboadhoc closed this Jan 5, 2026

fw-bot-adhoc mentioned this pull request Jan 5, 2026

[WIP] #865

Closed

roboadhoc deleted the 18.0-t-61301-jjs branch January 5, 2026 14:36

-            invoice_vals["fiscal_position_id"] = self.wizard_id.fiscal_position_id.id
+            invoice_vals["fiscal_position_id"] = self.wizard_id.fiscal_position_id.id
+        elif self.wizard_id.fiscal_position == "automatic":
+            # Si la posición fiscal es automática, establecemos explícitamente fiscal_position_id en False
+            # para dejar claro que Odoo aplicará su lógica automática de detección de posición fiscal.
+            invoice_vals["fiscal_position_id"] = False

	[("exchange_move_id", "=", exchange_move.ids)]
	[("exchange_move_id", "in", exchange_move.ids)]

	# en este approach iteramos sobre cada exhcange diff y generamos lineas para cada grupo de impuestos
	# en este approach iteramos sobre cada exchange diff y generamos lineas para cada grupo de impuestos

-        for exchange_move in exch_moves:
-            partial_reconcile = self.env["account.partial.reconcile"].search(
-                [("exchange_move_id", "=", exchange_move.ids)]
-            )
+        # Buscar todos los partial_reconcile de una sola vez
+        all_partial_reconciles = self.env["account.partial.reconcile"].search(
+            [("exchange_move_id", "in", exch_moves.ids)]
+        )
+        # Agrupar por exchange_move_id
+        partials_by_exchange = {}
+        for pr in all_partial_reconciles:
+            exchange_id = pr.exchange_move_id.id
+            partials_by_exchange.setdefault(exchange_id, self.env["account.partial.reconcile"].browse()). |= pr
+        for exchange_move in exch_moves:
+            partial_reconcile = partials_by_exchange.get(exchange_move.id, self.env["account.partial.reconcile"].browse())

	"account.fiscal.position", string="Fiscal Position", help="Fiscal position to use for all the customers."
	"account.fiscal.position", string="Fiscal Position", help="Fiscal position to use for all the customers.", check_company=True

[WIP] #851

[WIP] #851

Uh oh!

Conversation

jjscarafia commented Dec 16, 2025

Uh oh!

roboadhoc commented Dec 16, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

cav-adhoc commented Jan 5, 2026

Uh oh!

roboadhoc commented Jan 5, 2026

Uh oh!

cav-adhoc commented Jan 5, 2026

Uh oh!

roboadhoc commented Jan 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants