The function "workflow_access_form_alter" doesn't follow the Drupal 6 API. I have fixed it and a minor error using a deprecated function (db_num_rows).

I attach my patch file. Please, review my code.

CommentFileSizeAuthor
workflow_access.patch1.07 KBLk2

Comments

jvandyk’s picture

Status: Needs review » Fixed

I removed db_num_rows() from workflow_access.

avpaderno’s picture

Status: Closed (fixed) » Fixed

In this case, the function needs to know if there are any results from the SQL query.
I would change the code in:

/**
 * Implementation of hook_form_alter().
 *
 * Add a "three dimensional" (state, role, permission type) configuration 
 * interface to the workflow edit form.
 */
function workflow_access_form_alter(&$form, $form_state, $form_id) {
  if ($form_id == 'workflow_edit_form') {
    // A list of roles available on the site and our 
    // special -1 role used to represent the node author.
    // TODO i think there is an API call for this -- user_roles() perhaps?
    $rids = array('-1' => t('author'));
    $result = db_query("SELECT r.rid, r.name FROM {role} r ORDER BY r.name");
    while ($obj = db_fetch_object($result)) {
      $rids[$obj->rid] = $obj->name;
    }
    
    $form['workflow_access'] = array('#type' => 'fieldset', 
      '#title' => t('Access control'),
      '#collapsible' => TRUE,
      '#tree' => TRUE,
    );
    
    // Add a table for every workflow state.
    $states = workflow_get_states($form['wid']['#value']);
    foreach ($states as $sid => $state) {
      
      if (workflow_is_system_state($state)) {
        continue; // no need to set perms on creation
      }
      
      $view = $update = $delete = array();
      
      $result = db_query("SELECT * from {workflow_access} where sid = %d", $sid);
      
      $count = 0;
      
      while ($access = db_fetch_object($result)) {
        $count++;
        if ($access->grant_view) {
          $view[] = $access->rid;
        }
        if ($access->grant_update) {
          $update[] = $access->rid;
        }
        if ($access->grant_delete) {
          $delete[] = $access->rid;
        }
      }
      
      // Allow view grants by default for anonymous and authenticated users, 
      // if no grants were set up earlier.
      if (!$count) {
        $view = array(DRUPAL_ANONYMOUS_RID, DRUPAL_AUTHENTICATED_RID);
      }
      
      // TODO better tables using a #theme function instead of direct #prefixing
      $form['workflow_access'][$sid] = array(
        '#type' => 'fieldset', 
        '#title' => $state,
        '#collapsible' => TRUE,
        '#tree' => TRUE,
      );
      $form['workflow_access'][$sid]['view'] = array(
        '#type' => 'checkboxes',
        '#options' => $rids,
        '#default_value' => $view,
        '#title' => t('Roles who can view posts in this state'),
        '#prefix' => '<table width="100%" style="border: 0;"><tbody style="border: 0;"><tr><td>',
      );
      $form['workflow_access'][$sid]['update'] = array(
        '#type' => 'checkboxes',
        '#options' => $rids,
        '#default_value' => $update,
        '#title' => t('Roles who can edit posts in this state'),
        '#prefix' => "</td><td>",
      );
      $form['workflow_access'][$sid]['delete'] = array(
        '#type' => 'checkboxes',
        '#options' => $rids,
        '#default_value' => $delete,
        '#title' => t('Roles who can delete posts in this state'),
        '#prefix' => "</td><td>",
        '#suffix' => "</td></tr></tbody></table>",
      );
    }
    
    // Place our block comfortably down the page.
    $form['submit']['#weight'] = 10;
    $form['#submit'] += array('workflow_access_form_submit' => array());
  }
}

Using then the new way to name the hook_form_alter() implementation, the function would be:

function workflow_access_form_workflow_edit_form_alter(&$form, $form_state) {
  // A list of roles available on the site and our 
  // special -1 role used to represent the node author.
  // TODO i think there is an API call for this -- user_roles() perhaps?
  $rids = array('-1' => t('author'));
  // ...
}

which would be called only for the form 'workflow_edit_form', and therefore there isn't the need to check for the $form_id which not even passed to the function.

jvandyk’s picture

Great suggestion. I also updated the #submit syntax to D6 syntax. Please do consider submitting proper patches.

Anonymous’s picture

Status: Fixed » Closed (fixed)

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

Status: Fixed » Closed (fixed)

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