Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multilang: correcting Published Request Form Menu Item url in Privacy Dashboard #22909

Merged
merged 4 commits into from
Jan 6, 2019

Conversation

infograf768
Copy link
Member

@infograf768 infograf768 commented Nov 2, 2018

Pull Request for Issue #22869 (comment)

Summary of Changes

Correct the link (language included) and the Published/Unpublished status of the Published Request Form Menu Item url, making it totally independent of the site default language when we do have such menu items.
Also when we have no menu item, correcting the Itemid to use the default site language home page one instead of the one set to ALL languages.

Testing Instructions

See #22869 (comment)
Test and test2.
SEF off to see the non-sef url.

After patch

When we have one or multiple menu items:
The url obtained will anyway be the one of the lowest id in case there are multiple menu items of this kind (no change there as we have only one line for an url in the dashboard), but at least:
The Itemid and the language will be the correct ones in case there are multiple menu items, this independantly from the site default language.

Example when there is a fr-FR such menu item

screen shot 2018-11-02 at 08 16 39

Also, when there is NO menu item, the Itemid used will not be anymore the Itemid of the default home page set to ALL languages, but the one of the default home page for the default site language. That is important concerning modules assignment.

Example when French is default language and the French home page Itemid is 105 while the home set to ALL is 101.
Before patch
screen shot 2018-11-02 at 09 54 04

After patch
screen shot 2018-11-02 at 09 52 38

@mbabker

@@ -92,28 +87,67 @@ public function getRequestFormPublished()

$db = $this->getDbo();
$query = $db->getQuery(true)
->select($db->quoteName('id'))
->select($db->quoteName('id') . ', ' . $db->quoteName('published') . ', ' . $db->quoteName('language'))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
->select($db->quoteName('id') . ', ' . $db->quoteName('published') . ', ' . $db->quoteName('language'))
->select($db->quoteName(array('id', 'published', 'language')))

@jsubri
Copy link
Contributor

jsubri commented Nov 3, 2018

I have tested this item ✅ successfully on 7d2fb99

With the patch the Itemid is now the id of the default site language instead of *
With SEF off: index.php?option=com_privacy&view=request&Itemid=269&lang=fr
With SEF on: index.php/fr/mon-acces/requete-de-confidentialite
If the menu is unpublished,we have the home menu in the site language /index.php?option=com_privacy&view=request&lang=fr&Itemid=101
I tested on mon-lingual, no regression


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/22909.

@viocassel
Copy link
Contributor

I have tested this item ✅ successfully on 7d2fb99


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/22909.

@infograf768
Copy link
Member Author

RTC. Thanks for testing.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/22909.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jan 4, 2019
@infograf768
Copy link
Member Author

No need to ask for new tests. Works fine here.
@mbabker

@mbabker mbabker merged commit 57ca7a4 into joomla:staging Jan 6, 2019
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jan 6, 2019
@infograf768 infograf768 deleted the privacydashboardmultilanglink branch January 6, 2019 16:28
@zero-24 zero-24 removed the PR-3.9-dev label Jan 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants