From c6824b126288a4788ea9640fa38e4f8da2b8d67b Mon Sep 17 00:00:00 2001
From: Brenda Wallace <brenda@wallace.net.nz>
Date: Sat, 11 Nov 2017 22:51:55 +1300
Subject: [PATCH 01/12] Planting's owner must be the same as its garden's owner

---
 app/models/planting.rb | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/app/models/planting.rb b/app/models/planting.rb
index f5f482a5d..4fe2ccc0a 100644
--- a/app/models/planting.rb
+++ b/app/models/planting.rb
@@ -50,6 +50,7 @@ class Planting < ActiveRecord::Base
   validates :garden, presence: true
   validates :crop, presence: true, approved: { message: "must be present and exist in our database" }
   validate :finished_must_be_after_planted
+  validate :owner_must_match_garden_owner
   validates :quantity, allow_nil: true, numericality: {
     only_integer: true, greater_than_or_equal_to: 0
   }
@@ -142,4 +143,8 @@ class Planting < ActiveRecord::Base
     return unless planted_at && finished_at # only check if we have both
     errors.add(:finished_at, "must be after the planting date") unless planted_at < finished_at
   end
+
+  def owner_must_match_garden_owner
+    errors.add(:owner, "must be the same as garden") unless owner == garden.owner
+  end
 end

From 7ca76e69cf1c187da4b1fe28be1f942248ebe519 Mon Sep 17 00:00:00 2001
From: Brenda Wallace <brenda@wallace.net.nz>
Date: Sat, 11 Nov 2017 22:52:40 +1300
Subject: [PATCH 02/12] Get planting factory's owner from its garden

---
 spec/factories/planting.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/spec/factories/planting.rb b/spec/factories/planting.rb
index 2deb014c5..2bcfac397 100644
--- a/spec/factories/planting.rb
+++ b/spec/factories/planting.rb
@@ -1,7 +1,7 @@
 FactoryBot.define do
   factory :planting do
-    garden
     owner
+    garden { FactoryBot.create :garden, owner: owner }
     crop
     planted_at Time.zone.local(2014, 7, 30)
     quantity 33

From a2ae429f6b1d17604a52f801209ef0195f0db056 Mon Sep 17 00:00:00 2001
From: Brenda Wallace <brenda@wallace.net.nz>
Date: Sun, 12 Nov 2017 15:51:29 +1300
Subject: [PATCH 03/12] If a harvest has a planting, it must have the same
 owner

---
 app/models/harvest.rb      | 6 ++++++
 spec/factories/harvests.rb | 3 ++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/app/models/harvest.rb b/app/models/harvest.rb
index 21339ab47..c2ad7eab1 100644
--- a/app/models/harvest.rb
+++ b/app/models/harvest.rb
@@ -58,6 +58,7 @@ class Harvest < ActiveRecord::Base
     in: WEIGHT_UNITS_VALUES.values, message: "%<value>s is not a valid unit"
   }
   validate :crop_must_match_planting
+  validate :owner_must_match_planting
   validate :harvest_must_be_after_planting
 
   def time_from_planting_to_harvest
@@ -131,6 +132,11 @@ class Harvest < ActiveRecord::Base
     errors.add(:planting, "must be the same crop") unless crop == planting.crop
   end
 
+  def owner_must_match_planting
+    return if planting.blank? # only check if we are linked to a planting
+    errors.add(:owner, "of harvest must be the same as planting") unless owner == planting.owner
+  end
+
   def harvest_must_be_after_planting
     # only check if we are linked to a planting
     return unless harvested_at.present? && planting.present? && planting.planted_at.present?
diff --git a/spec/factories/harvests.rb b/spec/factories/harvests.rb
index e1ff04fb3..c910f49da 100644
--- a/spec/factories/harvests.rb
+++ b/spec/factories/harvests.rb
@@ -4,7 +4,8 @@ FactoryBot.define do
   factory :harvest do
     crop
     plant_part
-    owner
+    planting
+    owner { planting.owner }
     harvested_at Time.zone.local(2015, 9, 17)
     quantity "3"
     unit "individual"

From 2fbba52bc8929b226627e20dbbe3126b847b9d24 Mon Sep 17 00:00:00 2001
From: Brenda Wallace <brenda@wallace.net.nz>
Date: Sat, 11 Nov 2017 22:53:07 +1300
Subject: [PATCH 04/12] Updating specs to use same owner for garden and
 planting

---
 spec/controllers/harvests_controller_spec.rb  | 101 +++++++++---------
 spec/factories/harvests.rb                    |  10 +-
 spec/features/gardens_spec.rb                 |   4 +-
 .../plantings/planting_a_crop_spec.rb         |   4 +-
 spec/models/crop_spec.rb                      |  34 +++---
 spec/models/garden_spec.rb                    |  40 +++----
 spec/models/planting_spec.rb                  |  16 ++-
 spec/views/crops/_grown_for.html.haml_spec.rb |  18 ++--
 .../crops/_planting_advice.html.haml_spec.rb  |  82 +++++++-------
 spec/views/gardens/show.html.haml_spec.rb     |   2 +-
 spec/views/harvests/show.html.haml_spec.rb    |  17 +--
 spec/views/plantings/_form.html.haml_spec.rb  |   1 +
 spec/views/plantings/edit.html.haml_spec.rb   |   2 +-
 spec/views/plantings/index.html.haml_spec.rb  |  40 +++----
 spec/views/plantings/new.html.haml_spec.rb    |   3 +-
 spec/views/plantings/show.html.haml_spec.rb   |  42 +++-----
 16 files changed, 213 insertions(+), 203 deletions(-)

diff --git a/spec/controllers/harvests_controller_spec.rb b/spec/controllers/harvests_controller_spec.rb
index ff4972a7a..c0dea9f2e 100644
--- a/spec/controllers/harvests_controller_spec.rb
+++ b/spec/controllers/harvests_controller_spec.rb
@@ -25,63 +25,61 @@ describe HarvestsController do
   end
 
   describe "GET index" do
-    before do
-      @member1 = FactoryBot.create(:member)
-      @member2 = FactoryBot.create(:member)
-      @tomato = FactoryBot.create(:tomato)
-      @maize = FactoryBot.create(:maize)
-      @harvest1 = FactoryBot.create(:harvest, owner_id: @member1.id, crop_id: @tomato.id)
-      @harvest2 = FactoryBot.create(:harvest, owner_id: @member2.id, crop_id: @maize.id)
+    let(:member1) { FactoryBot.create(:member) }
+    let(:member2) { FactoryBot.create(:member) }
+    let(:tomato) { FactoryBot.create(:tomato) }
+    let(:maize) { FactoryBot.create(:maize) }
+    let(:harvest1) { FactoryBot.create(:harvest, owner_id: member1.id, crop_id: tomato.id) }
+    let(:harvest2) { FactoryBot.create(:harvest, owner_id: member2.id, crop_id: maize.id) }
+
+    describe "assigns all harvests as @harvests" do
+      before { get :index, {} }
+      it { assigns(:harvests).should =~ [harvest1, harvest2] }
     end
 
-    it "assigns all harvests as @harvests" do
-      get :index, {}
-      assigns(:harvests).should =~ [@harvest1, @harvest2]
+    describe "picks up owner from params and shows owner's harvests only" do
+      before { get :index, owner: member1.slug }
+      it { expect(assigns(:owner)).to eq member1 }
+      it { expect(assigns(:harvests)).to eq [harvest1] }
     end
 
-    it "picks up owner from params and shows owner's harvests only" do
-      get :index, owner: @member1.slug
-      assigns(:owner).should eq @member1
-      assigns(:harvests).should eq [@harvest1]
+    describe "picks up crop from params and shows the harvests for the crop only" do
+      before { get :index, crop: maize.name }
+      it { expect(assigns(:crop)).to eq maize }
+      it { expect(assigns(:harvests)).to eq [harvest2] }
     end
 
-    it "picks up crop from params and shows the harvests for the crop only" do
-      get :index, crop: @maize.name
-      assigns(:crop).should eq @maize
-      assigns(:harvests).should eq [@harvest2]
-    end
-
-    it "generates a csv" do
-      get :index, format: "csv"
-      response.status.should eq 200
+    describe "generates a csv" do
+      before { get :index, format: "csv" }
+      it { expect(response.status).to eq 200 }
     end
   end
 
   describe "GET show" do
-    it "assigns the requested harvest as @harvest" do
-      harvest = Harvest.create! valid_attributes
-      get :show, id: harvest.to_param
-      assigns(:harvest).should eq(harvest)
+    let(:harvest) { Harvest.create! valid_attributes }
+    describe "assigns the requested harvest as @harvest" do
+      before { get :show, id: harvest.to_param }
+      it { expect(assigns(:harvest)).to eq(harvest) }
     end
   end
 
   describe "GET new" do
-    it "assigns a new harvest as @harvest" do
-      get :new, {}
-      assigns(:harvest).should be_a_new(Harvest)
+    before { get :new, {} }
+
+    describe "assigns a new harvest as @harvest" do
+      it { expect(assigns(:harvest)).to be_a_new(Harvest) }
     end
 
-    it "sets the date of the harvest to today" do
-      get :new, {}
-      assigns(:harvest).harvested_at.should == Time.zone.today
+    describe "sets the date of the harvest to today" do
+      it { expect(assigns(:harvest).harvested_at).to eq(Time.zone.today) }
     end
   end
 
   describe "GET edit" do
-    it "assigns the requested harvest as @harvest" do
-      harvest = Harvest.create! valid_attributes
-      get :edit, id: harvest.to_param
-      assigns(:harvest).should eq(harvest)
+    let(:harvest) { Harvest.create! valid_attributes }
+    describe "assigns the requested harvest as @harvest" do
+      before { get :edit, id: harvest.to_param }
+      it { expect(assigns(:harvest)).to eq(harvest) }
     end
   end
 
@@ -104,10 +102,10 @@ describe HarvestsController do
         response.should redirect_to(Harvest.last)
       end
 
-      it "links to planting" do
-        planting = FactoryBot.create(:planting, owner_id: member.id)
-        post :create, harvest: valid_attributes.merge(planting_id: planting.id)
-        expect(Harvest.last.planting.id).to eq(planting.id)
+      describe "links to planting" do
+        let(:planting) { FactoryBot.create(:planting, owner_id: member.id, garden: member.gardens.first) }
+        before { post :create, harvest: valid_attributes.merge(planting_id: planting.id) }
+        it { expect(Harvest.last.planting.id).to eq(planting.id) }
       end
     end
 
@@ -129,10 +127,13 @@ describe HarvestsController do
     describe "not my planting" do
       let(:not_my_planting) { FactoryBot.create(:planting) }
       let(:harvest) { FactoryBot.create(:harvest) }
-      it "does not save planting_id" do
-        allow(Harvest).to receive(:new).and_return(harvest)
-        post :create, harvest: valid_attributes.merge(planting_id: not_my_planting.id)
-        expect(harvest.planting_id).to eq(nil)
+
+      describe "does not save planting_id" do
+        before do
+          allow(Harvest).to receive(:new).and_return(harvest)
+          post :create, harvest: valid_attributes.merge(planting_id: not_my_planting.id)
+        end
+        it { expect(harvest.planting_id).not_to eq(not_my_planting.id) }
       end
     end
   end
@@ -181,10 +182,12 @@ describe HarvestsController do
     describe "not my planting" do
       let(:not_my_planting) { FactoryBot.create(:planting) }
       let(:harvest) { FactoryBot.create(:harvest) }
-      it "does not save planting_id" do
-        put :update, id: harvest.to_param,
-                     harvest: valid_attributes.merge(planting_id: not_my_planting.id)
-        expect(harvest.planting_id).to eq(nil)
+      describe "does not save planting_id" do
+        before do
+          put :update, id: harvest.to_param,
+                       harvest: valid_attributes.merge(planting_id: not_my_planting.id)
+        end
+        it { expect(harvest.planting_id).to eq(nil) }
       end
     end
   end
diff --git a/spec/factories/harvests.rb b/spec/factories/harvests.rb
index c910f49da..c707a2e2c 100644
--- a/spec/factories/harvests.rb
+++ b/spec/factories/harvests.rb
@@ -2,16 +2,20 @@
 
 FactoryBot.define do
   factory :harvest do
-    crop
+    crop { planting.present? ? planting.crop : FactoryBot.create(:crop) }
     plant_part
-    planting
-    owner { planting.owner }
+    planting nil
+    owner { planting.present? ? planting.owner : FactoryBot.create(:member) }
     harvested_at Time.zone.local(2015, 9, 17)
     quantity "3"
     unit "individual"
     weight_quantity 6
     weight_unit "kg"
     description "A lovely harvest"
+
+    factory :harvest_with_planting do
+      planting
+    end
   end
 
   trait :long_description do
diff --git a/spec/features/gardens_spec.rb b/spec/features/gardens_spec.rb
index 3d78cbe5d..e9a20d060 100644
--- a/spec/features/gardens_spec.rb
+++ b/spec/features/gardens_spec.rb
@@ -2,9 +2,9 @@ require 'rails_helper'
 
 feature "Planting a crop", js: true do
   let!(:garden) { create :garden }
-  let!(:planting) { create :planting, garden: garden, planted_at: Date.parse("2013-3-10") }
+  let!(:planting) { create :planting, garden: garden, owner: garden.owner, planted_at: Date.parse("2013-3-10") }
   let!(:tomato) { create :tomato }
-  let!(:finished_planting) { create :finished_planting, garden: garden, crop: tomato }
+  let!(:finished_planting) { create :finished_planting, owner: garden.owner, garden: garden, crop: tomato }
 
   background do
     login_as garden.owner
diff --git a/spec/features/plantings/planting_a_crop_spec.rb b/spec/features/plantings/planting_a_crop_spec.rb
index 177cf9942..0c984837b 100644
--- a/spec/features/plantings/planting_a_crop_spec.rb
+++ b/spec/features/plantings/planting_a_crop_spec.rb
@@ -5,7 +5,9 @@ feature "Planting a crop", :js, :elasticsearch do
   let(:member) { create :member }
   let!(:maize) { create :maize }
   let(:garden) { create :garden, owner: member }
-  let!(:planting) { create :planting, garden: garden, planted_at: Date.parse("2013-3-10") }
+  let!(:planting) do
+    create :planting, garden: garden, owner: member, planted_at: Date.parse("2013-3-10")
+  end
 
   background do
     login_as member
diff --git a/spec/models/crop_spec.rb b/spec/models/crop_spec.rb
index 406a65eb0..860e2e935 100644
--- a/spec/models/crop_spec.rb
+++ b/spec/models/crop_spec.rb
@@ -348,7 +348,23 @@ describe Crop do
     end
   end
 
+  let(:maize) { FactoryBot.create(:maize) }
+  let(:pp1) { FactoryBot.create(:plant_part) }
+  let(:pp2) { FactoryBot.create(:plant_part) }
+
   context "harvests" do
+    let(:h1) do
+      FactoryBot.create(:harvest,
+        crop: maize,
+        plant_part: pp1)
+    end
+
+    let(:h2) do
+      FactoryBot.create(:harvest,
+        crop: maize,
+        plant_part: pp2)
+    end
+
     it "has harvests" do
       crop = FactoryBot.create(:crop)
       harvest = FactoryBot.create(:harvest, crop: crop)
@@ -356,20 +372,6 @@ describe Crop do
     end
   end
 
-  it 'has plant_parts' do
-    @maize = FactoryBot.create(:maize)
-    @pp1 = FactoryBot.create(:plant_part)
-    @pp2 = FactoryBot.create(:plant_part)
-    @h1 = FactoryBot.create(:harvest,
-      crop: @maize,
-      plant_part: @pp1)
-    @h2 = FactoryBot.create(:harvest,
-      crop: @maize,
-      plant_part: @pp2)
-    @maize.plant_parts.should include @pp1
-    @maize.plant_parts.should include @pp2
-  end
-
   it "doesn't duplicate plant_parts" do
     @maize = FactoryBot.create(:maize)
     @pp1 = FactoryBot.create(:plant_part)
@@ -385,9 +387,7 @@ describe Crop do
   context "search", :elasticsearch do
     let(:mushroom) { FactoryBot.create(:crop, name: 'mushroom') }
 
-    before do
-      sync_elasticsearch([mushroom])
-    end
+    before { sync_elasticsearch([mushroom]) }
 
     it "finds exact matches" do
       Crop.search('mushroom').should eq [mushroom]
diff --git a/spec/models/garden_spec.rb b/spec/models/garden_spec.rb
index 867b333d1..cbb097d91 100644
--- a/spec/models/garden_spec.rb
+++ b/spec/models/garden_spec.rb
@@ -64,30 +64,30 @@ describe Garden do
     let(:walnut) { FactoryBot.create(:walnut) }
 
     it "should fetch < 4 featured plantings if insufficient exist" do
-      @p1 = FactoryBot.create(:planting, crop: tomato, garden: garden)
-      @p2 = FactoryBot.create(:planting, crop: maize, garden: garden)
+      @p1 = FactoryBot.create(:planting, crop: tomato, garden: garden, owner: garden.owner)
+      @p2 = FactoryBot.create(:planting, crop: maize, garden: garden, owner: garden.owner)
 
       garden.featured_plantings.should eq [@p2, @p1]
     end
 
     it "should fetch most recent 4 featured plantings" do
-      @p1 = FactoryBot.create(:planting, crop: tomato, garden: garden)
-      @p2 = FactoryBot.create(:planting, crop: maize, garden: garden)
-      @p3 = FactoryBot.create(:planting, crop: chard, garden: garden)
-      @p4 = FactoryBot.create(:planting, crop: apple, garden: garden)
-      @p5 = FactoryBot.create(:planting, crop: walnut, garden: garden)
+      @p1 = FactoryBot.create(:planting, crop: tomato, garden: garden, owner: garden.owner)
+      @p2 = FactoryBot.create(:planting, crop: maize, garden: garden, owner: garden.owner)
+      @p3 = FactoryBot.create(:planting, crop: chard, garden: garden, owner: garden.owner)
+      @p4 = FactoryBot.create(:planting, crop: apple, garden: garden, owner: garden.owner)
+      @p5 = FactoryBot.create(:planting, crop: walnut, garden: garden, owner: garden.owner)
 
       garden.featured_plantings.should eq [@p5, @p4, @p3, @p2]
     end
 
     it "should skip repeated plantings" do
-      @p1 = FactoryBot.create(:planting, crop: tomato, garden: garden)
-      @p2 = FactoryBot.create(:planting, crop: maize, garden: garden)
-      @p3 = FactoryBot.create(:planting, crop: chard, garden: garden)
-      @p4 = FactoryBot.create(:planting, crop: apple, garden: garden)
-      @p5 = FactoryBot.create(:planting, crop: walnut, garden: garden)
-      @p6 = FactoryBot.create(:planting, crop: apple, garden: garden)
-      @p7 = FactoryBot.create(:planting, crop: pear, garden: garden)
+      @p1 = FactoryBot.create(:planting, crop: tomato, garden: garden, owner: garden.owner)
+      @p2 = FactoryBot.create(:planting, crop: maize, garden: garden, owner: garden.owner)
+      @p3 = FactoryBot.create(:planting, crop: chard, garden: garden, owner: garden.owner)
+      @p4 = FactoryBot.create(:planting, crop: apple, garden: garden, owner: garden.owner)
+      @p5 = FactoryBot.create(:planting, crop: walnut, garden: garden, owner: garden.owner)
+      @p6 = FactoryBot.create(:planting, crop: apple, garden: garden, owner: garden.owner)
+      @p7 = FactoryBot.create(:planting, crop: pear, garden: garden, owner: garden.owner)
 
       garden.featured_plantings.should eq [@p7, @p6, @p5, @p3]
     end
@@ -103,8 +103,8 @@ describe Garden do
 
   it "destroys plantings when deleted" do
     garden = FactoryBot.create(:garden, owner: owner)
-    @planting1 = FactoryBot.create(:planting, garden: garden)
-    @planting2 = FactoryBot.create(:planting, garden: garden)
+    @planting1 = FactoryBot.create(:planting, garden: garden, owner: garden.owner)
+    @planting2 = FactoryBot.create(:planting, garden: garden, owner: garden.owner)
     garden.plantings.size.should eq(2)
     all = Planting.count
     garden.destroy
@@ -185,8 +185,8 @@ describe Garden do
 
   it "marks plantings as finished when garden is inactive" do
     garden = FactoryBot.create(:garden)
-    p1 = FactoryBot.create(:planting, garden: garden)
-    p2 = FactoryBot.create(:planting, garden: garden)
+    p1 = FactoryBot.create(:planting, garden: garden, owner: garden.owner)
+    p2 = FactoryBot.create(:planting, garden: garden, owner: garden.owner)
 
     p1.finished.should eq false
     p2.finished.should eq false
@@ -203,8 +203,8 @@ describe Garden do
   it "doesn't mark the wrong plantings as finished" do
     g1 = FactoryBot.create(:garden)
     g2 = FactoryBot.create(:garden)
-    p1 = FactoryBot.create(:planting, garden: g1)
-    p2 = FactoryBot.create(:planting, garden: g2)
+    p1 = FactoryBot.create(:planting, garden: g1, owner: g1.owner)
+    p2 = FactoryBot.create(:planting, garden: g2, owner: g2.owner)
 
     # mark the garden as inactive
     g1.active = false
diff --git a/spec/models/planting_spec.rb b/spec/models/planting_spec.rb
index 462bee3e4..ec9eb0746 100644
--- a/spec/models/planting_spec.rb
+++ b/spec/models/planting_spec.rb
@@ -4,7 +4,7 @@ describe Planting do
   let(:crop) { FactoryBot.create(:tomato) }
   let(:garden_owner) { FactoryBot.create(:member) }
   let(:garden) { FactoryBot.create(:garden, owner: garden_owner) }
-  let(:planting) { FactoryBot.create(:planting, crop: crop, garden: garden) }
+  let(:planting) { FactoryBot.create(:planting, crop: crop, garden: garden, owner: garden.owner) }
   let(:finished_planting) do
     FactoryBot.create :planting, planted_at: 4.days.ago, finished_at: 2.days.ago, finished: true
   end
@@ -120,7 +120,10 @@ describe Planting do
     describe 'planting has first harvest' do
       let(:planting) { FactoryBot.create :planting, planted_at: 100.days.ago }
       before do
-        FactoryBot.create :harvest, planting: planting, crop: planting.crop, harvested_at: 10.days.ago
+        FactoryBot.create(:harvest,
+          planting: planting,
+          crop: planting.crop,
+          harvested_at: 10.days.ago)
         planting.update_harvest_days
         planting.crop.update_harvest_medians
       end
@@ -148,12 +151,6 @@ describe Planting do
     planting.owner.should be_an_instance_of Member
   end
 
-  it "owner isn't necessarily the garden owner" do
-    # a new owner should be created automatically by FactoryBot
-    # note that formerly, the planting belonged to an owner through the garden
-    planting.owner.should_not eq garden_owner
-  end
-
   it "generates a location" do
     planting.location.should eq "#{garden_owner.login_name}'s #{garden.name}"
   end
@@ -355,7 +352,8 @@ describe Planting do
         # this one is newer, and has the same owner, through the garden
         @planting2 = FactoryBot.create(:planting,
           created_at: 1.minute.ago,
-          owner_id: @planting1.owner.id)
+          garden: @planting1.garden,
+          owner: @planting1.owner)
         @planting2.photos << FactoryBot.create(:photo)
         @planting2.save
 
diff --git a/spec/views/crops/_grown_for.html.haml_spec.rb b/spec/views/crops/_grown_for.html.haml_spec.rb
index 290d5d8c6..183445346 100644
--- a/spec/views/crops/_grown_for.html.haml_spec.rb
+++ b/spec/views/crops/_grown_for.html.haml_spec.rb
@@ -13,17 +13,17 @@
 require 'rails_helper'
 
 describe "crops/_grown_for" do
-  before(:each) do
-    @crop = FactoryBot.create(:crop)
-    @pp = FactoryBot.create(:plant_part)
-    @harvest = FactoryBot.create(:harvest,
-      crop: @crop,
-      plant_part: @pp)
+  let(:crop) { FactoryBot.create(:crop) }
+  let(:plant_path) { FactoryBot.create(:plant_part) }
+  let!(:harvest) do
+    FactoryBot.create(:harvest,
+      crop: crop,
+      plant_part: plant_path)
   end
 
   it 'shows plant parts' do
-    render partial: 'crops/grown_for', locals: { crop: @crop }
-    rendered.should have_content @pp.name
-    assert_select "a", href: plant_part_path(@pp)
+    render partial: 'crops/grown_for', locals: { crop: crop }
+    rendered.should have_content plant_path.name
+    assert_select "a", href: plant_part_path(plant_path)
   end
 end
diff --git a/spec/views/crops/_planting_advice.html.haml_spec.rb b/spec/views/crops/_planting_advice.html.haml_spec.rb
index 58df0584d..2e6bf9e93 100644
--- a/spec/views/crops/_planting_advice.html.haml_spec.rb
+++ b/spec/views/crops/_planting_advice.html.haml_spec.rb
@@ -13,58 +13,62 @@
 require 'rails_helper'
 
 describe "crops/_planting_advice" do
-  before(:each) do
-    @owner = FactoryBot.create(:member)
-    @crop = FactoryBot.create(:crop)
-    @garden   = FactoryBot.create(:garden, owner: @owner)
-    @planting = FactoryBot.create(:planting,
-      garden: @garden,
-      crop: @crop)
+  let(:planting) { FactoryBot.create(:planting) }
+  subject { rendered }
+
+  shared_examples "render planting_advice" do
+    before { render 'crops/planting_advice', crop: planting.crop }
   end
 
-  context "sunniness" do
-    it "doesn't show sunniness if none are set" do
-      render partial: 'crops/planting_advice', locals: { crop: @crop }
-      rendered.should have_content "Plant in: not known."
+  describe "sunniness" do
+    context "with no sunniness set" do
+      include_examples "render planting_advice"
+      it "doesn't show sunniness" do
+        is_expected.to have_content "Plant in: not known."
+      end
     end
 
-    it "shows sunniness frequencies" do
-      FactoryBot.create(:sunny_planting, crop: @crop)
-      render partial: 'crops/planting_advice', locals: { crop: @crop }
-      rendered.should have_content "Plant in:"
-      rendered.should have_content "sun (1)"
+    context "with sunniness frequencies" do
+      before { FactoryBot.create(:sunny_planting, crop: planting.crop) }
+      include_examples "render planting_advice"
+      it { is_expected.to have_content "Plant in:" }
+      it { is_expected.to have_content "sun (1)" }
     end
 
-    it "shows multiple sunniness frequencies" do
-      FactoryBot.create(:sunny_planting, crop: @crop)
-      FactoryBot.create(:sunny_planting, crop: @crop)
-      FactoryBot.create(:shady_planting, crop: @crop)
-      render partial: 'crops/planting_advice', locals: { crop: @crop }
-      rendered.should have_content "Plant in:"
-      rendered.should have_content "sun (2), shade (1)"
+    context "with multiple sunniness frequencies" do
+      before do
+        FactoryBot.create_list(:sunny_planting, 2, crop: planting.crop)
+        FactoryBot.create(:shady_planting, crop: planting.crop)
+      end
+      include_examples "render planting_advice"
+      it { is_expected.to have_content "Plant in:" }
+      it { is_expected.to have_content "sun (2), shade (1)" }
     end
   end
 
-  context "planted from" do
-    it "doesn't show planted_from if none are set" do
-      render partial: 'crops/planting_advice', locals: { crop: @crop }
-      rendered.should have_content "Plant from: not known."
+  describe "planted from" do
+    context "when none are set" do
+      include_examples "render planting_advice"
+      it "doesn't show planted_from " do
+        is_expected.to have_content "Plant from: not known."
+      end
     end
 
-    it "shows planted_from frequencies" do
-      FactoryBot.create(:seed_planting, crop: @crop)
-      render partial: 'crops/planting_advice', locals: { crop: @crop }
-      rendered.should have_content "Plant from:"
-      rendered.should have_content "seed (1)"
+    context "with planted_from frequencies" do
+      before { FactoryBot.create(:seed_planting, crop: planting.crop) }
+      include_examples "render planting_advice"
+      it { is_expected.to have_content "Plant from:" }
+      it { is_expected.to have_content "seed (1)" }
     end
 
-    it "shows multiple planted_from frequencies" do
-      FactoryBot.create(:seed_planting, crop: @crop)
-      FactoryBot.create(:seed_planting, crop: @crop)
-      FactoryBot.create(:cutting_planting, crop: @crop)
-      render partial: 'crops/planting_advice', locals: { crop: @crop }
-      rendered.should have_content "Plant from:"
-      rendered.should have_content "seed (2), cutting (1)"
+    context "with multiple planted_from frequencies" do
+      before do
+        FactoryBot.create_list(:seed_planting, 2, crop: planting.crop)
+        FactoryBot.create(:cutting_planting, crop: planting.crop)
+      end
+      include_examples "render planting_advice"
+      it { is_expected.to have_content "Plant from:" }
+      it { is_expected.to have_content "seed (2), cutting (1)" }
     end
   end
 end
diff --git a/spec/views/gardens/show.html.haml_spec.rb b/spec/views/gardens/show.html.haml_spec.rb
index 83f993a16..b22ee0452 100644
--- a/spec/views/gardens/show.html.haml_spec.rb
+++ b/spec/views/gardens/show.html.haml_spec.rb
@@ -17,7 +17,7 @@ describe "gardens/show" do
     @owner = FactoryBot.create(:member)
     controller.stub(:current_user) { @owner }
     @garden   = FactoryBot.create(:garden, owner: @owner)
-    @planting = FactoryBot.create(:planting, garden: @garden)
+    @planting = FactoryBot.create(:planting, garden: @garden, owner: @garden.owner)
     assign(:garden, @garden)
     assign(:current_plantings, [@planting])
     assign(:finished_plantings, [])
diff --git a/spec/views/harvests/show.html.haml_spec.rb b/spec/views/harvests/show.html.haml_spec.rb
index 129648c71..b13d53ed6 100644
--- a/spec/views/harvests/show.html.haml_spec.rb
+++ b/spec/views/harvests/show.html.haml_spec.rb
@@ -13,16 +13,19 @@
 require 'rails_helper'
 
 describe "harvests/show" do
-  before(:each) do
+  let!(:harvest) { FactoryBot.create(:harvest) }
+
+  before do
     controller.stub(:current_user) { nil }
-    @crop = FactoryBot.create(:tomato)
-    @harvest = assign(:harvest, FactoryBot.create(:harvest, crop: @crop))
+    assign(:harvest, harvest)
     render
   end
 
-  it "renders attributes" do
-    rendered.should have_content @crop.name
-    rendered.should have_content @harvest.harvested_at.to_s
-    rendered.should have_content @harvest.plant_part.to_s
+  subject { render }
+
+  describe "renders attributes" do
+    it { is_expected.to have_content harvest.crop.name }
+    it { is_expected.to have_content harvest.harvested_at.to_s }
+    it { is_expected.to have_content harvest.plant_part.to_s }
   end
 end
diff --git a/spec/views/plantings/_form.html.haml_spec.rb b/spec/views/plantings/_form.html.haml_spec.rb
index 59d4d832b..926150f73 100644
--- a/spec/views/plantings/_form.html.haml_spec.rb
+++ b/spec/views/plantings/_form.html.haml_spec.rb
@@ -24,6 +24,7 @@ describe "plantings/_form" do
     @planting = FactoryBot.create(:planting,
       garden: @garden,
       crop: @crop,
+      owner: @member,
       planted_at: Date.new(2013, 3, 1))
     render
   end
diff --git a/spec/views/plantings/edit.html.haml_spec.rb b/spec/views/plantings/edit.html.haml_spec.rb
index c37f9947a..fc7ff7f30 100644
--- a/spec/views/plantings/edit.html.haml_spec.rb
+++ b/spec/views/plantings/edit.html.haml_spec.rb
@@ -28,7 +28,7 @@ describe "plantings/edit" do
     @garden2 = FactoryBot.create(:garden_a, owner: @member)
 
     @planting = assign(:planting,
-      FactoryBot.create(:planting, garden: @garden, crop: @tomato))
+      FactoryBot.create(:planting, garden: @garden, crop: @tomato, owner: @member))
   end
 
   context "logged in" do
diff --git a/spec/views/plantings/index.html.haml_spec.rb b/spec/views/plantings/index.html.haml_spec.rb
index e7831889f..10de32f81 100644
--- a/spec/views/plantings/index.html.haml_spec.rb
+++ b/spec/views/plantings/index.html.haml_spec.rb
@@ -13,29 +13,31 @@
 require 'rails_helper'
 
 describe "plantings/index" do
+  let(:member) { FactoryBot.create(:member) }
+  let(:garden) { FactoryBot.create(:garden, owner: member) }
+  let(:tomato) { FactoryBot.create(:tomato) }
+  let(:maize)  { FactoryBot.create(:maize) }
   before(:each) do
     controller.stub(:current_user) { nil }
-    @member = FactoryBot.create(:member)
-    @garden = FactoryBot.create(:garden, owner: @member)
-    @tomato = FactoryBot.create(:tomato)
-    @maize  = FactoryBot.create(:maize)
     page = 1
     per_page = 3
     total_entries = 3
     plantings = WillPaginate::Collection.create(page, per_page, total_entries) do |pager|
       pager.replace([
                       FactoryBot.create(:planting,
-                        garden: @garden,
-                        crop: @tomato,
-                        owner: @member),
+                        garden: garden,
+                        crop: tomato,
+                        owner: member),
                       FactoryBot.create(:planting,
-                        garden: @garden,
-                        crop: @maize,
+                        garden: garden,
+                        crop: maize,
+                        owner: garden.owner,
                         description: '',
                         planted_at: Time.zone.local(2013, 1, 13)),
                       FactoryBot.create(:planting,
-                        garden: @garden,
-                        crop: @tomato,
+                        garden: garden,
+                        owner: garden.owner,
+                        crop: tomato,
                         planted_at: Time.zone.local(2013, 1, 13),
                         finished_at: Time.zone.local(2013, 1, 20),
                         finished: true)
@@ -46,10 +48,10 @@ describe "plantings/index" do
   end
 
   it "renders a list of plantings" do
-    rendered.should have_content @tomato.name
-    rendered.should have_content @maize.name
-    rendered.should have_content @member.login_name
-    rendered.should have_content @garden.name
+    rendered.should have_content tomato.name
+    rendered.should have_content maize.name
+    rendered.should have_content member.login_name
+    rendered.should have_content garden.name
   end
 
   it "displays planting time" do
@@ -69,14 +71,14 @@ describe "plantings/index" do
   end
 
   it "displays member's name in title" do
-    assign(:owner, @member)
+    assign(:owner, member)
     render
-    view.content_for(:title).should have_content @member.login_name
+    view.content_for(:title).should have_content member.login_name
   end
 
   it "displays crop's name in title" do
-    assign(:crop, @tomato)
+    assign(:crop, tomato)
     render
-    view.content_for(:title).should have_content @tomato.name
+    view.content_for(:title).should have_content tomato.name
   end
 end
diff --git a/spec/views/plantings/new.html.haml_spec.rb b/spec/views/plantings/new.html.haml_spec.rb
index 9eb941d1d..db1da2826 100644
--- a/spec/views/plantings/new.html.haml_spec.rb
+++ b/spec/views/plantings/new.html.haml_spec.rb
@@ -25,7 +25,8 @@ describe "plantings/new" do
 
     assign(:planting, FactoryBot.create(:planting,
       garden: @garden_a,
-      crop: @crop2))
+      crop: @crop2,
+      owner: @member))
   end
 
   context "logged in" do
diff --git a/spec/views/plantings/show.html.haml_spec.rb b/spec/views/plantings/show.html.haml_spec.rb
index f948ca394..b68ceb6c1 100644
--- a/spec/views/plantings/show.html.haml_spec.rb
+++ b/spec/views/plantings/show.html.haml_spec.rb
@@ -13,25 +13,22 @@
 require 'rails_helper'
 
 describe "plantings/show" do
-  def create_planting_for(member)
-    @garden = FactoryBot.create(:garden, owner: @member)
-    @crop = FactoryBot.create(:tomato)
-    @planting = assign(:planting,
-      FactoryBot.create(:planting, garden: @garden, crop: @crop,
-                                   planted_from: 'cutting'))
+  let(:crop) { FactoryBot.create(:tomato) }
+  let(:member) { FactoryBot.create(:member) }
+  let(:garden) { FactoryBot.create(:garden, owner: member) }
+  let(:planting) do
+    FactoryBot.create(:planting, garden: garden, crop: crop,
+                                 owner: garden.owner,
+                                 planted_from: 'cutting')
   end
 
   before(:each) do
-    @member = FactoryBot.create(:member)
-    controller.stub(:current_user) { @member }
-    @p = create_planting_for(@member)
+    assign(:planting, planting)
+    controller.stub(:current_user) { member }
   end
 
   context 'sunniness' do
-    before(:each) do
-      @p = assign(:planting,
-        FactoryBot.create(:sunny_planting))
-    end
+    let(:planting) { FactoryBot.create(:sunny_planting) }
 
     it "shows the sunniness" do
       render
@@ -41,10 +38,7 @@ describe "plantings/show" do
   end
 
   context 'planted from' do
-    before(:each) do
-      @p = assign(:planting, FactoryBot.create(:cutting_planting))
-    end
-
+    let(:planting) { FactoryBot.create(:cutting_planting) }
     it "shows planted_from" do
       render
       rendered.should have_content 'Planted from:'
@@ -52,8 +46,7 @@ describe "plantings/show" do
     end
 
     it "doesn't show planted_from if blank" do
-      @p.planted_from = ''
-      @p.save
+      planting.update(planted_from:  '')
       render
       rendered.should_not have_content 'Planted from:'
       rendered.should_not have_content 'cutting'
@@ -61,10 +54,10 @@ describe "plantings/show" do
   end
 
   it "shows photos" do
-    @photo = FactoryBot.create(:photo, owner: @member)
-    @p.photos << @photo
+    photo = FactoryBot.create(:photo, owner: member)
+    planting.photos << photo
     render
-    assert_select "img[src='#{@photo.thumbnail_url}']"
+    assert_select "img[src='#{photo.thumbnail_url}']"
   end
 
   it "shows a link to add photos" do
@@ -96,13 +89,12 @@ describe "plantings/show" do
 
   context "location set" do
     before(:each) do
-      @p.owner.location = 'Greenwich, UK'
-      @p.owner.save
+      planting.owner.update(location: 'Greenwich, UK')
       render
     end
 
     it "shows the member's location in parentheses" do
-      rendered.should have_content "(#{@p.owner.location})"
+      rendered.should have_content "(#{planting.owner.location})"
     end
   end
 end

From df7a9448b0ff9a5e8405fce5d43ee2e2e7e8a965 Mon Sep 17 00:00:00 2001
From: Shiny <brenda@wallace.net.nz>
Date: Mon, 13 Nov 2017 22:54:55 +1300
Subject: [PATCH 05/12] Underscore prefix on our last unused args (#1442)

---
 .rubocop_todo.yml                      | 7 -------
 app/controllers/sessions_controller.rb | 2 +-
 config/unicorn.rb                      | 4 ++--
 3 files changed, 3 insertions(+), 10 deletions(-)

diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
index b303fd313..211de1ca6 100644
--- a/.rubocop_todo.yml
+++ b/.rubocop_todo.yml
@@ -10,13 +10,6 @@ Lint/HandleExceptions:
   Exclude:
     - 'lib/tasks/testing.rake'
 
-# Cop supports --auto-correct.
-# Configuration parameters: IgnoreEmptyBlocks, AllowUnusedKeywordArguments.
-Lint/UnusedBlockArgument:
-  Exclude:
-    - 'app/controllers/sessions_controller.rb'
-    - 'config/unicorn.rb'
-
 # Cop supports --auto-correct.
 # Configuration parameters: AllowUnusedKeywordArguments, IgnoreEmptyMethods.
 Lint/UnusedMethodArgument:
diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb
index 968e71e33..6106e0c75 100644
--- a/app/controllers/sessions_controller.rb
+++ b/app/controllers/sessions_controller.rb
@@ -2,7 +2,7 @@ class SessionsController < Devise::SessionsController
   respond_to :json
 
   def create
-    super do |resource|
+    super do |_resource|
       if Crop.pending_approval.present? && current_member.role?(:crop_wrangler)
         flash[:alert] = "There are crops waiting to be wrangled."
       end
diff --git a/config/unicorn.rb b/config/unicorn.rb
index e4153a81f..6a63af23f 100644
--- a/config/unicorn.rb
+++ b/config/unicorn.rb
@@ -3,7 +3,7 @@ worker_processes 3
 timeout 30
 preload_app true
 
-before_fork do |server, worker|
+before_fork do |_server, _worker|
   Signal.trap 'TERM' do
     puts 'Unicorn master intercepting TERM and sending myself QUIT instead'
     Process.kill 'QUIT', Process.pid
@@ -13,7 +13,7 @@ before_fork do |server, worker|
     ActiveRecord::Base.connection.disconnect!
 end
 
-after_fork do |server, worker|
+after_fork do |_server, _worker|
   Signal.trap 'TERM' do
     puts 'Unicorn worker intercepting TERM and doing nothing. Wait for master to sent QUIT'
   end

From 2060d9ab9eacec14a22875d0cc80cea95b73a48e Mon Sep 17 00:00:00 2001
From: deppbot <deppbot@jollygoodcode.com>
Date: Wed, 15 Nov 2017 04:10:21 +0800
Subject: [PATCH 06/12] Bundle Update on 2017-11-15

---
 Gemfile.lock | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/Gemfile.lock b/Gemfile.lock
index 5f8bcf612..b973bae24 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -84,8 +84,8 @@ GEM
       activesupport (>= 3.0.0)
       uniform_notifier (~> 1.10.0)
     byebug (9.1.0)
-    cancancan (2.1.0)
-    capybara (2.15.4)
+    cancancan (2.1.1)
+    capybara (2.16.0)
       addressable
       mini_mime (>= 0.1.3)
       nokogiri (>= 1.3.3)
@@ -139,7 +139,7 @@ GEM
       term-ansicolor (~> 1.3)
       thor (~> 0.19.1)
       tins (~> 1.6)
-    crass (1.0.2)
+    crass (1.0.3)
     csv_shaper (1.3.0)
       activesupport (>= 3.0.0)
     d3-rails (3.5.17)
@@ -323,7 +323,7 @@ GEM
     multi_xml (0.6.0)
     multipart-post (2.0.0)
     nenv (0.3.0)
-    newrelic_rpm (4.5.0.337)
+    newrelic_rpm (4.6.0.338)
     nokogiri (1.8.1)
       mini_portile2 (~> 2.3.0)
     notiffany (0.1.1)
@@ -360,8 +360,8 @@ GEM
       cocaine (~> 0.5.5)
       mime-types
       mimemagic (~> 0.3.0)
-    parser (2.4.0.0)
-      ast (~> 2.2)
+    parser (2.4.0.2)
+      ast (~> 2.3)
     pg (0.21.0)
     phantomjs (2.1.1.0)
     platform-api (2.1.0)
@@ -374,7 +374,7 @@ GEM
       cliver (~> 0.3.1)
       websocket-driver (>= 0.2.0)
     powerpack (0.1.1)
-    pry (0.11.2)
+    pry (0.11.3)
       coderay (~> 1.1.0)
       method_source (~> 0.9.0)
     public_suffix (3.0.1)
@@ -469,7 +469,7 @@ GEM
     sass-listen (4.0.0)
       rb-fsevent (~> 0.9, >= 0.9.4)
       rb-inotify (~> 0.9, >= 0.9.7)
-    sass-rails (5.0.6)
+    sass-rails (5.0.7)
       railties (>= 4.0.0, < 6)
       sass (~> 3.1)
       sprockets (>= 2.8, < 4.0)

From d01a5d7976dd0822a2f0cd8fffcead75486e6cb5 Mon Sep 17 00:00:00 2001
From: Brenda Wallace <brenda@wallace.net.nz>
Date: Mon, 13 Nov 2017 22:08:38 +1300
Subject: [PATCH 07/12] Don't show lifespan of perennials

---
 app/views/crops/_predictions.html.haml | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/app/views/crops/_predictions.html.haml b/app/views/crops/_predictions.html.haml
index 86aaf8f43..7d947db1c 100644
--- a/app/views/crops/_predictions.html.haml
+++ b/app/views/crops/_predictions.html.haml
@@ -10,19 +10,19 @@
         an annual crop
       (living and reproducing in a single year or less)
 
-- unless crop.median_lifespan.nil?
+- if crop.annual? && crop.median_lifespan.present?
   %p
     Median lifespan of #{crop.name} plants is
     %b= crop.median_lifespan
     days
 
-- unless crop.median_days_to_first_harvest.nil?
+- if crop.median_days_to_first_harvest.present?
   %p
     First harvest expected
     %b= crop.median_days_to_first_harvest
     days after planting
 
-- if crop.perennial == false && crop.median_days_to_last_harvest.present?
+- if crop.annual? && crop.median_days_to_last_harvest.present?
   %p
     Last harvest expected
     %b= crop.median_days_to_last_harvest

From 8884056ff30db99e41b04f10f810238141d179cc Mon Sep 17 00:00:00 2001
From: Brenda Wallace <brenda@wallace.net.nz>
Date: Mon, 13 Nov 2017 23:13:55 +1300
Subject: [PATCH 08/12] Run existing crop specs with annual crop

---
 app/models/crop.rb                           |  8 ++-
 spec/features/crops/crop_detail_page_spec.rb | 60 +++++++++++++-------
 2 files changed, 45 insertions(+), 23 deletions(-)

diff --git a/app/models/crop.rb b/app/models/crop.rb
index 44251ec36..0b6a63bc1 100644
--- a/app/models/crop.rb
+++ b/app/models/crop.rb
@@ -170,7 +170,7 @@ class Crop < ActiveRecord::Base
   end
 
   def annual?
-    perennial != true
+    !perennial
   end
 
   def interesting?
@@ -215,6 +215,12 @@ class Crop < ActiveRecord::Base
     where(["lower(crops.name) = :value", { value: name.downcase }])
   end
 
+  def update_medians
+    plantings.each(&:update_harvest_days)
+    update_lifespan_medians
+    update_harvest_medians
+  end
+
   def update_lifespan_medians
     # Median lifespan of plantings
     update(median_lifespan: Planting.where(crop: self).median(:lifespan))
diff --git a/spec/features/crops/crop_detail_page_spec.rb b/spec/features/crops/crop_detail_page_spec.rb
index 45d855bc2..e33e2fe90 100644
--- a/spec/features/crops/crop_detail_page_spec.rb
+++ b/spec/features/crops/crop_detail_page_spec.rb
@@ -169,41 +169,54 @@ feature "crop detail page", js: true do
     end
   end
 
-  shared_examples "lots of harvests" do
-    def planting
-      FactoryBot.create :planting, crop: crop, planted_at: 100.days.ago, finished_at: 1.day.ago
+  shared_examples "predicts harvest" do
+    describe 'with harvest history data' do
+      before do
+        other_planting = FactoryBot.create :planting, crop: planting.crop, planted_at: 200.days.ago
+        # 50 days to harvest
+        FactoryBot.create(:harvest, planting: other_planting, harvested_at: 150.days.ago, crop: planting.crop)
+        # 20 days to harvest
+        FactoryBot.create(:harvest, planting: other_planting, harvested_at: 180.days.ago, crop: planting.crop)
+        # 10 days to harvest
+        FactoryBot.create(:harvest, planting: other_planting, harvested_at: 190.days.ago, crop: planting.crop)
+      end
+      # it "predicts harvest" do
+      #   is_expected.to have_text("First harvest expected 20 days after planting")
+      # end
     end
-    before do
-      # 50 days to harvest
-      FactoryBot.create(:harvest, harvested_at: 50.days.ago, crop: crop, planting: planting)
-      # 20 days to harvest
-      FactoryBot.create(:harvest, harvested_at: 80.days.ago, crop: crop, planting: planting)
-      # 10 days to harvest
-      FactoryBot.create(:harvest, harvested_at: 90.days.ago, crop: crop, planting: planting)
-      planting.crop.plantings.each(&:update_harvest_days)
-      planting.crop.update_lifespan_medians
-      planting.crop.update_harvest_medians
-    end
-    it { is_expected.to have_text("First harvest expected 20 days after planting") }
-    it { is_expected.to have_text "Median lifespan of #{crop.name} plants is 99 days" }
   end
 
   subject do
+    # Update the medians after all the
+    # data has been loaded
+    crop.reload
+    crop.update_medians
+
     visit crop_path(crop)
     page
   end
 
   context 'predictions' do
+    let!(:planting) do
+      FactoryBot.create(:planting, crop: crop,
+                                   planted_at: 100.days.ago,
+                                   finished_at: 1.day.ago)
+    end
     context 'crop is an annual' do
-      let(:crop) { FactoryBot.create :annual_crop }
+      let(:crop) { FactoryBot.create(:annual_crop) }
 
       describe 'with no harvests' do
       end
 
       describe 'with harvests' do
-        include_examples "lots of harvests"
+        include_examples "predicts harvest"
       end
-      it do
+
+      it "predicts lifespan" do
+        is_expected.to have_text "Median lifespan of #{crop.name} plants is 99 days"
+      end
+
+      it "describes annual crops" do
         is_expected.to have_text(
           "#{crop.name} is an annual crop (living and reproducing in a single year or less)"
         )
@@ -217,9 +230,12 @@ feature "crop detail page", js: true do
       end
 
       describe 'with harvests' do
-        include_examples "lots of harvests"
+        include_examples "predicts harvest"
+      end
+
+      it "describes perennial crops" do
+        is_expected.to have_text("#{crop.name} is a perennial crop (living more than two years)")
       end
-      it { is_expected.to have_text("#{crop.name} is a perennial crop (living more than two years)") }
     end
 
     context 'crop perennial value is null' do
@@ -229,7 +245,7 @@ feature "crop detail page", js: true do
       end
 
       describe 'with harvests' do
-        include_examples "lots of harvests"
+        include_examples "predicts harvest"
       end
     end
   end

From 86ea8eab398b7095f5527c8cc9fea72e20a02b60 Mon Sep 17 00:00:00 2001
From: Brenda Wallace <brenda@wallace.net.nz>
Date: Wed, 15 Nov 2017 21:44:32 +1300
Subject: [PATCH 09/12] Reducing crop class size

---
 app/models/crop.rb | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/app/models/crop.rb b/app/models/crop.rb
index 0b6a63bc1..4fa4ef487 100644
--- a/app/models/crop.rb
+++ b/app/models/crop.rb
@@ -117,7 +117,6 @@ class Crop < ActiveRecord::Base
   def update_index(_name_obj)
     __elasticsearch__.index_document if ENV["GROWSTUFF_ELASTICSEARCH"] == "true"
   end
-
   # End Elasticsearch section
 
   def to_s
@@ -128,7 +127,6 @@ class Crop < ActiveRecord::Base
     scientific_names.first.name unless scientific_names.empty?
   end
 
-  # crop.default_photo
   # currently returns the first available photo, but exists so that
   # later we can choose a default photo based on different criteria,
   # eg. popularity
@@ -140,7 +138,6 @@ class Crop < ActiveRecord::Base
     harvest_with_photo.photos.first if harvest_with_photo
   end
 
-  # crop.sunniness
   # returns hash indicating whether this crop is grown in
   # sun/semi-shade/shade
   # key: sunniness (eg. 'sun')
@@ -149,7 +146,6 @@ class Crop < ActiveRecord::Base
     count_uses_of_property 'sunniness'
   end
 
-  # crop.planted_from
   # returns a hash of propagation methods (seed, seedling, etc),
   # key: propagation method (eg. 'seed')
   # value: count of how many times it's been used by plantings
@@ -157,7 +153,6 @@ class Crop < ActiveRecord::Base
     count_uses_of_property 'planted_from'
   end
 
-  # crop.popular_plant_parts
   # returns a hash of most harvested plant parts (fruit, seed, etc)
   # key: plant part (eg. 'fruit')
   # value: count of how many times it's been used by harvests
@@ -206,15 +201,6 @@ class Crop < ActiveRecord::Base
     reason_for_rejection
   end
 
-  # # Crop.search(string)
-  def self.search(query)
-    CropSearchService.search(query)
-  end
-
-  def self.case_insensitive_name(name)
-    where(["lower(crops.name) = :value", { value: name.downcase }])
-  end
-
   def update_medians
     plantings.each(&:update_harvest_days)
     update_lifespan_medians
@@ -231,6 +217,14 @@ class Crop < ActiveRecord::Base
     update(median_days_to_last_harvest: Planting.where(crop: self).median(:days_to_last_harvest))
   end
 
+  def self.search(query)
+    CropSearchService.search(query)
+  end
+
+  def self.case_insensitive_name(name)
+    where(["lower(crops.name) = :value", { value: name.downcase }])
+  end
+
   private
 
   def count_uses_of_property(col_name)

From 67dec795a0c65f2e8e1fbf522815b2ae52a347d9 Mon Sep 17 00:00:00 2001
From: Brenda Wallace <brenda@wallace.net.nz>
Date: Wed, 15 Nov 2017 22:43:47 +1300
Subject: [PATCH 10/12] Reinstated test of first harvest prediction display

---
 app/views/crops/_predictions.html.haml       |  6 +++---
 spec/features/crops/crop_detail_page_spec.rb | 16 +++++++++-------
 2 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/app/views/crops/_predictions.html.haml b/app/views/crops/_predictions.html.haml
index 7d947db1c..dbde9c783 100644
--- a/app/views/crops/_predictions.html.haml
+++ b/app/views/crops/_predictions.html.haml
@@ -13,17 +13,17 @@
 - if crop.annual? && crop.median_lifespan.present?
   %p
     Median lifespan of #{crop.name} plants is
-    %b= crop.median_lifespan
+    %strong= crop.median_lifespan
     days
 
 - if crop.median_days_to_first_harvest.present?
   %p
     First harvest expected
-    %b= crop.median_days_to_first_harvest
+    %strong= crop.median_days_to_first_harvest
     days after planting
 
 - if crop.annual? && crop.median_days_to_last_harvest.present?
   %p
     Last harvest expected
-    %b= crop.median_days_to_last_harvest
+    %strong= crop.median_days_to_last_harvest
     days after planting
diff --git a/spec/features/crops/crop_detail_page_spec.rb b/spec/features/crops/crop_detail_page_spec.rb
index e33e2fe90..05ebdb6e7 100644
--- a/spec/features/crops/crop_detail_page_spec.rb
+++ b/spec/features/crops/crop_detail_page_spec.rb
@@ -172,17 +172,19 @@ feature "crop detail page", js: true do
   shared_examples "predicts harvest" do
     describe 'with harvest history data' do
       before do
-        other_planting = FactoryBot.create :planting, crop: planting.crop, planted_at: 200.days.ago
         # 50 days to harvest
-        FactoryBot.create(:harvest, planting: other_planting, harvested_at: 150.days.ago, crop: planting.crop)
+        FactoryBot.create(:harvest, harvested_at: 150.days.ago, crop: planting.crop,
+                                    planting: FactoryBot.create(:planting, planted_at: 200.days.ago, crop: crop))
         # 20 days to harvest
-        FactoryBot.create(:harvest, planting: other_planting, harvested_at: 180.days.ago, crop: planting.crop)
+        FactoryBot.create(:harvest, harvested_at: 180.days.ago, crop: planting.crop,
+                                    planting: FactoryBot.create(:planting, planted_at: 200.days.ago, crop: crop))
         # 10 days to harvest
-        FactoryBot.create(:harvest, planting: other_planting, harvested_at: 190.days.ago, crop: planting.crop)
+        FactoryBot.create(:harvest, harvested_at: 190.days.ago, crop: planting.crop,
+                                    planting: FactoryBot.create(:planting, planted_at: 200.days.ago, crop: crop))
+      end
+      it "predicts harvest" do
+        is_expected.to have_text("First harvest expected 20 days after planting")
       end
-      # it "predicts harvest" do
-      #   is_expected.to have_text("First harvest expected 20 days after planting")
-      # end
     end
   end
 

From 28fdb6e1ed125781caa937bb48431951f8eaccd9 Mon Sep 17 00:00:00 2001
From: Brenda Wallace <brenda@wallace.net.nz>
Date: Thu, 16 Nov 2017 12:29:39 +1300
Subject: [PATCH 11/12] Reducing class line count

---
 app/models/planting.rb | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/app/models/planting.rb b/app/models/planting.rb
index 4fe2ccc0a..4dd0b57c4 100644
--- a/app/models/planting.rb
+++ b/app/models/planting.rb
@@ -6,17 +6,9 @@ class Planting < ActiveRecord::Base
   # Constants
   SUNNINESS_VALUES = %w(sun semi-shade shade)
   PLANTED_FROM_VALUES = [
-    'seed',
-    'seedling',
-    'cutting',
-    'root division',
-    'runner',
-    'bulb',
-    'root/tuber',
-    'bare root plant',
-    'advanced plant',
-    'graft',
-    'layering'
+    'seed', 'seedling', 'cutting', 'root division', 'runner',
+    'bulb', 'root/tuber', 'bare root plant', 'advanced plant',
+    'graft', 'layering'
   ]
 
   ##

From c1836cc78e759663647339f6ca48b27056882f6d Mon Sep 17 00:00:00 2001
From: Brenda Wallace <brenda@wallace.net.nz>
Date: Fri, 17 Nov 2017 21:23:28 +1300
Subject: [PATCH 12/12] Removed un-used method, to reduce class size

---
 app/models/crop.rb | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/app/models/crop.rb b/app/models/crop.rb
index 4fa4ef487..96ba2bc01 100644
--- a/app/models/crop.rb
+++ b/app/models/crop.rb
@@ -102,16 +102,6 @@ class Crop < ActiveRecord::Base
     Photo.joins(:harvests).where("harvests.crop_id": id)
   end
 
-  def as_indexed_json(_options = {})
-    as_json(
-      only: [:id, :name, :approval_status],
-      include: {
-        scientific_names: { only: :name },
-        alternate_names: { only: :name }
-      }
-    )
-  end
-
   # update the Elasticsearch index (only if we're using it in this
   # environment)
   def update_index(_name_obj)