From 7719d1f896e5c9d1db56c1a55b386829984dbd75 Mon Sep 17 00:00:00 2001
From: Brenda Wallace <brenda@wallace.net.nz>
Date: Sun, 26 Nov 2017 13:35:10 +1300
Subject: [PATCH 01/29] Remote default ordering scopes, and clean up models

---
 app/models/comment.rb      |  1 -
 app/models/crop.rb         |  1 -
 app/models/garden.rb       |  2 +-
 app/models/harvest.rb      |  2 +-
 app/models/member.rb       | 53 +++++++++-------------
 app/models/notification.rb |  1 -
 app/models/order.rb        | 11 +++--
 app/models/photo.rb        |  2 +-
 app/models/planting.rb     |  2 +-
 app/models/post.rb         | 67 +++++++++++++++------------
 app/models/product.rb      | 13 +++---
 app/models/seed.rb         | 93 +++++++++++++-------------------------
 12 files changed, 107 insertions(+), 141 deletions(-)

diff --git a/app/models/comment.rb b/app/models/comment.rb
index f91c9cb42..7098e0b46 100644
--- a/app/models/comment.rb
+++ b/app/models/comment.rb
@@ -2,7 +2,6 @@ class Comment < ActiveRecord::Base
   belongs_to :author, class_name: 'Member'
   belongs_to :post
 
-  default_scope { order("created_at DESC") }
   scope :post_order, -> { reorder("created_at ASC") } # for display on post page
 
   after_create do
diff --git a/app/models/crop.rb b/app/models/crop.rb
index 96ba2bc01..ceb275b8b 100644
--- a/app/models/crop.rb
+++ b/app/models/crop.rb
@@ -24,7 +24,6 @@ class Crop < ActiveRecord::Base
 
   ##
   ## Scopes
-  default_scope { order("lower(crops.name) asc") }
   scope :recent, -> { approved.reorder("created_at desc") }
   scope :toplevel, -> { approved.where(parent_id: nil) }
   scope :popular, -> { approved.reorder("plantings_count desc, lower(name) asc") }
diff --git a/app/models/garden.rb b/app/models/garden.rb
index 571eca7ba..bde976de0 100644
--- a/app/models/garden.rb
+++ b/app/models/garden.rb
@@ -14,7 +14,7 @@ class Garden < ActiveRecord::Base
   after_validation :empty_unwanted_geocodes
   after_save :mark_inactive_garden_plantings_as_finished
 
-  default_scope { joins(:owner).order("lower(name) asc") }
+  default_scope { joins(:owner) } # Ensures owner exists
   scope :active, -> { where(active: true) }
   scope :inactive, -> { where(active: false) }
 
diff --git a/app/models/harvest.rb b/app/models/harvest.rb
index c2ad7eab1..1e4ceb2dc 100644
--- a/app/models/harvest.rb
+++ b/app/models/harvest.rb
@@ -39,7 +39,7 @@ class Harvest < ActiveRecord::Base
 
   ##
   ## Scopes
-  default_scope { joins(:owner) }
+  default_scope { joins(:owner) } # Ensures owner exists
 
   ##
   ## Validations
diff --git a/app/models/member.rb b/app/models/member.rb
index 5108cf711..f17e9b36e 100644
--- a/app/models/member.rb
+++ b/app/models/member.rb
@@ -6,48 +6,40 @@ class Member < ActiveRecord::Base
 
   friendly_id :login_name, use: %i(slugged finders)
 
+  #
+  # Relationshops
   has_many :posts, foreign_key: 'author_id'
   has_many :comments, foreign_key: 'author_id'
   has_many :forums, foreign_key: 'owner_id'
   has_many :gardens, foreign_key: 'owner_id'
   has_many :plantings, foreign_key: 'owner_id'
-
   has_many :seeds, foreign_key: 'owner_id'
   has_many :harvests, foreign_key: 'owner_id'
-
   has_and_belongs_to_many :roles # rubocop:disable Rails/HasAndBelongsToMany
-
   has_many :notifications, foreign_key: 'recipient_id'
   has_many :sent_notifications, foreign_key: 'sender_id'
-
   has_many :authentications
-
   has_many :orders
   has_one  :account
   has_one  :account_type, through: :account
-
   has_many :photos
-
   has_many :requested_crops, class_name: Crop, foreign_key: 'requester_id'
   has_many :likes, dependent: :destroy
+  has_many :follows, class_name: "Follow", foreign_key: "follower_id"
+  has_many :followed, through: :follows
+  has_many :inverse_follows, class_name: "Follow", foreign_key: "followed_id"
+  has_many :followers, through: :inverse_follows, source: :follower
 
-  default_scope { order("lower(login_name) asc") }
-
+  #
+  # Scopes
   scope :confirmed, -> { where.not(confirmed_at: nil) }
   scope :located, -> { where.not(location: '').where.not(latitude: nil).where.not(longitude: nil) }
   scope :recently_signed_in, -> { reorder(updated_at: :desc) }
   scope :recently_joined, -> { reorder(confirmed_at: :desc) }
   scope :wants_newsletter, -> { where(newsletter: true) }
   scope :interesting, -> { confirmed.located.recently_signed_in.has_plantings }
-
   scope :has_plantings, -> { joins(:plantings).group("members.id") }
 
-  has_many :follows, class_name: "Follow", foreign_key: "follower_id"
-  has_many :followed, through: :follows
-
-  has_many :inverse_follows, class_name: "Follow", foreign_key: "followed_id"
-  has_many :followers, through: :inverse_follows, source: :follower
-
   # Include default devise modules. Others available are:
   # :token_authenticatable, :confirmable,
   # :lockable, :timeoutable and :omniauthable
@@ -57,45 +49,42 @@ class Member < ActiveRecord::Base
 
   # set up geocoding
   geocoded_by :location
-  after_validation :geocode
-  after_validation :empty_unwanted_geocodes
 
   # Virtual attribute for authenticating by either username or email
   # This is in addition to a real persisted field like 'username'
   attr_accessor :login
 
+  #
+  # Validations
   # Requires acceptance of the Terms of Service
-  validates :tos_agreement, acceptance: { allow_nil: true,
-                                          accept: true }
-
+  validates :tos_agreement, acceptance: { allow_nil: true, accept: true }
   validates :login_name,
     length: {
-      minimum: 2,
-      maximum: 25,
-      message: "should be between 2 and 25 characters long"
+      minimum: 2, maximum: 25, message: "should be between 2 and 25 characters long"
     },
     exclusion: {
-      in: %w(growstuff admin moderator staff nearby),
-      message: "name is reserved"
+      in: %w(growstuff admin moderator staff nearby), message: "name is reserved"
     },
     format: {
-      with: /\A\w+\z/,
-      message: "may only include letters, numbers, or underscores"
+      with: /\A\w+\z/, message: "may only include letters, numbers, or underscores"
     },
     uniqueness: {
       case_sensitive: false
     }
 
-  # Give each new member a default garden
-  after_create { |member| Garden.create(name: "Garden", owner_id: member.id) }
+  #
+  # Triggers
+  after_validation :geocode
+  after_validation :empty_unwanted_geocodes
+  after_save :update_newsletter_subscription
 
+  # Give each new member a default garden
   # and an account record (for paid accounts etc)
   # we use find_or_create to avoid accidentally creating a second one,
   # which can happen sometimes especially with FactoryBot associations
+  after_create { |member| Garden.create(name: "Garden", owner_id: member.id) }
   after_create { |member| Account.find_or_create_by(member_id: member.id) }
 
-  after_save :update_newsletter_subscription
-
   # allow login via either login_name or email address
   def self.find_first_by_auth_conditions(warden_conditions)
     conditions = warden_conditions.dup
diff --git a/app/models/notification.rb b/app/models/notification.rb
index a7320692e..1b09b1e5e 100644
--- a/app/models/notification.rb
+++ b/app/models/notification.rb
@@ -5,7 +5,6 @@ class Notification < ActiveRecord::Base
 
   validates :subject, length: { maximum: 255 }
 
-  default_scope { order('created_at DESC') }
   scope :unread, -> { where(read: false) }
   scope :by_recipient, ->(recipient) { where(recipient_id: recipient) }
 
diff --git a/app/models/order.rb b/app/models/order.rb
index 3993ce620..53a309c35 100644
--- a/app/models/order.rb
+++ b/app/models/order.rb
@@ -1,17 +1,22 @@
 class Order < ActiveRecord::Base
+  #
+  # Relationships
   belongs_to :member, with_deleted: true
-
   has_many :order_items, dependent: :destroy
 
-  default_scope { order('created_at DESC') }
-
+  #
+  # Validations
   validates :referral_code, format: {
     with: /\A[a-zA-Z0-9 ]*\z/,
     message: "may only include letters and numbers"
   }
 
+  #
+  # Teiggers
   before_save :standardize_referral_code
 
+  #
+  # Scopes
   scope :by_member, ->(member) { where(member: member) }
 
   # total price of an order
diff --git a/app/models/photo.rb b/app/models/photo.rb
index 402eebd7e..244687fb5 100644
--- a/app/models/photo.rb
+++ b/app/models/photo.rb
@@ -8,7 +8,7 @@ class Photo < ActiveRecord::Base
 
   before_destroy { all_associations.clear }
 
-  default_scope { joins(:owner).order(created_at: :desc) }
+  default_scope { joins(:owner) } # Ensures the owner still exists
 
   def associations?
     plantings.any? || harvests.any? || gardens.any? || seeds.any?
diff --git a/app/models/planting.rb b/app/models/planting.rb
index ec0355a52..c4503bfb4 100644
--- a/app/models/planting.rb
+++ b/app/models/planting.rb
@@ -22,7 +22,7 @@ class Planting < ActiveRecord::Base
 
   ##
   ## Scopes
-  default_scope { joins(:owner).order(created_at: :desc) }
+  default_scope { joins(:owner) } # Ensures the owner still exists
   scope :finished, -> { where(finished: true) }
   scope :current, -> { where(finished: false) }
   scope :interesting, -> { has_photos.one_per_owner }
diff --git a/app/models/post.rb b/app/models/post.rb
index df7303c9e..6014e8f64 100644
--- a/app/models/post.rb
+++ b/app/models/post.rb
@@ -2,45 +2,27 @@ class Post < ActiveRecord::Base
   extend FriendlyId
   include Likeable
   friendly_id :author_date_subject, use: %i(slugged finders)
+
+  #
+  # Relationships
   belongs_to :author, class_name: 'Member'
   belongs_to :forum
   has_many :comments, dependent: :destroy
   has_and_belongs_to_many :crops # rubocop:disable Rails/HasAndBelongsToMany
-  before_destroy { |post| post.crops.clear }
-  after_save :update_crops_posts_association
   # also has_many notifications, but kinda meaningless to get at them
   # from this direction, so we won't set up an association for now.
 
-  after_create do
-    recipients = []
-    sender = author.id
-    body.scan(Haml::Filters::GrowstuffMarkdown::MEMBER_REGEX) do |_m|
-      # find member case-insensitively and add to list of recipients
-      member = Member.case_insensitive_login_name(Regexp.last_match(1)).first
-      recipients << member if member && !recipients.include?(member)
-    end
-    body.scan(Haml::Filters::GrowstuffMarkdown::MEMBER_AT_REGEX) do |_m|
-      # find member case-insensitively and add to list of recipients
-      member = Member.case_insensitive_login_name(Regexp.last_match(1)[1..-1]).first
-      recipients << member if member && !recipients.include?(member)
-    end
-    # don't send notifications to yourself
-    recipients.map(&:id).each do |recipient|
-      next unless recipient != sender
-      Notification.create(
-        recipient_id: recipient,
-        sender_id: sender,
-        subject: "#{author} mentioned you in their post #{subject}",
-        body: body
-      )
-    end
-  end
+  #
+  # Triggers
+  before_destroy { |post| post.crops.clear }
+  after_save :update_crops_posts_association
+  after_create  :send_notification
 
-  default_scope { joins(:author).order(created_at: :desc) }
+  default_scope { joins(:author) } # Ensures the owner still exists
 
-  validates :subject,
-    presence: true,
-    length: { maximum: 255 }
+  #
+  # Validations
+  validates :subject, presence: true, length: { maximum: 255 }
 
   def author_date_subject
     # slugs are created before created_at is set
@@ -78,4 +60,29 @@ class Post < ActiveRecord::Base
       crops << crop if crop && !crops.include?(crop)
     end
   end
+
+  def send_notification
+    recipients = []
+    sender = author.id
+    body.scan(Haml::Filters::GrowstuffMarkdown::MEMBER_REGEX) do |_m|
+      # find member case-insensitively and add to list of recipients
+      member = Member.case_insensitive_login_name(Regexp.last_match(1)).first
+      recipients << member if member && !recipients.include?(member)
+    end
+    body.scan(Haml::Filters::GrowstuffMarkdown::MEMBER_AT_REGEX) do |_m|
+      # find member case-insensitively and add to list of recipients
+      member = Member.case_insensitive_login_name(Regexp.last_match(1)[1..-1]).first
+      recipients << member if member && !recipients.include?(member)
+    end
+    # don't send notifications to yourself
+    recipients.map(&:id).each do |recipient_id|
+      next unless recipient_id != sender
+      Notification.create(
+        recipient_id: recipient_id,
+        sender_id: sender,
+        subject: "#{author} mentioned you in their post #{subject}",
+        body: body
+      )
+    end
+  end
 end
diff --git a/app/models/product.rb b/app/models/product.rb
index 646edd1a6..01b5a8902 100644
--- a/app/models/product.rb
+++ b/app/models/product.rb
@@ -1,13 +1,12 @@
 class Product < ActiveRecord::Base
-  has_and_belongs_to_many :orders # rubocop:disable Rails/HasAndBelongsToMany
+  #
+  # Relationships
   belongs_to :account_type
+  has_and_belongs_to_many :orders # rubocop:disable Rails/HasAndBelongsToMany
 
-  validates :paid_months,
-    numericality: {
-      only_integer: true,
-      greater_than_or_equal_to: 0
-    },
-    allow_nil: true
+  #
+  # Validations
+  validates :paid_months, allow_nil: true, numericality: { only_integer: true, greater_than_or_equal_to: 0 }
   validates :min_price, presence: true
 
   def to_s
diff --git a/app/models/seed.rb b/app/models/seed.rb
index 9b2fc510b..219e29b79 100644
--- a/app/models/seed.rb
+++ b/app/models/seed.rb
@@ -3,81 +3,50 @@ class Seed < ActiveRecord::Base
   include PhotoCapable
   friendly_id :seed_slug, use: [:slugged, :finders]
 
+  TRADABLE_TO_VALUES = %w(nowhere locally nationally internationally).freeze
+  ORGANIC_VALUES = ['certified organic', 'non-certified organic', 'conventional/non-organic', 'unknown'].freeze
+  GMO_VALUES = ['certified GMO-free', 'non-certified GMO-free', 'GMO', 'unknown'].freeze
+  HEIRLOOM_VALUES = %w(heirloom hybrid unknown).freeze
+
+  #
+  # Relationships
   belongs_to :crop
   belongs_to :owner, class_name: 'Member', foreign_key: 'owner_id', counter_cache: true
 
-  default_scope { joins(:owner).order(created_at: :desc) }
-
+  #
+  # Validations
   validates :crop, approved: true
   delegate :name, to: :crop
   delegate :default_photo, to: :crop
-
   validates :crop, presence: { message: "must be present and exist in our database" }
-  validates :quantity,
-    numericality: {
-      only_integer: true,
-      greater_than_or_equal_to: 0
-    },
-    allow_nil: true
-  validates :days_until_maturity_min,
-    numericality: {
-      only_integer: true,
-      greater_than_or_equal_to: 0
-    },
-    allow_nil: true
-  validates :days_until_maturity_max,
-    numericality: {
-      only_integer: true,
-      greater_than_or_equal_to: 0
-    },
-    allow_nil: true
+  validates :quantity, allow_nil: true,
+                       numericality: { only_integer: true, greater_than_or_equal_to: 0 }
+  validates :days_until_maturity_min, allow_nil: true,
+                                      numericality: { only_integer: true, greater_than_or_equal_to: 0 }
+  validates :days_until_maturity_max, allow_nil: true,
+                                      numericality: { only_integer: true, greater_than_or_equal_to: 0 }
+  validates :tradable_to, allow_nil: false, allow_blank: false,
+                          inclusion: { in: TRADABLE_TO_VALUES, message: "You may only trade seed nowhere, "\
+                                                "locally, nationally, or internationally" }
+  validates :organic, allow_nil: false, allow_blank: false,
+                      inclusion: { in: ORGANIC_VALUES, message: "You must say whether the seeds "\
+                                             "are organic or not, or that you don't know" }
+  validates :gmo, allow_nil: false, allow_blank: false,
+                  inclusion: { in: GMO_VALUES, message: "You must say whether the seeds are "\
+                                                        "genetically modified or not, or that you don't know" }
+  validates :heirloom, allow_nil: false, allow_blank: false,
+                       inclusion: { in: HEIRLOOM_VALUES, message: "You must say whether the seeds"\
+                                                                  "are heirloom, hybrid, or unknown" }
 
+  #
+  # Scopes
+  default_scope { joins(:owner) } # Ensure owner exists
   scope :tradable, -> { where.not(tradable_to: 'nowhere') }
   scope :interesting, -> { tradable.has_location }
   scope :has_location, -> { joins(:owner).where.not("members.location": nil) }
-  TRADABLE_TO_VALUES = %w(nowhere locally nationally internationally).freeze
-  validates :tradable_to, inclusion: { in: TRADABLE_TO_VALUES,
-                                       message: "You may only trade seed nowhere, "\
-                                                "locally, nationally, or internationally" },
-                          allow_nil: false,
-                          allow_blank: false
-
-  ORGANIC_VALUES = [
-    'certified organic',
-    'non-certified organic',
-    'conventional/non-organic',
-    'unknown'
-  ].freeze
-  validates :organic, inclusion: { in: ORGANIC_VALUES,
-                                   message: "You must say whether the seeds "\
-                                             "are organic or not, or that you don't know" },
-                      allow_nil: false,
-                      allow_blank: false
-
-  GMO_VALUES = [
-    'certified GMO-free',
-    'non-certified GMO-free',
-    'GMO',
-    'unknown'
-  ].freeze
-  validates :gmo, inclusion: { in: GMO_VALUES,
-                               message: "You must say whether the seeds are "\
-                                        "genetically modified or not, or that you don't know" },
-                  allow_nil: false,
-                  allow_blank: false
-
-  HEIRLOOM_VALUES = %w(heirloom hybrid unknown).freeze
-  validates :heirloom, inclusion: { in: HEIRLOOM_VALUES,
-                                    message: "You must say whether the seeds are heirloom, hybrid, or unknown" },
-                       allow_nil: false,
-                       allow_blank: false
 
   def tradable?
-    if tradable_to == 'nowhere'
-      false
-    else
-      true
-    end
+    tradable_to == 'nowhere'
   end
 
   def seed_slug

From 0446c0a4055c2f1801ada9ba495756b8949dad6b Mon Sep 17 00:00:00 2001
From: Brenda Wallace <brenda@wallace.net.nz>
Date: Sun, 26 Nov 2017 13:36:23 +1300
Subject: [PATCH 02/29] Add ordering back into controller #index

---
 app/controllers/account_types_controller.rb    | 2 +-
 app/controllers/accounts_controller.rb         | 2 +-
 app/controllers/alternate_names_controller.rb  | 2 +-
 app/controllers/comments_controller.rb         | 2 +-
 app/controllers/crops_controller.rb            | 2 +-
 app/controllers/forums_controller.rb           | 2 +-
 app/controllers/gardens_controller.rb          | 2 +-
 app/controllers/notifications_controller.rb    | 2 +-
 app/controllers/photos_controller.rb           | 4 +++-
 app/controllers/plant_parts_controller.rb      | 2 +-
 app/controllers/plantings_controller.rb        | 2 +-
 app/controllers/posts_controller.rb            | 2 +-
 app/controllers/products_controller.rb         | 2 +-
 app/controllers/roles_controller.rb            | 2 +-
 app/controllers/scientific_names_controller.rb | 2 +-
 app/controllers/seeds_controller.rb            | 2 +-
 16 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/app/controllers/account_types_controller.rb b/app/controllers/account_types_controller.rb
