Skip to content
This repository was archived by the owner on Aug 19, 2025. It is now read-only.

Conversation

@sebbulon
Copy link
Contributor

WIP, added search keyword action

Copy link
Member

Choose a reason for hiding this comment

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

👍

@sebbulon sebbulon force-pushed the add_search_keyword_delta branch from 3ad2898 to f47d076 Compare December 15, 2015 08:41
@sebbulon
Copy link
Contributor Author

@PhilippSpo happy for me to merge this? Tests go green :)

@emmenko
Copy link
Member

emmenko commented Dec 15, 2015

@sebbulon one question: search keywords can be defined with different tokenizers. Do you consider this in the implementation? Or how does that work with the csv import?

@sebbulon
Copy link
Contributor Author

@emmenko indeed .. custom tokenisers are not supported right now by this PR

@emmenko
Copy link
Member

emmenko commented Dec 15, 2015

So how is this going to work?

@PhilippSpo
Copy link
Contributor

@sebbulon There are no tests for updating existing search keywords, right?

@sebbulon
Copy link
Contributor Author

@emmenko I didn't really change anything there as you can see. The CSV import already supported search keyword export to some extend, I added Search Keyword import - there was only one little change necessary.
When you create the product you need to select the tokeniser.
If you create the product with this tool right now, it will use default tokeniser - which seems to be not white space, which is not ideal I have to admit.

I guess we need to make this clear in the documentation + have another change request to support the different tokenisers.

@sebbulon
Copy link
Contributor Author

@PhilippSpo right tests are not complete

@sebbulon
Copy link
Contributor Author

@emmenko to answer your question - the ususal case how people use this is to tokenise the search keywords already. If they are provided separated by semicolon (or whatever the secondary delimiter is), it will be imported into separate text elements, each of them being a different token. So they technically don't need a whitespace tokeniser.
Of course we need to add the support later down the line though to support the complete feature.

I will add this to the readme

@sebbulon sebbulon force-pushed the add_search_keyword_delta branch from 89a39ef to 5fa18d9 Compare December 15, 2015 11:53
philippspo added 5 commits December 15, 2015 22:18
…-node-product-csv-sync into add_search_keyword_delta
To check whether the existing search keywords sayed untouched by the import
…sync into add_search_keyword_delta

# Conflicts:
#	README.md
#	package.json
apparently only sku is not enough - the variantId also needs to be provided (which might be problematic for customers)
@PhilippSpo
Copy link
Contributor

OK we can merge this in my opinion.
Later on we also need to add support for only updating search keywords of a specific language, since this is a frequent use case. The problem here is, that there is no update action for only updating the searchKeywords of one language. Thus we would need to enrich the action with the current search keywords before sending the update action..

hajoeichler added a commit that referenced this pull request Feb 24, 2016
work in progress, add search keyword update action
@hajoeichler hajoeichler merged commit 8fbf90b into master Feb 24, 2016
@hajoeichler hajoeichler deleted the add_search_keyword_delta branch February 24, 2016 14:13
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants