[5.4] Change the table event name from onBeforeDelete to on TableBeforeDelete

[rdeutz]

Pull Request for Issue #45584 .

Summary of Changes

At some moment in the past we changed the events for the Table* classes from onAction to onTableAction. It was obviously missed to make this change for the delete method of the nested table class. This results in the situation that history entires for categories and tags are not deleted.

a) This fix is a bug fix but also kind of b/c break.
We need to document the b/c break. I would expect that It is unlikely that the event is used.

b) Further more we are not cleaning up the existing entires in the history table for not longer existing tags and categories.
Maybe something we can do in a helath checker or pre-update check, but not as part of this PR.

Testing Instructions

• Create one tag
• Check database table __history and see item_id entry com_tags.tag.id
• Trash and delete the tag

Actual result BEFORE applying this Pull Request

The database table __history entry for com_tags.tag.id is still existing

Expected result AFTER applying this Pull Request

The database table __history entry for com_tags.tag.id is not existing

Link to documentations

Please select:

• Documentation link for docs.joomla.org:

• No documentation changes for docs.joomla.org needed

• Pull Request link for manual.joomla.org:

• No documentation changes for manual.joomla.org needed

[exlemor]

I have tested this item ✅ successfully on 401b41a

Hi there, I was able to test it successfully! Thanks @rdeutz

---

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

[muhme]

I have tested this item ✅ successfully on 401b41a

Tested with JBT, before the PR with 5.3-dev branch

• Seen the error before with com_tags.tag and com_content.category

Installed the PR with graft full package

• Enabled 'Debug System' and 'Log Almost Everything'
• Created a tag with one history entry and another tag with three history entries, all history entries are deleted together with the tags
• Created a content category with one history entry and another content category with three history entries, all history entries are deleted together with the content categories
• Disabled versioning
  • Component > Tags > Options > Enable Versions > No
    • Created tag, trashed and deleted
  • Content > Categories > Options > Editing Layout > Enable Versions > No
    • Created content category, trashed and deleted
• Checked administrator/logs/* and PHP log

  ---

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

[alikon]

RTC

---

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

[bembelimen]

We should call the old event, too and deprecate it. Otherwise we make all plugins, which are using the wrong methods broken.

[rdeutz]

@bembelimen done, called old event and depreciated it as requested

[muhme]

Reset to “Pending” as there have been code changes

---

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

[rdeutz]

Reset to “Pending” as there have been code changes

It is only bringing back the old event call, would be a surprise if that breaks something.

[muhme]

Reset to “Pending” as there have been code changes
It is only bringing back the old event call, would be a surprise if that breaks something.

Sorry, I don't have this overview. Does anyone else want to set RTC again or should I just retest?

[rdeutz]

@muhme retest would be fine but I don't think we need two tests.

[muhme]

I have tested this item ✅ successfully on c293027

After branch update, successfully tested again as outlined in #46020 (comment)

---

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

[richard67]

RTC

---

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

[richard67]

@rdeutz Now as the old event is called, too, can the "b/c break" label be removed?

[brianteeman]

It says deprecated without replacement and then goes on to say use ....

Surely that is the replacement

[richard67]

It says deprecated without replacement and then goes on to say use ....

Surely that is the replacement

@brianteeman Not sure if it is really a replacement as they are not the same events, and the calls to the old events will be removed without replacement.

@rdeutz What do you think? Maybe just remove the , use onTable...Delete event at the end of the deprecation comments?

[laoneo]

This has to go into 5.4 as we don't do deprecations in patch releases.

[rdeutz]

It is a bugfix so it is 5.3.

For the message I guess you can argue in many ways, I made what Richard suggested.

[richard67]

Rebased to 5.4-dev. RTC is still valid as it was a clean rebase.

[muhme]

✅ Final retest before merge as in #46020 (comment)

[muhme]

Thank you @RobertDeutz for your contribution. Thank you @richard67, @laoneo, @bembelimen and @brianteeman for support. Thank you @exlemor for testing.