index 15d75016a..f86892346 100644
--- a/app/controllers/account_types_controller.rb
+++ b/app/controllers/account_types_controller.rb
@@ -5,7 +5,7 @@ class AccountTypesController < ApplicationController
 
   # GET /account_types
   def index
-    @account_types = AccountType.all
+    @account_types = AccountType.all.order(:name)
     respond_with(@account_types)
   end
 
diff --git a/app/controllers/accounts_controller.rb b/app/controllers/accounts_controller.rb
index 86c465809..040092aba 100644
--- a/app/controllers/accounts_controller.rb
+++ b/app/controllers/accounts_controller.rb
@@ -5,7 +5,7 @@ class AccountsController < ApplicationController
 
   # GET /accounts
   def index
-    @accounts = Account.all
+    @accounts = Account.all.order(created_at: :desc)
     respond_with(@accounts)
   end
 
diff --git a/app/controllers/alternate_names_controller.rb b/app/controllers/alternate_names_controller.rb
index 097c172c4..b673a63d7 100644
--- a/app/controllers/alternate_names_controller.rb
+++ b/app/controllers/alternate_names_controller.rb
@@ -7,7 +7,7 @@ class AlternateNamesController < ApplicationController
   # GET /alternate_names
   # GET /alternate_names.json
   def index
-    @alternate_names = AlternateName.all
+    @alternate_names = AlternateName.all.order(:name)
     respond_with(@alternate_names)
   end
 
diff --git a/app/controllers/comments_controller.rb b/app/controllers/comments_controller.rb
index e0f983425..d99c9c1eb 100644
--- a/app/controllers/comments_controller.rb
+++ b/app/controllers/comments_controller.rb
@@ -6,7 +6,7 @@ class CommentsController < ApplicationController
   responders :flash
 
   def index
-    @comments = Comment.paginate(page: params[:page])
+    @comments = Comment.order("comments.created_at": :desc).paginate(page: params[:page])
     respond_with(@comments)
   end
 
diff --git a/app/controllers/crops_controller.rb b/app/controllers/crops_controller.rb
index 1deb716c7..2a16206bb 100644
--- a/app/controllers/crops_controller.rb
+++ b/app/controllers/crops_controller.rb
@@ -185,7 +185,7 @@ class CropsController < ApplicationController
   def crops
     q = Crop.approved.includes(:scientific_names, plantings: :photos)
     q = q.popular unless @sort == 'alpha'
-    q.includes(:photos).paginate(page: params[:page])
+    q.order("LOWER(crops.name)").includes(:photos).paginate(page: params[:page])
   end
 
   def requested_crops
diff --git a/app/controllers/forums_controller.rb b/app/controllers/forums_controller.rb
index 78daf807d..949de10b0 100644
--- a/app/controllers/forums_controller.rb
+++ b/app/controllers/forums_controller.rb
@@ -5,7 +5,7 @@ class ForumsController < ApplicationController
   # GET /forums
   # GET /forums.json
   def index
-    @forums = Forum.all
+    @forums = Forum.all.order(:name)
     respond_with(@forums)
   end
 
diff --git a/app/controllers/gardens_controller.rb b/app/controllers/gardens_controller.rb
index 32b392093..6b345dc70 100644
--- a/app/controllers/gardens_controller.rb
+++ b/app/controllers/gardens_controller.rb
@@ -68,6 +68,6 @@ class GardensController < ApplicationController
   def gardens
     g = @owner ? @owner.gardens : Garden.all
     g = g.active unless @show_all
-    g.joins(:owner).order(:updated_at).paginate(page: params[:page])
+    g.joins(:owner).order(:name).paginate(page: params[:page])
   end
 end
diff --git a/app/controllers/notifications_controller.rb b/app/controllers/notifications_controller.rb
index f69130f07..7c3c1f322 100644
--- a/app/controllers/notifications_controller.rb
+++ b/app/controllers/notifications_controller.rb
@@ -6,7 +6,7 @@ class NotificationsController < ApplicationController
 
   # GET /notifications
   def index
-    @notifications = Notification.by_recipient(current_member).page(params[:page])
+    @notifications = Notification.by_recipient(current_member).order(:created_at).page(params[:page])
   end
 
   # GET /notifications/1
diff --git a/app/controllers/photos_controller.rb b/app/controllers/photos_controller.rb
index 80d481f60..a36064684 100644
--- a/app/controllers/photos_controller.rb
+++ b/app/controllers/photos_controller.rb
@@ -12,7 +12,9 @@ class PhotosController < ApplicationController
     else
       @photos = Photo.all
     end
-    @photos = @photos.includes(:owner).order(:created_at).paginate(page: params[:page])
+    @photos = @photos.order("photos.created_at": :desc)
+      .includes(:owner)
+      .paginate(page: params[:page])
     respond_with(@photos)
   end
 
diff --git a/app/controllers/plant_parts_controller.rb b/app/controllers/plant_parts_controller.rb
index e7c3a38e5..2841e54e7 100644
--- a/app/controllers/plant_parts_controller.rb
+++ b/app/controllers/plant_parts_controller.rb
@@ -4,7 +4,7 @@ class PlantPartsController < ApplicationController
   responders :flash
 
   def index
-    @plant_parts = PlantPart.all
+    @plant_parts = PlantPart.all.order("plant_parts.name")
     respond_with(@plant_parts)
   end
 
diff --git a/app/controllers/plantings_controller.rb b/app/controllers/plantings_controller.rb
index d6e937a98..fff2b9558 100644
--- a/app/controllers/plantings_controller.rb
+++ b/app/controllers/plantings_controller.rb
@@ -96,8 +96,8 @@ class PlantingsController < ApplicationController
         end
     p = p.current unless @show_all
     p.joins(:owner, :crop, :garden)
+      .order("plantings.created_at": :desc)
       .includes(:crop, :owner, :garden)
-      .order(:created_at)
       .paginate(page: params[:page])
   end
 end
diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb
index dbda9be38..b72d22fae 100644
--- a/app/controllers/posts_controller.rb
+++ b/app/controllers/posts_controller.rb
@@ -66,6 +66,6 @@ class PostsController < ApplicationController
       @author.posts
     else
       Post
-    end.includes(:author, comments: :author).paginate(page: params[:page])
+    end.order("posts.created_at": :desc).includes(:author, comments: :author).paginate(page: params[:page])
   end
 end
diff --git a/app/controllers/products_controller.rb b/app/controllers/products_controller.rb
index fbd57e6b4..8e0a1fb08 100644
--- a/app/controllers/products_controller.rb
+++ b/app/controllers/products_controller.rb
@@ -5,7 +5,7 @@ class ProductsController < ApplicationController
   responders :flash
 
   def index
-    @products = Product.all
+    @products = Product.all.order(:name)
     respond_with @products
   end
 
diff --git a/app/controllers/roles_controller.rb b/app/controllers/roles_controller.rb
index 30a88bee6..6b43cf9da 100644
--- a/app/controllers/roles_controller.rb
+++ b/app/controllers/roles_controller.rb
@@ -5,7 +5,7 @@ class RolesController < ApplicationController
   responders :flash
 
   def index
-    @roles = Role.all
+    @roles = Role.all.order(:name)
     respond_with @roles
   end
 
diff --git a/app/controllers/scientific_names_controller.rb b/app/controllers/scientific_names_controller.rb
index 5768943b8..954a94dbc 100644
--- a/app/controllers/scientific_names_controller.rb
+++ b/app/controllers/scientific_names_controller.rb
@@ -7,7 +7,7 @@ class ScientificNamesController < ApplicationController
   # GET /scientific_names
   # GET /scientific_names.json
   def index
-    @scientific_names = ScientificName.all
+    @scientific_names = ScientificName.all.order(:name)
     respond_with(@scientific_names)
   end
 
diff --git a/app/controllers/seeds_controller.rb b/app/controllers/seeds_controller.rb
index 0c64e5984..283f65744 100644
--- a/app/controllers/seeds_controller.rb
+++ b/app/controllers/seeds_controller.rb
@@ -75,7 +75,7 @@ class SeedsController < ApplicationController
       crop.seeds
     else
       Seed
-    end.includes(:owner, :crop).paginate(page: params[:page])
+    end.order("seeds.created_at": :desc).includes(:owner, :crop).paginate(page: params[:page])
   end
 
   def csv_filename

From 9aff65e956c74dbe2b72162f80355c0758deb036 Mon Sep 17 00:00:00 2001
From: Brenda Wallace <brenda@wallace.net.nz>
Date: Sun, 26 Nov 2017 13:54:00 +1300
Subject: [PATCH 03/29] Added ordering to finding first photo

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

diff --git a/app/models/harvest.rb b/app/models/harvest.rb
index 1e4ceb2dc..77db04014 100644
--- a/app/models/harvest.rb
+++ b/app/models/harvest.rb
@@ -124,7 +124,7 @@ class Harvest < ActiveRecord::Base
   end
 
   def default_photo
-    photos.first || crop.default_photo
+    photos.order(created_at: :desc).first || crop.default_photo
   end
 
   def crop_must_match_planting

From 023c870d4990b4426ad1255028d4aa7b72599825 Mon Sep 17 00:00:00 2001
From: Brenda Wallace <brenda@wallace.net.nz>
Date: Sun, 26 Nov 2017 13:55:40 +1300
Subject: [PATCH 04/29] Fix up crop.recent

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

diff --git a/app/models/crop.rb b/app/models/crop.rb
index ceb275b8b..4d21c4894 100644
--- a/app/models/crop.rb
+++ b/app/models/crop.rb
@@ -24,7 +24,7 @@ class Crop < ActiveRecord::Base
 
   ##
   ## Scopes
-  scope :recent, -> { approved.reorder("created_at desc") }
+  scope :recent, -> { approved.order(created_at: :desc) }
   scope :toplevel, -> { approved.where(parent_id: nil) }
   scope :popular, -> { approved.reorder("plantings_count desc, lower(name) asc") }
   # ok on sqlite and psql, but not on mysql

From 4bed6da4224114768c0bf7ecce5a21deacf4ab5e Mon Sep 17 00:00:00 2001
From: Brenda Wallace <brenda@wallace.net.nz>
Date: Sun, 26 Nov 2017 13:56:11 +1300
Subject: [PATCH 05/29] Crop validation methods can be private

---
 app/models/harvest.rb | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/app/models/harvest.rb b/app/models/harvest.rb
index 77db04014..6bb9aa048 100644
--- a/app/models/harvest.rb
+++ b/app/models/harvest.rb
@@ -127,6 +127,8 @@ class Harvest < ActiveRecord::Base
     photos.order(created_at: :desc).first || crop.default_photo
   end
 
+  private
+
   def crop_must_match_planting
     return if planting.blank? # only check if we are linked to a planting
     errors.add(:planting, "must be the same crop") unless crop == planting.crop

From d77d1d9a77177682b2205dc3f1b4e7b30024fc7e Mon Sep 17 00:00:00 2001
From: Brenda Wallace <brenda@wallace.net.nz>
Date: Sun, 26 Nov 2017 13:57:57 +1300
Subject: [PATCH 06/29] spec for notifications needs to be agnostic about the
 order

---
 spec/models/notification_spec.rb | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/spec/models/notification_spec.rb b/spec/models/notification_spec.rb
index be5fd019d..4f04573d5 100644
--- a/spec/models/notification_spec.rb
+++ b/spec/models/notification_spec.rb
@@ -20,7 +20,8 @@ describe Notification do
     @n2 = FactoryBot.create(:notification, read: true)
     Notification.unread.should eq [notification]
     @n3 = FactoryBot.create(:notification, read: false)
-    Notification.unread.should eq [@n3, notification]
+    Notification.unread.should include @n3
+    Notification.unread.should include notification
   end
 
   it "counts unread" do

From 962b3b574da34760673f99c89c831daabe96d360 Mon Sep 17 00:00:00 2001
From: Brenda Wallace <brenda@wallace.net.nz>
Date: Sun, 26 Nov 2017 14:01:40 +1300
Subject: [PATCH 07/29] Fixed up planting spec to not care about ordering

---
 spec/models/planting_spec.rb | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/spec/models/planting_spec.rb b/spec/models/planting_spec.rb
index ec9eb0746..48a035bb4 100644
--- a/spec/models/planting_spec.rb
+++ b/spec/models/planting_spec.rb
@@ -155,12 +155,6 @@ describe Planting do
     planting.location.should eq "#{garden_owner.login_name}'s #{garden.name}"
   end
 
-  it "sorts plantings in descending order of creation" do
-    @planting1 = FactoryBot.create(:planting)
-    @planting2 = FactoryBot.create(:planting)
-    Planting.first.should eq @planting2
-  end
-
   it "should have a slug" do
     planting.slug.should match(/^member\d+-springfield-community-garden-tomato$/)
   end
@@ -321,12 +315,14 @@ describe Planting do
         p.save
       end
 
-      Planting.interesting.should eq [
+      [
         @planting4,
         @planting3,
         @planting2,
         @planting1
-      ]
+      ].each do |p|
+        Planting.interesting.should include p
+      end
     end
 
     context "default arguments" do

From 6aa689b0d826ef572119d99e9a275ae205b6a754 Mon Sep 17 00:00:00 2001
From: Brenda Wallace <brenda@wallace.net.nz>
Date: Sun, 26 Nov 2017 14:02:22 +1300
Subject: [PATCH 08/29] Removed some model ordering specs

---
 spec/models/comment_spec.rb |  4 ----
 spec/models/crop_spec.rb    |  4 ----
 spec/models/forum_spec.rb   |  6 ------
 spec/models/member_spec.rb  | 10 ----------
 spec/models/order_spec.rb   |  5 -----
 spec/models/post_spec.rb    | 12 ------------
 6 files changed, 41 deletions(-)

diff --git a/spec/models/comment_spec.rb b/spec/models/comment_spec.rb
index 70d0a8386..b98c52062 100644
--- a/spec/models/comment_spec.rb
+++ b/spec/models/comment_spec.rb
@@ -49,10 +49,6 @@ describe Comment do
       @c2 = FactoryBot.create(:comment, post: @p, author: @m)
     end
 
-    it 'is in DESC order by default' do
-      Comment.all.should eq [@c2, @c1]
-    end
-
     it 'has a scope for ASC order for displaying on post page' do
       Comment.post_order.should eq [@c1, @c2]
     end
diff --git a/spec/models/crop_spec.rb b/spec/models/crop_spec.rb
index 860e2e935..5d9b671c0 100644
--- a/spec/models/crop_spec.rb
+++ b/spec/models/crop_spec.rb
@@ -39,10 +39,6 @@ describe Crop do
       @lowercase = FactoryBot.create(:lowercasecrop, created_at: 2.days.ago)
     end
 
-    it "should be sorted case-insensitively" do
-      Crop.first.should == @lowercase
-    end
-
     it 'recent scope sorts by creation date' do
       Crop.recent.first.should == @uppercase
     end
diff --git a/spec/models/forum_spec.rb b/spec/models/forum_spec.rb
index 5fab0bfed..310c36842 100644
--- a/spec/models/forum_spec.rb
+++ b/spec/models/forum_spec.rb
@@ -20,10 +20,4 @@ describe Forum do
     @post2 = FactoryBot.create(:forum_post, forum: forum)
     forum.posts.size.should == 2
   end
-
-  it "orders posts in reverse chron order" do
-    @post1 = FactoryBot.create(:forum_post, forum: forum, created_at: 2.days.ago)
-    @post2 = FactoryBot.create(:forum_post, forum: forum, created_at: 1.day.ago)
-    forum.posts.first.should eq @post2
-  end
 end
diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb
index 4a42a8409..7dd56f1cb 100644
--- a/spec/models/member_spec.rb
+++ b/spec/models/member_spec.rb
@@ -140,16 +140,6 @@ describe 'member' do
     end
   end
 
-  context 'ordering' do
-    before do
-      FactoryBot.create(:member, login_name: "Zoe")
-      FactoryBot.create(:member, login_name: "Anna")
-    end
-    it "should be sorted by name" do
-      expect(Member.first.login_name).to eq("Anna")
-    end
-  end
-
   context 'invalid login names' do
     it "doesn't allow short names" do
       member = FactoryBot.build(:invalid_member_shortname)
diff --git a/spec/models/order_spec.rb b/spec/models/order_spec.rb
index 2d43473e5..9abb381ba 100644
--- a/spec/models/order_spec.rb
+++ b/spec/models/order_spec.rb
@@ -25,11 +25,6 @@ describe Order do
     @order.order_items.first.should eq @order_item
   end
 
-  it 'sorts by created_at DESC' do
-    @order2 = FactoryBot.create(:order)
-    Order.all.should eq [@order2, @order]
-  end
-
   it 'updates the account details' do
     @member = FactoryBot.create(:member)
     @order = FactoryBot.create(:order, member: @member)
diff --git a/spec/models/post_spec.rb b/spec/models/post_spec.rb
index 479a04bff..566f451cb 100644
--- a/spec/models/post_spec.rb
+++ b/spec/models/post_spec.rb
@@ -4,18 +4,6 @@ describe Post do
   let(:member) { FactoryBot.create(:member) }
   it_behaves_like "it is likeable"
 
-  it "should be sorted in reverse order" do
-    FactoryBot.create(:post,
-      subject: 'first entry',
-      author: member,
-      created_at: 2.days.ago)
-    FactoryBot.create(:post,
-      subject: 'second entry',
-      author: member,
-      created_at: 1.day.ago)
-    Post.first.subject.should == "second entry"
-  end
-
   it "should have a slug" do
     post = FactoryBot.create(:post, author: member)
     time = post.created_at

From 7357524271264a0e078f2b4ec899fcac6d9f5780 Mon Sep 17 00:00:00 2001
From: Brenda Wallace <brenda@wallace.net.nz>
Date: Sun, 26 Nov 2017 15:25:30 +1300
Subject: [PATCH 09/29] Garden's default photo should be the most recent photo

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

diff --git a/app/models/garden.rb b/app/models/garden.rb
index bde976de0..1df5397b3 100644
--- a/app/models/garden.rb
+++ b/app/models/garden.rb
@@ -88,6 +88,6 @@ class Garden < ActiveRecord::Base
   end
 
   def default_photo
-    photos.first
+    photos.order(created_at: :desc).first
   end
 end

From 9f8a32e9f88b3016e6a7c43d7f4c7964bbf8bc71 Mon Sep 17 00:00:00 2001
From: Brenda Wallace <brenda@wallace.net.nz>
Date: Sun, 26 Nov 2017 15:50:00 +1300
Subject: [PATCH 10/29] Feature plantings need to be ordered

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

diff --git a/app/models/garden.rb b/app/models/garden.rb
index 1df5397b3..0c3378b00 100644
--- a/app/models/garden.rb
+++ b/app/models/garden.rb
@@ -62,7 +62,7 @@ class Garden < ActiveRecord::Base
     unique_plantings = []
     seen_crops = []
 
-    plantings.includes(:garden, :crop, :owner, :harvests).each do |p|
+    plantings.order(created_at: :desc).includes(:garden, :crop, :owner, :harvests).each do |p|
       unless seen_crops.include?(p.crop)
         unique_plantings.push(p)
         seen_crops.push(p.crop)

From 0205b345d0bd8a325c33a23ab385d1ab09da4cf8 Mon Sep 17 00:00:00 2001
From: Brenda Wallace <brenda@wallace.net.nz>
Date: Sun, 26 Nov 2017 15:50:55 +1300
Subject: [PATCH 11/29] A planting's photo is their most recent photo

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

diff --git a/app/models/planting.rb b/app/models/planting.rb
index c4503bfb4..85e5277c0 100644
--- a/app/models/planting.rb
+++ b/app/models/planting.rb
@@ -72,7 +72,7 @@ class Planting < ActiveRecord::Base
   end
 
   def default_photo
-    photos.first
+    photos.order(created_at: :desc).first
   end
 
   def planted?

From 00e6aeed5ca527fc731498fa014a356330efd011 Mon Sep 17 00:00:00 2001
From: Brenda Wallace <brenda@wallace.net.nz>
Date: Sun, 26 Nov 2017 15:52:28 +1300
Subject: [PATCH 12/29] seed.tradable was back to front logic. fixed

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

diff --git a/app/models/seed.rb b/app/models/seed.rb
index 219e29b79..4213a7886 100644
--- a/app/models/seed.rb
+++ b/app/models/seed.rb
@@ -46,7 +46,7 @@ class Seed < ActiveRecord::Base
   scope :has_location, -> { joins(:owner).where.not("members.location": nil) }
 
   def tradable?
-    tradable_to == 'nowhere'
+    tradable_to != 'nowhere'
   end
 
   def seed_slug

From 33816d4312dca97d0bc8a1ebd0a261a8e328bad1 Mon Sep 17 00:00:00 2001
From: Brenda Wallace <brenda@wallace.net.nz>
Date: Sun, 26 Nov 2017 20:39:49 +1300
Subject: [PATCH 13/29] Use faker gem, in account types - and add controller
 specs

---
 Gemfile                                       |  1 +
 Gemfile.lock                                  |  4 ++-
 app/models/account_type.rb                    |  6 ++++
 .../account_types_controller_spec.rb          | 28 ++++++++++++++++---
 spec/factories/account_types.rb               |  8 +++++-
 5 files changed, 41 insertions(+), 6 deletions(-)

diff --git a/Gemfile b/Gemfile
index af2f2ebc8..36a8608c0 100644
--- a/Gemfile
+++ b/Gemfile
@@ -134,6 +134,7 @@ group :development, :test do
   gem 'coveralls', require: false       # coverage analysis
   gem 'database_cleaner'
   gem 'factory_bot_rails'               # for creating test data
+  gem 'faker'
   gem 'haml-i18n-extractor'
   gem 'haml-rails'                      # HTML templating language
   gem 'haml_lint'                       # Checks haml files for goodness
diff --git a/Gemfile.lock b/Gemfile.lock
index 017c433c1..74118c7e9 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -181,6 +181,8 @@ GEM
     factory_bot_rails (4.8.2)
       factory_bot (~> 4.8.2)
       railties (>= 3.0.0)
+    faker (1.8.4)
+      i18n (~> 0.5)
     faraday (0.12.2)
       multipart-post (>= 1.2, < 3)
     ffi (1.9.18)
@@ -578,6 +580,7 @@ DEPENDENCIES
   elasticsearch-model
   elasticsearch-rails
   factory_bot_rails
+  faker
   figaro
   flickraw
   font-awesome-sass
@@ -633,7 +636,6 @@ DEPENDENCIES
   will_paginate
   xmlrpc
 
-
 RUBY VERSION
    ruby 2.4.1p111
 
diff --git a/app/models/account_type.rb b/app/models/account_type.rb
index d6be51a94..06416751a 100644
--- a/app/models/account_type.rb
+++ b/app/models/account_type.rb
@@ -1,6 +1,12 @@
 class AccountType < ActiveRecord::Base
+  #
+  # Relationships
   has_many :products
 
+  #
+  # Validations
+  validates :name, presence: true, uniqueness: true
+
   def to_s
     name
   end
diff --git a/spec/controllers/account_types_controller_spec.rb b/spec/controllers/account_types_controller_spec.rb
index 3a7559f63..37dbfa706 100644
--- a/spec/controllers/account_types_controller_spec.rb
+++ b/spec/controllers/account_types_controller_spec.rb
@@ -1,10 +1,30 @@
 require 'rails_helper'
 
 describe AccountTypesController do
-  # This automatically creates a "Free" account type
-  login_member(:admin_member)
+  subject { response }
 
-  def valid_attributes
-    { "name" => "MyString" }
+  context 'anon' do
+    describe '#index' do
+      before { get :index }
+      it { is_expected.not_to be_success }
+    end
+  end
+  context 'member' do
+    login_member(:member)
+    describe '#index' do
+      before { get :index }
+      it { is_expected.not_to be_success }
+    end
+  end
+  context 'admin' do
+    login_member(:admin_member)
+    describe '#index' do
+      let!(:aaa) { FactoryBot.create :account_type, name: 'aaa' }
+      let!(:zzz) { FactoryBot.create :account_type, name: 'zzz' }
+      before { get :index }
+      it { is_expected.to be_success }
+      it { expect(assigns[:account_types].first).to eql(aaa) }
+      it { expect(assigns[:account_types].last).to eql(zzz) }
+    end
   end
 end
diff --git a/spec/factories/account_types.rb b/spec/factories/account_types.rb
index a983e24e4..66717492f 100644
--- a/spec/factories/account_types.rb
+++ b/spec/factories/account_types.rb
@@ -2,10 +2,16 @@
 
 FactoryBot.define do
   factory :account_type do
-    name "Free"
+    name { Faker::Name.unique.name }
     is_paid false
     is_permanent_paid false
 
+    factory :free_account_type do
+      name "Free"
+      is_paid false
+      is_permanent_paid false
+    end
+
     factory :paid_account_type do
       name "Paid"
       is_paid true

From d5fece87d0c54eff4c82cc02c887fb5946046e1b Mon Sep 17 00:00:00 2001
From: Brenda Wallace <brenda@wallace.net.nz>
Date: Sun, 26 Nov 2017 20:58:40 +1300
Subject: [PATCH 14/29] Tests for comments#index (and bugfix)

---
 app/controllers/comments_controller.rb       |  2 +-
 spec/controllers/comments_controller_spec.rb | 30 +++++++++++---------
 2 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/app/controllers/comments_controller.rb b/app/controllers/comments_controller.rb
index d99c9c1eb..f85302d74 100644
--- a/app/controllers/comments_controller.rb
+++ b/app/controllers/comments_controller.rb
@@ -6,7 +6,7 @@ class CommentsController < ApplicationController
   responders :flash
 
   def index
-    @comments = Comment.order("comments.created_at": :desc).paginate(page: params[:page])
+    @comments = Comment.order(created_at: :desc).paginate(page: params[:page])
     respond_with(@comments)
   end
 
diff --git a/spec/controllers/comments_controller_spec.rb b/spec/controllers/comments_controller_spec.rb
index 23210a875..0e986cc93 100644
--- a/spec/controllers/comments_controller_spec.rb
+++ b/spec/controllers/comments_controller_spec.rb
@@ -1,10 +1,11 @@
 require 'rails_helper'
 
 describe CommentsController do
+  subject { response }
+  let(:member) { FactoryBot.create(:member) }
   before(:each) do
-    @member = FactoryBot.create(:member)
-    sign_in @member
-    controller.stub(:current_member) { @member }
+    sign_in member
+    controller.stub(:current_member) { member }
   end
 
   def valid_attributes
@@ -13,11 +14,14 @@ describe CommentsController do
   end
 
   describe "GET RSS feed" do
-    it "returns an RSS feed" do
-      get :index, format: "rss"
-      response.should be_success
-      response.should render_template("comments/index")
-      response.content_type.should eq("application/rss+xml")
+    let!(:first_comment) { FactoryBot.create :comment, created_at: 10.days.ago }
+    let!(:last_comment) { FactoryBot.create :comment, created_at: 4.minutes.ago }
+    describe "returns an RSS feed" do
+      before { get :index, format: "rss" }
+      it { is_expected.to be_success }
+      it { is_expected.to render_template("comments/index") }
+      it { expect(response.content_type).to eq("application/rss+xml") }
+      it { expect(assigns(:comments)).to eq([last_comment, first_comment]) }
     end
   end
 
@@ -48,10 +52,10 @@ describe CommentsController do
     before { get :edit, id: comment.to_param }
 
     describe "my comment" do
-      let!(:comment) { FactoryBot.create :comment, author: @member, post: post }
+      let!(:comment) { FactoryBot.create :comment, author: member, post: post }
       let!(:old_comment) { FactoryBot.create(:comment, post: post, created_at: Time.zone.yesterday) }
       it "assigns previous comments as @comments" do
-        assigns(:comments).should eq([comment, old_comment])
+        expect(assigns(:comments)).to eq([comment, old_comment])
       end
     end
 
@@ -65,7 +69,7 @@ describe CommentsController do
     before { put :update, id: comment.to_param, comment: valid_attributes }
 
     describe "my comment" do
-      let(:comment) { FactoryBot.create :comment, author: @member }
+      let(:comment) { FactoryBot.create :comment, author: member }
       it "redirects to the comment's post" do
         expect(response).to redirect_to(comment.post)
       end
@@ -78,7 +82,7 @@ describe CommentsController do
       let(:post) { FactoryBot.create :post, subject: 'our post' }
       let(:other_post) { FactoryBot.create :post, subject: 'the other post' }
       let(:valid_attributes) { { post_id: other_post.id, body: "kōrero" } }
-      let(:comment) { FactoryBot.create :comment, author: @member, post: post }
+      let(:comment) { FactoryBot.create :comment, author: member, post: post }
       it "does not change post_id" do
         comment.reload
         expect(comment.post_id).to eq(post.id)
@@ -90,7 +94,7 @@ describe CommentsController do
     before { delete :destroy, id: comment.to_param }
 
     describe "my comment" do
-      let(:comment) { FactoryBot.create :comment, author: @member }
+      let(:comment) { FactoryBot.create :comment, author: member }
       it "redirects to the post the comment was on" do
         expect(response).to redirect_to(comment.post)
       end

From 9c993e9f4766a69161c6e1dc8c7d6287d51fdbe3 Mon Sep 17 00:00:00 2001
From: Brenda Wallace <brenda@wallace.net.nz>
Date: Mon, 27 Nov 2017 18:59:40 +1300
Subject: [PATCH 15/29] Plantings controller ordering and specs

---
 app/controllers/plantings_controller.rb       |   2 +-
 spec/controllers/plantings_controller_spec.rb | 108 +++++++++---------
 2 files changed, 54 insertions(+), 56 deletions(-)

diff --git a/app/controllers/plantings_controller.rb b/app/controllers/plantings_controller.rb
index fff2b9558..334118dc5 100644
--- a/app/controllers/plantings_controller.rb
+++ b/app/controllers/plantings_controller.rb
@@ -96,7 +96,7 @@ class PlantingsController < ApplicationController
         end
     p = p.current unless @show_all
     p.joins(:owner, :crop, :garden)
-      .order("plantings.created_at": :desc)
+      .order(created_at: :desc)
       .includes(:crop, :owner, :garden)
       .paginate(page: params[:page])
   end
diff --git a/spec/controllers/plantings_controller_spec.rb b/spec/controllers/plantings_controller_spec.rb
index 1bacacc8c..dd1340d44 100644
--- a/spec/controllers/plantings_controller_spec.rb
+++ b/spec/controllers/plantings_controller_spec.rb
@@ -11,85 +11,83 @@ describe PlantingsController do
   end
 
   describe "GET index" do
-    before do
-      @member1 = FactoryBot.create(:member)
-      @member2 = FactoryBot.create(:member)
-      @tomato = FactoryBot.create(:tomato)
-      @maize = FactoryBot.create(:maize)
-      @planting1 = FactoryBot.create(:planting, crop: @tomato, owner: @member1)
-      @planting2 = FactoryBot.create(:planting, crop: @maize, owner: @member2)
+    let!(:member1) { FactoryBot.create(:member) }
+    let!(:member2) { FactoryBot.create(:member) }
+    let!(:tomato) { FactoryBot.create(:tomato) }
+    let!(:maize) { FactoryBot.create(:maize) }
+    let!(:planting1) { FactoryBot.create :planting, crop: tomato, owner: member1, created_at: 1.day.ago }
+    let!(:planting2) { FactoryBot.create :planting, crop: maize, owner: member2, created_at: 5.days.ago }
+
+    describe "assigns all plantings as @plantings" do
+      before { get :index, {} }
+      it { expect(assigns(:plantings)).to match [planting1, planting2] }
     end
 
-    it "assigns all plantings as @plantings" do
-      get :index, {}
-      assigns(:plantings).should =~ [@planting1, @planting2]
+    describe "picks up owner from params and shows owner's plantings only" do
+      before { get :index, owner: member1.slug }
+      it { expect(assigns(:owner)).to eq member1 }
+      it { expect(assigns(:plantings)).to eq [planting1] }
     end
 
-    it "picks up owner from params and shows owner's plantings only" do
-      get :index, owner: @member1.slug
-      assigns(:owner).should eq @member1
-      assigns(:plantings).should eq [@planting1]
-    end
-
-    it "picks up crop from params and shows the plantings for the crop only" do
-      get :index, crop: @maize.name
-      assigns(:crop).should eq @maize
-      assigns(:plantings).should eq [@planting2]
+    describe "picks up crop from params and shows the plantings for the crop only" do
+      before { get :index, crop: maize.name }
+      it { expect(assigns(:crop)).to eq maize }
+      it { expect(assigns(:plantings)).to eq [planting2] }
     end
   end
 
   describe "GET new" do
-    it "picks up crop from params" do
-      crop = FactoryBot.create(:crop)
-      get :new, crop_id: crop.id
-      assigns(:crop).should eq(crop)
+    describe "picks up crop from params" do
+      let(:crop) { FactoryBot.create(:crop) }
+      before { get :new, crop_id: crop.id }
+      it { expect(assigns(:crop)).to eq(crop) }
     end
 
-    it "doesn't die if no crop specified" do
-      get :new, {}
-      assigns(:crop).should be_a_new(Crop)
+    describe "doesn't die if no crop specified" do
+      before { get :new, {} }
+      it { expect(assigns(:crop)).to be_a_new(Crop) }
     end
 
-    it "picks up member's garden from params" do
-      garden = FactoryBot.create(:garden, owner: member)
-      get :new, garden_id: garden.id
-      assigns(:garden).should eq(garden)
+    describe "picks up member's garden from params" do
+      let(:garden) { FactoryBot.create(:garden, owner: member) }
+      before { get :new, garden_id: garden.id }
+      it { expect(assigns(:garden)).to eq(garden) }
     end
 
-    it "Doesn't display another member's garden on planting form" do
-      member = FactoryBot.create(:member) # over-riding member from login_member()
-      garden = FactoryBot.create(:garden, owner: member)
-      get :new, garden_id: garden.id
-      assigns(:garden).should_not eq(garden)
+    describe "Doesn't display another member's garden on planting form" do
+      let(:another_member) { FactoryBot.create(:member) } # over-riding member from login_member()
+      let(:garden) { FactoryBot.create(:garden, owner: another_member) }
+      before { get :new, garden_id: garden.id }
+      it { expect(assigns(:garden)).not_to eq(garden) }
     end
 
-    it "Doesn't display un-approved crops on planting form" do
-      crop = FactoryBot.create(:crop, approval_status: 'pending')
-      FactoryBot.create(:garden, owner: member)
-      get :new, crop_id: crop.id
-      assigns(:crop).should_not eq(crop)
+    describe "Doesn't display un-approved crops on planting form" do
+      let(:crop) { FactoryBot.create(:crop, approval_status: 'pending') }
+      let!(:garden) { FactoryBot.create(:garden, owner: member) }
+      before { get :new, crop_id: crop.id }
+      it { expect(assigns(:crop)).not_to eq(crop) }
     end
 
-    it "Doesn't display rejected crops on planting form" do
-      crop = FactoryBot.create(:crop, approval_status: 'rejected', reason_for_rejection: 'nope')
-      FactoryBot.create(:garden, owner: member)
-      get :new, crop_id: crop.id
-      assigns(:crop).should_not eq(crop)
+    describe "Doesn't display rejected crops on planting form" do
+      let(:crop) { FactoryBot.create(:crop, approval_status: 'rejected', reason_for_rejection: 'nope') }
+      let!(:garden) { FactoryBot.create(:garden, owner: member) }
+      before { get :new, crop_id: crop.id }
+      it { expect(assigns(:crop)).not_to eq(crop) }
     end
 
-    it "doesn't die if no garden specified" do
-      get :new, {}
-      assigns(:garden).should be_a_new(Garden)
+    describe "doesn't die if no garden specified" do
+      before { get :new, {} }
+      it { expect(assigns(:garden)).to be_a_new(Garden) }
     end
 
-    it "sets the date of the planting to today" do
-      get :new, {}
-      assigns(:planting).planted_at.should == Time.zone.today
+    describe "sets the date of the planting to today" do
+      before { get :new, {} }
+      it { expect(assigns(:planting).planted_at).to eq Time.zone.today }
     end
 
-    it "sets the owner automatically" do
-      post :create, planting: valid_attributes
-      assigns(:planting).owner.should eq subject.current_member
+    describe "sets the owner automatically" do
+      before { post :create, planting: valid_attributes }
+      it { expect(assigns(:planting).owner).to eq subject.current_member }
     end
   end
 end

From d0b8ae3411668befe8b2ac5ef9cba247db48f08e Mon Sep 17 00:00:00 2001
From: Brenda Wallace <brenda@wallace.net.nz>
Date: Mon, 27 Nov 2017 19:04:35 +1300
Subject: [PATCH 16/29] Delete follows records when a member is deleted

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

diff --git a/app/models/member.rb b/app/models/member.rb
index f17e9b36e..9828a005c 100644
--- a/app/models/member.rb
+++ b/app/models/member.rb
@@ -25,9 +25,9 @@ class Member < ActiveRecord::Base
   has_many :photos
   has_many :requested_crops, class_name: Crop, foreign_key: 'requester_id'
   has_many :likes, dependent: :destroy
-  has_many :follows, class_name: "Follow", foreign_key: "follower_id"
+  has_many :follows, class_name: "Follow", foreign_key: "follower_id", dependent: :destroy
+  has_many :inverse_follows, class_name: "Follow", foreign_key: "followed_id", dependent: :destroy
   has_many :followed, through: :follows
-  has_many :inverse_follows, class_name: "Follow", foreign_key: "followed_id"
   has_many :followers, through: :inverse_follows, source: :follower
 
   #

From b813669d71d93692503bc8957858af655fcf0c60 Mon Sep 17 00:00:00 2001
From: Brenda Wallace <brenda@wallace.net.nz>
Date: Mon, 27 Nov 2017 19:12:46 +1300
Subject: [PATCH 17/29] Updated shop controller specs

---
 spec/controllers/shop_controller_spec.rb | 46 ++++++++++++------------
 1 file changed, 24 insertions(+), 22 deletions(-)

diff --git a/spec/controllers/shop_controller_spec.rb b/spec/controllers/shop_controller_spec.rb
index 6298b7e76..c25039d2d 100644
--- a/spec/controllers/shop_controller_spec.rb
+++ b/spec/controllers/shop_controller_spec.rb
@@ -1,33 +1,35 @@
 require 'rails_helper'
 
 describe ShopController do
-  before :each do
-    @product1 = FactoryBot.create(:product)
-    @product2 = FactoryBot.create(:product)
-  end
+  let!(:product1) { FactoryBot.create(:product, name: 'aaa') }
+  let!(:product2) { FactoryBot.create(:product, name: 'zzz') }
 
   describe "GET index" do
-    it "assigns all products as @products" do
-      get :index, {}
-      assigns(:products).should eq([@product1, @product2])
-    end
+    describe 'not logged in' do
+      before { get :index, {} }
 
-    it "assigns a new @order_item to build forms" do
-      get :index, {}
-      assigns(:order_item).should be_an_instance_of OrderItem
-    end
+      describe "assigns all products as @products ordered by name" do
+        it { expect(assigns(:products)).to eq([product1, product2]) }
+      end
 
-    it "assigns @order as nil if the user doesn't have one" do
-      get :index, {}
-      assigns(:order).should be_nil
-    end
+      describe "assigns a new @order_item to build forms" do
+        it { expect(assigns(:order_item)).to be_an_instance_of OrderItem }
+      end
 
-    it "assigns @order as current_order if there is one" do
-      @member = FactoryBot.create(:member)
-      sign_in @member
-      @order = FactoryBot.create(:order, member: @member)
-      get :index, {}
-      assigns(:order).should eq @order
+      describe "assigns @order as nil if the user doesn't have one" do
+        it { expect(assigns(:order)).to be_nil }
+      end
+    end
+    describe 'logged in' do
+      describe "assigns @order as current_order if there is one" do
+        let(:member) { FactoryBot.create(:member) }
+        let!(:order) { FactoryBot.create(:order, member: member) }
+        before do
+          sign_in member
+          get :index, {}
+        end
+        it { expect(assigns(:order)).to eq order }
+      end
     end
   end
 end

From 594e466fd4523ba353b5093548e9b0ee5e92357d Mon Sep 17 00:00:00 2001
From: Brenda Wallace <brenda@wallace.net.nz>
Date: Mon, 27 Nov 2017 19:57:46 +1300
Subject: [PATCH 18/29] Fixed photos ordering

---
 app/controllers/photos_controller.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/controllers/photos_controller.rb b/app/controllers/photos_controller.rb
index a36064684..1f091b19b 100644
--- a/app/controllers/photos_controller.rb
+++ b/app/controllers/photos_controller.rb
@@ -12,7 +12,7 @@ class PhotosController < ApplicationController
     else
       @photos = Photo.all
     end
-    @photos = @photos.order("photos.created_at": :desc)
+    @photos = @photos.order(created_at: :desc)
       .includes(:owner)
       .paginate(page: params[:page])
     respond_with(@photos)

From 48f8085e2d592b5c9a61ef30476c5b026b660e5d Mon Sep 17 00:00:00 2001
From: Brenda Wallace <brenda@wallace.net.nz>
Date: Mon, 27 Nov 2017 19:58:31 +1300
Subject: [PATCH 19/29] Fix seeds order

---
 app/controllers/seeds_controller.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/controllers/seeds_controller.rb b/app/controllers/seeds_controller.rb
index 283f65744..f21b1b61a 100644
--- a/app/controllers/seeds_controller.rb
+++ b/app/controllers/seeds_controller.rb
@@ -75,7 +75,7 @@ class SeedsController < ApplicationController
       crop.seeds
     else
       Seed
-    end.order("seeds.created_at": :desc).includes(:owner, :crop).paginate(page: params[:page])
+    end.order(created_at: :desc).includes(:owner, :crop).paginate(page: params[:page])
   end
 
   def csv_filename

From e3886ce5b5f4abbc041f81e180694247b1aaa193 Mon Sep 17 00:00:00 2001
From: Brenda Wallace <brenda@wallace.net.nz>
Date: Mon, 27 Nov 2017 20:10:01 +1300
Subject: [PATCH 20/29] Fixed up unescaped model attribute

---
 app/views/plant_parts/index.html.haml | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/app/views/plant_parts/index.html.haml b/app/views/plant_parts/index.html.haml
index c4a8e9b82..34846e9c3 100644
--- a/app/views/plant_parts/index.html.haml
+++ b/app/views/plant_parts/index.html.haml
@@ -9,7 +9,8 @@
     - if plant_part.crops.empty?
       No crops are harvested for this plant part (yet).
     - else
-      != plant_part.crops.map { |c| link_to(c, c) }.join(", ")
+      - plant_part.crops.limit(100).each do |crop|
+        = link_to(crop, crop_path(crop))
   %p
     = link_to "More detail", plant_part
 
@@ -18,6 +19,3 @@
     = link_to 'Edit', edit_plant_part_path(plant_part), class: 'btn btn-default btn-xs'
   - if can? :destroy, plant_part
     = link_to 'Delete', plant_part, method: :delete, data: { confirm: 'Are you sure?' }, class: 'btn btn-default btn-xs'
-
-
-

From 3058160a59904da493238e2491f255f18983343c Mon Sep 17 00:00:00 2001
From: Brenda Wallace <brenda@wallace.net.nz>
Date: Mon, 27 Nov 2017 20:12:28 +1300
Subject: [PATCH 21/29] Removed an orderng spec for gardens

---
 spec/models/garden_spec.rb | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/spec/models/garden_spec.rb b/spec/models/garden_spec.rb
index cbb097d91..f8e5a3311 100644
--- a/spec/models/garden_spec.rb
+++ b/spec/models/garden_spec.rb
@@ -93,14 +93,6 @@ describe Garden do
     end
   end
 
-  context 'ordering' do
-    it "should be sorted alphabetically" do
-      FactoryBot.create(:garden_z)
-      a = FactoryBot.create(:garden_a)
-      Garden.first.should == a
-    end
-  end
-
   it "destroys plantings when deleted" do
     garden = FactoryBot.create(:garden, owner: owner)
     @planting1 = FactoryBot.create(:planting, garden: garden, owner: garden.owner)

From 9e000233329d3420d1859f6f8c9ea07aaa22b53d Mon Sep 17 00:00:00 2001
From: Brenda Wallace <brenda@wallace.net.nz>
Date: Mon, 27 Nov 2017 20:53:05 +1300
Subject: [PATCH 22/29] Fixed post ordering

---
 app/controllers/posts_controller.rb | 2 +-
 app/models/post.rb                  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb
index b72d22fae..287ef6f2a 100644
--- a/app/controllers/posts_controller.rb
+++ b/app/controllers/posts_controller.rb
@@ -66,6 +66,6 @@ class PostsController < ApplicationController
       @author.posts
     else
       Post
-    end.order("posts.created_at": :desc).includes(:author, comments: :author).paginate(page: params[:page])
+    end.order(created_at: :desc).includes(:author, comments: :author).paginate(page: params[:page])
   end
 end
diff --git a/app/models/post.rb b/app/models/post.rb
index 6014e8f64..76dfcd5d8 100644
--- a/app/models/post.rb
+++ b/app/models/post.rb
@@ -38,7 +38,7 @@ class Post < ActiveRecord::Base
   # i.e. the time of the most recent comment, or of the post itself if
   # there are no comments.
   def recent_activity
-    comments.present? ? comments.reorder('created_at DESC').first.created_at : created_at
+    comments.present? ? comments.order(created_at: :desc).first.created_at : created_at
   end
 
   # return posts sorted by recent activity

From 3a3f4eefbf939382d3bb04d49913ecacf141c41a Mon Sep 17 00:00:00 2001
From: Brenda Wallace <brenda@wallace.net.nz>
Date: Mon, 27 Nov 2017 20:56:29 +1300
Subject: [PATCH 23/29] Added sort to members controller

---
 app/controllers/members_controller.rb | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/app/controllers/members_controller.rb b/app/controllers/members_controller.rb
index c7460dafc..1353a8720 100644
--- a/app/controllers/members_controller.rb
+++ b/app/controllers/members_controller.rb
@@ -96,8 +96,10 @@ class MembersController < ApplicationController
   end
 
   def members
-    q = Member.confirmed
-    q = q.recently_joined if @sort == 'recently_joined'
-    q.paginate(page: params[:page])
+    if @sort == 'recently_joined'
+      Member.recently_joined
+    else
+      Member.order(:login_name)
+    end.confirmed.paginate(page: params[:page])
   end
 end

From d7ec4eacee45d1bd99a3f0f1041919f2a7d594e6 Mon Sep 17 00:00:00 2001
From: Brenda Wallace <brenda@wallace.net.nz>
Date: Mon, 27 Nov 2017 21:36:14 +1300
Subject: [PATCH 24/29] Fix sort on plant parts

---
 app/controllers/plant_parts_controller.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/controllers/plant_parts_controller.rb b/app/controllers/plant_parts_controller.rb
index 2841e54e7..1e594fd72 100644
--- a/app/controllers/plant_parts_controller.rb
+++ b/app/controllers/plant_parts_controller.rb
@@ -4,7 +4,7 @@ class PlantPartsController < ApplicationController
   responders :flash
 
   def index
-    @plant_parts = PlantPart.all.order("plant_parts.name")
+    @plant_parts = PlantPart.all.order(:name)
     respond_with(@plant_parts)
   end
 

From 8d0bec34a67e328bdc667a227f90d834d5e2b5fc Mon Sep 17 00:00:00 2001
From: Brenda Wallace <brenda@wallace.net.nz>
Date: Mon, 27 Nov 2017 21:38:09 +1300
Subject: [PATCH 25/29] Crops controller spec, use let, other cleanup

---
 spec/controllers/crops_controller_spec.rb | 38 ++++++++++++-----------
 1 file changed, 20 insertions(+), 18 deletions(-)

diff --git a/spec/controllers/crops_controller_spec.rb b/spec/controllers/crops_controller_spec.rb
index 1f9e06d43..5b682f6c2 100644
--- a/spec/controllers/crops_controller_spec.rb
+++ b/spec/controllers/crops_controller_spec.rb
@@ -11,37 +11,39 @@ describe CropsController do
     }
   end
 
+  subject { response }
+
   describe "GET crop wrangler homepage" do
-    it 'fetches the crop wrangler homepage' do
-      get :wrangle
-      response.should be_success
-      response.should render_template("crops/wrangle")
-      expect(assigns[:crop_wranglers]).to eq(Role.crop_wranglers)
+    describe 'fetches the crop wrangler homepage' do
+      before { get :wrangle }
+      it { is_expected.to be_success }
+      it { is_expected.to render_template("crops/wrangle") }
+      it { expect(assigns[:crop_wranglers]).to eq(Role.crop_wranglers) }
     end
   end
 
   describe "GET crop hierarchy " do
-    it 'fetches the crop hierarchy page' do
-      get :hierarchy
-      response.should be_success
-      response.should render_template("crops/hierarchy")
+    describe 'fetches the crop hierarchy page' do
+      before { get :hierarchy }
+      it { is_expected.to be_success }
+      it { is_expected.to render_template("crops/hierarchy") }
     end
   end
 
   describe "GET crop search" do
-    it 'fetches the crop search page' do
-      get :search
-      response.should be_success
-      response.should render_template("crops/search")
+    describe 'fetches the crop search page' do
+      before { get :search }
+      it { is_expected.to be_success }
+      it { is_expected.to render_template("crops/search") }
     end
   end
 
   describe "GET RSS feed" do
-    it "returns an RSS feed" do
-      get :index, format: "rss"
-      response.should be_success
-      response.should render_template("crops/index")
-      response.content_type.should eq("application/rss+xml")
+    describe "returns an RSS feed" do
+      before { get :index, format: "rss" }
+      it { is_expected.to be_success }
+      it { is_expected.to render_template("crops/index") }
+      it { expect(response.content_type).to eq("application/rss+xml") }
     end
   end
 end

From 09f82d599560e5792bc542af71762572a2dd5db3 Mon Sep 17 00:00:00 2001
From: Brenda Wallace <brenda@wallace.net.nz>
Date: Tue, 28 Nov 2017 09:30:14 +1300
Subject: [PATCH 26/29] Tidy up home/crops partial view

---
 spec/views/home/_crops.html.haml_spec.rb | 21 ++++++---------------
 1 file changed, 6 insertions(+), 15 deletions(-)

diff --git a/spec/views/home/_crops.html.haml_spec.rb b/spec/views/home/_crops.html.haml_spec.rb
index cc91976b8..254fcb2f6 100644
--- a/spec/views/home/_crops.html.haml_spec.rb
+++ b/spec/views/home/_crops.html.haml_spec.rb
@@ -1,27 +1,18 @@
 require 'rails_helper'
 
 describe 'home/_crops.html.haml', type: "view" do
-  before(:each) do
-    # we need to set up an "interesting" crop
-    @crop = FactoryBot.create(:crop)
-    (1..3).each do
-      @planting = FactoryBot.create(:planting, crop: @crop)
-    end
-    @photo = FactoryBot.create(:photo)
-    (1..3).each do
-      @crop.plantings.first.photos << @photo
-    end
-    render
-  end
-
+  let!(:crop) { FactoryBot.create(:crop, plantings: FactoryBot.create_list(:planting, 3)) }
+  let!(:photo) { FactoryBot.create(:photo, plantings: [crop.plantings.first]) }
+  let(:planting) { crop.plantings.first }
+  before(:each) { render }
   it 'shows crops section' do
     assert_select 'h2', text: 'Some of our crops'
-    assert_select "a[href='#{crop_path(@crop)}']"
+    assert_select "a[href='#{crop_path(crop)}']"
   end
 
   it 'shows plantings section' do
     assert_select 'h2', text: 'Recently planted'
-    rendered.should have_content @planting.location
+    rendered.should have_content planting.location
   end
 
   it 'shows recently added crops' do

From a62b668781aa6bcaf239e957aa26d5b66c3bfc7f Mon Sep 17 00:00:00 2001
From: Brenda Wallace <brenda@wallace.net.nz>
Date: Tue, 28 Nov 2017 09:37:44 +1300
Subject: [PATCH 27/29] Fix up duplicate order item in controller spec, and DRY

---
 app/controllers/order_items_controller.rb     | 23 ++++--
 .../order_items_controller_spec.rb            | 81 +++++++++----------
 2 files changed, 53 insertions(+), 51 deletions(-)

diff --git a/app/controllers/order_items_controller.rb b/app/controllers/order_items_controller.rb
index 0eb381a89..326cff21f 100644
--- a/app/controllers/order_items_controller.rb
+++ b/app/controllers/order_items_controller.rb
@@ -1,28 +1,35 @@
 class OrderItemsController < ApplicationController
   before_action :authenticate_member!
   load_and_authorize_resource
+  respond_to :html
+  responders :flash
 
   # POST /order_items
   def create
     if params[:order_item][:price]
       params[:order_item][:price] = params[:order_item][:price].to_f * 100 # convert to cents
     end
+
     @order_item = OrderItem.new(order_item_params)
     @order_item.order = current_member.current_order || Order.create(member_id: current_member.id)
 
-    respond_to do |format|
-      if @order_item.save
-        format.html { redirect_to @order_item.order, notice: 'Added item to your order.' }
-      else
-        errors = @order_item.errors.empty? ?
-          "There was a problem with your order." : @order_item.errors.full_messages.to_sentence
-        format.html { redirect_to shop_path, alert: errors }
-      end
+    if @order_item.save
+      redirect_to @order_item.order, notice: 'Added item to your order.'
+    else
+      redirect_to shop_path, alert: errors
     end
   end
 
   private
 
+  def errors
+    if @order_item.errors.empty?
+      "There was a problem with your order."
+    else
+      @order_item.errors.full_messages.to_sentence
+    end
+  end
+
   def order_item_params
     params.require(:order_item).permit(:order_id, :price, :product_id, :quantity)
   end
diff --git a/spec/controllers/order_items_controller_spec.rb b/spec/controllers/order_items_controller_spec.rb
index 1da3c3b8d..4de8eb98d 100644
--- a/spec/controllers/order_items_controller_spec.rb
+++ b/spec/controllers/order_items_controller_spec.rb
@@ -3,53 +3,48 @@ require 'rails_helper'
 describe OrderItemsController do
   login_member(:admin_member)
 
-  before(:each) do
-    @member = FactoryBot.create(:member)
-    sign_in @member
-    @product = FactoryBot.create(:product)
-    @order = FactoryBot.create(:order, member: @member)
-    @order_item = FactoryBot.create(:order_item,
-      order: @order,
-      product: @product,
-      price: @product.min_price)
+  let(:member) { FactoryBot.create(:member) }
+  let(:product) { FactoryBot.create(:product) }
+  let(:order) { FactoryBot.create(:order, member: member) }
+  let(:order_item) do
+    FactoryBot.create(:order_item,
+      order: order,
+      product: product,
+      price: product.min_price)
   end
 
-  describe "POST create" do
-    it "redirects to order" do
-      @order = FactoryBot.create(:order, member: @member)
-      post :create, order_item: {
-        order_id: @order.id,
-        product_id: @product.id,
-        price: @product.min_price
-      }
-      response.should redirect_to(OrderItem.last.order)
-    end
+  context 'signed in' do
+    before { sign_in member }
 
-    it 'creates an order for you' do
-      @member = FactoryBot.create(:member)
-      sign_in @member
-      @product = FactoryBot.create(:product)
-      expect {
-        post :create, order_item: {
-          product_id: @product.id,
-          price: @product.min_price
-        }
-      }.to change(Order, :count).by(1)
-      OrderItem.last.order.should be_an_instance_of Order
-    end
+    describe "POST create" do
+      describe "redirects to order" do
+        before do
+          post :create, order_item: { order_id: order.id, product_id: product.id, price: product.min_price }
+        end
+        it { expect(response).to redirect_to(OrderItem.last.order) }
+        it { expect(OrderItem.last.order).to be_an_instance_of Order }
+      end
 
-    describe "with non-int price" do
-      it "converts 3.33 to 333 cents" do
-        @order = FactoryBot.create(:order, member: @member)
-        @product = FactoryBot.create(:product, min_price: 1)
-        expect {
-          post :create, order_item: {
-            order_id: @order.id,
-            product_id: @product.id,
-            price: 3.33
-          }
-        }.to change(OrderItem, :count).by(1)
-        OrderItem.last.price.should eq 333
+      describe 'creates an order for you' do
+        it do
+          expect do
+            post :create, order_item: {
+              product_id: product.id,
+              price: product.min_price
+            }
+          end.to change(Order, :count).by(1)
+        end
+      end
+
+      describe "with non-int price" do
+        it "converts 3.33 to 333 cents" do
+          order = FactoryBot.create(:order, member: member)
+          product = FactoryBot.create(:product, min_price: 1)
+          expect do
+            post :create, order_item: { order_id: order.id, product_id: product.id, price: 3.33 }
+          end.to change(OrderItem, :count).by(1)
+          OrderItem.last.price.should eq 333
+        end
       end
     end
   end

From c8a200bf19b93a50833183e2f39b42e7eb3ffd96 Mon Sep 17 00:00:00 2001
From: Brenda Wallace <brenda@wallace.net.nz>
Date: Tue, 28 Nov 2017 20:19:43 +1300
Subject: [PATCH 28/29] Changed selector in view spec

it was having trouble finding the link
---
 spec/features/gardens_spec.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/spec/features/gardens_spec.rb b/spec/features/gardens_spec.rb
index e9a20d060..3d00d4a5f 100644
--- a/spec/features/gardens_spec.rb
+++ b/spec/features/gardens_spec.rb
@@ -58,7 +58,7 @@ feature "Planting a crop", js: true do
     end
 
     scenario "button on index to edit garden" do
-      first(".garden-info").click_link("Edit")
+      first(".garden-info").click_link("edit_garden_link")
       expect(page).to have_content 'Edit garden'
     end
   end

From 6ed7376c58b6861fce1c737fadd2a645515b2246 Mon Sep 17 00:00:00 2001
From: Brenda Wallace <brenda@wallace.net.nz>
Date: Wed, 29 Nov 2017 10:41:55 +1300
Subject: [PATCH 29/29] Name garden in spec to ensure it's ordered first

---
 spec/features/gardens_spec.rb | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/spec/features/gardens_spec.rb b/spec/features/gardens_spec.rb
index 3d00d4a5f..a299a0197 100644
--- a/spec/features/gardens_spec.rb
+++ b/spec/features/gardens_spec.rb
@@ -1,7 +1,8 @@
 require 'rails_helper'
 
 feature "Planting a crop", js: true do
-  let!(:garden) { create :garden }
+  # name is aaa to ensure it is ordered first
+  let!(:garden) { create :garden, name: 'aaa' }
   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, owner: garden.owner, garden: garden, crop: tomato }