Skip to content

Commit 52a7732

Browse files
KangOlaj-fuentes
andcommitted
[IMP] util.delete_unused
Also consider child records when searching for record usage. i.e. when try to remove a product category, also search for its sub categories as they will be cascade deleted if unused. closes #205 Signed-off-by: Nicolas Seinlet (nse) <[email protected]> Co-authored-by: Alvaro Fuentes <[email protected]>
1 parent e1c9190 commit 52a7732

File tree

2 files changed

+159
-19
lines changed

2 files changed

+159
-19
lines changed

src/base/tests/test_util.py

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1337,6 +1337,99 @@ def test_replace_record_references_batch__uniqueness(self):
13371337
[count] = self.env.cr.fetchone()
13381338
self.assertEqual(count, 1)
13391339

1340+
def _prepare_test_delete_unused(self):
1341+
def create_cat():
1342+
name = f"test_{uuid.uuid4().hex}"
1343+
cat = self.env["res.partner.category"].create({"name": name})
1344+
self.env["ir.model.data"].create(
1345+
{"name": name, "module": "base", "model": "res.partner.category", "res_id": cat.id}
1346+
)
1347+
return cat
1348+
1349+
cat_1 = create_cat()
1350+
cat_2 = create_cat()
1351+
cat_3 = create_cat()
1352+
1353+
# `category_id` is a m2m, so in ON DELETE CASCADE. We need a m2o.
1354+
self.env.cr.execute(
1355+
"ALTER TABLE res_partner ADD COLUMN _cat_id integer REFERENCES res_partner_category(id) ON DELETE SET NULL"
1356+
)
1357+
p1 = self.env["res.partner"].create({"name": "test delete_unused"})
1358+
1359+
# set the `_cat_id` value in SQL as it is not know by the ORM
1360+
self.env.cr.execute("UPDATE res_partner SET _cat_id=%s WHERE id=%s", [cat_1.id, p1.id])
1361+
1362+
if hasattr(self, "_savepoint_id"):
1363+
self.addCleanup(self.env.cr.execute, f"SAVEPOINT test_{self._savepoint_id}")
1364+
self.addCleanup(cat_1.unlink)
1365+
self.addCleanup(cat_2.unlink)
1366+
self.addCleanup(cat_3.unlink)
1367+
self.addCleanup(p1.unlink)
1368+
1369+
self.addCleanup(self.env.cr.execute, "ALTER TABLE res_partner DROP COLUMN _cat_id")
1370+
1371+
return cat_1, cat_2, cat_3
1372+
1373+
def test_delete_unused_base(self):
1374+
tx = self.env["res.currency"].create({"name": "TX1", "symbol": "TX1"})
1375+
self.env["ir.model.data"].create({"name": "TX1", "module": "base", "model": "res.currency", "res_id": tx.id})
1376+
1377+
deleted = util.delete_unused(self.env.cr, "base.TX1")
1378+
self.assertEqual(deleted, ["base.TX1"])
1379+
self.assertFalse(tx.exists())
1380+
1381+
def test_delete_unused_cascade(self):
1382+
cat_1, cat_2, cat_3 = self._prepare_test_delete_unused()
1383+
deleted = util.delete_unused(self.env.cr, f"base.{cat_1.name}", f"base.{cat_2.name}", f"base.{cat_3.name}")
1384+
1385+
self.assertEqual(set(deleted), {f"base.{cat_2.name}", f"base.{cat_3.name}"})
1386+
self.assertTrue(cat_1.exists())
1387+
self.assertFalse(cat_2.exists())
1388+
self.assertFalse(cat_3.exists())
1389+
1390+
def test_delete_unused_tree(self):
1391+
cat_1, cat_2, cat_3 = self._prepare_test_delete_unused()
1392+
1393+
cat_1.parent_id = cat_2.id
1394+
cat_2.parent_id = cat_3.id
1395+
util.flush(cat_1)
1396+
util.flush(cat_2)
1397+
1398+
deleted = util.delete_unused(self.env.cr, f"base.{cat_3.name}")
1399+
1400+
self.assertEqual(deleted, [])
1401+
self.assertTrue(cat_1.exists())
1402+
self.assertTrue(cat_2.exists())
1403+
self.assertTrue(cat_3.exists())
1404+
1405+
def test_delete_unused_multi_cascade_fk(self):
1406+
"""
1407+
When there are multiple children, the hierarchy can be build from different columns
1408+
1409+
cat_3
1410+
| via `_test_id`
1411+
cat_2
1412+
| via `parent_id`
1413+
cat_1
1414+
"""
1415+
cat_1, cat_2, cat_3 = self._prepare_test_delete_unused()
1416+
1417+
self.env.cr.execute(
1418+
"ALTER TABLE res_partner_category ADD COLUMN _test_id integer REFERENCES res_partner_category(id) ON DELETE CASCADE"
1419+
)
1420+
self.addCleanup(self.env.cr.execute, "ALTER TABLE res_partner_category DROP COLUMN _test_id")
1421+
1422+
cat_1.parent_id = cat_2.id
1423+
util.flush(cat_1)
1424+
self.env.cr.execute("UPDATE res_partner_category SET _test_id = %s WHERE id = %s", [cat_3.id, cat_2.id])
1425+
1426+
deleted = util.delete_unused(self.env.cr, f"base.{cat_3.name}")
1427+
1428+
self.assertEqual(deleted, [])
1429+
self.assertTrue(cat_1.exists())
1430+
self.assertTrue(cat_2.exists())
1431+
self.assertTrue(cat_3.exists())
1432+
13401433

13411434
class TestEditView(UnitTestCase):
13421435
@parametrize(

src/util/records.py

Lines changed: 66 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1200,7 +1200,9 @@ def delete_unused(cr, *xmlids, **kwargs):
12001200
Remove unused records.
12011201
12021202
This function will remove records pointed by `xmlids` only if they are not referenced
1203-
from any table.
1203+
from any table. For hierarchical records (like product categories), it verifies
1204+
if the children marked as cascade removal are also not referenced. In which case
1205+
the record and its children are all removed.
12041206
12051207
.. note::
12061208
The records that cannot be removed are set as `noupdate=True`.
@@ -1250,9 +1252,45 @@ def delete_unused(cr, *xmlids, **kwargs):
12501252
table = table_of_model(cr, model)
12511253
res_id_to_xmlid = dict(zip(ids, xids))
12521254

1253-
sub = " UNION ".join(
1255+
cascade_children = [
1256+
fk_col
1257+
for fk_tbl, fk_col, _, fk_act in get_fk(cr, table, quote_ident=False)
1258+
if fk_tbl == table and fk_act == "c"
1259+
]
1260+
if cascade_children:
1261+
if len(cascade_children) == 1:
1262+
join = format_query(cr, "t.{}", cascade_children[0])
1263+
else:
1264+
join = sql.SQL("ANY(ARRAY[{}])").format(
1265+
sql.SQL(", ").join(sql.Composed([sql.SQL("t."), sql.Identifier(cc)]) for cc in cascade_children)
1266+
)
1267+
1268+
kids_query = format_query(
1269+
cr,
1270+
"""
1271+
WITH RECURSIVE _child AS (
1272+
SELECT id AS root, id AS child
1273+
FROM {0}
1274+
WHERE id = ANY(%(ids)s)
1275+
UNION -- don't use UNION ALL in case we have a loop
1276+
SELECT c.root, t.id as child
1277+
FROM {0} t
1278+
JOIN _child c
1279+
ON c.child = {1}
1280+
)
1281+
SELECT root AS id, array_agg(child) AS children
1282+
FROM _child
1283+
GROUP BY root
1284+
""",
1285+
table,
1286+
join,
1287+
)
1288+
else:
1289+
kids_query = format_query(cr, "SELECT id, ARRAY[id] AS children FROM {0} WHERE id = ANY(%(ids)s)", table)
1290+
1291+
sub = " UNION ALL ".join(
12541292
[
1255-
'SELECT 1 FROM "{}" x WHERE x."{}" = t.id'.format(fk_tbl, fk_col)
1293+
format_query(cr, "SELECT 1 FROM {} x WHERE x.{} = ANY(s.children)", fk_tbl, fk_col)
12561294
for fk_tbl, fk_col, _, fk_act in get_fk(cr, table, quote_ident=False)
12571295
# ignore "on delete cascade" fk (they are indirect dependencies (lines or m2m))
12581296
if fk_act != "c"
@@ -1261,18 +1299,27 @@ def delete_unused(cr, *xmlids, **kwargs):
12611299
]
12621300
)
12631301
if sub:
1264-
cr.execute(
1265-
"""
1266-
SELECT id
1267-
FROM "{}" t
1268-
WHERE id = ANY(%s)
1269-
AND NOT EXISTS({})
1270-
""".format(table, sub),
1271-
[list(ids)],
1302+
query = format_query(
1303+
cr,
1304+
r"""
1305+
WITH _kids AS (
1306+
{}
1307+
)
1308+
SELECT t.id
1309+
FROM {} t
1310+
JOIN _kids s
1311+
ON s.id = t.id
1312+
WHERE NOT EXISTS({})
1313+
""",
1314+
kids_query,
1315+
table,
1316+
sql.SQL(sub),
12721317
)
1273-
ids = map(itemgetter(0), cr.fetchall()) # noqa: PLW2901
1318+
cr.execute(query, {"ids": list(ids)})
1319+
sub_ids = list(map(itemgetter(0), cr.fetchall()))
1320+
else:
1321+
sub_ids = list(ids)
12741322

1275-
ids = list(ids) # noqa: PLW2901
12761323
if model == "res.lang" and table_exists(cr, "ir_translation"):
12771324
cr.execute(
12781325
"""
@@ -1281,16 +1328,16 @@ def delete_unused(cr, *xmlids, **kwargs):
12811328
WHERE t.lang = l.code
12821329
AND l.id = ANY(%s)
12831330
""",
1284-
[ids],
1331+
[sub_ids],
12851332
)
1286-
for tid in ids:
1287-
remove_record(cr, (model, tid))
1288-
deleted.append(res_id_to_xmlid[tid])
1333+
1334+
remove_records(cr, model, sub_ids)
1335+
deleted.extend(res_id_to_xmlid[r] for r in sub_ids if r in res_id_to_xmlid)
12891336

12901337
if deactivate:
1291-
deactivate_ids = tuple(set(res_id_to_xmlid.keys()) - set(ids))
1338+
deactivate_ids = tuple(set(sub_ids) - set(ids))
12921339
if deactivate_ids:
1293-
cr.execute('UPDATE "{}" SET active = false WHERE id IN %s'.format(table), [deactivate_ids])
1340+
cr.execute(format_query(cr, "UPDATE {} SET active = false WHERE id IN %s", table), [deactivate_ids])
12941341

12951342
if not keep_xmlids:
12961343
query = """

0 commit comments

Comments
 (0)