Routing: Remove IDs from tags URLs, use menu item of tags view as default for tag view #11166

csthomas · 2016-07-17T19:35:04Z

Pull Request for Issue #11163.

Summary of changes

Remove tag id from SEF URL, but it is B/C.
Treat (second menu item to view=tags if exists or) first menu item for tags as default menu item for tag view.

If tags list does not have any menu item then
replace
/component/tags/tag/123-mytag
to
~~/component/tags/mytag~~
/component/tags/tag/mytag

If we have menu item with /tags alias for tags view:
then PR replace
/tags/tag/123-mytag
to
/tags/mytag

If we have second menu item with alias="tag"
then:
replace
/tags/tag/123-mytag
to
/tag/mytag

but link to list of tags is:
/tags

Testing Instructions

Create a few tags and content with tags and test front end URLs.

Documentation Changes Required

I do not know but there is one thing that can surprise.

Tag route changes:

First as usual we try to find menu item for a tag.
If menu item for the tag does not exists then:
- try to find second menu item for view=tags, (ex. tag/...)
- if not exists then try to find first menu item for view=tags. (ex. tags/...)

yvesh · 2016-07-20T13:43:16Z

components/com_tags/helpers/route.php

+			else
+			{
+				$needles = array('tags' => array(0));
+				if ($item = self::_findItem($needles))


Code style (Please insert an empty line before the if)

… second Itemid for default tag view

coolcat-creations · 2016-07-23T11:09:12Z

I have following error in the com_patchtester: Could not connect to GitHub: No commit found for the ref tagroute2

coolcat-creations · 2016-07-23T11:17:04Z

I have tested this item 🔴 unsuccessfully on 40b7c93

*In testing environment i have following tag-urls before the patch:
*

Red: index.php/tagged-items
Yellow: index.php/compact-tagged
Green index.php/component/tags/tag/4-green
Lime index.php/component/tags/tag/5-lime

after the patch:

Red: index.php/tagged-items
Yellow: index.php/compact-tagged
Green: index.php/all-tags/green
Lime: index.php/all-tags/lime
Red and Yellow is still wrong
_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11166.}

brianteeman · 2016-07-23T11:25:23Z

@designbengel that looks perfect to me.

On 23 July 2016 at 13:17, designbengel [email protected] wrote:

I have tested this item 🔴 unsuccessfully on 40b7c93
40b7c93

In testing environment i have following tag-urls before the patch: *

Red: index.php/tagged-items

Yellow: index.php/compact-tagged

Green index.php/component/tags/tag/4-green

Lime index.php/component/tags/tag/5-lime

after the patch:

Red: index.php/tagged-items

Yellow: index.php/compact-tagged

Green: index.php/all-tags/green

Lime: index.php/all-tags/lime Red and Yellow is still wrong
This comment was created with the J!Tracker Application
https://github.com/joomla/jissues at
issues.joomla.org/joomla-cms/11166
https://issues.joomla.org/tracker/joomla-cms/11166.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#11166 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABPH8eD5Za2GvniAJdAgTplfk1LLkrH6ks5qYfg0gaJpZM4JOSyf
.

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

csthomas · 2016-07-23T11:48:13Z

Lime: index.php/all-tags/lime Red and Yellow is still wrong
@designbengel What URL do you expected?
Can you write more details about your menu structure?

If you have menu item for "tagged" or "compact" tags then it takes precedence before "all-tags".

You can have some extra menu item like "/my-extra-featured-sponsored-tags" and it can not be replaced by default tags menu, which is menu item to list of all tags.

Of course you can set parent in menu item which may change it to:
/all-tags/my-extra-featured-sponsored-tags but I have not tested it and it is probably unusable.

coolcat-creations · 2016-07-27T19:53:27Z

Ok i thought it should be index.php/all-tags/red for the red and index.php/all-tags/yellow for the yellow. If it´s good so then i mark it as successful.

csthomas · 2016-07-27T20:19:20Z

If you set menu item with alias "red" for tag 'red' and you want URL like /all-tags/red
then in the right side (menu item edit form, details):

change menu if you need and
select parent item (menu item from above menu) to "all-tags"

csthomas · 2016-07-28T07:43:03Z

Ok i thought it should be index.php/all-tags/red for the red and index.php/all-tags/yellow for the yellow. If it´s good so then i mark it as successful.

Yes it is good.

Bakual · 2016-07-28T10:34:25Z

If I read the code right, then this will no longer parse the old URLs correctly. Eg index.php/component/tags/tag/5-lime will result in a 404 because it expects a direct "alias" not "id-alias". Or do I miss something?

csthomas · 2016-07-28T11:09:14Z

Eg index.php/component/tags/tag/5-lime will result in a 404 because it expects a direct "alias" not "id-alias"

It will work correctly.

[UPDATED]

You mentioned about fixSegment method which only add a prefix id if find row.

For index.php/component/tags/tag/5-lime if fixSegment method does not find a row then return $segment not changed, means return '5-lime'.

For index.php/component/tags/tag/lime if fixSegment method finds a row with id=5 then return '5-lime'

Both return the same '5-lime'.

hardiktailored · 2016-07-30T07:41:10Z

I have tested this item 🔴 unsuccessfully on

After applying patch it removes the ID of tags but not replaces tags/tag/ with tags/ from the URL as you said in first place.
It works as you said for menu items. When menu item is present, tags/tag/ replaces with menu alias name.

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

hardiktailored · 2016-07-30T07:43:36Z

See results:

Before applying patch and without menu item

After applying patch without menu item

After applying patch with menu item added

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

Bakual · 2016-07-30T14:25:53Z

For index.php/component/tags/tag/5-lime if fixSegment method does not find a row then return $segment not changed, means return '5-lime'.

Ah true, missed that one.

csthomas · 2016-07-30T17:07:54Z

After applying patch it removes the ID of tags but not replaces tags/tag/ with tags/ from the URL as you said in first place.

You right,

[updated]
you talk about:
/component/tags/tag NOT /tags/tag

my description was incorrect, I wrote to fast,
I concerned on (SEF enabled) links that have some menu item.

JoshJourney · 2016-09-21T10:05:50Z

Just checking in on this. Would like to see this make it into J 3.7 😄

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

zero-24 · 2016-09-21T10:11:00Z

This means we need test here @JoshuaLewis ;)

JoshJourney · 2016-09-21T10:36:12Z

I have tested this item ✅ successfully on 40b7c93

Works quite well. 😄 Great little PR!

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

JoshJourney · 2016-09-22T07:23:30Z

@zero-24 Can you test it as well?

stellainformatica · 2016-10-31T10:27:06Z

Hi, I report a issue with tags menu items in a multilanguage site.
If a tag menu item (i.e. .../index.php/eventi) is set to all language, the link of the tag in the article is right the same.
If the tag menu item is set to a specific language, the link of the tag in tha article (of the same language) is not correct .../index.php/component/tags/tag/eventi

I tested a 3.6.4 with this patch but the issue still remain. Is ti related or better to open a new tracker?

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

csthomas · 2016-10-31T11:41:46Z

It is better to create another issue.

ghost · 2017-01-08T17:25:59Z

@csthomas is this pr still to test?

csthomas · 2017-01-08T17:51:42Z

Yes.

ghost · 2017-01-09T11:09:12Z

I have tested this item ✅ successfully on 40b7c93

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

wilsonge · 2017-01-17T13:38:05Z

Two good tests here. So I'm setting RTC. BUT I want @rdeutz to review this as I don't know if we want to go down this path or move tags across to "new" routing before we try this...

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

rdeutz · 2017-01-17T13:47:09Z

Seems to me a good change

@mbabker

