Pull homepage data directly from elasticsearch (#2316)

* Pull homepage data directly from elasticsearch

* Removing non-elasticsearch options

* [CodeFactor] Apply fixes to commit c46b2e7

.travis.yml

@@ -16,9 +16,8 @@ addons:
       secure: "PfhLGBKRgNqhKuYCJsK+VPhdAzcgWFGeeOyxC/eS8gtlvIISVdgyZE+r30uIei0DFI6zEiN62eW4d+xtT4j7/e2ZcAcx7U52mza/SnQNuu3nCGQDJB8VOvV5NbnwXfi8vfr4e889Mt7k3ocd2c4gqB4UtRqrzhygj7HN+B/GfEk="
 env:
   matrix:
-    - GROWSTUFF_ELASTICSEARCH=true RSPEC_TAG=elasticsearch COVERAGE=true ELASTIC_SEARCH_VERSION="6.2.3"
-    - GROWSTUFF_ELASTICSEARCH=false RSPEC_TAG=~elasticsearch COVERAGE=false
-    - STATIC_CHECKS=true PERCY=true
+    - COVERAGE=true
+    - STATIC_CHECKS=true
   global:
     - secure: "Z5TpM2jEX4UCvNePnk/LwltQX48U2u9BRc+Iypr1x9QW2o228QJhPIOH39a8RMUrepGnkQIq9q3ZRUn98RfrJz1yThtlNFL3NmzdQ57gKgjGwfpa0e4Dwj/ZJqV2D84tDGjvdVYLP7zzaYZxQcwk/cgNpzKf/jq97HLNP7CYuf4="
     - GROWSTUFF_EMAIL="noreply@test.growstuff.org"
@@ -30,29 +29,21 @@ before_install:
   - sudo apt install dpkg
   - sudo apt install google-chrome-stable
   - ./script/install_linters.sh
-  - ./script/install_elasticsearch.sh
+  - ELASTIC_SEARCH_VERSION="6.2.3" ./script/install_elasticsearch.sh
 script:
-  - set -e
   - >
     if [ "${STATIC_CHECKS}" = "true" ]; then
       ./script/check_static.rb
     else
-      set +e;
-      RAILS_ENV=test bundle exec rake db:create db:migrate search:reindex;
-      bundle exec rake assets:precompile;
-      bundle exec rspec -fd --tag $RSPEC_TAG spec/;
+      RAILS_ENV=test bundle exec rake db:create db:migrate search:reindex assets:precompile;
+      npx percy exec -- bundle exec rspec spec
     fi;
-  - set +e
 after_script:
   - >
     if [ "${COVERAGE}" = "true" ]; then
       gem install codeclimate-test-reporter
       codeclimate-test-reporter
     fi
-  - >
-    if [ "${PERCY}" = "true" ]; then
-      ./scripts/percy.sh
-    fi
 before_deploy:
   - bundle exec script/heroku_maintenance.rb on
 deploy:

app/models/alternate_name.rb

@@ -4,7 +4,7 @@ class AlternateName < ApplicationRecord
   validates :name, presence: true
   validates :crop, presence: true
 
-  after_commit :reindex if ENV["GROWSTUFF_ELASTICSEARCH"] == "true"
+  after_commit :reindex
 
   delegate :reindex, to: :crop
 end

app/models/concerns/crop_search.rb

@@ -0,0 +1,34 @@
+module CropSearch
+  extend ActiveSupport::Concern
+
+  included do
+    ####################################
+    # Elastic search configuration
+    searchkick word_start: %i(name alternate_names scientific_names), case_sensitive: false
+
+    # Special scope to control if it's in the search index
+    scope :search_import, -> { includes(:scientific_names, :photos) }
+
+    def should_index?
+      approved?
+    end
+
+    def search_data
+      {
+        name:             name,
+        slug:             slug,
+        alternate_names:  alternate_names.pluck(:name),
+        scientific_names: scientific_names.pluck(:name),
+        # boost the crops that are planted the most
+        plantings_count:  plantings_count,
+        # boost this crop for these members
+        planters_ids:     plantings.pluck(:owner_id),
+        has_photos:       photos.size.positive?,
+        photo:            default_photo&.thumbnail_url,
+        scientific_name:  default_scientific_name&.name,
+        description:      description,
+        created_at:       created_at.to_i
+      }
+    end
+  end
+end

app/models/crop.rb

@@ -2,6 +2,8 @@ class Crop < ApplicationRecord
   extend FriendlyId
   include PhotoCapable
   include OpenFarmData
+  include CropSearch
+
   friendly_id :name, use: %i(slugged finders)
 
   ##
@@ -37,9 +39,6 @@ class Crop < ApplicationRecord
   scope :has_photos, -> { includes(:photos).where.not(photos: { id: nil }) }
   scope :joins_members, -> { joins("INNER JOIN members ON members.id = harvests.owner_id") }
 
-  # Special scope to control if it's in the search index
-  scope :search_import, -> { approved }
-
   ##
   ## Validations
   # Reasons are only necessary when rejecting
@@ -54,10 +53,6 @@ class Crop < ApplicationRecord
             },
             if:     :approved?
 
