Browse Source

Merge pull request #1 from nigelgbanks/7.x-code-review

Code Review - Refactor and fixes.
pull/317/head^2
Jordan Dukart 12 years ago
parent
commit
f4b86beaaf
  1. 322
      includes/ingest.form.inc
  2. 18
      islandora.module

322
includes/ingest.form.inc

@ -135,6 +135,50 @@ function islandora_ingest_form_get_step(array &$form_state, $step_id = NULL) {
return 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. * 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 * @param array $form_state
* The Drupal 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 * @return string
* The next step ID if found, NULL otherwise. * The next step ID if found, NULL otherwise.
*/ */
function islandora_ingest_form_get_next_step_id(array &$form_state) { function islandora_ingest_form_get_next_step_id(array &$form_state, $step_id = NULL) {
$step_id = islandora_ingest_form_get_current_step_id($form_state); $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)); $step_ids = array_keys(islandora_ingest_form_get_steps($form_state));
$index = array_search($step_id, $step_ids); $index = array_search($step_id, $step_ids);
$count = count($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 * @param array $form_state
* The Drupal 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 * @return string
* The previous step ID if found, NULL otherwise. * The previous step ID if found, NULL otherwise.
*/ */
function islandora_ingest_form_get_previous_step_id(array &$form_state) { function islandora_ingest_form_get_previous_step_id(array &$form_state, $step_id = NULL) {
$step_id = islandora_ingest_form_get_current_step_id($form_state); $step_id = isset($step_id) ? $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)); $step_ids = array_keys(islandora_ingest_form_get_steps($form_state));
$index = array_search($step_id, $step_ids); $index = array_search($step_id, $step_ids);
if ($index !== FALSE && --$index >= 0) { 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. * Build a list of steps given only configuration.
* *
* XXX: This is used to give an indication of whether there are any steps for a * 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 * @param array $configuration
* The list of key/value pairs of configuration. * The list of key/value pairs of configuration.
*/ */
function islandora_ingest_get_approximate_steps(array $configuration) { function islandora_ingest_get_approximate_steps(array $configuration) {
try { 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); islandora_ingest_form_validate_configuration($configuration);
} }
catch (InvalidArgumentException $e) { catch (InvalidArgumentException $e) {
@ -279,27 +329,70 @@ function islandora_ingest_form_execute_step(array $form, array &$form_state, $st
islandora_ingest_form_load_include($form_state); 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); $step = isset($step_id) ? islandora_ingest_form_get_step($form_state) : islandora_ingest_form_get_step($form_state, $step_id);
switch ($step['type']) { 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': case 'form':
return islandora_ingest_form_execute_form_step($form, $form_state, $step);
case 'batch':
// @todo Implement if possible.
break;
}
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 = array($form, &$form_state);
$args = isset($step['args']) ? array_merge($args, $step['args']) : $args; $args = isset($step['args']) ? array_merge($args, $step['args']) : $args;
$form = call_user_func_array($step['form_id'], $args); $form = call_user_func_array($step['form_id'], $args);
return islandora_ingest_form_stepify($form, $form_state, $step); return islandora_ingest_form_stepify($form, $form_state, $step);
}
case 'batch': /**
// @todo Implement if possible. * Assumes the given $step is a 'callback' step.
break; */
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');
}
case '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 = array(&$form_state);
$args = isset($step['do_function']['args']) ? array_merge($args, $step['do_function']['args']) : $args; $args = isset($step['do_function']['args']) ? array_merge($args, $step['do_function']['args']) : $args;
call_user_func_array($step['do_function']['function'], $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);
} }
/**
* 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');
} }
return array();
/**
* 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);
} }
/** /**
@ -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. * The stepified Drupal form definition for the given step.
*/ */
function islandora_ingest_form_stepify(array $form, array &$form_state, $step) { function islandora_ingest_form_stepify(array $form, array &$form_state, $step) {
$first_step = islandora_ingest_form_on_first_step($form_state); $first_form_step = islandora_ingest_form_on_first_form_step($form_state);
$last_step = islandora_ingest_form_on_last_step($form_state); $last_form_step = islandora_ingest_form_on_last_form_step($form_state);
$form['prev'] = $first_step ? NULL : islandora_ingest_form_previous_button($form_state); $form['prev'] = $first_form_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); $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(). // Allow for a hook_form_FORM_ID_alter().
drupal_alter(array('form_' . $step['form_id'], 'form'), $form, $form_state, $step['form_id']); drupal_alter(array('form_' . $step['form_id'], 'form'), $form, $form_state, $step['form_id']);
return $form; return $form;
} }
/** /**
* Checks if we are on the first step. * Checks if we are on the first form step.
* *
* @param array $form_state * @param array $form_state
* The Drupal form state. * The Drupal form state.
@ -333,14 +425,16 @@ function islandora_ingest_form_stepify(array $form, array &$form_state, $step) {
* @return bool * @return bool
* TRUE if we are currently on the first step, FALSE otherwise. * TRUE if we are currently on the first step, FALSE otherwise.
*/ */
function islandora_ingest_form_on_first_step(array &$form_state) { function islandora_ingest_form_on_first_form_step(array &$form_state) {
$step_id = islandora_ingest_form_get_current_step_id($form_state); $step = NULL;
$step_ids = array_keys(islandora_ingest_form_get_steps($form_state)); do {
return array_search($step_id, $step_ids) == 0; $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 * @param array $form_state
* The Drupal form state. * The Drupal form state.
@ -348,29 +442,12 @@ function islandora_ingest_form_on_first_step(array &$form_state) {
* @return bool * @return bool
* TRUE if we are currently on the last step, FALSE otherwise. * TRUE if we are currently on the last step, FALSE otherwise.
*/ */
function islandora_ingest_form_on_last_step(array &$form_state) { function islandora_ingest_form_on_last_form_step(array &$form_state) {
$next_id = islandora_ingest_form_get_next_step_id($form_state); $step = NULL;
if (!$next_id) { do {
return TRUE; $step = islandora_ingest_form_get_next_step($form_state, $step);
} } while (isset($step) && $step['type'] != 'form');
else { return !$step;
$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;
}
} }
/** /**
@ -387,37 +464,10 @@ function islandora_ingest_form_on_last_step(array &$form_state) {
function islandora_ingest_form_previous_button(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 // Before we move back to the previous step we should tell the previous step
// to undo whatever its submit handler did. // 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_previous_step($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']; $form_id = $prev_step['form_id'];
$submit_callback = $form_id . '_undo_submit'; $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'); $submit = function_exists($submit_callback) ? array($submit_callback, 'islandora_ingest_form_previous_submit') : array('islandora_ingest_form_previous_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( return array(
'#type' => 'submit', '#type' => 'submit',
'#value' => t('Previous'), '#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. * 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) { function islandora_ingest_form_previous_submit(array $form, array &$form_state) {
islandora_ingest_form_decrement_step($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; $form_state['rebuild'] = TRUE;
} }
@ -571,19 +592,7 @@ function islandora_ingest_form_ingest_button(array &$form_state) {
$validate_callback = $form_id . '_validate'; $validate_callback = $form_id . '_validate';
$validate = function_exists($validate_callback) ? array($validate_callback) : NULL; $validate = function_exists($validate_callback) ? array($validate_callback) : NULL;
$submit_callback = $form_id . '_submit'; $submit_callback = $form_id . '_submit';
$submit = array(); $submit = function_exists($submit_callback) ? array($submit_callback, 'islandora_ingest_form_submit') : array('islandora_ingest_form_submit');
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';
}
return array( return array(
'#type' => 'submit', '#type' => 'submit',
'#name' => 'ingest', '#name' => 'ingest',
@ -601,7 +610,14 @@ function islandora_ingest_form_ingest_button(array &$form_state) {
* @param array $form_state * @param array $form_state
* The Drupal 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) { foreach ($form_state['islandora']['objects'] as $object) {
try { try {
islandora_add_object($object); 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. * 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); $shared_storage = &islandora_ingest_form_get_shared_storage($form_state);
foreach (islandora_build_hook_list(ISLANDORA_INGEST_STEP_HOOK, $shared_storage['models']) as $hook) { foreach (islandora_build_hook_list(ISLANDORA_INGEST_STEP_HOOK, $shared_storage['models']) as $hook) {
// Required for pass by reference. // 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) { foreach (module_implements($hook) as $module) {
$function = $module . '_' . $hook; $function = $module . '_' . $hook;
$module_steps = (array) $function($form_state); $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) { foreach (islandora_build_hook_list(ISLANDORA_INGEST_STEP_HOOK, $shared_storage['models']) as $hook) {
drupal_alter($hook, $steps, $form_state); drupal_alter($hook, $steps, $form_state);
} }
// Check to see if the callback functions are implemented (existed). Remove // Add any defaults.
// any implementations from the steps that are incorrectly defined. foreach ($steps as $key => &$step) {
foreach ($steps as $key => $step) { $step['id'] = $key;
$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]);
}
}
} }
uasort($steps, 'drupal_sort_weight'); uasort($steps, 'drupal_sort_weight');
return $steps; return $steps;
} }

18
islandora.module

@ -1097,21 +1097,3 @@ function islandora_download_clip(AbstractObject $object) {
} }
exit(); 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',
),
),
);
}

Loading…
Cancel
Save