[IMP] l10n_latam_check_ux: Added restriction on checks delete by feg-adhoc · Pull Request #659 · ingadhoc/account-payment

feg-adhoc · 2025-03-25T12:05:17Z

No description provided.

roboadhoc · 2025-03-25T12:05:46Z

roboadhoc · 2025-09-08T12:52:51Z

roboadhoc · 2025-09-08T12:52:52Z

@feg-adhoc I didn't know about this PR and had to retrieve its information, you may have to re-approve it as I didn't see previous commands.

Esto es necesario porque ahora en pagos de clientes el onchage de withholdings no computa amount Además, sin l10n_Ar_tax no estamos mandaando el amount Quedaría una mejora "linda" que sería evaluar si el default journal que se va a usar es en otra moneda, tal vez deberíamos convertir el importe que se manda a esa otra moenda (pero la verdad no parece ser un caso necesario por el momento) closes ingadhoc#765 Related: ingadhoc/odoo-argentina#1189 Signed-off-by: Camila Vives <cav@adhoc.inc>

Copilot

Pull request overview

Este PR añade restricciones para prevenir la eliminación y modificación de cheques que están siendo utilizados en transacciones procesadas o pagadas. La implementación incluye validaciones tanto en el modelo de cheques como en el modelo de pagos para mantener la integridad de datos.

Se añade _check_unlink_restriction() para prevenir la eliminación de cheques con transacciones activas
Se añade _onchange_check_data() para prevenir modificaciones en datos críticos de cheques en uso
Se implementa _checks_state() en account.payment para validar en cascada al eliminar pagos

Reviewed changes

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

File	Description
l10n_latam_check_ux/models/l10n_latam_check.py	Añade métodos de validación `@api.ondelete` y `@api.onchange` para proteger cheques contra eliminación y modificación cuando están en transacciones activas
l10n_latam_check_ux/models/account_payment.py	Implementa validación `@api.ondelete` para invocar restricciones de cheques cuando se elimina un pago, asegurando validación en cascada

Copilot · 2025-12-01T13:48:38Z

l10n_latam_check_ux/models/l10n_latam_check.py

+    def _check_unlink_restriction(self):
+        for check in self:
+            if any(payment.state in ["in_process", "paid"] for payment in check.operation_ids):
+                raise UserError("You must unlink all transactions related to this check before deleting it.")


Los mensajes de error deben estar envueltos con la función _() para permitir su traducción. Según la convención del módulo (ver account_check_action_wizard.py), todos los mensajes de error para el usuario deberían ser traducibles.

Sugerencia:

raise UserError(_("You must unlink all transactions related to this check before deleting it."))

Suggested change

raise UserError("You must unlink all transactions related to this check before deleting it.")

raise UserError(_("You must unlink all transactions related to this check before deleting it."))

Copilot · 2025-12-01T13:48:39Z

l10n_latam_check_ux/models/l10n_latam_check.py

+    def _onchange_check_data(self):
+        for check in self:
+            if any(payment.state in ["in_process", "paid"] for payment in check.operation_ids):
+                raise UserError("You cannot change the check data because it is already used in a transaction.")


Los mensajes de error deben estar envueltos con la función _() para permitir su traducción. Según la convención del módulo (ver account_check_action_wizard.py), todos los mensajes de error para el usuario deberían ser traducibles.

Sugerencia:

raise UserError(_("You cannot change the check data because it is already used in a transaction."))

Suggested change

raise UserError("You cannot change the check data because it is already used in a transaction.")

raise UserError(_("You cannot change the check data because it is already used in a transaction."))

Copilot · 2025-12-01T13:48:39Z

l10n_latam_check_ux/models/l10n_latam_check.py

+            if any(payment.state in ["in_process", "paid"] for payment in check.operation_ids):
+                raise UserError("You must unlink all transactions related to this check before deleting it.")
+
+    @api.onchange("amount", "payment_date", "name")
+    def _onchange_check_data(self):
+        for check in self:
+            if any(payment.state in ["in_process", "paid"] for payment in check.operation_ids):


La validación solo verifica operation_ids pero no incluye payment_id. Según el código existente (líneas 30, 36, 45, 65), un cheque puede tener transacciones tanto en payment_id como en operation_ids. Si payment_id tiene estado "in_process" o "paid", el cheque también debería estar protegido contra eliminación.

Sugerencia:

if check.payment_id.state in ["in_process", "paid"] or any(payment.state in ["in_process", "paid"] for payment in check.operation_ids):

Suggested change

if any(payment.state in ["in_process", "paid"] for payment in check.operation_ids):

raise UserError("You must unlink all transactions related to this check before deleting it.")

@api.onchange("amount", "payment_date", "name")

def _onchange_check_data(self):

for check in self:

if any(payment.state in ["in_process", "paid"] for payment in check.operation_ids):

if check.payment_id.state in ["in_process", "paid"] or any(payment.state in ["in_process", "paid"] for payment in check.operation_ids):

raise UserError("You must unlink all transactions related to this check before deleting it.")

@api.onchange("amount", "payment_date", "name")

def _onchange_check_data(self):

for check in self:

if check.payment_id.state in ["in_process", "paid"] or any(payment.state in ["in_process", "paid"] for payment in check.operation_ids):

Copilot · 2025-12-01T13:48:39Z

l10n_latam_check_ux/models/l10n_latam_check.py

+            if any(payment.state in ["in_process", "paid"] for payment in check.operation_ids):
+                raise UserError("You must unlink all transactions related to this check before deleting it.")
+
+    @api.onchange("amount", "payment_date", "name")
+    def _onchange_check_data(self):
+        for check in self:
+            if any(payment.state in ["in_process", "paid"] for payment in check.operation_ids):


La validación solo verifica operation_ids pero no incluye payment_id. Según el código existente (líneas 30, 36, 45, 65), un cheque puede tener transacciones tanto en payment_id como en operation_ids. Si payment_id tiene estado "in_process" o "paid", los datos del cheque también deberían estar protegidos contra modificación.

Sugerencia:

if check.payment_id.state in ["in_process", "paid"] or any(payment.state in ["in_process", "paid"] for payment in check.operation_ids):

Suggested change

if any(payment.state in ["in_process", "paid"] for payment in check.operation_ids):

raise UserError("You must unlink all transactions related to this check before deleting it.")

@api.onchange("amount", "payment_date", "name")

def _onchange_check_data(self):

for check in self:

if any(payment.state in ["in_process", "paid"] for payment in check.operation_ids):

if (check.payment_id.state in ["in_process", "paid"] or

any(payment.state in ["in_process", "paid"] for payment in check.operation_ids)):

raise UserError("You must unlink all transactions related to this check before deleting it.")

@api.onchange("amount", "payment_date", "name")

def _onchange_check_data(self):

for check in self:

if (check.payment_id.state in ["in_process", "paid"] or

any(payment.state in ["in_process", "paid"] for payment in check.operation_ids)):

Copilot · 2025-12-01T13:48:40Z

l10n_latam_check_ux/models/account_payment.py

        super().action_draft()
+
+    @api.ondelete(at_uninstall=False)
+    def _checks_state(self):


El nombre del método _checks_state es poco descriptivo y no sigue la convención de Odoo para métodos @api.ondelete. Debería indicar claramente que valida restricciones de eliminación.

Sugerencia: renombrar a _check_new_checks_unlink_restriction para mayor claridad y consistencia con _check_unlink_restriction en el modelo de cheques.

Suggested change

def _checks_state(self):

def _check_new_checks_unlink_restriction(self):

Copilot · 2025-12-01T13:48:40Z

l10n_latam_check_ux/models/account_payment.py

+            if rec.l10n_latam_new_check_ids:
+                rec.l10n_latam_new_check_ids._check_unlink_restriction()


La validación solo verifica l10n_latam_new_check_ids, pero según la línea 88, los pagos también pueden tener cheques en l10n_latam_move_check_ids. Estos cheques también deberían validarse antes de eliminar un pago.

Sugerencia:

all_checks = rec.l10n_latam_new_check_ids + rec.l10n_latam_move_check_ids if all_checks: all_checks._check_unlink_restriction()

Suggested change

if rec.l10n_latam_new_check_ids:

rec.l10n_latam_new_check_ids._check_unlink_restriction()

all_checks = rec.l10n_latam_new_check_ids + rec.l10n_latam_move_check_ids

if all_checks:

all_checks._check_unlink_restriction()

feg-adhoc force-pushed the 18.0-t-42765-feg branch from 19e8974 to 751933c Compare April 11, 2025 12:44

feg-adhoc changed the title ~~[ADD] l10n_latam_check: Added restriction on checks delete~~ [ADD] l10n_latam_check_ux: Added restriction on checks delete Apr 11, 2025

feg-adhoc changed the title ~~[ADD] l10n_latam_check_ux: Added restriction on checks delete~~ [IMP] l10n_latam_check_ux: Added restriction on checks delete May 29, 2025

feg-adhoc force-pushed the 18.0-t-42765-feg branch from 751933c to 2c28c88 Compare May 29, 2025 19:13

feg-adhoc force-pushed the 18.0-t-42765-feg branch 4 times, most recently from 25186b2 to 480ab5a Compare June 27, 2025 23:04

feg-adhoc force-pushed the 18.0-t-42765-feg branch from 3ed3e64 to 8f68dfc Compare August 5, 2025 15:30

feg-adhoc force-pushed the 18.0-t-42765-feg branch from 8f68dfc to 0bb4c49 Compare September 8, 2025 12:52

feg-adhoc force-pushed the 18.0-t-42765-feg branch 4 times, most recently from 534c7f7 to d3cf885 Compare September 8, 2025 12:58

feg-adhoc force-pushed the 18.0-t-42765-feg branch from d3cf885 to fb14a92 Compare December 1, 2025 13:44

Copilot AI review requested due to automatic review settings December 1, 2025 13:44

Copilot started reviewing on behalf of feg-adhoc December 1, 2025 13:44 View session

Copilot finished reviewing on behalf of feg-adhoc December 1, 2025 13:47

Copilot AI reviewed Dec 1, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

[IMP] l10n_latam_check_ux: Added restriction on checks delete#659

[IMP] l10n_latam_check_ux: Added restriction on checks delete#659
feg-adhoc wants to merge 1 commit intoingadhoc:18.0from
adhoc-dev:18.0-t-42765-feg

feg-adhoc commented Mar 25, 2025

Uh oh!

roboadhoc commented Mar 25, 2025

Uh oh!

roboadhoc commented Sep 8, 2025

Uh oh!

roboadhoc commented Sep 8, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 1, 2025

Uh oh!

Copilot AI Dec 1, 2025

Uh oh!

Copilot AI Dec 1, 2025

Uh oh!

Copilot AI Dec 1, 2025

Uh oh!

Copilot AI Dec 1, 2025

Uh oh!

Copilot AI Dec 1, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	raise UserError("You must unlink all transactions related to this check before deleting it.")
	raise UserError(_("You must unlink all transactions related to this check before deleting it."))

	raise UserError("You cannot change the check data because it is already used in a transaction.")
	raise UserError(_("You cannot change the check data because it is already used in a transaction."))

	def _checks_state(self):
	def _check_new_checks_unlink_restriction(self):

		if rec.l10n_latam_new_check_ids:
		rec.l10n_latam_new_check_ids._check_unlink_restriction()

-            if rec.l10n_latam_new_check_ids:
-                rec.l10n_latam_new_check_ids._check_unlink_restriction()
+            all_checks = rec.l10n_latam_new_check_ids + rec.l10n_latam_move_check_ids
+            if all_checks:
+                all_checks._check_unlink_restriction()

Comments

Conversation

feg-adhoc commented Mar 25, 2025

Uh oh!

roboadhoc commented Mar 25, 2025

Uh oh!

roboadhoc commented Sep 8, 2025

Uh oh!

roboadhoc commented Sep 8, 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 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants