Saving nodes resets nodeorder

I don't know if this is the same but I experience a reset of node order when I drag and drop nodes under a taxonomy term. This, I posted originally on the main forum.

By adapting the code, i could get it working. The thing is now, that I have a changed module and I don't know if/where/... it should be reported or is it even correct?
The orignal code in nodeorder.admin.inc calls a standard array value key (so 0, 1, 2, 3, ...) and assign this as the node-id ($nid). Clearly this isn't working as in the tables the nid is not alike an array value key but is a different value. As the form for dragging nodes in the correct order carry nid-values, I extracted them out of the form-object and used that as $nid. Result:

  foreach ($form_state['values'] as $key => $node) {
    // Only take form elements that are blocks.
    if (is_array($node) && array_key_exists('weight', $node)) {
       // Find node id
      $nid = $form[$key]['#node']->nid;
      db_update('taxonomy_index')->fields(array('weight' => $node['weight']))
        ->condition('tid', $tid)
        ->condition('nid', $nid)
        ->execute();
    }

Compare to the original:

  foreach ($form_state['values'] as $nid => $node) {
    // Only take form elements that are blocks.
    if (is_array($node) && array_key_exists('weight', $node)) {
      db_update('taxonomy_index')->fields(array('weight' => $node['weight']))
        ->condition('tid', $tid)
        ->condition('nid', $nid)
        ->execute();
    }

The db_update is now working properly as the system finds back the corresponding node-id.

Did I find a bug or did I have a buggy situation originally and created a workaround for my problem? I honestly don't know... .

I can confirm that on saving the node weight is reset to 0...very frustrating. Are there any maintainers out there? The above code does not fix this.

/**
 * Implements hook_node_update().
 */
function taxonomy_node_update($node) {
  // Always rebuild the node's taxonomy index entries on node save.
  taxonomy_delete_node_index($node);
  taxonomy_build_node_index($node);
}

The taxonomy index table seems not a very good place to store the node weight data...

So yeah, I'd call the 7.x version of this module seriously flawed at the moment.

Edit: hm, I see that nodeorder does attempt to account for this issue already in its own hook_node_update().
Edit 2: ... but this "attempt" is smattered with Drupal 6 code and bound to fail!

Replaced to #1883038: Ordering doesn't work when node has multiple taxonomy terms per the comment below - you are right grahamC, thanks.

@syngi: I think that should go into a separate issue, if you don't mind.

This is about the weight being reset to 0 during node save rather than any issue loading it.

Second try :)
After my issue above I also stumbled onto this problem. But I was able to track it down. I believe it is due to a problem in the function nodeorder_can_be_ordered, the condition which checks on the $nodeorder_vocabularies is set to field vid while is should be field machine_name. So on line 589 in file nodeorder.module, change

->condition('v.vid', $nodeorder_vocabularies, 'IN')

to

->condition('v.machine_name', $nodeorder_vocabularies, 'IN')

The field vid is just an integer and we are comparing to the machine name values. So I believe nodeorder_can_be_ordered was never working before...

Note that fixing this will make the function nodeorder_node_update to throw a warning because $tids isn't defined there. I just added

$tids = array();

on line 796, but this is just to get rid of the warning .. the code of the function needs to be reviewed, but I don't have the time for that now.

Just tried this but unfortunately this didn't solve the issue of editing nodes re-ordering the weights of 'node order'

Thanks for everybody contributing. I'am still stuck for the moment with my change i've put in comment number one.

Third time is a charm they say. I rewrote parts of the module to handle the node order correctly when inserting, updating and deleting nodes. The changes are now live with a client, for some weeks already, and no problems occurred up until now.
See the files attached. This is just the module as I have it now, not a patch. I hope someone can confirm these changes and make a patch for it.

As a side-note, I concur with grahamC and feel like this module needs a complete rewrite or at least and update to D7. The code I rewrote was just not working or shortcoming. I could do this, but only on a paid basis I'm afraid. Feel free to PM me.

Hooray! Thanks syngi!

I've just had a little go with it, and no issues _so far_. I've spun it into a patch (commented debug functions removed).

(I'd run off to try nodequeue instead, but its taxonomy smartqueue thing doesn't seem fully baked yet either... :-/)

I've applied the patch from grahamC, following Syngi's work and confirm that saving nodeorders within taxonomy terms seems to work now.
I'am willing to work on the rewrite or work on the module. I'am very newbie in this field, but maybe this is the moment to step forward. Anyway, let me know.

I switched back to the 7.1.1 version as the new version does not allow me to save or edit a node. In the new version of nodeorder the function nodeorder_orderable_tids_by_node($node) is not working on line 629 in nodeorder.module. Editing a node gives an sql error on a table that does not exist. Some changes are needed in the new version. I want to help if I can.

@Jfair

I'm not sure what version you're testing with, as line 629 doesn't correspond with anything likely in either syngi's zip file nor 7.x-1.x with my patch applied...

What code in that function is triggering the error? What is the SQL query that is failing, and which table is it failing to find?

(if you haven't already, you might like to set up Xdebug to try and work out what's going wrong)

Hi grahamC.
Drupal updates always want to install version 7.x.1.2. and there the error happens. This is apparently the latest version. I'll have a look on the 7.x.1.1. . I'll get back.

Hi grahamC.
I've used the 7.x.1.1. and applied the patch. I can reshuffle the nodes. But when I'am trying to change the content of the topnode (having a negative weight now), I get the error message 'Weight must not be negative'. Also, Drupal wants me to upgrade to 7.x.1.2.
By the way, I tried to use xdebug in netbeans, but get only a waiting for connection message (sigh...). Xdebug is working in php though... .

I get the error message 'Weight must not be negative'.

Is this happening with Ubercart products? I think I found that nodeorder conflicts with Ubercart products, because they both try to set/use $node->weight. I created the issue #1936454: Nodeorder weight conflicts with Ubercart Product weight to document that.

Yes Nicedawg, it's an Ubercart product.

Strange, but in 7.х-1.2 it is not fixed.

This has not yet been committed, you'll need to apply the patch to the 7.x-1.1 version.

Using syngi's module, you can't reorder the nodes if you want the node to be on a different page. I modified line 30 of node.admin.inc to $node_ids = taxonomy_select_nodes($tid, FALSE, FALSE, array('t.weight' => 'ASC')); for now.

Any progress on this?

when all patches applied it works, while logged in as use0, but any other user I get a mysql error:

PDOException: SQLSTATE[42000]: Syntax error or access violation: 1064 You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near ')) AND (tid = '94')' at line 2: UPDATE {taxonomy_index} SET weight=weight + 1 WHERE (nid IN ()) AND (tid = :db_condition_placeholder_0) ; Array ( [:db_condition_placeholder_0] => 94 ) in nodeorder_add_node_to_list() (regel 943 van /var/www/vhosts/www.mysite.com/htdocs/sites/all/modules/contrib/nodeorder/nodeorder.module).

jeroenhoutmeyers, this patch should fix your problem, can you check it ?

This sounds like the module doesn't complete one of its main functions - is that accurate? A note on the project page to that effect would save people time trying to figure out why it's not working.

Stumbled upon this problem as well, the complete module code is a real mess, beside the fact that it's missing one of it's main features. I'm just refactoring some of it.

Bit of a frustrating issue this one as we rely on the module to order categorised lists of products within a view - and the product nodes receive updates regularly. As soon as an updated node is saved, the order we saved goes missing. Not a massive problem in theory but when we have a taxonomy view consisting of 70-odd nodes it can be tiresome to clean up the mess following the save...

Module maintainership has been handed over to me, so I will work towards testing and committing the patch in here.

The codebase is a total mess, so it might take me some time to get used to it and start refactoring. If anyone has done a lot of this or is interested in helping out, I'm happy to accept a co-maintainer.

I've committed a whole bunch of changes relating to modification that syngi made, but I don't think I've nailed down the issue with the resetting of weights on a node save. Will take a good look at this tomorrow.

I just ran across this problem in a site I'm managing. After pulling the latest git code, a big problem that jumps out at me is the inconsistent use of the "nodeorder" attribute added to node objects. The nodeorder_node_update and nodeorder_node_presave functions both look to it to determine what they should do, but I can't find anywhere it actually gets loaded onto the node.

It seems like nodeorder_node_load should be adding it, but there's an odd bit of logic at the beginning to make sure it only runs on a taxonomy term page, and even then it's just adding a 'weight' attribute for the term whose page is being viewed. I'm going to attempt to build the nodeorder array the other functions are expecting there, and if I get something that works I'll put up a patch for review.

Here's a patch that seems to work on my site. I made changes as described in my previous comment:

• Changed nodeorder_node_load to always generate the nodeorder array for a node object and attach it.
• Made the admin page use this array instead of looking for the 'weight' attribute as had been previously loaded.

The admin page (nodeorder_admin_display_form in nodeorder.admin.inc) is the only place I could find that used the weight attribute that nodeorder_node_load had been previously setting on the nodes. I haven't been able to find any other places in the code that would be affected, but would appreciate others taking a look.

While your patch does fix the inconsistency with the use of looking for the nodeorder weight, it doesn't seem to fix the ordering being reset on node save for me.

I'm committing this patch as I believe it fixes this issue (#1936454: Nodeorder weight conflicts with Ubercart Product weight) for a start.

I've just done some more testing, and find that nodeorder weights do stick on editing and saving a node. Can someone else confirm this for me? If there are no complaints in the next week or two I will tag the current dev for release.

Patch #31 did indeed fix this problem, and my comments in #33 were incorrect.

What I'm experiencing is actually a problem with entity reference fields, since the current logic only supports taxonomy reference fields. But I'll open that as a separate issue (feature request).

I've been using it on a production site since I posted the patch, and we've edited a lot of content without this problem occurring again.

I thought maybe I hadn't generalized something for it to work elsewhere, but I couldn't find any other areas in the code to explain the behavior.

Closing this issue and creating a new release, since the module actually works properly now.

Thanks for your work.

I am having this issue in ver 7.1.4. Every time I save the Node order gets set back to zero.

I too was still having issues with this. Turns out that while the patches work, the node_update hook in the nodeorder module was being fired before the node_update hook in the taxonomy module - so the weight was being fixed in taxonomy_index BEFORE the taxonomy was being rebuilt.

I was able to get around this by adding this to the module, and then flushing the caches:

<?php
function nodeorder_module_implements_alter(&$module_list, $context){
 if($context === "node_update"){
   $temp = $module_list['nodeorder'];
   // Removing the mymodule key/value
   unset($module_list['nodeorder']);
   // Adding the mymodule key value as the last member in the list
   $module_list['nodeorder'] = $temp;
  }
}
?>

This tells Drupal to fire the node_update hook for nodeorder last - thus after the taxonomy module.

I got this solution from here: https://lionide.wordpress.com/2010/12/06/drupal-7-hook-execution-order/

Somewhat new to Drupal, so I will defer to others whether this should be in the .module file or in the .install. Hope this helps out someone who has been banging their head against the table like I was!

Thanks for the info, let's see what the maintainer thinks.

Okay guys, thanks for all the help. I've rolled the changes suggested by @dmaietta into a patch.

My only concern here is that this change will also move this hook to be called after any custom modules that might be calling node_update, thus meaning that if they are relying on some change from nodeorder it will no longer be there. Of course they could then also implement hook_module_implements_alter() to fire their hooks after nodeorder if required.

I might just be getting worried over nothing though, but please do let me know your thoughts and also test this out to see if it works.

Hi,

I have the same problem. My node order isn't save. After debugging, I see an error in nodeorder.admin.inc with the function nodeorder_admin_display_form_submit(). Effectively, the nid variable is wrong in the update query, it has only two values 0 or 1 and not the node id like the query wants.

To resolve this problem, I change the query condition like that :

foreach ($form_state['values'] as $key => $node) {
    // Only take form elements that are blocks.
    if (is_array($node) && array_key_exists('weight', $node)) {
      $nid = $form[$key]['#node']->nid;  
      db_update('taxonomy_index')->fields(array('weight' => $node['weight']))
        ->condition('tid', $tid)
        ->condition('nid', $nid)
        ->execute();
    }
  }

Now, my order is ok :)

