I've created a preliminary test for Menu access control. Right now, its a modified clone of the existing Taxonomy access control. It's ugly, there's duplicate functionality, etc., but it is there.

Some ways it departs from the existing Taxonomy test:

  • It uses the "minimal" profile for testing. I'm pretty sure I had a reason for this, but I don't remember it now.
  • Instead of using the Pages content type, a new content type is generated.
  • An administer user is created and used to populate values through Drupal, like the created menu. See the "ugly" statement before. If there is an obvious way it should be changed, let me know.
  • I've removed the section on machine_name renaming of Taxonomy. I don't think you can rename a menu's machine name?
  • Converted most (all?) db_querys to abstract functions. Started with the new ones and then just did them all

The nodes are made with the following menu hierarchy.

/**
* Assign menu links for nodes.
* The hierarchy should four levels deep with two options per level
* A helpful diagram
*
* A
*   A1
*     A1a
*        A1a1
*        A1a2
*     A1b
*        A1b1
*        A1b2
*   A2
*     A2a
*        A2a1
*        A2a2
*     A2b
*        A2b1
*        A2b2
* B
*   B1
*     B1a
*        B1a1
*        B1a2
*     B1b
*        B1b1
*        B1b2
*   B2
*     B2a
*        B2a1
*        B2a2
*     B2b
*        B2b1
*        B2b2
*/

Currently, all of the tests appear to be passing. I'm fairly certain they all are passing properly.

I plan on extending the test to check for the issues in #1422868: Menu hierarchy - permissions descendancy and #1101638: Default menu form support. That said, it looks like my work schedule is going to be picking up soon, so I thought it'd be good to push up now. Patch in the next post.

Comments

jbylsma’s picture

StatusFileSize
new22.06 KB

Patch attached.

agentrickard’s picture

You are my new hero.

Two notes:

1) I'd prefer to use the standard profile in tests.

2) I'd love to have a web-based "install test hierarchy" like we have for taxonomy, though that would be tricky and likely require Devel to generate nodes.

jbylsma’s picture

Glad to help. Work (LMD) encouraged and allowed for a lot of time, which helped tremendously.

1) Thought about it more, I think I switched to minimal because I was getting my testing sea legs and I was running the test constantly. Switching back to standard should also eliminate having to create the content type and probably eliminate adding a new menu. I'll check it out.

2) If it uses the same mentality as Taxonomy's test (predetermined titles, mostly), this should be doable without Devel and without too much heavy lifting. I'll check it out x2.

agentrickard’s picture

We should also split this patch into two parts, I think.

Part one moves all the Taxonomy-based tests. That shouldn't be controversial and makes the second patch easier to review.

Also, function workbenchAccessNodeForm needs to be in the base class and usable by all test classes.

agentrickard’s picture

To split the tests properly, we should fix #1802522: Workbench Access without Taxonomy first. I want to take a stab at that this afternoon (US).

agentrickard’s picture

OK, that's out of the way. Now to re-roll this patch.

agentrickard’s picture

StatusFileSize
new20.29 KB

Part of the test is failing, but that's ok. I've started the refactor so we can get moving.

We shouldn't need to use the $admin_user, so let's work on removing that part.

agentrickard’s picture

StatusFileSize
new21.79 KB

OK, a refactor to get consistent behavior. Next to-do:

* Remove the admin user bit and form-based menu creation. That's not needed.

jbylsma’s picture

StatusFileSize
new14.43 KB

I've got the user and form-based removed. This is based on #7. Pretty sure its mostly working. Taking care of sick kid, so can't do much at the moment. Hopefully shouldn't be a difficult merge.

agentrickard’s picture

Status: Needs work » Needs review
StatusFileSize
new21.87 KB

OK, here is initial test coverage for Menu, separated from Taxonomy.

I think we can commit this and then work on the test coverage for the other issues.

jbylsma’s picture

Making more commit-ready

  • Remove debug
  • Code formatting (newlines)
  • Match Menu's description to Taxonomy's
  • Used createWorkbenchAccessNodes() in Menu
  • Made $items more specific $mlids
  • Removed now unnecessary $type variable
  • For consistency's sake, switched db_select calls to db_query

Non cumulative attached to ease diff

Status: Needs review » Needs work

The last submitted patch, 1878338-11-wa-menu-tests-non-cumulative.patch, failed testing.

agentrickard’s picture

StatusFileSize
new21.09 KB

A few minor line cleanups and removed the protected $admin_user variable, which we no longer use.

Committed, so we can use this to test other issues.

   fdec46f..1fbf6f3  7.x-1.x -> 7.x-1.x

Awesome work!

agentrickard’s picture

Status: Needs work » Fixed

Status: Fixed » Closed (fixed)

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

Anonymous’s picture

Issue summary: View changes

Forgot about converting db_query => db_select, added to original comment.