Skip to content

Conversation

@joomdonation
Copy link
Contributor

@joomdonation joomdonation commented Mar 28, 2025

Pull Request for Issue # .

Summary of Changes

This is follow PR for #45165. It makes the following changes to components Table classes :

  • 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:

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

@HLeithner
Copy link
Member

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

@joomdonation
Copy link
Contributor Author

Similar with other PR, I'm closing this one and will open multiple smaller PRs to make it possible for real user tests.

@joomdonation joomdonation reopened this May 9, 2025
@richard67
Copy link
Member

PR has been reopened and testing instructions have been updated.

Let's hope we find testers now.

@richard67
Copy link
Member

@joomdonation Any reason why administrator/components/com_finder/src/Table/FilterTable.php is not touched by this PR?

@joomdonation
Copy link
Contributor Author

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

@richard67
Copy link
Member

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

@richard67
Copy link
Member

richard67 commented May 12, 2025

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.

@richard67
Copy link
Member

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/

@exlemor
Copy link

exlemor commented May 12, 2025

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.

@richard67
Copy link
Member

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45242.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label May 13, 2025
@richard67 richard67 added this to the Joomla! 5.4.0 milestone May 13, 2025
@muhme
Copy link
Contributor

muhme commented May 15, 2025

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

@muhme muhme merged commit d0e1922 into joomla:5.4-dev May 16, 2025
3 checks passed
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label May 16, 2025
@muhme
Copy link
Contributor

muhme commented May 16, 2025

thanks to all

@joomdonation
Copy link
Contributor Author

Thanks all from me, too :)

@joomdonation joomdonation deleted the use_getdatabase_for_component_table_classes branch September 9, 2025 10:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants