-
-
Notifications
You must be signed in to change notification settings - Fork 698
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
base: 13.0
Are you sure you want to change the base?
Conversation
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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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. :)
There was a problem hiding this comment.
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!
There was a problem hiding this comment.
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...
fixes #4376