From f9eb6c6b4c112d4b2cf0659f1e111d9ae686163a Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Mon, 16 Oct 2017 10:54:00 -0700 Subject: [PATCH 1/2] Test populate with successful response I don't think the original intention of this spec was for the controller to error. This wasn't noticed because the error was being swallowed by rescue_from render_404. --- frontend/spec/controllers/spree/orders_controller_spec.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/frontend/spec/controllers/spree/orders_controller_spec.rb b/frontend/spec/controllers/spree/orders_controller_spec.rb index 9106946bc2d..3b0cf3b22d2 100644 --- a/frontend/spec/controllers/spree/orders_controller_spec.rb +++ b/frontend/spec/controllers/spree/orders_controller_spec.rb @@ -16,7 +16,8 @@ context "#populate" do it "should create a new order when none specified" do - post :populate + post :populate, params: { variant_id: variant.id } + expect(response).to be_redirect expect(cookies.signed[:guest_token]).not_to be_blank order_by_token = Spree::Order.find_by(guest_token: cookies.signed[:guest_token]) From 11658e4643d50e0daf49e55bee5334c309bdd82e Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Mon, 16 Oct 2017 11:12:18 -0700 Subject: [PATCH 2/2] Remove rescue_from :render_404 from frontend Rails knows how to render a 404, so this is unnecessary and can hide errors. Doing this is also necessary in order to test without full rails installation directory (specifically with a public/404.html). --- core/lib/spree/core/controller_helpers/common.rb | 7 ------- frontend/app/controllers/spree/orders_controller.rb | 1 - frontend/app/controllers/spree/products_controller.rb | 1 - frontend/app/controllers/spree/taxons_controller.rb | 1 - .../spree/orders_controller_ability_spec.rb | 10 ++++++---- .../spec/controllers/spree/products_controller_spec.rb | 5 +++-- 6 files changed, 9 insertions(+), 16 deletions(-) diff --git a/core/lib/spree/core/controller_helpers/common.rb b/core/lib/spree/core/controller_helpers/common.rb index 88e0143e640..a54ce8248d5 100644 --- a/core/lib/spree/core/controller_helpers/common.rb +++ b/core/lib/spree/core/controller_helpers/common.rb @@ -41,13 +41,6 @@ def accurate_title current_store.seo_title end - def render_404(_exception = nil) - respond_to do |type| - type.html { render status: :not_found, file: "#{::Rails.root}/public/404", formats: [:html], layout: nil } - type.all { render status: :not_found, nothing: true } - end - end - private def set_user_language diff --git a/frontend/app/controllers/spree/orders_controller.rb b/frontend/app/controllers/spree/orders_controller.rb index aa93bbef35a..ed7391946a8 100644 --- a/frontend/app/controllers/spree/orders_controller.rb +++ b/frontend/app/controllers/spree/orders_controller.rb @@ -1,7 +1,6 @@ module Spree class OrdersController < Spree::StoreController before_action :check_authorization - rescue_from ActiveRecord::RecordNotFound, with: :render_404 helper 'spree/products', 'spree/orders' respond_to :html diff --git a/frontend/app/controllers/spree/products_controller.rb b/frontend/app/controllers/spree/products_controller.rb index 1fefd28f3de..e729ad582dc 100644 --- a/frontend/app/controllers/spree/products_controller.rb +++ b/frontend/app/controllers/spree/products_controller.rb @@ -3,7 +3,6 @@ class ProductsController < Spree::StoreController before_action :load_product, only: :show before_action :load_taxon, only: :index - rescue_from ActiveRecord::RecordNotFound, with: :render_404 helper 'spree/taxons' respond_to :html diff --git a/frontend/app/controllers/spree/taxons_controller.rb b/frontend/app/controllers/spree/taxons_controller.rb index dd3701c3ca3..61907c7fd80 100644 --- a/frontend/app/controllers/spree/taxons_controller.rb +++ b/frontend/app/controllers/spree/taxons_controller.rb @@ -1,6 +1,5 @@ module Spree class TaxonsController < Spree::StoreController - rescue_from ActiveRecord::RecordNotFound, with: :render_404 helper 'spree/products' respond_to :html diff --git a/frontend/spec/controllers/spree/orders_controller_ability_spec.rb b/frontend/spec/controllers/spree/orders_controller_ability_spec.rb index 176e1e0e1eb..37a33224528 100644 --- a/frontend/spec/controllers/spree/orders_controller_ability_spec.rb +++ b/frontend/spec/controllers/spree/orders_controller_ability_spec.rb @@ -8,6 +8,7 @@ module Spree let(:user) { create(:user) } let(:guest_user) { create(:user) } let(:order) { Spree::Order.create } + let(:variant) { create(:variant) } it 'should understand order routes with token' do expect(spree.token_order_path('R123456', 'ABCDEF')).to eq('/orders/R123456/token/ABCDEF') @@ -25,11 +26,11 @@ module Spree context '#populate' do it 'should check if user is authorized for :edit' do expect(controller).to receive(:authorize!).with(:edit, order, token) - post :populate, params: { token: token } + post :populate, params: { variant_id: variant.id, token: token } end it "should check against the specified order" do expect(controller).to receive(:authorize!).with(:edit, specified_order, token) - post :populate, params: { id: specified_order.number, token: token } + post :populate, params: { id: specified_order.number, variant_id: variant.id, token: token } end end @@ -95,8 +96,9 @@ module Spree context 'when no token present' do it 'should respond with 404' do - get :show, params: { id: 'R123' } - expect(response.code).to eq('404') + expect { + get :show, params: { id: 'R123' } + }.to raise_error(ActiveRecord::RecordNotFound) end end end diff --git a/frontend/spec/controllers/spree/products_controller_spec.rb b/frontend/spec/controllers/spree/products_controller_spec.rb index 24d71c17bc8..c26ea0850eb 100644 --- a/frontend/spec/controllers/spree/products_controller_spec.rb +++ b/frontend/spec/controllers/spree/products_controller_spec.rb @@ -11,8 +11,9 @@ end it "cannot view non-active products" do - get :show, params: { id: product.to_param } - expect(response.status).to eq(404) + expect { + get :show, params: { id: product.to_param } + }.to raise_error(ActiveRecord::RecordNotFound) end it "should provide the current user to the searcher class" do