Add to queue from Node edit form

Forgot to say, current patch is against 6.x-2.0-rc1 ... will be brought up to date when feedback is received.

This patch basically removes and replaces all of nodequeue.module in one giant hunk. I tried to apply it to nodequeue 6 rc1 as you specified, but got the message, "Reversed (or previously applied) patch detected!"

I'd like to give some feedback on the patch and am willing to review it even though it's not against a CVS checkout, but perhaps you could reroll so it:

a)applies
b) at least has discrete hunks?

Thanks.

Hi ezra-g - we are finally using Subversion, which can create CVS-style patches of the sort you require. It is one of our tasks for this sprint (which is only one week long) to re-roll these patches in a Drupal-friendly format. We will post them on this issue.

Thanks! =)

hey guys,
please find patch file attached.
includes functionality to allow you to add nodes to a nodequeue on the node/add form and add/delete nodes from a nodequeue on the node edit form.
code still needs a fair bit of work so open to suggestions / contributions :)

Any news on this? Anyone able to review the patches? We're sort of on hold here. We want to re-roll them for dev, but not until we've had feedback, as there's little point in us re-rolling a patch the maintainers don't actually want.

Oops, status change might help!

I think this would be a good addition. It should be configurable on a per-queue basis whether this form is added to the node edit form.

I'm unable to apply this patch, and manually adding this implementation of hook_form_alter causes a whitescreen error for me, with "Trying to get property of non-object in ...includes/theme.inc on line 220"

I would be happy to help with rolling these patches. I'm in IRC as ezra_ or contact me and we can figure out another way to communicate.

$ patch -p0 < nodequeue_node_add_form_add_to_queue.patch
(Stripping trailing CRs from patch.)
patching file nodequeue.module
Hunk #1 succeeded at 213 (offset -2 lines).
Hunk #2 FAILED at 284.
Hunk #3 FAILED at 339.
Hunk #4 succeeded at 744 (offset 49 lines).
Hunk #5 succeeded at 761 (offset 49 lines).
Hunk #6 succeeded at 2082 (offset 90 lines).
Hunk #7 succeeded at 2127 (offset 89 lines).
Hunk #8 succeeded at 2666 (offset 133 lines).
patch unexpectedly ends in middle of line
patch: **** malformed patch at line 194:

Hi ezra-g,
I was able to apply this patch to 6.x-2.0-rc1 , did you accidentally try and apply the patch to HEAD?

Thanks
G.

Patches should only be submitted against -dev versions, never stable releases. Applying the patch to -rc1 doesn't do any good since there's no way to check it in.

To explain the patch:

We did it against -rc1 because we needed the functionality working for ourselves immediately on the most stable available version. We are aware it needs re-rolling against -dev before it can be checked in, but we do not want to do that if the feature won't be adopted, so we're asking for it to be checked first, to see if this will be something you want. If so, is the current implementation ok? If it is we will, or course, re-roll it against -dev.

If we re-roll it against -dev and you don't want it, then we wasted our time, so the -rc1 version is for you guys to check it as a feature add in principle and provide feedback on - then we can respond to your feedback and produce a proper patch against -dev you can commit. If we had created a patch against -dev in the first place, we would already have had to change it about four times prior to this post, just for review's sake, which would've been a waste of time! Patched against -rc1, we can tell you which specific version to use to *check* it and then tell us what we need to change before prorviding a patch on -dev. Hope that makes sense. =)

Changing title for clarity

This is just what I'm looking for but I'm not able to apply the patch...tells me it's malformed. Are others able to apply this (even against rc1)?

what's the status of this patch?
do we need to re-roll against dev?

Yes, it would be easier to read and evaluate if you re-rolled it. In the meantime, some feedback on how it could be improved.

- It would be more efficient to use nodequeue_load_queues_by_type than loading all queues at every node edit form using $queues = nodequeue_load_queues(nodequeue_get_all_qids(NULL));

- To correctly implement hook_nodequeue_form_submit you should use nodequeue_nodequeue_form_submit instead of just nodequeue_form_submit

hi ezra,
thanks for the feedback,
i've now re-rolled patch against dev snapshot =)

thanks
Graham.

I was just about to submit a feature request for this and luckily searched and found this issue first. This would be a great addition to the module!

Patch re-rolled against HEAD, fixed and updated to conform to Drupal coding standards. Now it should be easy for ezra to review and evaluate.

I also tested the feature and it works fine for me.

Thanks for re-rolling this! I took a quick look at this, and as I said in #7, "I think this would be a good addition. It should be configurable on a per-queue basis whether this form is added to the node edit form." -- This is an important detail so as to prevent cluttering the node edit form for folks who don't want this feature. Also, I see some direct queries to the nodequeue tables and I'd encourage you to make sure there's not an API function available.

Thanks!

I really could use this functionality, and I'd be willing to finish this up. I will work on adding the config options to allow the user to select whether the node is added automatically to the queue. As for API functions to replace direct calls to nodequeue tables, did you have anything specific in mind? This is my first foray into nodequeue, so I'm looking for API docs. Does this page look like it's accurate?

Thanks.

I changed the code a bit, because my node was added to all subqueues.
Replace all 3 instances off

Any change this will be included in the next version?

It could be but it's still marked as "needs work". Firstly, you'll need to apply the patch against HEAD, make your proposed changes, create a new patch and attach it here to include your changes.

Secondly, someone needs to write the functionality requested in #18 or this will stay as "needs work" forever. It is the maintainer's position that #18 is required before this will be considered, which is fair enough. So if you want this in Nodequeue and you have the time, it would be really cool if you could add #18 to the patch while you're at it. =)

(I don't have any time to look at this right now, sadly, or I would do it myself.)

New patch including the option to show a certain queueu on the node edit form, feedback appreciated

Tested on my one of my sites, but with only the queues defined so needs more testing

Marking as needs review since I didn't see that there was a patch here until just now.

subscribing, thx! would be wonderful if we can add the node to a certain queue directly from edit or submit form ..

Thanks for this patch! It looks like a good start but needs some work:

A) There is no access checking to make sure that the current user can manipulate the subqueues before they are added to the node edit form. See the nodequeue_api_queue_access and nodequeue_api_subqueue access functions

B) It seems like you're identifying the node form in an unconventional way. See an example from og.module or another module that alters the node edit form.

C) The first hunk fails and so the patch won't apply.

This seems like a good start though and I'd love to see this feature added. Thanks!

Let me look at this next week. Doesn't sound like it needs much more work. =)

its been a while since i've been in this issue, but i've re-rolled this patch with the following -

- added collapsible fieldset to the node edit form for the list of queues
- fixed B) in #26, identifying the node form in a more conventional way (same as OG basically)
- fixed A) added nodequeue_api_queue_access check in the hook_form_alter to see if the current user has the ability to manipulate all queues

I haven't added a call to nodequeue_api_subqueue_access, is this neccessary?

hopefully the patch should apply.

some irregularity caused patch to fail locally when i tried to re-apply it again. Re-rolled again, tested locally, seems to apply, this one should work.