PrestaShop / ADR

Architecture Decision Records for the PrestaShop project
10 stars 15 forks source link

0012 - Establish rules for form processing #17

Open JevgenijVisockij opened 3 years ago

saulaski commented 2 years ago

@JevgenijVisockij you make a serious point about form usage.

In my humble opinion:

  1. to keep URL consistent, the same route should handle both form display (GET) and processing (POST/PUT)
  2. in a controller the form should be created and processed in the same action method
  3. redirection should only happen after the form is successfully processed, no matter what
  4. sounds kinda weird to let the DataProvider validate the form, whereas form constraints or domain constraints (class annotations) are way more standard
saulaski commented 2 years ago

If a page contains multiple forms, each form submission can be detected and processed individually:

public function myAction(Request $request): Response
{
    $form1 = $this
        ->createForm(/*...*/)
        ->handleRequest($request)
    ;
    if ($form1->isSubmitted() && $form1->isValid()) {
        // ...
    }

    $form2 = $this
        ->createForm(/*...*/)
        ->handleRequest($request)
    ;
    if ($form2->isSubmitted() && $form2->isValid()) {
        // ...
    }

    return $this->render('...', [
        'form1' => $form1->createView(),
        'form2' => $form2->createView(),
    ]);
}
saulaski commented 2 years ago

If I am not wrong, we could use the same POST route to process a form and a list: admin_emails_save_options: /configure/advanced/emails/options[POST]

If we use the FormInterface::isSubmitted() method to detect form submission, we could process this use case alongside the list. Testing FormInterface::isValid() alone won't work in this particular case. FormInterface::isSubmitted() is supposed to recognize the form name in POST data.