Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

unsubmitted form doesn't get violation mapped #30011

Closed
videni opened this issue Jan 29, 2019 · 4 comments
Closed

unsubmitted form doesn't get violation mapped #30011

videni opened this issue Jan 29, 2019 · 4 comments

Comments

@videni
Copy link

videni commented Jan 29, 2019

I am not sure if this is a bug, but the behavior looks strange.

CarBrandType

class CarBrandType extends AbstractType
{
    public function buildForm(FormBuilderInterface $builder, array $options): void
    {
        $builder
            ->add('name', FormTypes\TextType::class, [
                'label' => 'acme.car_brand.name.label',
                'required' => true,
            ])
            ->add('enabled',  FormTypes\CheckboxType::class, [
                'label' => 'acme.car_brand.enabled.label',
                'required' => false,
            ])
            ->add('image', FormTypes\TextType::class, [
                'label' => 'acme.car_brand.image.label',
                'required' => false,
            ]);
    }
}

violation rules

Acme\Bundle\AutoBundle\Entity\CarBrand:
    properties:
         name:
            - NotBlank: { groups: ['acme'] }
            - Length: { groups: ['acme'] , max: 40}

Test

I submitted data without the required field name, I can see a violation happened at Symfony\Component\Form\Extension\Validator\EventListener\ValidationListener class, however, calling $form->isValid() will return true, because the Symfony\Component\Form\Extension\Validator\ViolationMapper\ViolationMapper doesn't map violation to unsubmitted form. that is the isValid method is not 100% sure the form has no violations. what is the sure way?

 /**
     * @test
     */
    public function it_allows_to_create_a_car_brand()
    {
        $this->client->setServerParameter('HTTP_Authorization', $this->getMainAdminToken());

        $data =
<<<EOT
        {
            "image": "/image_path"
        }
EOT;

        $this->client->request('POST', '/api/admin/car_brands', [], [], ['CONTENT_TYPE' => 'application/json', 'ACCEPT' => 'application/json'], $data);

        $response = $this->client->getResponse();

        $this->assertResponse($response, 'car/brand/create_response', Response::HTTP_CREATED);
    }

@derrabus
Copy link
Member

The Form component is designed to process submitted HTML forms. As far as I can tell, you're not submitting a form, you're posting a JSON object. You can instead use the Symfony Validator to validate the submitted data structure.

@videni
Copy link
Author

videni commented Jan 30, 2019

hi, @derrabus , don't get confused by that json data which will be converted to array fed to Symfony form, the same way as FosRestBundle does.

@xabbuh
Copy link
Member

xabbuh commented Jan 30, 2019

@videni You are right here, but I am still going to close your issue as this is already covered by #24453. Thank you for understanding.

@xabbuh xabbuh closed this as completed Jan 30, 2019
@xabbuh
Copy link
Member

xabbuh commented Feb 15, 2019

Can you confirm that the changes proposed in #30265 will fix this issue?

xabbuh added a commit that referenced this issue Feb 21, 2019
…requests (xabbuh)

This PR was merged into the 3.4 branch.

Discussion
----------

[Form] do not validate non-submitted form fields in PATCH requests

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #11493, #19788, #20805, #24453, #30011
| License       | MIT
| Doc PR        |

When a form field is not embedded as part of a HTTP PATCH requests, its
validation constraints configured through the `constraints` option must
not be evaluated. The fix from #10567 achieved this by not mapping their
violations to the underlying form field. This however also means that
constraint violations caused by validating the whole underlying data
object will never cause the form to be invalid. This breaks use cases
where some constraints may, for example, depend on the value of other
properties that were changed by the submitted data.

Commits
-------

a60d802 do not validate non-submitted form fields in PATCH requests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants