[#37321] - Refactored Factory::getUser() to ::getApplication()->getIdentity()
- Closed
- 29 Jun 2022
- Medium
- Build: 4.1-dev
- # 37321
- Diff
- pe7er:refactoring-getuser
User tests: Successful: Unsuccessful:
Joomla 4.1 uses the deprecated Factory::getUser(). I've changed it in a couple of files to Factory::getApplication()->getIdentity().
- Does it make sense to manually replace it like this?
- Or will it be done automatically with a global search & replace?
- Please advice me if I could continue to refactor it like this in other files as well?
Summary of Changes
This PR changes Factory::getUser in 5 files to Factory::getApplication()->getIdentity.
In some cases I added Exception to the code block and as name space.
$user = Factory::getUser($userId);
to
$user = Factory::getApplication()->getIdentity($userId);
$user = Factory::getUser();
to
$user = Factory::getApplication()->getIdentity();
if (!Factory::getUser()->authorise('core.admin'))
to
if (!Factory::getApplication()->getIdentity()->authorise('core.admin'))
Testing Instructions
Please do a code inspection.
It should not change any functionality and Joomla should continue work correctly.
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Administration com_admin |
| Labels |
Added:
?
|
||
A small addition:
Factory::getUser()->id
or
Factory::getUser()->get('id')
to
Factory::getApplication()->getIdentity()->id
It should not change any functionality and Joomla should continue work correctly.
Point of interest:
#30834 (comment) and
#30834 (comment)
Generally speaking this is OK. Worth noting that behaviours aren't exactly identical as noted in #36448 so you can't do a full auto-replace in an IDE.
@laoneo HTMLView's at least will also be hard coupled to the application for the active template (https://github.com/joomla/joomla-cms/blob/4.1-dev/libraries/src/MVC/View/HtmlView.php#L377). So unless we plan to start injecting that whilst obviously the statics aren't ideal - it might be that we inject the application into the view in the constructor going forwards anyhow. Even if we inject the user because there's going to be authentication checks throughout the view probably makes more sense in the constructor than as a service there anyhow.
What about the UserFactory?
Agree, this one should be closed as users will be injected into views and models.
This pull request has automatically rebased to 4.2-dev.
@joomdonation thanks for your explanation and for your PR to solve this in a better way
@laoneo thanks for your PR to solve this in a better way
closing this one...
| Status | Pending | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2022-06-29 08:18:47 |
| Closed_By | ⇒ | pe7er | |
| Labels |
Added:
Information Required
|
||
Thanks for your pr. Honestly I would like to see a change to introduce a user service which can be injected into the view, to get a user from rather exchanging a static function call with another.