Description:

I have an exportables item edit form and I have added some handling for an "add more" button to allow for multiple values on a specific field. If I visit this form without javascript enabled, the form submits completely and I am redirected to the /admin/structure/myexportable page even though the submit function that is triggered by my "add more" button is correctly run and $form_state['rebuild'] = TRUE; is correctly set.

How to reproduce:

  • I've attached a test module called "foo". It creates an exportable called a foo_bar.
  • install the foo module
  • add a new "foo_bar" (admin/structure/foo_bar/add)
  • click the "BTN TEST" button
  • Notice by the dsm() that we correctly run foo_bar_test_submit(). And also notice that that function sets $form_state['rebuild'] = TRUE;

Expected behavior

I would expect that the "add/edit" form should be rebuilt and displayed again instead of the admin/structure/foo_bar page. I should only be redirected to that page on a subimt if rebuild===FALSE.

Some relavant code

In case you dont want to download/install the test module.. here is the relavant code from foo_export_ui.inc:


/**
 * @file
 * A Ctools Export UI plugin for DFP ads.
 */

/**
 * Define this Export UI plugin.
 */
$plugin = array(
  'schema' => 'foo_bar',
  'access' => 'access content',

  // Define the menu item.
  'menu' => array(
    'menu item' => 'foo_bar',
    'menu prefix' => 'admin/structure',
    'menu title' => 'Foo Bars',
    'menu description' => 'Create and manage your foo bars.',
  ),

  // Define user interface texts.
  'title singular' => t('Foo bar'),
  'title plural' => t('Foo bars'),
  'title singular proper' => t('Foo bar'),
  'title plural proper' => t('Foo bar'),

  // Define the class to use as a handler for DFP ad tags.
  'handler' => array(
     'class' => 'foo_bar_ui',
     'parent' => 'ctools_export_ui',
  ),

  // Define the names of the functions that provide the add/edit forms.

  'form' => array(
    'settings' => 'foo_bar_form',
    'submit' => 'foo_bar_form_submit',
    'validate' => 'foo_bar_form_validate',
  ),
);

/**
 * Form builder.
 */
function foo_bar_form(&$form, &$form_state) {
  $form['thing'] = array(
    '#type' => 'textfield',
    '#title' => t('Thing'),
    '#required' => TRUE,
    '#maxlength' => 128,
  );
  $form['machinename'] = array(
    '#type' => 'machine_name',
    '#title' => t('Unique Name'),
    '#maxlength' => 128,
    '#machine_name' => array(
      'exists' => 'foo_bar_name_exists',
      'source' => array('thing'),
    ),
  ) + $form['info']['machinename'];
  unset($form['info']);

  $form['btn'] = array(
    '#type' => 'submit',
    '#value' => 'BTN TEST',
    '#submit' => array('foo_bar_btn_test_submit'),
  );
}

function foo_bar_form_submit($form, &$form_state) {
  dpm('in foo_bar_form_submit');
}

function foo_bar_form_validate(&$form, &$form_state) {
  dpm('in foo_bar_form_validate');
}

function foo_bar_btn_test_submit($form, &$form_state) {
  $form_state['rebuild'] = TRUE;
  dpm('in foo_bar_test_submit');
}

/**
 * Check if the given machinename is unique in the dfp_tags table.
 */
function foo_bar_name_exists($machinename) {
  $select = db_select('foo_bar', 'fb');
  $select->addExpression('COUNT(*)');
  $select->condition('fb.machinename', $machinename);

  return $select->execute()->fetchField();
}
CommentFileSizeAuthor
#4 1524598.patch1.47 KBbleen
#3 1524598.patch1.45 KBbleen
#1 1524598.patch890 bytesbleen
foo.zip3.05 KBbleen

Comments

bleen’s picture

Issue summary: View changes

Updated issue summary.

bleen’s picture

Status: Active » Needs review
StatusFileSize
new890 bytes

Ok ... I think I found the problem here. There are a couple of places where ctools is only checking if the form has been executed and not wether or not rebuild == true.

Even if I add unset($form_state['executed']) in my submit function, then Drupal core just sets it back to TRUE in drupal_process_form().

That said, I believe this patch solves the problem by checking both for "executed" and "rebuild" before saving the form data and before redirecting the user back to the list of existing exportable objects.

merlinofchaos’s picture

empty() should be used for safety.

bleen’s picture

StatusFileSize
new1.45 KB

a couple more places where this check should be done (for editing and cloning)...

bleen’s picture

StatusFileSize
new1.47 KB

cross posted with merlinofchaos ... here is an adjustment of the patch in #3 with the feedback from #2

bleen’s picture

Issue summary: View changes

fixing bullet points

devin carlson’s picture

Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

The patch in #4 fixed the issues I had with a ctools plugin add/edit form being redirected back to the export ui page when triggering a submit function through a custom button.

The button rebuilds the form to determine the available options in a dropdown where the form has two dropdowns and the options in the second dropdown depend on the choice made in the first dropdown.

japerry’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.