refactor(GUI): central error/warning messages function (#1193)

* Centralise drive error/warning messages with a function
  `.getDriveImageCompatibilityStatuses` in `lib/shared/drive-constraints.js`
  -- tests included
* Fix an error where several labels show at once
* Clarify the source drive label to 'Drive Contains Image'
* Remove label icons and make text bold to match Zeplin's design

Closes: https://github.com/resin-io/etcher/issues/1143
Closes: https://github.com/resin-io/etcher/issues/1144

lib/gui/components/drive-selector/controllers/drive-selector.js

@@ -16,6 +16,7 @@
 
 'use strict';
 
+const angular = require('angular');
 const _ = require('lodash');
 const messages = require('../../../../shared/messages');
 const constraints = require('../../../../shared/drive-constraints');
@@ -170,4 +171,91 @@ module.exports = function(
     });
   };
 
+  /**
+   * @summary Memoize ImmutableJS list reference
+   * @function
+   * @private
+   *
+   * @description
+   * This workaround is needed to avoid AngularJS from getting
+   * caught in an infinite digest loop when using `ngRepeat`
+   * over a function that returns a mutable version of an
+   * ImmutableJS object.
+   *
+   * The problem is that every time you call `myImmutableObject.toJS()`
+   * you will get a new object, whose reference is different from
+   * the one you previously got, even if the data is exactly the same.
+   *
+   * @param {Function} func - function that returns an ImmutableJS list
+   * @returns {Function} memoized function
+   *
+   * @example
+   * const getList = () => {
+   *   return Store.getState().toJS().myList;
+   * };
+   *
+   * const memoizedFunction = memoizeImmutableListReference(getList);
+   */
+  this.memoizeImmutableListReference = (func) => {
+    let previousTuples = [];
+
+    return (...restArgs) => {
+      let areArgsInTuple = false;
+      let state = Reflect.apply(func, this, restArgs);
+
+      previousTuples = _.map(previousTuples, ([ oldArgs, oldState ]) => {
+        if (angular.equals(oldArgs, restArgs)) {
+          areArgsInTuple = true;
+
+          if (angular.equals(state, oldState)) {
+
+            // Use the previously memoized state for this argument
+            state = oldState;
+          }
+
+          // Update the tuple state
+          return [ oldArgs, state ];
+        }
+
+        // Return the tuple unchanged
+        return [ oldArgs, oldState ];
+      });
+
+      // Add the state associated with these args to be memoized
+      if (!areArgsInTuple) {
+        previousTuples.push([ restArgs, state ]);
+
+      }
+
+      return state;
+    };
+  };
+
+  this.getDrives = this.memoizeImmutableListReference(() => {
+    return this.drives.getDrives();
+  });
+
+  /**
+   * @summary Get a drive's compatibility status object(s)
+   * @function
+   * @public
+   *
+   * @description
+   * Given a drive, return its compatibility status with the selected image,
+   * containing the status type (ERROR, WARNING), and accompanying
+   * status message.
+   *
+   * @returns {Object[]} list of objects containing statuses
+   *
+   * @example
+   * const statuses = DriveSelectorController.getDriveStatuses(drive);
+   *
+   * for ({ type, message } of statuses) {
+   *   // do something
+   * }
+   */
+  this.getDriveStatuses = this.memoizeImmutableListReference((drive) => {
+    return this.constraints.getDriveImageCompatibilityStatuses(drive, this.state.getImage());
+  });
+
 };

lib/gui/components/drive-selector/templates/drive-selector-modal.tpl.html

@@ -5,7 +5,7 @@
 
 <div class="modal-body">
   <ul class="list-group">
-    <li class="list-group-item" ng-repeat="drive in modal.drives.getDrives() track by drive.device"
+    <li class="list-group-item" ng-repeat="drive in modal.getDrives() track by drive.device"
       ng-disabled="!modal.constraints.isDriveValid(drive, modal.state.getImage())"
       ng-dblclick="modal.selectDriveAndClose(drive)"
       ng-click="modal.toggleDrive(drive)">
@@ -15,35 +15,13 @@
 
           <footer class="list-group-item-footer">
 
-            <span class="label label-warning"
+            <span class="label" ng-repeat="status in modal.getDriveStatuses(drive)"
-                ng-show="modal.constraints.isDriveLargeEnough(drive, modal.state.getImage()) && !modal.constraints.isDriveLocked(drive) && !modal.constraints.isDriveSizeRecommended(drive, modal.state.getImage())">
+              ng-class="{
-                <i class="glyphicon glyphicon-warning-sign"></i>
+                'label-warning': status.type === modal.constraints.COMPATIBILITY_STATUS_TYPES.WARNING,
-                Not Recommended</span>
+                'label-danger': status.type === modal.constraints.COMPATIBILITY_STATUS_TYPES.ERROR
-
+              }">
-            <!-- There can be a case where the device it not large enough, and it's also locked. -->
+              <b>{{ status.message }}</b>
-            <!-- Since in this case both labels will be displayed, we chose to only show the -->
+            </span>
-            <!-- "not large enough label", since from the point of view of the user, the locked -->
-            <!-- state is irrelevant if the drive is not large enough anyway. -->
-
-            <span class="label label-danger"
-              ng-hide="modal.constraints.isDriveLargeEnough(drive, modal.state.getImage())">
-                <i class="glyphicon glyphicon-resize-small"></i>
-                Too Small For Image</span>
-
-            <span class="label label-danger"
-              ng-show="modal.constraints.isDriveLocked(drive) && modal.constraints.isDriveLargeEnough(drive, modal.state.getImage())">
-                <i class="glyphicon glyphicon-lock"></i>
-                Locked</span>
-
-            <span class="label label-danger"
-              ng-show="modal.constraints.isSystemDrive(drive)">
-                <i class="glyphicon glyphicon-hdd"></i>
-                System Drive</span>
-
-            <span class="label label-danger"
-              ng-show="modal.constraints.isSourceDrive(drive, modal.state.getImage())">
-                <i class="glyphicon glyphicon-circle-arrow-down"></i>
-                Source Drive</span>
 
           </footer>
         </div>

lib/gui/models/drives.js

@@ -64,45 +64,6 @@ Drives.service('DrivesModel', function() {
     });
   };
 
-  /**
-   * @summary Memoize ImmutableJS list reference
-   * @function
-   * @private
-   *
-   * @description
-   * This workaround is needed to avoid AngularJS from getting
-   * caught in an infinite digest loop when using `ngRepeat`
-   * over a function that returns a mutable version of an
-   * ImmutableJS object.
-   *
-   * The problem is that every time you call `myImmutableObject.toJS()`
-   * you will get a new object, whose reference is different from
-   * the one you previously got, even if the data is exactly the same.
-   *
-   * @param {Function} func - function that returns an ImmutableJS list
-   * @returns {Function} memoized function
-   *
-   * @example
-   * const getList = () => {
-   *   return Store.getState().toJS().myList;
-   * };
-   *
-   * const memoizedFunction = memoizeImmutableListReference(getList);
-   */
-  const memoizeImmutableListReference = (func) => {
-    let previous = [];
-
-    return (...args) => {
-      const list = Reflect.apply(func, this, args);
-
-      if (!_.isEqual(list, previous)) {
-        previous = list;
-      }
-
-      return previous;
-    };
-  };
-
   /**
    * @summary Get detected drives
    * @function
@@ -113,9 +74,9 @@ Drives.service('DrivesModel', function() {
    * @example
    * const drives = DrivesModel.getDrives();
    */
-  this.getDrives = memoizeImmutableListReference(() => {
+  this.getDrives = () => {
     return Store.getState().toJS().availableDrives;
-  });
+  };
 
 });
 

lib/shared/drive-constraints.js

@@ -232,3 +232,152 @@ exports.isDriveValid = (drive, image) => {
 exports.isDriveSizeRecommended = (drive, image) => {
   return _.get(drive, [ 'size' ], UNKNOWN_SIZE) >= _.get(image, [ 'recommendedDriveSize' ], UNKNOWN_SIZE);
 };
+
+/**
+ * @summary Drive/image compatibility status messages.
+ * @public
+ * @type {Object}
+ *
+ * @description
+ * Status messages intended to be shown to the user.
+ */
+exports.COMPATIBILITY_STATUS_MESSAGES = {
+
+  /**
+   * @property {String} SIZE_NOT_RECOMMENDED
+   * @memberof COMPATIBILITY_STATUS_MESSAGES
+   *
+   * @description
+   * The image and drive compatibility is not recommended; happens when the
+   * actual drive size is smaller than the image's recommended drive size.
+   */
+  SIZE_NOT_RECOMMENDED: 'Not Recommended',
+
+  /**
+   * @property {String} TOO_SMALL
+   * @memberof COMPATIBILITY_STATUS_MESSAGES
+   *
+   * @description
+   * The drive is too small for the image.
+   */
+  TOO_SMALL: 'Too Small For Image',
+
+  /**
+   * @property {String} LOCKED
+   * @memberof COMPATIBILITY_STATUS_MESSAGES
+   *
+   * @description
+   * The drive is locked (e.g. the lock-tab on SD cards) and cannot be written to.
+   */
+  LOCKED: 'Locked',
+
+  /**
+   * @property {String} SYSTEM
+   * @memberof COMPATIBILITY_STATUS_MESSAGES
+   *
+   * @description
+   * The drive is a system drive and should not be written to.
+   */
+  SYSTEM: 'System Drive',
+
+  /**
+   * @property {String} CONTAINS_IMAGE
+   * @memberof COMPATIBILITY_STATUS_MESSAGES
+   *
+   * @description
+   * The drive contains the image and therefore cannot be written to.
+   */
+  CONTAINS_IMAGE: 'Drive Contains Image'
+};
+
+/**
+ * @summary Drive/image compatibility status types.
+ * @public
+ * @type {Object}
+ *
+ * @description
+ * Status types classifying what kind of message it is, i.e. error, warning.
+ */
+exports.COMPATIBILITY_STATUS_TYPES = {
+  WARNING: 1,
+  ERROR: 2
+};
+
+/**
+ * @summary Get drive/image compatibility in an object
+ * @function
+ * @public
+ *
+ * @description
+ * Given an image and a drive, return their compatibility status object
+ * containing the status type (ERROR, WARNING), and accompanying
+ * status message.
+ *
+ * @param {Object} drive - drive
+ * @param {Object} image - image
+ * @returns {Object[]} list of compatibility status objects
+ *
+ * @example
+ * const drive = {
+ *   device: '/dev/disk2',
+ *   name: 'My Drive',
+ *   size: 4000000000
+ * };
+ *
+ * const image = {
+ *   path: '/path/to/rpi.img',
+ *   size: 2000000000
+ *   recommendedDriveSize: 4000000000
+ * });
+ *
+ * const statuses = constraints.getDriveImageCompatibilityStatuses(drive, image);
+ *
+ * for ({ type, message } of statuses) {
+ *   if (type === constraints.COMPATIBILITY_STATUS_TYPES.WARNING) {
+ *     // do something
+ *   } else if (type === constraints.COMPATIBILITY_STATUS_TYPES.ERROR) {
+ *     // do something else
+ *   }
+ * }
+ */
+exports.getDriveImageCompatibilityStatuses = (drive, image) => {
+  const statusList = [];
+
+  // Mind the order of the if-statements if you modify.
+  if (exports.isSourceDrive(drive, image)) {
+    statusList.push({
+      type: exports.COMPATIBILITY_STATUS_TYPES.ERROR,
+      message: exports.COMPATIBILITY_STATUS_MESSAGES.CONTAINS_IMAGE
+    });
+
+  } else if (exports.isDriveLocked(drive)) {
+    statusList.push({
+      type: exports.COMPATIBILITY_STATUS_TYPES.ERROR,
+      message: exports.COMPATIBILITY_STATUS_MESSAGES.LOCKED
+    });
+
+  } else if (!_.isNil(drive) && !exports.isDriveLargeEnough(drive, image)) {
+    statusList.push({
+      type: exports.COMPATIBILITY_STATUS_TYPES.ERROR,
+      message: exports.COMPATIBILITY_STATUS_MESSAGES.TOO_SMALL
+    });
+
+  } else {
+
+    if (exports.isSystemDrive(drive)) {
+      statusList.push({
+        type: exports.COMPATIBILITY_STATUS_TYPES.WARNING,
+        message: exports.COMPATIBILITY_STATUS_MESSAGES.SYSTEM
+      });
+    }
+
+    if (!_.isNil(drive) && !exports.isDriveSizeRecommended(drive, image)) {
+      statusList.push({
+        type: exports.COMPATIBILITY_STATUS_TYPES.WARNING,
+        message: exports.COMPATIBILITY_STATUS_MESSAGES.SIZE_NOT_RECOMMENDED
+      });
+    }
+  }
+
+  return statusList;
+};

tests/gui/components/drive-selector.spec.js

@@ -0,0 +1,103 @@
+'use strict';
+
+const _ = require('lodash');
+const m = require('mochainon');
+const angular = require('angular');
+require('angular-mocks');
+
+describe('Browser: DriveSelector', function() {
+
+  beforeEach(angular.mock.module(
+    require('../../../lib/gui/components/drive-selector/drive-selector')
+  ));
+
+  describe('DriveSelectorController', function() {
+
+    let $controller;
+    let $rootScope;
+    let $q;
+    let $uibModalInstance;
+    let DrivesModel;
+    let SelectionStateModel;
+    let WarningModalService;
+    let AnalyticsService;
+
+    let controller;
+
+    beforeEach(angular.mock.inject(function(
+      _$controller_,
+      _$rootScope_,
+      _$q_,
+      _DrivesModel_, _SelectionStateModel_,
+      _WarningModalService_,
+      _AnalyticsService_
+    ) {
+      $controller = _$controller_;
+      $rootScope = _$rootScope_;
+      $q = _$q_;
+      $uibModalInstance = {};
+      DrivesModel = _DrivesModel_;
+      SelectionStateModel = _SelectionStateModel_;
+      WarningModalService = _WarningModalService_;
+      AnalyticsService = _AnalyticsService_;
+    }));
+
+    beforeEach(() => {
+      controller = $controller('DriveSelectorController', {
+        $scope: $rootScope.$new(),
+        $q,
+        $uibModalInstance,
+        DrivesModel,
+        SelectionStateModel,
+        WarningModalService,
+        AnalyticsService
+      });
+    });
+
+    describe('.memoizeImmutableListReference()', function() {
+
+      it('constant true should return memoized true', function() {
+        const memoizedConstTrue = controller.memoizeImmutableListReference(_.constant(true));
+        m.chai.expect(memoizedConstTrue()).to.be.true;
+      });
+
+      it('should reflect state changes', function() {
+        let stateA = false;
+        const memoizedStateA = controller.memoizeImmutableListReference(() => {
+          return stateA;
+        });
+
+        m.chai.expect(memoizedStateA()).to.be.false;
+
+        stateA = true;
+
+        m.chai.expect(memoizedStateA()).to.be.true;
+      });
+
+      it('should reflect different arguments', function() {
+        const memoizedParameter = controller.memoizeImmutableListReference(_.identity);
+
+        m.chai.expect(memoizedParameter(false)).to.be.false;
+        m.chai.expect(memoizedParameter(true)).to.be.true;
+      });
+
+      it('should handle equal angular objects with different hashes', function() {
+        const memoizedParameter = controller.memoizeImmutableListReference(_.identity);
+        const angularObjectA = {
+          $$hashKey: 1,
+          keyA: true
+        };
+        const angularObjectB = {
+          $$hashKey: 2,
+          keyA: true
+        };
+
+        m.chai.expect(memoizedParameter(angularObjectA)).to.equal(angularObjectA);
+        m.chai.expect(memoizedParameter(angularObjectB)).to.equal(angularObjectA);
+      });
+
+    });
+
+  });
+
+});

tests/shared/drive-constraints.spec.js

@@ -1,6 +1,7 @@
 'use strict';
 
 const m = require('mochainon');
+const _ = require('lodash');
 const path = require('path');
 const constraints = require('../../lib/shared/drive-constraints');
 
@@ -771,4 +772,233 @@ describe('Shared: DriveConstraints', function() {
 
   });
 
+  describe('.getDriveImageCompatibilityStatuses', function() {
+
+    beforeEach(function() {
+      if (process.platform === 'win32') {
+        this.mountpoint = 'E:';
+        this.separator = '\\';
+      } else {
+        this.mountpoint = '/mnt/foo';
+        this.separator = '/';
+      }
+
+      this.drive = {
+        device: '/dev/disk2',
+        name: 'My Drive',
+        protected: false,
+        system: false,
+        mountpoints: [
+          {
+            path: this.mountpoint
+          }
+        ],
+        size: 4000000000
+      };
+
+      this.image = {
+        path: path.join(__dirname, 'rpi.img'),
+        size: {
+          original: this.drive.size - 1,
+          final: {
+            estimation: false
+          }
+        }
+      };
+    });
+
+    const expectStatusTypesAndMessagesToBe = (resultList, expectedTuples) => {
+
+      // Sort so that order doesn't matter
+      const expectedTuplesSorted = _.sortBy(_.map(expectedTuples, (tuple) => {
+        return {
+          type: constraints.COMPATIBILITY_STATUS_TYPES[tuple[0]],
+          message: constraints.COMPATIBILITY_STATUS_MESSAGES[tuple[1]]
+        };
+      }), [ 'message' ]);
+      const resultTuplesSorted = _.sortBy(resultList, [ 'message' ]);
+
+      m.chai.expect(resultTuplesSorted).to.deep.equal(expectedTuplesSorted);
+    };
+
+    describe('given there are no errors or warnings', () => {
+
+      it('should return an empty list', function() {
+        const result = constraints.getDriveImageCompatibilityStatuses(this.drive, {
+          path: '/mnt/disk2/rpi.img',
+          size: 1000000000
+        });
+
+        m.chai.expect(result).to.deep.equal([]);
+      });
+
+    });
+
+    describe('given the drive contains the image', () => {
+
+      it('should return the contains-image error', function() {
+        this.image.path = path.join(this.mountpoint, 'rpi.img');
+
+        const result = constraints.getDriveImageCompatibilityStatuses(this.drive, this.image);
+        const expectedTuples = [ [ 'ERROR', 'CONTAINS_IMAGE' ] ];
+
+        expectStatusTypesAndMessagesToBe(result, expectedTuples);
+      });
+
+    });
+
+    describe('given the drive is a system drive', () => {
+
+      it('should return the system drive warning', function() {
+        this.drive.system = true;
+
+        const result = constraints.getDriveImageCompatibilityStatuses(this.drive, this.image);
+        const expectedTuples = [ [ 'WARNING', 'SYSTEM' ] ];
+
+        expectStatusTypesAndMessagesToBe(result, expectedTuples);
+      });
+
+    });
+
+    describe('given the drive is too small', () => {
+
+      it('should return the too small error', function() {
+        this.image.size.final.value = this.drive.size + 1;
+
+        const result = constraints.getDriveImageCompatibilityStatuses(this.drive, this.image);
+        const expectedTuples = [ [ 'ERROR', 'TOO_SMALL' ] ];
+
+        expectStatusTypesAndMessagesToBe(result, expectedTuples);
+      });
+
+    });
+
+    describe('given the drive is locked', () => {
+
+      it('should return the locked drive error', function() {
+        this.drive.protected = true;
+
+        const result = constraints.getDriveImageCompatibilityStatuses(this.drive, this.image);
+        const expectedTuples = [ [ 'ERROR', 'LOCKED' ] ];
+
+        expectStatusTypesAndMessagesToBe(result, expectedTuples);
+      });
+
+    });
+
+    describe('given the drive is smaller than the recommended size', () => {
+
+      it('should return the smaller than recommended size warning', function() {
+        this.image.recommendedDriveSize = this.drive.size + 1;
+
+        const result = constraints.getDriveImageCompatibilityStatuses(this.drive, this.image);
+        const expectedTuples = [ [ 'WARNING', 'SIZE_NOT_RECOMMENDED' ] ];
+
+        expectStatusTypesAndMessagesToBe(result, expectedTuples);
+      });
+
+    });
+
+    describe('given the image is null', () => {
+
+      it('should return an empty list', function() {
+        const result = constraints.getDriveImageCompatibilityStatuses(this.drive, null);
+
+        m.chai.expect(result).to.deep.equal([]);
+      });
+
+    });
+
+    describe('given the drive is null', () => {
+
+      it('should return an empty list', function() {
+        const result = constraints.getDriveImageCompatibilityStatuses(null, this.image);
+
+        m.chai.expect(result).to.deep.equal([]);
+      });
+
+    });
+
+    describe('given a locked drive and image is null', () => {
+
+      it('should return locked drive error', function() {
+        this.drive.protected = true;
+
+        const result = constraints.getDriveImageCompatibilityStatuses(this.drive, null);
+        const expectedTuples = [ [ 'ERROR', 'LOCKED' ] ];
+
+        expectStatusTypesAndMessagesToBe(result, expectedTuples);
+      });
+
+    });
+
+    describe('given a system drive and image is null', () => {
+
+      it('should return system drive warning', function() {
+        this.drive.system = true;
+
+        const result = constraints.getDriveImageCompatibilityStatuses(this.drive, null);
+        const expectedTuples = [ [ 'WARNING', 'SYSTEM' ] ];
+
+        expectStatusTypesAndMessagesToBe(result, expectedTuples);
+      });
+
+    });
+
+    describe('given the drive contains the image and the drive is locked', () => {
+
+      it('should return the contains-image drive error by precedence', function() {
+        this.drive.protected = true;
+        this.image.path = path.join(this.mountpoint, 'rpi.img');
+
+        const result = constraints.getDriveImageCompatibilityStatuses(this.drive, this.image);
+        const expectedTuples = [ [ 'ERROR', 'CONTAINS_IMAGE' ] ];
+
+        expectStatusTypesAndMessagesToBe(result, expectedTuples);
+      });
+
+    });
+
+    describe('given a locked and too small drive', () => {
+
+      it('should return the locked error by precedence', function() {
+        this.drive.protected = true;
+
+        const result = constraints.getDriveImageCompatibilityStatuses(this.drive, this.image);
+        const expectedTuples = [ [ 'ERROR', 'LOCKED' ] ];
+
+        expectStatusTypesAndMessagesToBe(result, expectedTuples);
+      });
+
+    });
+
+    describe('given a too small and system drive', () => {
+
+      it('should return the too small drive error by precedence', function() {
+        this.image.size.final.value = this.drive.size + 1;
+        this.drive.system = true;
+
+        const result = constraints.getDriveImageCompatibilityStatuses(this.drive, this.image);
+        const expectedTuples = [ [ 'ERROR', 'TOO_SMALL' ] ];
+
+        expectStatusTypesAndMessagesToBe(result, expectedTuples);
+      });
+
+    });
+
+    describe('given a system drive and not recommended drive size', () => {
+
+      it('should return both warnings', function() {
+        this.drive.system = true;
+        this.image.recommendedDriveSize = this.drive.size + 1;
+
+        const result = constraints.getDriveImageCompatibilityStatuses(this.drive, this.image);
+        const expectedTuples = [ [ 'WARNING', 'SIZE_NOT_RECOMMENDED' ], [ 'WARNING', 'SYSTEM' ] ];
+
+        expectStatusTypesAndMessagesToBe(result, expectedTuples);
+      });
+
+    });
+  });
+
 });