Add support for Rules, Actions, and Triggers

Forgot to set it to "needs review".

This update does the following:
- removes a drupal_set_message() debug statement I forgot to remove.
- defines the provided variable 'workbench_access_states' as an array of taxonomy terms instead of the type called 'unknown'.

I never tried with VBO (view bulk operations, right?).

The patches I made are with git diff, but I am using the 7.x-1.x branch and not master.
Should I be rolling the patches against master instead of 7.x-1.x?

I fixed the comments to be for workbench_access and not workbench_moderation and also added more description about what the functions do.

I noticed that I was using 'state' and 'states' everywhere. This is because workbench_moderation has 'states'.
I changed the code to use 'section' and 'sections' instead of 'state' and 'states'. This should make it more consistent. The downside is it will mean any existing rules from the previous patch might need to be re-created. A cache-clear may be necessary as well.

There is another area I forgot to address. Workbench access supports multiple kinds of access, namely taxonomy based AND menu based. What I wrote is for taxonomy based. Do I need to do anything differently for menu based and if so, what?

Edit: I noticed a little late that I was searching for 'state' with case-sensitive on.
I found one-more area to cleanup:
line #104 of workbench_access.rules.inc, 'label' => t("Workbench Access States"), should become 'label' => t("Workbench Access Sections"),
I will add the change to the next patch as I am sure there will be things to fix or add once this current patch gets reviewed..

The code in question crashes because the property workbench_access_id does not exist in the node object.

First, it may be a good idea to ensure that the said property exists before acting on it.
That is easy to do with a new patch.

Second, the real issue is that the said property does not exist.
It should exist, in fact, because it does not exist we have the following to scenarios that I can think up at the moment:
1) The $node variable that is passed to the function is not a valid node object.
2) The hook_node_load() for workbench_access is not being called and thereby is not making the appropraite changes to the node object.
3) There is a problem with the workbench_access hook_node_load() that is preventing it from altering the node object.
4) Your content type does not have workbench access enabled.

Without the workbench access id, the "contents current control section .. " condition will not function because it cannot tell if the function is associated with some workbench access id.

Now, lets assume #4 is your case.
This means that workbench access id is not enabled and therefore cannot ever match the condition.
The solution will be to test for the object property workbench_access_id and if it does not exist return FALSE.
Doing this will cause cases 1->3 to return FALSE and thus fail silently, but I suspect solving #4 is still the best way to go right now.

I will make a patch shortly.
Could you confirm if you have content that does not have "Enforce workbench access control" enabled?

Here is the patch with the change mentioned in #14.

Try turning on the rules debugging, which is found under admin/config/workflow/rules/settings.

Either select "Log debug information to the system log " or set "Show debug information" to "Always".
This might help identify where the problem may be.

good catch.

I am not sure about that.

To get the list of active options, workbench access does the following all over the place:

<?php 
  $active = workbench_access_get_active_tree();
  $tree = workbench_access_get_user_tree();
  workbench_access_build_tree($tree);
  $options += workbench_access_options($tree, $active['active']);
?>

So, in order to call workbench_access_options() to get what is active, it seems that
those 4 lines must be used every time.
I hope I am wrong with this, I would rather have fewer lines of code and certainly less redundancy.

Perhaps the argument should be to just do those 4 lines every time and not add an additional function whose name can confuse?

Hopefully one of the worbench_access developers can clarify this issue.

Oops, Wrong window/tab.

I re-rolled #20's patch.

However, I am wondering about what #22 and then @25 said.
Would it not be better to wait for #1561822: Add workbench_access_active_options() to be committed and then recreate this patch using the new function(s)?

#32, I've finally looked at the new native form support in dev branch.

In some aspects it can support the functionality added by this patch, but the rules support here adds much more than just form save actions.
These workbench_access rules allows for loading workbench access states for any kind of operation, such as during cron jobs, new account creation, or even page views.

The native forms support is just restricted to forms, so I believe rules support is still needed.

