From fb980981fb8068a1e5cb38c350ed45c412dc30be Mon Sep 17 00:00:00 2001
From: Rohan Mitchell <rohan@rohanmitchell.com>
Date: Thu, 11 Dec 2014 11:15:00 +1100
Subject: [PATCH] Display variant override errors

---
 .../variant_overrides_controller.js.coffee    | 18 +++++++++--
 spec/features/admin/variant_overrides_spec.rb | 32 +++++++++++++++++--
 ...ariant_overrides_controller_spec.js.coffee | 14 +++++++-
 3 files changed, 59 insertions(+), 5 deletions(-)

diff --git a/app/assets/javascripts/admin/controllers/variant_overrides_controller.js.coffee b/app/assets/javascripts/admin/controllers/variant_overrides_controller.js.coffee
index 560bd975e7..3ef79b4b37 100644
--- a/app/assets/javascripts/admin/controllers/variant_overrides_controller.js.coffee
+++ b/app/assets/javascripts/admin/controllers/variant_overrides_controller.js.coffee
@@ -46,7 +46,21 @@ angular.module("ofn.admin").controller "AdminVariantOverridesCtrl", ($scope, $ti
       DirtyVariantOverrides.save()
       .success (data) ->
         DirtyVariantOverrides.clear()
-        #VariantOverrides.update data.variant_overrides
         $timeout -> StatusMessage.display 'success', 'Changes saved.'
       .error (data, status) ->
-        $timeout -> StatusMessage.display 'failure', 'Oh no!'
+        $timeout -> StatusMessage.display 'failure', $scope.updateError(data, status)
+
+
+  $scope.updateError = (data, status) ->
+    if status == 401
+      "I couldn't get authorisation to save those changes, so they remain unsaved."
+
+    else if status == 400 && data.errors?
+      errors = []
+      for field, field_errors of data.errors
+        errors = errors.concat field_errors
+      errors = errors.join ', '
+      "I had some trouble saving: #{errors}"
+
+    else
+      "Oh no! I was unable to save your changes."
diff --git a/spec/features/admin/variant_overrides_spec.rb b/spec/features/admin/variant_overrides_spec.rb
index b1e112dfbc..98d2251a31 100644
--- a/spec/features/admin/variant_overrides_spec.rb
+++ b/spec/features/admin/variant_overrides_spec.rb
@@ -14,7 +14,7 @@ feature %q{
   let!(:hub) { create(:distributor_enterprise) }
   let!(:hub2) { create(:distributor_enterprise) }
   let!(:producer) { create(:supplier_enterprise) }
-  let!(:er) { create(:enterprise_relationship, parent: producer, child: hub,
+  let!(:er1) { create(:enterprise_relationship, parent: producer, child: hub,
                      permissions_list: [:add_to_order_cycle]) }
 
   context "as an enterprise user" do
@@ -41,7 +41,7 @@ feature %q{
       let!(:variant) { create(:variant, product: product, unit_value: 1, price: 1.23, on_hand: 12) }
       let!(:producer2) { create(:supplier_enterprise) }
       let!(:product2) { create(:simple_product, supplier: producer2) }
-      let!(:er) { create(:enterprise_relationship, parent: producer2, child: hub2,
+      let!(:er2) { create(:enterprise_relationship, parent: producer2, child: hub2,
                          permissions_list: [:add_to_order_cycle]) }
 
       before do
@@ -84,6 +84,34 @@ feature %q{
           vo.price.should == 777.77
           vo.count_on_hand.should == 123
         end
+
+        it "displays an error when unauthorised to access the page" do
+          fill_in "variant-overrides-#{variant.id}-price", with: '777.77'
+          fill_in "variant-overrides-#{variant.id}-count-on-hand", with: '123'
+          page.should have_content "Changes to one override remain unsaved."
+
+          user.enterprises.clear
+
+          expect do
+            click_button 'Save Changes'
+            page.should have_content "I couldn't get authorisation to save those changes, so they remain unsaved."
+          end.to change(VariantOverride, :count).by(0)
+        end
+
+        it "displays an error when unauthorised to update a particular override" do
+          fill_in "variant-overrides-#{variant.id}-price", with: '777.77'
+          fill_in "variant-overrides-#{variant.id}-count-on-hand", with: '123'
+          page.should have_content "Changes to one override remain unsaved."
+
+          EnterpriseRole.where(user_id: user).where('enterprise_id != ?', producer).destroy_all
+          er1.destroy
+          er2.destroy
+
+          expect do
+            click_button 'Save Changes'
+            page.should have_content "I couldn't get authorisation to save those changes, so they remain unsaved."
+          end.to change(VariantOverride, :count).by(0)
+        end
       end
 
       context "with overrides" do
diff --git a/spec/javascripts/unit/admin/controllers/variant_overrides_controller_spec.js.coffee b/spec/javascripts/unit/admin/controllers/variant_overrides_controller_spec.js.coffee
index 410d75cf50..bdb62e8d37 100644
--- a/spec/javascripts/unit/admin/controllers/variant_overrides_controller_spec.js.coffee
+++ b/spec/javascripts/unit/admin/controllers/variant_overrides_controller_spec.js.coffee
@@ -42,4 +42,16 @@ describe "VariantOverridesCtrl", ->
     it "does nothing when no selection has been made", ->
       scope.hub_id = ''
       scope.selectHub
-      expect(scope.hub).toBeNull
\ No newline at end of file
+      expect(scope.hub).toBeNull
+
+  describe "updating", ->
+    describe "error messages", ->
+      it "returns an unauthorised message upon 401", ->
+        expect(scope.updateError({}, 401)).toEqual "I couldn't get authorisation to save those changes, so they remain unsaved."
+
+      it "returns errors when they are provided", ->
+        data = {errors: {base: ["Hub can't be blank", "Variant can't be blank"]}}
+        expect(scope.updateError(data, 400)).toEqual "I had some trouble saving: Hub can't be blank, Variant can't be blank"
+
+      it "returns a generic message otherwise", ->
+        expect(scope.updateError({}, 500)).toEqual "Oh no! I was unable to save your changes."
\ No newline at end of file