Islandora-Collaboration-Group / islandora_webform

islandora_webform
GNU General Public License v3.0
6 stars 8 forks source link

Add alter hook to iwi_preview_ingest_submission_form #14

Closed patdunlavey closed 8 years ago

patdunlavey commented 8 years ago

It's very difficult or impossible to account for all possible special cases of how users may want to translate webform submissions into values in fields in the xml form preview. A couple of examples:

  1. The contents of a webform text field is meant to be interpreted as comma-separated values to put several values into a multi-value tag field.
  2. "Append" and "replace" are not sufficiently explicit, the user wants this value to go into a field in tabset 0, another value to go into a field in tabset 1, and another value to go into a field in tabset 3.

To implement this, create a drupal_alter hook in iwi_preview_ingest_submission_form that permits other modules to modify $new_form just before it is added back to $form: https://github.com/commonmedia/islandora_webform/blob/master/submodules/islandora_webform_ingest/includes/ingest_page.inc#L225

I think that these hook implementations would work primarily by calling iwi_set_form_element_byhash : https://github.com/commonmedia/islandora_webform/blob/master/submodules/islandora_webform_ingest/includes/utilities.inc#L426.

I think iwi_set_form_element_byhash will need to be modified to add some more ability to define the delta for multi-value tabsets and tag fields.

patdunlavey commented 8 years ago

@DiegoPino, I wonder if you have any thoughts about how to modify iwi_set_form_element_byhash to support explicitly defined deltas for tabsets and tag (multiple-value) fields? My thinking is to provide more options for $mode besides "append" and "replace". Perhaps a string that can be parsed to obtain instructions for tabset and tag deltas.

DiegoPino commented 8 years ago

Hi @patdunlavey, let me thing about this tomorrow. Do you have already some use cases?

patdunlavey commented 8 years ago

@DiegoPino, One use case (I'm not making this one up, it's what I'm actually being asked to implement): The webform has two text fields: "Personal contributor" and "Corporate contributor". They are both mapped to "nameTabs:namePanel:nameSet:namePart". The user enters a list of names in each, separated by commas (let's say three personal contributor names, and two corporate contributors). On ingest preview, the values are split by commas into multiple values. A new "nameTabs:namePanel:nameSet" is created for each name, five in all. The "personal contributors" will have the field "nameTabs:namePanel:nameSet:nameType" set to "personal", while the corporate contributors will have that field set to "corporate".

This case illustrates how an alter hook is really the only way to go - there is no way to build this kind of flexibility into the iwi module. It also shows how we need a way to explicitly define destination deltas for multi-value fields (we need to be able to put the correct 'namePart' with its 'nameType').

patdunlavey commented 8 years ago

Example of the hook implementation:

/**
 * Implements hook_iwi_set_form_element_alter().
 */
function my_module_iwi_set_form_alter(&$form, $form_state, $submission) {
  $cloned_elements = array();
  // Delta is meant to set which tabset delta to write to - this doesn't work (yet).
  $delta = 0;
  $attributes = array(
    'class' => array(
      'submitted-value',
    ),
  );
  // Corporations.
  if(!empty($submission->data[1][0])) {
    $names = array_map('trim', explode(',', $submission->data[1][0]));
    foreach($names as $name) {
      $input_value = array(
        '#default_value' => 'corporate',
        '#attributes' => $attributes,
        );
      iwi_set_form_element_byhash($form, $form_state, $cloned_elements, 'nameTabs:namePanel:nameSet:nameType', $input_value, $delta);
      $input_value = array(
        '#default_value' => $name,
        '#attributes' => $attributes,
      );
      iwi_set_form_element_byhash($form, $form_state, $cloned_elements, 'nameTabs:namePanel:nameSet:namePart', $input_value, $delta);
      $delta++;
    }
  }
  // Private individuals.
  if(!empty($submission->data[5][0])) {
    $names = array_map('trim', explode(',', $submission->data[5][0]));
    foreach($names as $name) {
      $input_value = array(
        '#default_value' => 'personal',
        '#attributes' => $attributes,
      );
      iwi_set_form_element_byhash($form, $form_state, $cloned_elements, 'nameTabs:namePanel:nameSet:nameType', $input_value, $delta);
      $input_value = array(
        '#default_value' => $name,
        '#attributes' => $attributes,
      );
      iwi_set_form_element_byhash($form, $form_state, $cloned_elements, 'nameTabs:namePanel:nameSet:namePart', $input_value, $delta);
      $delta++;
    }
  }
}
patdunlavey commented 8 years ago

@DiegoPino I created a branch on my fork of this module that you can look at: https://github.com/patdunlavey/islandora_webform/tree/feature/ingest_form_alter_hook

DiegoPino commented 8 years ago

Great, will start tomorrow morning.

patdunlavey commented 8 years ago

Thanks so much Diego!

DiegoPino commented 8 years ago

Still on this @patdunlavey, i'm building some test forms and objects to test further. Will have a solution tomorrow

patdunlavey commented 8 years ago

You're the best, @DiegoPino!

DiegoPino commented 8 years ago

@patdunlavey, i got it and resolved it. Will publish tomorrow my code, afternoon. Do you have some sparetime after 3:00PM to talk via Skype tomorrow?

patdunlavey commented 8 years ago

@DiegoPino Cool!!! As it happens, I will be out after 2:00 tomorrow. Earlier tomorrow, or Thursday morning would work for me.

DiegoPino commented 8 years ago

@patdunlavey Thursday morning is perfect. See you

DiegoPino commented 8 years ago

https://github.com/commonmedia/islandora_webform/pull/18

patdunlavey commented 8 years ago

Just to make things more complicated...

I'm using hook_iwi_set_form_element_alter to populate names in a tabset, and for each name, to populate roles for that name. Here's my test input:

Chris Froome, Alberto Contador (rider, captain), Nairo Quintana(rider), Laurens Ten Dam (rider, domestique)

And to help visualize: screen shot 2015-11-06 at 4 01 58 pm

And here's the destination form schema: screen shot 2015-11-06 at 3 45 54 pm

Here's my implementation of that hook:

/**
 * Implements hook_iwi_set_form_element_alter().
 */
function mymodule_iwi_set_form_alter(&$form, $form_state, $submission) {
    // Restrict to "Test append and replace" webform
    if ($submission->nid == 14) {
        $attributes = array(
            'class' => array(
                'submitted-value',
            ),
        );

        // Get a list of previously added nameTabs so we can remove them later.
        $old_name_tabs = element_children($form['nameTabs']);

        // Corporations.
        if (!empty($submission->data[1][0])) {
            $names = _names_roles_parser($submission->data[1][0]);
            foreach ($names as $name) {
                $offset = md5(uniqid('offset', TRUE));
                $input_value = array(
                    '#default_value' => 'family',
                    '#attributes' => $attributes,
                );
                iwi_set_form_element_byhash($form, $form_state, 'nameTabs:namePanel:nameSet:nameType', $input_value, 'append', $offset);

                // Put in the name value
                $input_value = array(
                    '#default_value' => $name['name'],
                    '#attributes' => $attributes,
                );
                iwi_set_form_element_byhash($form, $form_state, 'nameTabs:namePanel:nameSet:namePart', $input_value, 'append', $offset);

                // Put in the roles.
                foreach($name['roles'] as $role) {
//                  $offset = md5(uniqid('roffset', TRUE));
                    $input_value = array(
                        '#default_value' => $role,
                        '#attributes' => $attributes,
                    );
                    iwi_set_form_element_byhash($form, $form_state, 'nameTabs:namePanel:roleSet:rolesTags:0', $input_value, 'append', $offset);
                }
            }
        }
        // Private individuals.
        if (!empty($submission->data[5][0])) {
            $names = _names_roles_parser($submission->data[5][0]);
            foreach ($names as $name) {
                $offset = md5(uniqid('offset', TRUE));
                $input_value = array(
                    '#default_value' => 'personal',
                    '#attributes' => $attributes,
                );
                iwi_set_form_element_byhash($form, $form_state, 'nameTabs:namePanel:nameSet:nameType', $input_value, 'append', $offset);
                $input_value = array(
                    '#default_value' => $name['name'],
                    '#attributes' => $attributes,
                );
                iwi_set_form_element_byhash($form, $form_state, 'nameTabs:namePanel:nameSet:namePart', $input_value, 'append', $offset);

                    // Put in the roles.
                foreach($name['roles'] as $role) {
//                  $offset = md5(uniqid('roffset', TRUE));
                    $input_value = array(
                        '#default_value' => $role,
                        '#attributes' => $attributes,
                    );
                    iwi_set_form_element_byhash($form, $form_state, 'nameTabs:namePanel:roleSet:rolesTags:0', $input_value, 'append', $offset);
                }
            }
        }
        // Remove the original nameTabs that we saved above.
        foreach ($old_name_tabs as $old_name_tab) {
            // Get the objective form hash from the drupal form element.
            $hash = $form['nameTabs'][$old_name_tab]['#hash'];
            // Find that element in the objective form storage and orphan (remove) it.
            // We have to do this so that ajax won't reload those original tabs when the user
            // adds a new tab.
            $form_state['storage']['objective_form_storage']['root']->findElement($hash)->orphan();
            // Finally, we can safely delete the drupal form element.
            unset($form['nameTabs'][$old_name_tab]);
        }
    }
}

/**
 * Utility function extracts names and roles from a string.
 *
 * The string should look like this:
 * First Name (Role 1, Role 2), Second Name (Role), Third Name, Fourth Name (Role 4, Role 5), etc...
 * I.e. Names are required, roles are optional and enclosed in parentheses in the comma-separated group
 * with the name.
 *
 * @param string $string
 *
 * @return array
 */
function _names_roles_parser($string) {
    $tab_split_regex = '/,(?![^(]*\))/';
    $tabs = array_map('trim', preg_split($tab_split_regex, $string));

    $results = array();
    foreach($tabs as $tab) {
        $roles = array();
        if(preg_match('/\((.+?)\)/', $tab, $roles_match) && !empty($roles_match[1])) {
            $roles = array_map('trim', preg_split($tab_split_regex, $roles_match[1]));
        }
        $results[] = array(
            'name' => trim(str_replace($roles_match, '', $tab)),
            'roles' => $roles,
        );
    }
    return $results;
}

The parse function does a good job with splitting that string into a nice structured array that I can walk through. But the part of my hook_iwi_set_form_element_alter() that tries to populate the roles for each name doesn't work. It does add some roles, but they show no value, and where they appear do not correspond to the name tabs. In this case, the third name shows one blank role added, and the fourth name shows two blank roles added: screen shot 2015-11-06 at 5 44 10 pm

I'm calling iwi_set_form_element_byhash() using the same $offset as used for the name fields. I think that must be how it works, because otherwise you can't identify which name gets this tag. The problem, I think, is where it looks to see if $offset is one of the existing $special_element_children. It will not be because in this case $offset refers to something higher up in the ancestry. $element, in this case, is of type "tags", and not "tabs".

I really hate to ask you to look at this again @DiegoPino, but I have stared at this for a few hours without making real progress. I suspect that, because these xml form elements can be nested like this example, there may be some more fundamental work needed in the iwi_set_form_element_byhash() function.

DiegoPino commented 8 years ago

pat, well... Will look at this tomorrow. Don't worry, there must be some simple fix. Let's talk on Monday Best

El 06-11-2015, a las 19:49, Pat Dunlavey notifications@github.com escribió:

Just to make things more complicated...

I'm using hook_iwi_set_form_element_alter to populate names in a tabset, and for each name, to populate roles for that name. Here's my test input:

