Removing menu children on component install can destroy the whole menu tree

[klas]

Menu table inherits Nested and since delete without second argument set to false, removes node and its children. As all child ids that need to be removed are already collected in previous step this means:
a) errors are generated if tree is consistent - all children are already removed in first delete, so all following deletes fail
b) with inconsistent trees this can delete random ranges of menus as delete only looks at lft-rgt values. This is not a unusual scenario as admin menus were previously not checked for consistency - I just had the whole menu wiped out due to faulty record.

To prevent this I propose that we delete only explicit records as I assume was also originally intended due to ids collection in previous step.

Summary of Changes

Set Children parameter to false in delete

Testing Instructions

Copy existing record from menu tree from one of installed 3rd party components (needs to be installable, not core) to create a faulty record: : copy existing record, change parent to non-existing id and set lft to 0 and rgt to 99999. It needs to be an admin menu (client = 1 ).

Update(reinstall) component with faulty record.

Expected result

Record disapears, the rest of the records remain.

Actual result

All records are erased.

Documentation Changes Required

[csthomas]

It helps because it do not need to delete children, which are already checked.

But it still leaves a mess after call this:

joomla-cms/libraries/src/Table/Nested.php

Lines 625 to 659 in e0e8c95

	// Delete the node.
	$query->clear()
	->delete($this->_tbl)
	->where('lft = ' . (int) $node->lft);
	$this->_runQuery($query, 'JLIB_DATABASE_ERROR_DELETE_FAILED');
	// Shift all node's children up a level.
	$query->clear()
	->update($this->_tbl)
	->set('lft = lft - 1')
	->set('rgt = rgt - 1')
	->set('level = level - 1')
	->where('lft BETWEEN ' . (int) $node->lft . ' AND ' . (int) $node->rgt);
	$this->_runQuery($query, 'JLIB_DATABASE_ERROR_DELETE_FAILED');
	// Adjust all the parent values for direct children of the deleted node.
	$query->clear()
	->update($this->_tbl)
	->set('parent_id = ' . (int) $node->parent_id)
	->where('parent_id = ' . (int) $node->$k);
	$this->_runQuery($query, 'JLIB_DATABASE_ERROR_DELETE_FAILED');
	// Shift all of the left values that are right of the node.
	$query->clear()
	->update($this->_tbl)
	->set('lft = lft - 2')
	->where('lft > ' . (int) $node->rgt);
	$this->_runQuery($query, 'JLIB_DATABASE_ERROR_DELETE_FAILED');
	// Shift all of the right values that are right of the node.
	$query->clear()
	->update($this->_tbl)
	->set('rgt = rgt - 2')
	->where('rgt > ' . (int) $node->rgt);
	$this->_runQuery($query, 'JLIB_DATABASE_ERROR_DELETE_FAILED');

[csthomas]

I have tested this item ✅ successfully on 904d02b

Code review.

Besides my doubts (above) but there is no better solution now. I mark it as success.

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/18404.}

[alikon]

@csthomas open a new issue for that

[alikon]

I have tested this item ✅ successfully on 904d02b

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/18404.}

[ghost]

RTC after two successful tests.

[izharaazmi]

@alikon @csthomas What happens to the children after we delete a parent node, with this PR?

[csthomas]

It depends how much is your database table corrupted.

1. If there is no errors then there should not be any children because all menu items of the component are deleted, one by one.

2. Otherwise, for example, if the values of column lft and rgt are corrupted for items X and Y that are contained (based on lft and rgt values) in item Z and you want to delete Z, then X and Y will be move one level up, lft and rgt will be updated, [X,Y].parent_id = Z.parent_id. This is the good side. The bad side is, when you delete corrupted item, you could move others menu items to one level up and other menu items may start be corrupted too.

If you want to read the code then take a look at code that I pasted above, joomla-cms/libraries/src/Table/Nested.php line 625.

Such operation, delete, runs a few sql queries in non atomic operation, executed separately and based on lft column, which is not unique in database tables.