Adding an empty node deletes first item in queue

michaelsauter - October 2, 2009 - 03:36
Project:Nodequeue
Version:6.x-2.4
Component:Code
Category:bug report
Priority:normal
Assigned:Unassigned
Status:closed
Description

This bug appears on the admin view page of Nodequeue.
If no node title has been entered, clicking on "Add node & save queue" can result in deleting the first item of the queue. I do not know why this does not happen every time though.

This small patch checks if the nid of the given node is null before adding a node, which seems to fix the problem.

AttachmentSize
nodequeue_adding_empty_node_does_no_delete.patch592 bytes

#1

ezra-g - October 3, 2009 - 19:19

Thanks for calling this to my attention. It turns out there are a few things wrong here:

Clicking the "Add node & save queue" button with no text can result in adding an value to nodequeue_nodes with a nid of 0. This value then counts against the queue's size limit, resulting in the removed node. What's more, nodequeue_check_subqueue_size() is called at the wrong point -- it should be called after we add the node and increase the subqueue size.

I plan to commit this but it would be great go get your testing on this revised patch.

AttachmentSize
593858.patch 1.82 KB

#2

ezra-g - October 3, 2009 - 19:22

Actually, we should also add a database update that clears invalid data from the nodequeue_nodes table.

#3

ezra-g - October 3, 2009 - 20:21

This patch performs validation when the form is submitted, as well as before the values are saved to the database. The form_set_error doesn't highlight any fields, but this is significantly better than what we have now which has no validation and saves bad data to the database.

AttachmentSize
593858-1.patch 4.13 KB

#4

ezra-g - October 3, 2009 - 20:32
Status:needs review» fixed

This is applied. Thanks again for pointing this out.

#5

System Message - October 17, 2009 - 20:40
Status:fixed» closed

Automatically closed -- issue fixed for 2 weeks with no activity.

 
 

Drupal is a registered trademark of Dries Buytaert.