[17.0][OU-FIX] openupgrade_framework: cascade inactive child views on force unlink#19
Closed
eantones wants to merge 1 commit into
Closed
[17.0][OU-FIX] openupgrade_framework: cascade inactive child views on force unlink#19eantones wants to merge 1 commit into
eantones wants to merge 1 commit into
Conversation
… force unlink Odoo's View.unlink() emulates an ondelete cascade on inherit_children_ids when _force_unlink is set (as ir.model.data._process_end does), but reads that One2many with active_test enabled, so INACTIVE child views are skipped. They still hold the inherit_id RESTRICT foreign key, so the parent's DELETE is rejected (benign bad query) and the obsolete parent view survives ORPHANED -- contrary to the assumption that a later -u all sweeps obsolete views, it never does. Hit at 16.0 -> 17.0: website_sale.payment_footer survives because its child payment_sale_note is active="False". Patch the cascade to read the children with active_test=False. This is a core defect (present 10.0 -> 18.0, since the cascade was added by odoo/odoo#25499); patched here so migrations do not depend on it being fixed upstream. No-op once core reads inactive children itself.
Member
Author
|
Superseded: fixing this in Odoo core instead (nuobit/odoo#9 + an 18.0 counterpart). The defect is not migration-specific — |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Patches a core defect that silently leaves obsolete parent views in the database after a migration hop.
The bug (in Odoo core, not in OpenUpgrade)
View.unlink()emulates an ondelete cascade overinherit_children_idswhen_force_unlinkis set — the contextir.model.data._process_end()uses while removing records that became obsolete after a module update:inherit_children_idsis a plainOne2many, so it is read withactive_testenabled and inactive child views are not returned. The cascade skips them, but they still hold theinherit_idRESTRICT foreign key at database level, so the parent'sDELETEis rejected, logged as a benignbad query, and the obsolete parent view survives orphaned._process_end()walks its obsolete records once and never revisits the parent.Why this matters for OpenUpgrade
The common assumption is that obsolete views are naturally removed by the next real
-u allafter the migration. For this class of view they are not: every-u allre-runs the same_process_end→ same cascade → same skipped inactive child → same rejectedDELETE. The orphan is permanent unless something deletes the child first.Hit at 16.0 → 17.0:
website_sale.payment_footer(dropped from core in 17.0) survives, because its childwebsite_sale.payment_sale_noteisactive="False":Two conditions must coincide, which is why it is rarely seen: the parent must be processed before the child (
ORDER BY id DESConir_model_data.id), and the child must be inactive.The fix
Wrap
View.unlinkso the cascade reads its children withactive_test=False. The child is then removed before the parent'sDELETE, which succeeds, and no orphan is left.Why here and not only in core
This should be fixed in core, and the same one-line change applies there. It is patched in
openupgrade_frameworkbecause:odoo_patchexists for — correcting core behaviour that misbehaves during a migration.The patch is a no-op once core reads inactive children itself, so it is safe to keep across versions.
Scope
The cascade was added to core in 2018 (odoo/odoo PR 25499), which fixed the active child case and left the inactive one. The defect is present unchanged from 10.0 through 18.0.