Use params keyword with ActionController::TestCase

Largely fixes #905.  Remaining warnings are from a helper class.

Saves hundreds of lines of deprecation warnings per CI run.

spec/controllers/catalog_controller_spec.rb

@@ -35,34 +35,32 @@
 
     context 'Searching all works' do
       it 'returns all the works' do
-        get 'index', 'f' => { 'generic_type_sim' => 'Work' }
+        get 'index', params: { 'f' => { 'generic_type_sim' => 'Work' } }
         expect(response).to be_successful
         expect(assigns(:document_list).count).to eq 2
-        [work1.id, work2.id].each do |work_id|
-          expect(assigns(:document_list).map(&:id)).to include(work_id)
-        end
+        expect(assigns(:document_list).map(&:id)).to include(work1.id, work2.id)
       end
     end
 
     context 'Searching all collections' do
       it 'returns all the works' do
-        get 'index', 'f' => { 'generic_type_sim' => 'Collection' }
+        get 'index', params: { 'f' => { 'generic_type_sim' => 'Collection' } }
         expect(response).to be_successful
         expect(assigns(:document_list).map(&:id)).to eq [collection.id]
       end
     end
 
     context 'searching just my works' do
       it 'returns just my works' do
-        get 'index', works: 'mine', 'f' => { 'generic_type_sim' => 'Work' }
+        get 'index', params: { works: 'mine', 'f' => { 'generic_type_sim' => 'Work' } }
         expect(response).to be_successful
         expect(assigns(:document_list).map(&:id)).to eq [work1.id]
       end
     end
 
     context 'searching for one kind of work' do
       it 'returns just the specified type' do
-        get 'index', 'f' => { 'human_readable_type_sim' => 'Generic Work' }
+        get 'index', params: { 'f' => { 'human_readable_type_sim' => 'Generic Work' } }
         expect(response).to be_successful
         expect(assigns(:document_list).map(&:id)).to include(work1.id, work2.id)
       end
@@ -72,7 +70,7 @@
       let!(:work) { FactoryGirl.create(:generic_work, user: user, title: ["All my #{srand}"]) }
       render_views
       it 'returns json' do
-        xhr :get, :index, format: :json, q: work.title.first
+        get :index, xhr: true, params: { format: :json, q: work.title.first }
         json = JSON.parse(response.body)
         # Grab the doc corresponding to work and inspect the json
         work_json = json['docs'].first
@@ -95,17 +93,17 @@
 
     context 'searching all works' do
       it "returns other users' private works" do
-        get 'index', 'f' => { 'generic_type_sim' => 'Work' }
+        get 'index', params: { 'f' => { 'generic_type_sim' => 'Work' } }
         expect(response).to be_successful
         expect(assigns(:document_list).map(&:id)).to include(work1.id)
       end
       it "returns other users' embargoed works" do
-        get 'index', 'f' => { 'generic_type_sim' => 'Work' }
+        get 'index', params: { 'f' => { 'generic_type_sim' => 'Work' } }
         expect(response).to be_successful
         expect(assigns(:document_list).map(&:id)).to include(work2.id)
       end
       it "returns other users' private collections" do
-        get 'index', 'f' => { 'generic_type_sim' => 'Collection' }
+        get 'index', params: { 'f' => { 'generic_type_sim' => 'Collection' } }
         expect(response).to be_successful
         expect(assigns(:document_list).map(&:id)).to include(collection.id)
       end

spec/controllers/curation_concerns/classify_concerns_controller_spec.rb

@@ -19,7 +19,7 @@
   describe '#create' do
     context 'without logging in' do
       it 'redirect to login page if user is not logged in' do
-        post :create, classify: { curation_concern_type: 'GenericWork' }
+        post :create, params: { classify: { curation_concern_type: 'GenericWork' } }
         expect(response).to redirect_to(main_app.user_session_path)
       end
     end
@@ -34,7 +34,7 @@
       let(:new_curation_concern_generic_work_path) { '/stub/path' }
 
       it 'requires authentication' do
-        post :create, classify_concern: { curation_concern_type: 'GenericWork' }
+        post :create, params: { classify_concern: { curation_concern_type: 'GenericWork' } }
         expect(response).to redirect_to(main_app.new_curation_concerns_generic_work_path)
       end
     end

spec/controllers/curation_concerns/collections_controller_spec.rb

@@ -25,9 +25,7 @@
   let(:collection) { create(:collection, title: ['Collection Title'], user: user) }
 
   describe '#new' do
-    before do
-      sign_in user
-    end
+    before { sign_in user }
 
     it 'assigns @collection' do
       get :new
@@ -36,20 +34,18 @@
   end
 
   describe '#create' do
-    before do
-      sign_in user
-    end
+    before { sign_in user }
 
     it 'creates a Collection' do
       expect do
-        post :create, collection: collection_attrs.merge(visibility: 'open')
+        post :create, params: { collection: collection_attrs.merge(visibility: 'open') }
       end.to change { Collection.count }.by(1)
       expect(assigns[:collection].visibility).to eq 'open'
     end
 
     it 'removes blank strings from params before creating Collection' do
       expect do
-        post :create, collection: collection_attrs.merge(creator: [''])
+        post :create, params: { collection: collection_attrs.merge(creator: ['']) }
       end.to change { Collection.count }.by(1)
       expect(assigns[:collection].title).to eq ['My First Collection']
       expect(assigns[:collection].creator).to eq([])
@@ -58,16 +54,17 @@
     it 'creates a Collection with files I can access' do
       [asset1, asset2].map(&:save) # bogus_depositor_asset is already saved
       expect do
-        post :create, collection: collection_attrs,
-                      batch_document_ids: [asset1.id, asset2.id, bogus_depositor_asset.id]
+        post :create, params: { collection: collection_attrs,
+                                batch_document_ids: [asset1.id, asset2.id, bogus_depositor_asset.id]
+                              }
       end.to change { Collection.count }.by(1)
       collection = assigns(:collection)
       expect(collection.members).to match_array [asset1, asset2]
     end
 
     it 'adds docs to the collection if a batch id is provided and add the collection id to the documents in the collection' do
       asset1.save
-      post :create, batch_document_ids: [asset1.id], collection: collection_attrs
+      post :create, params: { batch_document_ids: [asset1.id], collection: collection_attrs }
       expect(assigns[:collection].members).to eq [asset1]
 
       asset_results = ActiveFedora::SolrService.instance.conn.get 'select', params: { fq: ["id:\"#{asset1.id}\""], fl: ['id'] }
@@ -106,9 +103,10 @@
       xit 'appends members to the collection in order, allowing duplicates' do
         # TODO: Using size until count is fixed https://github.com/projecthydra-labs/activefedora-aggregation/issues/78
         expect {
-          put :update, id: collection,
-                       collection: { members: 'add' },
-                       batch_document_ids: [asset2.id, asset1.id]
+          put :update, params: { id: collection,
+                                 collection: { members: 'add' },
+                                 batch_document_ids: [asset2.id, asset1.id]
+                               }
         }.to change { collection.reload.members.size }.by(2)
         expect(response).to redirect_to routes.url_helpers.collection_path(collection)
         expect(assigns[:collection].members).to eq [asset1, asset2, asset2, asset1]
@@ -122,9 +120,10 @@
 
       it "adds members to the collection" do
         expect {
-          put :update, id: collection,
-                       collection: { members: 'add' },
-                       batch_document_ids: [asset4.id]
+          put :update, params: { id: collection,
+                                 collection: { members: 'add' },
+                                 batch_document_ids: [asset4.id]
+                               }
         }.to change { collection.reload.members.size }.by(1)
         expect(response).to redirect_to routes.url_helpers.collection_path(collection)
         expect(assigns[:collection].members).to match_array [asset1, asset2, asset4]
@@ -139,9 +138,10 @@
       it "removes members from the collection" do
         # TODO: Using size until count is fixed https://github.com/projecthydra-labs/activefedora-aggregation/issues/78
         expect {
-          put :update, id: collection,
-                       collection: { members: 'remove' },
-                       batch_document_ids: [asset2]
+          put :update, params: { id: collection,
+                                 collection: { members: 'remove' },
+                                 batch_document_ids: [asset2]
+                               }
         }.to change { collection.reload.members.size }.by(-1)
         asset_results = ActiveFedora::SolrService.instance.conn.get 'select', params: { fq: ["id:\"#{asset2.id}\""], fl: ['id'] }
         expect(asset_results['response']['numFound']).to eq 1
@@ -155,34 +155,39 @@
       let(:asset1) { create(:generic_work, user: user) }
       let(:asset2) { create(:generic_work, user: user) }
       let(:asset3) { create(:generic_work, user: user) }
-      before do
-        collection.members = [asset1, asset2, asset3]
-        collection.save!
-      end
       let(:collection2) do
         Collection.create(title: ['Some Collection']) do |col|
           col.apply_depositor_metadata(user.user_key)
         end
       end
+      before do
+        collection.members = [asset1, asset2, asset3]
+        collection.save!
+      end
 
       it 'moves the members' do
-        put :update, id: collection, collection: { members: 'move' },
-                     destination_collection_id: collection2, batch_document_ids: [asset2, asset3]
+        put :update,
+            params: {
+              id: collection,
+              collection: { members: 'move' },
+              destination_collection_id: collection2,
+              batch_document_ids: [asset2, asset3]
+            }
         expect(collection.reload.members).to eq [asset1]
         expect(collection2.reload.members).to match_array [asset2, asset3]
       end
     end
 
     context 'updating a collections metadata' do
       it 'saves the metadata' do
-        put :update, id: collection, collection: { creator: ['Emily'], visibility: 'open' }
+        put :update, params: { id: collection, collection: { creator: ['Emily'], visibility: 'open' } }
         collection.reload
         expect(collection.creator).to eq ['Emily']
         expect(collection.visibility).to eq 'open'
       end
 
       it 'removes blank strings from params before updating Collection metadata' do
-        put :update, id: collection, collection: collection_attrs.merge(creator: [''])
+        put :update, params: { id: collection, collection: collection_attrs.merge(creator: ['']) }
         expect(assigns[:collection].title).to eq ['My First Collection']
         expect(assigns[:collection].creator).to eq([])
       end
@@ -202,7 +207,7 @@
       end
 
       it 'returns the collection and its members' do
-        get :show, id: collection
+        get :show, params: { id: collection }
         expect(response).to be_successful
         expect(assigns[:presenter]).to be_kind_of CurationConcerns::CollectionPresenter
         expect(assigns[:presenter].to_s).to eq 'Collection Title'
@@ -211,15 +216,16 @@
 
       context 'when the q parameter is passed' do
         it 'loads the collection (paying no attention to the q param)' do
-          get :show, id: collection, q: 'no matches'
+          get :show, params: { id: collection, q: 'no matches' }
           expect(response).to be_successful
           expect(assigns[:presenter]).to be_kind_of CurationConcerns::CollectionPresenter
           expect(assigns[:presenter].to_s).to eq 'Collection Title'
         end
       end
+
       context 'when the page parameter is passed' do
         it 'loads the collection (paying no attention to the page param)' do
-          get :show, id: collection, page: '2'
+          get :show, params: { id: collection, page: '2' }
           expect(response).to be_successful
           expect(assigns[:presenter]).to be_kind_of CurationConcerns::CollectionPresenter
           expect(assigns[:presenter].to_s).to eq 'Collection Title'
@@ -233,7 +239,7 @@
         collection.save
       end
       it 'forces me to log in' do
-        get :show, id: collection
+        get :show, params: { id: collection }
         expect(response).to redirect_to(main_app.new_user_session_path)
       end
     end
@@ -243,7 +249,7 @@
     before { sign_in user }
 
     it 'does not show flash' do
-      get :edit, id: collection
+      get :edit, params: { id: collection }
       expect(flash[:notice]).to be_nil
     end
   end

spec/controllers/curation_concerns/file_sets_controller_json_spec.rb

@@ -56,7 +56,7 @@
         allow_any_instance_of(FileSet).to receive(:persisted?).and_return(true)
         allow_any_instance_of(FileSet).to receive(:to_param).and_return('999')
 
-        post :create, file_set: { title: ['a title'], files: [file] }, parent_id: parent.id, format: :json
+        post :create, params: { file_set: { title: ['a title'], files: [file] }, parent_id: parent.id, format: :json }
 
         expect(assigns[:file_set]).to be_instance_of ::FileSet # this object is used by the jbuilder template
         expect(controller).to render_template('curation_concerns/file_sets/jq_upload')
@@ -67,24 +67,24 @@
     end
 
     describe 'failed create: no file' do
-      before { post :create, file_set: { title: ["foo"] }, parent_id: parent.id, format: :json }
+      before { post :create, params: { file_set: { title: ["foo"] }, parent_id: parent.id, format: :json } }
       it { is_expected.to respond_bad_request(message: 'Error! No file to save') }
     end
 
     describe 'failed create: bad file' do
-      before { post :create, file_set: { files: ['not a file'] }, parent_id: parent.id, format: :json }
+      before { post :create, params: { file_set: { files: ['not a file'] }, parent_id: parent.id, format: :json } }
       it { is_expected.to respond_bad_request(message: 'Error! No file for upload', description: 'unknown file') }
     end
 
     describe 'failed create: empty file' do
-      before { post :create, file_set: { files: [empty_file] }, parent_id: parent.id, format: :json }
+      before { post :create, params: { file_set: { files: [empty_file] }, parent_id: parent.id, format: :json } }
       it { is_expected.to respond_unprocessable_entity(errors: { files: "#{empty_file.original_filename} has no content! (Zero length file)" }, description: I18n.t('curation_concerns.api.unprocessable_entity.empty_file')) }
     end
 
     describe 'failed create: solr error' do
       before do
         allow(controller).to receive(:process_file).and_raise(RSolr::Error::Http.new(controller.request, response))
-        post :create, file_set: { files: [file] }, parent_id: parent.id, format: :json
+        post :create, params: { file_set: { files: [file] }, parent_id: parent.id, format: :json }
       end
 
       it { is_expected.to respond_internal_error(message: 'Error occurred while creating a FileSet.') }
@@ -112,7 +112,7 @@
         else
           expect(actor).to receive(:update_metadata).with(ActionController::Parameters.new(expected_params).permit!).and_return(true)
         end
-        put :update, id: resource, file_set: { title: ['updated title'] }, format: :json
+        put :update, params: { id: resource, file_set: { title: ['updated title'] }, format: :json }
         expect(assigns[:file_set]).to be_instance_of ::FileSet # this object is used by the jbuilder template
         expect(response.status).to eq 200
         expect(controller).to render_template('curation_concerns/file_sets/show')
@@ -124,7 +124,7 @@
     describe "integration update" do
       render_views
       it "works" do
-        put :update, id: resource.id, file_set: { title: ['test'] }, format: :json
+        put :update, params: { id: resource.id, file_set: { title: ['test'] }, format: :json }
         expect(response.status).to eq 200
       end
     end
@@ -135,7 +135,7 @@
           controller.curation_concern.errors.add(:some_field, "This is not valid. Fix it.")
           false
         end
-        post :update, id: resource, file_set: { title: nil, depositor: nil }, format: :json
+        post :update, params: { id: resource, file_set: { title: nil, depositor: nil }, format: :json }
       }
       it "returns 422 and the errors" do
         expect(response).to respond_unprocessable_entity(errors: { some_field: ["This is not valid. Fix it."] })