Skip to content

[16.0][IMP] l10n_es_vat_prorate: Constrain para prevenir errores manuales #4166

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: 16.0
Choose a base branch
from

Conversation

EmilioPascual
Copy link
Contributor

La cuenta en una apunte marcado con la prorrata del IVA puede ser cambiado manualmente. Con esta restricción se evita que se puedan asingar cuentas no deseadas, como llevarlo al alguna cuenta que esté configurada en la distribución del impuesto.

MT-9939 @moduon

@EmilioPascual EmilioPascual changed the title [16.0][IMP] Constrain para prevenir errores manuales [16.0][IMP] l10n_es_vat_prorate: Constrain para prevenir errores manuales Apr 24, 2025
@EmilioPascual EmilioPascual force-pushed the 16.0-add-constrain_account_vat_prorate branch from 0f520df to fed2687 Compare April 24, 2025 12:11
@EmilioPascual
Copy link
Contributor Author

EmilioPascual commented Apr 29, 2025

Copy link
Member

@rafaelbn rafaelbn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Muchas gracias @EmilioPascual .

Visto el vídeo y visto en staging y prod. Gracias a esta mejora el módulo es más resiliente y maduro y evita errores que se nos han dado en los clientes y muy laborioso de encontrar y arreglar.

Perfecto! 💯 ❤️ 🎸 ⭐

Copy link
Contributor

@ArantxaSudon ArantxaSudon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Visto el vídeo y comprobado en el runboat. Perfecto!

Captura desde 2025-04-30 14-01-55

Gracias @EmilioPascual

@pedrobaeza pedrobaeza added this to the 16.0 milestone May 1, 2025
Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aunque comparto que está bien prevenir errores que pueda provocar el usuario, la forma de implementarlo impacta seriamente en el rendimiento, ya que el constraint tiene lugar siempre para todos los apuntes, y como mínimo, hay que comprobar el flag vat_prorate. Si el requisito es que no se permita cambiar la cuenta, que se haga el chequeo precisamente en el write, no siempre vía @api.constrains, que entiendo que además tiene todo ese follón de código de mapeo y demás para que no salte en la creación del apunte.

Además, se debería incluir un test que cubra el caso que se está añadiendo.

@EmilioPascual EmilioPascual force-pushed the 16.0-add-constrain_account_vat_prorate branch from fed2687 to 941e82c Compare May 5, 2025 15:12
@pedrobaeza
Copy link
Member

Por qué hace falta todo ese tema del mapeo de la cuenta contable? No se puede pasar un contexto cuando el cambio viene del propio Odoo, y prevenir todos los demás?

@EmilioPascual EmilioPascual force-pushed the 16.0-add-constrain_account_vat_prorate branch 2 times, most recently from 521cf51 to 88a1347 Compare May 8, 2025 15:13
@EmilioPascual
Copy link
Contributor Author

He vuelto al constrain porque después de hacer pruebas es más eficiente, el write se llama muchas veces aunque se haga una salida rápida mientras que el contrain sólo se llama cuando uno de los campos se actualiza.

En cuanto al mapeo es necesario porque las cuentas que se restringen son las cuentas de la reparticiones de impuestos del impuesto originador. Da igual que sea manualmente el cambio o sea el propio odoo el que cometiera el fallo, se quiere prevenir en todos los casos para evitar incongruencias.

@pedrobaeza
Copy link
Member

Pues yo personalmente creo que esta sobrecarga es demasiado con el escaso beneficio que trae. ¿Qué piensas tú, @etobella? Una opción sería añadir ese check en un módulo extra para así que fuera opcional. Como una configuración dentro de este módulo no lo plantearía, puesto que al final, como mínimo habría que comprobar esa configuración cada vez.

@etobella
Copy link
Member

etobella commented May 8, 2025

Personalmente es un mal uso del cliente. Yo no veo añadirlo. Al final es explicarle por que no debe cambiar los impuestos por que si.

@rafaelbn
Copy link
Member

@etobella , personalmente no es un mal uso del cliente si no una debilidad del software mal programado permitiendo errores en usos normales. Es decir el software no es resiliente.

Gracias a todos por las revisiones y el entendimiento.

@etobella
Copy link
Member

Estais proponiendo esto solo para este modulo, cuando puede pasar con cualquier impuesto. Si os parece grave, podeis hacer algo genérico para que los usuarioa no puedan tocar ciertas cosas de los impuestos, no solo en la prorata. Al final es igual si cambian cosas fuera de la prorata no? O si ponen la cuenta de capital social en una factura. No deben hacerlo pero pueden.

Al final, IMO es cuestión de formar correctamente a los usuarios.

@yajo yajo force-pushed the 16.0-add-constrain_account_vat_prorate branch from 88a1347 to 3a24b3f Compare July 31, 2025 08:58
The account can be changed manually and you might find move lines with vat prorate in a undesired account

MT-9939 @moduon
@yajo yajo force-pushed the 16.0-add-constrain_account_vat_prorate branch from 3a24b3f to 27bf0e2 Compare July 31, 2025 11:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants