Closed (fixed)
Project:
Nodequeue
Version:
7.x-2.x-dev
Component:
Code
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Reporter:
Created:
21 Jan 2008 at 12:09 UTC
Updated:
11 Sep 2009 at 21:00 UTC
Jump to comment: Most recent file
Comments
Comment #1
merlinofchaos commentedThis should be pretty easy. Patches welcome! =)
Comment #2
baldwinlouie commented+1
Comment #3
baldwinlouie commentedI am also needing a hook after a node is added/removed from a queue. Here is a quick patch I created with those hooks. Please review and feedback is appreciated. This is rolled against 5.x-2.2 branch.
Comment #4
ezra-g commentedBumping to 6.x. This would be a good feature.
Comment #5
ezra-g commentedThanks for the patch!
This looks good but the hook definitions should follow the naming convention nodequeue_api_...
Comment #6
baldwinlouie commentedI rerolled it with the naming convention suggested.
Comment #7
ezra-g commentedUpdating status. I hope to test this soon.
Comment #8
ezra-g commentedSorry about the delay in reviewing this. I was hoping to get it into the upcoming release, but it currently applies by putting the new code inside of the comments of an existing function definition :-&. Could you reroll with the diff -up option? This is why -up is listed as a best practice for creating patches ;)
Comment #9
baldwinlouie commented@ezra-g
Here is a re-roll with -up option. The patch is currently for the D5 branch. Let me know if you want a roll for the D6 branch as well.
Comment #10
baldwinlouie commented@ezra-g, one more re-roll. I realized my local copy of nodequeue was not the D5 dev branch. This re-roll uses the D5 dev branch.
Comment #11
Scott Reynolds commentedMay i suggest this be one hook not two. hook_nodequeue_api($op, $subqueue, $nid). Where $op would be 'add' or 'remove'. Also, no need for the for loops. just do module_invoke_all('nodequeue_api', 'add', $subqueue, $nid)
Comment #12
daniboy commentedI agree with Scott, using a single hook similar to how hook_nodeapi() works would prevent the API from becoming too big and hopefully help in making this more extensible in the future (A quick idea off the top of my head - add another hook call with $op = "pre add" so that other modules can allow or disallow adding a node to a subqueue)
Comment #13
baldwinlouie commentedpoints taken....I'll reroll with those suggestions.
Comment #14
dave reidKeep in mind with Drupal 7 we are eliminating the all-encompassing hook_something_api($op) type hooks in favor of splitting them out. Maybe we should keep the hook separate and make it easier to upgrade modules to Drupal 7?
Comment #15
cyberswat commentedHere's another attempt at a patch.
Comment #16
ezra-g commentedMarking as needs review.
Comment #17
robloachDon't think hook_nodequeue_swap would be used at all, but I can see why add/remove would be nice. Here's an updated patch with some more documentation.
Comment #18
cyberswat commentedIn our systems we use a custom type of caching that stores the output of pages. The swap hook is used to notify when delta's have changed so that we can rebuild that part of the cache. I think it's a pretty important item to have in place.
Comment #19
robloachAh, then making sure you get notified when the position has switches is important ;-) .
Comment #20
cyberswat commentedThis patch builds on what was done in #17 by adding hook_nodequeue_swap back in with documentation
Comment #21
ezra-g commentedThis is committed, minus the nodequeue.api.php. We already have a readme file and a documentation page on drupal.org. @Cyberswat: Could you update the documentation page with these new functions please :)?
Thanks!
Comment #22
damienmckennaThanks for the work on this, am looking forward to the next version :)
Comment #23
cyberswat commented@ezra-g: I added a Nodequeue API section to the documentation page. I seriously suck at documentation so feel free to revise :) Thanks for getting these hooks committed.
Comment #24
dave reid+1 for nodequeue.api.php. It's the new standard for documenting hooks.