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

Scope image by product_id or variant_id in ImagesController #3510

Merged

Conversation

SamuelMartini
Copy link
Contributor

Description
The image route is under the product and variant resources:
/api/products/:product_id/images/:id
/api/variants/:variant_id/images/:id

When reading or performing any change on the product/variant image, the controller should scope the product or the variant when finding the image id.
The risk in only find by image id is to delete or read an image that belongs to another product.
The route path gives an expectation that the controller must meet: a request to a product or variant image.

NOTE: The product scope was present in the controller but removed by this pr #1580. Not clear how that removal was needed but putting back the scope now is not breaking any spec and the functionality in admin still working.

Checklist:

  • I have followed Pull Request guidelines
  • I have added a detailed description into each commit message
  • I have added tests to cover this change (if needed)

The image route is under the product and variant resources:
`/api/products/:product_id/images/:id`
`/api/variants/:variant_id/images/:id`

When reading or perfoming any change on the product/variant image, the controller
should scope the product or the variant when finding the image id.
The risk in only find by image id is to delete or read an image which belongs to
another product.
The route path give an expectation that the controller must meet:
a request to a product or variant image.
@SamuelMartini SamuelMartini force-pushed the SamuelMartini/scope_product_images branch from 5aeab81 to 30b9732 Compare February 7, 2020 15:17
Copy link
Member

@kennyadsl kennyadsl left a comment

Choose a reason for hiding this comment

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

Thanks, @SamuelMartini!

Copy link
Member

@spaghetticode spaghetticode left a comment

Choose a reason for hiding this comment

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

@SamuelMartini thank you 👍

@spaghetticode spaghetticode merged commit f6844df into solidusio:master Feb 14, 2020
@spaghetticode spaghetticode deleted the SamuelMartini/scope_product_images branch February 14, 2020 10:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants