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.

Sign up for GitHub

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

Jump to bottom

[API] [Product Review] Configure product review as a API resource with tests #11173

Merged
merged 3 commits into from
Mar 6, 2020
Merged

[API] [Product Review] Configure product review as a API resource with tests #11173

merged 3 commits into from
Mar 6, 2020

Conversation

AdamKasp
Copy link
Contributor

@AdamKasp AdamKasp commented Mar 3, 2020

Q A
Branch? api
Bug fix? no
New feature? yes
BC breaks? no
License MIT

@AdamKasp AdamKasp requested a review from a team as a code owner March 3, 2020 13:42
AdamKasp
AdamKasp commented Mar 3, 2020
View reviewed changes
src/Sylius/Behat/Context/Api/Admin/ManagingProductReviewsContext.php Outdated Show resolved Hide resolved
src/Sylius/Behat/Resources/config/services/contexts/api.xml Outdated Show resolved Hide resolved
src/Sylius/Bundle/ApiBundle/Resources/config/api_resources/ProductReview.xml
<attribute name="controller">sylius.api.state_machine_transition_applicator:accept</attribute>
</itemOperation>
<itemOperation name="reject_product_review">
<attribute name="method">PATCH</attribute>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know how configure serialization group to have empty request body in state machine transition paths

Copy link
Member

Choose a reason for hiding this comment

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

How does it look right now?

Copy link
Contributor Author

@AdamKasp AdamKasp Mar 5, 2020 •
edited

Choose a reason for hiding this comment

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

Screenshot 2020-03-05 at 14 58 12

but as I sad we should have an empty body

Copy link
Member

Choose a reason for hiding this comment

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

It is not possible to update any of these fields according to https://github.com/Sylius/Sylius/pull/11173/files#diff-8705efddce0ceb9c7db5b7c1aa26bdc9R30-R35 (at least I think so).

Something, that should be reiterated later, but we can accept the current solution right now.

@AdamKasp AdamKasp force-pushed the api-product-reviews-v3 branch 2 times, most recently from 39fdeb9 to 0c392c3 Compare March 4, 2020 09:56
@AdamKasp AdamKasp force-pushed the api-product-reviews-v3 branch 4 times, most recently from f8713c9 to a4d6239 Compare March 5, 2020 12:27
Zales0123
Zales0123 reviewed Mar 5, 2020
View reviewed changes
src/Sylius/Behat/Context/Api/Admin/ManagingProductReviewsContext.php Show resolved Hide resolved
src/Sylius/Behat/Context/Api/Admin/ManagingProductReviewsContext.php Outdated Show resolved Hide resolved
src/Sylius/Behat/Context/Api/Admin/ManagingProductReviewsContext.php Outdated Show resolved Hide resolved
src/Sylius/Behat/Context/Api/Admin/ManagingProductReviewsContext.php Outdated Show resolved Hide resolved
src/Sylius/Behat/Context/Api/Admin/ManagingProductReviewsContext.php Outdated Show resolved Hide resolved
src/Sylius/Behat/Resources/config/suites/api/product/managing_product_reviews.yml Show resolved Hide resolved
src/Sylius/Bundle/ApiBundle/Applicator/ProductReviewStateMachineTransitionApplicator.php Show resolved Hide resolved
src/Sylius/Bundle/ApiBundle/Resources/config/api_resources/Customer.xml Show resolved Hide resolved
src/Sylius/Bundle/ApiBundle/Resources/config/api_resources/ProductReview.xml
<attribute name="controller">sylius.api.state_machine_transition_applicator:accept</attribute>
</itemOperation>
<itemOperation name="reject_product_review">
<attribute name="method">PATCH</attribute>
Copy link
Member

Choose a reason for hiding this comment

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

How does it look right now?

@Zales0123 Zales0123 added API APIs related issues and PRs. Feature New feature proposals. labels Mar 5, 2020
@Sylius Sylius deleted a comment from AdamKasp Mar 5, 2020
@AdamKasp AdamKasp force-pushed the api-product-reviews-v3 branch 3 times, most recently from 71f492d to d47acaf Compare March 6, 2020 07:03
@AdamKasp AdamKasp force-pushed the api-product-reviews-v3 branch 2 times, most recently from e7f5219 to bd7a575 Compare March 6, 2020 08:38
GSadee
GSadee approved these changes Mar 6, 2020
View reviewed changes
src/Sylius/Behat/Client/ApiClientInterface.php Show resolved Hide resolved
src/Sylius/Behat/Client/ApiPlatformClient.php Show resolved Hide resolved
src/Sylius/Behat/Context/Api/Admin/ManagingProductReviewsContext.php Show resolved Hide resolved
src/Sylius/Behat/Context/Api/Admin/ManagingProductReviewsContext.php
/**
* @When I want to modify the :productReview product review
*/
public function iWantToModifyTheProductReview(ReviewInterface $productReview): void
Copy link
Member

Choose a reason for hiding this comment

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

Could be

Suggested change
public function iWantToModifyTheProductReview(ReviewInterface $productReview): void
public function iWantToModifyTheProductReview(ProductReviewInterface $productReview): void

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure because we don't have this interface :D
ProductReview`` extends Reviewwhich implementsReviewInterface` :)

src/Sylius/Behat/Context/Api/Admin/ManagingProductReviewsContext.php Show resolved Hide resolved
src/Sylius/Bundle/ApiBundle/Applicator/ProductReviewStateMachineTransitionApplicator.php
$this->stateMachineFactory = $stateMachineFactory;
}

public function accept(ReviewInterface $data): ReviewInterface
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
public function accept(ReviewInterface $data): ReviewInterface
public function accept(ReviewInterface $review): ReviewInterface

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it doesn't work with argument named different that $data in the documentation they mentioned about the argument in __invoke but I assume in rest of controller function we have the same rule (https://api-platform.com/docs/core/controllers/)

src/Sylius/Bundle/ApiBundle/Resources/config/services.xml Show resolved Hide resolved
@GSadee GSadee merged commit 0258f0d into Sylius:api Mar 6, 2020
@GSadee
Copy link
Member

GSadee commented Mar 6, 2020

Thanks, Adam! 🎉

@AdamKasp
Copy link
Contributor Author

AdamKasp commented Mar 9, 2020

all suggestions a fixed in #11197

@AdamKasp AdamKasp mentioned this pull request Mar 10, 2020
Merged
@lchrusciel
Copy link
Member

Part of #11250

@AdamKasp AdamKasp deleted the api-product-reviews-v3 branch July 19, 2021 06:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lchrusciel lchrusciel lchrusciel approved these changes

@Tomanhez Tomanhez Tomanhez left review comments

@GSadee GSadee GSadee approved these changes

@Zales0123 Zales0123 Zales0123 approved these changes

Assignees
No one assigned
Labels
API APIs related issues and PRs. Feature New feature proposals.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@AdamKasp @GSadee @lchrusciel @Zales0123 @Tomanhez