From a0aa42cd5837848d402825b31e5e4feae4d12838 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Tue, 2 Jun 2020 13:26:12 +0100 Subject: [PATCH 01/10] Remove resource_scoping from api/variants spec --- spec/controllers/api/variants_controller_spec.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/spec/controllers/api/variants_controller_spec.rb b/spec/controllers/api/variants_controller_spec.rb index a5c81329dd..c91f25f2ec 100644 --- a/spec/controllers/api/variants_controller_spec.rb +++ b/spec/controllers/api/variants_controller_spec.rb @@ -113,7 +113,6 @@ describe Api::VariantsController, type: :controller do let(:product) { create(:product) } let(:variant) { product.master } - let(:resource_scoping) { { product_id: variant.product.to_param } } context "deleted variants" do before do @@ -121,7 +120,7 @@ describe Api::VariantsController, type: :controller do end it "are visible by admin" do - api_get :index, show_deleted: 1 + api_get :index, show_deleted: 1, product_id: variant.product.to_param expect(json_response.count).to eq(2) end @@ -129,7 +128,7 @@ describe Api::VariantsController, type: :controller do it "can create a new variant" do original_number_of_variants = variant.product.variants.count - api_post :create, variant: { sku: "12345", unit_value: "weight", unit_description: "L" } + api_post :create, variant: { sku: "12345", unit_value: "weight", unit_description: "L" }, product_id: variant.product.to_param expect(attributes.all?{ |attr| json_response.include? attr.to_s }).to eq(true) expect(response.status).to eq(201) From 092e047b44c7076f8aabfd16b03d7300494ef9e5 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Tue, 2 Jun 2020 13:35:48 +0100 Subject: [PATCH 02/10] Remove resource_scoping from api/shipments_controller spec --- spec/controllers/api/shipments_controller_spec.rb | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/spec/controllers/api/shipments_controller_spec.rb b/spec/controllers/api/shipments_controller_spec.rb index d79eaf0c08..2f66d0b158 100644 --- a/spec/controllers/api/shipments_controller_spec.rb +++ b/spec/controllers/api/shipments_controller_spec.rb @@ -5,7 +5,6 @@ describe Api::ShipmentsController, type: :controller do let!(:shipment) { create(:shipment) } let!(:attributes) { [:id, :tracking, :number, :cost, :shipped_at, :stock_location_name, :order_id] } - let!(:resource_scoping) { { order_id: shipment.order.to_param, id: shipment.to_param } } let(:current_api_user) { build(:user) } before do @@ -14,12 +13,12 @@ describe Api::ShipmentsController, type: :controller do context "as a non-admin" do it "cannot make a shipment ready" do - api_put :ready + api_put :ready, order_id: shipment.order.to_param, id: shipment.to_param assert_unauthorized! end it "cannot make a shipment shipped" do - api_put :ship + api_put :ship, order_id: shipment.order.to_param, id: shipment.to_param assert_unauthorized! end end @@ -92,7 +91,7 @@ describe Api::ShipmentsController, type: :controller do it "can make a shipment ready" do allow_any_instance_of(Spree::Order).to receive_messages(paid?: true, complete?: true) - api_put :ready + api_put :ready, order_id: shipment.order.to_param, id: shipment.to_param expect(attributes.all?{ |attr| json_response.key? attr.to_s }).to be_truthy expect(json_response["state"]).to eq("ready") @@ -101,7 +100,7 @@ describe Api::ShipmentsController, type: :controller do it "cannot make a shipment ready if the order is unpaid" do allow_any_instance_of(Spree::Order).to receive_messages(paid?: false) - api_put :ready + api_put :ready, order_id: shipment.order.to_param, id: shipment.to_param expect(json_response["error"]).to eq("Cannot ready shipment.") expect(response.status).to eq(422) @@ -109,10 +108,9 @@ describe Api::ShipmentsController, type: :controller do context 'for completed shipments' do let(:order) { create :completed_order_with_totals } - let!(:resource_scoping) { { order_id: order.to_param, id: order.shipments.first.to_param } } it 'adds a variant to a shipment' do - api_put :add, variant_id: variant.to_param, quantity: 2 + api_put :add, variant_id: variant.to_param, quantity: 2, order_id: order.to_param, id: order.shipments.first.to_param expect(response.status).to eq(200) expect(order.shipment.reload.inventory_units.select { |h| h['variant_id'] == variant.id }.size).to eq 2 @@ -120,7 +118,7 @@ describe Api::ShipmentsController, type: :controller do it 'removes a variant from a shipment' do order.contents.add(variant, 2) - api_put :remove, variant_id: variant.to_param, quantity: 1 + api_put :remove, variant_id: variant.to_param, quantity: 1, order_id: order.to_param, id: order.shipments.first.to_param expect(response.status).to eq(200) expect(order.shipment.reload.inventory_units.select { |h| h['variant_id'] == variant.id }.size).to eq(1) From 2c2263ab78d35b9a9fdc3641f9e85b3652101fa4 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Tue, 2 Jun 2020 13:41:46 +0100 Subject: [PATCH 03/10] Fix rubocop issues in shipments_controller_spec --- .rubocop_manual_todo.yml | 1 - .../api/shipments_controller_spec.rb | 22 ++++++++++++++----- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/.rubocop_manual_todo.yml b/.rubocop_manual_todo.yml index e55124201f..ace78ed248 100644 --- a/.rubocop_manual_todo.yml +++ b/.rubocop_manual_todo.yml @@ -142,7 +142,6 @@ Layout/LineLength: - spec/controllers/api/product_images_controller_spec.rb - spec/controllers/api/products_controller_spec.rb - spec/controllers/api/promo_images_controller_spec.rb - - spec/controllers/api/shipments_controller_spec.rb - spec/controllers/api/variants_controller_spec.rb - spec/controllers/cart_controller_spec.rb - spec/controllers/checkout_controller_spec.rb diff --git a/spec/controllers/api/shipments_controller_spec.rb b/spec/controllers/api/shipments_controller_spec.rb index 2f66d0b158..3643f0a58d 100644 --- a/spec/controllers/api/shipments_controller_spec.rb +++ b/spec/controllers/api/shipments_controller_spec.rb @@ -4,7 +4,9 @@ describe Api::ShipmentsController, type: :controller do render_views let!(:shipment) { create(:shipment) } - let!(:attributes) { [:id, :tracking, :number, :cost, :shipped_at, :stock_location_name, :order_id] } + let!(:attributes) do + [:id, :tracking, :number, :cost, :shipped_at, :stock_location_name, :order_id] + end let(:current_api_user) { build(:user) } before do @@ -110,18 +112,24 @@ describe Api::ShipmentsController, type: :controller do let(:order) { create :completed_order_with_totals } it 'adds a variant to a shipment' do - api_put :add, variant_id: variant.to_param, quantity: 2, order_id: order.to_param, id: order.shipments.first.to_param + api_put :add, variant_id: variant.to_param, + quantity: 2, + order_id: order.to_param, + id: order.shipments.first.to_param expect(response.status).to eq(200) - expect(order.shipment.reload.inventory_units.select { |h| h['variant_id'] == variant.id }.size).to eq 2 + expect(inventory_units_for(variant).size).to eq 2 end it 'removes a variant from a shipment' do order.contents.add(variant, 2) - api_put :remove, variant_id: variant.to_param, quantity: 1, order_id: order.to_param, id: order.shipments.first.to_param + api_put :remove, variant_id: variant.to_param, + quantity: 1, + order_id: order.to_param, + id: order.shipments.first.to_param expect(response.status).to eq(200) - expect(order.shipment.reload.inventory_units.select { |h| h['variant_id'] == variant.id }.size).to eq(1) + expect(inventory_units_for(variant).size).to eq(1) end end @@ -138,7 +146,9 @@ describe Api::ShipmentsController, type: :controller do it "can transition a shipment from ready to ship" do shipment.reload - api_put :ship, order_id: shipment.order.to_param, id: shipment.to_param, shipment: { tracking: "123123" } + api_put :ship, order_id: shipment.order.to_param, + id: shipment.to_param, + shipment: { tracking: "123123" } expect(attributes.all?{ |attr| json_response.key? attr.to_s }).to be_truthy expect(json_response["state"]).to eq("shipped") From cd3bc54c3758c9db91143cd1c2ac9ef7e4855272 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Tue, 2 Jun 2020 13:43:33 +0100 Subject: [PATCH 04/10] Remove resource_scoping to make things more simple --- spec/support/controller_hacks.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/spec/support/controller_hacks.rb b/spec/support/controller_hacks.rb index 7645e87372..978710068b 100644 --- a/spec/support/controller_hacks.rb +++ b/spec/support/controller_hacks.rb @@ -18,10 +18,8 @@ module ControllerHacks end def api_process(action, params = {}, session = nil, flash = nil, method = "get") - scoping = respond_to?(:resource_scoping) ? resource_scoping : {} process(action, params. - merge(scoping). reverse_merge!(use_route: :spree, format: :json), session, flash, From 5d0856e5a3235cef90944fed61af4f1e8b01f0f3 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Tue, 2 Jun 2020 13:47:59 +0100 Subject: [PATCH 05/10] Rename ControllerHacks to ControllerRequestsHelper and move it's configuration to spec_helper --- spec/spec_helper.rb | 3 ++- .../{controller_hacks.rb => controller_requests_helper.rb} | 6 +----- 2 files changed, 3 insertions(+), 6 deletions(-) rename spec/support/{controller_hacks.rb => controller_requests_helper.rb} (88%) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index e145fa7909..2ff99b5f11 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -148,8 +148,9 @@ RSpec.configure do |config| config.include Spree::UrlHelpers config.include Spree::CheckoutHelpers config.include Spree::MoneyHelper - config.include Spree::TestingSupport::ControllerRequests, type: :controller config.include Spree::TestingSupport::Preferences + config.include Spree::TestingSupport::ControllerRequests, type: :controller + config.include ControllerRequestsHelper, type: :controller config.include Devise::TestHelpers, type: :controller config.extend Spree::Api::TestingSupport::Setup, type: :controller config.include OpenFoodNetwork::ApiHelper, type: :controller diff --git a/spec/support/controller_hacks.rb b/spec/support/controller_requests_helper.rb similarity index 88% rename from spec/support/controller_hacks.rb rename to spec/support/controller_requests_helper.rb index 978710068b..73afb2c1f3 100644 --- a/spec/support/controller_hacks.rb +++ b/spec/support/controller_requests_helper.rb @@ -1,6 +1,6 @@ require 'active_support/all' -module ControllerHacks +module ControllerRequestsHelper def api_get(action, params = {}, session = nil, flash = nil) api_process(action, params, session, flash, "GET") end @@ -26,7 +26,3 @@ module ControllerHacks method) end end - -RSpec.configure do |config| - config.include ControllerHacks, type: :controller -end From 2f76e0b15baf240de6c050a177acc61d330dbb31 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Tue, 2 Jun 2020 13:54:33 +0100 Subject: [PATCH 06/10] Bring methods from Spree::TestingSupport::ControllerRequests to our ControllerRequestsHelper so we can merge them later --- spec/spec_helper.rb | 2 -- spec/support/controller_requests_helper.rb | 28 ++++++++++++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 2ff99b5f11..edc9519e94 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -36,7 +36,6 @@ WebMock.disable_net_connect!( # Requires supporting ruby files with custom matchers and macros, etc, # in spec/support/ and its subdirectories. Dir[Rails.root.join("spec/support/**/*.rb")].each { |f| require f } -require 'spree/testing_support/controller_requests' require 'spree/testing_support/capybara_ext' require 'spree/api/testing_support/setup' require 'spree/testing_support/authorization_helpers' @@ -149,7 +148,6 @@ RSpec.configure do |config| config.include Spree::CheckoutHelpers config.include Spree::MoneyHelper config.include Spree::TestingSupport::Preferences - config.include Spree::TestingSupport::ControllerRequests, type: :controller config.include ControllerRequestsHelper, type: :controller config.include Devise::TestHelpers, type: :controller config.extend Spree::Api::TestingSupport::Setup, type: :controller diff --git a/spec/support/controller_requests_helper.rb b/spec/support/controller_requests_helper.rb index 73afb2c1f3..42575f7eaf 100644 --- a/spec/support/controller_requests_helper.rb +++ b/spec/support/controller_requests_helper.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'active_support/all' module ControllerRequestsHelper @@ -17,6 +19,27 @@ module ControllerRequestsHelper api_process(action, params, session, flash, "DELETE") end + def spree_get(action, parameters = nil, session = nil, flash = nil) + process_spree_action(action, parameters, session, flash, "GET") + end + + # Executes a request simulating POST HTTP method and set/volley the response + def spree_post(action, parameters = nil, session = nil, flash = nil) + process_spree_action(action, parameters, session, flash, "POST") + end + + # Executes a request simulating PUT HTTP method and set/volley the response + def spree_put(action, parameters = nil, session = nil, flash = nil) + process_spree_action(action, parameters, session, flash, "PUT") + end + + # Executes a request simulating DELETE HTTP method and set/volley the response + def spree_delete(action, parameters = nil, session = nil, flash = nil) + process_spree_action(action, parameters, session, flash, "DELETE") + end + + private + def api_process(action, params = {}, session = nil, flash = nil, method = "get") process(action, params. @@ -25,4 +48,9 @@ module ControllerRequestsHelper flash, method) end + + def process_spree_action(action, parameters = nil, session = nil, flash = nil, method = "GET") + parameters ||= {} + process(action, parameters.merge!(use_route: :spree), session, flash, method) + end end From 3136aa5a8bbfe28c6c0826bb712fb1bead8ee9ef Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Tue, 2 Jun 2020 14:00:24 +0100 Subject: [PATCH 07/10] Make api_process use process_spree_action --- spec/support/controller_requests_helper.rb | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/spec/support/controller_requests_helper.rb b/spec/support/controller_requests_helper.rb index 42575f7eaf..93f1853cb2 100644 --- a/spec/support/controller_requests_helper.rb +++ b/spec/support/controller_requests_helper.rb @@ -41,16 +41,18 @@ module ControllerRequestsHelper private def api_process(action, params = {}, session = nil, flash = nil, method = "get") + process_spree_action(action, + params.reverse_merge!(format: :json), + session, + flash, + method) + end + + def process_spree_action(action, parameters = {}, session = nil, flash = nil, method = "GET") process(action, - params. - reverse_merge!(use_route: :spree, format: :json), + parameters.merge!(use_route: :spree), session, flash, method) end - - def process_spree_action(action, parameters = nil, session = nil, flash = nil, method = "GET") - parameters ||= {} - process(action, parameters.merge!(use_route: :spree), session, flash, method) - end end From 13836a62bb66c5139d65d6bea16252bedcfb3036 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Tue, 2 Jun 2020 14:36:06 +0100 Subject: [PATCH 08/10] Rename parameters to params and change default from nil to {} --- spec/support/controller_requests_helper.rb | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/spec/support/controller_requests_helper.rb b/spec/support/controller_requests_helper.rb index 93f1853cb2..4aa4c8535b 100644 --- a/spec/support/controller_requests_helper.rb +++ b/spec/support/controller_requests_helper.rb @@ -19,23 +19,23 @@ module ControllerRequestsHelper api_process(action, params, session, flash, "DELETE") end - def spree_get(action, parameters = nil, session = nil, flash = nil) - process_spree_action(action, parameters, session, flash, "GET") + def spree_get(action, params = {}, session = nil, flash = nil) + process_spree_action(action, params, session, flash, "GET") end # Executes a request simulating POST HTTP method and set/volley the response - def spree_post(action, parameters = nil, session = nil, flash = nil) - process_spree_action(action, parameters, session, flash, "POST") + def spree_post(action, params = {}, session = nil, flash = nil) + process_spree_action(action, params, session, flash, "POST") end # Executes a request simulating PUT HTTP method and set/volley the response - def spree_put(action, parameters = nil, session = nil, flash = nil) - process_spree_action(action, parameters, session, flash, "PUT") + def spree_put(action, params = {}, session = nil, flash = nil) + process_spree_action(action, params, session, flash, "PUT") end # Executes a request simulating DELETE HTTP method and set/volley the response - def spree_delete(action, parameters = nil, session = nil, flash = nil) - process_spree_action(action, parameters, session, flash, "DELETE") + def spree_delete(action, params = {}, session = nil, flash = nil) + process_spree_action(action, params, session, flash, "DELETE") end private @@ -48,9 +48,9 @@ module ControllerRequestsHelper method) end - def process_spree_action(action, parameters = {}, session = nil, flash = nil, method = "GET") + def process_spree_action(action, params = {}, session = nil, flash = nil, method = "GET") process(action, - parameters.merge!(use_route: :spree), + params.merge!(use_route: :spree), session, flash, method) From 7b89f4d6c5f8bb67eb54782ef3b45f34bf29197c Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Tue, 2 Jun 2020 15:00:57 +0100 Subject: [PATCH 09/10] Switch controller helper calls routes from spree to main_app, this will include all routes all the time --- spec/controllers/api/shops_controller_spec.rb | 2 +- spec/support/controller_requests_helper.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/controllers/api/shops_controller_spec.rb b/spec/controllers/api/shops_controller_spec.rb index 33343e8d84..91f88abc0a 100644 --- a/spec/controllers/api/shops_controller_spec.rb +++ b/spec/controllers/api/shops_controller_spec.rb @@ -34,7 +34,7 @@ module Api describe "#closed_shops" do it "returns data for all closed shops" do - spree_get :closed_shops, nil + spree_get :closed_shops, {} expect(json_response).not_to match hub.name diff --git a/spec/support/controller_requests_helper.rb b/spec/support/controller_requests_helper.rb index 4aa4c8535b..3ac1103014 100644 --- a/spec/support/controller_requests_helper.rb +++ b/spec/support/controller_requests_helper.rb @@ -50,7 +50,7 @@ module ControllerRequestsHelper def process_spree_action(action, params = {}, session = nil, flash = nil, method = "GET") process(action, - params.merge!(use_route: :spree), + params.merge!(use_route: :main_app), session, flash, method) From 53a80de503436c1dbce458f01063160d0e23d803 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Tue, 2 Jun 2020 17:32:09 +0100 Subject: [PATCH 10/10] Improve method names and set use_route with reverse_merge so it can be overridden --- spec/support/controller_requests_helper.rb | 27 ++++++++++------------ 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/spec/support/controller_requests_helper.rb b/spec/support/controller_requests_helper.rb index 3ac1103014..7187670f12 100644 --- a/spec/support/controller_requests_helper.rb +++ b/spec/support/controller_requests_helper.rb @@ -4,53 +4,50 @@ require 'active_support/all' module ControllerRequestsHelper def api_get(action, params = {}, session = nil, flash = nil) - api_process(action, params, session, flash, "GET") + process_json_action(action, params, session, flash, "GET") end def api_post(action, params = {}, session = nil, flash = nil) - api_process(action, params, session, flash, "POST") + process_json_action(action, params, session, flash, "POST") end def api_put(action, params = {}, session = nil, flash = nil) - api_process(action, params, session, flash, "PUT") + process_json_action(action, params, session, flash, "PUT") end def api_delete(action, params = {}, session = nil, flash = nil) - api_process(action, params, session, flash, "DELETE") + process_json_action(action, params, session, flash, "DELETE") end def spree_get(action, params = {}, session = nil, flash = nil) - process_spree_action(action, params, session, flash, "GET") + process_action_with_route(action, params, session, flash, "GET") end - # Executes a request simulating POST HTTP method and set/volley the response def spree_post(action, params = {}, session = nil, flash = nil) - process_spree_action(action, params, session, flash, "POST") + process_action_with_route(action, params, session, flash, "POST") end - # Executes a request simulating PUT HTTP method and set/volley the response def spree_put(action, params = {}, session = nil, flash = nil) - process_spree_action(action, params, session, flash, "PUT") + process_action_with_route(action, params, session, flash, "PUT") end - # Executes a request simulating DELETE HTTP method and set/volley the response def spree_delete(action, params = {}, session = nil, flash = nil) - process_spree_action(action, params, session, flash, "DELETE") + process_action_with_route(action, params, session, flash, "DELETE") end private - def api_process(action, params = {}, session = nil, flash = nil, method = "get") - process_spree_action(action, + def process_json_action(action, params = {}, session = nil, flash = nil, method = "get") + process_action_with_route(action, params.reverse_merge!(format: :json), session, flash, method) end - def process_spree_action(action, params = {}, session = nil, flash = nil, method = "GET") + def process_action_with_route(action, params = {}, session = nil, flash = nil, method = "GET") process(action, - params.merge!(use_route: :main_app), + params.reverse_merge!(use_route: :main_app), session, flash, method)