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

Addding assertions for product in current channel #530

Merged
merged 3 commits into from
Sep 22, 2019

Conversation

mamazu
Copy link
Member

@mamazu mamazu commented Sep 5, 2019

Fixes #59

What does it do?

Currently you can add any product to any cart. After this pull request the product needs to be available in the cart's channel. So that if a product is in channel A and the cart is picked up in channel B then the product-add endpoint will now throw an error (500).

Why a 500 and not a 400 with validation errors?

The problem is that the Validation would need to take two properties of the request into consideration: the current cart token, the product code. Which means that it has to be a "Class Constraint" and needs to access properties of the request object. This is currently not possible as the current request objects don't have getter and the properties are private. So there is no way of validating them. Therefore this pull request only adds an assertion that it is true. (validation may follow if it will be possible to do so in the future)

PS: @JakobTolkemit you might be interested.

@mamazu mamazu requested a review from a team as a code owner September 5, 2019 21:52
@mamazu mamazu force-pushed the product_from_channel branch 2 times, most recently from 1365c5d to ebf6364 Compare September 5, 2019 22:01
@lchrusciel lchrusciel added the Bug label Sep 9, 2019
@lchrusciel
Copy link
Member

We should fix it before 1.0. Nevertheless, I would vote for opening Request object, as this solution would be much cleaner.

@mamazu
Copy link
Member Author

mamazu commented Sep 9, 2019

How would that work though?

@mamazu mamazu force-pushed the product_from_channel branch 4 times, most recently from a7f3295 to ae7c09c Compare September 14, 2019 13:48
@lchrusciel lchrusciel merged commit fc87069 into Sylius:master Sep 22, 2019
@lchrusciel
Copy link
Member

Thank you, @mamazu! 🎉

@mamazu mamazu deleted the product_from_channel branch September 22, 2019 09:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adding a product from different channel.
2 participants