From 5deab4f3e3df5ba6a62009e17f29772125bcf70c Mon Sep 17 00:00:00 2001
From: Jordan Dukart <jordan@discoverygarden.ca>
Date: Tue, 16 Apr 2013 13:25:58 -0300
Subject: [PATCH 1/6] Update to ingest_steps to add the callback type and logic
 associated with that.

---
 includes/ingest.form.inc | 187 ++++++++++++++++++++++++++++++++++++---
 islandora.api.php        |  33 +++++--
 2 files changed, 202 insertions(+), 18 deletions(-)

diff --git a/includes/ingest.form.inc b/includes/ingest.form.inc
index db031dc0..9a6c2b9c 100644
--- a/includes/ingest.form.inc
+++ b/includes/ingest.form.inc
@@ -188,6 +188,8 @@ function islandora_ingest_form_get_next_step_id(array &$form_state) {
  */
 function islandora_ingest_form_get_previous_step_id(array &$form_state) {
   $step_id = islandora_ingest_form_get_current_step_id($form_state);
+  $step = islandora_ingest_form_get_step($form_state, $step_id);
+
   $step_ids = array_keys(islandora_ingest_form_get_steps($form_state));
   $index = array_search($step_id, $step_ids);
   if ($index !== FALSE && --$index >= 0) {
@@ -270,10 +272,10 @@ function islandora_ingest_get_approximate_steps(array $configuration) {
  * @return array
  *   The form definition of the current step.
  */
-function islandora_ingest_form_execute_step(array $form, array &$form_state) {
+function islandora_ingest_form_execute_step(array $form, array &$form_state, $step_id = NULL) {
   // Load any required files for the current step.
   islandora_ingest_form_load_include($form_state);
-  $step = islandora_ingest_form_get_step($form_state);
+  $step = isset($step_id) ? islandora_ingest_form_get_step($form_state) : islandora_ingest_form_get_step($form_state, $step_id);
   switch ($step['type']) {
     case 'form':
       $args = array($form, &$form_state);
@@ -284,10 +286,36 @@ function islandora_ingest_form_execute_step(array $form, array &$form_state) {
     case 'batch':
       // @todo Implement if possible.
       break;
+
+    case 'callback':
+      call_user_func_array($step['do_function']['function'], $step['do_function']['args']);
+      $next_step = islandora_ingest_form_get_next_step_id($form_state);
+      if ($next_step) {
+        islandora_ingest_form_increment_step($form_state);
+        return islandora_ingest_form_execute_step($form, $form_state, $next_step);
+      }
   }
   return array();
 }
 
+/**
+ * Handles occurances where a callback is executed to grab the next form type.
+ *
+ * @param array $form_state
+ *   The Drupal form state.
+ * @param array $step
+ *   Describes the current step.
+ *
+ * @return array
+ *   The stepified Drupal form definition for the given step.
+ */
+function islandora_ingest_callback_stepify(array &$form_state, $step) {
+  $next_step = islandora_ingest_form_get_step($form_state, $step);
+  $form = call_user_func_array($next_step['form_id'], $args);
+  islandora_ingest_form_increment_step($form_state);
+  return islandora_ingest_form_stepify($form, $form_state, $next_step);
+}
+
 /**
  * Append Prev/Next buttons submit/validation handlers etc.
  *
@@ -297,7 +325,7 @@ function islandora_ingest_form_execute_step(array $form, array &$form_state) {
  *   The Drupal form state.
  *
  * @return array
- *   The stepified drupal form definition for the given step.
+ *   The stepified Drupal form definition for the given step.
  */
 function islandora_ingest_form_stepify(array $form, array &$form_state, $step) {
   $first_step = islandora_ingest_form_on_first_step($form_state);
@@ -307,7 +335,6 @@ function islandora_ingest_form_stepify(array $form, array &$form_state, $step) {
 
   // Allow for a hook_form_FORM_ID_alter().
   drupal_alter(array('form_' . $step['form_id'], 'form'), $form, $form_state, $step['form_id']);
-
   return $form;
 }
 
@@ -336,10 +363,28 @@ function islandora_ingest_form_on_first_step(array &$form_state) {
  *   TRUE if we are currently on the last step, FALSE otherwise.
  */
 function islandora_ingest_form_on_last_step(array &$form_state) {
-  $step_id = islandora_ingest_form_get_current_step_id($form_state);
-  $step_ids = array_keys(islandora_ingest_form_get_steps($form_state));
-  $count = count($step_ids);
-  return array_search($step_id, $step_ids) == --$count;
+  $next_id = islandora_ingest_form_get_next_step_id($form_state);
+  if (!$next_id) {
+    return TRUE;
+  }
+  else {
+    $step_ids = array_keys(islandora_ingest_form_get_steps($form_state));
+    $next_step_num = array_search($next_id, $step_ids);
+    $callback_chain = TRUE;
+
+    while ($next_step_num < count($step_ids)) {
+      $next_step = islandora_ingest_form_get_step($form_state, $step_ids[$next_step_num]);
+      // Still is a form somewhere down our chain.
+      if ($next_step['type'] === 'form') {
+        $callback_chain = FALSE;
+        break;
+      }
+      else {
+        $next_step_num++;
+      }
+    }
+    return $callback_chain;
+  }
 }
 
 /**
@@ -358,9 +403,35 @@ function islandora_ingest_form_previous_button(array &$form_state) {
   // to undo whatever its submit handler did.
   $prev_step_id = islandora_ingest_form_get_previous_step_id($form_state);
   $prev_step = islandora_ingest_form_get_step($form_state, $prev_step_id);
-  $form_id = $prev_step['form_id'];
-  $submit_callback = $form_id . '_undo_submit';
-  $submit = function_exists($submit_callback) ? array($submit_callback, 'islandora_ingest_form_previous_submit') : array('islandora_ingest_form_undo_submit');
+
+  if ($prev_step['type'] === 'form') {
+    $form_id = $prev_step['form_id'];
+    $submit_callback = $form_id . '_undo_submit';
+    $submit = function_exists($submit_callback) ? array($submit_callback, 'islandora_ingest_form_previous_submit') : array('islandora_ingest_form_undo_submit');
+  }
+  elseif ($prev_step['type'] === 'callback') {
+    // Need to check if we have one or more callbacks at the start of our steps.
+    $steps = islandora_ingest_form_get_steps($form_state);
+    $keys = array_keys($steps);
+    $form = FALSE;
+    $current_step = array_search($prev_step_id, $keys);
+    while ($current_step >= 0 && !$form) {
+      $step = islandora_ingest_form_get_step($form_state, $keys[$current_step]);
+      if ($step['type'] === 'form') {
+        $form = TRUE;
+        break;
+      }
+      $current_step--;
+    }
+    // Only render a previous button if there a form somewhere behind us in our
+    // history chain.
+    if ($form) {
+      $submit = array('islandora_ingest_form_callback_previous');
+    }
+    else {
+      return;
+    }
+  }
   return array(
     '#type' => 'submit',
     '#value' => t('Previous'),
@@ -377,6 +448,38 @@ function islandora_ingest_form_previous_button(array &$form_state) {
   );
 }
 
+/**
+ * Handles the execution of "undoing" callbacks until a form type is reached.
+ *
+ * @param array $form
+ *   The Drupal form.
+ * @param array $form_state
+ *   The Drupal form state.
+ */
+function islandora_ingest_form_callback_previous(array $form, array &$form_state) {
+  // Arbitrarily move at least 1 back.
+  islandora_ingest_form_decrement_step($form_state);
+  $step_id = islandora_ingest_form_get_current_step_id($form_state);
+  $steps = islandora_ingest_form_get_steps($form_state);
+  $keys = array_keys($steps);
+  $current_step = array_search($step_id, $keys);
+
+  $form = FALSE;
+
+  while ($current_step >= 0 && !$form) {
+    $step = islandora_ingest_form_get_step($form_state, $keys[$current_step]);
+    islandora_ingest_form_decrement_step($form_state);
+    if ($step['type'] === 'callback') {
+      call_user_func_array($step['undo_function']['function'], $step['undo_function']['args']);
+    }
+    else {
+      $form = TRUE;
+    }
+    $current_step--;
+  }
+  $form_state['rebuild'] = TRUE;
+}
+
 /**
  * The submit handler for the ingest form previous button.
  *
@@ -446,8 +549,10 @@ function islandora_ingest_form_next_submit(array $form, array &$form_state) {
  */
 function islandora_ingest_form_stash_info(array &$form_state) {
   $storage = &islandora_ingest_form_get_step_storage($form_state);
-  $storage['values'] = $form_state['values'];
-  unset($form_state['values']);
+  if ($storage) {
+    $storage['values'] = $form_state['values'];
+    unset($form_state['values']);
+  }
 }
 
 /**
@@ -479,6 +584,15 @@ function islandora_ingest_form_ingest_button(array &$form_state) {
   $validate = function_exists($validate_callback) ? array($validate_callback) : NULL;
   $submit_callback = $form_id . '_submit';
   $submit = function_exists($submit_callback) ? array($submit_callback, 'islandora_ingest_form_submit') : array('islandora_ingest_form_submit');
+
+  // Because of callback shananigans we have to check if there are a chain of
+  // n callbacks that are weighted after the current step.
+  $next_id = islandora_ingest_form_get_next_step_id($form_state);
+
+  if ($next_id) {
+    $submit[] = 'islandora_ingest_form_callback_ingest';
+  }
+
   return array(
     '#type' => 'submit',
     '#name' => 'ingest',
@@ -513,6 +627,28 @@ function islandora_ingest_form_submit(array $form, array &$form_state) {
   }
 }
 
+/**
+ * Execute any callbacks that are weighted at end of the ingest steps on ingest.
+ *
+ * @param array $form
+ *   The Drupal form.
+ * @param array $form_state
+ *   The Drupal form state.
+ */
+function islandora_ingest_form_callback_ingest(array $form, array &$form_state) {
+  $next_id = islandora_ingest_form_get_next_step_id($form_state);
+  $steps = islandora_ingest_form_get_steps($form_state);
+  $keys = array_keys($steps);
+  $current_step = array_search($next_id, $keys);
+
+  // Execute our n chain of callbacks.
+  while ($current_step < count($steps)) {
+    $step = islandora_ingest_form_get_step($form_state, $keys[$current_step]);
+    call_user_func_array($step['do_function']['function'], $step['do_function']['args']);
+    $current_step++;
+  }
+}
+
 /**
  * Gets a reference to the stored NewFedoraObject's.
  *
@@ -635,6 +771,31 @@ function islandora_ingest_form_get_steps(array &$form_state) {
   foreach (islandora_build_hook_list(ISLANDORA_INGEST_STEP_HOOK, $shared_storage['models']) as $hook) {
     drupal_alter($hook, $steps, $form_state);
   }
+  // Check to see if the callback functions are implemented (existed). Remove
+  // any implementations from the steps that are incorrectly defined.
+  foreach ($steps as $key => $step) {
+    $watchdog = FALSE;
+    if ($step['type'] === 'callback') {
+      if (!isset($step['do_function']) || !isset($step['undo_function'])) {
+        $watchdog = TRUE;
+      }
+      elseif (!isset($step['do_function']['function']) || !isset($step['undo_function']['function'])) {
+        $watchdog = TRUE;
+      }
+
+      if ($watchdog) {
+        watchdog('islandora', 'The @module module is incorrectly implementing the
+          callback functionality of islandora_ingest_steps. The step @step has
+          not been included in the list of steps.', array(
+            '@module' => $step['module'],
+            '@step' => $key,
+          ), WATCHDOG_ERROR);
+        dd('errorsaursu');
+        unset($steps[$key]);
+      }
+    }
+  }
   uasort($steps, 'drupal_sort_weight');
+
   return $steps;
 }
diff --git a/islandora.api.php b/islandora.api.php
index d1d58f4f..fbe49be6 100644
--- a/islandora.api.php
+++ b/islandora.api.php
@@ -386,15 +386,26 @@ function hook_islandora_undeletable_datastreams(array $models) {
  * @return array
  *   An associative array of associative arrays which define each step in the
  *   ingest process.  Each step should consist of a unique name mapped to an
- *   array of properties (keys) including:
- *   - type: The type of step.  Currently, only "form" is implemented.
- *   - weight: The "weight" of this step--heavier(/"larger") values sink to the
- *     end of the process while smaller(/"lighter") values are executed first.
+ *   array of properties (keys) which take different paramaters based upon type:
+ *   - type: Type of step.  Only "form" and "callback" are implemented so far.
+ *   Required "form" type specific parameters:
  *   - form_id: The form building function to call to get the form structure
  *     for this step.
  *   - args: An array of arguments to pass to the form building function.
- *   And may optionally include both:
+ *   Required "callback" type specific parameters:
+ *   - do_function: An associate array including:
+ *       - 'function': The callback function to be called.
+ *       - 'args': An array of arguments to pass to the callback function.
+ *   - undo_function: An associate array including:
+ *       - 'function': The callback function to be called to reverse the
+ *          executed action in the ingest steps.
+ *       - 'args': An array of arguments to pass to the callback function.
+ *   Shared parameters between both types:
+ *   - weight: The "weight" of this step--heavier(/"larger") values sink to the
+ *     end of the process while smaller(/"lighter") values are executed first.
+ *   Both types may optionally include:
  *   - module: A module from which we want to load an include.
+ *   "Form type" may optionally include:
  *   - file: A file to include (relative to the module's path, including the
  *     file's extension).
  */
@@ -406,6 +417,18 @@ function hook_islandora_ingest_steps(array $form_state) {
       'form_id' => 'my_cool_form',
       'args' => array('arg_one', 'numero deux'),
     ),
+    'my_cool_step_callback' => array(
+      'type' => 'callback',
+      'weight' => 2,
+      'do_function' => array(
+        'function' => 'my_cool_execute_function',
+        'args' => array('arg_one', 'numero deux'),
+      ),
+      'undo_function' => array(
+        'function' => 'my_cool_undo_function',
+        'args' => array('arg_one', 'numero deux'),
+      ),
+    ),
   );
 }
 /**

From a462a4448cd50bb4d075ac97f4c146ace5940bab Mon Sep 17 00:00:00 2001
From: Jordan Dukart <jordan@discoverygarden.ca>
Date: Tue, 16 Apr 2013 16:48:37 -0300
Subject: [PATCH 2/6] Ingest functionality of the ingest form is now a
 callback.

---
 includes/ingest.form.inc | 22 ++++++++++++++--------
 islandora.api.php        |  2 +-
 islandora.module         | 18 ++++++++++++++++++
 3 files changed, 33 insertions(+), 9 deletions(-)

diff --git a/includes/ingest.form.inc b/includes/ingest.form.inc
index 9a6c2b9c..53c48639 100644
--- a/includes/ingest.form.inc
+++ b/includes/ingest.form.inc
@@ -288,7 +288,9 @@ function islandora_ingest_form_execute_step(array $form, array &$form_state, $st
       break;
 
     case 'callback':
-      call_user_func_array($step['do_function']['function'], $step['do_function']['args']);
+      $args = array(&$form_state);
+      $args = isset($step['do_function']['args']) ? array_merge($args, $step['do_function']['args']) : $args;
+      call_user_func_array($step['do_function']['function'], $args);
       $next_step = islandora_ingest_form_get_next_step_id($form_state);
       if ($next_step) {
         islandora_ingest_form_increment_step($form_state);
@@ -470,7 +472,9 @@ function islandora_ingest_form_callback_previous(array $form, array &$form_state
     $step = islandora_ingest_form_get_step($form_state, $keys[$current_step]);
     islandora_ingest_form_decrement_step($form_state);
     if ($step['type'] === 'callback') {
-      call_user_func_array($step['undo_function']['function'], $step['undo_function']['args']);
+      $args = array(&$form_state);
+      $args = isset($step['undo_function']['args']) ? array_merge($args, $step['undo_function']['args']) : $args;
+      call_user_func_array($step['undo_function']['function'], $args);
     }
     else {
       $form = TRUE;
@@ -583,7 +587,10 @@ function islandora_ingest_form_ingest_button(array &$form_state) {
   $validate_callback = $form_id . '_validate';
   $validate = function_exists($validate_callback) ? array($validate_callback) : NULL;
   $submit_callback = $form_id . '_submit';
-  $submit = function_exists($submit_callback) ? array($submit_callback, 'islandora_ingest_form_submit') : array('islandora_ingest_form_submit');
+  $submit = array();
+  if (function_exists($submit_callback)) {
+    $submit[] = $submit_callback;
+  }
 
   // Because of callback shananigans we have to check if there are a chain of
   // n callbacks that are weighted after the current step.
@@ -607,12 +614,10 @@ function islandora_ingest_form_ingest_button(array &$form_state) {
  *
  * Attempts to ingest every object built by the previous steps.
  *
- * @param array $form
- *   The Drupal form.
  * @param array $form_state
  *   The Drupal form state.
  */
-function islandora_ingest_form_submit(array $form, array &$form_state) {
+function islandora_ingest_form_ingest(array &$form_state) {
   foreach ($form_state['islandora']['objects'] as $object) {
     try {
       islandora_add_object($object);
@@ -644,7 +649,9 @@ function islandora_ingest_form_callback_ingest(array $form, array &$form_state)
   // Execute our n chain of callbacks.
   while ($current_step < count($steps)) {
     $step = islandora_ingest_form_get_step($form_state, $keys[$current_step]);
-    call_user_func_array($step['do_function']['function'], $step['do_function']['args']);
+    $args = array(&$form_state);
+    $args = isset($step['do_function']['args']) ? array_merge($args, $step['do_function']['args']) : $args;
+    call_user_func_array($step['do_function']['function'], $args);
     $current_step++;
   }
 }
@@ -790,7 +797,6 @@ function islandora_ingest_form_get_steps(array &$form_state) {
             '@module' => $step['module'],
             '@step' => $key,
           ), WATCHDOG_ERROR);
-        dd('errorsaursu');
         unset($steps[$key]);
       }
     }
diff --git a/islandora.api.php b/islandora.api.php
index fbe49be6..f6fd02a7 100644
--- a/islandora.api.php
+++ b/islandora.api.php
@@ -405,7 +405,7 @@ function hook_islandora_undeletable_datastreams(array $models) {
  *     end of the process while smaller(/"lighter") values are executed first.
  *   Both types may optionally include:
  *   - module: A module from which we want to load an include.
- *   "Form type" may optionally include:
+ *   "Form" type may optionally include:
  *   - file: A file to include (relative to the module's path, including the
  *     file's extension).
  */
diff --git a/islandora.module b/islandora.module
index ad9e569e..6f1d0144 100644
--- a/islandora.module
+++ b/islandora.module
@@ -1097,3 +1097,21 @@ function islandora_download_clip(AbstractObject $object) {
   }
   exit();
 }
+
+/**
+ * Implements hook_islandora_ingest_steps().
+ */
+function islandora_islandora_ingest_steps(&$form_state) {
+  return array(
+    'islandora_ingest_form_submit' => array(
+      'type' => 'callback',
+      'weight' => 50,
+      'do_function' => array(
+        'function' => 'islandora_ingest_form_ingest',
+      ),
+      'undo_function' => array(
+        'function' => 'islandora_ingest_form_undo_ingest',
+      ),
+    ),
+  );
+}

From d11075a1e695f3b424c8331e6a5419b3f6ffb62f Mon Sep 17 00:00:00 2001
From: Jordan Dukart <jordan@discoverygarden.ca>
Date: Thu, 18 Apr 2013 09:30:10 -0300
Subject: [PATCH 3/6] API documentation update, minor revisions to ingest form.

---
 includes/ingest.form.inc | 11 ++++++-----
 islandora.api.php        |  2 +-
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/includes/ingest.form.inc b/includes/ingest.form.inc
index 53c48639..8462deef 100644
--- a/includes/ingest.form.inc
+++ b/includes/ingest.form.inc
@@ -26,7 +26,8 @@
  *     be related.
  *   - models: An array of content model PIDs, to which the new object might
  *     subscribe
- *
+ *   - parent: The parent of the child to be ingested. This is needed for XACML
+ *     to correctly apply the parent's POLICY to children.
  * @return array
  *   The form definition of the current step.
  */
@@ -373,8 +374,8 @@ function islandora_ingest_form_on_last_step(array &$form_state) {
     $step_ids = array_keys(islandora_ingest_form_get_steps($form_state));
     $next_step_num = array_search($next_id, $step_ids);
     $callback_chain = TRUE;
-
-    while ($next_step_num < count($step_ids)) {
+    $num_steps = count($step_ids);
+    while ($next_step_num < $num_steps) {
       $next_step = islandora_ingest_form_get_step($form_state, $step_ids[$next_step_num]);
       // Still is a form somewhere down our chain.
       if ($next_step['type'] === 'form') {
@@ -645,9 +646,9 @@ function islandora_ingest_form_callback_ingest(array $form, array &$form_state)
   $steps = islandora_ingest_form_get_steps($form_state);
   $keys = array_keys($steps);
   $current_step = array_search($next_id, $keys);
-
+  $num_steps = count($steps);
   // Execute our n chain of callbacks.
-  while ($current_step < count($steps)) {
+  while ($current_step < $num_steps) {
     $step = islandora_ingest_form_get_step($form_state, $keys[$current_step]);
     $args = array(&$form_state);
     $args = isset($step['do_function']['args']) ? array_merge($args, $step['do_function']['args']) : $args;
diff --git a/islandora.api.php b/islandora.api.php
index f6fd02a7..17db59ad 100644
--- a/islandora.api.php
+++ b/islandora.api.php
@@ -304,7 +304,7 @@ function hook_islandora_datastream_modified(FedoraObject $object, FedoraDatastre
 }
 
 /**
- * Notify modules that the given datastream was ingested.
+ * Notify modules that the given datastream was modified.
  *
  * @see hook_islandora_datastream_modified()
  */

From 2d32249ab8e190a210fedfafdfb45205dfe38cd2 Mon Sep 17 00:00:00 2001
From: Jordan Dukart <jordan@discoverygarden.ca>
Date: Thu, 18 Apr 2013 10:28:06 -0300
Subject: [PATCH 4/6] Missing newline, crufty func.

---
 includes/ingest.form.inc | 19 +------------------
 1 file changed, 1 insertion(+), 18 deletions(-)

diff --git a/includes/ingest.form.inc b/includes/ingest.form.inc
index 8462deef..31cff4e5 100644
--- a/includes/ingest.form.inc
+++ b/includes/ingest.form.inc
@@ -28,6 +28,7 @@
  *     subscribe
  *   - parent: The parent of the child to be ingested. This is needed for XACML
  *     to correctly apply the parent's POLICY to children.
+ *
  * @return array
  *   The form definition of the current step.
  */
@@ -301,24 +302,6 @@ function islandora_ingest_form_execute_step(array $form, array &$form_state, $st
   return array();
 }
 
-/**
- * Handles occurances where a callback is executed to grab the next form type.
- *
- * @param array $form_state
- *   The Drupal form state.
- * @param array $step
- *   Describes the current step.
- *
- * @return array
- *   The stepified Drupal form definition for the given step.
- */
-function islandora_ingest_callback_stepify(array &$form_state, $step) {
-  $next_step = islandora_ingest_form_get_step($form_state, $step);
-  $form = call_user_func_array($next_step['form_id'], $args);
-  islandora_ingest_form_increment_step($form_state);
-  return islandora_ingest_form_stepify($form, $form_state, $next_step);
-}
-
 /**
  * Append Prev/Next buttons submit/validation handlers etc.
  *

From 49c489a83cfb18d8602556170f33f0eebb35ed7b Mon Sep 17 00:00:00 2001
From: Nigel Banks <nigel.g.banks@gmail.com>
Date: Thu, 18 Apr 2013 20:10:46 +0200
Subject: [PATCH 5/6] Code Review - Refactor and fixes.

---
 includes/ingest.form.inc | 336 ++++++++++++++++++---------------------
 islandora.module         |  18 ---
 2 files changed, 155 insertions(+), 199 deletions(-)

diff --git a/includes/ingest.form.inc b/includes/ingest.form.inc
index 31cff4e5..a94aaa30 100644
--- a/includes/ingest.form.inc
+++ b/includes/ingest.form.inc
@@ -135,6 +135,50 @@ function islandora_ingest_form_get_step(array &$form_state, $step_id = NULL) {
   return NULL;
 }
 
+/**
+ * Gets the next step.
+ *
+ * If the current step is not defined, its assumed to be the first step.
+ *
+ * @param array $form_state
+ *   The Drupal form state.
+ * @param string $step
+ *   The step relative to the result, if not provided the current step is used.
+ *
+ * @return string
+ *   The next step if found, NULL otherwise.
+ */
+function islandora_ingest_form_get_next_step(array &$form_state, array $step = NULL) {
+  $step = isset($step) ? $step : islandora_ingest_form_get_step($form_state);
+  $next_step_id = islandora_ingest_form_get_next_step_id($form_state, $step['id']);
+  if ($next_step_id) {
+    return islandora_ingest_form_get_step($form_state, $next_step_id);
+  }
+  return NULL;
+}
+
+/**
+ * Gets the next step.
+ *
+ * If the current step is not defined, its assumed to be the first step.
+ *
+ * @param array $form_state
+ *   The Drupal form state.
+ * @param string $step
+ *   The step relative to the result, if not provided the current step is used.
+ *
+ * @return string
+ *   The next step if found, NULL otherwise.
+ */
+function islandora_ingest_form_get_previous_step(array &$form_state, array $step = NULL) {
+  $step = isset($step) ? $step : islandora_ingest_form_get_step($form_state);
+  $next_step_id = islandora_ingest_form_get_previous_step_id($form_state, $step['id']);
+  if ($next_step_id) {
+    return islandora_ingest_form_get_step($form_state, $next_step_id);
+  }
+  return NULL;
+}
+
 /**
  * Gets the ID of the current step.
  *
@@ -162,12 +206,15 @@ function islandora_ingest_form_get_current_step_id(array &$form_state) {
  *
  * @param array $form_state
  *   The Drupal form state.
+ * @param string $step_id
+ *   The ID of the  step relative to the result, if not provided the current
+ *   step_id is used.
  *
  * @return string
  *   The next step ID if found, NULL otherwise.
  */
-function islandora_ingest_form_get_next_step_id(array &$form_state) {
-  $step_id = islandora_ingest_form_get_current_step_id($form_state);
+function islandora_ingest_form_get_next_step_id(array &$form_state, $step_id = NULL) {
+  $step_id = isset($step_id) ? $step_id : islandora_ingest_form_get_current_step_id($form_state);
   $step_ids = array_keys(islandora_ingest_form_get_steps($form_state));
   $index = array_search($step_id, $step_ids);
   $count = count($step_ids);
@@ -184,14 +231,15 @@ function islandora_ingest_form_get_next_step_id(array &$form_state) {
  *
  * @param array $form_state
  *   The Drupal form state.
+ * @param string $step_id
+ *   The ID of the step relative to the result, if not provided the current
+ *   step_id is used.
  *
  * @return string
  *   The previous step ID if found, NULL otherwise.
  */
-function islandora_ingest_form_get_previous_step_id(array &$form_state) {
-  $step_id = islandora_ingest_form_get_current_step_id($form_state);
-  $step = islandora_ingest_form_get_step($form_state, $step_id);
-
+function islandora_ingest_form_get_previous_step_id(array &$form_state, $step_id = NULL) {
+  $step_id = isset($step_id) ? $step_id : islandora_ingest_form_get_current_step_id($form_state);
   $step_ids = array_keys(islandora_ingest_form_get_steps($form_state));
   $index = array_search($step_id, $step_ids);
   if ($index !== FALSE && --$index >= 0) {
@@ -237,13 +285,15 @@ function islandora_ingest_form_decrement_step(array &$form_state) {
  * Build a list of steps given only configuration.
  *
  * XXX: This is used to give an indication of whether there are any steps for a
- *   given list of content models.
+ *   given configuration.
  *
  * @param array $configuration
  *   The list of key/value pairs of configuration.
  */
 function islandora_ingest_get_approximate_steps(array $configuration) {
   try {
+    // @todo, we need to expand the configuration before we can validate it?
+    // I think this need some thinking.
     islandora_ingest_form_validate_configuration($configuration);
   }
   catch (InvalidArgumentException $e) {
@@ -279,29 +329,72 @@ function islandora_ingest_form_execute_step(array $form, array &$form_state, $st
   islandora_ingest_form_load_include($form_state);
   $step = isset($step_id) ? islandora_ingest_form_get_step($form_state) : islandora_ingest_form_get_step($form_state, $step_id);
   switch ($step['type']) {
+    case 'callback':
+      // Execute all the consecutive callbacks, and move then attempt to process
+      // the next step.
+      islandora_ingest_form_execute_consecutive_callback_steps($form, $form_state, $step);
+      return islandora_ingest_form_execute_step($form, $form_state);
+
     case 'form':
-      $args = array($form, &$form_state);
-      $args = isset($step['args']) ? array_merge($args, $step['args']) : $args;
-      $form = call_user_func_array($step['form_id'], $args);
-      return islandora_ingest_form_stepify($form, $form_state, $step);
+      return islandora_ingest_form_execute_form_step($form, $form_state, $step);
 
     case 'batch':
       // @todo Implement if possible.
       break;
-
-    case 'callback':
-      $args = array(&$form_state);
-      $args = isset($step['do_function']['args']) ? array_merge($args, $step['do_function']['args']) : $args;
-      call_user_func_array($step['do_function']['function'], $args);
-      $next_step = islandora_ingest_form_get_next_step_id($form_state);
-      if ($next_step) {
-        islandora_ingest_form_increment_step($form_state);
-        return islandora_ingest_form_execute_step($form, $form_state, $next_step);
-      }
   }
   return array();
 }
 
+/**
+ * Assumes the given $step is a 'form' step.
+ */
+function islandora_ingest_form_execute_form_step(array $form, array &$form_state, array $step) {
+  $args = array($form, &$form_state);
+  $args = isset($step['args']) ? array_merge($args, $step['args']) : $args;
+  $form = call_user_func_array($step['form_id'], $args);
+  return islandora_ingest_form_stepify($form, $form_state, $step);
+}
+
+/**
+ * Assumes the given $step is a 'callback' step.
+ */
+function islandora_ingest_form_execute_consecutive_callback_steps(array $form, array &$form_state, array $step) {
+  do {
+    islandora_ingest_form_execute_callback_step($form, $form_state, $step);
+    islandora_ingest_form_increment_step($form_state);
+    $step = islandora_ingest_form_get_step($form_state);
+  } while (isset($step) && $step['type'] == 'callback');
+}
+
+/**
+ * Assumes the given $step is a 'callback' step.
+ */
+function islandora_ingest_form_execute_callback_step(array $form, array &$form_state, array $step) {
+  $args = array(&$form_state);
+  $args = isset($step['do_function']['args']) ? array_merge($args, $step['do_function']['args']) : $args;
+  call_user_func_array($step['do_function']['function'], $args);
+}
+
+/**
+ * Assumes the given $step is a 'callback' step.
+ */
+function islandora_ingest_form_undo_consecutive_callback_steps(array $form, array &$form_state, array $step) {
+  do {
+    islandora_ingest_form_undo_callback_step($form, $form_state, $step);
+    islandora_ingest_form_decrement_step($form_state);
+    $step = islandora_ingest_form_get_step($form_state);
+  } while (isset($step) && $step['type'] == 'callback');
+}
+
+/**
+ * Assumes the given $step is a 'callback' step.
+ */
+function islandora_ingest_form_undo_callback_step(array $form, array &$form_state, array $step) {
+  $args = array(&$form_state);
+  $args = isset($step['undo_function']['args']) ? array_merge($args, $step['undo_function']['args']) : $args;
+  call_user_func_array($step['undo_function']['function'], $args);
+}
+
 /**
  * Append Prev/Next buttons submit/validation handlers etc.
  *
@@ -314,18 +407,17 @@ function islandora_ingest_form_execute_step(array $form, array &$form_state, $st
  *   The stepified Drupal form definition for the given step.
  */
 function islandora_ingest_form_stepify(array $form, array &$form_state, $step) {
-  $first_step = islandora_ingest_form_on_first_step($form_state);
-  $last_step = islandora_ingest_form_on_last_step($form_state);
-  $form['prev'] = $first_step ? NULL : islandora_ingest_form_previous_button($form_state);
-  $form['next'] = $last_step ? islandora_ingest_form_ingest_button($form_state) : islandora_ingest_form_next_button($form_state);
-
+  $first_form_step = islandora_ingest_form_on_first_form_step($form_state);
+  $last_form_step = islandora_ingest_form_on_last_form_step($form_state);
+  $form['prev'] = $first_form_step ? NULL : islandora_ingest_form_previous_button($form_state);
+  $form['next'] = $last_form_step ? islandora_ingest_form_ingest_button($form_state) : islandora_ingest_form_next_button($form_state);
   // Allow for a hook_form_FORM_ID_alter().
   drupal_alter(array('form_' . $step['form_id'], 'form'), $form, $form_state, $step['form_id']);
   return $form;
 }
 
 /**
- * Checks if we are on the first step.
+ * Checks if we are on the first form step.
  *
  * @param array $form_state
  *   The Drupal form state.
@@ -333,14 +425,16 @@ function islandora_ingest_form_stepify(array $form, array &$form_state, $step) {
  * @return bool
  *   TRUE if we are currently on the first step, FALSE otherwise.
  */
-function islandora_ingest_form_on_first_step(array &$form_state) {
-  $step_id = islandora_ingest_form_get_current_step_id($form_state);
-  $step_ids = array_keys(islandora_ingest_form_get_steps($form_state));
-  return array_search($step_id, $step_ids) == 0;
+function islandora_ingest_form_on_first_form_step(array &$form_state) {
+  $step = NULL;
+  do {
+    $step = islandora_ingest_form_get_previous_step($form_state, $step);
+  } while (isset($step) && $step['type'] != 'form');
+  return !$step;
 }
 
 /**
- * Checks if we are on the last step.
+ * Checks if we are on the last form step.
  *
  * @param array $form_state
  *   The Drupal form state.
@@ -348,29 +442,12 @@ function islandora_ingest_form_on_first_step(array &$form_state) {
  * @return bool
  *   TRUE if we are currently on the last step, FALSE otherwise.
  */
-function islandora_ingest_form_on_last_step(array &$form_state) {
-  $next_id = islandora_ingest_form_get_next_step_id($form_state);
-  if (!$next_id) {
-    return TRUE;
-  }
-  else {
-    $step_ids = array_keys(islandora_ingest_form_get_steps($form_state));
-    $next_step_num = array_search($next_id, $step_ids);
-    $callback_chain = TRUE;
-    $num_steps = count($step_ids);
-    while ($next_step_num < $num_steps) {
-      $next_step = islandora_ingest_form_get_step($form_state, $step_ids[$next_step_num]);
-      // Still is a form somewhere down our chain.
-      if ($next_step['type'] === 'form') {
-        $callback_chain = FALSE;
-        break;
-      }
-      else {
-        $next_step_num++;
-      }
-    }
-    return $callback_chain;
-  }
+function islandora_ingest_form_on_last_form_step(array &$form_state) {
+  $step = NULL;
+  do {
+    $step = islandora_ingest_form_get_next_step($form_state, $step);
+  } while (isset($step) && $step['type'] != 'form');
+  return !$step;
 }
 
 /**
@@ -387,37 +464,10 @@ function islandora_ingest_form_on_last_step(array &$form_state) {
 function islandora_ingest_form_previous_button(array &$form_state) {
   // Before we move back to the previous step we should tell the previous step
   // to undo whatever its submit handler did.
-  $prev_step_id = islandora_ingest_form_get_previous_step_id($form_state);
-  $prev_step = islandora_ingest_form_get_step($form_state, $prev_step_id);
-
-  if ($prev_step['type'] === 'form') {
-    $form_id = $prev_step['form_id'];
-    $submit_callback = $form_id . '_undo_submit';
-    $submit = function_exists($submit_callback) ? array($submit_callback, 'islandora_ingest_form_previous_submit') : array('islandora_ingest_form_undo_submit');
-  }
-  elseif ($prev_step['type'] === 'callback') {
-    // Need to check if we have one or more callbacks at the start of our steps.
-    $steps = islandora_ingest_form_get_steps($form_state);
-    $keys = array_keys($steps);
-    $form = FALSE;
-    $current_step = array_search($prev_step_id, $keys);
-    while ($current_step >= 0 && !$form) {
-      $step = islandora_ingest_form_get_step($form_state, $keys[$current_step]);
-      if ($step['type'] === 'form') {
-        $form = TRUE;
-        break;
-      }
-      $current_step--;
-    }
-    // Only render a previous button if there a form somewhere behind us in our
-    // history chain.
-    if ($form) {
-      $submit = array('islandora_ingest_form_callback_previous');
-    }
-    else {
-      return;
-    }
-  }
+  $prev_step = islandora_ingest_form_get_previous_step($form_state);
+  $form_id = $prev_step['form_id'];
+  $submit_callback = $form_id . '_undo_submit';
+  $submit = function_exists($submit_callback) ? array($submit_callback, 'islandora_ingest_form_previous_submit') : array('islandora_ingest_form_previous_submit');
   return array(
     '#type' => 'submit',
     '#value' => t('Previous'),
@@ -434,40 +484,6 @@ function islandora_ingest_form_previous_button(array &$form_state) {
   );
 }
 
-/**
- * Handles the execution of "undoing" callbacks until a form type is reached.
- *
- * @param array $form
- *   The Drupal form.
- * @param array $form_state
- *   The Drupal form state.
- */
-function islandora_ingest_form_callback_previous(array $form, array &$form_state) {
-  // Arbitrarily move at least 1 back.
-  islandora_ingest_form_decrement_step($form_state);
-  $step_id = islandora_ingest_form_get_current_step_id($form_state);
-  $steps = islandora_ingest_form_get_steps($form_state);
-  $keys = array_keys($steps);
-  $current_step = array_search($step_id, $keys);
-
-  $form = FALSE;
-
-  while ($current_step >= 0 && !$form) {
-    $step = islandora_ingest_form_get_step($form_state, $keys[$current_step]);
-    islandora_ingest_form_decrement_step($form_state);
-    if ($step['type'] === 'callback') {
-      $args = array(&$form_state);
-      $args = isset($step['undo_function']['args']) ? array_merge($args, $step['undo_function']['args']) : $args;
-      call_user_func_array($step['undo_function']['function'], $args);
-    }
-    else {
-      $form = TRUE;
-    }
-    $current_step--;
-  }
-  $form_state['rebuild'] = TRUE;
-}
-
 /**
  * The submit handler for the ingest form previous button.
  *
@@ -482,6 +498,11 @@ function islandora_ingest_form_callback_previous(array $form, array &$form_state
  */
 function islandora_ingest_form_previous_submit(array $form, array &$form_state) {
   islandora_ingest_form_decrement_step($form_state);
+  $step = islandora_ingest_form_get_step($form_state);
+  // Undo all callbacks that occured after the previous step.
+  if ($step['type'] == 'callback') {
+    islandora_ingest_form_undo_consecutive_callback_steps($form, $form_state, $step);
+  }
   $form_state['rebuild'] = TRUE;
 }
 
@@ -571,19 +592,7 @@ function islandora_ingest_form_ingest_button(array &$form_state) {
   $validate_callback = $form_id . '_validate';
   $validate = function_exists($validate_callback) ? array($validate_callback) : NULL;
   $submit_callback = $form_id . '_submit';
-  $submit = array();
-  if (function_exists($submit_callback)) {
-    $submit[] = $submit_callback;
-  }
-
-  // Because of callback shananigans we have to check if there are a chain of
-  // n callbacks that are weighted after the current step.
-  $next_id = islandora_ingest_form_get_next_step_id($form_state);
-
-  if ($next_id) {
-    $submit[] = 'islandora_ingest_form_callback_ingest';
-  }
-
+  $submit = function_exists($submit_callback) ? array($submit_callback, 'islandora_ingest_form_submit') : array('islandora_ingest_form_submit');
   return array(
     '#type' => 'submit',
     '#name' => 'ingest',
@@ -601,7 +610,14 @@ function islandora_ingest_form_ingest_button(array &$form_state) {
  * @param array $form_state
  *   The Drupal form state.
  */
-function islandora_ingest_form_ingest(array &$form_state) {
+function islandora_ingest_form_submit(array $form, array &$form_state) {
+  // Execute any remainng callbacks.
+  islandora_ingest_form_increment_step($form_state);
+  $step = islandora_ingest_form_get_step($form_state);
+  if (isset($step) && $step['type'] == 'callback') {
+    islandora_ingest_form_execute_consecutive_callback_steps($form, $form_state, $step);
+  }
+  // Ingest the objects.
   foreach ($form_state['islandora']['objects'] as $object) {
     try {
       islandora_add_object($object);
@@ -616,30 +632,6 @@ function islandora_ingest_form_ingest(array &$form_state) {
   }
 }
 
-/**
- * Execute any callbacks that are weighted at end of the ingest steps on ingest.
- *
- * @param array $form
- *   The Drupal form.
- * @param array $form_state
- *   The Drupal form state.
- */
-function islandora_ingest_form_callback_ingest(array $form, array &$form_state) {
-  $next_id = islandora_ingest_form_get_next_step_id($form_state);
-  $steps = islandora_ingest_form_get_steps($form_state);
-  $keys = array_keys($steps);
-  $current_step = array_search($next_id, $keys);
-  $num_steps = count($steps);
-  // Execute our n chain of callbacks.
-  while ($current_step < $num_steps) {
-    $step = islandora_ingest_form_get_step($form_state, $keys[$current_step]);
-    $args = array(&$form_state);
-    $args = isset($step['do_function']['args']) ? array_merge($args, $step['do_function']['args']) : $args;
-    call_user_func_array($step['do_function']['function'], $args);
-    $current_step++;
-  }
-}
-
 /**
  * Gets a reference to the stored NewFedoraObject's.
  *
@@ -752,6 +744,8 @@ function islandora_ingest_form_get_steps(array &$form_state) {
   $shared_storage = &islandora_ingest_form_get_shared_storage($form_state);
   foreach (islandora_build_hook_list(ISLANDORA_INGEST_STEP_HOOK, $shared_storage['models']) as $hook) {
     // Required for pass by reference.
+    // @todo Change this around so that it isn't passed by reference, there
+    // Is an alter below that can handle that requirement.
     foreach (module_implements($hook) as $module) {
       $function = $module . '_' . $hook;
       $module_steps = (array) $function($form_state);
@@ -762,30 +756,10 @@ function islandora_ingest_form_get_steps(array &$form_state) {
   foreach (islandora_build_hook_list(ISLANDORA_INGEST_STEP_HOOK, $shared_storage['models']) as $hook) {
     drupal_alter($hook, $steps, $form_state);
   }
-  // Check to see if the callback functions are implemented (existed). Remove
-  // any implementations from the steps that are incorrectly defined.
-  foreach ($steps as $key => $step) {
-    $watchdog = FALSE;
-    if ($step['type'] === 'callback') {
-      if (!isset($step['do_function']) || !isset($step['undo_function'])) {
-        $watchdog = TRUE;
-      }
-      elseif (!isset($step['do_function']['function']) || !isset($step['undo_function']['function'])) {
-        $watchdog = TRUE;
-      }
-
-      if ($watchdog) {
-        watchdog('islandora', 'The @module module is incorrectly implementing the
-          callback functionality of islandora_ingest_steps. The step @step has
-          not been included in the list of steps.', array(
-            '@module' => $step['module'],
-            '@step' => $key,
-          ), WATCHDOG_ERROR);
-        unset($steps[$key]);
-      }
-    }
+  // Add any defaults.
+  foreach ($steps as $key => &$step) {
+    $step['id'] = $key;
   }
   uasort($steps, 'drupal_sort_weight');
-
   return $steps;
 }
diff --git a/islandora.module b/islandora.module
index 6f1d0144..ad9e569e 100644
--- a/islandora.module
+++ b/islandora.module
@@ -1097,21 +1097,3 @@ function islandora_download_clip(AbstractObject $object) {
   }
   exit();
 }
-
-/**
- * Implements hook_islandora_ingest_steps().
- */
-function islandora_islandora_ingest_steps(&$form_state) {
-  return array(
-    'islandora_ingest_form_submit' => array(
-      'type' => 'callback',
-      'weight' => 50,
-      'do_function' => array(
-        'function' => 'islandora_ingest_form_ingest',
-      ),
-      'undo_function' => array(
-        'function' => 'islandora_ingest_form_undo_ingest',
-      ),
-    ),
-  );
-}

From 44b8b384c9e493960e7ea4aa56dc02fce6b34040 Mon Sep 17 00:00:00 2001
From: Jordan Dukart <jordan@discoverygarden.ca>
Date: Thu, 18 Apr 2013 17:02:21 -0300
Subject: [PATCH 6/6] Notice and comment fix.

---
 includes/ingest.form.inc | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/includes/ingest.form.inc b/includes/ingest.form.inc
index a94aaa30..f39d38ff 100644
--- a/includes/ingest.form.inc
+++ b/includes/ingest.form.inc
@@ -158,7 +158,7 @@ function islandora_ingest_form_get_next_step(array &$form_state, array $step = N
 }
 
 /**
- * Gets the next step.
+ * Gets the previous step.
  *
  * If the current step is not defined, its assumed to be the first step.
  *
@@ -465,9 +465,14 @@ function islandora_ingest_form_previous_button(array &$form_state) {
   // Before we move back to the previous step we should tell the previous step
   // to undo whatever its submit handler did.
   $prev_step = islandora_ingest_form_get_previous_step($form_state);
-  $form_id = $prev_step['form_id'];
-  $submit_callback = $form_id . '_undo_submit';
-  $submit = function_exists($submit_callback) ? array($submit_callback, 'islandora_ingest_form_previous_submit') : array('islandora_ingest_form_previous_submit');
+  if ($prev_step['type'] == 'form') {
+    $form_id = $prev_step['form_id'];
+    $submit_callback = $form_id . '_undo_submit';
+    $submit = function_exists($submit_callback) ? array($submit_callback, 'islandora_ingest_form_previous_submit') : array('islandora_ingest_form_previous_submit');
+  }
+  else {
+    $submit = array('islandora_ingest_form_previous_submit');
+  }
   return array(
     '#type' => 'submit',
     '#value' => t('Previous'),
@@ -611,7 +616,7 @@ function islandora_ingest_form_ingest_button(array &$form_state) {
  *   The Drupal form state.
  */
 function islandora_ingest_form_submit(array $form, array &$form_state) {
-  // Execute any remainng callbacks.
+  // Execute any remaining callbacks.
   islandora_ingest_form_increment_step($form_state);
   $step = islandora_ingest_form_get_step($form_state);
   if (isset($step) && $step['type'] == 'callback') {