-  ####################################
-  # Elastic search configuration
-  searchkick word_start: %i(name alternate_names scientific_names), case_sensitive: false if ENV["GROWSTUFF_ELASTICSEARCH"] == "true"
-
   def to_s
     name
   end
@@ -154,24 +149,6 @@ class Crop < ApplicationRecord
     where(["lower(crops.name) = :value", { value: name.downcase }])
   end
 
-  def should_index?
-    approved?
-  end
-
-  def search_data
-    {
-      name:             name,
-      alternate_names:  alternate_names.pluck(:name),
-      scientific_names: scientific_names.pluck(:name),
-      plantings_count:  plantings_count, # boost the crops that are planted the most
-      planters_ids:     plantings.pluck(:owner_id) # boost this product for these members
-    }
-  end
-
-  def fetch_from_openfarm!
-    OpenfarmService.new.update_crop(self)
-  end
-
   private
 
   def count_uses_of_property(col_name)

app/models/scientific_name.rb

@@ -3,7 +3,7 @@ class ScientificName < ApplicationRecord
   belongs_to :creator, class_name: 'Member', inverse_of: :created_scientific_names
   validates :name, presence: true
   validates :crop, presence: true
-  after_commit :reindex if ENV["GROWSTUFF_ELASTICSEARCH"] == "true"
+  after_commit :reindex
   delegate :reindex, to: :crop
 
   def to_s

app/services/crop_search_service.rb

@@ -1,14 +1,6 @@
 class CropSearchService
   # Crop.search(string)
   def self.search(query, page: 1, per_page: 12, current_member: nil)
-    if ENV["GROWSTUFF_ELASTICSEARCH"] == "true"
-      elasticsearch(query, page: page, per_page: per_page, current_member: current_member)
-    else
-      dbsearch(query, page: page, per_page: per_page)
-    end
-  end
-
-  def self.elasticsearch(query, page: 1, per_page: 12, current_member: nil)
     search_params = {
       page:         page,
       per_page:     per_page,
@@ -24,26 +16,27 @@ class CropSearchService
     Crop.search(query, search_params)
   end
 
-  def self.dbsearch(query, page: 1, per_page: 12)
-    # if we don't have elasticsearch, just do a basic SQL query.
-    # also, make sure it's an actual array not an activerecord
-    # collection, so it matches what we get from elasticsearch and we can
-    # manipulate it in the same ways (eg. deleting elements without deleting
-    # the whole record from the db)
-    matcher = "%#{query}%"
-    matches = Crop.approved
-      .left_outer_joins(:alternate_names, :scientific_names)
-      .where("crops.name ILIKE ? OR alternate_names.name ILIKE ? OR scientific_names.name ILIKE ?",
-        matcher, matcher, matcher)
+  def self.random_with_photos(limit)
+    body = {
+      "query": {
+        "function_score": {
+          "query":        { "query_string": { "query": 'has_photos:true' } },
+          "random_score": { "seed": DateTime.now.to_i }
+        }
+      }
+    }
+    Crop.search(
+      limit: limit,
+      load:  false,
+      body:  body
+    ).response['hits']['hits'].map { |c| c['_source'] }
+  end
 
-    matches = matches.to_a
-    # we want to make sure that exact matches come first, even if not
-    # using elasticsearch (eg. in development)
-    exact_match = Crop.approved.find_by(name: query)
-    if exact_match
-      matches.delete(exact_match)
-      matches.unshift(exact_match)
-    end
-    matches.paginate(page: page, per_page: per_page)
+  def self.recent(limit)
+    Crop.search(
+      limit:    limit,
+      load:     false,
+      boost_by: { created_at: { factor: 100 } } # default factor is 1
+    ).response['hits']['hits'].map { |c| c['_source'] }
   end
 end

app/views/home/_crops.html.haml

@@ -1,5 +1,16 @@
 - cache cache_key_for(Crop, 'homepage'), expires_in: 1.day do
   .index-cards
-    - Crop.interesting.includes(:scientific_names, :photos, :parent).shuffle.first(18).each do |crop|
-      = render 'crops/thumbnail', crop: crop
+    - CropSearchService.random_with_photos(16).each do |crop|
+      .card.crop-thumbnail
+        = link_to crop_path(slug: crop['slug']) do
+          = image_tag(crop['photo'],
+                                alt: crop['name'],
+                                class: 'img img-card')
+
+        .text
+          %h3.crop-name
+            = link_to crop['name'], crop_path(slug: crop['slug'])
+          %h5.crop-sci-name
+             
+            = crop['scientific_name']
 

app/views/home/index.html.haml

@@ -26,8 +26,7 @@
       - cache cache_key_for(Crop, 'recent') do
         %h2= t('.recently_added')
         %p
-          != Crop.recent.limit(30).map { |c| link_to(c, c) }.join(", ")
-
+          != CropSearchService.recent(30).map { |c| link_to(c['name'], crop_path(slug: c['slug'])) }.join(", ")
   .col-xl-2.col
     %section.plantings
       = cute_icon

config/application.yml.example

@@ -57,13 +57,6 @@ GROWSTUFF_FACEBOOK_SECRET: ""
 GROWSTUFF_MAPBOX_MAP_ID: ""
 GROWSTUFF_MAPBOX_ACCESS_TOKEN: ""
 
-# Elasticsearch is used for flexible search and it requires another component
-# to be installed. To make it easy for people who don't need to test this feature
-# it's been turned off for test and development environment as a default.
-# If you want to test this functionality, install elasticsearch and
-# set this flag to "true".
-GROWSTUFF_ELASTICSEARCH: "false"
-
 ##############################################################################
 # Other environments
 # You can override the above for staging, production, etc.
@@ -83,8 +76,6 @@ test:
 
 staging:
   GROWSTUFF_SITE_NAME: Growstuff (staging)
-  GROWSTUFF_ELASTICSEARCH: "true"
 
 production:
   GROWSTUFF_SITE_NAME: Growstuff
-  GROWSTUFF_ELASTICSEARCH: "true"

spec/controllers/crops_controller_spec.rb

@@ -42,7 +42,7 @@ describe CropsController do
     describe 'fetches the crop search page' do
       let!(:tomato) { FactoryBot.create :tomato }
       let!(:maize)  { FactoryBot.create :maize }
-      before { Crop.reindex if ENV["GROWSTUFF_ELASTICSEARCH"] == "true" }
+      before { Crop.reindex }
       describe 'search form page' do
         before { get :search }
 

spec/features/harvests/harvesting_a_crop_spec.rb

@@ -1,7 +1,7 @@
 require 'rails_helper'
 require 'custom_matchers'
 
-describe "Harvesting a crop", :js, :elasticsearch do
+describe "Harvesting a crop", :js do
   context 'signed in' do
     include_context 'signed in member'
     let!(:maize)      { create :maize }

spec/features/home/home_spec.rb

@@ -21,6 +21,7 @@ describe "home page" do
     planting.photos << photo
     seed.photos << photo
     harvest.photos << photo
+    Crop.reindex
   end
 
   before { visit root_path }
@@ -54,6 +55,7 @@ describe "home page" do
 
   shared_examples "show crops" do
     describe 'shows crops section' do
+      before { crop.reindex }
       it { is_expected.to have_text 'Some of our crops' }
       it { is_expected.to have_link href: crop_path(crop) }
     end

spec/features/percy/percy_spec.rb

@@ -77,6 +77,7 @@ rest of the garden.
       maize:    'https://farm66.staticflickr.com/65535/46739264475_7cb55b2cbb_q.jpg'
     }.each do |crop_type, photo_url|
       crop = FactoryBot.create crop_type, creator: someone_else
+      crop.reindex
       owner = FactoryBot.create :interesting_member, login_name: crop_type.to_s.reverse, email: "#{crop.name}@example.com"
       planting = FactoryBot.create :planting, crop: crop, owner: owner, garden: owner.gardens.first
       photo = FactoryBot.create(:photo, owner: owner,

spec/features/plantings/planting_a_crop_spec.rb

@@ -1,7 +1,7 @@
 require "rails_helper"
 require 'custom_matchers'
 
-describe "Planting a crop", :js, :elasticsearch do
+describe "Planting a crop", :js do
   let!(:maize) { FactoryBot.create :maize }
   let(:garden) { FactoryBot.create :garden, owner: member, name: 'Orchard' }
   let!(:planting) do

spec/features/plantings/prediction_spec.rb

@@ -1,6 +1,6 @@
 require "rails_helper"
 require 'custom_matchers'
-describe "Display a planting", :js, :elasticsearch do
+describe "Display a planting", :js do
   describe 'planting perennial' do
     let(:garden) { FactoryBot.create :garden, location: 'Edinburgh' }
     let(:crop) { FactoryBot.create(:crop, name: 'feijoa', perennial: true) }

spec/features/plantings/show_spec.rb

@@ -1,7 +1,7 @@
 require "rails_helper"
 require 'custom_matchers'
 
-describe "Display a planting", :js, :elasticsearch do
+describe "Display a planting", :js do
   context 'anonymous' do
     before { visit planting_path(planting) }
 

spec/features/seeds/adding_seeds_spec.rb

@@ -1,7 +1,7 @@
 require 'rails_helper'
 require 'custom_matchers'
 
-describe "Seeds", :js, :elasticsearch do
+describe "Seeds", :js do
   context 'signed in' do
     include_context 'signed in member'
     let!(:maize) { create :maize }

spec/features/signin_spec.rb

@@ -5,6 +5,11 @@ describe "signin", js: true do
   let(:recipient)    { FactoryBot.create :member                             }
   let(:wrangler)     { FactoryBot.create :crop_wrangling_member              }
 
+  before do
+    crop = FactoryBot.create :tomato
+    crop.reindex
+  end
+
   def login
     fill_in 'Login', with: member.login_name
     fill_in 'Password', with: member.password

spec/services/crop_search_service_spec.rb

@@ -3,7 +3,7 @@
 require 'rails_helper'
 
 RSpec.describe CropSearchService, type: :service do
-  describe 'search', :elasticsearch do
+  describe 'search' do
     def search(term)
       CropSearchService.search(term).map(&:name)
     end
@@ -26,7 +26,7 @@ RSpec.describe CropSearchService, type: :service do
 
         # Child record
         FactoryBot.create(:crop, name: 'portobello', parent: mushroom)
-        Crop.reindex if ENV['GROWSTUFF_ELASTICSEARCH'] == 'true'
+        Crop.reindex
       end
 
       describe 'finds exact match' do
@@ -37,47 +37,45 @@ RSpec.describe CropSearchService, type: :service do
         it { expect(search('mush')).to eq ['mushroom'] }
       end
 
-      if ENV['GROWSTUFF_ELASTICSEARCH'] == 'true'
-        describe 'finds mispellings matches' do
-          it { expect(search('muhsroom')).to eq ['mushroom'] }
-          it { expect(search('mushrom')).to eq ['mushroom'] }
-          it { expect(search('zuchini')).to eq ['zucchini'] }
-          it { expect(search('brocoli')).to eq ['broccoli'] }
+      describe 'finds mispellings matches' do
+        it { expect(search('muhsroom')).to eq ['mushroom'] }
+        it { expect(search('mushrom')).to eq ['mushroom'] }
+        it { expect(search('zuchini')).to eq ['zucchini'] }
+        it { expect(search('brocoli')).to eq ['broccoli'] }
+      end
+
+      describe 'biased' do
+        # Make some crops with planting counts
+        let!(:mushroom_parent) { FactoryBot.create :crop, name: 'mushroom' }
+        let!(:oyster)  { FactoryBot.create :crop, name: 'oyster mushroom', parent: mushroom_parent }
+        let!(:shitake) { FactoryBot.create :crop, name: 'shitake mushroom', parent: mushroom_parent }
+        let!(:common)  { FactoryBot.create :crop, name: 'common mushroom', parent: mushroom_parent }
+        let!(:brown)   { FactoryBot.create :crop, name: 'brown mushroom', parent: mushroom_parent }
+        let!(:white)   { FactoryBot.create :crop, name: 'white mushroom', parent: mushroom_parent }
+
+        describe 'biased to higher planting counts' do
+          subject { search('mushroom') }
+          before do
+            # Having plantings should bring these crops to the top of the search results
+            FactoryBot.create_list :planting, 10, crop: white
+            FactoryBot.create_list :planting, 4, crop: shitake
+            Crop.reindex
+          end
+          it { expect(subject.first).to eq 'white mushroom' }
+          it { expect(subject.second).to eq 'shitake mushroom' }
         end
-
-        describe 'biased' do
-          # Make some crops with planting counts
-          let!(:mushroom_parent) { FactoryBot.create :crop, name: 'mushroom' }
-          let!(:oyster)  { FactoryBot.create :crop, name: 'oyster mushroom', parent: mushroom_parent }
-          let!(:shitake) { FactoryBot.create :crop, name: 'shitake mushroom', parent: mushroom_parent }
-          let!(:common)  { FactoryBot.create :crop, name: 'common mushroom', parent: mushroom_parent }
-          let!(:brown)   { FactoryBot.create :crop, name: 'brown mushroom', parent: mushroom_parent }
-          let!(:white)   { FactoryBot.create :crop, name: 'white mushroom', parent: mushroom_parent }
-
-          describe 'biased to higher planting counts' do
-            subject { search('mushroom') }
-            before do
-              # Having plantings should bring these crops to the top of the search results
-              FactoryBot.create_list :planting, 10, crop: white
-              FactoryBot.create_list :planting, 4, crop: shitake
-              Crop.reindex
-            end
-            it { expect(subject.first).to eq 'white mushroom' }
-            it { expect(subject.second).to eq 'shitake mushroom' }
-          end
-          describe "biased to crops you've planted" do
-            subject { CropSearchService.search('mushroom', current_member: owner).map(&:name) }
-            let(:owner) { FactoryBot.create :member }
-            before do
-              FactoryBot.create_list :planting, 10, crop: brown
-              FactoryBot.create :planting, crop: oyster, owner: owner
-              FactoryBot.create :planting, crop: oyster, owner: owner
-              FactoryBot.create :planting, crop: shitake, owner: owner
-              Crop.reindex
-            end
-            it { expect(subject.first).to eq oyster.name }
-            it { expect(subject.second).to eq shitake.name }
+        describe "biased to crops you've planted" do
+          subject { CropSearchService.search('mushroom', current_member: owner).map(&:name) }
+          let(:owner) { FactoryBot.create :member }
+          before do
+            FactoryBot.create_list :planting, 10, crop: brown
+            FactoryBot.create :planting, crop: oyster, owner: owner
+            FactoryBot.create :planting, crop: oyster, owner: owner
+            FactoryBot.create :planting, crop: shitake, owner: owner
+            Crop.reindex
           end
+          it { expect(subject.first).to eq oyster.name }
+          it { expect(subject.second).to eq shitake.name }
         end
       end
 
@@ -85,11 +83,9 @@ RSpec.describe CropSearchService, type: :service do
         it { expect(search('coffee')).to eq [] }
       end
 
-      if ENV['GROWSTUFF_ELASTICSEARCH'] == 'true'
-        describe 'finds plurals' do
-          it { expect(search('mushrooms')).to eq ['mushroom'] }
-          it { expect(search('tomatoes')).to eq ['tomato'] }
-        end
+      describe 'finds plurals' do
+        it { expect(search('mushrooms')).to eq ['mushroom'] }
+        it { expect(search('tomatoes')).to eq ['tomato'] }
       end
 
       describe 'searches case insensitively' do

spec/spec_helper.rb

@@ -35,13 +35,11 @@ RSpec.configure do |config|
   end
 
   config.before(:suite) do
-    if ENV["GROWSTUFF_ELASTICSEARCH"] == "true"
-      # reindex models
-      Crop.reindex
+    # reindex models
+    Crop.reindex
 
-      # and disable callbacks
-      Searchkick.disable_callbacks
-    end
+    # and disable callbacks
+    Searchkick.disable_callbacks
   end
 
   config.around(:each, search: true) do |example|

spec/support/feature_helpers.rb

@@ -1,6 +1,6 @@
 module FeatureHelpers
   def fill_autocomplete(field, options = {})
-    Crop.reindex if ENV["GROWSTUFF_ELASTICSEARCH"] == "true"
+    Crop.reindex
     fill_in field, with: options[:with]
   end
 

spec/views/home/index_spec.rb

@@ -16,6 +16,8 @@ describe 'home/index.html.haml', type: "view" do
     assign(:crops, [@crop])
     assign(:recent_crops, [@crop])
     assign(:seeds, [FactoryBot.create(:tradable_seed)])
+
+    Crop.reindex
   end
 
   context 'logged out' do