Updated: Comment #N

Problem/Motivation

As we convert forms to use FormInterface, we come across cases where the form is used not as the route controller directly, and drupal_get_form() must be used.

We end up with code like this:

drupal_get_form(new SearchBlockForm());
drupal_get_form(ConfirmDeleteMultiple::create(\Drupal::getContainer()));

Proposed resolution

Make drupal_get_form() smart enough to take a class name, and instantiate it. Follow the same code flow as HtmlFormController (the class that process _form controllers):

drupal_get_form('Drupal\comment\Form\ConfirmDeleteMultiple');
drupal_get_form('Drupal\search\Form\SearchBlockForm');

Remaining tasks

N/A

User interface changes

N/A

API changes

API addition: Allow class names to be passed to drupal_get_form().

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tim.plunkett’s picture

Status: Active » Needs review
FileSize
13.48 KB
tim.plunkett’s picture

FileSize
13.5 KB
579 bytes

Duh.

Also, we could decide to prevent passing an object to drupal_get_form(), which will reduce the number of ways to do one thing, and will force proper usage of ContainerInjectionInterface when dependencies are needed.

However, that would make this an API Change, not Addition.

Crell’s picture

This was actually a problem I ran into when creating a training demo module that had a form inside of a block. We had to pass several otherwise-irrelevant dependencies through multiple layers to get it to the form. Very +1 in concept. I've only skimmed the code so far; let's give the bot a chance.

ParisLiakos’s picture

Status: Needs review » Reviewed & tested by the community

+1000
this patch is a beauty

webchick’s picture

Title: Provide an easier mechanism for using drupal_get_form() directly » Change notice: Provide an easier mechanism for using drupal_get_form() directly
Priority: Normal » Major
Status: Reviewed & tested by the community » Active
Issue tags: +Needs change record

This is one of my favourite DX patches of all time, and I think it is worthy of "major" status:

a) You eliminate a bunch of stupid "use" statements that are not in fact "used" anywhere in the file.
b) You don't need to struggle with whether it should be FooForm::create() or new FooForm, depending on if DI is used.
c) You empower copy/paste/modify developers to get stuff done without having to understand the entire inheritance tree of their dependencies.

So, in other words, plus freaking one!

Committed and pushed to 8.x.

Could we get a quick turnaround on this change notice? Alpha 4 is due out tomorrow. :)

andypost’s picture

Issue tags: -Needs change record

Also this needs major follow-up to update doc-block of drupal_get_form() to current state

andypost’s picture

Issue tags: +Needs change record

tag back

tim.plunkett’s picture

Assigned: tim.plunkett » Unassigned
Issue summary: View changes
Xano’s picture

Does this need a dedicated change notice, or is the mention of this feature in https://drupal.org/node/1932058 enough?

Shyamala’s picture

Assigned: Unassigned » Shyamala

Writing Change notification.

Shyamala’s picture

Status: Active » Needs review

Summary

  • Drupal 7, forms were built by a procedural function, and validation and submission were named functions.
  • In Drupal 8, there is now an interface called FormInterface with four methods:
    getFormID(), buildForm(), validateForm(), submitForm()
  • Refer FormInterface Change Request at: https://drupal.org/node/1932058
  • This change request cleans up drupal_get_form() to initiate an existing or new class name

Before

drupal_get_form(new SearchBlockForm());
drupal_get_form(ConfirmDeleteMultiple::create(\Drupal::getContainer()));

drupal_get_form was used as - <code>return drupal_get_form(new SearchBlockForm(), $this->request);

After

drupal_get_form('Drupal\comment\Form\ConfirmDeleteMultiple');
drupal_get_form('Drupal\search\Form\SearchBlockForm');

drupal_get_form now used as - return drupal_get_form('Drupal\search\Form\SearchBlockForm');

tim.plunkett’s picture

I would include the use statements for the "Before" section, otherwise this looks great! I think it can be published.

Shyamala’s picture

Title: Change notice: Provide an easier mechanism for using drupal_get_form() directly » Provide an easier mechanism for using drupal_get_form() directly
Assigned: Shyamala » Unassigned
Priority: Major » Normal
Status: Needs review » Fixed

Updated change notification at https://drupal.org/node/2173127

Shyamala’s picture

Issue tags: -Needs change record

Removed Change notification tag.

Status: Fixed » Closed (fixed)

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

quietone’s picture

publish change record