Skip to content

Tags router: do not force incorrect Itemid#40448

Merged
sdwjoomla merged 2 commits intojoomla:4.3-devfrom
Fedik:fix-40442
Apr 21, 2023
Merged

Tags router: do not force incorrect Itemid#40448
sdwjoomla merged 2 commits intojoomla:4.3-devfrom
Fedik:fix-40442

Conversation

@Fedik
Copy link
Copy Markdown
Member

@Fedik Fedik commented Apr 21, 2023

Pull Request for Issue #40442 .

Summary of Changes

Do not force Itemid when menu not found

Testing Instructions

Please follow #40442

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

@Hackwar
Copy link
Copy Markdown
Member

Hackwar commented Apr 21, 2023

While I agree with this change, this is differing from the behavior all other core components have.

@brianteeman
Copy link
Copy Markdown
Contributor

While I agree with this change, this is differing from the behavior all other core components have.

Perhaps but then again com_tags is not the same as any other component

@Fedik
Copy link
Copy Markdown
Member Author

Fedik commented Apr 21, 2023

this is differing from the behavior all other core components have

Maybe have to unset Itemid somewhere? but I cannot find how other component doing it

I found that other components doing it

// If not found, return language specific home link
$default = $this->router->menu->getDefault($language);
if (!empty($default->id)) {
$query['Itemid'] = $default->id;
}

But cannot find how it processed further

@Hackwar
Copy link
Copy Markdown
Member

Hackwar commented Apr 21, 2023

The other components are doing it the way it is now. That is why I'm saying this. And com_tags is crap, but it is otherwise the same as all other core components.

@MacJoom
Copy link
Copy Markdown
Contributor

MacJoom commented Apr 21, 2023

@Fedik
Copy link
Copy Markdown
Member Author

Fedik commented Apr 21, 2023

@Hackwar What do you think if I just do unset($query['Itemid']) in build(), that will be better or? (when option !== com_tags)

@formfranska
Copy link
Copy Markdown

I have tested this item ✅ successfully on e8e8af8

Thank you for this perfect patch 👍🏽😊🙏🏽


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

@richard67
Copy link
Copy Markdown
Member

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Apr 21, 2023
@tramber91
Copy link
Copy Markdown

Hi

I have similar issue. Wrong tab links after migration
I have also open a post in the Joomla forum without answer.

To follow my issue
go to each page links and click on tag France

Site Joomla 3.2.9 (PHP 8) - Site test (come from a backup)
(https://biomecaniquej4.en-toutes-lettres.fr/fr/carte/cartographie-labo-formation/81-laboratoires-de-biomecanique/382-auctus-inria-ims "J429 382-auctus-inria-ims")

the tag link alias is
/fr/tags-pays/france
and it is OK, I can navigate with all tags. I have created 3 dedicated menu links for each Main tags.

I take Same Site after migraion to Joomla 4.3.0 (Live Site Migration Done yesterday, PHP 8.1)
(https://www.biomecanique.org/fr/carte/cartographie-labo-formation/81-laboratoires-de-biomecanique/382-auctus-inria-ims "J430 382-auctus-inria-ims")
the tag link alias is now
/fr/component/tags/tag/france?Itemid=106
Not OK, go to Home page, menu Id=106

Same Site (Test) migrate to Joomla 4.3.0 but with your Batch (PHP 8 site Test)
I modify file Router.php folowing Github info.
(https://biomecanique.en-toutes-lettres.fr/fr/carte/cartographie-labo-formation/81-laboratoires-de-biomecanique/382-auctus-inria-ims "J430 Batch 382-auctus-inria-ims")

the tag link alias is now
/fr/component/tags/tag/france
Not OK, go to Home page too.
The itemid are removed by the batch (Thnks) but the link is always wrong

Thanks for your support

Bertrand


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

@tramber91
Copy link
Copy Markdown

/fr/tags-pays/france was be the good link
"tags-pays" alias of the menu tag (all tag list - Tag Parent Pays)
"france" alias of the tag France in Tag Parent Pays


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

@richard67
Copy link
Copy Markdown
Member

@tramber91 Are you sure that you have applied the changes from this patch correctly? Have you also removed the code at the top, lines 130 to 138, the complete if block with the comment telling it goes to the home page? See first red block in the diff:

j4 3-pr-40448

@sdwjoomla sdwjoomla added this to the Joomla! 4.3.1 milestone Apr 21, 2023
@sdwjoomla sdwjoomla merged commit c93bdfd into joomla:4.3-dev Apr 21, 2023
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Apr 21, 2023
@tramber91
Copy link
Copy Markdown

Yes, I have removed the block and replace the 2 lines

screen shot 2023-04-21 at 20 58 55


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

@richard67
Copy link
Copy Markdown
Member

Then there is maybe something else wrong on your site.

@tramber91
Copy link
Copy Markdown

Thanks, If you have an idea where to search?
Same Community Joomla website works with 4.2.9 for alias tag links, but no more with 4.3.0. :=(
I don't see any parameter to modified in Joomla. I have no override. My menus and components are OK and update !!
Seems to be a com_tag component issue

@ch2856
Copy link
Copy Markdown

ch2856 commented Apr 21, 2023

I applied the patch.
It dose remove the itemid=104, but if you use pagination in com_tags it brings it's back on the next page - /component/tags/tag/tagname?Itemid=104&start=xx

@Fedik Fedik deleted the fix-40442 branch April 22, 2023 05:06
@MacJoom
Copy link
Copy Markdown
Contributor

MacJoom commented Apr 22, 2023

I have confirmed the problem @tramber91 reported. Thank you for reporting!

@Fedik
Copy link
Copy Markdown
Member Author

Fedik commented Apr 22, 2023

Please test #40455, it redo of this PR with fixing for Pagination

@tramber91 If I not mistaken, your issue related to changes betwen 3 and 4 versions of Joomla.
There something with Multilanguage, the tag list should have menu for each language, or something like that.
I found that it does not pick the tag menu wich set to All on multilang site.

@richard67
Copy link
Copy Markdown
Member

richard67 commented Apr 22, 2023

@Fedik It seems the issue is confirmed and not related to a difference between 3.10 and 4 but between 4.2 and 4.3. Could you check this new issue #40454 ? Thanks in advance.

@MacJoom
Copy link
Copy Markdown
Contributor

MacJoom commented Apr 22, 2023

I could reproduce the issue of the changing links, but not the issue with the broken links (this afternoon i was sure i had a broken link)

@Fedik
Copy link
Copy Markdown
Member Author

Fedik commented Apr 23, 2023

@tramber91 please test changes from another PR #40460

@tramber91
Copy link
Copy Markdown

@Fedik

Did the test but same issue (Not working on my side)

same link (see below after the batch)
/fr/component/tags/tag/france?Itemid=106

should be (like in Joomla version 4.2)
/fr/tags-pays/france

I add/Change new codes in initial Router.php file (4.3.0)

i also did the test with SEF diseable
Without alias
result is
/index.php?option=com_tags&view=tag&id[0]=7:france&Itemid=106&lang=fr
Should be
/index.php?option=com_tags&view=tag&id[0]=7:france&Itemid=2622&lang=fr

the router not found the menu id 2622 (Menu Tags: All tags parent Id Countries (Pays))
https://biomecanique.en-toutes-lettres.fr/fr/tags-pays?parent_id=4
If you start from this link (above), you select Tag "France", you have list of article with tag "France"
You select one article and click on France tag
You move to Home page instead of page with tag="France "we have just seen before.

I did also the test including previous batch modification. Same think but without the ?Itemid=106

@richard67
Copy link
Copy Markdown
Member

@tramber91 Could you add your last comment to the pull request #40460 instead of this closed issue? Thanks in advance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.