[#45242] - [5.4] Refactor components Table classes
- Fixed in Code Base
- 16 May 2025
- Medium
- Build: 5.4-dev
- # 45242
- Diff
- joomdonation:use_getdatabase_for_component_table_classes
User tests: Successful: Unsuccessful:
Pull Request for Issue # .
Summary of Changes
This is follow PR for #45165. It makes the following changes to components Table classes :
- Replace
DatabaseDriverbyDatabaseInterfacefor $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:
Testing Instructions
Please try create and modify tags, user notes, private messages, template styles, install extension, set up and use MFA, use workflows... and make sure it is still working like before. These basic operations are already performed by our automated tests, and these tests are passed, so you do not need to test all of these actions. Just pick 2 from the list actions above would be enough.
Additional If you could do code review, it would be great.
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
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Administration com_banners com_contact com_content com_fields com_menus com_messages com_newsfeeds com_redirect com_tags com_templates com_users com_workflow |
| Labels |
Added:
PR-5.4-dev
|
||
| Status | Pending | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2025-05-09 13:51:54 |
| Closed_By | ⇒ | joomdonation | |
| Labels |
Added:
Code Review
|
||
Similar with other PR, I'm closing this one and will open multiple smaller PRs to make it possible for real user tests.
| Status | Closed | ⇒ | New |
| Closed_Date | 2025-05-09 13:51:54 | ⇒ | |
| Closed_By | joomdonation | ⇒ |
| Status | New | ⇒ | Pending |
PR has been reopened and testing instructions have been updated.
Let's hope we find testers now.
@joomdonation Any reason why administrator/components/com_finder/src/Table/FilterTable.php is not touched by this PR?
@richard67 Look like table classes from some components like com_finder, com_guidedtours were not processed for some reasons. I will check it again on tomorrow.
| Labels |
Removed:
Code Review
|
||
| Category | Administration com_banners com_contact com_content com_fields com_menus com_messages com_newsfeeds com_redirect com_tags com_templates com_users com_workflow | ⇒ | Administration com_banners com_contact com_content com_fields com_finder com_installer com_menus com_messages com_newsfeeds com_redirect com_tags com_templates com_users com_workflow |
@joomdonation Thanks for adding the other tables. I've allowed myself to modify the testing instructions a bit so they better cover this PR here.
I have tested this item ✅ successfully on 365a426
I've successfully tested Blog Sample Data installation, creation and modification of template styles, user notes, private messages, users, using MFA 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.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45242.
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/45242/downloads/84669/
I have tested this item ✅ successfully on 365a426
I have tested this successfully.
I did these 3 categories:
I installed multiple times Events Booking v5.0.5, J2Store v4 4.0.12, Convert Forms 5.0.0, Email Protector v6.1.8 and uninstalled them multiple times.
I did 2FA - PassKey (MacBook Pro) 6-7 times
I created 2 nested tags for articles.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45242.
| Status | Pending | ⇒ | Ready to Commit |
RTC
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45242.
Tested successfully together with 45243
- 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
| Labels |
Added:
RTC
|
||
| Status | Ready to Commit | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2025-05-16 04:38:00 |
| Closed_By | ⇒ | muhme |
thanks to all
Thanks all from me, too :)
@joomdonation code review is not enough, you please write test instruction, in this case test all touched table classes