[4.3] Fixing Itemid lookup in tags router in multilang by Hackwar · Pull Request #40474 · joomla/joomla-cms

Hackwar · 2023-04-24T18:42:28Z

Pull Request for Issue #40454.

Summary of Changes

When running in multilanguage mode, the router falsely only checked for the current language or the default language, not for all (*) language when looking up the right Itemid. This executes the logic twice, once for the current language, once for the * language. It exits for the first matched menu item.

Testing Instructions

Install Joomla
Install at least one additional language
Install multilanguage sample data
Install testing sample data
Go to for example the popular tags module. You can find it in the "Popular tags" menu item.
Check the URLs

Actual result BEFORE applying this Pull Request

URLs start with /component/tags/...

Expected result AFTER applying this Pull Request

URLs have different menu items. None start with /component/tags

Link to documentations

Please select:

Documentation link for docs.joomla.org:
No documentation changes for docs.joomla.org needed
Pull Request link for manual.joomla.org:
No documentation changes for manual.joomla.org needed

richard67 · 2023-04-24T20:52:42Z

Does this PR conflict somehow with PR #40455 ? Or would they work together?

Hackwar · 2023-04-24T20:54:59Z

This PR does not conflict with #40455

sdwjoomla · 2023-04-24T23:41:35Z

I have tested this item ✅ successfully on 73cc2ca

Test site with 3 languages installed (sample data) and pagination for one of the tags

With published menu item /component/ not present
Without published menu item /component/ not present

in either case no itemid

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

sdwjoomla · 2023-04-25T00:14:04Z

I have tested this item ✅ successfully on 18d024d

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

MacJoom · 2023-04-25T06:42:47Z

I have tested this item 🔴 unsuccessfully on 18d024d

The links on the Homepage below in Popular Tags (after index.php)
Joomla 4.2.9 before Tagged-content Menu Language en-GB
/en/component/tags/tag/millions-en-gb /en/component/tags/tag/worldwide-en-gb /en/component/tags/tag/love-en-gb /en/component/tags/tag/joomla-4-en-gb

with Tagged content Language Menu en-GB with Tags 'Millions' 'Love', tagged-content within 'Millions' and 'Love' (tagged in Menu)
/en/tagged-content/millions-en-gb /en/component/tags/tag/worldwide-en-gb /en/tagged-content/love-en-gb /en/component/tags/tag/joomla-4-en-gb

after Joomla 4.3.0 - component and itemid
/en/component/tags/tag/millions-en-gb?Itemid=104 /en/component/tags/tag/worldwide-en-gb?Itemid=104 /en/component/tags/tag/love-en-gb?Itemid=104 /en/component/tags/tag/joomla-4-en-gb?Itemid=104

after applying Patch 40474 - itemid gone but still with component
/en/component/tags/tag/millions-en-gb /en/component/tags/tag/worldwide-en-gb /en/component/tags/tag/love-en-gb /en/component/tags/tag/joomla-4-en-gb

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

richard67 · 2023-04-25T12:45:34Z

@Hackwar Shouldn't the $language be changed to $lang in lines 131 and 138 as those come below the for loop?

joomla-cms/components/com_tags/src/Service/Router.php

Lines 131 to 138 in 18d024d

    
               && ($language === '*' || \in_array($active->language, ['*', $language]) || !Multilanguage::isEnabled())) 
        
           ) { 
        
               $query['Itemid'] = $active->id; 
        
           } 
        
           // If not found, return language specific home link 
        
           if (!isset($query['Itemid'])) { 
        
               $default = $this->menu->getDefault($language);

Hackwar · 2023-04-25T12:47:08Z

Indeed, that needs to be changed.

Hackwar · 2023-04-25T12:48:48Z

done

obuisard · 2023-04-25T14:15:05Z

Martin @MacJoom can you double-check if that fixes it?

obuisard · 2023-04-25T14:16:39Z

Will this PR render #40460 obsolete?

richard67 · 2023-04-25T14:20:14Z

Will this PR render #40460 obsolete?

@obuisard Yes, this here is a replacement for that one. But I think this here is maybe not ready yet. It still not worked for me, but I am not sure if all was right with my test. The other one did not fully work either.

Fedik · 2023-04-25T14:43:18Z

Will this PR render #40460 obsolete?

They both produce identical result, however I think Hannes know Joomla Routing better than me, so I have closed mine.

obuisard · 2023-04-25T14:46:22Z

Will this PR render #40460 obsolete?

They both produce identical result, however I think Hannes know Joomla Routing better than me, so I have closed mine.

Fedir @Fedik, thank you so much for looking into it and providing a solution, this is really appreciated.

MacJoom · 2023-04-25T15:02:24Z

There is unfortunatly exactly the same result as before - being on f2240e5 - i can switch to the 4.2.9 branch with the same db and i have the original result.

MacJoom · 2023-04-25T15:09:43Z

Btw. The testing instructions above are missing the step to setup a menu item with the tagged items in en-gb between steps 5 and 6 (on my test site the menu item has the alias 'tagged-content' as the links for 4.2.9 show) and if this menu item is set to language 'all' the links will stay on 'component' - this may be a clue.

joomdonation · 2023-04-25T15:36:11Z

To be more clear, the issue here is that if you create a menu item to link to Tagged Items menu item type and select two tags A and B :

In Joomla 4.2 and earlier, link to tag A and tag B will use the above menu item as active menu item
In Joomla 4.3, link to tag A and tag B does not use the above menu item as active menu item although the menu item contains these two tags.

There was changed to the logic of code to find active menu item for a tag for some reasons. Unless the tagged items menu item type only select a single tag, it won't be used for link to a tag like before.

obuisard · 2023-04-25T16:42:51Z

Doing testing and getting same results as Martin @MacJoom.

Hackwar · 2023-04-25T20:33:06Z

Indeed, the lookup was changed, since the old code was unable to find menu items with more than one tag assigned to them. So if you created a URL with 2 tags, it couldn't find a menu item at all. Fixing that now results in menu items with multiple tags not matching to URLs with a single tag. I personally think that tags are a filter on the content and those filters compound, so a URL with two tags should only display the content which has both tags assigned to it, but apparently I'm wrong there, since the tagging code doesn't behave that way later on in the model.

I will change the code to match it again.

….3-tag-router

Hackwar · 2023-04-25T20:50:00Z

Done.

obuisard · 2023-04-25T23:23:04Z

I have tested this item ✅ successfully on 6e3e4c3

The last change seems to do it for me:

Tags that are associated to menu items are properly opening in that menu item (use of the alias of the menu item),
Tags that are not associated to menu items open in the home page (/component/tags/tag in the path).

Behavior is consistent to 4.2, no longer dimmed as b/c as far as I can tell.

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

Hackwar · 2023-05-08T12:16:46Z

@joeforjoomla Can you please answer @richard67 and me? This is a rather important issue and we are all waiting for you to either substantiate your claim or to merge this PR to be ready in time for 4.3.2. If you don't respond in 3 days, I will suggest to ignore your report.

joeforjoomla · 2023-05-08T16:11:50Z

@Hackwar @richard67 i would expect to have tag links as before:

http://joomla4/en/menualias/tag/tag1.html
http://joomla4/en/menualias/tag/tag2.html

and not for all tags:

http://joomla4/en/menualias.html

Why this? How to have a tag link then? What is the sense of this router?

Hackwar · 2023-05-09T09:23:16Z

I reviewed the whole code of tags again and you are right @joeforjoomla. I'm going to see what I can do here.

….3-tag-router

richard67 · 2023-05-09T09:49:28Z

Back to pending in the issue tracker.

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

Hackwar · 2023-05-11T17:05:46Z

@joeforjoomla Please check if this change fixes your issues.

joeforjoomla · 2023-05-11T22:09:41Z

@joeforjoomla Please check if this change fixes your issues.

@Hackwar Just perfect! :)

joeforjoomla · 2023-05-11T22:10:36Z

I have tested this item ✅ successfully on d5a6863

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

richard67 · 2023-05-13T09:45:37Z

I have tested this item ✅ successfully on d5a6863

URLs now after the last change are like they were in 4.2.9, i.e. everything as expected now.

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

richard67 · 2023-05-13T09:46:13Z

RTC

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

obuisard · 2023-05-13T14:16:43Z

Thank you Hannes @Hackwar and all testers, your help is greatly appreciated. Great team work.

Fixing Itemid lookup in tags router in multilang

73cc2ca

joomla-cms-bot added the PR-4.3-dev label Apr 24, 2023

Hackwar mentioned this pull request Apr 24, 2023

Tags router: fix Lookup when multilang is enabled #40460

Closed

4 tasks

richard67 added Release Blocker bug labels Apr 24, 2023

Merge branch '4.3-dev' into 4.3-tag-router

18d024d

Changing $language to $lang

2c6d550

Merge branch '4.3-dev' into 4.3-tag-router

f2240e5

Hackwar added 2 commits April 25, 2023 22:49

Match both on combined and single tag

0a42d27

Merge branch '4.3-tag-router' of github.com:Hackwar/joomla-cms into 4…

6e3e4c3

….3-tag-router

obuisard added this to the Joomla! 4.3.2 milestone May 8, 2023

Hackwar removed the RTC This Pull Request is Ready To Commit label May 9, 2023

Merge branch '4.3-dev' of https://github.com/joomla/joomla-cms into 4…

90ff9e7

….3-tag-router

joomla-cms-bot removed this from the Joomla! 4.3.2 milestone May 9, 2023

richard67 added this to the Joomla! 4.3.2 milestone May 9, 2023

Fedik mentioned this pull request May 10, 2023

[4.3] Rewriting com_tags router #39114

Merged

4 tasks

obuisard added the Updates Requested Indicates that this pull request needs an update from the author and should not be tested. label May 11, 2023

Hackwar added 2 commits May 11, 2023 18:52

Fix tag routing

8975c71

Codestyle

d5a6863

richard67 removed the Updates Requested Indicates that this pull request needs an update from the author and should not be tested. label May 11, 2023

richard67 added the Updates Requested Indicates that this pull request needs an update from the author and should not be tested. label May 13, 2023

joomla-cms-bot removed this from the Joomla! 4.3.2 milestone May 13, 2023

joomla-cms-bot added the RTC This Pull Request is Ready To Commit label May 13, 2023

richard67 removed the Updates Requested Indicates that this pull request needs an update from the author and should not be tested. label May 13, 2023

Merge branch '4.3-dev' into 4.3-tag-router

4a9afec

richard67 added this to the Joomla! 4.3.2 milestone May 13, 2023

obuisard merged commit 09a03cf into joomla:4.3-dev May 13, 2023

joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label May 13, 2023

richard67 removed the Release Blocker label May 14, 2023

Hackwar deleted the 4.3-tag-router branch March 22, 2024 10:44

Uh oh!

Conversation

Hackwar commented Apr 24, 2023

Summary of Changes

Testing Instructions

Actual result BEFORE applying this Pull Request

Expected result AFTER applying this Pull Request

Link to documentations

Uh oh!

richard67 commented Apr 24, 2023

Uh oh!

Hackwar commented Apr 24, 2023

Uh oh!

sdwjoomla commented Apr 24, 2023

Uh oh!

sdwjoomla commented Apr 25, 2023

Uh oh!

MacJoom commented Apr 25, 2023

Uh oh!

richard67 commented Apr 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Hackwar commented Apr 25, 2023

Uh oh!

Hackwar commented Apr 25, 2023

Uh oh!

obuisard commented Apr 25, 2023

Uh oh!

obuisard commented Apr 25, 2023

Uh oh!

richard67 commented Apr 25, 2023

Uh oh!

Fedik commented Apr 25, 2023

Uh oh!

obuisard commented Apr 25, 2023

Uh oh!

MacJoom commented Apr 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MacJoom commented Apr 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joomdonation commented Apr 25, 2023

Uh oh!

obuisard commented Apr 25, 2023

Uh oh!

Hackwar commented Apr 25, 2023

Uh oh!

Hackwar commented Apr 25, 2023

Uh oh!

obuisard commented Apr 25, 2023

Uh oh!

Hackwar commented May 8, 2023

Uh oh!

joeforjoomla commented May 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Hackwar commented May 9, 2023

Uh oh!

richard67 commented May 9, 2023

Uh oh!

Hackwar commented May 11, 2023

Uh oh!

joeforjoomla commented May 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joeforjoomla commented May 11, 2023

Uh oh!

richard67 commented May 13, 2023

Uh oh!

richard67 commented May 13, 2023

Uh oh!

obuisard commented May 13, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

richard67 commented Apr 25, 2023 •

edited

Loading

MacJoom commented Apr 25, 2023 •

edited

Loading

MacJoom commented Apr 25, 2023 •

edited

Loading

joeforjoomla commented May 8, 2023 •

edited

Loading

joeforjoomla commented May 11, 2023 •

edited

Loading