Skip to content

0012 - Establish rules for form processing#17

Open
JevgenijVisockij wants to merge 1 commit intoPrestaShop:masterfrom
JevgenijVisockij:0012-dont-use-constraints-in-some-option-forms
Open

0012 - Establish rules for form processing#17
JevgenijVisockij wants to merge 1 commit intoPrestaShop:masterfrom
JevgenijVisockij:0012-dont-use-constraints-in-some-option-forms

Conversation

@JevgenijVisockij
Copy link
Copy Markdown

No description provided.

@JevgenijVisockij JevgenijVisockij changed the title Establish rules for form processing 0012 - Establish rules for form processing Jan 14, 2021
@saulaski
Copy link
Copy Markdown

@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
Copy link
Copy Markdown

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
Copy link
Copy Markdown

saulaski commented Jan 25, 2022

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.

Copy link
Copy Markdown

@MeKeyCool MeKeyCool left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JevgenijVisockij thanks for your interest to Prestashop and for your efforts to make Prestashop great :)

I don't think this request is an " Architectural Decision Record" but more a UX/UI / product request that may imply some architectural optimizations (if a generic behavior is found)

I recommend to create an issue or to discuss with the product-team :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants