From feaf16d8784b3686abb34dbb0431f47b52bb443b Mon Sep 17 00:00:00 2001
From: luisramos0 <luisramos0@gmail.com>
Date: Tue, 16 Oct 2018 17:30:57 +0100
Subject: [PATCH] Fix bug in subscriptions address controller where the country
 id lookup was not exact and states returned were incorrect. Add unit tests to
 cover different cases

---
 .../controllers/address_controller.js.coffee  | 18 +++++---
 ...ee => providers_controller_spec.js.coffee} |  0
 .../address_controller_spec.js.coffee         | 43 +++++++++++++++++++
 3 files changed, 54 insertions(+), 7 deletions(-)
 rename spec/javascripts/unit/admin/controllers/{providers_controller_decorator.js.coffee => providers_controller_spec.js.coffee} (100%)
 create mode 100644 spec/javascripts/unit/admin/subscriptions/controllers/address_controller_spec.js.coffee

diff --git a/app/assets/javascripts/admin/subscriptions/controllers/address_controller.js.coffee b/app/assets/javascripts/admin/subscriptions/controllers/address_controller.js.coffee
index 02c78e3615..01b5e3d9c2 100644
--- a/app/assets/javascripts/admin/subscriptions/controllers/address_controller.js.coffee
+++ b/app/assets/javascripts/admin/subscriptions/controllers/address_controller.js.coffee
@@ -1,11 +1,21 @@
 angular.module("admin.subscriptions").controller "AddressController", ($scope, $filter, StatusMessage, availableCountries) ->
   $scope.countries = availableCountries
+
   $scope.statesFor = (country_id) ->
     return [] unless country_id
-    $filter('filter')(availableCountries, {id: country_id})[0].states
+    country = $filter('filter')(availableCountries, {id: country_id}, true)[0]
+    return [] unless country
+    country.states
+
   $scope.billStates = $scope.statesFor($scope.subscription.bill_address.country_id)
   $scope.shipStates = $scope.statesFor($scope.subscription.ship_address.country_id)
 
+  $scope.$watch 'subscription.bill_address.country_id', (newValue, oldValue) ->
+    $scope.billStates = $scope.statesFor(newValue) if newValue?
+
+  $scope.$watch 'subscription.ship_address.country_id', (newValue, oldValue) ->
+    $scope.shipStates = $scope.statesFor(newValue) if newValue?
+
   $scope.registerNextCallback 'address', ->
     $scope.subscription_form.$submitted = true
     if $scope.subscription_address_form.$valid
@@ -18,9 +28,3 @@ angular.module("admin.subscriptions").controller "AddressController", ($scope, $
   $scope.registerBackCallback 'address', ->
     StatusMessage.clear()
     $scope.setView('details')
-
-  $scope.$watch 'subscription.bill_address.country_id', (newValue, oldValue) ->
-    $scope.billStates = $scope.statesFor(newValue) if newValue?
-
-  $scope.$watch 'subscription.ship_address.country_id', (newValue, oldValue) ->
-    $scope.shipStates = $scope.statesFor(newValue) if newValue?
diff --git a/spec/javascripts/unit/admin/controllers/providers_controller_decorator.js.coffee b/spec/javascripts/unit/admin/controllers/providers_controller_spec.js.coffee
similarity index 100%
rename from spec/javascripts/unit/admin/controllers/providers_controller_decorator.js.coffee
rename to spec/javascripts/unit/admin/controllers/providers_controller_spec.js.coffee
diff --git a/spec/javascripts/unit/admin/subscriptions/controllers/address_controller_spec.js.coffee b/spec/javascripts/unit/admin/subscriptions/controllers/address_controller_spec.js.coffee
new file mode 100644
index 0000000000..c622906893
--- /dev/null
+++ b/spec/javascripts/unit/admin/subscriptions/controllers/address_controller_spec.js.coffee
@@ -0,0 +1,43 @@
+describe "AddressController", ->
+  scope = null
+  subscription = { id: 1 }
+
+  states_in_spain = [{id: 55, name: "CAT", abbr: "CAT"}]
+  states_in_portugal = [{id: 55, name: "ACT", abbr: "ACT"}, {id: 5, name: "BFT", abbr: "BFT"}]
+  availableCountries = [
+    {id: 9, name: "Australia", states: []},
+    {id: 119, name: "Spain", states: states_in_spain},
+    {id: 19, name: "Portugal", states: states_in_portugal}
+  ]
+
+  beforeEach ->
+    module('admin.subscriptions')
+
+    inject ($controller, $rootScope) ->
+      scope = $rootScope
+
+      scope.registerNextCallback = () ->
+      scope.registerBackCallback = () ->
+      scope.subscription = subscription
+      subscription.bill_address = {country_id: 1}
+      subscription.ship_address = {country_id: 2}
+      $controller 'AddressController', {$scope: scope, availableCountries: availableCountries}
+
+  describe "statesFor", ->
+    it "returns empty array for nil country id", ->
+      expect(scope.statesFor(null)).toEqual []
+
+    it "returns empty array for country id not in availableCountries", ->
+      expect(scope.statesFor(10)).toEqual []
+
+    it "returns empty array for country id in availableCountries but without states", ->
+      expect(scope.statesFor(9)).toEqual []
+
+    it "returns states for country id in availableCountries with states", ->
+      expect(scope.statesFor(119)).toEqual states_in_spain
+
+    it "returns empty array for country id (11) in availableCountries but only as part of other country id (119)", ->
+      expect(scope.statesFor(11)).toEqual []
+
+    it "returns states for country id (19) in availableCountries with states even if other country ids contain the requested id (119)", ->
+      expect(scope.statesFor(19)).toEqual states_in_portugal