? Release Blocker PR-4.3-dev Pending

User tests: Successful: Unsuccessful:

avatar Hackwar
Hackwar
31 Oct 2022

Pull Request for Issues #39105, #38996, #35382, #13598, #37444, #37492.

Summary of Changes

This PR rewrites the router of com_tags. Above you can see a whole list of issues which this should fix. Since these are very different issues, please don't close the issues right away without first testing this if it really fixes the issue.

This router improves a lot of things, but it also still is not perfect. While this better matches existing menu items, removes unnecessary URL parameters, allows for multiple tags per URL and fixes bugs in pagination and a few other areas, this still does not properly support language and multilanguage setups. It also fails to properly support nested tags.

The routes of the new router should be pretty much identical to the existing ones, except for where the current URLs aren't actually working. Right now the router can generate and parse URLs with several tags in one URL, where the IDs of the tags are handed over as an array in the non-SEF URL and a SEFed URL will have the aliases of those tags concatenated as segments of the URL.

Stuff not solved by this PR

The multilanguage issues are serious and I would love to solve them, but those fixes would build upon these changes here and these changes then again are already complex enough to test, so I'd like to first merge this here and then fix the language issues in another PR, especially since that PR would require a lot of changes to the models of the component.

The nested tags issue in the end means a new feature. Right now you can set a tag to be a child of another tag. However, URLs to a tag only contain the alias, not the complete path of the tag. This would be problematic when 2 child tags of different parents have the same alias. There is also no visual representation of this structure. My proposal would be to introduce an option to switch between the ability to select more than one tag per URL by using /tags/[tag1]/[tag2]/... or selecting a single, nested tag by using /tags/[tag1-level1]/[tag1-level2]/...

Another new feature could be to add a plus sign to tags in order to further filter a list of items down by adding yet another tag to that filter. Right now several tags in a menu item don't mean that only items with all tags associated to these items are shown, but all items which have at least one of those tags are displayed.

Testing Instructions

Pagination issue (fixes #35382 & #39105)

  1. Install testing sampledata
  2. Unpublish all tag related menu items (search for "tag" in the "all menu items" backend view)
  3. Create a tag which you then assign to at least 21 articles
  4. In the frontend, lookup one of the articles and click on the new tag
  5. In the single tag view for that tag, click on the second page.
  6. Without this change, this should result in an error. With this change, this should be fine.

Additional URL parameter (fixes #38996)

Please go to #38996 and follow the instructions there to rebuild the issue. After applying this change, this should be fixed.

Use the right menu item for tags (fixes #13598)

  1. Install testing sampledata
  2. Create two tags and create a menu item for one of the two with a single tag view
  3. Assign the new tags to an article
  4. Due to the "All Tags" menu item, after applying the change, the tag with the menu item should point to that menu item and the other should point to the all-tags menu item

Numeric tags don't work (fixes #37444)

  1. Install testing sampledata
  2. Create a tag with a numeric title
  3. Assign it to an article
  4. Try to click on that tag in the article in the frontend. Without this change, you don't get a valid URL, with this change, the right page is displayed.

Additional URL parameter v2 (#37492)

I can't reproduce this with this change applied. Can someone check if this is valid?

Improvements with these changes

  1. You can now have URLs with multiple tags. Such a non-SEFed URL would look like this: index.php?option=com_tags&view=tag&id[0]=1:alias-1&id[1]=2:alias-2...
  2. SEFed URLs with multiple tags would look like this: /[menu-item]/[tag-1]/[tag-2]/...

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

I don't know right now if we need documentation changes...

avatar Hackwar Hackwar - open - 31 Oct 2022
avatar Hackwar Hackwar - change - 31 Oct 2022
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 31 Oct 2022
Category com_tags Front End
avatar Hackwar Hackwar - change - 31 Oct 2022
Labels Added: PR-4.3-dev
avatar Hackwar Hackwar - change - 31 Oct 2022
The description was changed
avatar Hackwar Hackwar - edited - 31 Oct 2022
avatar Hackwar Hackwar - change - 31 Oct 2022
The description was changed
avatar Hackwar Hackwar - edited - 31 Oct 2022
avatar Hackwar Hackwar - change - 2 Nov 2022
The description was changed
avatar Hackwar Hackwar - edited - 2 Nov 2022
avatar Hackwar Hackwar - change - 2 Nov 2022
The description was changed
avatar Hackwar Hackwar - edited - 2 Nov 2022
avatar richard67
richard67 - comment - 2 Nov 2022

Setting the release blocker label as inherited from issue #39105 .

avatar jamfx
jamfx - comment - 3 Nov 2022

I have tested this item successfully on 8f23f4f

Followed the detailed instructions. Works as described. No errors found.


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

avatar jamfx jamfx - test_item - 3 Nov 2022 - Tested successfully
avatar Hackwar Hackwar - change - 6 Nov 2022
Labels Added: Release Blocker
avatar chmst
chmst - comment - 15 Nov 2022

Test on a fresh installation (4.3 dev) with testing sample data and additional own data, not multilingual on localhost (sef urls on but no rewrite rule).

Could not replicate before applying the patch (successful per se)
Pagination issue (fixes #35382 & #39105)
Numeric tags don't work (fixes #37444)

Could not replicate: Additional URL parameter v2 (#37492)

Successful
Additional URL parameter (fixes #38996)
Use the right menu item for tags (fixes #13598)

Looks good for me but recommend testing with multilingual sites. No documentation change needed.


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

avatar chmst chmst - test_item - 15 Nov 2022 - Tested successfully
avatar chmst
chmst - comment - 15 Nov 2022

I have tested this item successfully on a3cd27e


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

avatar richard67 richard67 - alter_testresult - 15 Nov 2022 - jamfx: Tested successfully
avatar richard67
richard67 - comment - 15 Nov 2022

I've restored @jamfx 's test result since all changes after it were not really functional.

avatar richard67 richard67 - change - 15 Nov 2022
Status Pending Ready to Commit
avatar richard67
richard67 - comment - 15 Nov 2022

RTC


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

avatar obuisard obuisard - change - 16 Nov 2022
Labels Added: ?
avatar obuisard obuisard - change - 16 Nov 2022
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2022-11-16 01:03:18
Closed_By obuisard
avatar obuisard obuisard - close - 16 Nov 2022
avatar obuisard obuisard - merge - 16 Nov 2022
avatar obuisard
obuisard - comment - 16 Nov 2022

Thank you Hannes @Hackwar for this much needed PR!

avatar ch2856
ch2856 - comment - 20 Apr 2023

J4.3.0
The new router adds ?Itemid=104 to all general tags (no menu)


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

avatar joeforjoomla
joeforjoomla - comment - 20 Apr 2023

Confirmed. The new router has broken routing adding ?Itemid=xxx to all links despite the menu item created.

avatar NicolasDerumigny
NicolasDerumigny - comment - 10 May 2023

I am unsure whether I should open an issue for it or just a comment, but when:

  • SEF Url is enable
  • a menu entry exists for "list all tags (with root as parent)", setting for example the alias tags for index.php?option=com_tags&view=tags&parent_id=1

Then tag URL are not rewritten from mysite.com/component/tags/tag/name_of_tag to mysite.com/tags/name_of_tag, though the latter is also valid URL and behave as expected.

Is this the correct behavior?

avatar Fedik
Fedik - comment - 10 May 2023

We have a PR that for fixing broken behavior of Tag router #40474. Please test, and report your result there. Thanks.

Add a Comment

Login with GitHub to post a comment