Preserve hierarchy when inserting/updating with node_save()

RobRoy - November 1, 2007 - 03:26
Project:Node Auto Term [NAT]
Version:6.x-1.1-beta2
Component:Code
Category:bug report
Priority:critical
Assigned:Zen
Status:patch (to be ported)
Description

$node->taxonomy in insert is actually structured differently than $node->taxnomy in update. Patch fixes this. Took me a looooong time to debug this. Frickin inconsistent Drupal core.

AttachmentSize
nat.update.patch2.15 KB

#1

RobRoy - November 7, 2007 - 04:14
Status:needs review» closed

Somehow this fixed my bug but I think it was cuz of other buggy code of mine. Now after reviewing again, my patch breaks NAT. Me so confused, but it's working now.

#2

RobRoy - December 3, 2007 - 21:54
Status:closed» active

So this is actually a bug in core that I just submitted. Doing a node_save($node) will completely screw up your hierarchial relationships in NAT because $node->taxonomy has different structure than when you do a node edit form submit. We should still find a work around for NAT so you can do a node_save() so I'm leaving it open just so people know.

When submitting a node edit form, hook_nodeapi's $node->taxonomy has completely different structure than when doing a node_save: http://drupal.org/node/197532

#3

RobRoy - December 4, 2007 - 00:23
Title:Update hierarchy code is buggy» Preserve hierarchy when inserting/updating with node_save()
Status:active» needs review

Okay, this is the one. Patch includes a workaround for the aforementioned inconsistency in Drupal core and now NAT preserves hierarchy during insert/update for both the node form and when using node_save(). Hopefully this can get in soon so no one else has to go through the debugging hell that I have! :P

AttachmentSize
nat.hierarchy.patch 2.32 KB

#4

jpsalter - December 8, 2007 - 05:38

This was the hack I used to maintain the parent->child relationship of updated terms:

starting around line 579'ish - in function _nat_update_terms, change the foreach to this:

<?php
 
foreach ($terms as $term) {
    if (isset(
$hierarchy[$term->vid])) {
     
$edit['parent'] = $hierarchy[$term->vid];
    }
    elseif(
count(taxonomy_get_parents($term->tid))) {
     
// preserves term's parent if it exists
     
$parents = taxonomy_get_parents($term->tid);
     
$parent = current($parents);
     
$edit['parent'] = array($parent->tid => $parent->tid);
    }
    else {
     
$edit['parent'] = array();
    }

    if (
count($relations)) {
     
$edit['relations'] = $relations[$term->vid];
    }

   
$edit['tid'] = $term->tid;

   
taxonomy_save_term($edit);
  }
?>

Note - if you're using the i18n module and/or have languages attached to terms - you'll need more code to preserve the language of the term

#5

Zen - December 19, 2007 - 09:50

Would it be possible to get a node_save code snippet that I can use to test this out?

Thanks,
-K

#6

RobRoy - December 20, 2007 - 18:52

Any node_save($node) will work...

Try this:

1. Assign a node to a term (so that it's associated NAT term will be a child of that parent term) by using a regular node form
2. Then run this snippet:

<?php
$node
= node_load(XXX);
node_save($node);
?>

You'll notice that the NAT term for that node is now at the root level. This patch fixes that bug.

#7

smokris - January 26, 2009 - 14:18
Version:5.x-2.1» 6.x-1.1-beta2

+1 for including jpsalter's 6-line patch in comment #4.

Preserving parents remains an issue with the current 6.x version, and jpsalter's patch works as-is in 6.x.

#8

nonsie - February 11, 2009 - 02:20

This is a slightly modified version:

<?php
 
foreach ($terms as $term) {
    if (isset(
$hierarchy[$term->vid])) {
     
$edit['parent'] = $hierarchy[$term->vid];
    }
    else {
     
// Find term parents
     
$parents = taxonomy_get_parents($term->tid);
      if(
count($parents)) {
       
// Preserves term's parent if it exists
       
$parent = current($parents);
       
$edit['parent'] = array($parent->tid => $parent->tid);
      }
      else {
       
$edit['parent'] = array();
      }
    }

    if (
count($relations)) {
     
$edit['relations'] = $relations[$term->vid];
    }

   
$edit['tid'] = $term->tid;

   
taxonomy_save_term($edit);
  }
?>

There is no reason to call taxonomy_get_parents() twice (the results of this function are not cached like taxonomy_get_term()). Patch against 6.x-1.1-beta2 attached

AttachmentSize
nat.module_update_terms.patch 961 bytes

#9

Zen - February 15, 2009 - 07:18
Assigned to:Anonymous» Zen
Status:needs review» needs work

IMO, the patches seem to be going about this the wrong way around. The $hierarchy array should be fixed before getting to _nat_update_terms, i.e., in hook_nodeapi. I'm not sure why taxonomy_get_parent calls are being used if the only issue is that the array is structured differently. It should be sufficient to just add a sanitiser function to restructure it as per the node form, right?

Thanks for everybody's work on this.
-K

#10

Zen - February 15, 2009 - 07:20
Priority:normal» critical

It seems that direct node_save calls also nuke all related terms and synonyms ...

#11

jpsalter - February 15, 2009 - 08:53

I have not looked at this problem in a long time. But, I think the goal is to have this module support the following:

<?php
  $nid
= ###;
 
$node = node_load($nid);
 
node_save($node);
?>

At a minimum - I think the module should "do no harm". If it is not receiving information the way it expects, it should abort, rather than corrupt data.

#12

Zen - February 15, 2009 - 20:59
Status:needs work» patch (to be ported)

Committed patch which restricts the update operation only to form submissions.

Patch to be backported and tested for D5 as well.

Cheers.
-K

#13

arlinsandbulte - September 6, 2009 - 03:14

It looks like this patch is now included in 6.x-1.1-beta3.
Can this issue be marked fixed?

#14

grigs - September 9, 2009 - 14:49

I'm pretty sure this is still unresolved. I just tested with the 6.x-1.x-dev and the 6.x-1.1-beta3 version and it still exhibits the same behavior where saving loses the hierarchy (parenting in my case).

@arlinsandbulte Is it working for you?

#15

grigs - September 9, 2009 - 14:56

Ok, to be more specific, 6.x.1.x-dev will preserve the order of top level terms, but not any children. If you move something to be a child term in the hierarchy, the next time you save the node, the term will be back to the top level instead of being a child.

#16

arlinsandbulte - September 9, 2009 - 18:00

It seems to be working ok for me...

Grigs, how are you creating children terms? What I do is add the Taxonomy vocabulary to my content type (group in this case).
Then, when creating a new taxonomy node/term using NAT, I select the term I want it to be a child of.
Then, I can later edit the taxonomy node select different term to change its parent.

DO NOT use the taxonomy term list page (/admin/content/taxonomy/1) to eidt parent/child relationsihps! This will mess up NAT and you taxonomy.
The only thing I use that page for is if I want to re-order the terms.

The only problem with this is that if the taxonomy vocabulary is marked a 'required,' it is not possible to create a root level taxonomy node/term as you are forced to select a taxonomy term when creating the taxonomy node. I might submit another issue for this.
Also, if you edit your taxonomy node, and select the taxonomy term created by the node, weird things can happen.

#17

grigs - September 12, 2009 - 23:05

@arlinsandbulte wow, that does work. what a strange way to go about editing things. so create child parent relationshipships on the node. order within levels on in taxonomy, but don't dare touch child/parent in taxonomy or you'll lose it.

How did you figure that out? Is it documented somewhere that I missed?

#18

arlinsandbulte - September 13, 2009 - 00:11

@grigs: not sure how I learned that little gem of information. It must have been somewhere here in the issue queue.

Like I said, the only problem with it is that if the vocabulary is marked as required, there is no possible way to create a root level taxonomy term with NAT... you NEED to select a term to save the NAT node and that makes the new NAT term a child of the term you select. The best way around that would be if vocabularies could set to required or not required per content type. I found a feature request for just this, but it has not been very active: #62681: Vocabularies required per content type.

Also, it would be nice if NAT would better integrate with the taxonomy ui to restrict things that should not be done. For instance, NAT terms could not have their parent/child relationships edited unless editing the NAT node. Or NAT terms could not be deleted unless the NAT node is deleted. Right now, if you delete a NAT term without deleting the NAT node, that can screw things up.

#19

jbomb - October 21, 2009 - 20:30

#16 works great for the parent/child relationships. Any ideas on how one would maintain the relationships for related terms?

 
 

Drupal is a registered trademark of Dries Buytaert.