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

Adding a "reference" field definition : ability to search by SKU. #129

Closed

Conversation

romainruaud
Copy link
Collaborator

This a proposal of implementation according to discussion in #100

Tested working against Luma default SKU values (MH06, MS09 etc...) and also with EAN13 values (3459379414678).

Let me know about it.

@romainruaud
Copy link
Collaborator Author

This PR brokes the Travis build because the "Field" class has now a complexity of 51... was 50 before.
Seems hard to optimize, let me know if we can ignore or not if we decide to merge this one.

@southerncomputer
Copy link
Contributor

Have you thought about pushing this patch into 2.3.x? it would help with complex sku field accuracy!

@afoucret
Copy link
Contributor

afoucret commented Dec 1, 2016

@southerncomputer :
I am not huge fan of this patch and work on a better solution that should be more generic and address the general problem of using custom analyzer on specific fields.
I fear, you will have to wait 2.4.0 for this patch. At the same time I don't think merging temporary solution is a good idea, since it can have side effect on ascendant compatibility ...

@afoucret afoucret modified the milestones: 2.3, 2.4 Jan 4, 2017
@maximehuran
Copy link

I need this functionality very quickly, if I switch to the 2.4 branch, will I can search products by SKU ?

@afoucret
Copy link
Contributor

This PR will not be merged since a better implementation have been proposed (PR #300)

@afoucret afoucret closed this Jan 19, 2017
@romainruaud romainruaud deleted the feature_search-by-sku branch March 15, 2018 11:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants