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.

Sign up for GitHub

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
Open

[FIX] account: reversed_entry_id migration was reversed #4534

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
aabdb35
[FIX] account: reversed_entry_id migration was reversed
sbidoul Jul 28, 2024
747ee55
[IMP] account: preserve refund_invoice_id as reversed_entry_id
sbidoul Jul 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions addons/account/migrations/13.0.1.1/post-migration.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -585,10 +585,9 @@ def fill_account_move_reversed_entry_id(env):
env.cr, """
UPDATE account_move am
SET reversed_entry_id = am2.id
FROM account_invoice ai
JOIN account_invoice ai2 ON ai.refund_invoice_id = ai2.id
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 •
edited
Loading

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 •
edited
Loading

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...

"""
)


Expand Down
1 change: 0 additions & 1 deletion addons/account/migrations/13.0.1.1/pre-migration.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@

_field_renames = [
('account.move', 'account_move', 'amount', 'amount_total'),
('account.move', 'account_move', 'reverse_entry_id', 'reversed_entry_id'),
]

_field_sale_renames = [
Expand Down
Loading