Closed (fixed)
Project:
Nodequeue
Version:
7.x-2.x-dev
Component:
Code
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
10 Jul 2015 at 13:16 UTC
Updated:
5 Dec 2019 at 19:29 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
danquah commentedComment #2
mian3010 commentedGreat patch.
It seems that the nodequeue-module has several ways of adding nodes to a queue. We have to handle when people call the
nodequeue_save_subqueue_order-function, as it manually clears queue and adds entries, in database. I have attached a revised patch.Comment #3
alansaviolobo commentedI have used this patch and tested the case of adding nodes from the ui. the patch works fine.
Comment #5
fizk commentedResubmit for testing.
Comment #10
fizk commentedShould apply to HEAD now.
Comment #11
fizk commentedComment #16
fizk commentedAny ideas why this error is showing up in the tests?
Comment #17
ramprassad commentedI also had a similar use case for implementing uniqueness in the queue. My patch also has a similar approach by having the 'unique_items' field but with much less code to evaluate uniqueness as I used the already available nodequeue_get_subqueue_position() to check if an item is available in the queue instead of a DB query. Also nodequeue_subqueue_add() will only be called only when the uniqueness check is passed.
Comment #18
ramprassad commentedComment #20
ramprassad commentedComment #24
danquah commentedThe way the test-nodequeue is being build in createNodequeue in http://cgit.drupalcode.org/nodequeue/tree/tests/nodequeue.test#n59 means that any defaults in the nodequeue_queue has to be duplicated there as well.
This updated patch should fix the notice.
Comment #25
danquah commentedSorry about that, I fixed the wrong patch :)
While the patch in #17 does use a bit less code, it misses the feature of clearing the nodequeue of duplicates as the uniqueness constraint is switched on.
I've attached an opdated version of the patch from #10 that with the same fix I applied in #24
Comment #26
dgale commentedI get this error when trying to apply the patch in #25 or #24
Comment #27
fizk commented@danquah Thanks, this works for me now and passes the tests.
It would be great to write a test for this new feature. If anyone has time to write a test, that would be great. Otherwise, I might commit this as-is.
Comment #28
fizk commentedComment #29
shaktikapplied patch in #25, its works fine...
attached screenshot
Comment #31
jenlamptonMerged into 7.x-2.x