[5.4] Refactor CMS Table classes

[joomdonation]

Pull Request for Issue # .

Summary of Changes

Similar to #45242, this is follow PR for #45165. It makes the following changes to CMS Table classes defined in libraries/src/Table folder:

• Replace DatabaseDriver by DatabaseInterface for $db param in class constructor
• Use $this->getDatabase() method to get db object instead of using deprecated $this->_db property
• Replace $this->getDbo() calls by $this->getDatabase()
• Introduce local $db variable to replace multiple $db->getDatabase() call if needed

I do not know if it is useful or not but I want to mention that the task #2 and #3 above can be done automatically using two useful two rector rules below:

• PropertyFetchToMethodCallRector
• RenameMethodRector

Testing Instructions

It is not needed to test any change, we can expect that when it works for some that it works for all. Please test if you can create an Article, Category, Menu Item, User, Viewlevel. Just pick 2 of the list.

Actual result BEFORE applying this Pull Request

Works

Expected result AFTER applying this Pull Request

Works, without using deprecated code

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

[brianteeman]

is there a list being maintained somewhere of all the rector rules that have been used

[joomdonation]

is there a list being maintained somewhere of all the rector rules that have been used

Currently, we don't. For every PR I made which use rector rules, I clearly mentioned the rules I use and link to documentation of the rule so that if someone wants to use it for his extensions, he can follow. Ideally, we should have rector rules to help automatically refactoring code from old Joomla version to compatible with newer Joomla version like other projects do, for example craftcms/rector, but right now, we do not have anyone can handle that work yet.

[HLeithner]

@joomdonation code review is not enough, cloud you please write test instruction, in this case test all touched table classes?

[joomdonation]

@HLeithner If we could not do code review and want to have testing instructions for human test, I think we would have to split this PR into multiple small PRs, each PR for one table class. If this is the case, I will close the PR and open separate small PRs

[HLeithner]

I think own prs are not needed, just a list of components touched and the test instruction to open and close and save something should be enough.

[joomdonation]

Aren't all these basic operations are covered by our tests already?

[HLeithner]

at this time I think that doesn't matter because we require 2 real user tests (can be discussed in maintainers), this pr came up in the maintainer meeting yesterday.

[joomdonation]

For this kind of PR, I don't think real user tests are enough because the testing instructions for changes in multiple places like that might not cover all the cases. Careful code review + tests pass should be better, I think.

[HLeithner]

code review is done by the person that merges anyway. As said discuss this in maintainers chat/meeting please.

[joomdonation]

I'm closing this PR because it touch multiple table classes and it is not easy for real user tests. I will open multiple small PRs instead to make it is easier for human testing

[richard67]

PR has been reopened and testing instructions have been updated.

Let's hope we find testers now.

[richard67]

I have tested this item ✅ successfully on 3621c2e

I've successfully tested Blog Sample Data installation, creation and modification of users and user groups, categories, articles and installing and using a 3rd party extension which uses categories and assets with MySQL 8 and some of that also with PostgreSQL, both on PHP 8.4.

In addition, I've made a detailed code review of the changes.

Finally I've triggered a branch update for this PR to make sure system tests are passing with the recently upmerged change that the system tests check for errors in server logs (PR #45409 ).

---

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

[richard67]

Hints for other testers:

• Make sure to have PHP error reporting set to maximum in global configuration so you would see any PHP notices or warnings or errors, if there were some.
• As this PR does not make any changes where the type of the database is relevant, it is sufficient to test with one kind of database (MySQL, MariaDB or PostgreSQL).
• The PR should be tested on a current 5.4-dev or latest 5.4 nightly build. If you don't have that, you can find here patched installation and update packages which are the latest 5.4-dev plus the changes from this PR: https://artifacts.joomla.org/drone/joomla/joomla-cms/5.4-dev/45243/downloads/84629/

[joomdonation]

Thanks Richard for helping with CS and testing.

[dautrich]

I have tested this item ✅ successfully on 3621c2e

---

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

[richard67]

RTC

---

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

[dautrich]

I tested on a Nightly Build 5.4-dev, PHP 8.3.6, MySQL 8.0.30 locally under Laragon.

[muhme]

Tested successfully together with 45242

• Environment Apple arm64 M3, JBT (Docker-based), pgsql socket, PHP 8.4.7 (with Display Errors: On and error_reporting: maximum), 5.4-dev + 45242 + 45243, Joomla Log Almost Everything and Log Deprectaed API
• ✅ Joomla System Tests
  • Individual tests fail, but pass when repeated as a single test (as is often the case)
• ✅ Manually tested
  • Created a user
  • Created an article and featured on home
  • Installed module zitat-service.de from JED, published and placed on all pages

[muhme]

thanks to all

[joomdonation]

Thanks all for your help !