Node access status missing when creating a node using Workflow Access and Workflow Field

I believe I'm running into this same problem. Here is my setup:

See attached screen shots for workflow modules enabled.

I have created a number of states (see screenshot for workflow-states)
The author is supposed to be able to transition from creation -> draft -> submitted (see screenshot for workflow-transitions)
The author is supposed to be able to edit when the piece of content is in the draft state (see screenshot for workflow-access)
The Applicant role is allowed to create the piece of content (see screenshot for sitewide-grant-application-permissions)
The Applicant role is allowed to view published content (see screenshot for sitewide-node-view-content-permissions)

A user with the Applicant role is allowed to create the grant application and save it as a draft status. But, then the user is not able to edit the piece of content. If I rebuild permissions, the user is then able to edit the piece of content.

I have tested this with the Workflow Access Priority set to -1 , 0 and 1; I encounter the same thing in all situations.

I run into the same issue. It is easy to reproduce: when user creates a node, the entry about node access doesn't get into {node_access} table and therefore node author is not able to edit the node, even though this is configured in workflow access module.

The fix is fairly simple and attached as a patch. The problem occurs because of the following logic in workflow_node_current_state():

if (!$sid || !empty($entity->is_new) || !empty($entity->in_preview)) {
  $sid = workflow_node_previous_state($entity, $entity_type, $field_name);
}

If the node is new, this code will always return previous state which is "Creation". Then Workflow Access tries to find granular permissions for "Creation" state with no success (because it is not controlled by Workflow module at all) and therefore doesn't grant any node accesses to anybody.

My patch fixes this behavior, because obviously it makes sense to get the previous state only in case if the new state is not available.

Great news @Toki, thanks for testing. If it works for you, then feel free to change the issue status to "Reviewed and tested by the community".

+++ b/workflow.module
@@ -707,7 +707,7 @@ function workflow_node_current_state($entity, $entity_type = 'node', &$field_nam
+  if (!$sid || (!$sid && (!empty($entity->is_new) || !empty($entity->in_preview)))) {

This logic makes no sense. Anding a FALSE statement with anything will make it FALSE. The only way for the second part of the OR condition to evaluate to TRUE is if !$sid evaluates to TRUE, in which case the first part of the OR statement will have already passed.

This patch is completely eliminating something that appears to need to be checked. The way the patch is written could be simplified to

if (!$sid) {

I'm just taking some notes here as I have a look at debugging this issue to see how easily I can resolve it. (Note: I may decide to just bypass the issue and use the "old fashioned" way of doing workflow instead of using Workflow Fields.)

I've added a debug statement in worfklow_access_worklow (screen shot attached).
I created a piece of content that has a workflow attached. You can see that workflow_access_workflow was called three times. (I haven't printed out any more debugging information, such as the $op. The comment says "This is only used for Workflow Node". So, I don't know if this *is* where the bug resides.) Then you get a message from workflow module that the state was set to draft. Finally you see the message that the piece of content was created.

Access is denied to edit the piece of content.

Now, I'll rebuild the permissions and see if my debug statement is triggered. There were no new messages in watchdog, but the user is now able to access the piece of content to edit it. At the moment, it's not looking too fruitful to pursue workflow_access_workflow further.

I guess it's worth taking a look at what's going on in workflow_node_current_state and compare what happens when using Workflow Field and Workflow Node to get an idea of whether that is where the problem resides. But, it seems to me that the Workflow Access module should be responsible for this and not the main Workflow module. So, I'm not convinced that the fix resides inside workflow.module.

@vegantriathlete you were right, there is really no sense in the second part of the "if" section in the previous patch. Many thanks for thorough review. I was trying to keep the existing behavior so hard that completely forgot to check if the logic can be simplified.

In the morning I debugged the code with Workflow Access and Workflow Field modules enabled. I found the reason why we should keep the previous node state if it's in the "preview" mode - it's necessary to do not let anybody to move between transitions before the node gets saved. But what I could not find is why we should return the previous state if the node has been just saved. A node gets is_new flag right before it is going to be saved the first time, and gets removed as soon as it saved to the database. So there is no way to manipulate with workflow transitions by using is_new state. I consider this condition as wrong one in workflow.module which should be removed.

I've just put a couple debug statements in workflow_node_current_state. When I create a new piece of content, the function is called twice. The first time it's called $sid is blank and there is no $entity->is_new or $entity->in_preview. The second time it's called $sid = 2 and $entity->is_new = 1.

I am going to run through this same scenario again using Workflow Node to see what happens.

I'm testing Workflow Node. I've added my workflow states to a content type and checked the box to display the workflow form on Post.

As soon as I go into node/add the function is called two times. $sid is blank and there is no $entity->is_new or $entity->in_preview.
When I save the node, there is one additional call to the function. $sid = 2 and entity->is_new = 1.

So, it's not looking like the issue is inside of this function and I don't think we should be applying the patch.

I'm going to take a closer look at when I use Workflow Field, since I didn't check the log file immediately after when I went into node/add. I just want to double check that things seem the same (at least with regards to the debug I've got in place right now).

Oh, I guess I forgot to explicitly mention that with Workflow Node, the author is allowed to edit the node and change the state.

I might as well test the next part of the transitions to see that access is controlled as expected (before I switch over to testing Workflow Field again).

When I edit the node again to change its state, I see another call to the function.
When I save my update to the state, there is another call to the function.
Finally, as expected (based on how I've set up the states), the author is not allowed to edit the node.

Workflow Node is working as expected with all the tests I have run.

Point of interest: After I have disabled and uninstalled Workflow Node and rebuilt permissions, the author is still not able to edit the node. This appears to be a separate bug that is unrelated to what I'm investigating here.

Of course the author can't edit the piece of content. I have not set the sitewide permissions to allow the role to edit own [content-type]. This is working as it's supposed to. There is no bug here.

I have enabled Workflow Field again.

As soon as I go to node/add, the function gets called two times. $sid is blank and there is no $entity->is_new or $entity->in_preview.
When I save the node, there is one more call to the function. $sid = 2 and $entity->is_new = 1.

I'm sticking with my assessment from #16 that the bug is not with the conditions in the if statement. I believe the problem lies elsewhere.

I'll need to do more digging to figure out where the problem is actually happening. I'll start by digging into workflow_node_previous_state.

I think I should pay attention to what {node_access} looks like when I use Workflow Node and compare that to what it looks like when I use Workflow Field.

I'm testing with Workflow Field right now.

When I go to node/add, there is no entry in {node_access}. This is expected behavior.
After I create the node (which gets assigned NID 15) {node_access} looks like:

MariaDB [carnegie]> select * from node_access;
+-----+-----+-------+------------+--------------+--------------+
| nid | gid | realm | grant_view | grant_update | grant_delete |
+-----+-----+-------+------------+--------------+--------------+
|   1 |   0 | all   |          1 |            0 |            0 |
|   2 |   0 | all   |          1 |            0 |            0 |
|   3 |   0 | all   |          1 |            0 |            0 |
|   4 |   0 | all   |          1 |            0 |            0 |
|  15 |   0 | all   |          1 |            0 |            0 |
+-----+-----+-------+------------+--------------+--------------+
5 rows in set (0.00 sec)

MariaDB [carnegie]>

I'm testing with Workflow Node right now.

When I go to node/add, there is no entry in {node_access}. This is expected behavior.
After I create the node (which is assigned NID 16) {node_access} looks like:

MariaDB [carnegie]> select * from node_access;
+-----+-----+-----------------------+------------+--------------+--------------+
| nid | gid | realm                 | grant_view | grant_update | grant_delete |
+-----+-----+-----------------------+------------+--------------+--------------+
|   1 |   0 | all                   |          1 |            0 |            0 |
|   2 |   0 | all                   |          1 |            0 |            0 |
|   3 |   0 | all                   |          1 |            0 |            0 |
|   4 |   0 | all                   |          1 |            0 |            0 |
|  16 |   6 | workflow_access       |          1 |            0 |            1 |
|  16 |   6 | workflow_access_owner |          1 |            1 |            1 |
+-----+-----+-----------------------+------------+--------------+--------------+
6 rows in set (0.00 sec)

MariaDB [carnegie]>

I'm going to look at what workflow_node_previous_state returns when I'm using Workflow Node. I've got a watchdog call in workflow_node_current_state right after the if statement that wraps the call to workflow_node_previous_state.

node/add logs two messages, both of which indicate that $sid == 1.
saving the node logs one more message which indicates that $sid == 2.

Now I'll run through the same test with Workflow Field.

node/add logs two message, both of which indicate that $sid == 1.
saving the node logs two more messages. the first comes from workflow itself (it's not my debug watchdog message) that indicates that the node was set to draft. the second is my debug statement that indicates that $sid == 1.
It does appear that the error is occurring somewhere in the call to workflow_node_previous_state.

One difference between the two tests is that with Workflow Node I am not able to choose the Draft state when I'm creating the node. It gets assigned to the Draft state when I save it. With Workflow Field I cannot choose the (creation) state. I must choose the Draft state when I'm saving the node. It seems likely that the logic is not taking into account that with Workflow Field the author is explicitly choosing a state.

Let me see what happens if I allow a transition from (creation) to more than one state.

I still have the same problem with Workflow Field.
Workflow Node continues to work as expected. If I created it in Draft state, the author is allowed to edit it. If I created it in the Submitted state, the author was not allowed to edit it.

I notice that I'm not seeing any watchdog messages from workflow when I'm creating the node through Workflow Node. I am seeing a message from workflow when I create the node with Workflow Field.

The method that generates the message is public function execute($force = FALSE), which is in the WorkflowTransition class inside of WorkflowTransition.php. I'm guessing the issue is that the method is being called (erroneously) when using Workflow Field.

Let me see what's happening in workflow_node_previous_state. I've got a watchdog debug statement in place.

I'm checking how Workflow Node works.

node/add generates one watchdog message with a blank $sid.
saving the node generates two watchdog messages. $sid == '' in the first one. $sid == 2 in the second one.

I've dropped in a couple more debug statements and will run through Workflow Node again.

node/add generates three messages (indicative of one call to the function). The third attempt finally assigns $sid = 1.
saving the node generates two more calls to the function.

I'll run through the test again with Workflow Field.

node/add generates the three messages (one call to the function). The third attempt finally assigns $sid = 1.
saving the node generates two more calls to the function. but, the calls are interrupted by the message from workflow itself. It looks like the first time through things are set properly. The second time through things don't go as they should.

I'm still suspecting that the method is being called erroneously. Or, the logic needs to recognize that the method is supposed to have been called and respond appropriately.

I need to dig into where the method is called and see what's going on differently for Workflow Node.

Does Workflow Node not even use the class?
Does Workflow Node use the class, but it recognizes not to call the method?

Here's the relevant part of the backtrace that shows where the execute method gets fired

    [0] => Array
        (
            [file] => /var/www/public_html/carnegie/sites/all/modules/contrib/workflow/includes/Form/WorkflowTransitionForm.php
            [line] => 648
            [function] => execute
            [class] => WorkflowTransition
            [type] => ->
        )

    [1] => Array
        (
            [file] => /var/www/public_html/carnegie/sites/all/modules/contrib/workflow/workflow_field/workflowfield.field.inc
            [line] => 206
            [function] => submitForm
            [class] => WorkflowTransitionForm
            [type] => ->
        )

    [2] => Array
        (
            [file] => /var/www/public_html/carnegie/sites/all/modules/contrib/workflow/workflow_field/workflowfield.field.inc
            [line] => 172
            [function] => workflowfield_field_update
        )

    [3] => Array
        (
            [file] => /var/www/public_html/carnegie/modules/field/field.attach.inc
            [line] => 209
            [function] => workflowfield_field_insert
        )

    [4] => Array
        (
            [file] => /var/www/public_html/carnegie/modules/field/field.attach.inc
            [line] => 938
            [function] => _field_invoke
        )

The key piece is that workflowfield_field_insert is being called when using Workflow Field.

Look at the watchdog entries when I'm using Workflow Field. I've got a watchdog entry inside of _field_invoke.

Compare that to what the watchdog entries look like when I'm using Workflow Node.

I don't think that it's a mistake for the workflowfield_field_insert to be fired. Going back to #29, I think that answer is that the method is not being called erroneously. I think the mistake is that the logic in workflow_node_previous_state needs to be more intelligent.

I've put a change in place that recognizes that there has been a workflow transition and I've placed the same three debug statements.

Now, I'll run through Workflow Field again and see how it compares to what I saw in #28 with Workflow Node.

node/add gives the same results.
Saving the node generates two more calls to the function. The second time through the function the $sid is set to 2, just as it was in #28.

I am able to edit the node without rebuilding permissions.

This looks like a good fix to me. I'll do more testing moving through various states. I have already noted in a bit of previous testing that the $entity contains $entity->original instead of $entity->workflow_transitions. I believe this is further evidence that I've nailed the solution.

My further testing looks good. The allowed transitions work as expected and the allowed access works as expected.

Patch attached.

Thanks for your elaborate updates and your testing and patch.
I'll review and commit it when I can dedicate time to the module again.

Works for me.

This also works for me.

#38 works for me too

Thanks all.
This is committed. In D8, the same problem may arise, but if so, it must be solved in a different way.