I can create a patch, if you want.

Best regards.

I put the update from comment #44 on my site and it worked.
Really appreciate the fix.

Please do create a patch, it will hopefully be reviewed quickly!

Hi,

This is the patch for comment #44.

Enjoy :)

i'm still having this issue with the latest release. Any idea?

same here. order weight is reset on node update

After some digging i found the cause of the problem.

Nodeorder only checks if term is referenced via taxonomy_term_reference field type while I was referencing the term with entity reference field. Also i was referencing the term inside field collection and nodeorder ignored that as well.

Here is the fix in nodeorder.module in function nodeorder_can_be_ordered

foreach ($fields as $field_name => $field) {
  if (($field['type'] != 'taxonomy_term_reference' && $field['type'] != 'entityreference')
	|| (empty($field['bundles']['node']) && empty($field['bundles']['field_collection_item']))
	|| (!empty($field['bundles']['node']) && !in_array($node->type, $field['bundles']['node']))) {
	continue;
  }
  if($field['type'] == 'entityreference'){
	foreach ($field['settings']['handler_settings']['target_bundles'] as $allowed_values) {
	  $nodeorder_vocabularies[] = $allowed_values;
	}
  }else{
	foreach ($field['settings']['allowed_values'] as $allowed_values) {
	  $nodeorder_vocabularies[] = $allowed_values['vocabulary'];
	}
  }
}

Here's the patch.