[Product] Product search for customer #6542
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pamil
reviewed
Oct 25, 2016
| * | ||
| * @return QueryBuilder | ||
| */ | ||
| public function createQueryBuilderForEnabledByTaxonCodeAndChannel($code, ChannelInterface $channel); | ||
| public function createQueryBuilderForEnabledByTaxonCodeAndChannel($code, ChannelInterface $channel, $locale); |
There was a problem hiding this comment.
It's not consistent, but in most repository methods we use $locale. I think it's clear enough, as in code it almost everywhere a locale code, not name, but we should obviously keep a consistency with this.
Zales0123
force-pushed
the
product-search-for-customer
branch
from
2509ae6
to
74eefc0
Compare
bretto36
reviewed
Oct 26, 2016
| @@ -51,14 +51,18 @@ public function createQueryBuilderWithLocaleCodeAndTaxonId($localeCode, $taxonId | |||
| /** | |||
| * {@inheritdoc} | |||
| */ | |||
| public function createQueryBuilderForEnabledByTaxonCodeAndChannel($code, ChannelInterface $channel) | |||
| public function createQueryBuilderForEnabledByTaxonCodeAndChannel($code, ChannelInterface $channel, $locale) | |||
There was a problem hiding this comment.
Should this have AndLocale in the function name?
pjedrzejewski
requested changes
Oct 26, 2016
| @@ -0,0 +1,4 @@ | |||
| {% form_theme form 'SyliusUiBundle:Form:theme.html.twig' %} | |||
|
|
|||
| {{ form_widget(form.type, { 'value': 'contains', 'attr': { 'style': 'display: none;' } }) }} | |||
There was a problem hiding this comment.
Twig CS! :) No spaces around { and '.
Zales0123
force-pushed
the
product-search-for-customer
branch
from
74eefc0
to
31e410a
Compare
| And I search for product with name "Magnum" | ||
| And I clear filter | ||
| Then I should see the product "44 Magnum" | ||
| And I should see the product "Glock 17" |
There was a problem hiding this comment.
How about scenarios that I'm browsing taxon X and I do not see products from taxon Y after search?
michalmarcinkowski
approved these changes
Oct 26, 2016
Zales0123
force-pushed
the
product-search-for-customer
branch
from
31e410a
to
0828783
Compare
bretto36
approved these changes
Oct 27, 2016
|
Thank you Mateusz! |
|
any ETA about when I could test this on http://demo.sylius.org/ ? |
|
any ETA on elasicsearch support yet, if any? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Maybe it doesn't look great, but at least it works 😄 🐫