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][MIG] l10n_es_vat_prorate: Migration to 17.0 #3583

Merged
merged 29 commits into from
May 23, 2024

Conversation

manuelregidor
Copy link
Contributor

@manuelregidor manuelregidor commented May 16, 2024

Standard migration + test adicional

T-6084

@HaraldPanten
Copy link
Contributor

/ocabot migration l10n_es_vat_prorate

@OCA-git-bot OCA-git-bot added this to the 17.0 milestone May 16, 2024
@OCA-git-bot OCA-git-bot mentioned this pull request May 16, 2024
47 tasks
etobella and others added 27 commits May 16, 2024 14:57
The VAT prorate tax line should contain the analytic info from the
expense line.

With this commit, we put such information, and make sure that is
synchronized on any change.

TT41832
When doing a refund, the account of the prorate line is changed to the
tax repartition line default one (472).

That's because there's some code on the method `_reverse_move_vals`
that is checking `tax_repartition_line_id` value in the returned
dictionary for ignoring the copied value. See
https://github.com/odoo/odoo/blob/82b1e68b60eb14896666adfd3ad1f753e944d393/addons/account/models/account_move.py#L2515-L2533

What we do for tricking it (as it's atomic and can't be intercepted in
other part) is:

- When copying values, we move the `tax_repartition_line_id` into
  another field name.
- Thus, `_reverse_move_vals` is not modifying the account.
- Immediately after returning from `_reverse_move_vals`, we restored
  the field value.

TT44656
The dynamic lines management has totally changed in this version, so the
code has been rewritten mostly from scratch, but now is cleaner thanks
to the avoid of a lot of hooks.

Some JSON from tests were not correct in previous version due to
rounding, and now they are OK: 10 / 1.2 = 8.3333333, which is 8.33, not
8.34.

TT43236
Currently translated at 100.0% (24 of 24 strings)

Translation: l10n-spain-16.0/l10n-spain-16.0-l10n_es_vat_prorate
Translate-URL: https://translation.odoo-community.org/projects/l10n-spain-16-0/l10n-spain-16-0-l10n_es_vat_prorate/es/
Currently translated at 100.0% (24 of 24 strings)

Translation: l10n-spain-16.0/l10n-spain-16.0-l10n_es_vat_prorate
Translate-URL: https://translation.odoo-community.org/projects/l10n-spain-16-0/l10n-spain-16-0-l10n_es_vat_prorate/ca/
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.

También mejor juntar los dos últimos commits.

l10n_es_vat_prorate/models/account_tax.py Outdated Show resolved Hide resolved
@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@manuelregidor
Copy link
Contributor Author

@pedrobaeza @HaraldPanten A la espera del OK de los últimos cambios para fusionar commits.

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.

Los commits no deben llevar la versión en su mensaje, y por favor haz squash de los 3 de la migración. No es necesario separarlo y luego hacer squash. GitHub ya informa de los cambios en cada push.

l10n_es_vat_prorate/models/prorate_taxes.py Outdated Show resolved Hide resolved
@manuelregidor manuelregidor changed the title [17.0][MIG] l10n_es_vat_prorate: Migration to 17.0 [MIG] l10n_es_vat_prorate: Migration to 17.0 May 23, 2024
@manuelregidor
Copy link
Contributor Author

@pedrobaeza Mensaje del commit cambiado

@manuelregidor
Copy link
Contributor Author

@pedrobaeza @HaraldPanten Tengo el problema localizado, subo cambios en un momento.

@manuelregidor
Copy link
Contributor Author

@pedrobaeza @HaraldPanten cambios aplicados

@HaraldPanten
Copy link
Contributor

Yo lo veo bien. En cuanto se valide técnicamente por parte de Pedro, fusionamos

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 nobump

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 17.0-ocabot-merge-pr-3583-by-pedrobaeza-bump-nobump, awaiting test results.

@pedrobaeza pedrobaeza changed the title [MIG] l10n_es_vat_prorate: Migration to 17.0 [17.0][MIG] l10n_es_vat_prorate: Migration to 17.0 May 23, 2024
@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@OCA-git-bot OCA-git-bot merged commit 1f9694d into OCA:17.0 May 23, 2024
7 checks passed
@OCA-git-bot
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants