[#38506] - [4.2] Change the db calls back to the getDbo
- Fixed in Code Base
- 18 Aug 2022
- Medium
- Build: 4.2-dev
- # 38506
- Diff
- Digital-Peak:model/dbo
User tests: Successful: Unsuccessful:
Pull Request for Issue #38505.
Summary of Changes
When an extension is overriding the getDbo function in a model and returning it's own database instance, then the parent classes are not using it anymore as they use getDatabase now. Correct way would be to inject the database instance through the argument in the constructor config or setDbo/setDatabase.
The following code snippet shows the code issue:
class MyModel extends ListModel {
private $mydb;
public function __construct(..) {
parent::__construct(...);
$ẗhis->mydb = MyHelper::getGridDB();
}
public function getDbo() {
return $this->mydb;
}
}
Correctly you would write:
class MyModel extends ListModel {
public function __construct(..) {
parent::__construct(...);
$this->setDbo(MyHelper::getGridDB());
}
}
To maintain full backwards compatibility, this pr changes back the usage to getDbo.
Testing Instructions
Use the extension from issue #38505.
Actual result BEFORE applying this Pull Request
It throws an error.
Expected result AFTER applying this Pull Request
It works as expected.
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Libraries |
| Labels |
Added:
Release Blocker
?
|
||
| Category | Libraries | ⇒ | Libraries Unit Tests |
| Labels |
Added:
?
|
||
I have tested this item
After applying the patch the external database connection is being used which it did not use before the patch.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38506.
@FoTo50 Can you test this pull request please?
Thanks very much for this good explanation. This now works just as expected, in 4.1.5 and in 4.2.0
I think this would be a good example to modify https://docs.joomla.org/Connecting_to_an_external_database with since this page is probably for most the starting point when asking Google for it
@FoTo50 Great to hear it is working again.
Could you please mark your test on our issue tracker as well? All you need to do is:
- Go to https://issues.joomla.org/tracker/joomla-cms/38506
- Click Login with Github
- After you are logged in you can click the
Test thisblue button - Write in the box that it works as expected
- Click on the
Submit test result
This will then be marked here as a human test result and we can get this merged. Thank you.
Even when this article still uses the old code, it shows you how to do it the correct way, by using the setDbo function and not overriding getDbo. It is not safe to rely on the base class, that it uses all the time getDbo when there is even a protected variable $_db in the BaseDatabaseModel class. So I suggest you adapt your code, as this change will be reverted in 5.0.
Anyway, happy that the pr works also for you now and the issue is fixed.
I have tested this item
works as expected, thnx
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38506.
| Status | Pending | ⇒ | Ready to Commit |
RTC
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38506.
| Status | Ready to Commit | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2022-08-18 08:59:46 |
| Closed_By | ⇒ | fancyFranci | |
| Labels |
Added:
?
|
||
Thank you very much!
@FoTo50 Can you test this pull request please?