Skip to content

[5.2] Tag Router: Allow numeric/CSV IDs (Regression)#44784

Merged
pe7er merged 5 commits intojoomla:5.2-devfrom
Hackwar:5.2-tags-routing-regression
Feb 3, 2025
Merged

[5.2] Tag Router: Allow numeric/CSV IDs (Regression)#44784
pe7er merged 5 commits intojoomla:5.2-devfrom
Hackwar:5.2-tags-routing-regression

Conversation

@Hackwar
Copy link
Member

@Hackwar Hackwar commented Jan 26, 2025

Pull Request for Issue #44703.

Summary of Changes

With Joomla 5.2.3 and PR #44540 the tag router was changed to rather throw a 404 when the URL was falsely extended by random segments. This unfortunately introduced a regression, where numeric segments and comma-separated lists of numeric IDs were treated as 404s. While by default all tag URLs should only produce alias-based URLs, we apparently have numeric URLs out there, which are now falsely are marked as broken. This PR checks for numeric IDs and for comma-separated IDs in the URL.

Testing Instructions

  1. Your site needs at least 2 tags (A and B) and content items which have once A, once B and once A and B assigned to them. You should also have a menu item which points to com_tags. The type is pretty much irrelevant. For ease of description A has ID 1 and B has ID 2. The menu item has the alias tagmenu.
  2. Open /component/tags/tag/a
  3. Open /component/tags/tag/b
  4. Open /component/tags/tag/a/b
  5. Open /component/tags/tag/1
  6. Open /component/tags/tag/2
  7. Open /component/tags/tag/1,2
  8. Open /component/tags/tag/1/2
  9. Open /tagmenu/a
  10. Open /tagmenu/b
  11. Open /tagmenu/1
  12. Open /tagmenu/2
  13. Open /tagmenu/1,2
  14. Open /tagmenu/1/2
  15. Open /component/tags/tag/a/2
  16. Open /component/tags/tag/a/1,2
  17. Open /tagmenu/a/2

Actual result BEFORE applying this Pull Request

Cases 2,3,4,9,10 return a page, all the rest throws a 404.

Expected result AFTER applying this Pull Request

Cases 15,16,17 throw a 404, the rest returns a successfull page.

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

@brianteeman
Copy link
Contributor

Should there be system tests for this?

@exlemor
Copy link

exlemor commented Jan 29, 2025

I have tested this item ✅ successfully on a9b93b6

I have successfully tested this. (BUT check notes below)

Not being 100% sure if this matters/is expected, but with the Patch turned on for:

  1. Open /tagmenu/1
  2. Open /tagmenu/2
  3. Open /tagmenu/1,2
  4. Open /tagmenu/1/2

There was no title (h1) in Bold above the Filter/Clear box:

(Enter Part of Title) Filter/Clear box
Joomla
Millions
Typography

like you have for:

  1. Open /component/tags/tag/a
  2. Open /component/tags/tag/b
  3. Open /component/tags/tag/a/b
  4. Open /component/tags/tag/1
  5. Open /component/tags/tag/2
  6. Open /component/tags/tag/1,2
  7. Open /component/tags/tag/1/2

Apple
(Enter Part of Title) Filter/Clear box
Joomla
Millions
Typography

if that is expected than it's perfect.


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

(I added screenshots via Github)

Joomla_PR-44784-screen-shot-2

Joomla_PR-44784-screen-shot-1

@Hackwar
Copy link
Member Author

Hackwar commented Jan 30, 2025

Hello @exlemor, thank you for testing this. Yes, the behavior is expected, since the /component/tags has no menu item attached to it and thus no configuration for the page.

@dautrich
Copy link

I have tested this item ✅ successfully on a9b93b6


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

@Hackwar
Copy link
Member Author

Hackwar commented Jan 31, 2025

Thank you for the tests.

@QuyTon
Copy link
Contributor

QuyTon commented Jan 31, 2025

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jan 31, 2025
@QuyTon QuyTon added this to the Joomla! 5.2.4 milestone Jan 31, 2025
@pe7er pe7er self-assigned this Feb 3, 2025
@pe7er pe7er merged commit da9ed62 into joomla:5.2-dev Feb 3, 2025
3 checks passed
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Feb 3, 2025
@pe7er
Copy link
Contributor

pe7er commented Feb 3, 2025

Thanks @Hackwar !

@Hackwar Hackwar deleted the 5.2-tags-routing-regression branch June 10, 2025 17:40
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.

8 participants