-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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]Unify change quantity endpoint with other orders endpoints #12530
Conversation
arti0090
commented
Apr 14, 2021
•
edited
Loading
edited
Q | A |
---|---|
Branch? | master |
Bug fix? | no |
New feature? | no |
BC breaks? | no |
Deprecations? | no |
License | MIT |
src/Sylius/Bundle/ApiBundle/Command/Cart/ChangeItemQuantityInCart.php
Outdated
Show resolved
Hide resolved
{ | ||
public function transform($object, string $to, array $context = []) | ||
{ | ||
$object->setOrderItemId($this->getOrderItemIdFromUri($context['request_uri'])); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if you can get some more information from context to choose this id in a better way instead of getting the last part of IRI. The current solution works for the implemented endpoint, but is not a generic one, what could be problematic in future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunatelly there are 3 places where there is ID 1st one was this, 2nd was full request ( with hostname) and 3rd place were orderItemId's that were in order ( but this is not something we could use) hence I choose this one
src/Sylius/Bundle/ApiBundle/DataTransformer/OrderItemIdAwareInputCommandDataTransformer.php
Outdated
Show resolved
Hide resolved
c41e82b
to
76a573a
Compare
public function transform($object, string $to, array $context = []) | ||
{ | ||
$explodedUri = explode('/', $context['request_uri']); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we fetch from the context that this is really our endpoint with orderItemId
at the end?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This Transformer is longer exist :D thanks to your comment I changed it to subresource one and it works fine
…ody value to quantity
6f81902
to
d161006
Compare
|
||
public function setSubresourceId(?string $subresourceId): void | ||
{ | ||
$this->orderItemId= $subresourceId; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$this->orderItemId= $subresourceId; | |
$this->orderItemId = $subresourceId; |
Thank you, @arti0090! 🥇 |