adding the same node more than once to a queue causes bad behavior

Also, if you add the same node two (sometimes three) times to the end of the queue, it deletes the first item of the queue.

@rokit88 Thanks for pointing this out. I'd love to see this fixed, but don't anticipate correcting it myself before the next release. I would be happy to review a patch though :).

Here's a patch that shows the general kind of change that might bring this feature back/fix this bug. It's incomplete and needs some work.

This patch changes the form structure so that it can display the queue with multiple values, and does part of the work for interpreting these values from the form and passing them to nodequeue_save_subqueue_order.

The next step here is to make sure that the $form_state['values'] contain both the nid and position of nodes in the queue. With this patch, the position is described twice, but the nid is not. Therefore, it is impossible to save the subqueue order.

I've spent a good amount of time working on this. It would be great if someone who needs this feature could take this patch the rest of the way.

Sorry, that was somewhat hilariously not the right patch. It is now attached.

FYI, this appears to be a blocker to #302671: Add to queue from Node edit form. The approach in #4 wouldn't resolve that issue, but whatever the outcome I think this issue will need to be resolved first.

FYI, I submitted a patch in #302671: Add to queue from Node edit form which contains a couple API changes for dealing with multiple appearances of a node in a queue. Not directly related to this particular issue, though.

I see that this issue is marked as 7.x-2.x-dev... is there any chance that it will be fixed for D6?
It isn't clear to me which patch is the best option right now. If I want to help out by testing a patch, which one should I use?

The patches above are all off the 6.x version, I believe. I think the 6.x-2.x-dev version got renamed to the 7.x-2.x-dev version and a new 6.x-2.x-dev was created in its place, which is why this is appearing as 7.x now.

@lesmana: thanks, I'll give the patch from #5 a try when I get back to work on monday.

Hi,

I just tried this patch, and it works fine. When i add a node that has already been added it does show now in the queue (there is a drupal_set_message in the code that should be deleted though on line 2757).

But now when I try to change the order of the queue the order isn't saved.

If this could be solved this patch would be fine to use!

Closed a bunch of duplicates: #1062014: Nodequeue duplicate nodes if add node twice or more, #701636: User is able to select node already in queue for adding into queue thus pushing a node out of the queue if it is full and #1022998: Code modification to avoid duplicate node entry in subqueue.

I will tackle this issue in a few days for both 7.x and 6.x.

Any news on this? Being able to properly manage a node being added multiple times to a queue in 6.x would be sweet.

just approached this bug, subscribing

subscribe

@bzbz, haven't you seen the news? http://drupal.org/node/1306444

@amateescu, I hadn't. thx.

Yet the aim of my post was to say: I still have this issue with latest (not dev) version of the 6.x module.

That why I changed it to 6.x-2.11. Was it wrong too?

@bzbz, well, we generally fix bugs in the latest release and then backport them, it's the Drupal core development model :)

I ran into this recently using 7.x-2.0-beta1. While I wouldn't normally see why somebody would want to add two of the same node to a queue, this bug will not only show one instance of the node on the form, but if your queue is of fixed size and already full, the first node in the queue will be removed, reducing the size of the queue by 2.

In any case, I've attached a patch which goes off the 7.x-3.x branch, but I imagine it can be backported to 7.x-2.x fairly easily. The patch does include some changes that may or may not have been necessary, for instance in _nodequeue_dragdrop_get_nodes(), it was doing a leftJoin instead of an innerJoin, innerJoin's are typically much faster and should generally be used unless a leftJoin is needed to get the desired result, which didn't look like the case to me here.

Let me know if you need help backporting it, or if anything needs work.

#20: nodequeue-add_same_node-593468-20.patch queued for re-testing.

"While I wouldn't normally see why somebody would want to add two of the same node to a queue"
Example: advertising banners, where certain banners are given a higher rotation (are in the Nodequeue multiple times) than others. Etc.

@jojonaloha
I just applied this to 7.x-2.0-beta1 and I've found a few issues:

1. Removing items from the queue doesn't work anymore: the token is wrong. This is because your patch changes the generation of the token to use $key instead of $node->nid
   

    // Inside the function 'nodequeue_arrange_subqueue_form'
   nodequeue_get_query_string($key, TRUE), 
   // This should be
   nodequeue_get_query_string($node->nid, TRUE), 
   

2. Removing always removes the top item, not the one clicked. This is the way the nodequeue handles the delete (it searches for the MIN(position) and removes that item). It would be great if your patch could fix this as well.

ps. for the most part, this patch applies to 7.x-2.0-beta1 just fine. There is only a single hunk that doesn't apply cleanly. (I've applied it by hand)

Attached patch from #20 with improvements from #25 adapted for 7.x-2.x branch.
It works for me as well (deleting also).

Ok the patch 20 it's for the 3.x branch right?

I found a bug in the #26 patch. Bug is present on click on remove link.

+    $form['nodes'][$key]['remove'] = array('#markup' => l(t('remove'), 'nodequeue/' . $queue->name . '/remove-node/' . $subqueue->sqid . '/' . $node->nid, $attr));

should be replaced with

+    $form['nodes'][$key]['remove'] = array('#markup' => l(t('remove'), 'nodequeue/' . $queue->qid . '/remove-node/' . $subqueue->sqid . '/' . $node->nid, $attr));

There is still bug if you add first element

Notice: Undefined index: nodes in nodequeue_arrange_subqueue_form_validate() (line 878 of /home/pity/workspace/welforum/sites/all/modules/contrib/nodequeue/includes/nodequeue.admin.inc).
Notice: Undefined index: nodes in nodequeue_arrange_subqueue_form_validate() (line 879 of /home/pity/workspace/welforum/sites/all/modules/contrib/nodequeue/includes/nodequeue.admin.inc).
Warning: Invalid argument supplied for foreach() in nodequeue_arrange_subqueue_form_validate() (line 879 of /home/pity/workspace/welforum/sites/all/modules/contrib/nodequeue/includes/nodequeue.admin.inc).

Applied patch from #26 but also got removement bug. Changed this as described in #28. It works.

valgibson you need to merge you patch with the #26 to test it ...

Rerolled for 7.x-2.x-dev. Works great for me.

Committed, thanks everyone!