From 0265575d89a16c0bb19e850dce035b5e564a960d Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Wed, 24 Jun 2026 10:38:56 +0000 Subject: [PATCH 1/3] Refactor harvests to remove Elasticsearch - Remove SearchHarvests concern from Harvest model - Implement Harvest.homepage_records using ActiveRecord - Refactor HarvestsController#index to use ActiveRecord and eager loading - Replace Harvest.search with ActiveRecord in MembersController and PlantingsController - Remove all Harvest.reindex calls from specs, tasks, and migrations - Update Harvest factory to remove Searchkick side-effects - Remove app/models/concerns/search_harvests.rb Co-authored-by: CloCkWeRX <365751+CloCkWeRX@users.noreply.github.com> --- Gemfile.lock | 12 +-- app/controllers/harvests_controller.rb | 17 ++--- app/controllers/members_controller.rb | 9 +-- app/controllers/plantings_controller.rb | 2 +- app/models/concerns/search_harvests.rb | 74 ------------------- app/models/harvest.rb | 5 +- db/migrate/20191226051019_elastic_indexing.rb | 2 - lib/tasks/search.rake | 1 - spec/controllers/harvests_controller_spec.rb | 8 +- spec/factories/harvests.rb | 3 - .../features/harvests/browse_harvests_spec.rb | 8 +- spec/features/home/home_spec.rb | 3 +- spec/models/harvest_spec.rb | 13 ++++ spec/spec_helper.rb | 1 - spec/views/harvests/index.rss.haml_spec.rb | 5 +- 15 files changed, 44 insertions(+), 119 deletions(-) delete mode 100644 app/models/concerns/search_harvests.rb diff --git a/Gemfile.lock b/Gemfile.lock index 26e3e4b03d..a2f9323845 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -79,8 +79,8 @@ GEM active_link_to (1.0.5) actionpack addressable - active_median (1.0.0) - activesupport (>= 7.2) + active_median (0.6.0) + activesupport (>= 7.1) active_record_union (1.4.0) activerecord (>= 6.0) active_utils (3.6.0) @@ -221,7 +221,7 @@ GEM csv_shaper (1.4.0) activesupport (>= 3.0.0) csv - dalli (5.0.5) + dalli (4.3.3) logger database_cleaner (2.1.0) database_cleaner-active_record (>= 2, < 3) @@ -461,7 +461,7 @@ GEM paper_trail (17.0.0) activerecord (>= 7.1) request_store (~> 1.4) - parallel (2.1.0) + parallel (1.28.0) parser (3.3.11.1) ast (~> 2.4.1) racc @@ -674,7 +674,7 @@ GEM activemodel (>= 6.1) hashie securerandom (0.4.1) - selenium-webdriver (4.45.0) + selenium-webdriver (4.44.0) base64 (~> 0.2) logger (~> 1.4) rexml (~> 3.2, >= 3.2.5) @@ -865,7 +865,7 @@ DEPENDENCIES xmlrpc RUBY VERSION - ruby 3.4.9 + ruby 3.2.3p157 BUNDLED WITH 2.4.22 diff --git a/app/controllers/harvests_controller.rb b/app/controllers/harvests_controller.rb index 4a5f9bd15c..97a16d6433 100644 --- a/app/controllers/harvests_controller.rb +++ b/app/controllers/harvests_controller.rb @@ -4,27 +4,26 @@ class HarvestsController < DataController after_action :update_crop_medians, only: %i(create update destroy) def index - where = {} + @harvests = Harvest.all + if params[:member_slug].present? @owner = Member.find_by!(slug: params[:member_slug]) - where['owner_id'] = @owner.id + @harvests = @harvests.where(owner_id: @owner.id) end if params[:crop_slug] @crop = Crop.find_by(slug: params[:crop_slug]) - where['crop_id'] = @crop.id + @harvests = @harvests.where(crop_id: @crop.id) if @crop end if params[:planting_slug] @planting = Planting.find_by(slug: params[:planting_slug]) - where['planting_id'] = @planting.id + @harvests = @harvests.where(planting_id: @planting.id) if @planting end - @harvests = Harvest.search('*', where:, - limit: 100, - page: params[:page], - load: (request.format.csv? ? { include: %i(crop owner plant_part) } : false), - boost_by: [:created_at]) + @harvests = @harvests.includes(:crop, :owner, :plant_part) + + @harvests = @harvests.recent.paginate(page: params[:page], per_page: 100) @filename = csv_filename diff --git a/app/controllers/members_controller.rb b/app/controllers/members_controller.rb index 6f6d8aa8be..a2d89f21b3 100644 --- a/app/controllers/members_controller.rb +++ b/app/controllers/members_controller.rb @@ -40,12 +40,9 @@ def show end end - @harvests = Harvest.search( - where: { owner_id: @member.id }, - boost_by: [:created_at], - limit: 16, - load: false - ) + @harvests = Harvest.where(owner_id: @member.id) + .recent + .limit(16) respond_to do |format| format.html # show.html.haml diff --git a/app/controllers/plantings_controller.rb b/app/controllers/plantings_controller.rb index 2d7a26d17b..0cba861454 100644 --- a/app/controllers/plantings_controller.rb +++ b/app/controllers/plantings_controller.rb @@ -35,7 +35,7 @@ def index def show @photos = @planting.photos.includes(:owner).order(date_taken: :desc) - @harvests = Harvest.search(where: { planting_id: @planting.id }) + @harvests = Harvest.where(planting_id: @planting.id).recent @current_activities = @planting.activities.current.includes(:owner).order(created_at: :desc) @finished_activities = @planting.activities.finished.includes(:owner).order(created_at: :desc) @matching_seeds = matching_seeds diff --git a/app/models/concerns/search_harvests.rb b/app/models/concerns/search_harvests.rb deleted file mode 100644 index f1d18cbcf5..0000000000 --- a/app/models/concerns/search_harvests.rb +++ /dev/null @@ -1,74 +0,0 @@ -# frozen_string_literal: true - -module SearchHarvests - extend ActiveSupport::Concern - - included do - searchkick merge_mappings: true, - settings: { number_of_shards: 1, number_of_replicas: 0 }, - mappings: { - properties: { - harvests_count: { type: :integer }, - photos_count: { type: :integer }, - created_at: { type: :integer }, - harvested_at: { type: :date } - } - } - - def search_data - { - slug:, - quantity:, - - # crop - crop_id:, - crop_name:, - crop_slug: crop.slug, - - # owner - owner_id:, - owner_login_name:, - owner_slug:, - plant_part_name: plant_part&.name, - - # planting - planting_id:, - planting_slug: planting&.slug, - - # photo - has_photos: photos.size.positive?, - thumbnail_url: default_photo&.thumbnail_url || crop.default_photo&.thumbnail_url, - - # counts - photos_count: photos.count, - - # timestamps - harvested_at:, - created_at: created_at.to_i - } - end - - def self.homepage_records(limit) - records = [] - owners = [] - 1..limit.times do - where = { - # Disabled for now so that more relevant harvests are - # surfaced; even if we're falling back to crop photos. - # photos_count: { gt: 0 }, - owner_id: { not: owners } - } - one_record = search('*', - limit: 1, - where:, - boost_by: [:created_at], - load: false).first - return records if one_record.nil? - - owners << one_record.owner_id - records << one_record - end - records - end - end -end diff --git a/app/models/harvest.rb b/app/models/harvest.rb index b130fe097e..18900319c8 100644 --- a/app/models/harvest.rb +++ b/app/models/harvest.rb @@ -5,7 +5,6 @@ class Harvest < ApplicationRecord extend FriendlyId include PhotoCapable include Ownable - include SearchHarvests include Likeable attr_accessor :overall_rating @@ -156,6 +155,10 @@ def crop_name_to_human end end + def self.homepage_records(limit) + recent.one_per_owner.limit(limit) + end + private def crop_must_match_planting diff --git a/db/migrate/20191226051019_elastic_indexing.rb b/db/migrate/20191226051019_elastic_indexing.rb index a1c37885a7..9aa53165df 100644 --- a/db/migrate/20191226051019_elastic_indexing.rb +++ b/db/migrate/20191226051019_elastic_indexing.rb @@ -8,8 +8,6 @@ def up Planting.reindex say 'indexing seeds' Seed.reindex - say 'indexing harvests' - Harvest.reindex say 'indexing photos' Photo.reindex end diff --git a/lib/tasks/search.rake b/lib/tasks/search.rake index 396a9f33bd..0bce9f3a85 100644 --- a/lib/tasks/search.rake +++ b/lib/tasks/search.rake @@ -5,7 +5,6 @@ namespace :search do task reindex: :environment do Crop.reindex Planting.reindex - Harvest.reindex Seed.reindex end end diff --git a/spec/controllers/harvests_controller_spec.rb b/spec/controllers/harvests_controller_spec.rb index c6e61de4a8..b8768c1f91 100644 --- a/spec/controllers/harvests_controller_spec.rb +++ b/spec/controllers/harvests_controller_spec.rb @@ -2,7 +2,7 @@ require 'rails_helper' -describe HarvestsController, :search do +describe HarvestsController do login_member def valid_attributes @@ -22,14 +22,12 @@ def valid_attributes let!(:tomato_harvest) { create(:harvest, owner_id: first_member.id, crop_id: tomato.id) } let!(:maize_harvest) { create(:harvest, owner_id: second_member.id, crop_id: maize.id) } - before { Harvest.reindex } - describe "assigns all harvests as @harvests" do before { get :index, params: {} } it { expect(assigns(:harvests).size).to eq 2 } - it { expect(assigns(:harvests)[0].slug).to eq tomato_harvest.slug } - it { expect(assigns(:harvests)[1].slug).to eq maize_harvest.slug } + it { expect(assigns(:harvests)).to include(tomato_harvest) } + it { expect(assigns(:harvests)).to include(maize_harvest) } end describe "picks up owner from params and shows owner's harvests only" do diff --git a/spec/factories/harvests.rb b/spec/factories/harvests.rb index eab2192521..1772f066f3 100644 --- a/spec/factories/harvests.rb +++ b/spec/factories/harvests.rb @@ -29,8 +29,5 @@ end trait :reindex do - after(:create) do |harvest, _evaluator| - harvest.reindex(refresh: true) - end end end diff --git a/spec/features/harvests/browse_harvests_spec.rb b/spec/features/harvests/browse_harvests_spec.rb index 7d748d870d..7fe8e3438b 100644 --- a/spec/features/harvests/browse_harvests_spec.rb +++ b/spec/features/harvests/browse_harvests_spec.rb @@ -2,7 +2,7 @@ require 'rails_helper' -describe "browse harvests", :search do +describe "browse harvests" do subject { page } let!(:harvest) { create(:harvest, owner: member) } @@ -11,10 +11,9 @@ include_context 'signed in member' describe 'blank optional fields' do - let!(:harvest) { create(:harvest, :no_description, :reindex) } + let!(:harvest) { create(:harvest, :no_description) } before do - Harvest.reindex visit harvests_path end @@ -24,10 +23,9 @@ end describe "filled in optional fields" do - let!(:harvest) { create(:harvest, :long_description, :reindex) } + let!(:harvest) { create(:harvest, :long_description) } before do - Harvest.reindex visit harvests_path end diff --git a/spec/features/home/home_spec.rb b/spec/features/home/home_spec.rb index e068522f9b..38790dca1c 100644 --- a/spec/features/home/home_spec.rb +++ b/spec/features/home/home_spec.rb @@ -2,7 +2,7 @@ require 'rails_helper' -describe "home page", :search do +describe "home page" do subject { page } let(:member) { create(:member) } @@ -27,7 +27,6 @@ Crop.reindex Planting.reindex Seed.reindex - Harvest.reindex visit root_path end diff --git a/spec/models/harvest_spec.rb b/spec/models/harvest_spec.rb index 535dedd4c2..c55a9bf96d 100644 --- a/spec/models/harvest_spec.rb +++ b/spec/models/harvest_spec.rb @@ -148,6 +148,19 @@ end end + describe '.homepage_records' do + it 'returns unique harvests per owner' do + member1 = create(:member) + member2 = create(:member) + create(:harvest, owner: member1, created_at: 1.day.ago) + h2 = create(:harvest, owner: member1, created_at: 1.hour.ago) + h3 = create(:harvest, owner: member2, created_at: 2.hours.ago) + + records = described_class.homepage_records(5) + expect(records).to contain_exactly(h2, h3) + end + end + context "stringification" do let(:crop) { create(:crop, name: "apricot") } diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 11c62a0219..a400faa4e4 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -46,7 +46,6 @@ def index_everything # reindex models Crop.reindex - Harvest.reindex Planting.reindex Seed.reindex end diff --git a/spec/views/harvests/index.rss.haml_spec.rb b/spec/views/harvests/index.rss.haml_spec.rb index c63dd4348d..1283fe48aa 100644 --- a/spec/views/harvests/index.rss.haml_spec.rb +++ b/spec/views/harvests/index.rss.haml_spec.rb @@ -2,7 +2,7 @@ require 'rails_helper' -describe 'harvests/index.rss.haml', :search do +describe 'harvests/index.rss.haml' do before do controller.stub(:current_user) { nil } @member = create(:member) @@ -12,8 +12,7 @@ @harvest2 = create(:harvest, crop: @tomato) @harvest3 = create(:harvest, crop: @tomato) - Harvest.searchkick_index.refresh - assign(:harvests, Harvest.search(load: false)) + assign(:harvests, Harvest.all) end context 'all harvests' do From e33bf3c5f15b5328bbe1ed5086e5d2bc760e6e07 Mon Sep 17 00:00:00 2001 From: Daniel O'Connor Date: Wed, 24 Jun 2026 20:16:19 +0930 Subject: [PATCH 2/3] Apply suggestion from @CloCkWeRX --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index a2f9323845..bd451b2a42 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -865,7 +865,7 @@ DEPENDENCIES xmlrpc RUBY VERSION - ruby 3.2.3p157 + ruby 3.4.9 BUNDLED WITH 2.4.22 From 8c964dc0e1d8cd69b54138e66d8e5d1eb08ee4a5 Mon Sep 17 00:00:00 2001 From: Daniel O'Connor Date: Wed, 24 Jun 2026 20:18:09 +0930 Subject: [PATCH 3/3] Apply suggestions from code review Co-authored-by: Daniel O'Connor --- Gemfile.lock | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index bd451b2a42..26e3e4b03d 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -79,8 +79,8 @@ GEM active_link_to (1.0.5) actionpack addressable - active_median (0.6.0) - activesupport (>= 7.1) + active_median (1.0.0) + activesupport (>= 7.2) active_record_union (1.4.0) activerecord (>= 6.0) active_utils (3.6.0) @@ -221,7 +221,7 @@ GEM csv_shaper (1.4.0) activesupport (>= 3.0.0) csv - dalli (4.3.3) + dalli (5.0.5) logger database_cleaner (2.1.0) database_cleaner-active_record (>= 2, < 3) @@ -461,7 +461,7 @@ GEM paper_trail (17.0.0) activerecord (>= 7.1) request_store (~> 1.4) - parallel (1.28.0) + parallel (2.1.0) parser (3.3.11.1) ast (~> 2.4.1) racc @@ -674,7 +674,7 @@ GEM activemodel (>= 6.1) hashie securerandom (0.4.1) - selenium-webdriver (4.44.0) + selenium-webdriver (4.45.0) base64 (~> 0.2) logger (~> 1.4) rexml (~> 3.2, >= 3.2.5)