[#34906] - [4.0] Use getIdentity for plugins
- Closed
- 29 Dec 2022
- Medium
- Build: 4.0-dev
- # 34906
- Diff
- joomdonation:getIdentify_for_plugins
User tests: Successful: Unsuccessful:
Pull Request for Issue # .
Summary of Changes
This PR does two things:
- Add $app property to plugins (when it is missing) and use that injected application object to replace Factory::getApplication()
- Use $this->app->getIdentity() to get current user object instead of the deprecated method Factory::getUser();
Testing Instructions
Add it is small changes but target multiple plugins, I prefer to have careful code review here.
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Front End Plugins |
richard67
- | Title |
|
||||||
| Labels |
Added:
?
|
||
You've included a system plugin :D I'm also unsure though
After looking at our Application classes more carefully, I see that the Identity is available very early in application life cycle. It is available in initialise phase, right after Factory::getUser() line is called https://github.com/joomla/joomla-cms/blob/4.0-dev/libraries/src/Application/SiteApplication.php#L621 , long before system plugins are imported and triggered, so it is safe to use $this->app->getIdentity(); to get current logged in user.
With that said, I do the replacement in system plugins, too. Will test with update from Joomla 3.10 to 4.0 to make sure everything is still working OK.
So I tested the update from 3.10.0-dev to the package on PR and it is working OK. Tested several features in the CMS like login, logout, action logs, editor-xtds... changes by this PR and it is working OK, too.
| Labels |
Added:
?
Removed: ? |
||
This pull request has automatically rebased to 4.2-dev.
Can you fix the conflicts here? Keep in mind that the service provider plugins have the app already injected. Would love to get this merged.
| Status | Pending | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2022-12-29 15:14:06 |
| Closed_By | ⇒ | joomdonation | |
| Labels |
Added:
Conflicting Files
?
Removed: ? |
||
There are so many conflicts now, so I'm closing this PR. Will review it at a later time base on latest code from plugins which we are having now.
You've included a system plugin :D I'm also unsure though. Action logs (and user plugins) will also be loaded on update - hence why they get the extra warnings in the pre-update checker - so this probably does need a test unless you remove all of those cases