Multilingual: Correcting login redirect #12932

infograf768 · 2016-11-18T10:01:11Z

Pull Request for Issue #12920

When login in a multilingual site, the user should be redirected, if possible, to the associated menu item of the page containing the login module.
If redirections are set (in the login module as well as in a login menu item) they should use the possible associations for the redirect.

Summary of Changes

Using API to get the possible Itemid in order to redirect properly when Automatic Change is set to Yes in the language filter plugin.
Added detailed comments explaining what each situation may obtain.

Testing Instructions

Create a multilingual site from a clean staging. Patch.
Add one more article per language and associate them.
Set the user login preferred site language to another language than the default site language.
Set the language filter plugin to use "Automatic Change"

NOTE: if the user preferred site language is not set, the default site language will be used. Testing does not change. Just replace in the instructions below user preferred site language by site default language.

The following should work when testing, I added the code comments to clarify.

--------------
If login through a login menu item using the Internal URL redirection (on a multilingual site, that url should contain &lang=xx-XX):
// The login menu item form contains an internal URL redirection.
// This will override the automatic change to the user preferred site language.
=> The user will be redirected to that Internal URL, whatever it is.

-------------
If login from a login menu item or a login module and a Menu Item redirection is set
// The login form contains a menu item redirection. Try to get associations from that menu item.
// If any association set to the user preferred site language, redirect to that page.

-------------
// The login form does not contain a Menu Item redirection.
// The active menu item has associations.
// We redirect to the user preferred site language associated page.

-------------
// We are on a Home page, we redirect to the user preferred site language Home page.
I.e. no need to associate the Home pages. They are automatically.

-------------
If Associations are not implemented, Automatic Change set to Yes, no redirection set to a Menu Item and login from another page than the Home page, the user should be redirected to the same page.
If login from a Home page, redirection will be set to the preferred site language Home page of the user.

-------------
If automatic change is set to No, behaviour will be the same as on a monolanguage site.
I.e. redirect to the same page or, if a redirection is set in the login module or login menu item, redirection to that url or menu item.

Documentation Changes Required

None. This broke with the new router because the code was not using the proper API.

Hackwar · 2016-11-18T10:47:05Z

plugins/system/languagefilter/languagefilter.php

-					// The login menu item contains a redirection.
-					// This will override the automatic change to the user preferred language
+					// Retrieves the Itemid from a login form.
+					$uri    = new JUri($this->app->getUserState('users.login.form.return'));


What happens if no return URL is set?

My test did not show any issue.
Do you prefer I first add a conditional?

if ($this->app->getUserState('users.login.form.return')) { $uri = new JUri($this->app->getUserState('users.login.form.return')); [..]

No, I'm saying that I don't know what JUri will do if it gets a NULL value or an empty string or similar. NULL will most likely result in the current URL being used.

Not sure I understand.
If JUri gets a null value, then $itemid is null and we switch to the next elseif when we do elseif ($itemid)
which is what we want here.

I mixed up the constructor with JUri::getInstance(). This one is ok.

Hackwar · 2016-11-18T10:48:27Z

plugins/system/languagefilter/languagefilter.php

 					if ($active->params['login_redirect_url'])
 					{
+						// The login menu item form contains an internal URL redirection.
+						// This will override the automatic change to the user preferred site language.
 						$this->app->setUserState('users.login.form.return', JRoute::_($this->app->getUserState('users.login.form.return'), false));


You are receiving the URL from the app, then send it through JRoute and set it again in the app. That feels pretty wrong... Looks like we'd have to do nothing here...

the case exists and should be checked before checking for an itemid. This code is working. I am open to any suggestions to simplify this.
Would simply use

$this->app->setUserState('users.login.form.return', $this->app->getUserState('users.login.form.return'));

be OK, as we don't need JRoute anymore?

Again, you are doing nothing in this place. You might as well write $something = $something;
That line is the equivalent of JRequest::setVar('test', JRequest::getVar('test')) If that should do anything, then this code is wrong. Otherwise this if-statement can simply be empty.

Hackwar · 2016-11-18T10:50:41Z

plugins/system/languagefilter/languagefilter.php

 						$this->app->setUserState('users.login.form.return', JRoute::_($this->app->getUserState('users.login.form.return'), false));
 					}
-					elseif ($this->app->getUserState('users.login.form.return'))
+					elseif ($itemid)


use $uri->hasVar('Itemid') instead. That way you don't have to unnecessarily assign the variable above.

That can be done indeed. Let's first agree on the above.

infograf768 · 2016-11-19T08:35:43Z

Thanks Hannes. Corrected.

Now, when testing this, I found out an issue which is also present in 3.6.4 and was not taken care of.
If a redirection is set in a login menu item (URL or Menu Item), it is that one which is used when we have a "Register to read more ..." instead of redirecting to the full article.

The problem comes from the fact that in both cases, we have the same $active menu item.

Any idea how to solve that one?

infograf768 · 2016-11-20T10:06:51Z

Ok. Tested on a monolanguage site and that issue is also present there...
Therefore we can now test this and merge if all OK.

Will create an issue for that problem.

infograf768 · 2016-11-20T10:24:39Z

See #12945 for the Read More issue.

andrepereiradasilva · 2016-11-20T12:26:09Z

If login from a login menu item or a login module and a Menu Item redirection is set
// The login form contains a menu item redirection. Try to get associations from that menu item.
// If any association set to the user preferred site language, redirect to that page.

If not, should redirect to the menu item selected?
It's redirecting the user prefered home page.

What is the order of execution priorities here? This?

User languagee default page
Menu item/login module Menu item Login Redirect
Site language default page

Also shouldn't exist a user default login and logout redirect parameters in com_users that would be the "use global" for both the menu item and the login modules? I'm saying this because we have other login form with /component/users/ that doesn't use any of this ...

infograf768 · 2016-11-21T06:42:38Z

First, one should consider the default preferred language for the user. It may be also be "default", i.e. the site default language.
Then one should understand that these login redirections are only available when "Automatic Change" is set to Yes. Otherwise the behavior is similar to a monolanguage site.

Login through the module:
Depends first if the active menu item is tagged or not to the user preferred site language (Home or not).

If it is and no redirection is set in the login module, then behavior is similar to a monolanguage site and login redirects to the same page.

If it is not and no redirection is set in the login module, then it looks for an associated menu item in the user preferred language and login redirects to it.

If there is no associated menu item in the site preferred language of the user and login is not done from the Home page, login redirects to the same page.
(Active Home pages menu items are always considered as associated).

If a redirection is set in the login module, then it tries to find an associated menu item in the site preferred language of the user and redirects to it.
If the redirection is already to a menu item tagged to the site preferred language of the user, it simply redirects to it.
If the redirection is to a menu item different than the site preferred language of the user and there is no associated menu item, the user is redirected to the redirection set in the module.

Login through a Login Menu Item
Depends first if the active login menu item is tagged or not to the user preferred site language.

If no redirection is set in the login menu item and it is not associated to a menu item tagged to the site preferred language of the user, the user will be redirected to the user Profile in the language where the login menu item is presented, whatever its preferred site language.

If no redirection but there is an associated menu item tagged to the site preferred language of the user, the redirection will be to that menu item.
If the associated menu item to which the user is redirected is also a login menu item, then it is not the profile which is presented but a Logout button.

If there is a redirection of type Menu Item, behavior is similar to logging from the module when the module has a redirection (see above).

If the redirection set in the login menu item is an Internal URL one. In that case, it is always that redirect which is used, whatever the language.

Also shouldn't exist a user default login and logout redirect parameters in com_users that would be the "use global" for both the menu item and the login modules? I'm saying this because we have other login form with /component/users/ that doesn't use any of this ...

That is totally unrelated to this PR and would be a new feature.

alikon · 2016-11-24T07:29:09Z

I have tested this item ✅ successfully on ab0f2d4

works as described

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/12932.}

dgrammatiko · 2016-11-24T17:36:30Z

I have tested this item ✅ successfully on ab0f2d4

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/12932.}

dgrammatiko · 2016-11-24T17:37:21Z

RTC

Correcting multilingual login redirect

44fb268

joomla-cms-bot added the PR-staging label Nov 18, 2016

infograf768 mentioned this pull request Nov 18, 2016

Regression: Multilanguage user login redirect #12920

Closed

cs

d306211

Hackwar reviewed Nov 18, 2016

View reviewed changes

Bakual approved these changes Nov 18, 2016

View reviewed changes

correcting useless code.

369c74f

infograf768 added 2 commits November 24, 2016 08:15

Simplifying. Thanks @Bakual

ccc0bfc

cs

ab0f2d4

joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Nov 24, 2016

brianteeman added this to the Joomla 3.7.0 milestone Nov 24, 2016

Code style on comments

bfb018d

wilsonge merged commit 58145de into joomla:staging Nov 28, 2016

joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Nov 28, 2016

infograf768 deleted the multilingual_login_redirect branch November 28, 2016 12:16

infograf768 mentioned this pull request Dec 1, 2016

Register to Read More redirects to the Login Menu item redirect if set #12945

Closed

infograf768 added a commit to infograf768/joomla-cms that referenced this pull request Dec 22, 2016

Correcting bug introduced in joomla#12932

d0010c0

infograf768 mentioned this pull request Dec 22, 2016

Multilanguage: Correcting login form redirect bug introduced in #12932 #13326

Merged

zero-24 pushed a commit that referenced this pull request Dec 22, 2016

Correcting bug introduced in #12932 (#13326)

044b0bb

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Multilingual: Correcting login redirect #12932

Multilingual: Correcting login redirect #12932

infograf768 commented Nov 18, 2016 •

edited by joomla-cms-bot

Hackwar Nov 18, 2016

infograf768 Nov 18, 2016 •

edited

Hackwar Nov 18, 2016

infograf768 Nov 18, 2016 •

edited

Hackwar Nov 18, 2016

Hackwar Nov 18, 2016

infograf768 Nov 18, 2016 •

edited

Hackwar Nov 18, 2016

Hackwar Nov 18, 2016

infograf768 Nov 18, 2016

infograf768 commented Nov 19, 2016

infograf768 commented Nov 20, 2016

infograf768 commented Nov 20, 2016

andrepereiradasilva commented Nov 20, 2016

infograf768 commented Nov 21, 2016

alikon commented Nov 24, 2016

dgrammatiko commented Nov 24, 2016

dgrammatiko commented Nov 24, 2016

Multilingual: Correcting login redirect #12932

Multilingual: Correcting login redirect #12932

Conversation

infograf768 commented Nov 18, 2016 • edited by joomla-cms-bot

Summary of Changes

Testing Instructions

Documentation Changes Required

Hackwar Nov 18, 2016

Choose a reason for hiding this comment

infograf768 Nov 18, 2016 • edited

Choose a reason for hiding this comment

Hackwar Nov 18, 2016

Choose a reason for hiding this comment

infograf768 Nov 18, 2016 • edited

Choose a reason for hiding this comment

Hackwar Nov 18, 2016

Choose a reason for hiding this comment

Hackwar Nov 18, 2016

Choose a reason for hiding this comment

infograf768 Nov 18, 2016 • edited

Choose a reason for hiding this comment

Hackwar Nov 18, 2016

Choose a reason for hiding this comment

Hackwar Nov 18, 2016

Choose a reason for hiding this comment

infograf768 Nov 18, 2016

Choose a reason for hiding this comment

infograf768 commented Nov 19, 2016

infograf768 commented Nov 20, 2016

infograf768 commented Nov 20, 2016

andrepereiradasilva commented Nov 20, 2016

infograf768 commented Nov 21, 2016

alikon commented Nov 24, 2016

dgrammatiko commented Nov 24, 2016

dgrammatiko commented Nov 24, 2016

infograf768 commented Nov 18, 2016 •

edited by joomla-cms-bot

infograf768 Nov 18, 2016 •

edited

infograf768 Nov 18, 2016 •

edited

infograf768 Nov 18, 2016 •

edited