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

Conversation

@PhilippSpo
Copy link
Contributor

@emmenko @hajoeichler @mmoelli
This is not the most beautiful solution, but it works for the moment.
We can now filter variants by attributes and we can filter the variants prices.
If a variant has no prices left, after the price filter was applied to it, it gets removed from the export. This gives use the possibility to export for certain countries only.
What do you think about this approach?

Copy link
Member

Choose a reason for hiding this comment

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

How do those 2 filter and the queryString options interfere with each other. Can they all be chosen independent?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The 2 filters are not added to the query, but are applied to the result. So yes they can be chosen independently.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the explanation. Makes sense ;)

philippspo added 2 commits February 1, 2016 17:40
if the price filter removes all prices from the variant, then the whole variant should be removed since the variant is not relevant regarding the filter
@emmenko
Copy link
Member

emmenko commented Feb 2, 2016

@PhilippSpo btw: we should prevent using the search endpoint for exports.

@PhilippSpo
Copy link
Contributor Author

@emmenko so how about getting rid of the queryType option and default to query?

@hajoeichler
Copy link
Member

Sound reasonable for me to remove the search option.

@butenkor
Copy link
Member

butenkor commented Feb 3, 2016

Why not using search endpoint for exports? Is it some common rule or just currently bad practice?

@PhilippSpo
Copy link
Contributor Author

@butenkor I just asked @agourlay on this and he gave me a great response:

  1. the search endpoint is eventually consistent
  2. the search endpoint is meant for "search" not for dumping large amount of data efficiently.

We would need to expose the scan/scroll api from ES for that https://www.elastic.co/guide/en/elasticsearch/reference/current/search-request-scroll.html
I guess you can use ES for dumping large amount of data right now, I am just not sure you will like the performance you will get

Thanks for clarifying @agourlay 👍

@butenkor
Copy link
Member

butenkor commented Feb 3, 2016

Thanks for info.

@PhilippSpo
Copy link
Contributor Author

OK I fixed the tests locally. We are experiencing some coveralls issues here...

PhilippSpo added a commit that referenced this pull request Feb 4, 2016
…ilters

feat(export): add variant and price filters
@PhilippSpo PhilippSpo merged commit 543f910 into master Feb 4, 2016
@PhilippSpo PhilippSpo deleted the feature/add-variant-and-price-filters branch February 4, 2016 16:06
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.

5 participants