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

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

tested successfully on postgresql

unable to mark successfull test on issue tracker

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

RTC

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?

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™.

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

@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.

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.

Try this #13592

Closing in favor of #13592