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

[FIX] account: reversed_entry_id migration was reversed #4534

Open
wants to merge 2 commits into
base: 13.0
Choose a base branch
from

Conversation

sbidoul
Copy link
Member

@sbidoul sbidoul commented Jul 28, 2024

fixes #4376

@pedrobaeza pedrobaeza added this to the 13.0 milestone Jul 30, 2024
JOIN account_move am2 ON am2.old_invoice_id = ai2.id
WHERE am.reversed_entry_id IS NULL AND am.old_invoice_id = ai.id"""
FROM account_move am2
WHERE am.reversed_entry_id IS NULL AND am2.reverse_entry_id = am.id
Copy link
Member

Choose a reason for hiding this comment

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

reverse_entry_id is not filled in this case AFAIK, so you should go to invoice table for this information.

Copy link
Member Author

Choose a reason for hiding this comment

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

@pedrobaeza I don't understand your remark. in 13.0, reverse_entry_id became reversed_entry_id with a reversed relation. This is what this update does. Since reverse_entry_id did exist in 12, we can assume it was populated no?

Regarding the invoices, the update before this PR did not work, and you suggested in #4376 (comment) that there was nothing to do.

In any case I can confirm this PR works for me and correctly set the Reversal Entry field.

Copy link
Member

Choose a reason for hiding this comment

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

But in v12, invoices were another thing, and this is filling the reverse link between invoices through the refund_invoice_id field. I don't think the reverse_entry_id was filled for the journal entries generated from the invoices that are refunded. Or am I wrong? Maybe both operations should be done.

Copy link
Member Author

@sbidoul sbidoul Jul 30, 2024

Choose a reason for hiding this comment

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

Ok, I updated to preserve the existing update but changing reverse_entry_id instead, then let the second update reverse the relation.

I can't test this as the database I'm working with does not have refund_invoice_id for some reason.

Copy link
Member

@rvalyi rvalyi Jul 30, 2024

Choose a reason for hiding this comment

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

Hello just in case this might help you to inspect or remember what Odoo SA did for v13 commit by commit (with their explanations): https://github.com/akretion/odoo-module-diff-analysis/tree/main/13.0/account

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks Raphael. Well in this case, the relevant commit is imp-ref-accounting-pocalypse-yeaaahh, so... yeah. :)

Copy link
Member Author

Choose a reason for hiding this comment

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

But your analysis tool is definitely super useful!

Copy link
Member

Choose a reason for hiding this comment

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

Once I'll have cleaned up it a bit more I'll propose to integrate these key change commits along the OpenUgrade analysis files. Then it will be up to the OpenUpgrade leaders to accept it inside the repo or not...

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.

4 participants