From 074be23ed203dc2d6aa67d3a1a572781814dd5c1 Mon Sep 17 00:00:00 2001
From: Dmitriy Baklikov <dmitriy@jetruby.com>
Date: Tue, 23 Feb 2016 17:58:27 +0300
Subject: [PATCH 1/9] Switch to aggregations instead of facets for
 Elasticsearch 2.x.

---
 app/helpers/spree/base_helper_decorator.rb    |  29 +-
 app/models/spree/product_decorator.rb         |  47 +--
 app/views/spree/shared/_filter_price.html.erb |  44 +--
 .../spree/shared/_filter_properties.html.erb  |  26 +-
 app/views/spree/shared/_filters.html.erb      |  12 +-
 spec/models/products_spec.rb                  | 319 ++++++++++--------
 6 files changed, 259 insertions(+), 218 deletions(-)

diff --git a/app/helpers/spree/base_helper_decorator.rb b/app/helpers/spree/base_helper_decorator.rb
index 91b7612..2f29bd1 100644
--- a/app/helpers/spree/base_helper_decorator.rb
+++ b/app/helpers/spree/base_helper_decorator.rb
@@ -3,16 +3,16 @@ module Spree
     # parses the properties facet result
     # input: Facet(name: "properties", type: "terms", body: {"terms" => [{"term" => "key1||value1", "count" => 1},{"term" => "key1||value2", "count" => 1}]}])
     # output: Facet(name: key1, type: terms, body: {"terms" => [{"term" => "value1", "count" => 1},{"term" => "value2", "count" => 1}]})
-    def expand_properties_facet_to_facet_array(facet)
+    def expand_properties_aggregation_to_aggregation_array(aggregation)
       # first step is to build a hash
       # {"property_name" => [{"term" => "value1", "count" => 1},{"term" => "value2", "count" => 1}]}}
       property_names = {}
-      facet["terms"].each do |term|
-        t = term["term"].split("||")
+      aggregation[:buckets].each do |term|
+        t = term[:key].split('||')
         property_name = t[0]
         property_value = t[1]
         # add a search_term to each term hash to allow searching on the element later on
-        property = {"term" => property_value, "count" => term["count"], "search_term" => term["term"]}
+        property = { term: property_value, count: term[:doc_count], search_term: term[:key] }
         if property_names.has_key?(property_name)
           property_names[property_name] << property
         else
@@ -24,10 +24,9 @@ def expand_properties_facet_to_facet_array(facet)
       # format: Facet(name: "property_name", type: type, body: {"terms" => [{"term" => "value1", "count" => 1},{"term" => "value2", "count" => 1}]}])
       result = {}
       property_names.each do |key,value|
-        value.sort_by!{|h| [-h["count"],h["term"].downcase]} # first sort on desc, then on term asc
+        value.sort_by!{ |value| [-value[:count], value[:term].downcase] } # first sort on desc, then on term asc
         # result << Spree::Search::Elasticsearch::Facet.new(name: key, search_name: facet.name, type: facet.type, body: {"terms" => value})
         result[key] = {
-          '_type' => facet['_type'],
           'terms' => value
         }
       end
@@ -37,19 +36,19 @@ def expand_properties_facet_to_facet_array(facet)
     # Helper method for interpreting facets from Elasticsearch. Something like a before filter.
     # Sorting, changings things, the world is your oyster
     # Input is a hash
-    def process_facets(facets)
-      new_facets = {}
+    def process_aggregations(aggregations)
+      new_aggregations = {}
       delete_keys = []
-      facets.map do |key, facet|
-        if key == "properties"
-          new_facets.merge! expand_properties_facet_to_facet_array(facet)
+      aggregations.map do |key, aggregation|
+        if key == 'properties'
+          new_aggregations.merge! expand_properties_aggregation_to_aggregation_array(aggregation)
           delete_keys << :properties
         else
-          facet
+          aggregation
         end
       end
-      delete_keys.each {|key| facets.delete(key) }
-      facets.merge! new_facets
+      delete_keys.each { |key| aggregations.delete(key) }
+      aggregations.merge! new_aggregations
     end
   end
-end
\ No newline at end of file
+end
diff --git a/app/models/spree/product_decorator.rb b/app/models/spree/product_decorator.rb
index c432777..b05239f 100644
--- a/app/models/spree/product_decorator.rb
+++ b/app/models/spree/product_decorator.rb
@@ -5,11 +5,12 @@ module Spree
     index_name Spree::ElasticsearchSettings.index
     document_type 'spree_product'
 
-    mapping _all: {"index_analyzer" => "nGram_analyzer", "search_analyzer" => "whitespace_analyzer"} do
+    mapping _all: { analyzer: 'nGram_analyzer', search_analyzer: 'whitespace_analyzer' } do
       indexes :name, type: 'multi_field' do
         indexes :name, type: 'string', analyzer: 'nGram_analyzer', boost: 100
         indexes :untouched, type: 'string', include_in_all: false, index: 'not_analyzed'
       end
+
       indexes :description, analyzer: 'snowball'
       indexes :available_on, type: 'date', format: 'dateOptionalTime', include_in_all: false
       indexes :price, type: 'double'
@@ -77,7 +78,7 @@ class Product::ElasticsearchQuery
       #   filter: { range: { price: { lte: , gte: } } },
       #   sort: [],
       #   from: ,
-      #   facets:
+      #   aggregations:
       # }
       def to_hash
         q = { match_all: {} }
@@ -92,31 +93,31 @@ def to_hash
           # to { terms: { properties: ["key1||value_a","key1||value_b"] }
           #    { terms: { properties: ["key2||value_a"] }
           # This enforces "and" relation between different property values and "or" relation between same property values
-          properties = @properties.map {|k,v| [k].product(v)}.map do |pair|
-            and_filter << { terms: { properties: pair.map {|prop| prop.join("||")} } }
+          properties = @properties.map{ |key, value| [key].product(value) }.map do |pair|
+            and_filter << { terms: { properties: pair.map { |property| property.join('||') } } }
           end
         end
 
         sorting = case @sorting
-        when "name_asc"
-          [ {"name.untouched" => { order: "asc" }}, {"price" => { order: "asc" }}, "_score" ]
-        when "name_desc"
-          [ {"name.untouched" => { order: "desc" }}, {"price" => { order: "asc" }}, "_score" ]
-        when "price_asc"
-          [ {"price" => { order: "asc" }}, {"name.untouched" => { order: "asc" }}, "_score" ]
-        when "price_desc"
-          [ {"price" => { order: "desc" }}, {"name.untouched" => { order: "asc" }}, "_score" ]
-        when "score"
-          [ "_score", {"name.untouched" => { order: "asc" }}, {"price" => { order: "asc" }} ]
+        when 'name_asc'
+          [ { 'name.untouched' => { order: 'asc' } }, { price: { order: 'asc' } }, '_score' ]
+        when 'name_desc'
+          [ { 'name.untouched' => { order: 'desc' } }, { price: { order: 'asc' } }, '_score' ]
+        when 'price_asc'
+          [ { 'price' => { order: 'asc' } }, { 'name.untouched' => { order: 'asc' } }, '_score' ]
+        when 'price_desc'
+          [ { 'price' => { order: 'desc' } }, { 'name.untouched' => { order: 'asc' } }, '_score' ]
+        when 'score'
+          [ '_score', { 'name.untouched' => { order: 'asc' } }, { price: { order: 'asc' } } ]
         else
-          [ {"name.untouched" => { order: "asc" }}, {"price" => { order: "asc" }}, "_score" ]
+          [ { 'name.untouched' => { order: 'asc' } }, { price: { order: 'asc' } }, '_score' ]
         end
 
-        # facets
-        facets = {
-          price: { statistical: { field: "price" } },
-          properties: { terms: { field: "properties", order: "count", size: 1000000 } },
-          taxon_ids: { terms: { field: "taxon_ids", size: 1000000 } }
+        # aggregations
+        aggregations = {
+          price: { stats: { field: 'price' } },
+          properties: { terms: { field: 'properties', order: { _count: 'asc' }, size: 1000000 } },
+          taxon_ids: { terms: { field: 'taxon_ids', size: 1000000 } }
         }
 
         # basic skeleton
@@ -125,7 +126,7 @@ def to_hash
           query: { filtered: {} },
           sort: sorting,
           from: from,
-          facets: facets
+          aggregations: aggregations
         }
 
         # add query and filters to filtered
@@ -133,8 +134,8 @@ def to_hash
         # taxon and property filters have an effect on the facets
         and_filter << { terms: { taxon_ids: taxons } } unless taxons.empty?
         # only return products that are available
-        and_filter << { range: { available_on: { lte: "now" } } }
-        result[:query][:filtered][:filter] = { "and" => and_filter } unless and_filter.empty?
+        and_filter << { range: { available_on: { lte: 'now' } } }
+        result[:query][:filtered][:filter] = { and: and_filter } unless and_filter.empty?
 
         # add price filter outside the query because it should have no effect on facets
         if price_min && price_max && (price_min < price_max)
diff --git a/app/views/spree/shared/_filter_price.html.erb b/app/views/spree/shared/_filter_price.html.erb
index b8fcd90..5ede448 100644
--- a/app/views/spree/shared/_filter_price.html.erb
+++ b/app/views/spree/shared/_filter_price.html.erb
@@ -1,14 +1,14 @@
-<fieldset class="facet">
-  <h4 class="filter"><%= facet.name %></h4>
-  <div class="facet-box">
-    <div class="slider">
-      <div class="rangeslider_top_margin"></div>
-      <input type="text" id="price" />
+<fieldset class='facet'>
+  <h4 class='filter'><%= aggregation.name %></h4>
+  <div class='facet-box'>
+    <div class='slider'>
+      <div class='rangeslider_top_margin'></div>
+      <input type='text' id='price' />
     </div>
-    <div class="slider-input">
-      <input type="text" id="price-min" name="search[price][min]" value"<%= facet["min"] %>">
+    <div class='slider-input'>
+      <input type='text' id='price-min' name='search[price][min]' value="<%= aggregation['min'] %>">
       <span>to</span>
-      <input type="text" id="price-max" name="search[price][max]" value"<%= facet["max"] %>">
+      <input type='text' id='price-max' name='search[price][max]' value="<%= aggregation['max'] %>">
     </div>
   </div>
 </fieldset>
@@ -16,22 +16,22 @@
 <% content_for :head do %>
   <script type='text/javascript'>
     $(document).ready(function(){
-      $("#price-min").change(function () {
+      $('#price-min').change(function () {
         // check if not below minimum
-        $("#price").ionRangeSlider("update", {
+        $('#price').ionRangeSlider('update', {
           from: $(this).val()
         });
       });
 
-      $("#price-max").change(function () {
+      $('#price-max').change(function () {
         // check if not above maximum
-        $("#price").ionRangeSlider("update", {
+        $('#price').ionRangeSlider('update', {
           to: $(this).val()
         });
       });
 
-      minimum = <%= (facet["min"] == facet["max"] ? (facet["min"] - 1) : facet["min"]).floor %>;
-      maximum = <%= (facet["max"] == facet["min"] ? (facet["min"] + 1) : facet["max"]).ceil %>;
+      minimum = <%= (aggregation['min'] == aggregation['max'] ? (aggregation['min'] - 1) : aggregation['min']).floor %>;
+      maximum = <%= (aggregation['max'] == aggregation['min'] ? (aggregation['min'] + 1) : aggregation['max']).ceil %>;
       min_param = minimum;
       <% if params[:search] && params[:search][:price] %>
         min_param = <%= params[:search][:price][:min] %>
@@ -41,7 +41,7 @@
         max_param = <%= params[:search][:price][:max] %>
       <% end %>
 
-      $("#price").ionRangeSlider({
+      $('#price').ionRangeSlider({
         min: minimum,
         max: maximum,
         from: min_param,
@@ -51,13 +51,13 @@
         prefix: "<%= ::Money.new(1, Spree::Config[:currency]).symbol %>",
         prettify: false,
         hasGrid: false,
-        onLoad: function(obj) { 
-          $("#price-min").val(obj.fromNumber);
-          $("#price-max").val(obj.toNumber);
+        onLoad: function(obj) {
+          $('#price-min').val(obj.fromNumber);
+          $('#price-max').val(obj.toNumber);
         },
-        onChange: function(obj) { 
-          $("#price-min").val(obj.fromNumber);
-          $("#price-max").val(obj.toNumber);
+        onChange: function(obj) {
+          $('#price-min').val(obj.fromNumber);
+          $('#price-max').val(obj.toNumber);
         }
       });
     });
diff --git a/app/views/spree/shared/_filter_properties.html.erb b/app/views/spree/shared/_filter_properties.html.erb
index d4d895d..fe97020 100644
--- a/app/views/spree/shared/_filter_properties.html.erb
+++ b/app/views/spree/shared/_filter_properties.html.erb
@@ -1,16 +1,16 @@
-<% facets.each do |facet_name, facet| %>
-  <fieldset class="facet">
-    <h4 class="filter"><%= facet_name %></h4>
-    <div class="facet-box">
-      <ul class="filter_choices unstyled">
-        <% facet["terms"].each do |term| %>
-        <li class="nowrap">
-          <input type="checkbox"
-                 id="<%= term["term"] %>"
-                 name="search[properties][<%= facet_name %>][]"
-                 value="<%= term["term"] %>"
-                 <%= params[:search][:properties] && params[:search][:properties][facet_name] && params[:search][:properties][facet_name].include?(term["term"]) ? "checked" : "" %> />
-          <label class="nowrap" for="<%= term["term"] %>"> <%= term["term"] %> (<%= term["count"] %>)</label>
+<% aggregations.each do |aggregation_name, aggregation| %>
+  <fieldset class='facet'>
+    <h4 class='filter'><%= aggregation_name %></h4>
+    <div class='facet-box'>
+      <ul class='filter_choices unstyled'>
+        <% aggregation[:terms].each do |term| %>
+        <li class='nowrap'>
+          <input type='checkbox'
+                 id="<%= term[:term] %>"
+                 name="search[properties][<%= aggregation_name %>][]"
+                 value="<%= term[:term] %>"
+                 <%= params[:search][:properties] && params[:search][:properties][aggregation_name] && params[:search][:properties][aggregation_name].include?(term[:term]) ? 'checked' : '' %> />
+          <label class='nowrap' for="<%= term[:term] %>"> <%= term[:term] %> (<%= term[:count] %>)</label>
         </li>
         <% end %>
       </ul>
diff --git a/app/views/spree/shared/_filters.html.erb b/app/views/spree/shared/_filters.html.erb
index 33d8744..f961724 100644
--- a/app/views/spree/shared/_filters.html.erb
+++ b/app/views/spree/shared/_filters.html.erb
@@ -1,11 +1,11 @@
 <% unless @products.empty? %>
-  <%= form_tag '', :method => :get, :id => 'sidebar_products_search' do %>
+  <%= form_tag '', method: :get, id: 'sidebar_products_search' do %>
     <% params[:search] ||= {} %>
     <%= hidden_field_tag 'per_page', params[:per_page] %>
-    <% facets = process_facets(@products.response.response['facets']) %>
-    <%= select_tag "sorting", options_for_select({Spree.t("search_sorting.name_asc") => "name_asc", Spree.t("search_sorting.name_desc") => "name_desc", Spree.t("search_sorting.price_asc") => "price_asc", Spree.t("search_sorting.price_desc") => "price_desc", Spree.t("search_sorting.relevancy") => "score"}, params[:sorting]), :onchange => "this.form.submit();" %>
-    <%= render :partial => 'spree/shared/filter_price', :locals => { :facet => facets['price'] } %>
-    <%= render :partial => 'spree/shared/filter_properties', :locals => { :facets => facets.select {|key,facet| ((key != "price") && (key != "taxon_ids")) && (facet['_type'] == 'terms')} || [] } %>
-    <%= submit_tag Spree.t(:search), :name => nil %>
+    <% aggregations = process_aggregations(@products.response.response['aggregations']) %>
+    <%= select_tag 'sorting', options_for_select({ Spree.t('search_sorting.name_asc') => 'name_asc', Spree.t('search_sorting.name_desc') => 'name_desc', Spree.t('search_sorting.price_asc') => 'price_asc', Spree.t('search_sorting.price_desc') => 'price_desc', Spree.t('search_sorting.relevancy') => 'score' }, params[:sorting]), onchange: 'this.form.submit();' %>
+    <%= render 'spree/shared/filter_price', aggregation: aggregations['price'] %>
+    <%= render 'spree/shared/filter_properties', aggregations: aggregations.select { |key, aggregation| ((key != 'price') && (key != 'taxon_ids')) } || [] %>
+    <%= submit_tag Spree.t(:search), name: nil %>
   <% end %>
 <% end %>
diff --git a/spec/models/products_spec.rb b/spec/models/products_spec.rb
index e1738d6..4cbe2d6 100644
--- a/spec/models/products_spec.rb
+++ b/spec/models/products_spec.rb
@@ -6,13 +6,15 @@ module Spree
     let(:a_product) { create(:product) }
     let(:another_product) { create(:product) }
 
-    before(:each) do
+    before do
       # for clean testing, delete index, create new one and create/update mapping
       Product.delete_all
       client = Elasticsearch::Client.new log: true, hosts: ElasticsearchSettings.hosts
+
       if Elasticsearch::Model.client.indices.exists index: Spree::ElasticsearchSettings.index
         client.indices.delete index: ElasticsearchSettings.index
       end
+
       client.indices.create \
         index: ElasticsearchSettings.index,
         body: {
@@ -22,19 +24,19 @@ module Spree
             analysis: {
               analyzer: {
                 nGram_analyzer: {
-                  type: "custom",
-                  filter: ["lowercase", "asciifolding", "nGram_filter"],
-                  tokenizer: "whitespace" },
+                  type: 'custom',
+                  filter: %w(lowercase asciifolding nGram_filter),
+                  tokenizer: 'whitespace' },
                 whitespace_analyzer: {
-                  type: "custom",
-                  filter: ["lowercase", "asciifolding"],
-                  tokenizer: "whitespace" }},
+                  type: 'custom',
+                  filter: %w(lowercase asciifolding),
+                  tokenizer: 'whitespace' }},
               filter: {
                 nGram_filter: {
-                  max_gram: "20",
-                  min_gram: "3",
-                  type: "nGram",
-                  token_chars: ["letter", "digit", "punctuation", "symbol"]
+                  max_gram: '20',
+                  min_gram: '3',
+                  type: 'nGram',
+                  token_chars: %w(letter digit punctuation symbol)
                 }
               }
             }
@@ -43,160 +45,199 @@ module Spree
         }
     end
 
-    context "#index" do
-      it "updates an existing product in the index" do
-        a_product.name = "updated name"
-        result = a_product.__elasticsearch__.index_document
-        result['_version'].should == 2
-        product_from_index = Product.get(a_product.id)
-        product_from_index.name.should == 'updated name'
+    context '#index' do
+      before { a_product.name = 'updated name' }
+
+      it 'updates an existing product in the index' do
+        expect(a_product.__elasticsearch__.index_document['_version']).to eq 2
+        expect(Product.get(a_product.id).name).to eq 'updated name'
       end
     end
 
     context 'get' do
-      it "retrieves a product form the index" do
-        product_from_index = Product.get(a_product.id)
-        product_from_index.name.should == a_product.name
-      end
+      subject { Product.get(a_product.id).name }
+
+      it { is_expected.to eq a_product.name }
     end
 
-    context 'search' do
-      it "retrieves a product based on name" do
-        another_product.name = "Foobar"
-        another_product.__elasticsearch__.index_document
-        sleep 3 # allow some time for elasticsearch
-        products = Product.__elasticsearch__.search(
-          Spree::Product::ElasticsearchQuery.new(query: another_product.name)
-        )
-        # products = Product.__elasticsearch__.search(query: another_product.name)
-        products.results.total.should == 1
-        products.results.any?{ |p| p.name == another_product.name }.should be_true
+    describe 'search' do
+      context 'retrieves a product based on name' do
+        before do
+          another_product.name = 'Foobar'
+          another_product.__elasticsearch__.index_document
+          Product.__elasticsearch__.refresh_index!
+        end
+
+        let(:products) do
+          Product.__elasticsearch__.search(Spree::Product::ElasticsearchQuery.new(query: another_product.name))
+        end
+
+        it { expect(products.results.total).to eq 1 }
+        it { expect(products.results.any?{ |product| product.name == another_product.name }).to be_truthy }
       end
 
-      it "retrieves products based on part of the name" do
-        a_product.name = "Product 1"
-        another_product.name = "Product 2"
-        a_product.__elasticsearch__.index_document
-        another_product.__elasticsearch__.index_document
-        sleep 3 # allow some time for elasticsearch
-        products = Product.__elasticsearch__.search(
-          Spree::Product::ElasticsearchQuery.new(query: 'Product')
-        )
-        products.results.total.should == 2
-        products.results.any?{ |p| p.name == a_product.name }.should be_true
-        products.results.any?{ |p| p.name == another_product.name }.should be_true
+      context 'retrieves products based on part of the name' do
+        before do
+          a_product.name = 'Product 1'
+          another_product.name = 'Product 2'
+          a_product.__elasticsearch__.index_document
+          another_product.__elasticsearch__.index_document
+          Product.__elasticsearch__.refresh_index!
+        end
+
+        let(:products) { Product.__elasticsearch__.search(Spree::Product::ElasticsearchQuery.new(query: 'Product')) }
+
+        it { expect(products.results.total).to eq 2 }
+        it { expect(products.results.any?{ |product| product.name == a_product.name }).to be_truthy }
+        it { expect(products.results.any?{ |product| product.name == another_product.name }).to be_truthy }
       end
 
-      it "retrieves products default sorted on name" do
-        a_product.name = "Product 1"
-        a_product.__elasticsearch__.index_document
-        another_product.name = "Product 2"
-        another_product.__elasticsearch__.index_document
-        sleep 3 # allow some time for elasticsearch
-        products = Product.__elasticsearch__.search(
-          Spree::Product::ElasticsearchQuery.new
-        )
-        products.results.total.should == 2
-        products.results.to_a[0].name.should == a_product.name
-        products.results.to_a[1].name.should == another_product.name
+      context 'retrieves products default sorted on name' do
+        before do
+          a_product.name = 'Product 1'
+          a_product.__elasticsearch__.index_document
+          another_product.name = 'Product 2'
+          another_product.__elasticsearch__.index_document
+          Product.__elasticsearch__.refresh_index!
+        end
+
+        let(:products) { Product.__elasticsearch__.search(Spree::Product::ElasticsearchQuery.new) }
+
+        it { expect(products.results.total).to eq 2 }
+        it { expect(products.results.to_a[0].name).to eq a_product.name }
+        it { expect(products.results.to_a[1].name).to eq another_product.name }
       end
 
-      it "filters products based on price" do
-        a_product.price = 1
-        a_product.__elasticsearch__.index_document
-        another_product.price = 3
-        another_product.__elasticsearch__.index_document
-        sleep 3 # allow some time for elasticsearch
-        products = Product.__elasticsearch__.search(
-          Spree::Product::ElasticsearchQuery.new(price_min: 2, price_max: 4)
-        )
-        products.results.total.should == 1
-        products.results.to_a[0].name.should == another_product.name
+      context 'filters products based on price' do
+        before do
+          a_product.price = 1
+          a_product.__elasticsearch__.index_document
+          another_product.price = 3
+          another_product.__elasticsearch__.index_document
+          Product.__elasticsearch__.refresh_index!
+        end
+
+        let(:products) do
+          Product.__elasticsearch__.search(Spree::Product::ElasticsearchQuery.new(price_min: 2, price_max: 4))
+        end
+
+        it { expect(products.results.total).to eq 1 }
+        it { expect(products.results.to_a[0].name).to eq another_product.name }
       end
 
-      it "ignores price filter when price_min is greater than price_max" do
-        a_product.price = 1
-        a_product.__elasticsearch__.index_document
-        another_product.price = 3
-        another_product.__elasticsearch__.index_document
-        sleep 3 # allow some time for elasticsearch
-        products = Product.__elasticsearch__.search(
-          Spree::Product::ElasticsearchQuery.new(price_min: 4, price_max: 2)
-        )
-        products.results.total.should == 2
+      context 'ignores price filter when price_min is greater than price_max' do
+        before do
+          a_product.price = 1
+          a_product.__elasticsearch__.index_document
+          another_product.price = 3
+          another_product.__elasticsearch__.index_document
+          Product.__elasticsearch__.refresh_index!
+        end
+
+        let(:products) do
+          Product.__elasticsearch__.search(Spree::Product::ElasticsearchQuery.new(price_min: 4, price_max: 2))
+        end
+
+        it { expect(products.results.total).to eq 2 }
       end
 
-      it "ignores price filter when price_min and/or price_max is nil" do
-        a_product.price = 1
-        a_product.__elasticsearch__.index_document
-        another_product.price = 3
-        another_product.__elasticsearch__.index_document
-        sleep 3 # allow some time for elasticsearch
-        products = Product.__elasticsearch__.search(
-          Spree::Product::ElasticsearchQuery.new(price_min: 2)
-        )
-        products.results.total.should == 2
-        products = Product.__elasticsearch__.search(
-          Spree::Product::ElasticsearchQuery.new(price_max: 2)
-        )
-        products.results.total.should == 2
-        products = Product.__elasticsearch__.search(
-          Spree::Product::ElasticsearchQuery.new(price_min: nil, price_max: nil)
-        )
-        products.results.total.should == 2
+      context 'ignores price filter when price_min and/or price_max is nil' do
+        before do
+          a_product.price = 1
+          a_product.__elasticsearch__.index_document
+          another_product.price = 3
+          another_product.__elasticsearch__.index_document
+          Product.__elasticsearch__.refresh_index!
+        end
+
+        let(:products_min_price) do
+          Product.__elasticsearch__.search(Spree::Product::ElasticsearchQuery.new(price_min: 2))
+        end
+        let(:products_max_price) do
+          Product.__elasticsearch__.search(Spree::Product::ElasticsearchQuery.new(price_max: 2))
+        end
+        let(:products) do
+          Product.__elasticsearch__.search(Spree::Product::ElasticsearchQuery.new(price_min: nil, price_max: nil))
+        end
+
+        it { expect(products_min_price.results.total).to eq 2 }
+        it { expect(products_max_price.results.total).to eq 2 }
+        it { expect(products.results.total).to eq 2 }
       end
 
-      context 'properties' do
-        it "allows searching on property" do
-          a_product.set_property('the_prop', 'a_value')
-          product = Product.find(a_product.id)
-          product.save
-          sleep 3 # allow some time for elasticsearch
-          products = Product.__elasticsearch__.search(
-            Spree::Product::ElasticsearchQuery.new(properties: { 'the_prop' => ['a_value', 'b_value'] })
-          )
-          products.results.count.should == 1
-          products.results.to_a[0].name.should == product.name
+      describe 'properties' do
+        let(:product) { Product.find(a_product.id) }
+
+        subject { products.results }
+
+        context 'allows searching on property' do
+          before do
+            a_product.set_property('the_prop', 'a_value')
+            product.save
+            Product.__elasticsearch__.refresh_index!
+          end
+
+
+          let(:products) do
+            Product.__elasticsearch__.search(
+              Spree::Product::ElasticsearchQuery.new(properties: { the_prop: %w(a_value b_value) })
+            )
+          end
+
+          it { expect(subject.count).to eq 1 }
+          it { expect(subject.to_a[0].name).to eq product.name }
         end
 
-        it "allows searching on different property values (OR relation)" do
-          a_product.set_property('the_prop', 'a_value')
-          product_one = Product.find(a_product.id)
-          product_one.save
-          another_product.set_property('the_prop', 'b_value')
-          product_two = Product.find(another_product.id)
-          product_two.save
-          sleep 3 # allow some time for elasticsearch
-          products = Product.__elasticsearch__.search(
-            Spree::Product::ElasticsearchQuery.new(properties: { 'the_prop' => ['a_value', 'b_value'] })
-          )
-          products.results.count.should == 2
-          products.results.to_a.find {|p| p.name == product_one.name}.should_not be_nil
-          products.results.to_a.find {|p| p.name == product_two.name}.should_not be_nil
+        context 'allows searching on different property values (OR relation)' do
+          before do
+            a_product.set_property('the_prop', 'a_value')
+            another_product.set_property('the_prop', 'b_value')
+            product_one.save
+            product_two.save
+            Product.__elasticsearch__.refresh_index!
+          end
+
+          let(:product_one) { Product.find(a_product.id) }
+          let(:product_two) { Product.find(another_product.id) }
+
+          let(:products) do
+            Product.__elasticsearch__.search(
+              Spree::Product::ElasticsearchQuery.new(properties: { the_prop: %w(a_value b_value) })
+            )
+          end
+
+          it { expect(subject.count).to eq 2 }
+          it { expect(subject.to_a.find { |product| product.name == product_one.name }).to_not be_nil }
+          it { expect(subject.to_a.find { |product| product.name == product_two.name }).to_not be_nil }
         end
 
-        it "allows searching on different properties (AND relation)" do
-          a_product.set_property('the_prop', 'a_value')
-          a_product.set_property('another_prop', 'a_value')
-          product = Product.find(a_product.id)
-          product.save
-          another_product.set_property('the_prop', 'a_value')
-          another_product.set_property('another_prop', 'b_value')
-          Product.find(another_product.id).save
-          sleep 3 # allow some time for elasticsearch
-          products = Product.__elasticsearch__.search(
-            Spree::Product::ElasticsearchQuery.new(properties: { 'the_prop' => ['a_value'], 'another_prop' => ['a_value'] })
-          )
-          products.results.count.should == 1
-          products.results.to_a[0].name.should == product.name
+        context 'allows searching on different properties (AND relation)' do
+          before do
+            a_product.set_property('the_prop', 'a_value')
+            a_product.set_property('another_prop', 'a_value')
+            product.save
+            another_product.set_property('the_prop', 'a_value')
+            another_product.set_property('another_prop', 'b_value')
+            Product.find(another_product.id).save
+            Product.__elasticsearch__.refresh_index!
+          end
+
+          let(:products) do
+            Product.__elasticsearch__.search(
+              Spree::Product::ElasticsearchQuery.new(properties: { the_prop: ['a_value'], another_prop: ['a_value'] })
+            )
+          end
+
+          it { expect(subject.count).to eq 1 }
+          it { expect(subject.to_a[0].name).to eq product.name }
         end
       end
     end
 
-    context 'document_type' do
-      it 'returns the name of the class' do
-        Product.document_type.should == 'spree_product'
-      end
+    context 'document_type returns the name of the class' do
+      subject { Product.document_type }
+
+      it { is_expected.to eq 'spree_product' }
     end
   end
 end

From 2c3eb7dd67946f019145803babdd0d5944dc9a2c Mon Sep 17 00:00:00 2001
From: Rohit <rohitagarwal.2k6@gmail.com>
Date: Thu, 11 Aug 2016 00:56:43 +0530
Subject: [PATCH 2/9] fix for No handler for type [multi_field] declared on
 field, changing to text

---
 app/models/spree/product_decorator.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/models/spree/product_decorator.rb b/app/models/spree/product_decorator.rb
index d5f12a1..b8bf189 100644
--- a/app/models/spree/product_decorator.rb
+++ b/app/models/spree/product_decorator.rb
@@ -6,8 +6,8 @@ module Spree
     document_type 'spree_product'
 
     mapping _all: { analyzer: 'nGram_analyzer', search_analyzer: 'whitespace_analyzer' } do
-      indexes :name, type: 'multi_field' do
-        indexes :name, type: 'string', analyzer: 'nGram_analyzer', boost: 100
+      indexes :name, type: 'text' do
+        indexes :name, type: 'text', analyzer: 'nGram_analyzer', boost: 100
         indexes :untouched, type: 'string', include_in_all: false, index: 'not_analyzed'
       end
 

From 5c37f07ace943fca8640b83c6e2211d49fee9401 Mon Sep 17 00:00:00 2001
From: Rohit <rohitagarwal.2k6@gmail.com>
Date: Thu, 11 Aug 2016 12:55:24 +0530
Subject: [PATCH 3/9] updaring query dsl for ES 2.3 version

---
 app/models/spree/product_decorator.rb | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/app/models/spree/product_decorator.rb b/app/models/spree/product_decorator.rb
index b8bf189..8980f5f 100644
--- a/app/models/spree/product_decorator.rb
+++ b/app/models/spree/product_decorator.rb
@@ -63,8 +63,8 @@ class Product::ElasticsearchQuery
       # The idea is to always to use the following schema and fill in the blanks.
       # {
       #   query: {
-      #     filtered: {
-      #       query: {
+      #     bool: {
+      #       must: {
       #         query_string: { query: , fields: [] }
       #       }
       #       filter: {
@@ -87,14 +87,14 @@ def to_hash
         end
         query = q
 
-        and_filter = []
+        and_filter = {}
         unless @properties.nil? || @properties.empty?
           # transform properties from [{"key1" => ["value_a","value_b"]},{"key2" => ["value_a"]}
           # to { terms: { properties: ["key1||value_a","key1||value_b"] }
           #    { terms: { properties: ["key2||value_a"] }
           # This enforces "and" relation between different property values and "or" relation between same property values
           properties = @properties.map{ |key, value| [key].product(value) }.map do |pair|
-            and_filter << { terms: { properties: pair.map { |property| property.join('||') } } }
+            and_filter[:terms] = { properties: pair.map { |property| property.join('||') } }
           end
         end
 
@@ -123,20 +123,20 @@ def to_hash
         # basic skeleton
         result = {
           min_score: 0.1,
-          query: { filtered: {} },
+          query: { bool: {} },
           sort: sorting,
           from: from,
           size:  Spree::Config.products_per_page,
           aggregations: aggregations
         }
 
-        # add query and filters to filtered
-        result[:query][:filtered][:query] = query
+        # add query and filters to bool
+        result[:query][:bool][:must] = query
         # taxon and property filters have an effect on the facets
-        and_filter << { terms: { taxon_ids: taxons } } unless taxons.empty?
+        and_filter[:terms] = { taxon_ids: taxons } unless taxons.empty?
         # only return products that are available
-        and_filter << { range: { available_on: { lte: 'now' } } }
-        result[:query][:filtered][:filter] = { and: and_filter } unless and_filter.empty?
+        and_filter[:range] = { available_on: { lte: 'now' } }
+        result[:query][:bool][:filter] = and_filter unless and_filter.empty?
 
         # add price filter outside the query because it should have no effect on facets
         if price_min && price_max && (price_min < price_max)

From 6a733100889c2e3b85921aadb4cf7944b7fa7404 Mon Sep 17 00:00:00 2001
From: Rohit <rohitagarwal.2k6@gmail.com>
Date: Thu, 11 Aug 2016 14:41:54 +0530
Subject: [PATCH 4/9] changing text type to string as per ES-2.3

---
 app/models/spree/product_decorator.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/models/spree/product_decorator.rb b/app/models/spree/product_decorator.rb
index 8980f5f..39cae43 100644
--- a/app/models/spree/product_decorator.rb
+++ b/app/models/spree/product_decorator.rb
@@ -6,8 +6,8 @@ module Spree
     document_type 'spree_product'
 
     mapping _all: { analyzer: 'nGram_analyzer', search_analyzer: 'whitespace_analyzer' } do
-      indexes :name, type: 'text' do
-        indexes :name, type: 'text', analyzer: 'nGram_analyzer', boost: 100
+      indexes :name, type: 'string' do
+        indexes :name, type: 'string', analyzer: 'nGram_analyzer', boost: 100
         indexes :untouched, type: 'string', include_in_all: false, index: 'not_analyzed'
       end
 

From bec5ed943f65ba4d1ce0b1bddb32282229d03e57 Mon Sep 17 00:00:00 2001
From: Rohit <rohitagarwal.2k6@gmail.com>
Date: Thu, 11 Aug 2016 15:28:54 +0530
Subject: [PATCH 5/9] fixing query dsl filter

---
 app/models/spree/product_decorator.rb | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/app/models/spree/product_decorator.rb b/app/models/spree/product_decorator.rb
index 39cae43..213e29b 100644
--- a/app/models/spree/product_decorator.rb
+++ b/app/models/spree/product_decorator.rb
@@ -87,14 +87,14 @@ def to_hash
         end
         query = q
 
-        and_filter = {}
+        and_filter = []
         unless @properties.nil? || @properties.empty?
           # transform properties from [{"key1" => ["value_a","value_b"]},{"key2" => ["value_a"]}
           # to { terms: { properties: ["key1||value_a","key1||value_b"] }
           #    { terms: { properties: ["key2||value_a"] }
           # This enforces "and" relation between different property values and "or" relation between same property values
           properties = @properties.map{ |key, value| [key].product(value) }.map do |pair|
-            and_filter[:terms] = { properties: pair.map { |property| property.join('||') } }
+            and_filter << { terms: { properties: pair.map { |property| property.join('||') } } }
           end
         end
 
@@ -133,9 +133,9 @@ def to_hash
         # add query and filters to bool
         result[:query][:bool][:must] = query
         # taxon and property filters have an effect on the facets
-        and_filter[:terms] = { taxon_ids: taxons } unless taxons.empty?
+        and_filter << { terms: { taxon_ids: taxons } }unless taxons.empty?
         # only return products that are available
-        and_filter[:range] = { available_on: { lte: 'now' } }
+        and_filter << { range: { available_on: { lte: 'now' } } }
         result[:query][:bool][:filter] = and_filter unless and_filter.empty?
 
         # add price filter outside the query because it should have no effect on facets

From 63e655d2d908105f30239db7c334fbf0b63b7d27 Mon Sep 17 00:00:00 2001
From: Rohit Agarwal <rohit@oyorooms.com>
Date: Fri, 11 Nov 2016 20:50:13 +0530
Subject: [PATCH 6/9] Updating product decorator model to allow sorting by
 inventory count

---
 app/models/spree/product_decorator.rb | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/app/models/spree/product_decorator.rb b/app/models/spree/product_decorator.rb
index 213e29b..e85f4cc 100644
--- a/app/models/spree/product_decorator.rb
+++ b/app/models/spree/product_decorator.rb
@@ -25,6 +25,7 @@ def as_indexed_json(options={})
         only: [:available_on, :description, :name],
         include: {
           variants: {
+            methods: [:total_on_hand],
             only: [:sku],
             include: {
               option_values: {
@@ -83,7 +84,7 @@ class Product::ElasticsearchQuery
       def to_hash
         q = { match_all: {} }
         unless query.blank? # nil or empty
-          q = { query_string: { query: query, fields: ['name^5','description','sku'], default_operator: 'AND', use_dis_max: true } }
+          q = { query_string: { query: query, fields: ['*name*^5','description','sku', 'variants.sku'], default_operator: 'AND', use_dis_max: true } }
         end
         query = q
 
@@ -110,13 +111,13 @@ def to_hash
         when 'score'
           [ '_score', { 'name.untouched' => { order: 'asc' } }, { price: { order: 'asc' } } ]
         else
-          [ { 'name.untouched' => { order: 'asc' } }, { price: { order: 'asc' } }, '_score' ]
+          [ { 'variants.total_on_hand' => { order: 'desc' } }, { price: { order: 'asc' } }, '_score' ]
         end
 
         # aggregations
         aggregations = {
           price: { stats: { field: 'price' } },
-          properties: { terms: { field: 'properties', order: { _count: 'asc' }, size: 1000000 } },
+          properties: { terms: { field: 'properties', size: 1000000 } },
           taxon_ids: { terms: { field: 'taxon_ids', size: 1000000 } }
         }
 

From f18e806a6d97e9acee421996b48f60d4a632ad54 Mon Sep 17 00:00:00 2001
From: Rohit Agarwal <rohit@oyorooms.com>
Date: Fri, 11 Nov 2016 20:51:16 +0530
Subject: [PATCH 7/9] fixing base helper for nil product properties

---
 app/helpers/spree/base_helper_decorator.rb | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/app/helpers/spree/base_helper_decorator.rb b/app/helpers/spree/base_helper_decorator.rb
index 2f29bd1..fa2cec8 100644
--- a/app/helpers/spree/base_helper_decorator.rb
+++ b/app/helpers/spree/base_helper_decorator.rb
@@ -11,12 +11,14 @@ def expand_properties_aggregation_to_aggregation_array(aggregation)
         t = term[:key].split('||')
         property_name = t[0]
         property_value = t[1]
-        # add a search_term to each term hash to allow searching on the element later on
-        property = { term: property_value, count: term[:doc_count], search_term: term[:key] }
-        if property_names.has_key?(property_name)
-          property_names[property_name] << property
-        else
-          property_names[property_name] = [property]
+        if property_value
+          # add a search_term to each term hash to allow searching on the element later on
+          property = { term: property_value, count: term[:doc_count], search_term: term[:key] }
+          if property_names.has_key?(property_name)
+            property_names[property_name] << property
+          else
+            property_names[property_name] = [property]
+          end
         end
       end
       # next step is to transform the hash to facet objects

From c552448ab39997e3763242e698ea16d518154ca5 Mon Sep 17 00:00:00 2001
From: Rohit Agarwal <rohit@oyorooms.com>
Date: Fri, 11 Nov 2016 23:25:02 +0530
Subject: [PATCH 8/9] Updating sorting options

---
 app/models/spree/product_decorator.rb | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/app/models/spree/product_decorator.rb b/app/models/spree/product_decorator.rb
index e85f4cc..6505f59 100644
--- a/app/models/spree/product_decorator.rb
+++ b/app/models/spree/product_decorator.rb
@@ -100,16 +100,16 @@ def to_hash
         end
 
         sorting = case @sorting
-        when 'name_asc'
-          [ { 'name.untouched' => { order: 'asc' } }, { price: { order: 'asc' } }, '_score' ]
-        when 'name_desc'
-          [ { 'name.untouched' => { order: 'desc' } }, { price: { order: 'asc' } }, '_score' ]
+        when 'default'
+          [ { 'variants.total_on_hand' => { order: 'desc' } }, { price: { order: 'asc' } }, '_score' ]
+        when 'score'
+          [ '_score', { price: { order: 'asc' } }, { 'name.untouched' => { order: 'asc' } } ]
         when 'price_asc'
           [ { 'price' => { order: 'asc' } }, { 'name.untouched' => { order: 'asc' } }, '_score' ]
         when 'price_desc'
           [ { 'price' => { order: 'desc' } }, { 'name.untouched' => { order: 'asc' } }, '_score' ]
-        when 'score'
-          [ '_score', { 'name.untouched' => { order: 'asc' } }, { price: { order: 'asc' } } ]
+        when 'new_arrival'
+          [ { 'available_on' => { order: 'desc' } }, { price: { order: 'asc' } }, '_score' ]
         else
           [ { 'variants.total_on_hand' => { order: 'desc' } }, { price: { order: 'asc' } }, '_score' ]
         end

From f704c55e0b766f9265b35b7f2f3a6bfebeb530ef Mon Sep 17 00:00:00 2001
From: Rohit Agarwal <rohit@oyorooms.com>
Date: Fri, 17 Mar 2017 16:31:29 +0530
Subject: [PATCH 9/9] Fixing search results query to suit use case.

---
 app/models/spree/product_decorator.rb | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/app/models/spree/product_decorator.rb b/app/models/spree/product_decorator.rb
index 6505f59..264c98b 100644
--- a/app/models/spree/product_decorator.rb
+++ b/app/models/spree/product_decorator.rb
@@ -84,7 +84,13 @@ class Product::ElasticsearchQuery
       def to_hash
         q = { match_all: {} }
         unless query.blank? # nil or empty
-          q = { query_string: { query: query, fields: ['*name*^5','description','sku', 'variants.sku'], default_operator: 'AND', use_dis_max: true } }
+          q = { query_string: { 
+                query: query,
+                fields: ['name^10','description','sku', 'variants.sku', 'variant.*', 'name.*^.1'],
+                default_operator: 'AND',
+                use_dis_max: true 
+                } 
+              }
         end
         query = q
 
@@ -111,7 +117,7 @@ def to_hash
         when 'new_arrival'
           [ { 'available_on' => { order: 'desc' } }, { price: { order: 'asc' } }, '_score' ]
         else
-          [ { 'variants.total_on_hand' => { order: 'desc' } }, { price: { order: 'asc' } }, '_score' ]
+          [ '_score', { 'variants.total_on_hand' => { order: 'desc' } }, { price: { order: 'asc' } } ]
         end
 
         # aggregations
@@ -123,7 +129,7 @@ def to_hash
 
         # basic skeleton
         result = {
-          min_score: 0.1,
+          min_score: 1,
           query: { bool: {} },
           sort: sorting,
           from: from,