[sergiocorato]

No description provided.

[hbrunn]

/ocabot merge nobump

[OCA-git-bot]

On my way to merge this fine PR!
Prepared branch master-ocabot-merge-pr-455-by-hbrunn-bump-nobump, awaiting test results.

[pedrobaeza]

I think the solution is to restrict that search to views, as if not, they won't have that field anyway. And document everything. Another option is to include the parent field name as optional.

[pedrobaeza]

Suggested change

	if delete_childs:
	if delete_childs and record._name == "ir.ui.view":

[hbrunn]

@pedrobaeza I disagree. any model that supports the child_of operator can profit from this. note this originates from a real use case in OCA/OpenUpgrade#5719 (comment)

[MiquelRForgeFlow]

I think @pedrobaeza is concerned with inherit_id. Not every model has that field.

[hbrunn]

ahh right. then this should indeed do inherit_id for ir.ui.view and id for the rest

[pedrobaeza]

Not id, but the field expressing the parent-child relation (parent_id or whatever). That's what inherit_id means in this context.

[hbrunn]

then we seem to differ about what this should do. for a generic model with a parent_id field, and entries

id: 42
parent_id: None

id: 43
parent_id: 42

I expect delete...xml_id(['xml_id_of_record_42'], delete_childs=True) to delete the record with id 43

[pedrobaeza]

Yeah, that's it, both 42 and 43 will be removed. You point the parent record, and all the childrens recursively are removed. What I'm saying is that the inherit_id field is the parent many2one reference.

[MiquelRForgeFlow]

and all the childrens recursively are removed

Not exactly. To be truly recursive, the method delete_records_safely_by_xml_id should call itself.

[pedrobaeza]

Then the implementation is half-baked in my opinion, as I understand the use case about removing "this view and any descendant". Removing a view (or other record), and only the children may give error.

[hbrunn]

I'm closing this in favor of the somewhat more advanced #466