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] account_invoice_refund_link: Migration to 17.0 #1670

Merged
merged 45 commits into from
May 3, 2024

Conversation

andrel-exo
Copy link

No description provided.

antespi and others added 20 commits February 27, 2024 12:27
Enhancements:

* Reverse link from invoice to refunds also available.
* Link at invoice line level, not only at invoice level.
* When installing the module after some refunds have been made, the module tries to find the link to the original invoice for them.
* OCA Guidelines compliance.
* Compatibility in OpenUpgrade for migrating from 8.0 module account_refund_original.
* Tests with 100% coverage.

Technical changes:

* Rename field origin_invoices_ids to origin_invoice_ids
* Rename field refund_invoices_description to refund_reason
* Rename addon to account_invoice_refund_link
Odoo already includes a method for preparing the refund, so using it
we don't have to make any guess about the returned domain and the
method also serves for refunds created directly by code.

Initial line match has also been improved comparing product or
description.
* [FIX] account_invoice_refund_link: Don't copy m2m fields

Many2many fields have copy=True property by default, having
an incorrect duplication of origin or refund invoices.

Steps to reproduce:

* Create an invoice
* Refund it
* Duplicate the original invoice
* The refund will be link to 2 original invoices

Included tests covering the use cases for avoiding future regressions.
Since v11, Odoo counts with a field called `refund_invoice_ids` of type
one2many, which conflicts with the m2m field defined in this module.
Moreover, these fields are not needed anymore.

There is only 2 OCA modules using this module right now, and they
don't need significant changes for this adaptation, but I will do it
when this get merged.
Currently translated at 100.0% (17 of 17 strings)

Translation: account-invoicing-12.0/account-invoicing-12.0-account_invoice_refund_link
Translate-URL: https://translation.odoo-community.org/projects/account-invoicing-12-0/account-invoicing-12-0-account_invoice_refund_link/de/
Currently translated at 100.0% (17 of 17 strings)

Translation: account-invoicing-12.0/account-invoicing-12.0-account_invoice_refund_link
Translate-URL: https://translation.odoo-community.org/projects/account-invoicing-12-0/account-invoicing-12-0-account_invoice_refund_link/pt_BR/
Currently translated at 100.0% (11 of 11 strings)

Translation: account-invoicing-13.0/account-invoicing-13.0-account_invoice_refund_link
Translate-URL: https://translation.odoo-community.org/projects/account-invoicing-13-0/account-invoicing-13-0-account_invoice_refund_link/pt_BR/
Currently translated at 100.0% (12 of 12 strings)

Translation: account-invoicing-13.0/account-invoicing-13.0-account_invoice_refund_link
Translate-URL: https://translation.odoo-community.org/projects/account-invoicing-13-0/account-invoicing-13-0-account_invoice_refund_link/pt_BR/
@andrel-exo andrel-exo mentioned this pull request Feb 27, 2024
46 tasks
@pedrobaeza
Copy link
Member

/ocabot migration account_invoice_refund_link

@OCA-git-bot OCA-git-bot added this to the 17.0 milestone Feb 27, 2024
@andrel-exo andrel-exo changed the title [17.0][MIG] account_invoice_refund_link [17.0][MIG] account_invoice_refund_link: Migration to 17.0 Feb 27, 2024
Copy link

@MohamedOsman7 MohamedOsman7 left a comment

Choose a reason for hiding this comment

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

Functional review LGTM

Copy link

@CRogos CRogos left a comment

Choose a reason for hiding this comment

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

Please have a look in to the migration guid:
https://github.com/OCA/maintainer-tools/wiki/Migration-to-version-17.0

  • The commit history is not cleaned up.
  • The required commits missing and your changes are mixed up into one commit
    • [IMP] $module: pre-commit auto fixes
    • [MIG] $module: Migration to 17.0

Copy link

Choose a reason for hiding this comment

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

The readme files need to be kept as md files!

@andrel-exo andrel-exo force-pushed the 17.0_account_invoice_refund_link branch from 5fdd8c5 to 9642655 Compare March 26, 2024 15:30
@andrel-exo andrel-exo requested a review from CRogos March 26, 2024 15:33
Copy link

@CRogos CRogos left a comment

Choose a reason for hiding this comment

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

Better but not yet 100%.
According to the migration guide, the last commits should look like this:
image

Here you can find the instruction how and which commits to squash: https://github.com/OCA/maintainer-tools/wiki/Merge-commits-in-pull-requests#mergesquash-the-commits-generated-by-bots-or-weblate

Here are some samples>
image

koenloodts and others added 17 commits April 22, 2024 15:58
Currently translated at 91.6% (11 of 12 strings)

Translation: account-invoicing-15.0/account-invoicing-15.0-account_invoice_refund_link
Translate-URL: https://translation.odoo-community.org/projects/account-invoicing-15-0/account-invoicing-15-0-account_invoice_refund_link/ca/
With the previous algorithm, if something modifies the order of the
returned lines, there are chances of linking invoice lines with non
refund lines (but other journal items).

We assure at least that only refund lines are linked restricting the
iteration on them.

TT38633
Currently translated at 100.0% (12 of 12 strings)

Translation: account-invoicing-16.0/account-invoicing-16.0-account_invoice_refund_link
Translate-URL: https://translation.odoo-community.org/projects/account-invoicing-16-0/account-invoicing-16-0-account_invoice_refund_link/hr/
Currently translated at 100.0% (12 of 12 strings)

Translation: account-invoicing-16.0/account-invoicing-16.0-account_invoice_refund_link
Translate-URL: https://translation.odoo-community.org/projects/account-invoicing-16-0/account-invoicing-16-0-account_invoice_refund_link/pt/
On the side by side comparison, exclude the lines that are not products, as that's what we do on the refund lines.
Hooking on a high level method like `_reverse_moves` (although still
being private), makes that other modules hooking into it may alter the
number of returned lines (like for example,
account_invoice_refund_line_selection).

We choose the low-level `copy_data` method that is used in such method
for linking the refund lines with their origin ones, avoiding the
problem.
Currently translated at 100.0% (12 of 12 strings)

Translation: account-invoicing-16.0/account-invoicing-16.0-account_invoice_refund_link
Translate-URL: https://translation.odoo-community.org/projects/account-invoicing-16-0/account-invoicing-16-0-account_invoice_refund_link/es/
- Include context keys for avoiding mail operations overhead.
Currently translated at 100.0% (12 of 12 strings)

Translation: account-invoicing-16.0/account-invoicing-16.0-account_invoice_refund_link
Translate-URL: https://translation.odoo-community.org/projects/account-invoicing-16-0/account-invoicing-16-0-account_invoice_refund_link/pt_BR/
Currently translated at 100.0% (12 of 12 strings)

Translation: account-invoicing-16.0/account-invoicing-16.0-account_invoice_refund_link
Translate-URL: https://translation.odoo-community.org/projects/account-invoicing-16-0/account-invoicing-16-0-account_invoice_refund_link/it/
@andrel-exo andrel-exo force-pushed the 17.0_account_invoice_refund_link branch from f2c12d9 to 676f4d8 Compare April 22, 2024 14:58
@CRogos
Copy link

CRogos commented May 3, 2024

@pedrobaeza can you merge?

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

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
Copy link
Contributor

On my way to merge this fine PR!
Prepared branch 17.0-ocabot-merge-pr-1670-by-pedrobaeza-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit d61a1f1 into OCA:17.0 May 3, 2024
5 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 216644c. 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.