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

[Doc] fix checkoutState parameter in PlacedOrder swagger definition #611

Merged
merged 3 commits into from
Jan 16, 2020
Merged

[Doc] fix checkoutState parameter in PlacedOrder swagger definition #611

merged 3 commits into from
Jan 16, 2020

Conversation

EmilMassey
Copy link
Contributor

I think the intention here was to display order's state (then the enum would be correct), but in fact checkout state is returned as in Cart definition.

Note that the checkout state will always be completed for placed orders.

@EmilMassey EmilMassey requested a review from a team as a code owner December 11, 2019 13:49
@EmilMassey
Copy link
Contributor Author

EmilMassey commented Dec 11, 2019

I think displaying order's state would be useful for some, though. Do you think we should add it to the view? I could work on implementation in other PR.

Copy link
Member

@mamazu mamazu left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution.

doc/swagger.yml Outdated Show resolved Hide resolved
@mamazu
Copy link
Member

mamazu commented Dec 11, 2019

The default state is probably cart for carts and only completed for orders.

Copy link
Member

@lchrusciel lchrusciel left a comment

Choose a reason for hiding this comment

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

This parameter will represent a checkout state indeed. Thanks for posting it 👍

doc/swagger.yml Outdated Show resolved Hide resolved
@mamazu mamazu merged commit ca7992b into Sylius:master Jan 16, 2020
@mamazu
Copy link
Member

mamazu commented Jan 16, 2020

Thanks, Emil! 🥇

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants