form with multiple button check

[raneomik]

Hello !

In this context, in functional tests :

• zenstruck/browser : 0.7.0
• php : 8.0.9
• phpunit : 9.5.8
• symfony : 5.3.6

as noticed here, the multiple button check with $form->getClickedButton() doesn't seem to work when following the recommandation in best practices and put it in controller or in form - this debug check in controller :

 if ($form->isSubmitted() && $form->isValid()) {
    dd($form->getClickedButton());
}

systematically outputs null.

Even the solution brought here and "slightly"(as illustration) followed like this in our case :

  <input type="submit" name="submit" value="draft"/>
  <button type="submit" name="submit" value="publish">
        publish
  </button>

the following test :

 $this->browser()
            ->get('/form-path')
... // filling object fields
            ->click('draft')
    ;

also actually outputs null when debugging it in controller like this :

 if ($form->isSubmitted() && $form->isValid()) {
    dd($request->request->get('submit'));
}

dd($request->request); somehow only outputs all other submited data except the 'submit' button.

It seems there's somewhere a process for SF5 where the form is cleaned up from submit buttons or there's something I'm missing...

Out of the test context, I manage to have the cliked button in both cases (in template using $request->request->get('submit') or in controller/form using $form->getClickedButton()) ...

[kbond]

First, does this test work as expected with v0.6.0 of browser?

Out of the test context, I manage to have the cliked button in both cases (in template using $request->request->get('submit') or in controller/form using $form->getClickedButton()) ...

Does this mean if you click these buttons manually in your dev environment, it works as expected?

[raneomik]

Does this mean if you click these buttons manually in your dev environment, it works as expected?

Yes, "out of test context" means in dev environment and with the same dd call we have the expected values.

First, does this test work as expected with v0.6.0 of browser?

I'll come back with this, sorry, I didn't have time yet to test it in 0.6. I only know that in 0.7 it works as initially described. (nb: an edition of it was made to precise the context)

[kbond]

Ok, yeah, if you could confirm it works as expected in 0.6.0 that would help narrow down the problem I think...

54 shows it works in the ways I know how to test this - hopefully we can find some html that can create a failing test.

[raneomik]

Hello @kbond, I finally set up a repo to test the differencies between 0.6.0 and 0.7.0 : https://github.com/raneomik/sf-sandbox.

The bypass with this snippet:

  <input type="submit" name="submit" value="firstPlainButton"/>
  <button type="submit" name="submit" value="secondPlainButton">
        secondPlainButton
  </button>

can actually be resolved by using the profiler in v0.6.0 (where BTW $form->getClickedButton() works well, when buttons set up in formType - cf. tests) like this :

        $request = $this
            ->browser()
            ->interceptRedirects()
            ->get('/')
            ->fillField('default[fieldOne]', '1')
            ->fillField('default[fieldTwo]', '2')
            ->withProfiling()
            ->click('secondPlainButton')
            ->profile()->getCollector('request');

        /** @var Data $submittedData */
        $submittedData = $request->getRequestRequest()->get('submit');
        self::assertSame('secondPlainButton', $submittedData->getValue());

but not in v0.7.0, where $submittedData seems to be null ...

The results are available here : v0.6.0 vs v0.7.0

regards

[kbond]

Thank you! This will be very helpful for developing a failing test and fix!

[kbond]

Aha, found how to recreate the problem in the test suite (#55). The button is dropped from the request if other fields in the form are filled in. Didn't catch with #54 as my test just submitted the form without filling any other fields.

[kbond]

56 now includes a fix. I tested in your reproducer and it now works. Can you test that PR in your primary project to ensure there are no other regressions?

[raneomik]

I applied your changes and tested here but apparently there is the "secondPlainButtonClick" case still failing

[kbond]

Thanks, this should be fixed now. It was failing because the type attribute was missing from the button, I removed this requirement.

[raneomik]

Okay, well done and thank you, I can confirm that it works as expected here

[kbond]

Excellent, released the fix in v0.7.1.

Thanks again for helping diagnose this regression!