From 5eadb33db94475f4895f31e188de5096a5b40cad Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Fri, 5 May 2017 11:44:05 +1000 Subject: [PATCH] Using changes_allowed? to authorize cancellation of line_items --- app/models/spree/ability_decorator.rb | 3 +- app/models/spree/order_decorator.rb | 4 - .../controllers/line_items_controller_spec.rb | 155 +++++++++++------- spec/factories.rb | 2 +- 4 files changed, 97 insertions(+), 67 deletions(-) diff --git a/app/models/spree/ability_decorator.rb b/app/models/spree/ability_decorator.rb index 6c5cad6fb3..2b3843bb6e 100644 --- a/app/models/spree/ability_decorator.rb +++ b/app/models/spree/ability_decorator.rb @@ -53,7 +53,8 @@ class AbilityDecorator def add_shopping_abilities(user) can [:destroy], Spree::LineItem do |item| - item.andand.order.andand.can_remove_items? user + user == item.order.user && + item.order.changes_allowed? end can [:cancel], Spree::Order do |order| order.user == user diff --git a/app/models/spree/order_decorator.rb b/app/models/spree/order_decorator.rb index 2691b15798..4ed49728ff 100644 --- a/app/models/spree/order_decorator.rb +++ b/app/models/spree/order_decorator.rb @@ -282,10 +282,6 @@ Spree::Order.class_eval do Delayed::Job.enqueue ConfirmOrderJob.new(id) unless account_invoice? end - def can_remove_items?(user) - user_id == user.id && distributor.andand.allow_order_changes? && order_cycle.andand.open? - end - def changes_allowed? complete? && distributor.andand.allow_order_changes? && order_cycle.andand.open? end diff --git a/spec/controllers/line_items_controller_spec.rb b/spec/controllers/line_items_controller_spec.rb index 2b7ee1c247..ba78537fd7 100644 --- a/spec/controllers/line_items_controller_spec.rb +++ b/spec/controllers/line_items_controller_spec.rb @@ -1,72 +1,105 @@ require 'spec_helper' describe LineItemsController do - let(:item) { create(:line_item) } let(:user) { create(:user) } let(:distributor) { create(:distributor_enterprise) } let(:order_cycle) { create(:simple_order_cycle) } - let(:completed_order) do - order = create(:completed_order_with_totals, user: user, distributor: distributor, order_cycle: order_cycle) - while !order.completed? do break unless order.next! end - order - end - let(:item_with_oc) do - item.order.user = user - item.order.order_cycle = order_cycle - item.order.distributor = distributor - item.order.save - item + + context "listing bought items" do + let!(:completed_order) do + order = create(:completed_order_with_totals, user: user, distributor: distributor, order_cycle: order_cycle) + while !order.completed? do break unless order.next! end + order + end + + before do + controller.stub spree_current_user: user + controller.stub current_order_cycle: order_cycle + controller.stub current_distributor: distributor + end + + it "lists items bought by the user from the same shop in the same order_cycle" do + get :index, { format: :json } + expect(response.status).to eq 200 + json_response = JSON.parse(response.body) + expect(json_response.length).to eq completed_order.line_items(:reload).count + expect(json_response[0]['id']).to eq completed_order.line_items.first.id + end end - it "lists items bought by the user from the same shop in the same order_cycle" do - controller.stub spree_current_user: completed_order.user - controller.stub current_order_cycle: item_with_oc.order.order_cycle - controller.stub current_distributor: item_with_oc.order.distributor - get :index, { format: :json } - expect(response.status).to eq 200 - json_response = JSON.parse(response.body) - expect(json_response.length).to eq completed_order.line_items(:reload).count - expect(json_response[0]['id']).to eq completed_order.line_items.first.id - end + describe "destroying a line item on a completed order" do + let(:item) do + order = create(:completed_order_with_totals) + item = create(:line_item, order: order) + while !order.completed? do break unless order.next! end + item + end - it "fails without line item id" do - expect { delete :destroy }.to raise_error - end + before { controller.stub spree_current_user: item.order.user } - it "denies deletion without order cycle" do - request = { format: :json, id: item } - delete :destroy, request - expect(response.status).to eq 403 - expect { item.reload }.to_not raise_error - end + context "without a line item id" do + it "fails and raises an error" do + expect { delete :destroy }.to raise_error + end + end - it "denies deletion without user" do - request = { format: :json, id: item_with_oc } - delete :destroy, request - expect(response.status).to eq 403 - expect { item.reload }.to_not raise_error - end + context "with a line item id" do + let(:params) { { format: :json, id: item } } - it "denies deletion if editing is not allowed" do - controller.stub spree_current_user: item.order.user - request = { format: :json, id: item_with_oc } - delete :destroy, request - expect(response.status).to eq 403 - expect { item.reload }.to_not raise_error - end + context "without a user" do + it "denies deletion" do + delete :destroy, params + expect(response.status).to eq 403 + expect { item.reload }.to_not raise_error + end + end - it "deletes the line item if allowed" do - controller.stub spree_current_user: item.order.user - distributor = item_with_oc.order.distributor - distributor.allow_order_changes = true - distributor.save - request = { format: :json, id: item_with_oc } - delete :destroy, request - expect(response.status).to eq 204 - expect { item.reload }.to raise_error - end + context "where the item's order is associated with the current user" do + before { item.order.update_attributes(user_id: user.id) } + + context "without an order cycle" do + it "denies deletion" do + delete :destroy, params + expect(response.status).to eq 403 + expect { item.reload }.to_not raise_error + end + end + + context "with an order cycle" do + before { item.order.update_attributes(order_cycle_id: order_cycle.id) } + + context "without a distributor" do + it "denies deletion" do + delete :destroy, params + expect(response.status).to eq 403 + expect { item.reload }.to_not raise_error + end + end + + context "where the item's order has a distributor" do + before { item.order.update_attributes(distributor_id: distributor.id) } + context "where changes are not allowed" do + it "denies deletion" do + delete :destroy, params + expect(response.status).to eq 403 + expect { item.reload }.to_not raise_error + end + end + + context "where changes are allowed" do + before { distributor.update_attributes(allow_order_changes: true) } + + it "deletes the line item" do + delete :destroy, params + expect(response.status).to eq 204 + expect { item.reload }.to raise_error + end + end + end + end + end + end - describe "destroying a line item" do context "where shipping and payment fees apply" do let(:distributor) { create(:distributor_enterprise, charges_sales_tax: true, allow_order_changes: true) } let(:shipping_fee) { 3 } @@ -107,19 +140,19 @@ describe LineItemsController do let(:variant) { create(:variant) } let(:distributor) { create(:distributor_enterprise, allow_order_changes: true) } let(:order_cycle) { create(:simple_order_cycle, distributors: [distributor]) } - let(:enterprise_fee) { create(:enterprise_fee, calculator: Spree::Calculator::PerItem.new ) } - let!(:exchange) { create(:exchange, sender: variant.product.supplier, receiver: order_cycle.coordinator, variants: [variant], enterprise_fees: [enterprise_fee]) } + let(:enterprise_fee) { create(:enterprise_fee, calculator: build(:calculator_per_item) ) } + let!(:exchange) { create(:exchange, incoming: true, sender: variant.product.supplier, receiver: order_cycle.coordinator, variants: [variant], enterprise_fees: [enterprise_fee]) } let!(:order) do - order = create(:order, distributor: distributor, order_cycle: order_cycle, user: user) - order.line_items << build(:line_item, variant: variant) + order = create(:completed_order_with_totals, user: user, distributor: distributor, order_cycle: order_cycle) + order.reload.line_items.first.update_attributes(variant_id: variant.id) + while !order.completed? do break unless order.next! end order.update_distribution_charge! - order.save! order end let(:params) { { format: :json, id: order.line_items.first } } it "updates the fees" do - expect(order.adjustment_total).to eq enterprise_fee.calculator.preferred_amount + expect(order.reload.adjustment_total).to eq enterprise_fee.calculator.preferred_amount controller.stub spree_current_user: user delete :destroy, params diff --git a/spec/factories.rb b/spec/factories.rb index 14f08ca619..5b4df5cf38 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -263,7 +263,7 @@ FactoryGirl.define do shipping_method do shipping_calculator = build(:calculator_per_item, preferred_amount: shipping_fee) - create(:shipping_method, calculator: shipping_calculator, require_ship_address: false) + create(:shipping_method, calculator: shipping_calculator, require_ship_address: false, distributors: [distributor]) end after(:create) do |order, evaluator|