Chris Froome, Alberto Contador (rider, captain), Nairo Quintana(rider), Laurens Ten Dam (rider, domestique)

And to help visualize:

And here's the destination form schema:

Here's my implementation of that hook:

/**

  • Implements hook_iwi_set_form_element_alter(). */ function mymodule_iwi_set_form_alter(&$form, $form_state, $submission) { // Restrict to "Test append and replace" webform if ($submission->nid == 14) { $attributes = array( 'class' => array( 'submitted-value', ), );

    // Get a list of previously added nameTabs so we can remove them later.
    $old_name_tabs = element_children($form['nameTabs']);
    
    // Corporations.
    if (!empty($submission->data[1][0])) {
      $names = _names_roles_parser($submission->data[1][0]);
      foreach ($names as $name) {
          $offset = md5(uniqid('offset', TRUE));
          $input_value = array(
              '#default_value' => 'family',
              '#attributes' => $attributes,
          );
          iwi_set_form_element_byhash($form, $form_state, 'nameTabs:namePanel:nameSet:nameType', $input_value, 'append', $offset);
    
          // Put in the name value
          $input_value = array(
              '#default_value' => $name['name'],
              '#attributes' => $attributes,
          );
          iwi_set_form_element_byhash($form, $form_state, 'nameTabs:namePanel:nameSet:namePart', $input_value, 'append', $offset);
    
          // Put in the roles.
          foreach($name['roles'] as $role) {

    // $offset = md5(uniqid('roffset', TRUE)); $input_value = array( '#default_value' => $role, '#attributes' => $attributes, ); iwi_set_form_element_byhash($form, $form_state, 'nameTabs:namePanel:roleSet:rolesTags:0', $input_value, 'append', $offset); } } } // Private individuals. if (!empty($submission->data[5][0])) { $names = _names_roles_parser($submission->data[5][0]); foreach ($names as $name) { $offset = md5(uniqid('offset', TRUE)); $input_value = array( '#default_value' => 'personal', '#attributes' => $attributes, ); iwi_set_form_element_byhash($form, $form_state, 'nameTabs:namePanel:nameSet:nameType', $input_value, 'append', $offset); $input_value = array( '#default_value' => $name['name'], '#attributes' => $attributes, ); iwi_set_form_element_byhash($form, $form_state, 'nameTabs:namePanel:nameSet:namePart', $input_value, 'append', $offset);

              // Put in the roles.
          foreach($name['roles'] as $role) {

    // $offset = md5(uniqid('roffset', TRUE)); $input_value = array( '#default_value' => $role, '#attributes' => $attributes, ); iwi_set_form_element_byhash($form, $form_state, 'nameTabs:namePanel:roleSet:rolesTags:0', $input_value, 'append', $offset); } } } // Remove the original nameTabs that we saved above. foreach ($old_name_tabs as $old_name_tab) { // Get the objective form hash from the drupal form element. $hash = $form['nameTabs'][$old_name_tab]['#hash']; // Find that element in the objective form storage and orphan (remove) it. // We have to do this so that ajax won't reload those original tabs when the user // adds a new tab. $form_state['storage']['objective_form_storage']['root']->findElement($hash)->orphan(); // Finally, we can safely delete the drupal form element. unset($form['nameTabs'][$old_name_tab]); } } }

/**

  • Utility function extracts names and roles from a string. *
  • The string should look like this:
  • First Name (Role 1, Role 2), Second Name (Role), Third Name, Fourth Name (Role 4, Role 5), etc...
  • I.e. Names are required, roles are optional and enclosed in parentheses in the comma-separated group
  • with the name. *
  • @param string $string *
  • @return array _/ function _names_roles_parser($string) { $tab_splitregex = '/,(?![^(]))/'; $tabs = array_map('trim', preg_split($tab_split_regex, $string));

    $results = array(); foreach($tabs as $tab) { $roles = array(); if(preg_match('/((.+?))/', $tab, $roles_match) && !empty($roles_match[1])) { $roles = array_map('trim', preg_split($tab_split_regex, $roles_match[1])); } $results[] = array( 'name' => trim(str_replace($roles_match, '', $tab)), 'roles' => $roles, ); } return $results; } The parse function does a good job with splitting that string into a nice structured array that I can walk through. But the part of my hook_iwi_set_form_element_alter() that tries to populate the roles for each name doesn't work. It does add some roles, but they show no value, and where they appear do not correspond to the name tabs. In this case, the third name shows one blank role added, and the fourth name shows two blank roles added:

I'm calling iwi_set_form_element_byhash() using the same $offset as used for the name fields. I think that must be how it works, because otherwise you can't identify which name gets this tag. The problem, I think, is where it looks to see if $offset is one of the existing $special_element_children. It will not be because in this case $offset refers to something higher up in the ancestry. $element, in this case, is of type "tags", and not "tabs".

I really hate to ask you to look at this again @DiegoPino, but I have stared at this for a few hours without making real progress. I suspect that, because these xml form elements can be nested like this example, there may be some more fundamental work needed in the iwi_set_form_element_byhash() function.

— Reply to this email directly or view it on GitHub.

patdunlavey commented 8 years ago

Thinking about this some more...

Our form path can have more than one element whose #type matches a $special_xmlform_element. In my use-case, it is: tabs:tabPanel:fieldset:tags:tag:0 ...but (correct me if I'm wrong) it could also be something crazy like: tabs:tabPanel:fieldset:tabs:tabPanel:fieldset:tabs:tabPanel:fieldset:tags:tag:0

In these cases of nested special form elements, we need to be able to define a specific offset for each one in order to have a complete "address" to write to.

My thought is to permit $offset to be an array. $offset[0] would be the first special element offset, $offset[1] the second, and so on. We should probably just cast $offset as an array. Then as we travel down through the parents, each time we hit one whose #type is special, we array_shift the next $offset value and use it for that element. At the moment, I think that the function assumes that when we hit a special element, its child will be our ultimate destination. Instead, I think we need to evaluate if the child is our ultimate destination, and if not, then look for an existing child at the active offset, and resume traveling down the path from that child if it exists. (If it doesn't exist, then I think we create a new element as though the offset were NULL, and proceed on down the path using NULL offsets.) [[Edit: this is wrong! If the offset doesn't exist, then we create an element and give it that offset. Proceed down the path, creating new elements.]]

Of course if there are not enough offsets provided for the number of special elements in the path, then NULL should be used after we run out of offsets. (Edge case: if NULL is provided as an offset in that array, then I think we need to assign NULL offset for all child special elements, even if real values are provided for later offsets.)

So I think the main re-work needed will be in that $parents loop, mainly here

DiegoPino commented 8 years ago

@patdunlavey, this is a big change in the API and i fear you are overcomplicating your taks.

Basically you want to have multiple offsets, like a tabs->tab + tags->tag inside them.

What iwi_set_form_element_byhash() does is:

The thing is it does this correctly! In case the path has two special elements like here:

nameTabs:namePanel:roleSet:rolesTags:0

It keeps iterating (passing over nameTabs even when this one is a special element) until it reaches rolesTags. At this point, on the next loop iteration you are standing at 0 and it does this:

So. Having this clear:

It's not a good idea to have a that complicated function. Passing and array with $offsets, I fear the next step will be an array of $mode :smile_cat:

So lets go back to this one: nameTabs:namePanel:roleSet:rolesTags:0 You just need to replace namePanel with a previously created element name in _alter() hook for which you have control over it's name!

so instead you pass nameTabs:aaabbbccccl:roleSet:rolesTags:0

So the real question is how you know where to put that one? Some alternatives:

Does this makes sense? Or i am understanding your need wrongly? Or there is a flaw in the current iwi_set_form_element_byhash() that does not allow that?

Just tell me what you need me to do and your timings.

Best!

patdunlavey commented 8 years ago

Thanks Diego! Using the name tab offset in the form path was the ticket! I had tried doing that, but I guess I didn't have everything exactly right.

I also had to remember to delete the old role (at offset 0).

For posterity's sake, here's my working hook_iwi_set_form_element_alter() function:

/**
 * Implements hook_iwi_set_form_element_alter().
 */
function mymodule_hooks_iwi_set_form_alter(&$form, $form_state, $submission) {
    // Restrict to "Test append and replace" webform
    if ($submission->nid == 14) {
        $attributes = array(
            'class' => array(
                'submitted-value',
            ),
        );

        // Get a list of previously added nameTabs so we can remove them later.
        $old_name_tabs = element_children($form['nameTabs']);

        // Corporations.
        if (!empty($submission->data[1][0])) {
            $names = _names_roles_parser($submission->data[1][0]);
            foreach ($names as $name) {
                $name_tab_offset = md5(uniqid('offset', TRUE));
                $input_value = array(
                    '#default_value' => 'family',
                    '#attributes' => $attributes,
                );
                iwi_set_form_element_byhash($form, $form_state, 'nameTabs:namePanel:nameSet:nameType', $input_value, 'append', $name_tab_offset);

                // Put in the name value
                $input_value = array(
                    '#default_value' => $name['name'],
                    '#attributes' => $attributes,
                );
                iwi_set_form_element_byhash($form, $form_state, 'nameTabs:namePanel:nameSet:namePart', $input_value, 'append', $name_tab_offset);

                // Put in the roles.
                foreach($name['roles'] as $role) {
                    $role_tag_offset = md5(uniqid('roffset', TRUE));
                    $input_value = array(
                        '#default_value' => $role,
                        '#attributes' => $attributes,
                    );
                    iwi_set_form_element_byhash($form, $form_state, 'nameTabs:' . $name_tab_offset . ':roleSet:rolesTags:0', $input_value, 'append', $role_tag_offset);
                }
            }
        }
        // Remove the original nameTabs that we saved above.
        foreach ($old_name_tabs as $old_name_tab) {
            // Get the objective form hash from the drupal form element.
            $hash = $form['nameTabs'][$old_name_tab]['#hash'];
            // Find that element in the objective form storage and orphan (remove) it.
            // We have to do this so that ajax won't reload those original tabs when the user
            // adds a new tab.
            $form_state['storage']['objective_form_storage']['root']->findElement($hash)->orphan();
            // Finally, we can safely delete the drupal form element.
            unset($form['nameTabs'][$old_name_tab]);
        }
    }
}

/**
 * Utility function extracts names and roles from a string.
 *
 * The string should look like this:
 * First Name (Role 1, Role 2), Second Name (Role), Third Name, Fourth Name (Role 4, Role 5), etc...
 * I.e. Names are required, roles are optional and enclosed in parentheses in the comma-separated group
 * with the name.
 *
 * @param string $string
 *
 * @return array
 */
function _names_roles_parser($string) {
    $tab_split_regex = '/,(?![^(]*\))/';
    $tabs = array_map('trim', preg_split($tab_split_regex, $string));

    $results = array();
    foreach($tabs as $tab) {
        $roles = array();
        if(preg_match('/\((.+?)\)/', $tab, $roles_match) && !empty($roles_match[1])) {
            $roles = array_map('trim', preg_split($tab_split_regex, $roles_match[1]));
        }
        $results[] = array(
            'name' => trim(str_replace($roles_match, '', $tab)),
            'roles' => $roles,
        );
    }
    return $results;
}
patdunlavey commented 8 years ago

So after all that, it turns out that the ingest process doesn't work for tag fields inside of tabsets. New issue created: https://github.com/commonmedia/islandora_webform/issues/23

patdunlavey commented 8 years ago

This issue is addressed in pull request #28 and can be closed when that pull request is accepted.