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

[IMP] util/records: enforce cascade removal for actions #81

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
31 changes: 31 additions & 0 deletions src/base/tests/test_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -1046,6 +1046,37 @@ def test_replace_record_references_batch__uniqueness(self):
[count] = self.env.cr.fetchone()
self.assertEqual(count, 1)

@unittest.skipUnless(util.version_gte("12.0"), "Only work on Odoo >= 12")
def test_remove_action_apply_cascade(self):
action = self.env["ir.actions.act_url"].create({"name": "act_test", "url": "test.com"})
self.env["ir.model.data"].create(
{"name": "act_test", "module": "base", "model": "ir.actions.act_url", "res_id": action.id}
)

filter_ = self.env["ir.filters"].create({"name": "filter", "model_id": "res.users", "action_id": action.id})

util.remove_record(self.env.cr, "base.act_test")

self.assertIsNone(util.ref(self.env.cr, "base.act_test"))
self.assertFalse(action.exists())
self.assertFalse(filter_.exists())

@unittest.skipUnless(util.version_gte("12.0"), "Only work on Odoo >= 12")
def test_remove_action_apply_setnull(self):
action = self.env["ir.actions.server"].create(
{"name": "act_test", "model_id": util.ref(self.env.cr, "base.model_res_users")}
)

user = self.env["res.users"].create({"login": "U1", "name": "U1", "action_id": action.id})

util.remove_action(self.env.cr, action_id=action.id)

util.invalidate(user)

self.assertFalse(action.exists())
self.assertTrue(user.exists())
self.assertFalse(user.action_id)


class TestMisc(UnitTestCase):
@parametrize(
Expand Down
49 changes: 49 additions & 0 deletions src/util/records.py
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,51 @@ def remove_asset(cr, name):
# fmt:on


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

action_model = None
if action_id:
aj-fuentes marked this conversation as resolved.
Show resolved Hide resolved
aj-fuentes marked this conversation as resolved.
Show resolved Hide resolved
cr.execute("SELECT type FROM ir_actions WHERE id=%s", [action_id])
[action_model] = cr.fetchone()
else:
module, _, name = xml_id.partition(".")
pauloamed marked this conversation as resolved.
Show resolved Hide resolved
cr.execute("SELECT model, res_id FROM ir_model_data WHERE module=%s AND name=%s", [module, name])
action_model, action_id = cr.fetchone()
if action_model not in {ihn.model for ihn in for_each_inherit(cr, "ir.actions.actions")}:
raise ValueError(
"%r should point to a model inheriting from 'ir.actions.actions', not a %r" % (xml_id, action_model)
)

# on_delete value is correct only from version 11, thus can only be used on upgrades to version > 11
if not version_gte("12.0"):
return remove_records(cr, action_model, [action_id])

cr.execute(
"""
SELECT name,
model,
on_delete
FROM ir_model_fields
WHERE relation = 'ir.actions.actions'
AND on_delete IN ( 'cascade', 'set null' )
pauloamed marked this conversation as resolved.
Show resolved Hide resolved
AND ttype = 'many2one'
"""
)

for column_name, model, on_delete in cr.fetchall():
model_table = table_of_model(cr, model)
if on_delete == "cascade":
query = format_query(cr, "SELECT id FROM {} WHERE {} = %s", model_table, column_name)
cr.execute(query, (action_id,))
remove_records(cr, model, [id_ for (id_,) in cr.fetchall()])
else:
query = format_query(cr, "UPDATE {} SET {} = NULL WHERE {} = %s", model_table, column_name, column_name)
explode_execute(cr, cr.mogrify(query, [action_id]).decode(), table=model_table)

return remove_records(cr, action_model, [action_id])


def remove_record(cr, name):
"""
Remove a record and its references corresponding to the given :term:`xml_id <external identifier>`.
Expand Down Expand Up @@ -352,6 +397,10 @@ def remove_record(cr, name):
_logger.log(NEARLYWARN, "Removing group %r", name)
return remove_group(cr, group_id=res_id)

if model in {ihn.model for ihn in for_each_inherit(cr, "ir.actions.actions")}:
_logger.log(NEARLYWARN, "Removing action %r", name)
return remove_action(cr, action_id=res_id)

return remove_records(cr, model, [res_id])


Expand Down