Skip to content

Commit

Permalink
[IMP] util/records: enforce cascade removal for actions
Browse files Browse the repository at this point in the history
The implementation of the python inheritance mechanism between the base class
`ir.actions.actions` and its child classes (eg. `ir.actions.act_window`) does
not allow the creation of foreign keys when `ir.actions.actions` is a M2O field
of another model; what leads to the not execution of some constraints, one of
them being the `ondelete='cascade'` constraint, which is set in PSQL level.

That said, when a `ir.actions.actions` record is deleted, if it is being
referenced as a M2O field by another model (eg. `ir.filters`), records from this
second model won't be affected, what leads to undesired behaviour: a
MissingError in the UI, indicating that the action was deleted.

Such behaviour of not creating foreign keys and thus constraints is specific to
`ir.actions.actions`.
This commit remedies this specific case, removing records with a M2O field to
`ir.actions.actions` with `ondelete=cascade` when the action referenced in such
field is removed using `upgrade-util`.
  • Loading branch information
pauloamed committed May 6, 2024
1 parent 3787631 commit 2019fa8
Showing 1 changed file with 50 additions and 1 deletion.
51 changes: 50 additions & 1 deletion src/util/records.py
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,52 @@ def remove_asset(cr, name):
remove_view(cr, name, silent=True)
# fmt:on

def get_models_inheriting_actions_actions(cr):
# NOTE: it would be nice to have this cached since its static info
# + opted for sql querying over hard-coding for better maintainability and handling custom models
cr.execute(
"""
SELECT child_model.model
FROM ir_model AS child_model
JOIN ir_model_inherit
ON child_model.id = ir_model_inherit.model_id
JOIN ir_model AS base_model
ON ir_model_inherit.parent_id = base_model.id
WHERE base_model.model = 'ir.actions.actions'
"""
)
return cr.fetchall()

def remove_action(cr, xml_id=None, action_id=None):
assert bool(xml_id) ^ bool(action_id)

if xml_id:
action_id = ref(cr, xml_id)

# NOTE: when comparing to `remove_group` it makes sense to unindent, since the id given by the user can be from
# a wrong model (if check is done for xml_id, why not do it too for normal id?)
if action_id:
# NOTE: we are repeating code from `remove_group` and `remove_view`, maybe we could create a method
# check_if_xml_is_from_models(xml_id, model_list)
module, _, name = xml_id.partition(".")
cr.execute("SELECT model FROM ir_model_data WHERE module=%s AND name=%s", [module, name])
[model] = cr.fetchone()
if not model in get_models_inheriting_actions_actions(cr):
raise ValueError(
"%r should point to a model inheriting from 'ir.actions.actions', not a %r" % (xml_id, model)
)

# NOTE: same note from get_models_inheriting_actions_actions(): sql query for maintainability + suggestion of
# caching for performance
cr.execute("SELECT name, model FROM ir_model_fields WHERE relation = 'ir.actions.actions' AND on_delete ='cascade'")
for column_name, model in cr.fetchall():
model_table = table_of_model(cr, model)
# NOTE: consider disabling+setting field to NULL instead of deleting for better UX
cr.execute(
'DELETE FROM "{}" WHERE "{}" = %s'.format(model_table, column_name),
(action_id,),
)


def remove_record(cr, name):
"""
Expand Down Expand Up @@ -351,10 +397,13 @@ def remove_record(cr, name):
if model == "res.groups":
_logger.log(NEARLYWARN, "Removing group %r", name)
return remove_group(cr, group_id=res_id)

if model in get_models_inheriting_actions_actions(cr):
_logger.log(NEARLYWARN, "Removing action %r", name)
return remove_action(cr, action_id=res_id)

return remove_records(cr, model, [res_id])


def remove_records(cr, model, ids):
if not ids:
return
Expand Down

0 comments on commit 2019fa8

Please sign in to comment.