* tinymce 4.5.2 Version 4.5.2 - January 4, 2017 * Clean up JModelForm * xml update version * Catch "expects parameter 2 to be string" error * Remove multiple parameter from user field * Remove default value from the field params to inherit from plugin * It's 2017. Happy New Year * Some improvements in tests #3: (#13402) * Some improvements in tests #3: - call static methods correctly * Fix T_PAAMAYIM_NEKUDOTAYIM (for all PHP 5.x) * Remove forgotten call * Changed a few things after conversation with @mbabker * Fixes according to @andrepereiradasilva's comments * Unnecessary double quotes in /libraries/joomla (#13372) * Replace unnecessary double quotes in /libraries/joomla * Formatting * CS Fix * Fixes, based on @andrepereiradasilva's comments. * Change remove string concatenations for some occurrences. * Fixes, based on @andrepereiradasilva's comments. * Fixes, based on @andrepereiradasilva's comments. * Fixes, based on @andrepereiradasilva's comments. * Fixing search for MySQL (#13571) * Use $query->castAsChar instead of casting to integer * Codestyle * Clean up old code in cache.php file (#12183) * Clean up in cache - part 1 * Remove old php4 style to catch exception which currently is useless. * Add shorten version of ternary pperators. * Add fix for contains() * Add more email cloaking unit tests and fix email cloaking bug (#13446) * Rsponsive article edit fields (#13586) * Fix BS grid (#13560) * Fixing Showon in plugins/modules/templates (#13549) * Adding field group to JFormHelper::parseShowOnConditions and rearranged argument order. * codestyle * $field->assigned_cat_ids may not exist. (#13570) * Clean up ModulesModelModule class (#13380) * Make the calendar work in the subform field (#13153) * [com_fields] Add Joomla loading overlay when form submit is triggered by category selector change (#13320) * Added overlay box and message box for submit and reload form after category change * Removed commented testing code * Added language strings * CS fix, added missing spaces * Used Joomla logo spinner instead of fixed message * Reverted changes in admin lang file * CS * Revert frontend lang changes * Added Joomla loading overlay to new field form when changing field type * Added Joomla.loadingLayer show to typeHasChanged JS method too * PostgreSQL - return the same string each time of call __toString() on update query (#13284) * TranslateFormat in all other forms (#13158) * TranslateFormat in all other forms * Revert for tracks filter bar * Fix for issue #13531 (#13535) * Fix for issue #13531 - [AND] and [OR] operators were not functioning correctly. Fixed. - some cleanup of whitespaces in XML file and removal of rulers when the encapsulated fields are not showing. * Removed $key, as it was an unused leftover * CS Fix * Whitespace Fix * Whitespace Fix * Whitespace Fix#2 * Conflict resolution (hopefully, Don't have a complete dev-environment right now.) * Added css classes to the mod_login submit buttons (#13379) * Added css classes to the mod_login submit buttons * Added css classes to the mod_login submit buttons * Changed class to login-button in both modules * PHPMailer update (#13575) * Correcting sidebar display LTR and RTL (replaces #13548) (#13593) * Fix for Issue #13588 - mod_articles_categories - Fatal error: Class 'ContentHelperRoute' not found... (#13590) * Update helper.php Remove JLoader::register('ContentHelperRoute', JPATH_SITE . '/components/com_content/helpers/route.php'); * Update mod_articles_categories.php * Make clear Exception messages in JTable (#13603) * Fix issue where fields is false (#13574) * Update jQuery Autocomplete to 1.2.27 (#13282) * Show text "No Information Entered" in users profile when no value is set (#13589) * Delete UCM content entries when Joomla articles are deleted (#13592) * [com_fields] Add base list plugin class which activates the list plugin (#13546) * Add base list plugin class * Update fieldslistplugin.php * Fixes #13177: Added where clause with block status (#13545) * Menu manager for Joomla Backend Menu (#13036) * Added client id column to menu_type table. Allow creating and editing of "menutype" records with client_id = 1 Add client_id filters in menu and menu items list views Sync menu type filter and client_id filter allowing only menu type in the URL query parameter (B/C) Both Lists now also filtered by client id. Client id selection updates the menu type list options to show choices only for that client id TBD: Reserved menu types: main & menu * In modal list view we currently hide client_id filter and show only site menu types, will be updated once we have more clear vision. Menu type assignment to backend mod_menu config from both menu manager and module manager. Though that is not functional within the module itself. * Add/edit menu item redirect with clientId from list filter. Load menu item form based on active client id Menu type dropdown choices limited to active client id value Show menu-item-type choices (modal) trigger with client id parameter in the url Switch edit layout based on client id * Menu item type loading from component metadata xml or mvc not identifies backend and frontend application separately. Not yet able to load menu item type from backend so returns empty list. Front-end is still intact and unaffected. * Edit menu item and create menu item set to follow client id and menu type value consistenty. When creating menu item alias, the referenced menu must also belong to same client id. Client id field removed from form, this should be auto-calculated from the menu type when saving. * Adding layout metadata xml in backend to reference menu item types as it was in front-end. Removed unnecessary admin specific layout added earlier as it is so far same as original edit.php, may be added back when needed. Remove page specific meta data fields from backend component type menu items. For now disable/unsupport association for backend menu items. Disallow change of client id for existing menu items, unexpected conflicts may occur if allowed so better be safe. Ref to #2 * Created each backend menu items using menu manager as a replica of existing Joomla backend menu. These are to be used for testing during upgrading menu module. language keys are not yet translated. Translation will be done as we are ready with most new or modified language keys application wide. Backend menu items does not require all those parameters as that with front-end menu items. Therefore segregated entire menu item xml for backend/frontend. Ref #2 * [a11y] Protostar back to top (#12446) * [a11y] Protostar - back to top link * Oops Andre was right * add anchor for non-js enabled browsers * Restructure mod_menu to load preset menu items as an option (default). Other options will be the menu-type and will cause us to load from database. Ref #2 * Disallow editing and set to home of protected menu type menu items viz. 'main' and 'menu' Allow explicit filtering by protected menu type choices in menu items list view. Not limited to #__menutypes table entries only. Unfiltered list still excludes those menu items. (B/C ok) Menu items created during installation of a component are now saved as published. When unpublished we won't load it in customised menu's component menu container. They will still be loaded irrespective of state as previously when preset is in use. (B/C ok) Home page can now be set one per client instead of one overall. Menu module only loads item from 'main' and 'menu' type menu items when requested for component menu items. This filter is now required because we are now going to have other custom menu types for backend which should not be included. Ref #2 * Load menu items from databases in correct hierarchy. Remove any extra separator type menu items created due to exclusion of certain menu items based on various conditions. Populate menu items loaded from db into the AdminCssMenu object for final rendering. Load new installed components menu items dynamically under the specified menu item with “components container” flag. Any unpublished menu items from the protected menutypes (viz. “main” and “menu”) will be skipped. When loading from system preset menu items, these components menu items are all included regardless of their published state. (B/C ok). Ref #2 * View manifests for menu item type and related language key updates. ref #2 * Minor mistake fix. * Translate menu item titles in list view. Ref #2 * Reset the preset menu structure to be same as the current J37 branch state, dropping implicit inclusion of #10657 improvement. Ref #2 * Allow the existing components to leverage the menu/submenu entries in their install manifest for admin menu manager menu link types. This provides ability to create links for then without requiring them to add layout manifests. Hence, full B/C solution. Ref #2 * Minor fix * Remove temporary dev phase files * Preparing for PR, database and install script updates. Ref #2 * Minor fix * Codestyle fix * CS fix * Don’t sort menu items * Sort lang keys Allow ‘component’ as first level alias in admin menu items Fix lang key Remove ‘home’ setting from admin menu items * apply in hathor * menu item alias check for site only * Post merge fixes. * Fixes as suggested by @infograf768 1. Group menu types by client id in lists and default admin menu 2. Hide association tab for admin menu items. 3. Hide client id filter for association mapping modal. * Add recovery mode for menu where the selected admin menu does not contain link to module manager and/or menu manager. * minor bug fix * Remove assoc column for admin menu items. Make recovery mode message more straight forward. Change radio to toggle buttons. * Add SMS to External URL menu item type (#13615) Allows data like sms://+15555555555 to be used instead of getting "Save Not Permitted" * Adding the Multilanguage Associations Manager (#13537) * Merge Associations rewrite * updated searchtool with the new way * udpated edit view title * added contact associationshelper class * temp fix * fix for category filter * added newsfeeds associations helper * CAPS for params * lang tag and added a helper function * added land tags * code style fix * better title in associations view * better title * use the usual naming * fix language tag, thanks to brian teeman and twitter :-) * initial review * on simple change * on simple change 2 * simple * some more helper changes * Update associations.php * Update associations.php * app isn’t set a model property * correct return value * simplify code adn use helper method * use typename directly * changed the tooltip position * Correct menu helper * remove unreacable code * correcting checked_out * com_menus * fixed not supportted message * installation * fix menu install * Spaces -> tabs * [com_associations] - mssql updates (#13617) the missed mssql updates for #13537 * [com_fields] Improved description in the "description" tooltip Fixes #13392 (#13557) * Fixes #13195: Added margin bottom to sidebar menu * Fixes #13392: Changed description field tooltip * Fixes #13392: Changed description field tooltip * Fixes #13392: Changed description field tooltip * sync admin menu menutype (#13618) * Routing: Remove IDs from tags URLs, use menu item of tags view as default for tag view (#11166) * Remove id from tags, use tags list menu item as default for tag * Code style, remove useless code, feature: first Itemid for tags view, second Itemid for default tag view * Updating install.xml en-GB administrator (#13623) * run grunt

joomla-cms-bot added the PR-staging label Jul 17, 2016

csthomas mentioned this pull request Jul 17, 2016

Tag routing is broken on multilingual sites #11163

Closed

yvesh reviewed Jul 20, 2016
View reviewed changes

Tomasz Narloch added 2 commits July 21, 2016 10:51

Remove id from tags, use tags list menu item as default for tag

0325796

Code style, remove useless code, feature: first Itemid for tags view,…

40b7c93

… second Itemid for default tag view

csthomas force-pushed the tagroute2 branch from 0d6cbe0 to 40b7c93 Compare July 21, 2016 08:58

csthomas changed the title ~~Remove id from tags, use tags list menu item as default for tag~~ Remove id from tags, use menu item of tags view as default for tag view Jul 21, 2016

csthomas changed the title ~~Remove id from tags, use menu item of tags view as default for tag view~~ Routing: Remove IDs from tags URLs, use menu item of tags view as default for tag view Jul 28, 2016

csthomas mentioned this pull request Jan 16, 2017

Fix single tags routing in Joomla 4 Branch #13600

Merged

joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jan 17, 2017

rdeutz merged commit 77af51f into joomla:staging Jan 17, 2017

joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jan 17, 2017

zero-24 added this to the Joomla 3.7.0 milestone Jan 17, 2017

csthomas deleted the tagroute2 branch January 17, 2017 15:16

csthomas mentioned this pull request Mar 27, 2017

New Router to #14915

Closed

csthomas mentioned this pull request Oct 18, 2017

Duplicate tags at joomla 3.8.1 #18363

Closed

Uh oh!

Routing: Remove IDs from tags URLs, use menu item of tags view as default for tag view #11166

Routing: Remove IDs from tags URLs, use menu item of tags view as default for tag view #11166

Uh oh!

Conversation

csthomas commented Jul 17, 2016 • edited by wilsonge Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary of changes

Testing Instructions

Documentation Changes Required

Tag route changes:

Uh oh!

yvesh Jul 20, 2016

Choose a reason for hiding this comment

Uh oh!

coolcat-creations commented Jul 23, 2016

Uh oh!

coolcat-creations commented Jul 23, 2016

Uh oh!

brianteeman commented Jul 23, 2016

Lime: index.php/all-tags/lime Red and Yellow is still wrong

Uh oh!

csthomas commented Jul 23, 2016

Uh oh!

coolcat-creations commented Jul 27, 2016

Uh oh!

csthomas commented Jul 27, 2016

Uh oh!

csthomas commented Jul 28, 2016

Uh oh!

Bakual commented Jul 28, 2016

Uh oh!

csthomas commented Jul 28, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hardiktailored commented Jul 30, 2016

Uh oh!

hardiktailored commented Jul 30, 2016

Uh oh!

Bakual commented Jul 30, 2016

Uh oh!

csthomas commented Jul 30, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JoshJourney commented Sep 21, 2016

Uh oh!

zero-24 commented Sep 21, 2016

Uh oh!

JoshJourney commented Sep 21, 2016

Uh oh!

JoshJourney commented Sep 22, 2016

Uh oh!

stellainformatica commented Oct 31, 2016

Uh oh!

csthomas commented Oct 31, 2016

Uh oh!

ghost commented Jan 8, 2017

Uh oh!

csthomas commented Jan 8, 2017

Uh oh!

ghost commented Jan 9, 2017

Uh oh!

wilsonge commented Jan 17, 2017

Uh oh!

rdeutz commented Jan 17, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

12 participants

csthomas commented Jul 17, 2016 •

edited by wilsonge

Loading

csthomas commented Jul 28, 2016 •

edited

Loading

csthomas commented Jul 30, 2016 •

edited

Loading