-
-
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
[17.0][MIG] l10n_es_aeat: Migration to 17.0 #3387
Conversation
6f17d83
to
3a6b9a7
Compare
@pedrobaeza como te comenté, esta es mi propuesta para los cambios en odoo 17, he creado dos modelos una para las plantillas de los impuestos y otro para las cuentas, yo personalmente creo que es mejor así que escribir los xml_id directamente en los mapeos, por lo menos visualmente es mejor si queremos hacer algún cambio. Los registros de esos dos modelos se crean con los CSV del plan contable, estos registros se crean/actualizan cuando se instala/actualiza el módulo de modo que si se crean nuevos impuestos en el plan contable de la localización española, al actualizar el módulo ya aparecerán en el listado para poder mapearlos. |
/ocabot migration l10n_es_aeat |
@ramiadavid Si haces rebase, ya te debería coger bien la dependencia 👍 |
@ramiadavid no veo mal la solución para reducir el impacto sobre todo el sistema de contabilidad española. Pero ¿Se ha tenido en cuenta también estos cambios como aplicarían al account.chat.update de oca? ¿Sería necesario realizar un módulo de extensión para que tenga en cuenta este cambio? |
No es un módulo que haya tocado y no conozco mucho cómo funciona, pero seguro que habrá que hacer cambios
|
65c5b3b
to
f0325da
Compare
new_report = self.env["l10n.es.aeat.report"].new({"name": "Test Report"}) | ||
new_report = self.env["l10n.es.aeat.report"] |
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.
Después de hacer el rebase ha comenzado a darme este error:
TestL10nEsAeatExportConfig.test_export_config_file 2024-02-16T19:04:19.9567010Z Traceback (most recent call last): 2024-02-16T19:04:19.9567904Z File "/__w/l10n-spain/l10n-spain/l10n_es_aeat/tests/test_l10n_es_aeat_export_config.py", line 166, in test_export_config_file 2024-02-16T19:04:19.9568806Z new_report = self.env["l10n.es.aeat.report"].new({"name": "Test Report"}) 2024-02-16T19:04:19.9569394Z File "/opt/odoo/odoo/models.py", line 6396, in new 2024-02-16T19:04:19.9569844Z record._update_cache(values, validate=False) 2024-02-16T19:04:19.9570318Z File "/opt/odoo/odoo/models.py", line 5996, in _update_cache 2024-02-16T19:04:19.9570810Z cache.set(self, field, value, check_dirty=False) 2024-02-16T19:04:19.9571241Z File "/opt/odoo/odoo/api.py", line 999, in set 2024-02-16T19:04:19.9571605Z record_id = record.id 2024-02-16T19:04:19.9572084Z AttributeError: 'l10n.es.aeat.report' object has no attribute 'id'
Por lo visto en la última versión de odoo17 no se puede hacer .new() a un AbrstactModel ya que da error, quitando esta parte el test finaliza correctamente, así que entiendo que estará correcto.
Hola David, Primero de todo, agradecerte la iniciativa de migración. Nos ha tocado asumir un cambio global en el sistema por lo de la desaparición de las Me toca hacer de poli malo ahora... Quizá se me escape algo, pero has tenido en cuenta los comentarios de Pedro en este comentario? --> #3382 (comment) Salvo error por mi parte, no acabo de ver claro que estemos todos en sintonía. @pedrobaeza ¿Es el planteamiento de David al que te referías con ello? No lo tengo claro... Si queréis podemos organizar meet por el Discord de AEOdoo para aclarar el planteamiento, y si hace falta heredamos nosotros (Sygel) la migración? Saludos. |
@HaraldPanten el planteamiento mio es similar al de @pedrobaeza, es solo una propuesta y hay que analizar pros y contras, El planteaba en los mapeos cambiarlo por campos tipo texto y escribir el xml_id del template, pero pienso que eso es muy poco visual si algun usuario tiene que tocar alguno, lo que yo planteo es volver a crear esos templates en un modelo nuevo con dos simples campos Nombre y xml_id de esta forma es mas sencillo de seleccionar desde una lista Esos templates se crean extrayendo los datos de los .csv de forma que si se añaden mas impuestos al actualizar el módulo ya aparecen en la lista Podéis continuar con ello vosotros si quereis, por mi no hay problema |
Hola @ramiadavid , En efecto esta manera sería más sencillo a nivel de persona técnica, pero... ¿y para una persona funcional y mantenimiento? Pienso que la creación de 2 modelos para el mantenimiento puede ser más problemático ¿Por que motivos?
Como bien se indica esto es una propuesta y vale la pena revisar ambos puntos de vista. |
@ramiadavid Desde mi punto de vista, se ha intentado transformar lo existente para reaprovechar el código antiguo. En este caso creo que es mejor hacer un refactoring. Si vamos añadiendo capas para que se adapte a lo original lo único que haremos es crear un monstruo. Realizarlo tal y como comentan @HaraldPanten @ValentinVinagre y @pedrobaeza es adapatarse a la lógica nueva de odoo y, a la larga, minimizar el código y las posibles fuentes de error. My 2 cents |
Coincido con vosotros. Sale más a cuenta adaptarse a los cambios del software (a veces nos gustarán más y otras veces menos) que recrear lógicas antiguas; ya que eso nos llevará a tener que ampliar mucho más el código en cada una de las versiones y en cada uno de los módulos relacionados. |
…c module for aeat models, 347 module was portedto v6.0 and adds new module to print AEAT model 349.
…ueñas mejoras generales (vistas, traducciones, código) y corrección pequeño bug por un olvido en la adaptación de la v5 a la v6
…s formatted correctly. With this number 'dec_part' would become 100 (instead of 0) and fail in the final assertion. Instead of providing a workaround for this case, I simply replace the formatting with '%0*.*f' which seems easier specially because it already does the appropriate rounding.
…recía Pexego en la licencia, quedan los compartidos.
…en la 6.0 de estos módulos
…ron marcados como no instalables mientras no se compruebe que funcionan o migren, para poder usarlos o probarlos se debería poner el atributo installable de los ficheros __openerp__.py de cada módulo a True
Fulfills the need (seen in l10n_es_aeat_mod369) to obtain the ISO code of a country based on the country code stored in Odoo. It is an inverse function of _map_aeat_country_code.
Fixes one more remaining unauthorized access to ir.model On export_config_id field, domain calculation searched on ir.model Added sudo() to avoid unauthorized access. fixes OCA#3016
COM (collectivité d'outre-mer - https://es.wikipedia.org/wiki/Colectividad_de_ultramar) are also considered France for the AEAT, so we need to map the existing countries to FR. TT46789
f0325da
to
d6ce3e5
Compare
Hola, acabo de empujar un commit (aparte de hacer un rebase) que incluye la visión que os quería transmitir, teniendo un modelo simple con solo el XML-ID como Enseguida empujo el 303, que lo he hecho también para probar que funciona. Ahora reviso el error de los tests, que en local no salía. |
- Switch to XML-ID names models - Adapt API and calls - Some other minor adjustments - Translations
d6ce3e5
to
66f713f
Compare
Y aquí está el 303: #3486 |
Gracias Pedro, a partir de mañana empezamos a revisar y a darle caña a la localización 💪🏻💪🏻 |
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.
Probado en entorno local con el PR del modelo 303 y no he detectado ningún comportamiento inesperado. A falta de probarlo en producción con casos reales.
LGTM
@ramiadavid te parece correcto el diseño? |
Estoy fuera unos dias y no puedo probarlo, pero si a vosotros os parece bien, por mi está bien, me interesa tirar esto para adelante lo antes posible |
Venga, pues gracias por la confianza. Vamos con ello para desbloquear modelos, que ya no van a tener mucho trabajo. Creé un script para transformar fácilmente los mapeados de v16 en los de v17: https://gist.github.com/pedrobaeza/f68bf5e15cd18f5c1d1d5813df7be935 /ocabot merge nobump |
On my way to merge this fine PR! |
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.
Gracias @pedrobaeza por desbloquearlo ❤️
Congratulations, your PR was merged at 8edca5b. Thanks a lot for contributing to OCA. ❤️ |
Depends on: