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.

Sign up for GitHub

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

Jump to bottom

[16.0][IMP] l10n_es_irnr[sii] - update references as l10n_es_irnr data has been included in l10n_es #3783

Merged
merged 1 commit into from
Oct 21, 2024
Merged

[16.0][IMP] l10n_es_irnr[sii] - update references as l10n_es_irnr data has been included in l10n_es #3783

merged 1 commit into from
Oct 21, 2024

Conversation

edlopen
Copy link
Member

@edlopen edlopen commented Oct 17, 2024

Hello,

the purpose of this PR is to remove redundant data in l10n_es_irnr and update l10n_es_irnr_sii.

As l10n_es_irnr data has been included in l10n_es this week (here) this module is now redundant.

I also tried to run the account_chart_update and I got any duplicates neither fiscal positions nor taxes and so on.

@Shide @yajo @rafaelbn @ArantxaSudon please review, if you can. Thank you in advance.

MT-7610

@OCA-git-bot
Copy link
Contributor

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

yajo
yajo approved these changes Oct 17, 2024
View reviewed changes
Copy link
Member

@yajo yajo left a comment

Choose a reason for hiding this comment

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

Thanks! The changes make sense to me. l10n_es_irnr is no longer installable and l10n_es_irnr_sii now depends directly on l10n_es.

pedrobaeza
pedrobaeza reviewed Oct 17, 2024
View reviewed changes
l10n_es_irnr/__manifest__.py Outdated
@@ -18,5 +18,5 @@
"author": "Tecnativa, Odoo Community Association (OCA)",
"website": "https://github.com/OCA/l10n-spain",
"license": "AGPL-3",
"installable": True,
"installable": False,
Copy link
Member

Choose a reason for hiding this comment

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

En lugar de poner a Falso, se debería dejar sin datos. Si no, saltará el problema en los logs. Quien quiera desinstalarlo, que lo haga, pero quien no, no tendrá ningún efecto.

Copy link
Member

Choose a reason for hiding this comment

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

OK, es menos invasivo.

Copy link
Member Author

Choose a reason for hiding this comment

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

Vale, también he actualizado la dependencia de l10n_es_aeat_mod216 que apuntaba al l10n_es_irnr. Creo que no me dejo nada.

El módulo se queda vacío con el instalable a True. He añadido al roadmap que se puede desinstalar, que la información del módulo ya la trae Odoo core y que este módulo ya no necesita ser migrado.

Copy link
Member Author

Choose a reason for hiding this comment

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

Perdón, se me pasaron dos referencias en el l10n_es_aeat_mod296. Ya debería ir todo bien.

@edlopen edlopen force-pushed the 16.0-imp-l10n_es_irnr__sii-l10n_es_update branch 4 times, most recently from 4fa5064 to 7207f0d Compare October 18, 2024 08:26
yajo
yajo suggested changes Oct 18, 2024
View reviewed changes
l10n_es_irnr/__manifest__.py Outdated
@@ -5,16 +5,11 @@

{
"name": "Retenciones IRNR (No residentes)",
"version": "16.0.1.1.0",
"version": "16.0.3.1.0",
Copy link
Member

Choose a reason for hiding this comment

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

Mucho salto de golpe! 😄

Suggested change
"version": "16.0.3.1.0",
"version": "16.0.2.0.0",

Copy link
Member

Choose a reason for hiding this comment

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

De hecho, si quieres ni lo cambies. Ya el bot lo hará cuando lo manden a fusionar con major

Copy link
Member Author

Choose a reason for hiding this comment

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

Done!

@edlopen edlopen force-pushed the 16.0-imp-l10n_es_irnr__sii-l10n_es_update branch from 7207f0d to 7057567 Compare October 18, 2024 09:27
rafaelbn
rafaelbn approved these changes Oct 18, 2024
View reviewed changes
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.

Ahora sí lo veo bien 👍🏼

¡Gracias!

De todas formas lo podemos volver a probar otra vez si quieres.

@pedrobaeza pedrobaeza added this to the 16.0 milestone Oct 18, 2024
pedrobaeza
pedrobaeza reviewed Oct 18, 2024
View reviewed changes
.pre-commit-config.yaml Outdated Show resolved Hide resolved
l10n_es_aeat_mod216/__manifest__.py Outdated Show resolved Hide resolved
l10n_es_irnr_sii/__manifest__.py Outdated Show resolved Hide resolved
@edlopen edlopen force-pushed the 16.0-imp-l10n_es_irnr__sii-l10n_es_update branch from 7057567 to dfeec27 Compare October 21, 2024 06:42
pedrobaeza
pedrobaeza reviewed Oct 21, 2024
View reviewed changes
l10n_es_irnr_sii/__manifest__.py
@@ -13,7 +13,6 @@
"installable": True,
"depends": [
"l10n_es_aeat_sii_oca",
Copy link
Member

Choose a reason for hiding this comment

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

¿No habría que también ya vaciar este módulo e incluir los mapeos en l10n_es_aeat_sii_oca?

@edlopen edlopen force-pushed the 16.0-imp-l10n_es_irnr__sii-l10n_es_update branch from dfeec27 to a4cb017 Compare October 21, 2024 10:48
@edlopen
[16.0][IMP] l10n_es_irnr[sii] - update references as l10n_es_irnr dat…
065aa18 
…a has been included in l10n_es module

MT-7610
@edlopen edlopen force-pushed the 16.0-imp-l10n_es_irnr__sii-l10n_es_update branch from a4cb017 to 065aa18 Compare October 21, 2024 10:49
pedrobaeza
pedrobaeza approved these changes Oct 21, 2024
View reviewed changes
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.

/ocabot merge minor

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 16.0-ocabot-merge-pr-3783-by-pedrobaeza-bump-minor, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit b961ebd into OCA:16.0 Oct 21, 2024
7 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 3452b84. Thanks a lot for contributing to OCA. ❤️

pedrobaeza added a commit to Tecnativa/OpenUpgrade that referenced this pull request Oct 21, 2024
@pedrobaeza
[OU-ADD] l10n_es_irnr/l10n_es_irnr_sii: Merged into l10n_es/l10n_es_a…
bb33a91 
…eat_sii_oca

Following OCA/l10n-spain#3783, those migrating
from 15 to 16 with these module don't need them, as they are useless
now.
pedrobaeza added a commit to Tecnativa/OpenUpgrade that referenced this pull request Oct 21, 2024
@pedrobaeza
[OU-ADD] l10n_es_irnr/l10n_es_irnr_sii: Merged into l10n_es/l10n_es_a…
d011acd 
…eat_sii_oca

Following OCA/l10n-spain#3783, those migrating
from 15 to 16 with these modules don't need them, as they are useless
now.
@pedrobaeza pedrobaeza mentioned this pull request Oct 21, 2024
Merged
pedrobaeza added a commit to Tecnativa/OpenUpgrade that referenced this pull request Oct 21, 2024
@pedrobaeza
[OU-ADD] l10n_es_irnr/l10n_es_irnr_sii: Merged into l10n_es/l10n_es_a…
8f01fc2 
…eat_sii_oca

Following OCA/l10n-spain#3783, those in 16 that have already installed
these modules, when migrating to 17, don't really need them (not even
in 16), so let's provide them a smooth transition.
@pedrobaeza pedrobaeza mentioned this pull request Oct 21, 2024
Merged
@yajo yajo deleted the 16.0-imp-l10n_es_irnr__sii-l10n_es_update branch October 22, 2024 06:22
@ljsalvatierra-factorlibre
Copy link
Contributor

Nos hemos encontrado con este error durante la actualización del módulo:

2024-10-22 11:43:12,226 275344 INFO <dbname> odoo.addons.base.models.ir_model: Deleting 28@account.tax.group (l10n_es_irnr.tax_group_irnr_ue) 
2024-10-22 11:43:12,230 275344 ERROR <dbname> odoo.sql_db: bad query: DELETE FROM "account_tax_group" WHERE id IN (28)
ERROR: update or delete on table "account_tax_group" violates foreign key constraint "account_tax_tax_group_id_fkey" on table "account_tax"
DETAIL:  Key (id)=(28) is still referenced from table "account_tax".

@edlopen
Copy link
Member Author

edlopen commented Oct 22, 2024

@ljsalvatierra-factorlibre visto, lo revisamos.

@pedrobaeza
Copy link
Member

Ya está resuelto en #3791

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yajo yajo yajo requested changes

@pedrobaeza pedrobaeza pedrobaeza approved these changes

@rafaelbn rafaelbn rafaelbn approved these changes

Assignees
No one assigned
Labels
merged 🎉
Projects
None yet
Milestone
16.0
Development

Successfully merging this pull request may close these issues.
6 participants
@edlopen @OCA-git-bot @ljsalvatierra-factorlibre @pedrobaeza @yajo @rafaelbn