-
-
Notifications
You must be signed in to change notification settings - Fork 520
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
[16.0][IMP] l10n_es_aeat: NIF del representante legal por defecto #3389
[16.0][IMP] l10n_es_aeat: NIF del representante legal por defecto #3389
Conversation
Hi @pedrobaeza, |
Entiendo que podría sacarse del empleado del usuario actual, pero puede que no esté instado RRHH y suele ser el CFO, así que me parece bien este cambio |
No entiendo esta aportación, por ejemplo, en el modelo 349 (página 9): https://www.agenciatributaria.es/static_files/Sede/Procedimiento_ayuda/GI28/instr_mod_349.pdf Si no es menor de 14 años debe aparecer vacío. En caso contrario se mostrará este error en la AEAT: *** No he hecho pruebas, así que quizá no lo estoy interpretando bien cual es el objetivo, por si lo podéis aclarar. Gracias, |
ed38c5e
to
7db714f
Compare
Lo que hace esta mejora es que puedes configurar (opcional) un NIF del representante legal en la compañía y cada vez que generes un modelo de AEAT el NIF del representante legal se rellenara automáticamente. Luego puede modificarse y poner otro. He grabado un pequeño vídeo con el flujo |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Gracias @EmilioPascual , una gran mejora!
This PR has the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incluye también la traducción.
Aquí tiene más valor hacerlo así que con un valor por defecto, ya que es para todas las declaraciones que existan (111, 115, 303...), así que no desestimable como el otro PR.
380c757
to
3079333
Compare
3079333
to
7772e12
Compare
El fallo de los tests parece general. Ver fix en moduon/oca-ci#1. |
7772e12
to
b71c29b
Compare
test-requirements.txt
Outdated
@@ -1 +1,2 @@ | |||
odoo_test_helper | |||
-c https://github.com/odoo/odoo/raw/16.0/requirements.txt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Este commit sobraría
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hay un problema que estamos viendo en OCA/oca-ci#66 por si quieres contexto, pero sin esto el build se peta. Claro, con esto se peta por otro lado... No es cosa fácil.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haz rebase cuando se fusione #3411 para ver si dejas de tener el problema.
739cdce
to
9c25936
Compare
Puedes eliminar el commit del build? |
9c25936
to
555d2c2
Compare
@@ -135,6 +135,8 @@ def _get_export_config(self, date): | |||
readonly=True, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
readonly=True, | |
readonly=False, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No se si es buena idea dejar cambiar el valor en una etapa que no sea borrador. Este campo se informa en el txt que se genera en muchos modelos
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Es que al hacerlo computed writable, la definición debe ser de esa forma. Luego en la vista invierte el readonly que exista para que eso, solo se pueda cambiar como estaba antes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No te estoy siguiendo.
Tal y como está ahora, que era como estaba antes, sólo se puede editar en borrador y no en el resto. ¿Me estoy perdiendo algo?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ésa es la cuestión, que no está como antes, porque ahora le añades compute=...
y store=True
, y por eso, hay que ponerlo readonly=False
para convertirlo en un computed writable (si no es un computed almacenado en su lugar). En el archivo XML, habrá que invertir las condiciones de readonly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pero con esto creo que queda resuelto:
states={"draft": [("readonly", False)]},
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No es lo mismo. Me suena que este debate lo tuve igual con tu compañero @Shide. El tema es que si la definición en el campo no es correcta, aunque ahora mismo funcione por la implementación del cliente web + framework, a nivel de lo esperable en ORM no será un computed writable, si no un simple computed. Eso puede dar lugar a que en un futuro, se descarten los valores escritos en un momento del pipeline debido a que el campo no debe escribir valores al ser un computed normal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actualice y lo puse readonly=False
. Revisa de nuevo por favor y me dices si ves algo más. Gracias
555d2c2
to
aa85e2d
Compare
aa85e2d
to
12a7edf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/ocabot merge minor
On my way to merge this fine PR! |
This PR has the |
Congratulations, your PR was merged at a8d420f. Thanks a lot for contributing to OCA. ❤️ |
Cada vez que generabas un nuevo modelo de AEAT había que rellenar el NIF del representante legal.
Con esta mejora puedes configurar el NIF del representante legal en la compañía y que se rellene siempre por defecto.
De todas formas, se puede modificar a posteriori.
@rafaelbn @ArantxaSudon @loida-vm @Shide @pedrobaeza revisad por favor,.
@moduon MT-4799