[#37498] - [4.2] Add interfaces for the objects who need a user
- Fixed in Code Base
- 30 May 2022
- Medium
- Build: 4.2-dev
- # 37498
- Diff
- Digital-Peak:j4/user/current
User tests: Successful: Unsuccessful:
Summary of Changes
As many classes need a user to check permissions or get an email address from it, there is no common way to get a current user instance. This pr introduces a new interface and trait to get/set a current user on an object. The identity aware trait offers a similar functionality but is intended to be used on applications only. But I'm open here for suggestions if we should reuse the identity trait or go with the approach in this pr.
Also I'm not sure about the name of the new interface or trait, also here open for suggestions.
This pr is mainly needed as @wilsonge doesn't want to inject the application into the models or views. If the app would be available, then we can just load the identity from it.
Testing Instructions
In the back end, open the articles list.
Actual result BEFORE applying this Pull Request
Works.
Expected result AFTER applying this Pull Request
Works.
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Administration com_content Libraries |
| Labels |
Added:
?
|
||
Not really as this is different than getIdentity in terms of logic. GetIdentity is basically the identity of the application which comes from the authentication process. While this here can be anything where the view or model should work with. But as I said, I'm open here for suggestions as the similarities are very obvious.
Just when I thought I understood the reason for this PR
| Category | Administration com_content Libraries | ⇒ | Administration com_content Libraries Unit Tests |
| Labels |
Added:
?
|
||
Why not make a trait to get the application object and the go from there?
Don't think so it would be a good decision to pass around the application in whole core. Like that there is only a dependency to a simple user and not the app. It brings the benefit to be usable on places where no application is available.
| Category | Administration com_content Libraries Unit Tests | ⇒ | Administration com_content |
| Labels |
Removed:
?
|
||
| Category | Administration com_content | ⇒ | Administration com_content Libraries Unit Tests |
| Labels |
Added:
?
|
||
Here, the current user object will only be available in model if the model is created by our normal MVC workflow (users access to the component via web browser). What if model is created by by MVCFactory ? In this case, getCurrentUser() would return null and we will get error (fatal error or at least logical error). Am I right?
That's not current user, but an empty user object?
After reading your comment a couple of times I think I do understand what you mean now. So when an extension is loading a model like $app->bootComponent('content')->getMVCFactory()->createModel('Article', 'Administrator') then the user is not the identity from the global application object. Is this what you mean? In this case I would probably fallback to Factory::getApplication()->getIdentity() here instead of an empty user and deprecate that usage for 5.
Yes, that's what I wanted to say.
@laoneo We have a codestyle failure:
FILE: /********/src/libraries/src/User/CurrentUserTrait.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
43 | ERROR | [x] No space found after comma in argument list
| | (Generic.Functions.FunctionCallArgumentSpacing.NoSpaceAfterComma)
@joomdonation Is everything OK for you now on this PR?
| Status | Pending | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2022-05-30 17:18:29 |
| Closed_By | ⇒ | roland-d |
Thanks everybody
will you be replacing getIdentity everywhere?