Skip to content
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][FIX] l10n_es_aeat_mod349: Fix tests compatibililty with account_edi_ubl_cii #3750

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

Conversation

victoralmau
Copy link
Member

Fix tests compatibililty with account_edi_ubl_cii

Related to #3503

The peppol_eas field exists only if account_edi_ubl_cii is installed, if we don't have this module installed (because we don't have it in the scaffolding) the test should not fail.

Please @pedrobaeza can you review it?

@Tecnativa

@OCA-git-bot
Copy link
Contributor

Hi @pedrobaeza,
some modules you are maintaining are being modified, check this out!

@pedrobaeza pedrobaeza added this to the 17.0 milestone Oct 4, 2024
@pedrobaeza
Copy link
Member

Realmente diría que habría que quitar ese código directamente, porque parece que es porque @manuelregidor estuvo ejecutando los tests después de instalar el módulo, en lugar de en la fase de instalación. ¿Puedes confirmar, Manuel?

@victoralmau
Copy link
Member Author

Realmente diría que habría que quitar ese código directamente, porque parece que es porque @manuelregidor estuvo ejecutando los tests después de instalar el módulo, en lugar de en la fase de instalación. ¿Puedes confirmar, Manuel?

Ok, el nos podrá confirmar; de todas formas, quitando ese código (lo relativo a definir peppol_eas) al tener instalado account_edi_ubl_cii fallarían los tests, por ese motivo el cambio realizado aquí (compatible en ambos casos).

@pedrobaeza
Copy link
Member

Hay algo que no me termina de cuadrar, ya que si esto está ocurriendo realmente ejecutando los tests en fase de instalación, lo que habría que arreglar es el módulo original PEPPOL, no cada módulo que cree un partner. @victoralmau tú has tenido ese mismo error original haciendo los tests como en el CI?

@manuelregidor
Copy link
Contributor

@pedrobaeza Correcto. De todos modos es lo que comenta @victoralmau, si el campo peppol_eas existe, creo que de algún modo hay que tenerlo en cuenta en estos test porque, de otro modo, fallan.

@pedrobaeza
Copy link
Member

Pues lo dicho en mi último comentario, pero ya os digo, creo que es simplemente porque luego ejecutasteis de nuevo los tests tras la instalación, con lo que la solución es no hacer eso en vuestro testing local, y aquí quitar directamente ese código.

@victoralmau victoralmau force-pushed the 17.0-fix-l10n_es_aeat_mod349-tests branch from 0e0f84f to 62ea4b4 Compare October 4, 2024 07:31
@victoralmau
Copy link
Member Author

Ya está quitado ese código y fallan los tests ahora en el CI.

@pedrobaeza
Copy link
Member

Ya, pues ahí parece haber un problema en el código del account_edi_ubl_cii que obliga a tener eso, y es lo que habría que reparar en Odoo. ¿Puedes investigar un poco al respecto? Si no, este problema lo tendremos cada vez que se haga una operación con el NIF según veo.

@victoralmau victoralmau force-pushed the 17.0-fix-l10n_es_aeat_mod349-tests branch from 62ea4b4 to 993d384 Compare October 4, 2024 11:42
@victoralmau
Copy link
Member Author

Revisando lo que indica en https://github.com/odoo/odoo/blob/17.0/addons/account_edi_ubl_cii/models/account_edi_common.py#L48 y en el test https://github.com/odoo/odoo/blob/17.0/addons/account_edi_ubl_cii/tests/test_ubl_cii.py#L179 parece ser necesario definir también el dato de company_registry en el caso de bélgica (casualidad).

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.

4 participants