Download & Extend

Add #redirect property for form #type 'submit' to eliminate various submit handlers

Project:Drupal core
Version:8.x-dev
Component:forms system
Category:task
Priority:normal
Assigned:Unassigned
Status:needs review
Issue tags:API clean-up

Issue Summary

Several forms in core has a Delete button whose #submit function just redirects from foo/123/edit to foo/123/delete.

If FAPI could handle this redirect, we could get rid of a number of very similar functions in core.

It would also make it easier to add Delete buttons everywhere applicable. Currently, not all forms in core follows the convention of having a Delete button on the edit form, e.g. admin/config/content/formats/2.

What do you think of this idea?

AttachmentSizeStatusTest resultOperations
redirect-2.patch15.44 KBIdlePASSED: [[SimpleTest]]: [MySQL] 18,690 pass(es).View details | Re-test

Comments

#1

Reroll.

AttachmentSizeStatusTest resultOperations
redirect-3.patch16.28 KBIdleFAILED: [[SimpleTest]]: [MySQL] Unable to apply patch redirect-3.patch.View details | Re-test

#2

#1: redirect-3.patch queued for re-testing.

#3

Status:needs review» needs work

The last submitted patch, redirect-3.patch, failed testing.

#4

#5

#6

Title:Add support for array('#type' => 'submit', '#redirect' => 'foo/bar')» Add #redirect property for form #type 'submit' to eliminate various submit handlers
Version:7.x-dev» 8.x-dev
Category:feature request» task

Good suggestion.

This essentially moves the 'redirect' declaration into the form constructor, which has been asked for many times in a range of other issues already.

We should:

1) Ensure that a button-level #redirect is applied to $form_state['redirect'] before submit handlers are invoked, so submit handlers are able to override it.

2) Still allow to declare a form-level $form_state['redirect'] - which is used when no button-level #redirect is defined.

On 2) specifically, there's #1251616: $form_state['redirect'] does not work in form constructors, which will ultimately bring consistency with regard to this issue; i.e., if you can specify #redirect on a button, then you should also be able to specify $form_state['redirect'] in the form constructor.

#7

There's a problem with the approach taken in #1:

A mere declaration of #redirect should not break the entire form submission. Essentially, we only want to take over the value of #redirect into $form_state['redirect'].

We still need to invoke button-level #submit handlers, if any are defined.

However, if no button-level #submit handlers are needed, then the form-level #submit handlers will be invoked, which is obviously wrong. We need to decide what to do in that case:

A) If a button defines #redirect, but no #submit, then automatically inject an empty array for #submit; i.e., no submit handlers are invoked, and form processing directly hops to drupal_redirect_form(); e.g.:

$form['actions']['delete'] = array(
  '#type' => 'submit',
  '#value' => t('Delete'),
  // Form API automatically injects an empty #submit array for this button.
  '#redirect' => 'foo/bar/delete',
);

B) If a button defines #redirect and does not need submit handlers, but does not want form-level submit handlers to get invoked, then it has to manually specify an empty #submit array, too.

$form['actions']['delete'] = array(
  '#type' => 'submit',
  '#value' => t('Delete'),
  '#redirect' => 'foo/bar/delete',
  // Prevent form-level submit handlers from getting invoked.
  '#submit' => array(),
);

#8

Status:needs work» needs review

Attached patch implements #7.A)

I did not include any of the conversions from #1, since I think we need to agree on the API behavior first.

AttachmentSizeStatusTest resultOperations
drupal8.form-button-redirect.8.patch2.22 KBIdlePASSED: [[SimpleTest]]: [MySQL] 50,471 pass(es).View details | Re-test