workflow_field_choices() has inconsistent result [#2037531]

workflow_field_choices($node) returns an array (if there are any choices) or FALSE otherwise.
This is not very consistent, and makes the code to process this result longer and harder to read. Beste practice is to return an empty array if there are no results. (After all, it is not an error.)
E.g., in the code count() is used: count(FALSE) == 1, and count(array()) == 0.
Also, the result is parsed in several ways, which is confusing to me.

3 files are using this function. Two of them need modification.
If you are interested, I can provide a patch.

Comment	File	Size	Author
#3	workflow_2037531_choices2.patch	483 bytes	johnv

Comments

Comment #1

johnv

Dutch

commented 8 July 2013 at 22:08

File workflow_actions.module seems ok:

function workflow_select_next_state_action($node, $context) {
  ...
  62   // Get the node's new state.
  63   $choices = workflow_field_choices($node);
  64   foreach ($choices as $sid => $name) {
         ...
  73   }

I haven't figured workflow.pages.inc out: line 128 seems wrong to me.

function workflow_tab_form($form_id, $form_state, $node, $wid, $states, $current) {
 124   $form['#tab'] = TRUE;
 125   $choices = workflow_field_choices($node);
 126   $min = $states[$current] == t('(creation)') ? 1 : 2;
 127   // Only build form if user has possible target state(s).
 128   if (count($choices) >= $min) {

File workflow.module needs some more love:

function workflow_field_choices($node, $force = FALSE) {
  global $user;
-  $choices = FALSE;
+  $choices = array();
...
        if (!in_array(FALSE, $result)) {
          // If not vetoed, add to list.
          $choices[$transition->state_id] = check_plain(t($transition->state_name));
        }

Comment #2

nancydru

she/her/hers

English

Boston

commented 10 July 2013 at 12:06

Line 128 (your number, 184 in my code) is to prevent a workflow change form if there are no states to switch to. That line has been there forever, even in the 6.x branch.

I have made the third change.