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][Admin] Managing product images #15509

Merged
merged 22 commits into from Nov 30, 2023
Merged

[API][Admin] Managing product images #15509

merged 22 commits into from Nov 30, 2023

Conversation

GSadee
Copy link
Member

@GSadee GSadee commented Nov 8, 2023 •
edited

Q A
Branch? 1.13
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Related tickets based on #15504
License MIT
image

@GSadee GSadee added Feature New feature proposals. Admin AdminBundle related issues and PRs. API APIs related issues and PRs. labels Nov 8, 2023
@GSadee GSadee changed the title [WIP][API][Admin] Managing product images [API][Admin] Managing product images Nov 14, 2023
@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 14, 2023 •
edited

Bunnyshell Preview Environment deleted

Available commands:

  • /bns:deploy to redeploy the environment

@GSadee GSadee force-pushed the api-managing-product-images branch 3 times, most recently from 1ce247c to de35ae5 Compare November 14, 2023 12:47
@GSadee GSadee marked this pull request as ready for review November 16, 2023 06:32
@GSadee GSadee requested a review from a team as a code owner November 16, 2023 06:32
diimpp
diimpp reviewed Nov 23, 2023
View reviewed changes
features/product/managing_products/changing_images_of_product.feature Show resolved Hide resolved
...ures/product/viewing_product_in_admin_panel/viewing_details_of_product_with_variants.feature Show resolved Hide resolved
src/Sylius/Behat/Context/Api/Admin/ManagingProductImagesContext.php Outdated Show resolved Hide resolved
src/Sylius/Behat/Context/Api/Admin/ManagingProductImagesContext.php Outdated Show resolved Hide resolved
src/Sylius/Behat/Context/Api/Admin/ManagingProductImagesContext.php Outdated Show resolved Hide resolved
src/Sylius/Bundle/ApiBundle/Resources/config/services.xml Outdated Show resolved Hide resolved
...ylius/Bundle/CoreBundle/Validator/Constraints/ProductImageVariantsBelongToOwnerValidator.php Outdated Show resolved Hide resolved
...dle/CoreBundle/spec/Validator/Constraints/ProductImageVariantsBelongToOwnerValidatorSpec.php Outdated Show resolved Hide resolved
...dle/CoreBundle/spec/Validator/Constraints/ProductImageVariantsBelongToOwnerValidatorSpec.php Show resolved Hide resolved
src/Sylius/Bundle/ApiBundle/Creator/ProductImageCreator.php Show resolved Hide resolved
@GSadee GSadee force-pushed the api-managing-product-images branch 3 times, most recently from e33ec2d to 6f9a61a Compare November 24, 2023 11:54
Rafikooo
Rafikooo previously approved these changes Nov 24, 2023
View reviewed changes
diimpp
diimpp previously approved these changes Nov 25, 2023
View reviewed changes
Wojdylak
Wojdylak previously approved these changes Nov 27, 2023
View reviewed changes
@GSadee
[Behat][API][Admin] Cover changing images of product
9b9cb4b
@GSadee
[API] Fixes to managing product images according to the changes on ma…
7a920da 
…naging taxon images
@GSadee
[API][Admin] Add managing product variants on product images
078ea04
@GSadee
[API][Admin] Validate product variants on product images
2bb655e
@GSadee
[Behat][API][Admin] Add scenarios for managing product variants on pr…
430fdcf 
…oduct images
@GSadee
[API][Admin] Add filtering product images by product variant
c1868ab
@GSadee
Fix static analysis
26a3070
@GSadee
[API][Admin] Add missing product variants of images to product response
4fd7216
@GSadee
[API][Admin] Fix problem with getting non-scalar value from parameter…
a35f10b 
… bag
@GSadee
[API][Admin] Improve request body schema for product image POST endpoint
ea2e870
@GSadee
[API] Extract controllers definitions to a separate file
a0d0e34
@GSadee
[API] Fixes after PR review
39338b9
@GSadee
[Core] Fix service id of validator for checking product images variants
13acfa6
@GSadee GSadee dismissed stale reviews from Wojdylak, diimpp, and Rafikooo via 13acfa6 November 28, 2023 13:45
diimpp
diimpp approved these changes Nov 28, 2023
View reviewed changes
tests/Api/Admin/ProductImagesTest.php
@@ -36,49 +38,262 @@ public function it_denies_access_to_a_product_images_list_for_not_authenticated_
/** @test */
public function it_gets_all_product_images(): void
{
$this->loadFixturesFromFiles(['product/product_image.yaml', 'authentication/api_administrator.yaml']);
$this->loadFixturesFromFiles(['authentication/api_administrator.yaml', 'product/product_image.yaml']);
$header = array_merge($this->logInAdminUser('api@example.com'), self::CONTENT_TYPE_HEADER);
Copy link
Member

Choose a reason for hiding this comment

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

Would be cool to make use of HeadersBuilder https://github.com/Sylius/Sylius/blob/1.13/tests/Api/Utils/HeadersBuilder.php here and in other places in this test, since it was introduces last week.

Copy link
Member Author

Choose a reason for hiding this comment

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

👍🏻 but I would be for doing it in the separate PR

@NoResponseMate NoResponseMate merged commit 098e18b into Sylius:1.13 Nov 30, 2023
25 checks passed
@NoResponseMate
Copy link
Contributor

Thank you, @GSadee!

@GSadee GSadee deleted the api-managing-product-images branch November 30, 2023 12:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@diimpp diimpp diimpp approved these changes

@NoResponseMate NoResponseMate NoResponseMate approved these changes

@Wojdylak Wojdylak Wojdylak approved these changes

@Rafikooo Rafikooo Rafikooo left review comments

Assignees
No one assigned
Labels
Admin AdminBundle related issues and PRs. 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
@GSadee @NoResponseMate @diimpp @Wojdylak @Rafikooo