There's a hook_workflow($op = 'pre') that allows modules to object to state changes. The workflow tab does not call this hook, it determines the target states based on transitions and roles.

I can provide a patch if desired.

CommentFileSizeAuthor
#9 155547.patch1.19 KBNancyDru
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

traxer’s picture

Title: Workflow tab should call hook_workflow($op = 'pre') to determine allowable transitions » Workflow tab should call hook_workflow($op = 'transition pre') to determine allowable transitions

It's actually hook_workflow($op = 'transition pre').

Bastlynn’s picture

Status: Active » Closed (won't fix)

Hi,

With the release of Drupal 7, Drupal 5 is no longer receiving security updates, reviews, or development from many contributed modules. Since 5 is now considered a depreciated version, you really should seriously look into upgrading to Drupal 6 or 7. The newer versions of Drupal work better, have more support, and will be safer (literally! security patches!) for your website. We are currently working on a new release for Workflow to Drupal 7. In light of that, further support for Drupal 5 issues is infeasible at the moment. Please consider upgrading to Drupal 6 or 7 in the near future - you'll be glad you did.

- Bastlynn

traxer’s picture

Version: 5.x-1.1 » 7.x-1.x-dev
Status: Closed (won't fix) » Active

Don't you want the feature in the next release?

Bastlynn’s picture

Status: Active » Needs work

Yep, but I needed to cut the issue queue down to size first. All the stuff closed in the first review of the issue queue will get looked at eventually but under the rules of triage I had to prioritize. Thankfully, having a pulse attached to the ticket bumps it up the priority list quite a lot. :)

Thanks for re-upping this one, I'll take a look at it.

Bastlynn’s picture

If you have a patch that will let me get to this issue much more quickly.

Bastlynn’s picture

Status: Needs work » Closed (works as designed)

Taking a closer look at this, I don't think that this hook does quite what you think it does. It's designed to check just before executing a transition if there are settings that need to be changed, not to provide a list of available transitions from the state the node is in. The current way of gathering that information is correct. The hook is being called during submission from the tab form as a part of the node transition, so no further work needs to be done on this.

jdwfly’s picture

Title: Workflow tab should call hook_workflow($op = 'transition pre') to determine allowable transitions » Workflow tab should call hook_workflow($op = 'transition pre') to allow vetos
Category: feature » bug
Status: Closed (works as designed) » Needs work

I thought hook_workflow was designed to allow even more customization and flexibility. I am using it to further restrict who can transition than just by role. If this hook is not being called on the workflow tab then I would label this as a bug.

This comes straight from workflow.module (6.x I know, but it is what I am using).

case 'transition pre':
      // The workflow module does nothing during this operation.
      // But your module's implementation of the workflow hook could
      // return FALSE here and veto the transition.
      break;

That is exactly how I am using the 'transition pre' $op. I went ahead and did a little more digging and found this code in workflow.pages.inc

/**
 * Submit handler for the form on the workflow tab.
 *
 * @see workflow_tab_form
 */
function workflow_tab_form_submit($form, &$form_state) {
  // The entire node object was stashed in the form.
  $node = $form_state['values']['node'];
  $node->workflow = $form_state['values']['workflow'];
  $node->workflow_comment = $form_state['values']['workflow_comment'];
  $node->workflow_scheduled = $form_state['values']['workflow_scheduled'];
  $node->workflow_scheduled_date = $form_state['values']['workflow_scheduled_date'];
  $node->workflow_scheduled_hour = $form_state['values']['workflow_scheduled_hour'];

  // Call node_save() to make sure any handlers that use the
  // new workflow values will see them.
  node_save($node);

  $form_state['redirect'] = 'node/' . $node->nid;
}

So as far as I can tell it is not calling the hook at all. This then allows for users in my case to bypass the extra authorization code I wrote in my custom module by just simply using the workflow tab. All that really needs to be done is to invoke the hook and if it returns false then it needs to exit without transitioning the node. Simple fix in my opinion and I'd get a patch up there but I am using 6.x so I am not sure how different it would be.

NancyDru’s picture

This code seems to do what jdwfly wants. If the workflow is very complex or a lot of hooks respond, it could take a while.

function workflow_tab_form($form_id, $form_state, $node, $wid, $states, $current) {
  $form['#tab'] = TRUE;
  $choices = workflow_field_choices($node);
  $min = $states[$current] == t('(creation)') ? 1 : 2;

  // Invoke a callback indicating a transition is about to occur. Modules
  // may veto the transition by returning FALSE.
  // In this case, the transition is not about to occur, but we want to
  // know if it should be dropped from the list of choices.
  foreach ($choices as $sid => $name) {
    if ($current == $sid) {
      continue;
    }
    $result = module_invoke_all('workflow', 'transition pre', $current, $sid, $node);
    // Stop if a module says so.
    if (in_array(FALSE, $result)) {
      // if vetoed, remove from list.
      unset($choices[$sid]);
    }
  }
NancyDru’s picture

Status: Needs work » Needs review
FileSize
1.19 KB

Here it is in patch form.

NancyDru’s picture

Assigned: Unassigned » NancyDru
Status: Needs review » Fixed

Committed

NancyDru’s picture

Status: Fixed » Closed (fixed)

Included in 7.x-1.1-beta2.