Subqueue name changes don't affect individual subqueues

Offlein - July 10, 2008 - 19:22
Project:Nodequeue
Version:5.x-2.x-dev
Component:Code
Category:bug report
Priority:normal
Assigned:Unassigned
Status:patch (to be ported)
Description

If a queue is renamed (from the node queue "edit" page), it seems to appear named correctly on the node queue list page.
However, on the "Node queue" tab of a valid node, the queue seems to retain its original name.

#1

merlinofchaos - July 10, 2008 - 19:32

Hmm. This probably means the subqueue needs to be renamed as well I think, at least in regular queues.

#2

Coyote - July 11, 2008 - 18:50

Yep. When I looked in the db, there was a single subqueue for each nodequeue I'd created, and their names were not updated when changes were made elsewhere!

#3

merlinofchaos - July 11, 2008 - 19:11

As a workaround you can rename the subqueue in the database; it's not ideal but it does mean you can get around the problem for the moment.

#4

ezra-g - August 9, 2008 - 20:58
Title:Renaming a Queue Does Not Work» Queue Name changes not reflected in tabs

Can you test the patch at http://drupal.org/node/277867 and see if it resolves your issue?

#5

ezra-g - August 16, 2008 - 15:34
Title:Queue Name changes not reflected in tabs» Subqueue name changes don't affect individual subqueues

I can reproduce a different symptom but show the same underlying problem, that subqueue settings are changed in the nodequeue_queue but not for each individual subqueue in the database. For me, however, they show correctly on the node/nid/nodequeue tab and incorrectly in the links.

We need a function to batch update subqueue titles after the (parent) queue title settings change. I'm changing the title of this issue so it's more clear to me which one it is ;).

#6

ezra-g - August 16, 2008 - 16:09

Since smartqueues can define subqueue titles, one solution is to create a new hook that gets called from nodequeue_save, and when the setting for subqueue titles is changed, nodequeue has smartqueues generate new titles, which nodequeue will then save.

What do you think, merlinofchaos?

#7

DanielTheViking - August 24, 2008 - 17:33

Subscribing.

#8

joachim - December 15, 2008 - 10:55

Subscribing.

#9

psynaptic - February 10, 2009 - 21:18

Just noticed this. I think it's this anyway..

I have renamed some of my nodequeues and the nodequeues at node/*/nodequeue still have the old names.

Going to rename in the db for now.

#10

ezra-g - February 14, 2009 - 18:26
Version:5.x-2.2» 6.x-2.0

This is still present in the D6 version.

#11

jmather - October 15, 2009 - 22:31

Is this fixed yet? We have a large number of normal queues and this causes significant pain anytime we rename a queue...

#12

ezra-g - October 15, 2009 - 22:42

This has not been fixed because nobody has taken the time to submit a patch for it. If you'd like to help it get fixed faster, I'd be happy to review a patch. Another option is to hire a developer to patch this.

#13

jmather - October 16, 2009 - 03:11

I attached a quick shot at it, not sure if this is the right way to make sure we are getting the master->queue->subqueue relationship. Thanks for a review.

AttachmentSize
subqueue_title.patch 1.27 KB

#14

jmather - October 16, 2009 - 03:28

Sorry that patch is bad, will put another up shortly...

#15

jmather - October 16, 2009 - 03:30

Okay, try this one..

AttachmentSize
subqueue_title.patch 1.27 KB

#16

ezra-g - October 19, 2009 - 04:04
Version:6.x-2.0» 6.x-2.4
Status:active» needs work

Thanks for the patch!

The code in this patch updates the nodequeue_subqueue table but uses the qid value instead of the sqid value, which won't work in all cases.

This prompted me to take a closer look at the code involved here. We want to make sure that title changes are done when updating a queue using the queue_edit_form, and also respect titles that are configured to use a string replacement in the name of the subqueue, such as the included smartqueue.module which replaces %s with the name of a taxonomy term.

Nodequeue already has a nodequeue_subqueue_update_title() function, but in cases where the subqueue titles are configured to use string replacement, we need to run this query for every subqueue when the edit_queue form is submitted :/. So, the complete solution here IMO is probably to implement a new hook that lets smartqueues declare all of their subqueues so that the titles can be processed and saved to the database using string replacement where necessary.
This sort of makes me wonder though, why we need to even store these processed titles in the database instead of just storing the pattern and doing the string replacement when the subqueue titles are displayed.

This bug could probably fixed for non-smartqueues relatively simply nodequeue_subqueue_update_title(), so I would commit a patch that does that in the meantime :).

#17

jmather - October 19, 2009 - 17:53

Thanks..I realized that later the sqid is not directly correlated to the qid even in non-smart queues.

My question is, since a non-subqueue, has a subqueue, how can you tell which which entry in the nodqueue_subqueue table correlates directly to the entry in the nodequeue_queue table?

I'm not totally sure I follow the simple fix for non-smartqueues but I would need to understand how the above works in order to patch with the nodequeue_subqueue_update_title function since that only takes the $sqid and $title as its args.

Thanks in advance ....

#18

ezra-g - October 24, 2009 - 20:52

My question is, since a non-subqueue, has a subqueue, how can you tell which which entry in the nodqueue_subqueue table correlates directly to the entry in the nodequeue_queue table?

The qid column indicates the queue for all subqueues.

#19

jmather - October 26, 2009 - 20:43

I'm not sure if I explained myself appropriately and in this case I'm only concerned with queues that are non-smart queues. I suppose I meant, how do I know what a non-subqueue's/non-smartqueue's correlated subqueue ID is? (Since every queue has a subqueue)

arghh..this should be very simple for non-smartqueues and I think it is, perhaps the reference field in the subqueue? I don't see how we can use nodequeue_subqueue_update_title($sqid, $title) in this case because we don't know what the non-smartqueues subqueue id is...

? subqueue_title.patch
Index: nodequeue.module
===================================================================
RCS file: /cvs/drupal-contrib/contributions/modules/nodequeue/nodequeue.module,v
retrieving revision 1.95
diff -u -p -r1.95 nodequeue.module
--- nodequeue.module 6 Oct 2009 19:46:18 -0000 1.95
+++ nodequeue.module 16 Oct 2009 03:30:08 -0000
@@ -1884,6 +1884,8 @@ function nodequeue_save(&$queue) {
     db_query("UPDATE {nodequeue_queue} set size = %d, title = '%s', subqueue_title = '%s', link = '%s', link_remove = '%s', owner = '%s', show_in_links = %d, show_in_tab = %d, show_in_ui = %d, i18n = %d, reverse = %d, reference = '%s' WHERE qid = %d", $queue->size, $queue->title, $queue->subqueue_title, $queue->link, $queue->link_remove, $queue->owner, $queue->show_in_links, $queue->show_in_tab, $queue->show_in_ui, $queue->i18n, $queue->reverse, $queue->reference, $queue->qid);
     db_query("DELETE FROM {nodequeue_roles} WHERE qid = %d", $queue->qid);
     db_query("DELETE FROM {nodequeue_types} WHERE qid = %d", $queue->qid);
+    //Set the subqueue title if it changed
+    db_query("UPDATE {nodequeue_subqueue} set title = '%s' WHERE qid = %d AND reference = %d", $queue->title, $queue->qid, $queue->qid);
   }

   if (is_array($queue->roles)) {

#20

joachim - October 27, 2009 - 00:22

It sounds more and more like the whole subqueue system is a mistake -- if it breaks regular, single queues and this hasn't been fixed in over a year :(

#21

ezra-g - October 27, 2009 - 00:33

@Joachim: Can you propose a better architecture? What are you talking about when you say "it breaks regular, single queues"?

This bug is actually relatively simple to fix, it seems there just hasn't been enough developer interest to fix it.

#22

joachim - October 27, 2009 - 00:37

What I mean by it breaking simple queues is the subject of this bug!

#23

ezra-g - October 27, 2009 - 01:25
Status:needs work» needs review

After taking a quick look at the code, I remembered that the hooks I described already exist -- Each queue owner should implement hook_nodequeue_form_submit() and update their subqueues appropriately.

This four line patch patch does this on behalf of nodequeue.module and in my testing resolves the bug. Smartqueue modules such as Smartqueue_users and smartqueue_og should implement this hook as well.

To fix this completely for the Nodequeue package we'll want to implement this hook for the included smartqueue (taxonomy) module, with an efficient single query to update all of its subqueues, taking into account string replacement.

Perhaps someone with an interest in this bug can follow the model here and finish up the patch?

Marking as NR so folks can confirm this fixes the problem in the simpler case.

AttachmentSize
281040.patch 1.3 KB

#24

joachim - November 4, 2009 - 09:22

Without patch:

- go to admin/content/nodequeue
- edit a queue
- change name and save
- go to node/NID/nodequeue for a node that's eligible for this queue
- the name has not changed.

With the patch:
- repeat same steps
- name at node/NID/nodequeue has changed.

Patch works for me :)
+1 and thanks ezra!

#25

ezra-g - November 5, 2009 - 02:18
Status:needs review» fixed

Thanks for the testing. This is committed!

#26

joachim - November 5, 2009 - 15:15
Version:6.x-2.4» 5.x-2.x-dev
Status:fixed» patch (to be ported)

Could do with a port to D5.
Will see if I have the time but can't promise anything :(

 
 

Drupal is a registered trademark of Dries Buytaert.