fix(bhCurrencySelect): do not select disabled vals

This commit fixes a bug that will select the provided currencyId even if that currency is disabled. It also improves performance by using Angular's component hooks for initialization and watching changes. The disabled states are far more obvious using `text-muted`. Finally, an HTML "title" attribute is used on disabled radio buttons to make it abundantly clear why the radio button is disabled.
IMA-WorldHealth · Aug 24, 2016 · 40579ca · 40579ca
1 parent ada250d
commit 40579ca
Show file tree

Hide file tree

Showing 5 changed files with 95 additions and 65 deletions.
diff --git a/client/src/i18n/en.json b/client/src/i18n/en.json
@@ -282,6 +282,7 @@
       "CREDIT_NOTE"          : "Credit Note",
       "DANGER_ZONE"          : "Danger Zone",
       "DELETE_SUCCESS"       : "Delete success",
+      "DISABLED_CURRENCY"    : "This currency is unusable since there is no account set for it.  Use the Cashbox Management module to set an account for this currency and cashbox.",
       "FINANCIAL_DETAIL"     : "Financial details",
       "FOUND"                : "Found",
       "GROUPS_DEBTOR"        : "Assign a new debtor group from the options below. Press 'confirm' to permanently update the patient's debtor group",

diff --git a/client/src/js/components/bhCurrencySelect.js b/client/src/js/components/bhCurrencySelect.js
@@ -1,17 +1,17 @@
 angular.module('bhima.components')
-.component('bhCurrencySelect', {
-  controller : bhCurrencySelect,
-  templateUrl : 'partials/templates/bhCurrencySelect.tmpl.html',
-  bindings : {
-    validationTrigger: '<',
-    currencyId: '=',
-    disableIds: '<?',
-    onChange: '&?',
-    cashboxId: '<?'
-  }
-});
+  .component('bhCurrencySelect', {
+    controller : bhCurrencySelect,
+    templateUrl : 'partials/templates/bhCurrencySelect.tmpl.html',
+    bindings : {
+      currencyId: '=',
+      validationTrigger: '<',
+      disableIds: '<?',
+      onChange: '&?',
+      cashboxId: '<?'
+    }
+  });
 
-bhCurrencySelect.$inject = [ '$scope', 'CurrencyService' ];
+bhCurrencySelect.$inject = ['CurrencyService'];
 
 /**
  * @class bhCurrencySelect
@@ -36,6 +36,7 @@ bhCurrencySelect.$inject = [ '$scope', 'CurrencyService' ];
  *
  *  - [disable-ids]
  *      an array of currency ids to be disabled as required.
+ *
  *  - [cashbox-id]
  *      the cashbox id of the bound cashbox
  *
@@ -57,45 +58,66 @@ bhCurrencySelect.$inject = [ '$scope', 'CurrencyService' ];
  *   validation-trigger="ParentForm.$submitted"
  *   >
  * </bh-currency-select>
+ *
+ * @requires services/CurrencyService
  */
-function bhCurrencySelect($scope, Currencies) {
+function bhCurrencySelect(Currencies) {
   var $ctrl = this;
+  var isArray = angular.isArray;
 
-  // bind the currency service to the view
-  $ctrl.service = Currencies;
   $ctrl.valid = true;
 
-  // default currencies to an empty list
-  $ctrl.currencies = [];
-
   // default to noop() if an onChange() method was not passed in
-  $ctrl.onChange = $ctrl.onChange || angular.noop();
+  $ctrl.onChange = $ctrl.onChange || angular.noop;
 
-  // load all the available currencies
-  Currencies.read()
-  .then(function (currencies) {
+  $ctrl.$onInit = function onInit() {
 
-    // cache a label for faster view rendering
-    currencies.forEach(function (currency) {
-      currency.label = Currencies.format(currency.id);
-    });
+    // load all the available currencies
+    Currencies.read()
+      .then(function (currencies) {
 
-    $ctrl.currencies = currencies;
-  });
+        // cache a label for faster view rendering
+        currencies.forEach(function (currency) {
+          currency.label = Currencies.format(currency.id);
+        });
 
-  // watch the disabledIds array for changes, and disable the ids in the the
-  // view based on which ids are present in it
-  $scope.$watchCollection('$ctrl.disableIds', function (array) {
-    if (!array) { return; }
+        $ctrl.currencies = currencies;
+      });
+  };
+
+  $ctrl.$onChanges = function onChanges(changes) {
+    if (changes.disableIds) {
+      digestDisableIds(changes.disableIds.currentValue);
+    }
+
+    if (changes.onChange) {
+      $ctrl.onChange = changes.onChange.currentValue;
+    }
+  };
+
+  function digestDisableIds(disabledIds) {
+
+    // make sure there is something to digest
+    if (!isArray(disabledIds)) { return; }
+    if (!isArray($ctrl.currencies)) { return; }
 
     // loop through the currencies, disabling the currencies with ids in the
-    // disabledIds array.
+    // disableIds array.
     $ctrl.currencies.forEach(function (currency) {
-      currency.disabled = array.indexOf(currency.id) > -1;
+      var disabled = disabledIds.indexOf(currency.id) > -1;
+      currency.disabled = disabled;
+      currency.title = disabled ?  'FORM.INFO.DISABLED_CURRENCY' : '';
     });
 
-    // if the two array lengths are equal, it means every currency is disabled
-    $ctrl.valid = ($ctrl.currencies.length !== array.length);
+    // make sure we haven't defaulted to a currency that is not allowed by this casbhox
+    // if so, delete it
+    if (disabledIds.indexOf($ctrl.currencyId) > -1) {
+      delete $ctrl.currencyId;
+    }
+
+    // if the two array lengths are equal, it means every currency is disabled.
+    // there is no possible $valid state.
+    $ctrl.valid = ($ctrl.currencies.length !== disabledIds.length);
     $ctrl.form.currency.$setValidity('currency', $ctrl.valid);
-  });
+  }
 }
diff --git a/client/src/js/services/CurrencyService.js b/client/src/js/services/CurrencyService.js
@@ -1,16 +1,20 @@
 angular.module('bhima.services')
-.service('CurrencyService', CurrencyService);
+  .service('CurrencyService', CurrencyService);
 
-CurrencyService.$inject = [ '$http', '$q', 'util' ];
+CurrencyService.$inject = [
+  '$http', '$q', 'util'
+];
 
 /**
-* Currency Service
-*
-* This service is responsible for reading currencies from the database.  It
-* maintains a local cache so that currencies are not fetched multiple times.
-*
-* @module services/CurrencyService
-*/
+ * @module services/CurrencyService
+ *
+ * Currency Service
+ *
+ * @description
+ * This service is responsible for reading currencies from the database.  It
+ * maintains a local cache so that currencies are not fetched multiple times.
+ *
+ */
 function CurrencyService($http, $q, util) {
   var service = this;
   var baseUrl = '/currencies';
@@ -48,15 +52,15 @@ function CurrencyService($http, $q, util) {
     if (cache) { return $q.resolve(cache); }
 
     return $http.get(baseUrl)
-    .then(util.unwrapHttpResponse)
-    .then(function (currencies) {
+      .then(util.unwrapHttpResponse)
+      .then(function (currencies) {
 
-      // cache currencies to avoid future HTTP lookups.
-      cache = currencies;
-      map = buildMap(currencies);
+        // cache currencies to avoid future HTTP lookups.
+        cache = currencies;
+        map = buildMap(currencies);
 
-      return cache;
-    });
+        return cache;
+      });
   }
 
   /**
@@ -73,18 +77,18 @@ function CurrencyService($http, $q, util) {
 
     // fetch the currency from the server
     return $http.get(baseUrl.concat('/' + id))
-    .then(util.unwrapHttpResponse)
-    .then(function (currency) {
+      .then(util.unwrapHttpResponse)
+      .then(function (currency) {
 
-      // ensure that the map exists
-      if (!map) { map = {}; }
+        // ensure that the map exists
+        if (!map) { map = {}; }
 
-      // cache the currency for later
-      map[currency.id] = currency;
+        // cache the currency for later
+        map[currency.id] = currency;
 
-      // return the fetched currency
-      return currency;
-    });
+        // return the fetched currency
+        return currency;
+      });
   }
 
   function buildMap(currencies) {

diff --git a/client/src/partials/cash/modals/transfer.modal.js b/client/src/partials/cash/modals/transfer.modal.js
@@ -69,6 +69,8 @@ function CashTransferModalController(Currencies, Vouchers, Cashboxes, Accounts,
 
   function loadAccountDetails() {
 
+    console.log('Loading Account Details...');
+
     var transferAccountId;
     var cashAccountId;
 

diff --git a/client/src/partials/templates/bhCurrencySelect.tmpl.html b/client/src/partials/templates/bhCurrencySelect.tmpl.html
@@ -1,20 +1,21 @@
 <ng-form name="$ctrl.form">
   <div class="radio" ng-class="{ 'has-error' : ($ctrl.validationTrigger && $ctrl.form.$invalid) || !$ctrl.valid }" data-bh-currency-select>
-    <p class="control-label">
+    <p class="control-label" style="margin-bottom:5px;">
       <strong>{{ "FORM.LABELS.CURRENCY" | translate }}</strong>
     </p>
 
     <!-- repeat each currency in the application -->
     <label
       ng-repeat="currency in $ctrl.currencies track by currency.id"
       class="radio-inline"
-      ng-class="{ 'disabled' : currency.disabled }">
+      title="{{currency.title | translate}}"
+      ng-class="{ 'disabled text-muted' : currency.disabled }">
       <input
         name="currency"
         type="radio"
         ng-model="$ctrl.currencyId"
         ng-value="currency.id"
-        ng-change="$ctrl.onChange"
+        ng-change="$ctrl.onChange()"
         data-currency-option="{{ currency.id }}"
         ng-disabled="currency.disabled"
         required>