From 494790bcd463bb91f35191699774c6b7e8cdb6d8 Mon Sep 17 00:00:00 2001
From: Mackenzie Morgan <macoafi@gmail.com>
Date: Tue, 11 Oct 2016 15:28:08 -0400
Subject: [PATCH 1/9] moving where clauses into scopes to keep that contained
 in models

---
 app/controllers/notifications_controller.rb | 2 +-
 app/controllers/orders_controller.rb        | 2 +-
 app/models/notification.rb                  | 4 ++++
 app/models/order.rb                         | 4 ++++
 4 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/app/controllers/notifications_controller.rb b/app/controllers/notifications_controller.rb
index 57b5836ba..cf50316a6 100644
--- a/app/controllers/notifications_controller.rb
+++ b/app/controllers/notifications_controller.rb
@@ -5,7 +5,7 @@ class NotificationsController < ApplicationController
 
   # GET /notifications
   def index
-    @notifications = Notification.where(recipient_id: current_member).page(params[:page])
+    @notifications = Notification.find_by_recipient(current_member).page(params[:page])
 
     respond_to do |format|
       format.html # index.html.erb
diff --git a/app/controllers/orders_controller.rb b/app/controllers/orders_controller.rb
index 82bb337f9..5de8e46e4 100644
--- a/app/controllers/orders_controller.rb
+++ b/app/controllers/orders_controller.rb
@@ -4,7 +4,7 @@ class OrdersController < ApplicationController
 
   # GET /orders
   def index
-    @orders = Order.where(member_id: current_member.id)
+    @orders = Order.by_member_id(current_member.id)
 
     respond_to do |format|
       format.html # index.html.erb
diff --git a/app/models/notification.rb b/app/models/notification.rb
index 567867b00..9f9a61257 100644
--- a/app/models/notification.rb
+++ b/app/models/notification.rb
@@ -11,6 +11,10 @@ class Notification < ActiveRecord::Base
   before_create :replace_blank_subject
   after_create :send_email
 
+  def self.find_by_recipient(recipient)
+    where(recipient_id: recipient)
+  end
+
   def self.unread_count
     self.unread.size
   end
diff --git a/app/models/order.rb b/app/models/order.rb
index f3a4eaafb..acf3a5878 100644
--- a/app/models/order.rb
+++ b/app/models/order.rb
@@ -12,6 +12,10 @@ class Order < ActiveRecord::Base
 
   before_save :standardize_referral_code
 
+  def by_member_id(member_id)
+    where(member_id: member_id)
+  end
+
   # total price of an order
   def total
     sum = 0

From cee3d192e092e1807d8c0e27e773a8f1e26de437 Mon Sep 17 00:00:00 2001
From: Mackenzie Morgan <macoafi@gmail.com>
Date: Tue, 11 Oct 2016 16:34:25 -0400
Subject: [PATCH 2/9] test the new order.by_member_id scope

---
 app/models/order.rb       |  2 +-
 spec/models/order_spec.rb | 13 +++++++++++++
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/app/models/order.rb b/app/models/order.rb
index acf3a5878..00b0b712d 100644
--- a/app/models/order.rb
+++ b/app/models/order.rb
@@ -12,7 +12,7 @@ class Order < ActiveRecord::Base
 
   before_save :standardize_referral_code
 
-  def by_member_id(member_id)
+  def self.by_member_id(member_id)
     where(member_id: member_id)
   end
 
diff --git a/spec/models/order_spec.rb b/spec/models/order_spec.rb
index 9291e1a3f..6a74baf11 100644
--- a/spec/models/order_spec.rb
+++ b/spec/models/order_spec.rb
@@ -8,6 +8,19 @@ describe Order do
       order_id: @order.id, product_id: @product.id)
   end
 
+  describe '#by_member_id' do
+    before do
+      @member1 = FactoryGirl.create(:member)
+      @member2 = FactoryGirl.create(:member)
+      @order1 = Order.create!(member_id: @member1.id)
+      @order2 = Order.create!(member_id: @member2.id)
+    end
+
+    it "only returns orders belonging to member" do
+      Order.by_member_id(@member1.id).should eq [@order1]
+    end
+  end
+
   it 'has order_items' do
     @order.order_items.first.should eq @order_item
   end

From 9dc02cd3d8e6fd250a87661a7c426f4f0e0cc42c Mon Sep 17 00:00:00 2001
From: Mackenzie Morgan <macoafi@gmail.com>
Date: Tue, 29 Nov 2016 12:10:48 -0500
Subject: [PATCH 3/9] make changes from code review

---
 app/controllers/crops_controller.rb         | 11 ++++++-----
 app/controllers/notifications_controller.rb |  2 +-
 app/controllers/orders_controller.rb        |  2 +-
 app/models/notification.rb                  |  4 +---
 app/models/order.rb                         |  4 +---
 5 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/app/controllers/crops_controller.rb b/app/controllers/crops_controller.rb
index dac763665..c77522b8f 100644
--- a/app/controllers/crops_controller.rb
+++ b/app/controllers/crops_controller.rb
@@ -10,14 +10,11 @@ class CropsController < ApplicationController
   def index
     @sort = params[:sort]
     if @sort == 'alpha'
-      # alphabetical order
       @crops = Crop.includes(:scientific_names, { plantings: :photos })
-      @paginated_crops = @crops.approved.paginate(page: params[:page])
     else
-      # default to sorting by popularity
-      @crops = Crop.popular.includes(:scientific_names, { plantings: :photos })
-      @paginated_crops = @crops.approved.paginate(page: params[:page])
+      @crops = popular_crops
     end
+    @paginated_crops = @crops.approved.paginate(page: params[:page])
 
     respond_to do |format|
       format.html
@@ -212,6 +209,10 @@ class CropsController < ApplicationController
 
   private
 
+  def popular_crops
+    Crop.popular.includes(:scientific_names, { plantings: :photos })
+  end
+
   def crop_params
     params.require(:crop).permit(:en_wikipedia_url,
       :name,
diff --git a/app/controllers/notifications_controller.rb b/app/controllers/notifications_controller.rb
index 0f8d38d19..5794a45cc 100644
--- a/app/controllers/notifications_controller.rb
+++ b/app/controllers/notifications_controller.rb
@@ -5,7 +5,7 @@ class NotificationsController < ApplicationController
 
   # GET /notifications
   def index
-    @notifications = Notification.find_by_recipient(current_member).page(params[:page])
+    @notifications = Notification.by_recipient(current_member).page(params[:page])
 
     respond_to do |format|
       format.html # index.html.erb
diff --git a/app/controllers/orders_controller.rb b/app/controllers/orders_controller.rb
index b0d744268..3e0e97806 100644
--- a/app/controllers/orders_controller.rb
+++ b/app/controllers/orders_controller.rb
@@ -4,7 +4,7 @@ class OrdersController < ApplicationController
 
   # GET /orders
   def index
-    @orders = Order.by_member_id(current_member.id)
+    @orders = Order.by_member(current_member)
 
     respond_to do |format|
       format.html # index.html.erb
diff --git a/app/models/notification.rb b/app/models/notification.rb
index 4be21e668..329ff17e5 100644
--- a/app/models/notification.rb
+++ b/app/models/notification.rb
@@ -7,13 +7,11 @@ class Notification < ActiveRecord::Base
 
   default_scope { order('created_at DESC') }
   scope :unread, -> { where(read: false) }
+  scope :by_recipient, ->(recipient) {where(recipient_id: recipient)}
 
   before_create :replace_blank_subject
   after_create :send_email
 
-  def self.find_by_recipient(recipient)
-    where(recipient_id: recipient)
-  end
 
   def self.unread_count
     self.unread.size
diff --git a/app/models/order.rb b/app/models/order.rb
index df074c5a9..85a3c526d 100644
--- a/app/models/order.rb
+++ b/app/models/order.rb
@@ -12,9 +12,7 @@ class Order < ActiveRecord::Base
 
   before_save :standardize_referral_code
 
-  def self.by_member_id(member_id)
-    where(member_id: member_id)
-  end
+  scope :by_member, ->(member) { where(member: member) }
 
   # total price of an order
   def total

From 99c8db72d6fee271e41213b632f4d9e6dff769ff Mon Sep 17 00:00:00 2001
From: Mackenzie Morgan <macoafi@gmail.com>
Date: Tue, 29 Nov 2016 12:14:16 -0500
Subject: [PATCH 4/9] syntax

---
 app/controllers/crops_controller.rb | 2 ++
 app/models/notification.rb          | 3 +--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/app/controllers/crops_controller.rb b/app/controllers/crops_controller.rb
index 27fd6a23b..d3a7c1b9b 100644
--- a/app/controllers/crops_controller.rb
+++ b/app/controllers/crops_controller.rb
@@ -197,6 +197,8 @@ class CropsController < ApplicationController
 
   def popular_crops
     Crop.popular.includes(:scientific_names, { plantings: :photos })
+  end
+
   def recreate_names(param_name, name_type)
     return unless params[param_name].present?
     destroy_names(name_type)
diff --git a/app/models/notification.rb b/app/models/notification.rb
index 329ff17e5..f19de9b34 100644
--- a/app/models/notification.rb
+++ b/app/models/notification.rb
@@ -7,12 +7,11 @@ class Notification < ActiveRecord::Base
 
   default_scope { order('created_at DESC') }
   scope :unread, -> { where(read: false) }
-  scope :by_recipient, ->(recipient) {where(recipient_id: recipient)}
+  scope :by_recipient, ->(recipient) { where(recipient_id: recipient) }
 
   before_create :replace_blank_subject
   after_create :send_email
 
-
   def self.unread_count
     self.unread.size
   end

From 4857fd8d2e86b80df59c957eafb631ec78f2032f Mon Sep 17 00:00:00 2001
From: Mackenzie Morgan <macoafi@gmail.com>
Date: Tue, 29 Nov 2016 12:20:17 -0500
Subject: [PATCH 5/9] return from conditional

---
 app/controllers/crops_controller.rb | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/app/controllers/crops_controller.rb b/app/controllers/crops_controller.rb
index d3a7c1b9b..f357e24fb 100644
--- a/app/controllers/crops_controller.rb
+++ b/app/controllers/crops_controller.rb
@@ -9,11 +9,11 @@ class CropsController < ApplicationController
   # GET /crops.json
   def index
     @sort = params[:sort]
-    if @sort == 'alpha'
-      @crops = Crop.includes(:scientific_names, { plantings: :photos })
-    else
-      @crops = popular_crops
-    end
+    @crops = if @sort == 'alpha'
+               Crop.includes(:scientific_names, { plantings: :photos })
+             else
+               popular_crops
+             end
     @paginated_crops = @crops.approved.paginate(page: params[:page])
 
     respond_to do |format|

From 72877aebaf30adfddc801441dac457f5117c3ccf Mon Sep 17 00:00:00 2001
From: Mackenzie Morgan <macoafi@gmail.com>
Date: Tue, 29 Nov 2016 13:41:05 -0500
Subject: [PATCH 6/9] update test for renamed scope

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

diff --git a/spec/models/order_spec.rb b/spec/models/order_spec.rb
index 30e6cf261..1f8bf88bf 100644
--- a/spec/models/order_spec.rb
+++ b/spec/models/order_spec.rb
@@ -17,7 +17,7 @@ describe Order do
     end
 
     it "only returns orders belonging to member" do
-      Order.by_member_id(@member1.id).should eq [@order1]
+      Order.by_member(@member1).should eq [@order1]
     end
   end
 

From 77c64d59258b7692231a7bf21b63570be9bf5359 Mon Sep 17 00:00:00 2001
From: Miles Gould <mgould@redhat.com>
Date: Tue, 29 Nov 2016 13:51:04 +0000
Subject: [PATCH 7/9] Test PlantingHelper.display_days_before_maturity

This ensures that #1079 is fixed.
---
 spec/helpers/plantings_helper_spec.rb | 63 +++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/spec/helpers/plantings_helper_spec.rb b/spec/helpers/plantings_helper_spec.rb
index 9485af81d..48a1ea5ad 100644
--- a/spec/helpers/plantings_helper_spec.rb
+++ b/spec/helpers/plantings_helper_spec.rb
@@ -1,6 +1,69 @@
 require 'rails_helper'
 
 describe PlantingsHelper do
+  describe "display_days_before_maturity" do
+    it "handles nil planted_at, nil finished_at, non-nil days_until_maturity" do
+      planting = FactoryGirl.build(:planting,
+        quantity: 5,
+        planted_at: nil,
+        finished_at: nil,
+        days_before_maturity: 17
+      )
+      result = helper.display_days_before_maturity(planting)
+      expect(result).to eq "unknown"
+    end
+
+    it "handles non-nil planted_at and d_b_m, nil finished_at" do
+      planting = FactoryGirl.build(:planting,
+        quantity: 5,
+        planted_at: Date.current,
+        finished_at: nil,
+        days_before_maturity: 17
+      )
+      result = helper.display_days_before_maturity(planting)
+      expect(result).to eq 16
+    end
+
+    it "handles completed plantings" do
+      planting = FactoryGirl.build(:planting, finished: true)
+      result = helper.display_days_before_maturity(planting)
+      expect(result).to eq 0
+    end
+
+    it "handles plantings that should have finished" do
+      planting = FactoryGirl.build(:planting,
+        quantity: 5,
+        planted_at: Date.new(0, 1, 1),
+        finished_at: nil,
+        days_before_maturity: 17
+      )
+      result = helper.display_days_before_maturity(planting)
+      expect(result).to eq 0
+    end
+
+    it "handles nil d_b_m and nil finished_at" do
+      planting = FactoryGirl.build(:planting,
+        quantity: 5,
+        planted_at: Date.new(2012, 5, 12),
+        finished_at: nil,
+        days_before_maturity: nil
+      )
+      result = helper.display_days_before_maturity(planting)
+      expect(result).to eq "unknown"
+    end
+
+    it "handles finished_at dates in the future" do
+      planting = FactoryGirl.build(:planting,
+        quantity: 5,
+        planted_at: Date.current,
+        finished_at: Date.current + 5,
+        days_before_maturity: nil
+      )
+      result = helper.display_days_before_maturity(planting)
+      expect(result).to eq 4
+    end
+  end
+
   describe "display_planting" do
     let!(:member) { FactoryGirl.build(:member, login_name: 'crop_lady') }
 

From b0b864a5d41278e6695c003a5114425f7ca25d2d Mon Sep 17 00:00:00 2001
From: Miles Gould <mgould@redhat.com>
Date: Tue, 29 Nov 2016 13:53:26 +0000
Subject: [PATCH 8/9] Fix off-by-one bug in display_days_before_maturity

We were counting from DateTime.now rather than Date.current, causing us to
under-count by a day.
---
 app/helpers/plantings_helper.rb       | 4 ++--
 spec/helpers/plantings_helper_spec.rb | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/app/helpers/plantings_helper.rb b/app/helpers/plantings_helper.rb
index a9cfebf61..964b8fb76 100644
--- a/app/helpers/plantings_helper.rb
+++ b/app/helpers/plantings_helper.rb
@@ -3,11 +3,11 @@ module PlantingsHelper
     if planting.finished?
       0
     elsif !planting.finished_at.nil?
-      ((p = planting.finished_at - DateTime.now).to_i) <= 0 ? 0 : p.to_i
+      ((p = planting.finished_at - Date.current).to_i) <= 0 ? 0 : p.to_i
     elsif planting.planted_at.nil? || planting.days_before_maturity.nil?
       "unknown"
     else
-      ((p = (planting.planted_at + planting.days_before_maturity) - DateTime.now).to_i <= 0) ? 0 : p.to_i
+      ((p = (planting.planted_at + planting.days_before_maturity) - Date.current).to_i <= 0) ? 0 : p.to_i
     end
   end
 
diff --git a/spec/helpers/plantings_helper_spec.rb b/spec/helpers/plantings_helper_spec.rb
index 48a1ea5ad..2370bf53a 100644
--- a/spec/helpers/plantings_helper_spec.rb
+++ b/spec/helpers/plantings_helper_spec.rb
@@ -21,7 +21,7 @@ describe PlantingsHelper do
         days_before_maturity: 17
       )
       result = helper.display_days_before_maturity(planting)
-      expect(result).to eq 16
+      expect(result).to eq 17
     end
 
     it "handles completed plantings" do
@@ -60,7 +60,7 @@ describe PlantingsHelper do
         days_before_maturity: nil
       )
       result = helper.display_days_before_maturity(planting)
-      expect(result).to eq 4
+      expect(result).to eq 5
     end
   end
 

From 9400225f65b283a6bf1ff4bca5dc1a9dcb87e8a3 Mon Sep 17 00:00:00 2001
From: Miles Gould <mgould@redhat.com>
Date: Wed, 30 Nov 2016 09:56:31 +0000
Subject: [PATCH 9/9] Return a string from display_days_before_maturity

Sometimes we were returning a string, and sometimes we were returning an
integer. We're only ever displaying the result, and this seems a little
more consistent.
---
 app/helpers/plantings_helper.rb       |  6 +++---
 spec/helpers/plantings_helper_spec.rb | 10 +++++-----
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/app/helpers/plantings_helper.rb b/app/helpers/plantings_helper.rb
index 964b8fb76..f4d8d45be 100644
--- a/app/helpers/plantings_helper.rb
+++ b/app/helpers/plantings_helper.rb
@@ -1,13 +1,13 @@
 module PlantingsHelper
   def display_days_before_maturity(planting)
     if planting.finished?
-      0
+      "0"
     elsif !planting.finished_at.nil?
-      ((p = planting.finished_at - Date.current).to_i) <= 0 ? 0 : p.to_i
+      ((p = planting.finished_at - Date.current).to_i) <= 0 ? "0" : p.to_i.to_s
     elsif planting.planted_at.nil? || planting.days_before_maturity.nil?
       "unknown"
     else
-      ((p = (planting.planted_at + planting.days_before_maturity) - Date.current).to_i <= 0) ? 0 : p.to_i
+      ((p = (planting.planted_at + planting.days_before_maturity) - Date.current).to_i <= 0) ? "0" : p.to_i.to_s
     end
   end
 
diff --git a/spec/helpers/plantings_helper_spec.rb b/spec/helpers/plantings_helper_spec.rb
index 2370bf53a..e776501b4 100644
--- a/spec/helpers/plantings_helper_spec.rb
+++ b/spec/helpers/plantings_helper_spec.rb
@@ -21,13 +21,13 @@ describe PlantingsHelper do
         days_before_maturity: 17
       )
       result = helper.display_days_before_maturity(planting)
-      expect(result).to eq 17
+      expect(result).to eq "17"
     end
 
     it "handles completed plantings" do
       planting = FactoryGirl.build(:planting, finished: true)
       result = helper.display_days_before_maturity(planting)
-      expect(result).to eq 0
+      expect(result).to eq "0"
     end
 
     it "handles plantings that should have finished" do
@@ -35,10 +35,10 @@ describe PlantingsHelper do
         quantity: 5,
         planted_at: Date.new(0, 1, 1),
         finished_at: nil,
-        days_before_maturity: 17
+        days_before_maturity: "17"
       )
       result = helper.display_days_before_maturity(planting)
-      expect(result).to eq 0
+      expect(result).to eq "0"
     end
 
     it "handles nil d_b_m and nil finished_at" do
@@ -60,7 +60,7 @@ describe PlantingsHelper do
         days_before_maturity: nil
       )
       result = helper.display_days_before_maturity(planting)
-      expect(result).to eq 5
+      expect(result).to eq "5"
     end
   end