I rerolled the patch against the latest dev.
For simplicity purposes, I have applied the changes from #1561822: Add workbench_access_active_options().
Doing this is likely to make this patch fail during testing.
As soon as the said issue gets commited, I will re-test this patch.

(Drupal really needs "Blocked By" and "Blocks" issue states..)

For those testing this, be sure to apply the patch from #1561822: Add workbench_access_active_options() if it is not yet commited.

I thought I found a case in the last patch where I did not, but could, use workbench_access_active_options(). However, it turns out that particular case needed both $tree and $active variables.

I am putting my change below in text, I will wait to re-roll in case more bugs are found.

diff --git a/workbench_access.module b/workbench_access.module
index af59524..0a5cece 100644
--- a/workbench_access.module
+++ b/workbench_access.module
@@ -1871,7 +1871,10 @@ function workbench_access_node_operations($form = array(), $form_state = array()
   if (!user_access('batch update workbench access')) {
     return;
   }
-  $options = workbench_access_active_options();
+  $active = workbench_access_get_active_tree();
+  $tree = $active['tree'];
+  workbench_access_build_tree($tree);
+  $options = workbench_access_options($tree, $active['active']);
   if (empty($options)) {
     return;
   }

The problem with the patch being applied seems to be that for some reason the patch you supplied is trying to modify an existing workbench_access.rules.inc and this patch is supposed to create that file not modify it.

I have been using this patch for a while to perform accessibility validation on node save and change the workflow state to either Published or Needs Work depending on the pass/fail results of the validation check.

I came across three problems:
1) Using a rule that responds on workflow state change to call another rule that then changes the workflow state to something else gets can cause rules to not call the workflow state change response function a second time to prevent recursion.
- This is not really a problem, but it does fill the logs with warnings (and makes me feel worried).
- Nothing seems to be broken.

2) The quick and easy inline workbench_moderation change moderation state block seems to crash or do something weird while the rules module is running.
- I cannot tell if this is a problem with workbench_moderation, rules, the workbench_* rules patches I made, or some combination of the three.
- However, if I disable the inline block (found ~around line #1789 of workbench_moderation.module), then the rules never have issues.
- As far as I can tell is that the said bug is related to calling drupal_render().

3) When changing states and reacting to states using this rules code, I noticed occasional weirdness with access id's not always being properly reacted to.
- I managed to isolate the problem to how I am processing workbench_access and workbench_access_id.
- It seems that the workbench_access array is the current/previous access_id information and if, on node save, a new value is applied, there is now a workbench_access_id array with the new data. Which means I need to first test for workbench_access_id (in case this is a node_save) and then test for workbench_access.
- I created a patch, but I have since noticed that there have been quite a lot of changes to the workbench_access 7.x-1.x-dev branch. I am not certain if the stuff I used in this patch is still valid for the new code.

In summary:
#1 = design limitation or log irritation.
#2 = unknown problem with workbench_moderation + rules patches that affects workbench_access rules patches.
#3 = fixed mistake with processing workbench_access_id when available and when not available, processing workbench_access.

The attached patch contains the fix from #3 and should commit properly.

The workbench moderation patch is completely separate as the *.rules.inc file is on a per-module/per-directory basis. There is no inter-depedency between the workbench_access and the workbench_moderation rules patches.

The actual complete _lack_ of documentation has led me to do the following:
1) git clone (project_url) some_project
2) cd some_project
3) hack..hack...slash...save
4) gid add -A
5) git diff HEAD > ../some_project-version-1.patch
6) upload!

I always felt I might be doing it wrong, but it works...
That said, I still have a lot to learn about git.
I was not aware of this -N process you used.

looking at git-add man page:

       -N, --intent-to-add
           Record only the fact that the path will be added later. An entry for the path is placed in the index with no content. This is useful for, among other things, showing the unstaged content of such files
           with git diff and committing them with git commit -a.

It sounds like git -N might be the more proper way to generate a patch than what I am doing.
So, perhaps the following will work better:

1) git clone (project_url) some_project
2) cd some_project
3) hack..hack...slash...save
4) gid add -N
5) git diff HEAD > ../some_project-version-1.patch
6) upload!