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

Fix: Check PropertyPath value for add error to form #11018

Merged
merged 3 commits into from
Apr 7, 2020

Conversation

lchrusciel
Copy link
Member

@lchrusciel lchrusciel commented Jan 14, 2020

Q A
Branch? 1.6
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Related tickets replaces #10689
License MIT

@Coosos
Copy link
Contributor

Coosos commented Jan 14, 2020

@lchrusciel
Last commit who remove is_null check is not included : b4edb10

EDIT : I let you do to avoid making a bad manipulation

``empty(null)`` or ``empty('')`` is also true

Co-Authored-By: Łukasz Chruściel <lchrusciel@gmail.com>
@Zales0123 Zales0123 added the Potential Bug Potential bugs or bugfixes, that needs to be reproduced. label Jan 16, 2020
Copy link
Member

@Zales0123 Zales0123 left a comment

Choose a reason for hiding this comment

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

Is there any possibility to add some test to that fix?

@lchrusciel
Copy link
Member Author

When I tried to code one, I've found one scenario added already: https://github.com/Sylius/Sylius/blob/master/features/inventory/cart_inventory/prevent_buying_more_products_than_available_in_a_stock.feature#L26-L30

Also, tested manually and it works at the current master:
image

@Coosos can you check it as well?

@Coosos
Copy link
Contributor

Coosos commented Jan 16, 2020

@lchrusciel

I've checked with demo version (1.5 and master) and I reproduced.
Example to reproduce (fresh installation) :

In \Sylius\Bundle\InventoryBundle\Validator\Constraints\InStockValidator, line 45, add (just after assert) :

$this->context->addViolation(
   $constraint->message,
   ['%itemName%' => 'Example item']
);

And with the browser, test to add a product, no error is displayed

image

With the fix, the error is displayed 👍

image

@Zales0123 Zales0123 closed this Jan 20, 2020
@Zales0123 Zales0123 reopened this Jan 20, 2020
@lchrusciel lchrusciel changed the base branch from 1.5 to 1.6 April 7, 2020 11:20
@lchrusciel lchrusciel merged commit c3a9ee5 into Sylius:1.6 Apr 7, 2020
@lchrusciel lchrusciel deleted the property-path branch April 7, 2020 11:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Potential Bug Potential bugs or bugfixes, that needs to be reproduced.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants