Improve redirect mechanism for merged terms

mh86 - September 5, 2009 - 10:25
Project:Taxonomy Manager
Version:6.x-2.x-dev
Component:Code
Category:task
Priority:normal
Assigned:Unassigned
Status:needs review
Description

As discussed in #567564: Disable the taxonomy/term display by default we shouldn't use the menu_alter to redirect merged terms, because this breaks other modules like Taxonomy Breadcrumb and Panels3.
By default, the redirect is now disabled and it's working with other modules, but there might be better ways to implement this.
So I started an issue where we can work on a patch

#1

DamienMcKenna - September 8, 2009 - 16:11

Subscribing. I suggest using Path_Redirect if available.

#2

DamienMcKenna - October 3, 2009 - 00:59

Initial patch that..

  • creates path_redirect values for the taxonomy/term/[tid] URL of the $merging_terms,
  • creates path_redirect values for any url_alias URLs of the $merging_terms,
  • updates any existing path_redirect values that already existed to point to the $main_term.
AttachmentSize
taxonomy_manager-n569086.patch 2.91 KB

#3

DamienMcKenna - October 3, 2009 - 00:59
Status:active» needs review

#4

dankohn - October 4, 2009 - 04:49

That patch has an error in it. You need to change module_enabled('path_redirect') to module_exists('path_redirect')

With that change, the patch appears to do the right thing and is a welcome improvement.

Given that I also confirmed the incompatibility with Panels 3, I would suggest removing the existing redirect functionality entirely, and have the module just do a path_redirect by default instead (but still allow the checkbox to be unchecked). That would make path_redirect a prerequisite for this module, which would be fine.

#5

DamienMcKenna - October 4, 2009 - 10:47

Gah. I forgot to update the module with my local test code. Here's a corrected patch.

AttachmentSize
taxonomy_manager-n569086-2.patch 2.91 KB

#6

digi24 - November 9, 2009 - 09:57

Thanks for the patch Damien. Maybe it would be useful to add a default option or omit the whole options thing. After all, it is the expected behaviour, when using the redirect module:

   if (count($options) > 0) {
     $form['merge']['options'] = array(
       '#type' => 'checkboxes',
       '#title' => t('Options'),
       '#options' => $options,
     );
+    if (module_exists('path_redirect')) {
+      $form['merge']['options']['#default_value'] = array('merge_redirect');
+    }

 
 

Drupal is a registered trademark of Dries Buytaert.