Skip to content

Conversation

@cewald
Copy link
Contributor

@cewald cewald commented Nov 21, 2019

Related issues

I needed a possibility to simply add an or/should filter to the quickSearchByQuery method.
This wasn't possible yet, so I extended the prepareElasticsearchQueryBody method of the ElasticSearch search-adapter to accept an or operator in the value property.
I also added an ES exists filter if the value is null if you just wan't the field to be null.

Short description and why it's useful

I had to make a product query where I wanted a collection of products with a specific value or an empty one. This wasn't possible yet because all filters are chained as 'AND' filters.

Now you can make query like this (or in the last two lines) to show all products with custom_attribute is 2 or 3 or custom_attribute is just empty (null):

let query = new SearchQuery()
query
  .applyFilter({ key: 'visibility', value: { in: [2, 3, 4] } })
  .applyFilter({ key: 'status', value: { in: [0, 1] } })
  .applyFilter({ key: 'category_ids', value: { in: [categoryId] } })
  .applyFilter({ key: 'custom_attribute', value: { or: null } })
  .applyFilter({ key: 'custom_attribute', value: { or: [2, 3] } })

In our use-case the products have clusters, so I wan't to show products to the customer which have his current cluster OR if there are not enough of them just regular products without a cluster.

With this changes I can make queries like this to the ES while I still can use all the advantages of quickSearchByQuery:

{
  "filter": {
    "bool": {
      "should": [
        {
          "terms": {
            "custom_attribute": [2, 3]
          }
        },
        {
          "bool": {
            "must_not": [
              {
                "exists": {
                  "field": "custom_attribute"
                }
              }
            ]
          }
        }
      ]
    }
  }
}

If you think it isn't an essential functionallity, just ignore and close this PR.
But I thought this might come in handy.

Which environment this relates to

Check your case. In case of any doubts please read about Release Cycle

  • Test version (https://test.storefrontcloud.io) - this is a new feature or improvement for Vue Storefront. I've created branch from develop branch and want to merge it back to develop
  • RC version (https://next.storefrontcloud.io) - this is a stabilisation fix for Release Candidate of Vue Storefront. I've created branch from release branch and want to merge it back to release
  • Stable version (https://demo.storefrontcloud.io) - this is an important fix for current stable version. I've created branch from hotfix or master branch and want to merge it back to hotfix

Upgrade Notes and Changelog

  • No upgrade steps required (100% backward compatibility and no breaking changes)
  • I've updated the Upgrade notes and Changelog on how to port existing VS sites with this new feature

IMPORTANT NOTICE - Remember to update CHANGELOG.md with description of your change

Contribution and currently important rules acceptance

Christian Ewald added 30 commits August 2, 2019 10:59
* Update translations
* Fix border rounded bug in footer newsletter input
* Bugfix for category header when loaded client-site
* Add todo comment
* Prefetch category-extras data without VSF asyncData method to prevent bug with `routeStore.route`  in getters (vuestorefront/vue-storefront#3328)
…od which is not cached to have a correct categories array when switching route from client-side
Remove default homepage and its assets and bugfix for getCurrentCategory
* Add small improvement to getters like in official PR for this bug
* Add some styling to placeholder home page
* Small improvements
* Transform parameter values type in `getCategoryByParams` to category value type
* Add support for fetching multiple entries at once of CMS (particularly `Storyblok`) via `icmaa-cms/list` action abstract method
* Add `icmaa-spotify` module to populate state with Spotify recommendations
* Improve `icmaa-category` overwrites for `catalog-next` module
* Improve some namespacing of `icmaa-cms` mutations
* Improve namespacing of  custom `icmaa`-module getters
* Use module names for our imports
* Improve icmaa-category/list
* Improve category-header styling (Draft)
* Bugfix for headers z-index
* For footer service logos and language-flags
* Create module inheritation for store-view configs
* Add default store-view meta-data (DRAFT)
* Add href-lang and canonical tags
* Remove unused CSS class (Tailwind) using PurgeCSS
* Add `postcss-clean` for minification of CSS
Christian Ewald added 16 commits December 12, 2019 17:56
* Show just current path if no category is in parameters
* Update README.md for `icmaa-url`
* Show default category header image if mobile image is missing
* Give custom url rewrites a higher priority
* Add changes of latest `DivanteLtd/develop` merge to our core overwrites
* Improve `ProductListingWidget`
  * Load products using `bodybuilder` query
  * Change products if cluster changes
* Remove custom search-adapter including the and/or options – we do this using a plain `bodybuilder` query
* Set cluster on product and category pages using custom mixin
* This is the latest `v1.11` release
@pkarw
Copy link
Collaborator

pkarw commented Dec 27, 2019

Related to #3934
@andrzejewsky its a good first step to make SearchQuery extendable
I’d suggest extracting the SearchQuery2Elastic as a separate repo / non even - make it extendable and useful for both vs-api and sf-api (as it should be removed from the VS itself: #2167

@pkarw pkarw requested a review from andrzejewsky December 27, 2019 09:02
@andrzejewsky andrzejewsky changed the base branch from release/v1.11 to develop December 27, 2019 16:55
Copy link
Contributor

@andrzejewsky andrzejewsky left a comment

Choose a reason for hiding this comment

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

looks good for me 👍

@andrzejewsky
Copy link
Contributor

@cewald can you rebase with the current develop branch? I wanted to do this on my own but I don't have access to your repository.

@cewald
Copy link
Contributor Author

cewald commented Dec 28, 2019

@andrzejewsky Sorry, but I messed up this branch by rebasing it with the wrong branch (too many repos, branches and remotes :)). I re-added the changes to a clean branch and opened up a new PR #3960 so we can close this one.

@pkarw
Copy link
Collaborator

pkarw commented Dec 28, 2019

Thanks Christian

@pkarw pkarw closed this Dec 28, 2019
@cewald cewald deleted the feature/elasticsearch-with-and-or-null branch December 28, 2019 09:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants