Hi all,

I needed to create a nodequeue service for the Services module to expose available nodequeues to an external website.

Why? This is an extension of the work we're doing here and here - hope to post proper patches, as requested, today... The jist is this:

We want to be able to know which nodequeues (and nodequeue ids) are available on a remote Drupal installation so we can use the Services module's views.getView method (with the nodequeue id as an argument to be passed) to retrieve nodes based on which nodequeue they are in using the multiqueue view (work in progress in #302659).

Since I've created this module, I thought I might as well publish it here and see if you would like to package it with nodequeue. I'm not sure whether it should be part of the Services module's core services or part of nodequeue, but my gut feeling is it should be part of the nodequeue package, since nodequeue is not core. Code attached.

CommentFileSizeAuthor
#13 content_distribution_module.jpg49.36 KBbintoro
#13 content_distribution_module_after.jpg59.12 KBbintoro
#8 nodequeue_service.inc_.txt3.03 KBgreg.harvey
#8 nodequeue_service.info.txt199 bytesgreg.harvey
#8 nodequeue_service.module.txt1.3 KBgreg.harvey
#1 nodequeue_service.module.txt1.46 KBgreg.harvey
nodequeue_service.info.txt199 bytesgreg.harvey
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

greg.harvey’s picture

greg.harvey
Primary language English
CreditAttribution: greg.harvey commented
FileSize
nodequeue_service.module.txt1.46 KB

Oops - didn't upload the .module file. Attached! =)

ezra-g’s picture

ezra-g CreditAttribution: ezra-g commented

I love the idea of exposing Nodequeue over services. I'm not familiar with the services integration API at this point but it's on my radar and I will look into it. Depending on the complexity of implementing this, it might make sense to make nodequeue services its own contributed module which might eventually get integrated into nodequeue.

On quick review of this patch, I see that this will return all qids but not subqueue ids. In cases such as a single nodequeue, the qid might be sufficient because that queue only has one subqueue (in these cases, this is indicated by the sqid in the database). However, for other queues, such as taxonomy queues and user queues, we'll want return the subqueues as well. Possibly as an array of queues and their subqueues.

I think there is a range of functionality that one could implement with services, perhaps roughly mirroring many of nodequeue's internal functions. Do we want remote sites to be able to manipulate queues, not just view their contents? View information about the queues and positions of a particular node?

Great to see development here.

ezra-g’s picture

ezra-g CreditAttribution: ezra-g commented
Status: Needs review » Needs work
greg.harvey’s picture

greg.harvey
Primary language English
CreditAttribution: greg.harvey commented

I think it definitely makes sense it's a separate module (as presented) but it could sit in a sub-folder of the Nodequeue module (like, for example, Image Attach resides within Image). But if you'd prefer, I can create a separate module with its own project page? No problem. =)

Subqueues is a good point, over-looked because we don't need them at this time. While I agree there is potentially lots to do with this module, I think there is enough here to pitch in a 6.x-1.x-dev release of a nodequeue_service module for the community to start working on, don't you think?

Edit: Awww, hell - you're twisting my arm. It can't take that long to add subqueues - I'll do it and submit tomorrow. ;-)

greg.harvey’s picture

greg.harvey
Primary language English
CreditAttribution: greg.harvey commented

Ps - the services API will make all your suggestions above fairly trivial to implement. I love it! =)

I'm actually toying with the idea of creating a separate "noqueue.getQueueContent" method, rather than using views.getView as I do right now. views.getView is great, but I worry that others might struggle with the concept of having to get your queue details from one service and your content from another. It becomes a bit disparate.

What do you think?

merlinofchaos’s picture

merlinofchaos CreditAttribution: merlinofchaos commented

Yes, I'm in favor of this; this can live as a separate module within nodequeue that can be activated.

I do think subqueues should be dealt with before we actually commit anything; we can use this issue as the place to work until we think it's ready for consumption. That will limit the risk of confused users and will also prevent us from adding it to an actual release until it's ready to go.

greg.harvey’s picture

greg.harvey
Primary language English
CreditAttribution: greg.harvey commented

Cool. I'll try and add subqueue functionality tomorrow, nesting subqueues under their parents in the returned array. That ought to do it?

I've also been checking some recent changes the Services module maintainers made to my file service, which they've adopted in to the Services core, and I can see a few things the Services API can do which I didn't realise were possible, so I'll tidy this up a bit while I'm at it and re-post. Watch this space! =)

greg.harvey’s picture

greg.harvey
Primary language English
CreditAttribution: greg.harvey commented
Status: Needs work » Needs review
FileSize
nodequeue_service.module.txt1.3 KB
nodequeue_service.info.txt199 bytes
nodequeue_service.inc_.txt3.03 KB

Alright, I think we're there for an initial release. I tidied this up. Changes are:

 - using the '#file' option in the Services API to move the actual service functions to an include
 - now returns a separate keyed array containing *all* nodequeue data, not just the title (at the moment key = qid, but this may not be necessary any more - though it does no harm)
 - separate method for returning subqueues by qid (e.g. pass a qid as an argument and the method returns all subqueues)
- available methods are:
    - nodequeue.getQueues
    - nodequeue.getSubQueues
- Doxygen/Javadoc syntax comments for documentation

All files attached. Hopefully you can review and release this as a first attempt. =)

greg.harvey’s picture

greg.harvey
Primary language English
CreditAttribution: greg.harvey commented

Just bumping this. Don't want it to be forgotten. =)

SeedTreeLLC’s picture

SeedTreeLLC CreditAttribution: SeedTreeLLC commented

I just added services to a dev site and was looking at a tutorial that included this module. I did a search to find out if it duplicated or could be integrated with the services module... I guess it is in the works. Very handy functionality. I will test the patch on the dev site.

bintoro’s picture

bintoro CreditAttribution: bintoro commented

This Patch is incompatible with the 6.10 version of Drupal core.

ezra-g’s picture

ezra-g CreditAttribution: ezra-g commented

@bintoro: Can you explain what you mean? This is a patch to a contributed module, not core.

bintoro’s picture

bintoro CreditAttribution: bintoro commented
FileSize
content_distribution_module_after.jpg59.12 KB
content_distribution_module.jpg49.36 KB

I installed content distribution module which request nodequeue_service.
See the screenshot after I installed nodequeue_service.inc , nodequeue_service.info and nodequeue_service.module into folder modules/nodequeue.
I think it is the provide patch for the module request.

ezra-g’s picture

ezra-g CreditAttribution: ezra-g commented

@greg.harvey: I was finally able to test this -- I'm apologize for the delay. It looks good and is almost commit-able!

I notice that you check_plain the queue title and this results in escaped apostrophes such as in "ezra's queue" becoming " ezra's queue" -- Should we really be escaping this rather than letting the consumer escape this data? Looking to the system service that comes with services.module, data from variable_get isn't escaped and maybe we should follow that here. If we *are* going to escape this data, we should also escape the custom link text which could contain malicious user-entered as well. I'm interested in your thoughts here.

Also, I'd change the error message to be more general from "Nodequeue module unavailable" to something like "Invalid qid". I could make that change in a reroll.

Hope we can get this in soon!

ezra-g’s picture

ezra-g CreditAttribution: ezra-g commented

Also @bintoro, it looks like your nodequeue_service module is corrupt. Try replacing it with the one from http://drupal.org/node/310177#comment-1019252 .

greg.harvey’s picture

greg.harvey
Primary language English
CreditAttribution: greg.harvey commented

Cool - thanks for coming back. I've pinged a dev at Defaqto about this, as I'm stacked this week, but maybe he can pick it up. One of us will make changes asap... =)

Re: escaped characters, I agree there is a need to be consistent. I would follow the lead of the core services team - if the system module doesn't escape, neither will we, unless that changes. What do you think?

ezra-g’s picture

ezra-g CreditAttribution: ezra-g commented
Status: Needs review » Needs work

I think we should probably not escape this text.

tayzlor’s picture

tayzlor CreditAttribution: tayzlor commented

hey,
i was going to have a look at this since i worked a bit on it with greg. sorry, i havent got round to it yet, i'll try get round to taking a look asap,
subscribing so i can keep an eye on this.

greg.harvey’s picture

greg.harvey
Primary language English
CreditAttribution: greg.harvey commented

*bump*

This could do with finishing and committing... =)

ezra-g’s picture

ezra-g CreditAttribution: ezra-g commented
Status: Needs work » Fixed

Good point.

I removed the unnecessary access check and made some coding-style corrections. This is committed! Thanks so much for the feature and for your patience.

greg.harvey’s picture

greg.harvey
Primary language English
CreditAttribution: greg.harvey commented

Awesome, thanks! Sorry I wasn't able to devote as much time to it as I would've liked. Glad to hear it's in. Nice work.

Status: Fixed » Closed (fixed)

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

gavri’s picture

gavri CreditAttribution: gavri commented

Hi i saw that the nodequeue_service has loading capabilities, which is a great thing by itslef..
wouldn't it be great if the services provided also adding+editing capabilities for nodequeus and subqueues , swapping order etc. ?

Think of cool flash/flex interface that lets you drag results from views to the queue and also lets you swap nodes inside the queue.

greg.harvey’s picture

greg.harvey
Primary language English
CreditAttribution: greg.harvey commented

Please start a new feature request. This issue is closed. Thanks. =)