Merge pull request #1284 from tvdeyen/deprecate-alchemy-request-helpers

Deprecate alchemy request helpers

CHANGELOG.md

@@ -1,5 +1,9 @@
 # Change Log
 
+## 4.0.0 (unreleased)
+
+* Deprecated controller requests test helpers [#1284](https://github.com/AlchemyCMS/alchemy_cms/pull/1284) by [tvdeyen](https://github.com/tvdeyen)
+
 ## 4.0.0.beta (2017-06-20)
 
 * Rails 5

lib/alchemy/deprecation.rb

@@ -0,0 +1,3 @@
+module Alchemy
+  Deprecation = ActiveSupport::Deprecation.new('5.0', 'Alchemy')
+end

lib/alchemy/test_support/controller_requests.rb

@@ -1,3 +1,11 @@
+# *There is generally no need* to use this module. Instead, in
+# a functional/controller test against a Alchemy controller, just
+# use standard Rails functionality by including:
+#
+#   routes { Alchemy::Engine.routes }
+#
+# And then use standard Rails test `get`, `post` etc methods.
+#
 # Use this module to easily test Alchemy actions within Alchemy components
 # or inside your application to test routes for the mounted Alchemy engine.
 #
@@ -25,35 +33,47 @@
 #
 # Note: Based on Spree::TestingSupport::ControllerRequests. Thanks <3
 #
+# @deprecated Use Rails build in test request methods instead
+#
 module Alchemy
   module TestSupport
     module ControllerRequests
       extend ActiveSupport::Concern
 
       # Executes a request simulating GET HTTP method
+      # @deprecated Use Rails test `get` helper instead
       def alchemy_get(action, parameters = nil, session = nil, flash = nil)
         process_alchemy_action(action, parameters, session, flash, "GET")
       end
+      deprecate alchemy_get: :get, deprecator: Alchemy::Deprecation
 
       # Executes a request simulating POST HTTP method
+      # @deprecated Use Rails test `post` helper instead
       def alchemy_post(action, parameters = nil, session = nil, flash = nil)
         process_alchemy_action(action, parameters, session, flash, "POST")
       end
+      deprecate alchemy_post: :post, deprecator: Alchemy::Deprecation
 
       # Executes a request simulating PUT HTTP method
+      # @deprecated Use Rails test `put` helper instead
       def alchemy_put(action, parameters = nil, session = nil, flash = nil)
         process_alchemy_action(action, parameters, session, flash, "PUT")
       end
+      deprecate alchemy_put: :put, deprecator: Alchemy::Deprecation
 
       # Executes a request simulating DELETE HTTP method
+      # @deprecated Use Rails test `delete` helper instead
       def alchemy_delete(action, parameters = nil, session = nil, flash = nil)
         process_alchemy_action(action, parameters, session, flash, "DELETE")
       end
+      deprecate alchemy_delete: :delete, deprecator: Alchemy::Deprecation
 
       # Executes a simulated XHR request
+      # @deprecated Use Rails test `xhr` helper instead
       def alchemy_xhr(method, action, parameters = nil, session = nil, flash = nil)
         process_alchemy_xhr_action(method, action, parameters, session, flash)
       end
+      deprecate alchemy_xhr: :xhr, deprecator: Alchemy::Deprecation
 
       private
 

lib/alchemy_cms.rb

@@ -32,6 +32,7 @@ module Alchemy
 require_relative 'alchemy/config'
 require_relative 'alchemy/configuration_methods'
 require_relative 'alchemy/controller_actions'
+require_relative 'alchemy/deprecation'
 require_relative 'alchemy/errors'
 require_relative 'alchemy/essence'
 require_relative 'alchemy/filetypes'

spec/controllers/alchemy/admin/attachments_controller_spec.rb

@@ -2,6 +2,8 @@
 
 module Alchemy
   describe Admin::AttachmentsController do
+    routes { Alchemy::Engine.routes }
+
     let(:attachment) { build_stubbed(:alchemy_attachment) }
 
     let(:file) do
@@ -18,13 +20,13 @@ module Alchemy
     describe "#index" do
       it "should always paginate the records" do
         expect_any_instance_of(ActiveRecord::Relation).to receive(:page).and_call_original
-        alchemy_get :index
+        get :index
       end
 
       context "when params[:tagged_with] is set" do
         it "should filter the records by tags" do
           expect(Attachment).to receive(:tagged_with).and_return(Attachment.all)
-          alchemy_get :index, tagged_with: "pdf"
+          get :index, params: {tagged_with: "pdf"}
         end
       end
 
@@ -34,15 +36,15 @@ module Alchemy
         context "is set" do
           it "it renders the archive_overlay partial" do
             expect(Content).to receive(:find_by).and_return(content)
-            alchemy_get :index, {content_id: content.id}
+            get :index, params: {content_id: content.id}
             expect(response).to render_template(partial: '_archive_overlay')
             expect(assigns(:content)).to eq(content)
           end
         end
 
         context "is not set" do
           it "should render the default index view" do
-            alchemy_get :index
+            get :index
             expect(response).to render_template(:index)
           end
         end
@@ -58,7 +60,7 @@ module Alchemy
 
         context 'with params[:file_type]' do
           it 'loads only attachments with matching content type' do
-            alchemy_get :index, file_type: 'image/jpeg'
+            get :index, params: {file_type: 'image/jpeg'}
             expect(assigns(:attachments).to_a).to eq([jpg])
             expect(assigns(:attachments).to_a).to_not eq([png])
           end
@@ -72,13 +74,13 @@ module Alchemy
       end
 
       it "renders the show template" do
-        alchemy_get :show, id: attachment.id
+        get :show, params: {id: attachment.id}
         expect(response).to render_template(:show)
       end
     end
 
     describe '#create' do
-      subject { alchemy_post :create, params }
+      subject { post :create, params: params }
 
       context 'with passing validations' do
         let(:params) { {attachment: {file: file}} }
@@ -110,7 +112,7 @@ module Alchemy
       end
 
       subject do
-        alchemy_put :update, params
+        put :update, params: params
       end
 
       let!(:attachment) { create(:alchemy_attachment) }
@@ -161,7 +163,7 @@ module Alchemy
           end
 
           subject do
-            alchemy_put :update, {
+            put :update, params: {
               id: attachment.id, attachment: {name: ''}
             }.merge(search_params)
           end
@@ -188,7 +190,7 @@ module Alchemy
 
       it "destroys the attachment and sets a success message" do
         expect(attachment).to receive(:destroy)
-        alchemy_xhr :delete, :destroy, id: 1
+        delete :destroy, params: {id: 1}, xhr: true
         expect(assigns(:attachment)).to eq(attachment)
         expect(assigns(:url)).not_to be_blank
         expect(flash[:notice]).not_to be_blank
@@ -206,7 +208,7 @@ module Alchemy
 
         it "passes them along" do
           expect(attachment).to receive(:destroy) { true }
-          alchemy_xhr :delete, :destroy, {id: 1}.merge(search_params)
+          delete :destroy, params: {id: 1}.merge(search_params), xhr: true
           expect(assigns(:url)).to eq admin_attachments_url(search_params.merge(host: 'test.host'))
         end
       end
@@ -218,7 +220,7 @@ module Alchemy
       end
 
       it "sends the file as download" do
-        alchemy_get :download, id: attachment.id
+        get :download, params: {id: attachment.id}
         expect(response.headers['Content-Disposition']).to match(/attachment/)
       end
     end

spec/controllers/alchemy/admin/clipboard_controller_spec.rb

@@ -2,6 +2,8 @@
 
 module Alchemy
   describe Admin::ClipboardController do
+    routes { Alchemy::Engine.routes }
+
     let(:public_page)     { build_stubbed(:alchemy_page, :public) }
     let(:element)         { build_stubbed(:alchemy_element, page: public_page) }
     let(:another_element) { build_stubbed(:alchemy_element, page: public_page) }
@@ -14,15 +16,15 @@ module Alchemy
     describe '#index' do
       context 'with `remarkable_type` being an allowed type' do
         it 'is successful' do
-          alchemy_get :index, {remarkable_type: 'elements'}
+          get :index, params: {remarkable_type: 'elements'}
           expect(response).to be_success
         end
       end
 
       context 'with `remarkable_type` not an allowed type' do
         it 'raises 400 Bad Request' do
           expect {
-            alchemy_get :index, {remarkable_type: 'evil'}
+            get :index, params: {remarkable_type: 'evil'}
           }.to raise_error(ActionController::BadRequest)
         end
       end
@@ -35,13 +37,13 @@ module Alchemy
 
       describe "#insert" do
         it "should hold element ids" do
-          alchemy_xhr :post, :insert, {remarkable_type: 'elements', remarkable_id: element.id}
+          post :insert, params: {remarkable_type: 'elements', remarkable_id: element.id}, xhr: true
           expect(session[:alchemy_clipboard]['elements']).to eq([{'id' => element.id.to_s, 'action' => 'copy'}])
         end
 
         it "should not have the same element twice" do
           session[:alchemy_clipboard]['elements'] = [{'id' => element.id.to_s, 'action' => 'copy'}]
-          alchemy_xhr :post, :insert, {remarkable_type: 'elements', remarkable_id: element.id}
+          post :insert, params: {remarkable_type: 'elements', remarkable_id: element.id}, xhr: true
           expect(session[:alchemy_clipboard]['elements'].collect { |e| e['id'] }).not_to eq([element.id, element.id])
         end
       end
@@ -50,7 +52,7 @@ module Alchemy
         it "should remove element ids from clipboard" do
           session[:alchemy_clipboard]['elements'] = [{'id' => element.id.to_s, 'action' => 'copy'}]
           session[:alchemy_clipboard]['elements'] << {'id' => another_element.id.to_s, 'action' => 'copy'}
-          alchemy_xhr :delete, :remove, {remarkable_type: 'elements', remarkable_id: another_element.id}
+          delete :remove, params: {remarkable_type: 'elements', remarkable_id: another_element.id}, xhr: true
           expect(session[:alchemy_clipboard]['elements']).to eq([{'id' => element.id.to_s, 'action' => 'copy'}])
         end
       end
@@ -60,15 +62,15 @@ module Alchemy
       context "with elements as remarkable_type" do
         it "should clear the elements clipboard" do
           session[:alchemy_clipboard]['elements'] = [{'id' => element.id.to_s}]
-          alchemy_xhr :delete, :clear, {remarkable_type: 'elements'}
+          delete :clear, params: {remarkable_type: 'elements'}, xhr: true
           expect(session[:alchemy_clipboard]['elements']).to be_empty
         end
       end
 
       context "with pages as remarkable_type" do
         it "should clear the pages clipboard" do
           session[:alchemy_clipboard]['pages'] = [{'id' => public_page.id.to_s}]
-          alchemy_xhr :delete, :clear, {remarkable_type: 'pages'}
+          delete :clear, params: {remarkable_type: 'pages'}, xhr: true
           expect(session[:alchemy_clipboard]['pages']).to be_empty
         end
       end

spec/controllers/alchemy/admin/contents_controller_spec.rb

@@ -2,6 +2,8 @@
 
 module Alchemy
   describe Admin::ContentsController do
+    routes { Alchemy::Engine.routes }
+
     before do
       authorize_user(:as_admin)
     end
@@ -12,17 +14,17 @@ module Alchemy
 
         it "creates a content from name" do
           expect {
-            alchemy_xhr :post, :create, {content: {element_id: element.id, name: 'headline'}}
+            post :create, params: {content: {element_id: element.id, name: 'headline'}}, xhr: true
           }.to change { Alchemy::Content.count }.by(1)
         end
 
         it "creates a content from essence_type" do
           expect {
-            alchemy_xhr :post, :create, {
+            post :create, params: {
               content: {
                 element_id: element.id, essence_type: 'EssencePicture'
               }
-            }
+            }, xhr: true
           }.to change { Alchemy::Content.count }.by(1)
         end
       end
@@ -43,13 +45,13 @@ module Alchemy
         end
 
         it "adds it into the gallery editor" do
-          alchemy_xhr :post, :create, attributes
+          post :create, params: attributes, xhr: true
           expect(assigns(:content_dom_id)).to eq("#add_picture_#{element.id}")
         end
 
         context 'with picture_id given' do
           it "assigns the picture to the essence" do
-            alchemy_xhr :post, :create, attributes.merge(picture_id: '1')
+            post :create, params: attributes.merge(picture_id: '1'), xhr: true
             expect(Alchemy::Content.last.essence.picture_id).to eq(1)
           end
         end
@@ -65,7 +67,7 @@ module Alchemy
 
       it "should update a content via ajax" do
         expect {
-          alchemy_xhr :post, :update, {id: content.id, content: {ingredient: 'Peters Petshop'}}
+          post :update, params: {id: content.id, content: {ingredient: 'Peters Petshop'}}, xhr: true
         }.to change { content.ingredient }.to 'Peters Petshop'
       end
     end
@@ -79,7 +81,7 @@ module Alchemy
         let(:content_ids) { element.contents.pluck(:id).shuffle }
 
         it "should reorder the contents" do
-          alchemy_xhr :post, :order, {content_ids: content_ids}
+          post :order, params: {content_ids: content_ids}, xhr: true
 
           expect(response.status).to eq(200)
           expect(element.contents(true).pluck(:id)).to eq(content_ids)