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

[API]Product image handling #12847

Merged
merged 3 commits into from
Jul 30, 2021
Merged

Conversation

arti0090
Copy link
Contributor

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

@arti0090 arti0090 requested a review from a team as a code owner July 28, 2021 08:39
@probot-autolabeler probot-autolabeler bot added the API APIs related issues and PRs. label Jul 28, 2021
@probot-autolabeler probot-autolabeler bot added the Maintenance CI configurations, READMEs, releases, etc. label Jul 28, 2021
@arti0090 arti0090 force-pushed the product-image-handling branch 3 times, most recently from 975c850 to ba52863 Compare July 28, 2021 08:55
@arti0090 arti0090 removed the Maintenance CI configurations, READMEs, releases, etc. label Jul 28, 2021
@probot-autolabeler probot-autolabeler bot added the Maintenance CI configurations, READMEs, releases, etc. label Jul 28, 2021
@arti0090 arti0090 force-pushed the product-image-handling branch 4 times, most recently from eac1b75 to c141d1c Compare July 28, 2021 09:06
@arti0090 arti0090 force-pushed the product-image-handling branch 6 times, most recently from 822230c to bcf281c Compare July 28, 2021 10:23
@@ -33,6 +33,11 @@ public function getConfigTreeBuilder(): TreeBuilder
->defaultFalse()
->end()
->end()
->children()
->variableNode('product_image_prefix')
->defaultValue('media/image')
Copy link
Member

Choose a reason for hiding this comment

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

I'm fine with defining this value as a parameter, however we should at least use another parameter, that will be used here as well:

data_root: "%sylius_core.public_dir%/media/image"

But once we will introduce the parameter there, I'm not sure we need it. Up to you.

@arti0090 arti0090 force-pushed the product-image-handling branch 2 times, most recently from c5a9a77 to db98dd4 Compare July 30, 2021 07:43
"id": "123",
"type": "thumbnail",
"path": "uo/product.jpg",
+ "media_path": "/media/image/uo/product.jpg"
Copy link
Member

Choose a reason for hiding this comment

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

If I see correctly, there is no more media_path, you are updating path field

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 have changed it a little now :)

@GSadee GSadee merged commit 5a8f8e8 into Sylius:master Jul 30, 2021
@GSadee
Copy link
Member

GSadee commented Jul 30, 2021

Thank you, @arti0090! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API APIs related issues and PRs. Maintenance CI configurations, READMEs, releases, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants