From 6586e67a5c4eec459557c19f64bc45e3de75bc8c Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Fri, 17 Jun 2016 14:47:04 +1000 Subject: [PATCH] Better messaging around deletion of customers --- .../customers/services/customers.js.coffee | 8 ++++- .../utils/services/info_dialog.js.coffee | 12 +++++++ .../templates/admin/info_dialog.html.haml | 9 ++++++ .../admin/components/info_dialog.css.scss | 28 +++++++++++++++++ app/controllers/admin/customers_controller.rb | 1 + app/models/customer.rb | 8 +++++ spec/features/admin/customers_spec.rb | 31 ++++++++++++++----- 7 files changed, 89 insertions(+), 8 deletions(-) create mode 100644 app/assets/javascripts/admin/utils/services/info_dialog.js.coffee create mode 100644 app/assets/javascripts/templates/admin/info_dialog.html.haml create mode 100644 app/assets/stylesheets/admin/components/info_dialog.css.scss diff --git a/app/assets/javascripts/admin/customers/services/customers.js.coffee b/app/assets/javascripts/admin/customers/services/customers.js.coffee index a9f4f0102f..cd4c9bbb44 100644 --- a/app/assets/javascripts/admin/customers/services/customers.js.coffee +++ b/app/assets/javascripts/admin/customers/services/customers.js.coffee @@ -1,4 +1,4 @@ -angular.module("admin.customers").factory "Customers", ($q, RequestMonitor, CustomerResource, CurrentShop) -> +angular.module("admin.customers").factory "Customers", ($q, InfoDialog, RequestMonitor, CustomerResource, CurrentShop) -> new class Customers customers: [] @@ -14,6 +14,12 @@ angular.module("admin.customers").factory "Customers", ($q, RequestMonitor, Cust CustomerResource.destroy params, => i = @customers.indexOf customer @customers.splice i, 1 unless i < 0 + , (response) => + errors = response.data.errors + if errors? + InfoDialog.open 'error', errors[0] + else + InfoDialog.open 'error', "Could not delete customer: #{customer.email}" index: (params) -> request = CustomerResource.index(params, (data) => @customers = data) diff --git a/app/assets/javascripts/admin/utils/services/info_dialog.js.coffee b/app/assets/javascripts/admin/utils/services/info_dialog.js.coffee new file mode 100644 index 0000000000..bb63eb7d20 --- /dev/null +++ b/app/assets/javascripts/admin/utils/services/info_dialog.js.coffee @@ -0,0 +1,12 @@ +angular.module("admin.customers").factory 'InfoDialog', ($rootScope, $compile, $injector, $templateCache, DialogDefaults) -> + new class InfoDialog + open: (type, message) -> + scope = $rootScope.$new() + scope.message = message + scope.dialog_class = type + template = $compile($templateCache.get('admin/info_dialog.html'))(scope) + template.dialog(DialogDefaults) + template.dialog('open') + scope.close = -> + template.dialog('close') + null diff --git a/app/assets/javascripts/templates/admin/info_dialog.html.haml b/app/assets/javascripts/templates/admin/info_dialog.html.haml new file mode 100644 index 0000000000..2755715e46 --- /dev/null +++ b/app/assets/javascripts/templates/admin/info_dialog.html.haml @@ -0,0 +1,9 @@ +#info-dialog{ ng: { class: "dialog_class" } } + .message.clearfix.margin-bottom-30 + .icon.text-center + %i.icon-exclamation-sign + .text + {{ message }} + .action-buttons.text-center + %button{ ng: { click: "close()" } } + OK diff --git a/app/assets/stylesheets/admin/components/info_dialog.css.scss b/app/assets/stylesheets/admin/components/info_dialog.css.scss new file mode 100644 index 0000000000..d8295c6813 --- /dev/null +++ b/app/assets/stylesheets/admin/components/info_dialog.css.scss @@ -0,0 +1,28 @@ +#info-dialog { + .message { + .text, .icon { + position: relative; + float:left; + display: inline; + } + + .text { + padding-top: 10px; + width: 80%; + font-size: 1rem; + } + + .icon { + width: 20%; + font-size: 2rem; + } + } + + &.error { + .message { + .icon { + color: #da5354; + } + } + } +} diff --git a/app/controllers/admin/customers_controller.rb b/app/controllers/admin/customers_controller.rb index 433efbf69e..c1f5f6bf9a 100644 --- a/app/controllers/admin/customers_controller.rb +++ b/app/controllers/admin/customers_controller.rb @@ -48,6 +48,7 @@ module Admin invoke_callbacks(:destroy, :fails) respond_with(@object) do |format| format.html { redirect_to location_after_destroy } + format.json { render json: { errors: @object.errors.full_messages }, status: :conflict } end end end diff --git a/app/models/customer.rb b/app/models/customer.rb index b2141df9ec..c208c619ba 100644 --- a/app/models/customer.rb +++ b/app/models/customer.rb @@ -3,6 +3,8 @@ class Customer < ActiveRecord::Base belongs_to :enterprise belongs_to :user, class_name: Spree.user_class + has_many :orders, class_name: Spree::Order + before_destroy :check_for_orders before_validation :downcase_email before_validation :empty_code @@ -28,4 +30,10 @@ class Customer < ActiveRecord::Base def associate_user self.user = user || Spree::User.find_by_email(email) end + + def check_for_orders + return true unless orders.any? + errors[:base] << "Delete failed: customer has associated orders" + false + end end diff --git a/spec/features/admin/customers_spec.rb b/spec/features/admin/customers_spec.rb index 0e2665b39b..61caae507f 100644 --- a/spec/features/admin/customers_spec.rb +++ b/spec/features/admin/customers_spec.rb @@ -25,6 +25,15 @@ feature 'Customers' do # Prompts for a hub for a list of my managed enterprises expect(page).to have_select2 "shop_id", with_options: [managed_distributor1.name,managed_distributor2.name], without_options: [unmanaged_distributor.name] + select2_select managed_distributor2.name, from: "shop_id" + + # Loads the right customers + expect(page).to_not have_selector "tr#c_#{customer1.id}" + expect(page).to_not have_selector "tr#c_#{customer2.id}" + expect(page).to_not have_selector "tr#c_#{customer3.id}" + expect(page).to have_selector "tr#c_#{customer4.id}" + + # Changing Shops select2_select managed_distributor1.name, from: "shop_id" # Loads the right customers @@ -47,14 +56,22 @@ feature 'Customers' do expect(page).to_not have_selector "th.email" expect(page).to_not have_content customer1.email - # Changing Shops - select2_select managed_distributor2.name, from: "shop_id" + # Deleting + create(:order, customer: customer1) + expect{ + within "tr#c_#{customer1.id}" do + find("a.delete-customer").click + end + expect(page).to have_selector "#info-dialog .text", text: "Delete failed: customer has associated orders" + click_button "OK" + }.to_not change{Customer.count} - # Loads the right customers - expect(page).to_not have_selector "tr#c_#{customer1.id}" - expect(page).to_not have_selector "tr#c_#{customer2.id}" - expect(page).to_not have_selector "tr#c_#{customer3.id}" - expect(page).to have_selector "tr#c_#{customer4.id}" + expect{ + within "tr#c_#{customer2.id}" do + find("a.delete-customer").click + end + expect(page).to_not have_selector "tr#c_#{customer2.id}" + }.to change{Customer.count}.by(-1) end it "allows updating of attributes" do