Not applying access by user and role settings correctly

I experienced some strange behavior from 7.x-1.3 and reverted back to 7.x-1.2. I tried your patch and it failed for me with this output:
--- Failed ---
sites\all\modules\workbench_access\workbench_access.module (Cannot apply hunk @@ 8 )

@pcoffey: Did you experience this sort of thing?

Create content = limited # of groups shown
Edit content = message displayed "Page 1 is assigned to the Administration editorial group(s), which may not be changed."

I upgraded from 7.x-1.2+10-dev to 7.x-1.3 and, without modifying the existing Workbench Access configuration, user roles that originally had access to certain content types were being denied access to the corresponding node forms. I can confirm the patch in #1 fixes this issue for me!

So it appears that the behavior I experienced is unrelated to this issue. I'll open a separate issue report. I've found a related issue report. Thanks for the clarification @matt.elkins.

Adding other posted issues related to this patch.

Patch fixes my issues.

The patch resolved an issue I was having: users with the role Administrator were given workbench access to all taxonomy terms via the root of the tree, and the 7.x-1.3 update broke this access. The patch restores the expected behavior. Thank you!

I can confirm patch in #1 works as well.

The patch works for me too. The problem was preventing many (all?) non-admin users from editing content. I hope a new release goes out soon with this patch in it.

While this patch "fixes the problem," all it really does is roll back a previous patch that was included in 7.x-1.3: https://www.drupal.org/node/1959590

The issue identified in #1959590 needs to be solved to truly fix this.

I can confirm patch in #1 works.

What this needs are tests.

How can I help?

What I experienced on upgrade to version 1.3 of the module for a site running workbench access with a taxonomy based access tree was that:
- the users that had role based access settings no longer had any access.
- the users with access assigned to their respective account/user kept theirs.

On running the patch, the per role based access settings were restored.

I have not studied the code in play though.

Well, the issue is that this got in to 7.x.1.3 because none of the existing tests failed. So first we need a failing test -- without this patch -- and then we can prove that this patch fixes the problem.

Here's the root problem that needs to be addressed first:

Nothing in workbench_access.test checks the role assignment handing. We test workbench_access_user_section_save() and workbench_access_user_section_delete() but not the role-based equivalents.

That's why all current tests passed (and still do).

OK. I understand the issue now. Here's a patch that adds some clarity to the sections form by noting which sections are by user and which are by role.

This still needs tests, but should be fine for production.

Testbot seems to be having trouble, but here's a test that fails and proves that the bug is accurate.

And here's both patches combined. This has a working test case:

* Check that user with no sections cannot edit.
* Assign user to a section by role.
* User can edit
* Remove that section from the role.
* User cannot edit.

Added another UI tweak to disable the user edit form for items assigned by role.

The tests pass, but testbot is not behaving.

Once we have confirmation that this patch is working for people, I can roll a new release.

Minor UI tweak, but still the same patch.

Anyone care to review? Lack of reviews caused the bad 1.3 release.

I would like to test your patch, but I can't get it to apply. I could supply the output from my terminal but I'm not sure if this is the place for that?

@agentrickard The patch in #32 didn't apply cleanly, but it DID appear to solve this issue.

patching file tests/workbench_access.test
patching file workbench_access.admin.inc
Hunk #2 FAILED at 426.
1 out of 3 hunks FAILED -- saving rejects to file workbench_access.admin.inc.rej
patching file workbench_access.module
Hunk #1 succeeded at 821 (offset 3 lines).

This patch would be applied to the -dev release.

@agentrickard Patch works for me and applies clean to 7.x-1.x-dev

I think between that review, the new tests, and the initial reports on this patch, we can commit this.