#272298: Modular Access control for queue, subqueue viewing and manipulation must be forward ported to 6. Assigning to me.

CommentFileSizeAuthor
#9 nq6_ma3.patch14.16 KBezra-g
#4 nq6_modular_access.patch13.75 KBezra-g
#3 278609.patch11.29 KBezra-g

Comments

ezra-g’s picture

ezra-g commented

In the 5.x version we used 2 separate functions for modular queue, subqueue access. Since the D6 menu system simplifies our code in hook_menu, it seems like it would make sense to have nodequeue_queue_access be the central access checking function and accept an optional subqueue parameter, and call nodeuqueue_api_subqueue_access from there.

Is this an acceptable approach?

merlinofchaos’s picture

merlinofchaos commented

That sounds reasonable.

ezra-g’s picture

ezra-g commented
Status: Active » Needs work
StatusFileSize
new 278609.patch11.29 KB

This is in progress. Placing the preliminary patch here.

ezra-g’s picture

ezra-g commented
Status: Needs work » Needs review
StatusFileSize
new nq6_modular_access.patch13.75 KB

This is ready for review. Thanks!

cyberswat’s picture

cyberswat commented

Tested against DRUPAL-6-3 Here's my notes:

  1. Generated users, taxonomies and content with Devel
  2. Navigated to admin/content/nodequeue/add/smartqueue_author
    • Entered Title "Author Queue", Subqueue title "Subqueue Author", selected no roles, Queue size of 0, add text "+ author", remove text "- author", types = Page and Story
      • Generated error: warning: Missing argument 3 for form_set_value(), called in /var/www/drupal/sites/all/modules/nodequeue/authorqueue.module on line 48 and defined in /var/www/drupal/includes/form.inc on line 1296.
      • Created the Queue listing subqueues as 0 in table view
  3. Navigated to admin/content/nodequeue/add/nodequeue
    • Entered Title "Node Queue", Queue size of 0, add text "+ node", remove text "- node", types = Page and Story
      • Created the queue listing subqueues as 1 (Queue empty)
  4. Navigated to admin/content/nodequeue/add/smartqueue_taxonomy
    • Entered Title "Taxonomy Queue", Subqueue title "Subqueue Taxonomy", Checked one taxonomy, Queue size of 0, add text "+ taxonomy", remove text "- taxonomy", types = Page and Story
      • Created the queue listing subqueues as 0
  5. Generated more content with Devel
  6. Navigated to admin/content/nodequeue - Author queue now shows 23 subqueus, Node Queue shows 1 (Queue empty), Taxonomy Queue shows 19 Subqueues
  7. Viewing the Author Queue at admin/content/nodequeue/3/view
    • Clicked View next to first author (uid 18) redirected to admin/content/nodequeue/3/view/2
    • Typed the letter a to bring up entries in the autocomplete field. First node was nid 150, checked author on node 150 and noted that it was a different author (uid 36). Added this node to queue.
    • Repeated above item to add a second item from uid 40.
    • Clicked Save
  8. Navigated to node/150/nodequeue Table view listed Subqueue Author, Subqueue Taxonomy (repeated 5 times), and Node Queue. All Queues were empty. Added node to each queue.
  9. Navigated to admin/content/nodequeue and verified node 150 showed up in the appropriate queues.
  10. Created a role to test with
  11. Assigned perms "manipulate queues" to new role
  12. Navigated to admin/content/nodequeue/3/edit to edit the Author Queue - Selected the new role and hit Submit
  13. Assigned new role to test user and logged out then in as the test user. Generated the following errors in a loop many times:
    • # warning: Invalid argument supplied for foreach() in /var/www/drupal/sites/all/modules/nodequeue/nodequeue.module on line 1536.
    • # warning: array_keys() [function.array-keys]: The first argument should be an array in /var/www/drupal/sites/all/modules/nodequeue/nodequeue.module on line 1487.
    • # warning: Invalid argument supplied for foreach() in /var/www/drupal/sites/all/modules/nodequeue/nodequeue.module on line 1638.
    • # warning: Invalid argument supplied for foreach() in /var/www/drupal/sites/all/modules/nodequeue/nodequeue.module on line 1666.
  14. Hit reload, errors persist so stopped testing
ezra-g’s picture

ezra-g commented

It sounds like you tested smartqueue_author, which is included in the nodequeue.module download and is separate from smartqueue_users. If that's the case, this review isn't really related to this patch ;). Can you confirm which module you tested?

ezra-g’s picture

ezra-g commented

I should clarify: I'm not sure that this particular component of Nodequeue has seen much attention in the upgrade to D6 and therefore might reflect errors that are not a result of this patch -- I will follow through your excellent patch review steps and report what I find.

ezra-g’s picture

ezra-g commented

From step 2: "Generated error: warning: Missing argument 3 for form_set_value(), called in /var/www/drupal/sites/all/modules/nodequeue/authorqueue.module on line 48 and defined in /var/www/drupal/includes/form.inc on line 1296."

This error is in fact caused because smartqueue_author has not been updated to Drupal 6.

From step 13: These errors are unrelated to the patch, were reported in #167465: SQL warnings when user has manipulate queue permission and are fixed as of a few minutes ago.

From step 8: Can you confirm whether the links which are repeated are in fact links for different taxonomy subqueues? It might help to change the configuration of the name of these queues to use the wildcard %subqueue option so the links have different text.

Aside from those issues, your testing was smooth, correct?

Thanks again for this thorough review.

ezra-g’s picture

ezra-g commented
StatusFileSize
new nq6_ma3.patch14.16 KB

This revised patch incorporates a fix that in my testing resolves #277867: Nodequeue tab on nodes even though they are disabled, and provides a working forward-port.

ezra-g’s picture

ezra-g commented
Status: Needs review » Fixed

Committed.

Anonymous’s picture

Anonymous (not verified) commented
Status: Fixed » Closed (fixed)

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