Information Required ? Pending

User tests: Successful: Unsuccessful:

avatar pe7er
pe7er
19 Mar 2022

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.

avatar pe7er pe7er - open - 19 Mar 2022
avatar pe7er pe7er - change - 19 Mar 2022
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 19 Mar 2022
Category Administration com_admin
avatar pe7er pe7er - change - 19 Mar 2022
Labels Added: ?
avatar laoneo
laoneo - comment - 20 Mar 2022

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.

avatar sandewt
sandewt - comment - 20 Mar 2022

A small addition:

Factory::getUser()->id
or
Factory::getUser()->get('id')
to
Factory::getApplication()->getIdentity()->id

avatar sandewt
sandewt - comment - 20 Mar 2022

It should not change any functionality and Joomla should continue work correctly.

Point of interest:
#30834 (comment) and
#30834 (comment)

avatar wilsonge
wilsonge - comment - 20 Mar 2022

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.

avatar laoneo
laoneo - comment - 20 Mar 2022

What about the UserFactory?

avatar joomdonation
joomdonation - comment - 25 Jun 2022

@pe7er You might want to close this PR because we now use a different approach:

  • PR #37498 introduced a new way to get current user object in views and models class
  • The Factory::getUser() replacement on view classes was merged #38006
  • There is also a PR made to replace Factory::getUser() in model classes, too, see #38090
avatar laoneo
laoneo - comment - 27 Jun 2022

Agree, this one should be closed as users will be injected into views and models.

avatar HLeithner
HLeithner - comment - 27 Jun 2022

This pull request has automatically rebased to 4.2-dev.

avatar pe7er
pe7er - comment - 29 Jun 2022

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

avatar pe7er pe7er - change - 29 Jun 2022
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2022-06-29 08:18:47
Closed_By pe7er
Labels Added: Information Required
avatar pe7er pe7er - close - 29 Jun 2022

Add a Comment

Login with GitHub to post a comment