Delete UCM content entries when Joomla articles are deleted

[roland-d]

Pull Request for Issue #8770 .

Summary of Changes

Deleting an article doesn't remove the article from the ucm_content table because an array is sent while an integer is expected. This pull request fixes this.

Testing Instructions

1. Create an article and make sure to assign a tag to the article
2. Check the ucm_content table to see that the article is there as well
3. Trash the article
4. Delete the article from trash
5. Check the ucm_content table and see that the article is still there
6. Apply patch
7. Create an article and make sure to assign a tag to the article
8. Check the ucm_content table to see that the article is there as well
9. Trash the article
10. Delete the article from trash
11. Check the ucm_content table and see that the article has now been removed

Documentation Changes Required

None

[wilsonge]

Currently you're only going to get the result of the last item. This should cascade over all items you are deleting

[wilsonge]

Why do we have an array here? It seems kinda weird?

[roland-d]

@wilsonge that is because an array is passed to this function. That is because you can select multiple items to be deleted at once.

[alikon]

tested successfully on postgresql

[alikon]

unable to mark successfull test on issue tracker

---

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

[sanderpotjer]

Successfully tested. Related #__ucm_content deleted after emptying the article trash.

---

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

[jeckodevelopment]

RTC

---

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

[wilsonge]

I still don't understand how this is getting an array. The call stack should look something like:

1. JTable::delete for the single ID (note doesn't support an array of data)
2. JTable::delete calls JTableObserverTags::onBeforeDelete via the observer's for JTable
3. JTableObserverTags::onBeforeDelete calls JHelperTags::deleteTagData

I don't understand why we have an array for a single item?

[mbabker]

deleteTagData isn't receiving an array, it's casting the $contentItemId parameter to an array. You're adding undocumented functionality.

The method is documented to receive an integer. Anything calling it with an array is Doing It Wrong™.

[wilsonge]

OK So our problem is this https://github.com/joomla/joomla-cms/blob/staging/libraries/joomla/table/table.php#L942 - So this is always an array - but so we support multi-key tables. I don't think ucm content supports the concept of a multikey special table. So what we need to do is grab the array here. Check it only has one value. If it does use that. If it doesn't, throw and InvalidArgumentException

[roland-d]

@mbabker I should change it further upstream then because Joomla is sending an array to this function.

@wilsonge since there is always an array are you saying we should throw that exception? As we can delete more than one item at once we should just loop through this array.

Whatever we do it should be fixed, it's a real mess now.

[mbabker]

It works now because $contentItemId doesn't even get used in the unTagItem() method (it gets passed but the method does other stuff to figure out what ID to remove, 🤦). So if this param is really an array I'd love to know how JTableCorecontent::deleteByContentId() works correctly because it has no provisioning to convert the value to a single scalar type, so perhaps the bug is with that method specifically and the changes here just happen to make it work while also introducing new undocumented API behavior to account for what is apparently already incorrect use of the API.

Either way here we have a massive documentation and behavior inconsistency and taking B/C into consideration since the behavior is documented now to expect an integer that needs to continue working until 4.0 at a minimum. If this patch is going to go through, the API MUST document the array behavior too; we already suck at documenting our API or how to use the code, let's not add more magical boxes please.

[wilsonge]

Try this #13592

[roland-d]

Closing in favor of #13592