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

[14.0][IMP] account_invoice_refund_link: Make more reliable the linking #1524

Conversation

pedrobaeza
Copy link
Member

@pedrobaeza pedrobaeza commented Aug 7, 2023

Forward-port of #1523

Hooking on a high level method like _reverse_move_vals (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.

@Tecnativa

Hooking on a high level method like `_reverse_move_vals` (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.
@pedrobaeza pedrobaeza added this to the 14.0 milestone Aug 7, 2023
@pedrobaeza
Copy link
Member Author

Unrelated CI error.

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.

/ocabot merge minor

@OCA-git-bot
Copy link
Contributor

On my way to merge this fine PR!
Prepared branch 14.0-ocabot-merge-pr-1524-by-rafaelbn-bump-minor, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Aug 9, 2023
Signed-off-by rafaelbn
@OCA-git-bot
Copy link
Contributor

@rafaelbn your merge command was aborted due to failed check(s), which you can inspect on this commit of 14.0-ocabot-merge-pr-1524-by-rafaelbn-bump-minor.

After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red.

@pedrobaeza
Copy link
Member Author

CI was red due to another module, failing also in main branch. I have disabled the conflicting tests in 9148906, and I'll ping the maintainer to check in an issue.

Now, we can merge:

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

On my way to merge this fine PR!
Prepared branch 14.0-ocabot-merge-pr-1524-by-pedrobaeza-bump-patch, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 14be6bc into OCA:14.0 Aug 9, 2023
4 of 6 checks passed
@OCA-git-bot
Copy link
Contributor

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

@pedrobaeza pedrobaeza deleted the 14.0-imp-account_invoice_refund_link-solid_linking branch August 9, 2023 14:22
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.

6 participants