GHOP #118: Create admin interface for the state machine API

http://code.google.com/p/google-highly-open-participation-drupal/issues/...

corsix, yes this a known bug of multistep forms in d5. it's odd.

I've been using this workaround in workflow-ng:

    //a formAPI bug lets default_value not work, so we have to care about this ourself
    if ($checked) {
      $form['checkbox']['#attributes']['checked'] = 'checked';
    }

I've just done a quick review, the code already looks good, I'll give it a deeper review in a few hours.

Just an idea:
Perhaps you can save some hidden values, if you use one value, which holds an array holding all values and serialize it. But implement it as you like.

* don't forget to add an upgrade procedure for creating your table
* don't call the admin menu point "State Machine API" - an API is for modules, not for users. Hm let's call it "Configurable State Machines" OR "Custom State Machines" - as you prefer.
* Please don't use "Node" in the UI, use "Content" instead like workflow-ng
* capitalizing - only capitalize the first letter of a sentence, so "Initial State" -> "Initial state"
* for states there is a difference between state == '' and state == NULL. That matters in particular for the init state, so better just set the init state to NULL or don't set it, if the init state is given as ''

apart from that, the patch looks already fine. In particular I like the 'review' step!

great!

* when updating a site I got this errors:
* warning: array_merge() [function.array-merge]: Argument #2 is not an array in /var/www/fago/web/drupal-5/update.php on line 309.
* warning: Invalid argument supplied for foreach() in /var/www/fago/web/drupal-5/update.php on line 542.

return array() in your update.

* codestyle at line 576 admin.inc: -> else {

* I would say it's more intuitive, to let the update case handle to create case too, instead of letting the create case handling the update case too. Soo I would suggest to let the "create submit handler" use the "update submit handler".

* You can set #validate to define another validation handler

* Please don't use arg(3) in a submit handler, instead put the value you need in a form value. E.g. you could put the sid_to_update in form element type value and so you could even use one submit handler for both forms - just detect the sid_to_update form_value. Again you can specify a form submit handler with #submit.

*when editing a state machine restricted to content types, the previously selected content types won't appear again.
*when editing the final button is also labelled "Create". This might be confusing for people

*first enable any states, go ahead, go back and deselect any states, go ahead and you get an error until there are states defined:
warning: implode() [function.implode]: Bad arguments. in /var/www/fago/web/drupal-5/sites/all/modules/workflow_ng_5/states/states_admin.inc on line 340.

*defining a user state machine works, but when I edit it I get errors:
warning: array_combine() [function.array-combine]: Both parameters should have at least 1 element in /var/www/fago/web/drupal-5/sites/all/modules/workflow_ng_5/states/states_admin.inc on line 617.

*In the custom state machine overview other available state machines, defined by other modules, are also listed. I think we should only list the custom defined state machines there. You could specify #module == 'States' to identify them.

regarding:
function states_admin_checkboxes_multistep_fix($checkboxes) {
foreach ($checkboxes as &$checkbox) {

the usual way to iterate over form elements is
foreach (element_children($elements) as $key) {
// work with $elements[$key];
}

@Tanc: Your help is welcome. Indeed, I had introduced a bug in the the workflow-ng development snapshot which prevent the action from working. I've fixed that now, so update workflow-ng.

you still had one arg(3) in the delete submit handler, but no big deal. I've just fixed that myself.

The patch works fine. Great that you have even removed the "unset" parameter. -> Committed to 5.x :)
-> Again, good work corsix! thanks!