Add payload to ConstraintViolationListNormalizer by pierre-H · Pull Request #1364 · api-platform/core

pierre-H · 2017-09-12T13:42:00Z

Q	A
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	#1357
License	MIT
Doc PR	api-platform/docs#263

see the ticket for more information

meyerbaptiste · 2017-09-12T15:20:29Z

I don't know why test don't pass

Run php-cs-fixer: https://travis-ci.org/api-platform/core/jobs/274610519#L1072-L1158

dunglas

Can you also add some tests and an option to disable this behavior please?

dunglas · 2017-09-13T05:35:27Z

src/Hydra/Serializer/ConstraintViolationListNormalizer.php

+                'message' => $violation->getMessage()
            ];

+            if (! empty($violation->getConstraint()->payload)) {


I suggest to store $violation->getConstraint()->payload in a variable to prevent an extra function call. Btw you can remove the call to empty, it is useless.

Done ! Thanks

pierre-H · 2017-09-13T07:51:36Z

@dunglas I added the option and some tests but I am not that tests are complete for ApiPlatformExtensionTest .

soyuka · 2017-09-13T08:11:05Z

src/Hydra/Serializer/ConstraintViolationListNormalizer.php

+     */
+    private $serializePayload;
+
+    public function __construct(UrlGeneratorInterface $urlGenerator, bool $serializePayload)


serializePayload needs a default value

@soyuka Done

meyerbaptiste

Can you please run PHP-CS-Fixer @pierre-H?

pierre-H · 2017-09-13T14:11:50Z

PHP-CS-Fixer and tests are ok

meyerbaptiste · 2017-09-13T16:40:52Z

src/Api/OperationTypeDeprecationHelper.php

            @trigger_error('Using a boolean for the Operation Type is deprecrated since API Platform 2.1 and will not be possible anymore in API Platform 3', E_USER_DEPRECATED);

-            $operationType = $operationType === true ? OperationType::COLLECTION : OperationType::ITEM;
+            $operationType = true === $operationType ? OperationType::COLLECTION : OperationType::ITEM;


true === is useless here!

though this is my bad haha, how did this end up in the code :P.

It looks like PHP-CS-Fixer has a new rule about Yoda style 😛

We should probably apply it on lower branches.

Yes it was PHP-CS-Fixer

dunglas · 2017-09-13T17:41:33Z

src/Bridge/Symfony/Bundle/DependencyInjection/Configuration.php

                ->end()
                ->scalarNode('name_converter')->defaultNull()->info('Specify a name converter to use.')->end()
                ->scalarNode('path_segment_name_generator')->defaultValue('api_platform.path_segment_name_generator.underscore')->info('Specify a path name generator to use.')->end()
+                ->booleanNode('serialize_payload_constraint')->defaultFalse()->info('Enable the serialization of the payload field when a validation error is thrown.')->end()


What do you think of something like:

validator: serialize_payload: true

Some people are thinking about new options regarding the validator, this way we'll be feature-proof?

Ok, I will change that tomorrow

pierre-H · 2017-09-15T07:23:26Z

Isn't better to define in the configuration which fields we want to serialize ?

For example :

validator:
    serialize_payload_fields:
        severity
        anotherField

This way we can use payload field with sensitive and public data?

dunglas · 2017-09-15T08:01:54Z

@pierre-H good idea! and

serialize_payload_fields: true

to serialize all fields.

soyuka · 2017-09-15T09:22:17Z

As I've applied the cs fixes to 2.1, @dunglas may you merge 2.1 on to master. Then @pierre-H can git rebase origin/master so that the patch is cleaner :).

Thanks!

pierre-H · 2017-09-16T19:49:26Z

@dunglas done

pierre-H · 2017-09-18T10:19:15Z

Should I do something more ?

dunglas

It looks good to me!

dunglas · 2017-09-18T10:23:08Z

Thanks @pierre-H

pierre-H · 2017-09-18T10:23:54Z

With pleasure !
Thank you for your great work !

* Add payload constraint option For api-platform/core#1364 * Add ability to define payload fields * Minor Markdown and wording fixes * Update TOC

Add payload to ConstraintViolationListNormalizer

pierre-H changed the title ~~Add payload infos to ConstraintViolationListNormalizer~~ Add payload to ConstraintViolationListNormalizer Sep 12, 2017

pierre-H mentioned this pull request Sep 12, 2017

Validation : support of errors levels #1357

Closed

dunglas requested changes Sep 13, 2017

View reviewed changes

soyuka reviewed Sep 13, 2017

View reviewed changes

pierre-H mentioned this pull request Sep 13, 2017

Add payload constraint option api-platform/docs#263

Merged

meyerbaptiste reviewed Sep 13, 2017

View reviewed changes

dunglas approved these changes Sep 13, 2017

View reviewed changes

meyerbaptiste approved these changes Sep 13, 2017

View reviewed changes

soyuka approved these changes Sep 13, 2017

View reviewed changes

dunglas reviewed Sep 13, 2017

View reviewed changes

pierre-H added 2 commits September 15, 2017 11:53

Add payload serialize option

4baf715

Merge branch 'master' into patch-2

221cb0e

dunglas approved these changes Sep 18, 2017

View reviewed changes

dunglas merged commit 751893a into api-platform:master Sep 18, 2017

dunglas pushed a commit to api-platform/docs that referenced this pull request Sep 18, 2017

Add payload constraint option (#263)

cf0b56a

* Add payload constraint option For api-platform/core#1364 * Add ability to define payload fields * Minor Markdown and wording fixes * Update TOC

jonag pushed a commit to jonag/docs that referenced this pull request Sep 22, 2017

Add payload constraint option (api-platform#263)

a778916

* Add payload constraint option For api-platform/core#1364 * Add ability to define payload fields * Minor Markdown and wording fixes * Update TOC

pierre-H deleted the patch-2 branch September 27, 2017 08:48

sh41 pushed a commit to sh41/docs that referenced this pull request Sep 30, 2017

Add payload constraint option (api-platform#263)

a6bab6e

* Add payload constraint option For api-platform/core#1364 * Add ability to define payload fields * Minor Markdown and wording fixes * Update TOC

dunglas mentioned this pull request Oct 11, 2017

Improve payload support and remove duplicate code in ConstraintViolationListNormalizer #1416

Merged

hoangnd25 pushed a commit to hoangnd25/core that referenced this pull request Feb 23, 2018

Merge pull request api-platform#1364 from pierre-H/patch-2

9fd261e

Add payload to ConstraintViolationListNormalizer

Uh oh!

Conversation

pierre-H commented Sep 12, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

meyerbaptiste commented Sep 12, 2017

Uh oh!

dunglas left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pierre-H commented Sep 13, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

meyerbaptiste left a comment

Choose a reason for hiding this comment

Uh oh!

pierre-H commented Sep 13, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

meyerbaptiste Sep 13, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pierre-H commented Sep 15, 2017

Uh oh!

dunglas commented Sep 15, 2017

Uh oh!

soyuka commented Sep 15, 2017

Uh oh!

pierre-H commented Sep 16, 2017

Uh oh!

pierre-H commented Sep 18, 2017

Uh oh!

dunglas left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dunglas commented Sep 18, 2017

Uh oh!

pierre-H commented Sep 18, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

pierre-H commented Sep 12, 2017 •

edited

Loading

meyerbaptiste Sep 13, 2017 •

edited

Loading

dunglas left a comment •

edited

Loading