From 2f9610a60fd2c2cfefb4c0d856f9687f48a30891 Mon Sep 17 00:00:00 2001
From: Brenda Wallace <brenda@wallace.net.nz>
Date: Thu, 4 Jan 2018 17:23:10 +1300
Subject: [PATCH] Http positional arguments in rails 5

---
 .rubocop.yml                                  |  4 --
 .../admin/orders_controller_spec.rb           |  4 +-
 spec/controllers/admin_controller_spec.rb     | 15 +++--
 spec/controllers/comments_controller_spec.rb  | 10 ++--
 spec/controllers/gardens_controller_spec.rb   | 14 ++---
 spec/controllers/harvests_controller_spec.rb  | 44 +++++++-------
 spec/controllers/likes_controller_spec.rb     |  4 +-
 spec/controllers/member_controller_spec.rb    | 16 ++---
 .../notifications_controller_spec.rb          | 14 ++---
 .../order_items_controller_spec.rb            | 16 +++--
 spec/controllers/orders_controller_spec.rb    |  8 +--
 .../photo_associations_controller_spec.rb     |  6 +-
 spec/controllers/photos_controller_spec.rb    | 58 ++++++++++++-------
 spec/controllers/places_controller_spec.rb    |  6 +-
 spec/controllers/plantings_controller_spec.rb | 20 +++----
 spec/controllers/posts_controller_spec.rb     | 14 ++---
 spec/controllers/roles_controller_spec.rb     |  2 +-
 .../scientific_names_controller_spec.rb       |  8 +--
 spec/controllers/seeds_controller_spec.rb     |  8 +--
 spec/controllers/shop_controller_spec.rb      |  4 +-
 20 files changed, 145 insertions(+), 130 deletions(-)

diff --git a/.rubocop.yml b/.rubocop.yml
index 545e1c1fa..571a090bd 100644
--- a/.rubocop.yml
+++ b/.rubocop.yml
@@ -39,10 +39,6 @@ Layout/AlignParameters:
 Metrics/LineLength:
   Max: 120
 
-# turn these back on in Rails 5
-Rails/HttpPositionalArguments: # See https://github.com/bbatsov/rubocop/issues/3629
-  Enabled: false
-
 Style/Documentation:
   Enabled: false
 
diff --git a/spec/controllers/admin/orders_controller_spec.rb b/spec/controllers/admin/orders_controller_spec.rb
index fb475b34c..207112f39 100644
--- a/spec/controllers/admin/orders_controller_spec.rb
+++ b/spec/controllers/admin/orders_controller_spec.rb
@@ -6,12 +6,12 @@ describe Admin::OrdersController do
   describe "GET search" do
     it "assigns @orders" do
       order = FactoryBot.create(:order)
-      get :search, search_by: 'order_id', search_text: order.id
+      get :search, params: { search_by: 'order_id', search_text: order.id }
       assigns(:orders).should eq([order])
     end
 
     it "sets an error message if nothing found" do
-      get :search, search_by: 'order_id', search_text: 'foo'
+      get :search, params: { search_by: 'order_id', search_text: 'foo' }
       flash[:alert].should have_text "Couldn't find order with"
     end
   end
diff --git a/spec/controllers/admin_controller_spec.rb b/spec/controllers/admin_controller_spec.rb
index 9f134bbab..5aa87e7cb 100644
--- a/spec/controllers/admin_controller_spec.rb
+++ b/spec/controllers/admin_controller_spec.rb
@@ -4,16 +4,15 @@ describe AdminController do
   login_member(:admin_member)
 
   describe "GET admin/newsletter" do
-    it 'fetches the admin newsletter page' do
-      get :newsletter
-      response.should be_success
-      response.should render_template("admin/newsletter")
+    before { get :newsletter }
+    describe 'fetches the admin newsletter page' do
+      it { expect(response).to be_success }
+      it { expect(response).to render_template("admin/newsletter") }
     end
 
-    it 'assigns @members' do
-      m = FactoryBot.create(:newsletter_recipient_member)
-      get :newsletter
-      assigns(:members).should eq [m]
+    describe 'assigns @members' do
+      let!(:m) { FactoryBot.create(:newsletter_recipient_member) }
+      it { expect(assigns(:members)).to eq [m] }
     end
   end
 end
diff --git a/spec/controllers/comments_controller_spec.rb b/spec/controllers/comments_controller_spec.rb
index 0e986cc93..6b2383b36 100644
--- a/spec/controllers/comments_controller_spec.rb
+++ b/spec/controllers/comments_controller_spec.rb
@@ -29,7 +29,7 @@ describe CommentsController do
     let(:post) { FactoryBot.create(:post) }
 
     describe "with valid params" do
-      before { get :new, post_id: post.id }
+      before { get :new, params: { post_id: post.id } }
 
       it "picks up post from params" do
         assigns(:post).should eq(post)
@@ -49,7 +49,7 @@ describe CommentsController do
 
   describe "GET edit" do
     let(:post) { FactoryBot.create(:post) }
-    before { get :edit, id: comment.to_param }
+    before { get :edit, params: { id: comment.to_param } }
 
     describe "my comment" do
       let!(:comment) { FactoryBot.create :comment, author: member, post: post }
@@ -66,7 +66,7 @@ describe CommentsController do
   end
 
   describe "PUT update" do
-    before { put :update, id: comment.to_param, comment: valid_attributes }
+    before { put :update, params: { id: comment.to_param, comment: valid_attributes } }
 
     describe "my comment" do
       let(:comment) { FactoryBot.create :comment, author: member }
@@ -74,10 +74,12 @@ describe CommentsController do
         expect(response).to redirect_to(comment.post)
       end
     end
+
     describe "not my comment" do
       let(:comment) { FactoryBot.create :comment }
       it { expect(response).not_to be_success }
     end
+
     describe "attempting to change post_id" do
       let(:post) { FactoryBot.create :post, subject: 'our post' }
       let(:other_post) { FactoryBot.create :post, subject: 'the other post' }
@@ -91,7 +93,7 @@ describe CommentsController do
   end
 
   describe "DELETE destroy" do
-    before { delete :destroy, id: comment.to_param }
+    before { delete :destroy, params: { id: comment.to_param } }
 
     describe "my comment" do
       let(:comment) { FactoryBot.create :comment, author: member }
diff --git a/spec/controllers/gardens_controller_spec.rb b/spec/controllers/gardens_controller_spec.rb
index b0ccae0b8..f661a7299 100644
--- a/spec/controllers/gardens_controller_spec.rb
+++ b/spec/controllers/gardens_controller_spec.rb
@@ -7,7 +7,7 @@ RSpec.describe GardensController, type: :controller do
   context "when not signed in" do
     let(:garden) { double('garden') }
     describe 'GET new' do
-      before { get :new, id: garden.to_param }
+      before { get :new, params: { id: garden.to_param } }
       it { expect(response).to redirect_to(new_member_session_path) }
     end
     describe 'PUT create' do
@@ -25,15 +25,15 @@ RSpec.describe GardensController, type: :controller do
         expect(garden).not_to receive(:destroy)
       end
       describe 'GET edit' do
-        before { get :edit, id: garden.to_param }
+        before { get :edit, params: { id: garden.to_param } }
         it { expect(response).to redirect_to(new_member_session_path) }
       end
       describe 'POST update' do
-        before { post :update, id: garden.to_param, garden: valid_params }
+        before { post :update, params: { id: garden.to_param, garden: valid_params } }
         it { expect(response).to redirect_to(new_member_session_path) }
       end
       describe 'DELETE' do
-        before { delete :destroy, id: garden.to_param, params: { garden: valid_params } }
+        before { delete :destroy, params: { id: garden.to_param, params: { garden: valid_params } } }
         it { expect(response).to redirect_to(new_member_session_path) }
       end
     end
@@ -55,15 +55,15 @@ RSpec.describe GardensController, type: :controller do
       end
 
       describe 'GET edit' do
-        before { get :edit, id: not_my_garden.to_param }
+        before { get :edit, params: { id: not_my_garden.to_param } }
         it { expect(response).to redirect_to(root_path) }
       end
       describe 'POST update' do
-        before { post :update, id: not_my_garden.to_param, garden: valid_params }
+        before { post :update, params: { id: not_my_garden.to_param, garden: valid_params } }
         it { expect(response).to redirect_to(root_path) }
       end
       describe 'DELETE' do
-        before { delete :destroy, id: not_my_garden.to_param, params: { garden: valid_params } }
+        before { delete :destroy, params: { id: not_my_garden.to_param, params: { garden: valid_params } } }
         it { expect(response).to redirect_to(root_path) }
       end
     end
diff --git a/spec/controllers/harvests_controller_spec.rb b/spec/controllers/harvests_controller_spec.rb
index 8a2edb56b..fca0bb840 100644
--- a/spec/controllers/harvests_controller_spec.rb
+++ b/spec/controllers/harvests_controller_spec.rb
@@ -21,18 +21,18 @@ describe HarvestsController do
     let(:harvest2) { FactoryBot.create(:harvest, owner_id: member2.id, crop_id: maize.id) }
 
     describe "assigns all harvests as @harvests" do
-      before { get :index, {} }
+      before { get :index, params: {} }
       it { assigns(:harvests).should =~ [harvest1, harvest2] }
     end
 
     describe "picks up owner from params and shows owner's harvests only" do
-      before { get :index, owner: member1.slug }
+      before { get :index, params: { owner: member1.slug } }
       it { expect(assigns(:owner)).to eq member1 }
       it { expect(assigns(:harvests)).to eq [harvest1] }
     end
 
     describe "picks up crop from params and shows the harvests for the crop only" do
-      before { get :index, crop: maize.name }
+      before { get :index, params: { crop: maize.name } }
       it { expect(assigns(:crop)).to eq maize }
       it { expect(assigns(:harvests)).to eq [harvest2] }
     end
@@ -46,13 +46,13 @@ describe HarvestsController do
   describe "GET show" do
     let(:harvest) { Harvest.create! valid_attributes }
     describe "assigns the requested harvest as @harvest" do
-      before { get :show, id: harvest.to_param }
+      before { get :show, params: { id: harvest.to_param } }
       it { expect(assigns(:harvest)).to eq(harvest) }
     end
   end
 
   describe "GET new" do
-    before { get :new, {} }
+    before { get :new, params: {} }
 
     describe "assigns a new harvest as @harvest" do
       it { expect(assigns(:harvest)).to be_a_new(Harvest) }
@@ -66,7 +66,7 @@ describe HarvestsController do
   describe "GET edit" do
     let(:harvest) { Harvest.create! valid_attributes }
     describe "assigns the requested harvest as @harvest" do
-      before { get :edit, id: harvest.to_param }
+      before { get :edit, params: { id: harvest.to_param } }
       it { expect(assigns(:harvest)).to eq(harvest) }
     end
   end
@@ -75,24 +75,24 @@ describe HarvestsController do
     describe "with valid params" do
       it "creates a new Harvest" do
         expect do
-          post :create, harvest: valid_attributes
+          post :create, params: { harvest: valid_attributes }
         end.to change(Harvest, :count).by(1)
       end
 
       it "assigns a newly created harvest as @harvest" do
-        post :create, harvest: valid_attributes
+        post :create, params: { harvest: valid_attributes }
         assigns(:harvest).should be_a(Harvest)
         assigns(:harvest).should be_persisted
       end
 
       it "redirects to the created harvest" do
-        post :create, harvest: valid_attributes
+        post :create, params: { harvest: valid_attributes }
         response.should redirect_to(Harvest.last)
       end
 
       describe "links to planting" do
         let(:planting) { FactoryBot.create(:planting, owner_id: member.id, garden: member.gardens.first) }
-        before { post :create, harvest: valid_attributes.merge(planting_id: planting.id) }
+        before { post :create, params: { harvest: valid_attributes.merge(planting_id: planting.id) } }
         it { expect(Harvest.last.planting.id).to eq(planting.id) }
       end
     end
@@ -101,13 +101,13 @@ describe HarvestsController do
       it "assigns a newly created but unsaved harvest as @harvest" do
         # Trigger the behavior that occurs when invalid params are submitted
         Harvest.any_instance.stub(:save).and_return(false)
-        post :create, harvest: { "crop_id" => "invalid value" }
+        post :create, params: { harvest: { "crop_id" => "invalid value" } }
         assigns(:harvest).should be_a_new(Harvest)
       end
 
       it "re-renders the 'new' template" do
         # Trigger the behavior that occurs when invalid params are submitted
-        post :create, harvest: { "crop_id" => "invalid value" }
+        post :create, params: { harvest: { "crop_id" => "invalid value" } }
         response.should render_template("new")
       end
     end
@@ -119,7 +119,7 @@ describe HarvestsController do
       describe "does not save planting_id" do
         before do
           allow(Harvest).to receive(:new).and_return(harvest)
-          post :create, harvest: valid_attributes.merge(planting_id: not_my_planting.id)
+          post :create, params: { harvest: valid_attributes.merge(planting_id: not_my_planting.id) }
         end
         it { expect(harvest.planting_id).not_to eq(not_my_planting.id) }
       end
@@ -135,18 +135,18 @@ describe HarvestsController do
         # receives the :update message with whatever params are
         # submitted in the request.
         Harvest.any_instance.should_receive(:update).with("crop_id" => "1", "owner_id": member.id)
-        put :update, id: harvest.to_param, harvest: { "crop_id" => "1" }
+        put :update, params: { id: harvest.to_param, harvest: { "crop_id" => "1" } }
       end
 
       it "assigns the requested harvest as @harvest" do
         harvest = Harvest.create! valid_attributes
-        put :update, id: harvest.to_param, harvest: valid_attributes
+        put :update, params: { id: harvest.to_param, harvest: valid_attributes }
         assigns(:harvest).should eq(harvest)
       end
 
       it "redirects to the harvest" do
         harvest = Harvest.create! valid_attributes
-        put :update, id: harvest.to_param, harvest: valid_attributes
+        put :update, params: { id: harvest.to_param, harvest: valid_attributes }
         response.should redirect_to(harvest)
       end
     end
@@ -156,13 +156,13 @@ describe HarvestsController do
         harvest = Harvest.create! valid_attributes
         # Trigger the behavior that occurs when invalid params are submitted
         Harvest.any_instance.stub(:save).and_return(false)
-        put :update, id: harvest.to_param, harvest: { "crop_id" => "invalid value" }
+        put :update, params: { id: harvest.to_param, harvest: { "crop_id" => "invalid value" } }
         assigns(:harvest).should eq(harvest)
       end
 
       it "re-renders the 'edit' template" do
         harvest = Harvest.create! valid_attributes
-        put :update, id: harvest.to_param, harvest: { "crop_id" => "invalid value" }
+        put :update, params: { id: harvest.to_param, harvest: { "crop_id" => "invalid value" } }
         response.should render_template("edit")
       end
     end
@@ -172,8 +172,8 @@ describe HarvestsController do
       let(:harvest) { FactoryBot.create(:harvest) }
       describe "does not save planting_id" do
         before do
-          put :update, id: harvest.to_param,
-                       harvest: valid_attributes.merge(planting_id: not_my_planting.id)
+          put :update, params: { id: harvest.to_param,
+                                 harvest: valid_attributes.merge(planting_id: not_my_planting.id) }
         end
         it { expect(harvest.planting_id).to eq(nil) }
       end
@@ -184,13 +184,13 @@ describe HarvestsController do
     it "destroys the requested harvest" do
       harvest = Harvest.create! valid_attributes
       expect do
-        delete :destroy, id: harvest.to_param
+        delete :destroy, params: { id: harvest.to_param }
       end.to change(Harvest, :count).by(-1)
     end
 
     it "redirects to the harvests list" do
       harvest = Harvest.create! valid_attributes
-      delete :destroy, id: harvest.to_param
+      delete :destroy, params: { id: harvest.to_param }
       response.should redirect_to(harvests_url)
     end
   end
diff --git a/spec/controllers/likes_controller_spec.rb b/spec/controllers/likes_controller_spec.rb
index 6f43760c1..59e0ddd89 100644
--- a/spec/controllers/likes_controller_spec.rb
+++ b/spec/controllers/likes_controller_spec.rb
@@ -10,7 +10,7 @@ describe LikesController do
 
   describe "POST create" do
     it { expect(response.content_type).to eq "application/json" }
-    before { post :create, post_id: blogpost.id, format: :json }
+    before { post :create, params: { post_id: blogpost.id, format: :json } }
     it { expect(Like.last.likeable_id).to eq(blogpost.id) }
     it { expect(Like.last.likeable_type).to eq('Post') }
     it { JSON.parse(response.body)["description"] == "1 like" }
@@ -25,7 +25,7 @@ describe LikesController do
   end
 
   describe "DELETE destroy" do
-    before { delete :destroy, id: like.id, format: :json }
+    before { delete :destroy, params: { id: like.id, format: :json } }
     it { expect(response.content_type).to eq "application/json" }
 
     describe "un-liking something i liked before" do
diff --git a/spec/controllers/member_controller_spec.rb b/spec/controllers/member_controller_spec.rb
index b04bcd799..dada0b013 100644
--- a/spec/controllers/member_controller_spec.rb
+++ b/spec/controllers/member_controller_spec.rb
@@ -10,7 +10,7 @@ describe MembersController do
 
   describe "GET index" do
     it "assigns only confirmed members as @members" do
-      get :index, {}
+      get :index, params: {}
       assigns(:members).should eq([@member])
     end
   end
@@ -24,38 +24,38 @@ describe MembersController do
 
   describe "GET show" do
     it "provides JSON for member profile" do
-      get :show, id: @member.id, format: 'json'
+      get :show, params: { id: @member.id }, format: 'json'
       response.should be_success
     end
 
     it "assigns @posts with the member's posts" do
-      get :show, id: @member.id
+      get :show, params: { id: @member.id }
       assigns(:posts).should eq(@posts)
     end
 
     it "assigns @twitter_auth" do
-      get :show, id: @member.id
+      get :show, params: { id: @member.id }
       assigns(:twitter_auth).should eq(@twitter_auth)
     end
 
     it "assigns @flickr_auth" do
-      get :show, id: @member.id
+      get :show, params: { id: @member.id }
       assigns(:flickr_auth).should eq(@flickr_auth)
     end
 
     it "doesn't show completely nonsense members" do
-      lambda { get :show, id: 9999 }.should raise_error(ActiveRecord::RecordNotFound)
+      lambda { get :show, params: { id: 9999 } }.should raise_error(ActiveRecord::RecordNotFound)
     end
 
     it "doesn't show unconfirmed members" do
       @member2 = FactoryBot.create(:unconfirmed_member)
-      lambda { get :show, id: @member2.id }.should raise_error(ActiveRecord::RecordNotFound)
+      lambda { get :show, params: { id: @member2.id } }.should raise_error(ActiveRecord::RecordNotFound)
     end
   end
 
   describe "GET member's RSS feed" do
     it "returns an RSS feed" do
-      get :show, id: @member.to_param, format: "rss"
+      get :show, params: { id: @member.to_param }, format: "rss"
       response.should be_success
       response.should render_template("members/show")
       response.content_type.should eq("application/rss+xml")
diff --git a/spec/controllers/notifications_controller_spec.rb b/spec/controllers/notifications_controller_spec.rb
index fd6f40aa1..a7ed81ae5 100644
--- a/spec/controllers/notifications_controller_spec.rb
+++ b/spec/controllers/notifications_controller_spec.rb
@@ -31,7 +31,7 @@ describe NotificationsController do
   describe "GET index" do
     it "assigns all notifications as @notifications" do
       notification = FactoryBot.create(:notification, recipient_id: subject.current_member.id)
-      get :index, {}
+      get :index, params: {}
       assigns(:notifications).should eq([notification])
     end
   end
@@ -39,14 +39,14 @@ describe NotificationsController do
   describe "GET show" do
     it "assigns the requested notification as @notification" do
       notification = FactoryBot.create(:notification, recipient_id: subject.current_member.id)
-      get :show, id: notification.to_param
+      get :show, params: { id: notification.to_param }
       assigns(:notification).should eq(notification)
     end
 
     it "assigns the reply link for a post comment" do
       notification = FactoryBot.create(:notification, recipient_id: subject.current_member.id)
 
-      get :show, id: notification.to_param
+      get :show, params: { id: notification.to_param }
       assigns(:reply_link).should_not be_nil
       assigns(:reply_link).should eq new_comment_url(
         post_id: notification.post.id
@@ -55,7 +55,7 @@ describe NotificationsController do
 
     it "marks notifications as read" do
       notification = FactoryBot.create(:notification, recipient_id: subject.current_member.id)
-      get :show, id: notification.to_param
+      get :show, params: { id: notification.to_param }
       # we need to fetch it from the db again, can't test against the old one
       n = Notification.find(notification.id)
       n.read.should eq true
@@ -65,7 +65,7 @@ describe NotificationsController do
   describe "GET reply" do
     it "marks notifications as read" do
       notification = FactoryBot.create(:notification, recipient_id: subject.current_member.id)
-      get :reply, id: notification.to_param
+      get :reply, params: { id: notification.to_param }
       # we need to fetch it from the db again, can't test against the old one
       n = Notification.find(notification.id)
       n.read.should eq true
@@ -75,7 +75,7 @@ describe NotificationsController do
   describe "GET new" do
     it "assigns a recipient" do
       @recipient = FactoryBot.create(:member)
-      get :new, recipient_id: @recipient.id
+      get :new, params: { recipient_id: @recipient.id }
       assigns(:recipient).should be_an_instance_of(Member)
     end
   end
@@ -84,7 +84,7 @@ describe NotificationsController do
     describe "with valid params" do
       it "redirects to the recipient's profile" do
         @recipient = FactoryBot.create(:member)
-        post :create, notification: { recipient_id: @recipient.id, subject: 'foo' }
+        post :create, params: { notification: { recipient_id: @recipient.id, subject: 'foo' } }
         response.should redirect_to(notifications_path)
       end
     end
diff --git a/spec/controllers/order_items_controller_spec.rb b/spec/controllers/order_items_controller_spec.rb
index 4de8eb98d..9064a0f2a 100644
--- a/spec/controllers/order_items_controller_spec.rb
+++ b/spec/controllers/order_items_controller_spec.rb
@@ -19,7 +19,9 @@ describe OrderItemsController do
     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 }
+          post :create, params: {
+            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 }
@@ -28,9 +30,11 @@ describe OrderItemsController do
       describe 'creates an order for you' do
         it do
           expect do
-            post :create, order_item: {
-              product_id: product.id,
-              price: product.min_price
+            post :create, params: {
+              order_item: {
+                product_id: product.id,
+                price: product.min_price
+              }
             }
           end.to change(Order, :count).by(1)
         end
@@ -41,7 +45,9 @@ describe OrderItemsController 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 }
+            post :create, params: {
+              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
diff --git a/spec/controllers/orders_controller_spec.rb b/spec/controllers/orders_controller_spec.rb
index 3c6631d8c..a36365aad 100644
--- a/spec/controllers/orders_controller_spec.rb
+++ b/spec/controllers/orders_controller_spec.rb
@@ -16,7 +16,7 @@ describe OrdersController do
       member = FactoryBot.create(:member)
       sign_in member
       order = Order.create!(member_id: member.id)
-      get :checkout, id: order.to_param, referral_code: 'FOOBAR'
+      get :checkout, params: { id: order.to_param, referral_code: 'FOOBAR' }
       order.reload
       order.referral_code.should eq 'FOOBAR'
     end
@@ -25,7 +25,7 @@ describe OrdersController do
       member = FactoryBot.create(:member)
       sign_in member
       order = Order.create!(member_id: member.id)
-      get :checkout, id: order.to_param
+      get :checkout, params: { id: order.to_param }
       response.status.should eq 302
       response.redirect_url.should match(/paypal\.com/)
     end
@@ -36,7 +36,7 @@ describe OrdersController do
       member = FactoryBot.create(:member)
       sign_in member
       order = Order.create!(member_id: member.id)
-      get :complete, id: order.to_param
+      get :complete, params: { id: order.to_param }
       assigns(:order).should eq(order)
     end
   end
@@ -46,7 +46,7 @@ describe OrdersController do
       member = FactoryBot.create(:member)
       sign_in member
       order = Order.create!(member_id: member.id)
-      delete :destroy, id: order.id
+      delete :destroy, params: { id: order.id }
       response.should redirect_to(shop_url)
     end
   end
diff --git a/spec/controllers/photo_associations_controller_spec.rb b/spec/controllers/photo_associations_controller_spec.rb
index 42e0ccc37..65a0dd37d 100644
--- a/spec/controllers/photo_associations_controller_spec.rb
+++ b/spec/controllers/photo_associations_controller_spec.rb
@@ -19,7 +19,7 @@ describe PhotoAssociationsController do
       let(:photo) { FactoryBot.create :photo, owner: member }
 
       it "removes link" do
-        expect { delete :destroy, valid_params }.to change { photo.harvests.count }.by(-1)
+        expect { delete :destroy, params: valid_params }.to change { photo.harvests.count }.by(-1)
       end
     end
 
@@ -29,13 +29,13 @@ describe PhotoAssociationsController do
       it do
         expect do
           begin
-            delete :destroy, valid_params
+            delete :destroy, params: valid_params
           rescue
             nil
           end
         end.not_to change(photo.harvests, :count)
       end
-      it { expect { delete :destroy, valid_params }.to raise_error(ActiveRecord::RecordNotFound) }
+      it { expect { delete :destroy, params: valid_params }.to raise_error(ActiveRecord::RecordNotFound) }
     end
   end
 end
diff --git a/spec/controllers/photos_controller_spec.rb b/spec/controllers/photos_controller_spec.rb
index 3c4f9dacb..b001141ab 100644
--- a/spec/controllers/photos_controller_spec.rb
+++ b/spec/controllers/photos_controller_spec.rb
@@ -38,7 +38,7 @@ describe PhotosController do
     end
 
     describe "planting photos" do
-      before(:each) { get :new, type: "planting", id: planting.id }
+      before(:each) { get :new, params: { type: "planting", id: planting.id } }
       it { assigns(:flickr_auth).should be_an_instance_of(Authentication) }
       it { assigns(:item).should eq planting }
       it { expect(flash[:alert]).not_to be_present }
@@ -46,7 +46,7 @@ describe PhotosController do
     end
 
     describe "harvest photos" do
-      before { get :new, type: "harvest", id: harvest.id }
+      before { get :new, params: { type: "harvest", id: harvest.id } }
       it { assigns(:item).should eq harvest }
       it { expect(flash[:alert]).not_to be_present }
     end
@@ -76,15 +76,22 @@ describe PhotosController do
     describe "with valid params" do
       before { controller.stub(:current_member) { member } }
       it "attaches the photo to a planting" do
-        post :create, photo: { flickr_photo_id: photo.flickr_photo_id }, type: "planting", id: planting.id
+        post :create, params: {
+          photo: { flickr_photo_id: photo.flickr_photo_id },
+          type: "planting", id: planting.id
+        }
         expect(flash[:alert]).not_to be_present
         Photo.last.plantings.first.should eq planting
       end
 
       describe "doesn't attach a photo to a planting twice" do
         before do
-          post :create, photo: { flickr_photo_id: photo.flickr_photo_id }, type: "planting", id: planting.id
-          post :create, photo: { flickr_photo_id: photo.flickr_photo_id }, type: "planting", id: planting.id
+          post :create, params: {
+            photo: { flickr_photo_id: photo.flickr_photo_id }, type: "planting", id: planting.id
+          }
+          post :create, params: {
+            photo: { flickr_photo_id: photo.flickr_photo_id }, type: "planting", id: planting.id
+          }
         end
         it { expect(flash[:alert]).not_to be_present }
         it { expect(Photo.last.plantings.size).to eq 1 }
@@ -97,8 +104,12 @@ describe PhotosController do
       end
 
       it "doesn't attach a photo to a harvest twice" do
-        post :create, photo: { flickr_photo_id: photo.flickr_photo_id }, type: "harvest", id: harvest.id
-        post :create, photo: { flickr_photo_id: photo.flickr_photo_id }, type: "harvest", id: harvest.id
+        post :create, params: {
+          photo: { flickr_photo_id: photo.flickr_photo_id }, type: "harvest", id: harvest.id
+        }
+        post :create, params: {
+          photo: { flickr_photo_id: photo.flickr_photo_id }, type: "harvest", id: harvest.id
+        }
         expect(flash[:alert]).not_to be_present
         Photo.last.harvests.size.should eq 1
       end
@@ -106,20 +117,19 @@ describe PhotosController do
       it "doesn't attach photo to a comment" do
         comment = FactoryBot.create(:comment)
         expect do
-          post :create, photo: { flickr_photo_id: photo.flickr_photo_id }, type: "comment", id: comment.id
+          post :create, params: {
+            photo: { flickr_photo_id: photo.flickr_photo_id }, type: "comment", id: comment.id
+          }
         end.to raise_error
       end
     end
 
     describe "for the second time" do
       let(:planting) { FactoryBot.create :planting, owner: member }
+      let(:valid_params) { { photo: { flickr_photo_id: 1 }, id: planting.id, type: 'planting' } }
       it "does not add a photo twice" do
-        expect do
-          post :create, photo: { flickr_photo_id: 1 }, id: planting.id, type: 'planting'
-        end.to change(Photo, :count).by(1)
-        expect do
-          post :create, photo: { flickr_photo_id: 1 }, id: planting.id, type: 'planting'
-        end.to change(Photo, :count).by(0)
+        expect { post :create, params: valid_params }.to change(Photo, :count).by(1)
+        expect { post :create, params: valid_params }.not_to change(Photo, :count)
       end
     end
 
@@ -128,15 +138,15 @@ describe PhotosController do
       it "creates the planting/photo link" do
         planting = FactoryBot.create(:planting, garden: garden, owner: member)
         photo = FactoryBot.create(:photo, owner: member)
-        post :create, photo: { flickr_photo_id: photo.flickr_photo_id }, type: "planting", id: planting.id
+        post :create, params: { photo: { flickr_photo_id: photo.flickr_photo_id }, type: "planting", id: planting.id }
         expect(flash[:alert]).not_to be_present
-        Photo.last.plantings.first.should eq planting
+        expect(Photo.last.plantings.first).to eq planting
       end
 
-      it "creates the harvest/photo link" do
-        post :create, photo: { flickr_photo_id: photo.flickr_photo_id }, type: "harvest", id: harvest.id
-        expect(flash[:alert]).not_to be_present
-        Photo.last.harvests.first.should eq harvest
+      describe "creates the harvest/photo link" do
+        before { post :create, photo: { flickr_photo_id: photo.flickr_photo_id }, type: "harvest", id: harvest.id }
+        it { expect(flash[:alert]).not_to be_present }
+        it { expect(Photo.last.harvests.first).to eq harvest }
       end
     end
 
@@ -146,7 +156,9 @@ describe PhotosController do
         # members will be auto-created, and different
         another_planting = FactoryBot.create(:planting)
         expect do
-          post :create, photo: { flickr_photo_id: photo.flickr_photo_id }, type: "planting", id: another_planting.id
+          post :create, params: {
+            photo: { flickr_photo_id: photo.flickr_photo_id }, type: "planting", id: another_planting.id
+          }
         end.to raise_error(ActiveRecord::RecordNotFound)
         Photo.last.plantings.first.should_not eq another_planting
       end
@@ -155,7 +167,9 @@ describe PhotosController do
         # members will be auto-created, and different
         another_harvest = FactoryBot.create(:harvest)
         expect do
-          post :create, photo: { flickr_photo_id: photo.flickr_photo_id }, type: "harvest", id: another_harvest.id
+          post :create, params: {
+            photo: { flickr_photo_id: photo.flickr_photo_id }, type: "harvest", id: another_harvest.id
+          }
         end.to raise_error(ActiveRecord::RecordNotFound)
         Photo.last.harvests.first.should_not eq another_harvest
       end
diff --git a/spec/controllers/places_controller_spec.rb b/spec/controllers/places_controller_spec.rb
index 8b6cb0a31..d235b98c9 100644
--- a/spec/controllers/places_controller_spec.rb
+++ b/spec/controllers/places_controller_spec.rb
@@ -12,19 +12,19 @@ describe PlacesController do
     end
 
     it "assigns place name" do
-      get :show, place: @member_london.location
+      get :show, params: { place: @member_london.location }
       assigns(:place).should eq @member_london.location
     end
 
     it "assigns nearby members" do
-      get :show, place: @member_london.location
+      get :show, params: { place: @member_london.location }
       assigns(:nearby_members).should eq [@member_london, @member_south_pole]
     end
   end
 
   describe "GET search" do
     it "redirects to the new place" do
-      get :search, new_place: "foo"
+      get :search, params: { new_place: "foo" }
       response.should redirect_to place_path("foo")
     end
   end
diff --git a/spec/controllers/plantings_controller_spec.rb b/spec/controllers/plantings_controller_spec.rb
index dd1340d44..c1e4cd72b 100644
--- a/spec/controllers/plantings_controller_spec.rb
+++ b/spec/controllers/plantings_controller_spec.rb
@@ -24,13 +24,13 @@ describe PlantingsController do
     end
 
     describe "picks up owner from params and shows owner's plantings only" do
-      before { get :index, owner: member1.slug }
+      before { get :index, params: { owner: member1.slug } }
       it { expect(assigns(:owner)).to eq member1 }
       it { expect(assigns(:plantings)).to eq [planting1] }
     end
 
     describe "picks up crop from params and shows the plantings for the crop only" do
-      before { get :index, crop: maize.name }
+      before { get :index, params: { crop: maize.name } }
       it { expect(assigns(:crop)).to eq maize }
       it { expect(assigns(:plantings)).to eq [planting2] }
     end
@@ -39,44 +39,44 @@ describe PlantingsController do
   describe "GET new" do
     describe "picks up crop from params" do
       let(:crop) { FactoryBot.create(:crop) }
-      before { get :new, crop_id: crop.id }
+      before { get :new, params: { crop_id: crop.id } }
       it { expect(assigns(:crop)).to eq(crop) }
     end
 
     describe "doesn't die if no crop specified" do
-      before { get :new, {} }
+      before { get :new, params: {} }
       it { expect(assigns(:crop)).to be_a_new(Crop) }
     end
 
     describe "picks up member's garden from params" do
       let(:garden) { FactoryBot.create(:garden, owner: member) }
-      before { get :new, garden_id: garden.id }
+      before { get :new, params: { garden_id: garden.id } }
       it { expect(assigns(:garden)).to eq(garden) }
     end
 
     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 }
+      before { get :new, params: { garden_id: garden.id } }
       it { expect(assigns(:garden)).not_to eq(garden) }
     end
 
     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 }
+      before { get :new, params: { crop_id: crop.id } }
       it { expect(assigns(:crop)).not_to eq(crop) }
     end
 
     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 }
+      before { get :new, params: { crop_id: crop.id } }
       it { expect(assigns(:crop)).not_to eq(crop) }
     end
 
     describe "doesn't die if no garden specified" do
-      before { get :new, {} }
+      before { get :new, params: {} }
       it { expect(assigns(:garden)).to be_a_new(Garden) }
     end
 
@@ -86,7 +86,7 @@ describe PlantingsController do
     end
 
     describe "sets the owner automatically" do
-      before { post :create, planting: valid_attributes }
+      before { post :create, params: { planting: valid_attributes } }
       it { expect(assigns(:planting).owner).to eq subject.current_member }
     end
   end
diff --git a/spec/controllers/posts_controller_spec.rb b/spec/controllers/posts_controller_spec.rb
index cbfe8b46c..2d7aba1ad 100644
--- a/spec/controllers/posts_controller_spec.rb
+++ b/spec/controllers/posts_controller_spec.rb
@@ -11,19 +11,19 @@ describe PostsController do
   describe "GET RSS feed" do
     it "returns an RSS feed" do
       get :index, format: "rss"
-      response.should be_success
-      response.should render_template("posts/index")
-      response.content_type.should eq("application/rss+xml")
+      expect(response).to be_success
+      expect(response).to render_template("posts/index")
+      expect(response).to.should eq("application/rss+xml")
     end
   end
 
   describe "GET RSS feed for individual post" do
     it "returns an RSS feed" do
       post = Post.create! valid_attributes
-      get :show, format: "rss", id: post.slug
-      response.should be_success
-      response.should render_template("posts/show")
-      response.content_type.should eq("application/rss+xml")
+      get :show, format: "rss", params: { id: post.slug }
+      expect(response).to be_success
+      expect(response).to render_template("posts/show")
+      expect(response).to.should eq("application/rss+xml")
     end
   end
 end
diff --git a/spec/controllers/roles_controller_spec.rb b/spec/controllers/roles_controller_spec.rb
index 9bf0ea1b5..17fe48dfe 100644
--- a/spec/controllers/roles_controller_spec.rb
+++ b/spec/controllers/roles_controller_spec.rb
@@ -10,7 +10,7 @@ describe RolesController do
   describe "GET index" do
     it "assigns all roles as @roles" do
       role = Role.create! valid_attributes
-      get :index, {}
+      get :index, params: {}
       # note that admin role exists because of login_admin_member
       assigns(:roles).should eq([Role.find_by(name: 'admin'), role])
     end
diff --git a/spec/controllers/scientific_names_controller_spec.rb b/spec/controllers/scientific_names_controller_spec.rb
index 412f53224..a56735454 100644
--- a/spec/controllers/scientific_names_controller_spec.rb
+++ b/spec/controllers/scientific_names_controller_spec.rb
@@ -3,17 +3,15 @@ require 'rails_helper'
 describe ScientificNamesController do
   login_member(:crop_wrangling_member)
 
-  before(:each) do
-    @crop = FactoryBot.create(:tomato)
-  end
+  let!(:crop) { FactoryBot.create(:tomato) }
 
   def valid_attributes
-    { name: 'Solanum lycopersicum', crop_id: @crop.id }
+    { name: 'Solanum lycopersicum', crop_id: crop.id }
   end
 
   describe "GET new" do
     it "assigns crop if specified" do
-      get :new, crop_id: 1
+      get :new, params: { crop_id: crop.id }
       assigns(:crop).should be_an_instance_of Crop
     end
   end
diff --git a/spec/controllers/seeds_controller_spec.rb b/spec/controllers/seeds_controller_spec.rb
index c2fd699eb..123516e43 100644
--- a/spec/controllers/seeds_controller_spec.rb
+++ b/spec/controllers/seeds_controller_spec.rb
@@ -2,10 +2,10 @@ require 'rails_helper'
 
 describe SeedsController do
   describe "GET index" do
-    it "picks up owner from params" do
-      owner = FactoryBot.create(:member)
-      get :index, owner: owner.slug
-      assigns(:owner).should eq(owner)
+    let(:owner) { FactoryBot.create(:member) }
+    describe "picks up owner from params" do
+      before { get :index, params: { owner: owner.slug } }
+      it { expect(assigns(:owner)).to eq(owner) }
     end
   end
 end
diff --git a/spec/controllers/shop_controller_spec.rb b/spec/controllers/shop_controller_spec.rb
index c25039d2d..c3fb1e3c0 100644
--- a/spec/controllers/shop_controller_spec.rb
+++ b/spec/controllers/shop_controller_spec.rb
@@ -6,7 +6,7 @@ describe ShopController do
 
   describe "GET index" do
     describe 'not logged in' do
-      before { get :index, {} }
+      before { get :index, params: {} }
 
       describe "assigns all products as @products ordered by name" do
         it { expect(assigns(:products)).to eq([product1, product2]) }
@@ -26,7 +26,7 @@ describe ShopController do
         let!(:order) { FactoryBot.create(:order, member: member) }
         before do
           sign_in member
-          get :index, {}
+          get :index, params: {}
         end
         it { expect(assigns(:order)).to eq order }
       end