-
-
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][Product] Expose default variant on product show and index #12649
Conversation
GSadee
commented
May 19, 2021
Q | A |
---|---|
Branch? | master |
Bug fix? | no |
New feature? | yes |
BC breaks? | no |
Deprecations? | no |
Related tickets | |
License | MIT |
c3ff2c1
to
1639cb0
Compare
{ | ||
Assert::isInstanceOf($object, ProductInterface::class); | ||
|
||
$context[self::ALREADY_CALLED] = true; |
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.
Perhaps we should add assert of early return in case this class is already called indeed?
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.
IMO there is no need, because this check is in supportsNormalization
method, but I can add it to be in 100% secured
|
||
public function supportsNormalization($data, $format = null, $context = []): bool | ||
{ | ||
if (isset($context[self::ALREADY_CALLED])) { |
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 would add one more spec for testing of this logic
Thanks, Grzegorz! 🥇 |
…er (GSadee) This PR was merged into the 1.10-dev branch. Discussion ---------- | Q | A | --------------- | ----- | Branch? | master | Bug fix? | yes? | New feature? | no | BC breaks? | no | Deprecations? | no | Related tickets | fixes ##12649 | License | MIT Commits ------- ef97569 [API][Product] Add additional specs for product normalizer
…er (GSadee) This PR was merged into the 1.10-dev branch. Discussion ---------- | Q | A | --------------- | ----- | Branch? | master | Bug fix? | yes? | New feature? | no | BC breaks? | no | Deprecations? | no | Related tickets | fixes #Sylius/Sylius#12649 | License | MIT Commits ------- ef975695645c3b0176d733da55a857aa77f2bfc5 [API][Product] Add additional specs for product normalizer