From b17d8c2fe3515b8bd3569acc3230cd7861b44dd0 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Thu, 1 Mar 2018 16:59:10 +1100 Subject: [PATCH] Add validation of open and close dates for order cycles --- .../services/order_cycle.js.coffee | 25 ++++--- .../resources/services/order_cycles.js.coffee | 5 +- .../admin/order_cycles_controller.rb | 13 ++-- app/models/order_cycle.rb | 7 ++ config/locales/en.yml | 6 ++ .../admin/order_cycles_controller_spec.rb | 73 +++++++++++++------ .../unit/order_cycle_spec.js.coffee | 4 +- spec/models/order_cycle_spec.rb | 11 ++- 8 files changed, 101 insertions(+), 43 deletions(-) diff --git a/app/assets/javascripts/admin/order_cycles/services/order_cycle.js.coffee b/app/assets/javascripts/admin/order_cycles/services/order_cycle.js.coffee index 2665ac9380..f35db6e3b5 100644 --- a/app/assets/javascripts/admin/order_cycles/services/order_cycle.js.coffee +++ b/app/assets/javascripts/admin/order_cycles/services/order_cycle.js.coffee @@ -151,23 +151,28 @@ angular.module('admin.orderCycles').factory 'OrderCycle', ($resource, $window, S return unless @confirmNoDistributors() oc = new OrderCycleResource({order_cycle: this.dataForSubmit()}) oc.$create (data) -> - if data['success'] - $window.location = destination + $window.location = destination + , (response) -> + if response.data.errors? + StatusMessage.display('failure', response.data.errors[0]) else - console.log('Failed to create order cycle') + StatusMessage.display('failure', 'Failed to create order cycle') update: (destination, form) -> return unless @confirmNoDistributors() oc = new OrderCycleResource({order_cycle: this.dataForSubmit()}) oc.$update {order_cycle_id: this.order_cycle.id, reloading: (if destination? then 1 else 0)}, (data) => - if data['success'] - form.$setPristine() if form - if destination? - $window.location = destination - else - StatusMessage.display 'success', t('js.order_cycles.update_success') + form.$setPristine() if form + if destination? + $window.location = destination else - console.log('Failed to update order cycle') + StatusMessage.display 'success', t('js.order_cycles.update_success') + , (response) -> + if response.data.errors? + StatusMessage.display('failure', response.data.errors[0]) + else + StatusMessage.display('failure', 'Failed to create order cycle') + confirmNoDistributors: -> if @order_cycle.outgoing_exchanges.length == 0 diff --git a/app/assets/javascripts/admin/resources/services/order_cycles.js.coffee b/app/assets/javascripts/admin/resources/services/order_cycles.js.coffee index 426d291919..e1d6569ca4 100644 --- a/app/assets/javascripts/admin/resources/services/order_cycles.js.coffee +++ b/app/assets/javascripts/admin/resources/services/order_cycles.js.coffee @@ -45,7 +45,10 @@ angular.module("admin.resources").factory 'OrderCycles', ($q, $injector, OrderCy form.$setPristine() if form? StatusMessage.display('success', "Order cycles have been updated.") , (response) => - StatusMessage.display('failure', "Oh no! I was unable to save your changes.") + if response.data.errors? + StatusMessage.display('failure', response.data.errors[0]) + else + StatusMessage.display('failure', "Oh no! I was unable to save your changes.") saved: (order_cycle) -> @diff(order_cycle).length == 0 diff --git a/app/controllers/admin/order_cycles_controller.rb b/app/controllers/admin/order_cycles_controller.rb index af4d806811..1cbb8da43a 100644 --- a/app/controllers/admin/order_cycles_controller.rb +++ b/app/controllers/admin/order_cycles_controller.rb @@ -51,10 +51,10 @@ module Admin invoke_callbacks(:create, :after) flash[:notice] = I18n.t(:order_cycles_create_notice) format.html { redirect_to admin_order_cycles_path } - format.json { render :json => {:success => true} } + format.json { render :json => { success: true } } else format.html - format.json { render :json => {:success => false} } + format.json { render :json => { errors: @order_cycle.errors.full_messages }, status: :unprocessable_entity } end end end @@ -71,9 +71,9 @@ module Admin invoke_callbacks(:update, :after) flash[:notice] = I18n.t(:order_cycles_update_notice) if params[:reloading] == '1' format.html { redirect_to main_app.edit_admin_order_cycle_path(@order_cycle) } - format.json { render :json => {:success => true} } + format.json { render :json => { :success => true } } else - format.json { render :json => {:success => false} } + format.json { render :json => { errors: @order_cycle.errors.full_messages }, status: :unprocessable_entity } end end end @@ -85,7 +85,8 @@ module Admin end else respond_to do |format| - format.json { render :json => {:success => false} } + order_cycle = order_cycle_set.collection.find{ |oc| oc.errors.present? } + format.json { render :json => { errors: order_cycle.errors.full_messages }, status: :unprocessable_entity } end end end @@ -219,7 +220,7 @@ module Admin end def order_cycle_set - OrderCycleSet.new(@order_cycles, params[:order_cycle_set]) + @order_cycle_set ||= OrderCycleSet.new(@order_cycles, params[:order_cycle_set]) end def require_order_cycle_set_params diff --git a/app/models/order_cycle.rb b/app/models/order_cycle.rb index 9dcf118bab..4d8f443a68 100644 --- a/app/models/order_cycle.rb +++ b/app/models/order_cycle.rb @@ -14,6 +14,7 @@ class OrderCycle < ActiveRecord::Base attr_accessor :incoming_exchanges, :outgoing_exchanges validates_presence_of :name, :coordinator_id + validate :orders_close_at_after_orders_open_at? after_save :refresh_products_cache @@ -272,4 +273,10 @@ class OrderCycle < ActiveRecord::Base distributed_variants.include?(product.master) && (product.variants & distributed_variants).empty? end + + def orders_close_at_after_orders_open_at? + return if orders_open_at.blank? || orders_close_at.blank? + return if orders_close_at > orders_open_at + errors.add(:orders_close_at, :after_orders_open_at) + end end diff --git a/config/locales/en.yml b/config/locales/en.yml index 9e173bb183..315ee8960c 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -66,6 +66,8 @@ en: email: Customer E-Mail spree/payment: amount: Amount + order_cycle: + orders_close_at: Close date errors: models: spree/user: @@ -74,6 +76,10 @@ en: taken: "There's already an account for this email. Please login or reset your password." spree/order: no_card: There are no valid credit cards available + order_cycle: + attributes: + orders_close_at: + after_orders_open_at: must be after open date activemodel: errors: models: diff --git a/spec/controllers/admin/order_cycles_controller_spec.rb b/spec/controllers/admin/order_cycles_controller_spec.rb index 7c7e20336e..fa290c3703 100644 --- a/spec/controllers/admin/order_cycles_controller_spec.rb +++ b/spec/controllers/admin/order_cycles_controller_spec.rb @@ -13,10 +13,10 @@ module Admin describe "#index" do describe "when the user manages a coordinator" do let!(:coordinator) { create(:distributor_enterprise, owner: distributor_owner) } - let!(:oc1) { create(:simple_order_cycle, orders_close_at: 60.days.ago ) } - let!(:oc2) { create(:simple_order_cycle, orders_close_at: 40.days.ago ) } - let!(:oc3) { create(:simple_order_cycle, orders_close_at: 20.days.ago ) } - let!(:oc4) { create(:simple_order_cycle, orders_close_at: nil ) } + let!(:oc1) { create(:simple_order_cycle, orders_open_at: 70.days.ago, orders_close_at: 60.days.ago ) } + let!(:oc2) { create(:simple_order_cycle, orders_open_at: 70.days.ago, orders_close_at: 40.days.ago ) } + let!(:oc3) { create(:simple_order_cycle, orders_open_at: 70.days.ago, orders_close_at: 20.days.ago ) } + let!(:oc4) { create(:simple_order_cycle, orders_open_at: 70.days.ago, orders_close_at: nil ) } context "html" do it "doesn't load any data" do @@ -125,18 +125,34 @@ module Admin allow(form_applicator_mock).to receive(:go!) { nil } end - it "does not run the OrderCycleFormApplicator" do - expect(order_cycle.exchanges.incoming).to eq [incoming_exchange] - expect(order_cycle.exchanges.outgoing).to eq [outgoing_exchange] - expect(order_cycle.prefers_product_selection_from_coordinator_inventory_only?).to be false - spree_put :update, id: order_cycle.id, order_cycle: { name: 'Some new name', preferred_product_selection_from_coordinator_inventory_only: true } - expect(form_applicator_mock).to_not have_received(:go!) - order_cycle.reload - expect(order_cycle.exchanges.incoming).to eq [incoming_exchange] - expect(order_cycle.exchanges.outgoing).to eq [outgoing_exchange] - expect(order_cycle.name).to eq 'Some new name' - expect(order_cycle.prefers_product_selection_from_coordinator_inventory_only?).to be true - end + it "does not run the OrderCycleFormApplicator" do + expect(order_cycle.exchanges.incoming).to eq [incoming_exchange] + expect(order_cycle.exchanges.outgoing).to eq [outgoing_exchange] + expect(order_cycle.prefers_product_selection_from_coordinator_inventory_only?).to be false + spree_put :update, id: order_cycle.id, order_cycle: { name: 'Some new name', preferred_product_selection_from_coordinator_inventory_only: true } + expect(form_applicator_mock).to_not have_received(:go!) + order_cycle.reload + expect(order_cycle.exchanges.incoming).to eq [incoming_exchange] + expect(order_cycle.exchanges.outgoing).to eq [outgoing_exchange] + expect(order_cycle.name).to eq 'Some new name' + expect(order_cycle.prefers_product_selection_from_coordinator_inventory_only?).to be true + end + end + + context "when a validation error occurs" do + let(:params) { + { + format: :json, + id: order_cycle.id, + order_cycle: { orders_open_at: order_cycle.orders_close_at + 1.day } + } + } + + it "returns an error message" do + spree_put :update, params + json_response = JSON.parse(response.body) + expect(json_response['errors']).to be_present + end end end @@ -206,15 +222,18 @@ module Admin let!(:coordinator) { oc.coordinator } context "when I manage the coordinator of an order cycle" do - before { create(:enterprise_role, user: distributor_owner, enterprise: coordinator) } - - it "updates order cycle properties" do - spree_put :bulk_update, format: :json, order_cycle_set: { collection_attributes: { '0' => { + let(:params) do + { format: :json, order_cycle_set: { collection_attributes: { '0' => { id: oc.id, orders_open_at: Date.current - 21.days, orders_close_at: Date.current + 21.days, - } } } + } } } } + end + before { create(:enterprise_role, user: distributor_owner, enterprise: coordinator) } + + it "updates order cycle properties" do + spree_put :bulk_update, params oc.reload expect(oc.orders_open_at.to_date).to eq Date.current - 21.days expect(oc.orders_close_at.to_date).to eq Date.current + 21.days @@ -227,6 +246,18 @@ module Admin json_response = JSON.parse(response.body) expect(json_response['errors']).to eq I18n.t('admin.order_cycles.bulk_update.no_data') end + + context "when a validation error occurs" do + before do + params[:order_cycle_set][:collection_attributes]['0'][:orders_open_at] = Date.current + 25.days + end + + it "returns an error message" do + spree_put :bulk_update, params + json_response = JSON.parse(response.body) + expect(json_response['errors']).to be_present + end + end end context "when I do not manage the coordinator of an order cycle" do diff --git a/spec/javascripts/unit/order_cycle_spec.js.coffee b/spec/javascripts/unit/order_cycle_spec.js.coffee index 93fe10c6f8..6c6f3440e6 100644 --- a/spec/javascripts/unit/order_cycle_spec.js.coffee +++ b/spec/javascripts/unit/order_cycle_spec.js.coffee @@ -812,7 +812,7 @@ describe 'OrderCycle services', -> spyOn(OrderCycle, 'dataForSubmit').and.returnValue('this is the submit data') $httpBackend.expectPOST('/admin/order_cycles.json', { order_cycle: 'this is the submit data' - }).respond {success: false} + }).respond 400, { errors: [] } OrderCycle.create('/destination/page') $httpBackend.flush() @@ -840,7 +840,7 @@ describe 'OrderCycle services', -> spyOn(OrderCycle, 'dataForSubmit').and.returnValue('this is the submit data') $httpBackend.expectPUT('/admin/order_cycles.json?reloading=1', { order_cycle: 'this is the submit data' - }).respond {success: false} + }).respond 400, { errors: [] } OrderCycle.update('/destination/page') $httpBackend.flush() diff --git a/spec/models/order_cycle_spec.rb b/spec/models/order_cycle_spec.rb index 7a42f94164..efd0f3ba03 100644 --- a/spec/models/order_cycle_spec.rb +++ b/spec/models/order_cycle_spec.rb @@ -11,6 +11,11 @@ describe OrderCycle do oc.should_not be_valid end + it 'should not be valid when open date is after close date' do + oc = build(:simple_order_cycle, orders_open_at: Time.zone.now, orders_close_at: 1.minute.ago) + expect(oc).to_not be_valid + end + it "has a coordinator and associated fees" do oc = create(:simple_order_cycle) @@ -196,13 +201,13 @@ describe OrderCycle do let(:d1) { create(:enterprise) } let(:d2) { create(:enterprise) } let!(:e0) { create(:exchange, incoming: true, - order_cycle: oc, sender: create(:enterprise), receiver: oc.coordinator) + order_cycle: oc, sender: create(:enterprise), receiver: oc.coordinator) } let!(:e1) { create(:exchange, incoming: false, - order_cycle: oc, sender: oc.coordinator, receiver: d1) + order_cycle: oc, sender: oc.coordinator, receiver: d1) } let!(:e2) { create(:exchange, incoming: false, - order_cycle: oc, sender: oc.coordinator, receiver: d2) + order_cycle: oc, sender: oc.coordinator, receiver: d2) } let!(:p0) { create(:simple_product) } let!(:p1) { create(:simple_product) }