From 789b640b9bb8979c9620d46c678573a510b9a52e Mon Sep 17 00:00:00 2001
From: Henry <akhenry@aitutaki.ndc.nasa.gov>
Date: Fri, 23 Oct 2015 16:28:42 -0700
Subject: [PATCH] Refactored dismiss and minimize out of NotificationService
 and on to returned Notification type

---
 .../src/NotificationLaunchController.js       |  19 +-
 .../general/res/templates/message-banner.html |  18 +-
 .../src/controllers/BannerController.js       |   9 +-
 .../src/NotificationIndicatorController.js    |   6 +-
 .../notification/src/NotificationService.js   | 233 +++++++++++-------
 .../test/NotificationServiceSpec.js           |  47 +++-
 6 files changed, 218 insertions(+), 114 deletions(-)

diff --git a/example/notifications/src/NotificationLaunchController.js b/example/notifications/src/NotificationLaunchController.js
index 7d0618ada4..851e0575f5 100644
--- a/example/notifications/src/NotificationLaunchController.js
+++ b/example/notifications/src/NotificationLaunchController.js
@@ -130,7 +130,7 @@ define(
              */
             $scope.newProgress = function(){
 
-                var notification = {
+                var notificationModel = {
                     title: "Progress notification example",
                     severity: "info",
                     progress: 0,
@@ -142,17 +142,18 @@ define(
                  * Simulate an ongoing process and update the progress bar.
                  * @param notification
                  */
-                function incrementProgress(notification) {
-                    notification.progress = Math.min(100, Math.floor(notification.progress + Math.random() * 30));
-                    notification.progressText = ["Estimated time remaining:" +
-                    " about ", 60 - Math.floor((notification.progress / 100) * 60), " seconds"].join(" ");
-                    if (notification.progress < 100) {
-                        $timeout(function(){incrementProgress(notification);}, 1000);
+                function incrementProgress(notificationModel) {
+                    notificationModel.progress = Math.min(100, Math.floor(notificationModel.progress + Math.random() * 30));
+                    notificationModel.progressText = ["Estimated time" +
+                    " remaining:" +
+                    " about ", 60 - Math.floor((notificationModel.progress / 100) * 60), " seconds"].join(" ");
+                    if (notificationModel.progress < 100) {
+                        $timeout(function(){incrementProgress(notificationModel);}, 1000);
                     }
                 }
 
-                notificationService.notify(notification);
-                incrementProgress(notification);
+                notificationService.notify(notificationModel);
+                incrementProgress(notificationModel);
             };
 
             /**
diff --git a/platform/commonUI/general/res/templates/message-banner.html b/platform/commonUI/general/res/templates/message-banner.html
index 29b107126e..44c7f915b6 100644
--- a/platform/commonUI/general/res/templates/message-banner.html
+++ b/platform/commonUI/general/res/templates/message-banner.html
@@ -1,20 +1,20 @@
 <div ng-controller="BannerController" ng-show="active.notification"
-     class="l-message-banner s-message-banner {{active.notification.severity}}" ng-class="{
-     'minimized': active.notification.minimized,
-     'new': !active.notification.minimized}"
+     class="l-message-banner s-message-banner {{active.notification.model.severity}}" ng-class="{
+     'minimized': active.notification.model.minimized,
+     'new': !active.notification.model.minimized}"
      ng-click="maximize(active.notification)">
     <span class="banner-elem label">
-        {{active.notification.title}}
+        {{active.notification.model.title}}
     </span>
-    <span ng-show="active.notification.progress !== undefined || active.notification.unknownProgress">
+    <span ng-show="active.notification.model.progress !== undefined || active.notification.model.unknownProgress">
         <mct-include key="'progress-bar'" class="banner-elem"
-                     ng-model="active.notification">
+                     ng-model="active.notification.model">
         </mct-include>
     </span>
-    <a ng-hide="active.notification.primaryOption === undefined"
+    <a ng-hide="active.notification.model.primaryOption === undefined"
        class="banner-elem l-action s-action"
-       ng-click="action(active.notification.primaryOption.callback, $event)">
-        {{active.notification.primaryOption.label}}
+       ng-click="action(active.notification.model.primaryOption.callback, $event)">
+        {{active.notification.model.primaryOption.label}}
     </a>
     <a class="banner-elem ui-symbol close" ng-click="dismiss(active.notification, $event)">
         &#x78;</a>
diff --git a/platform/commonUI/general/src/controllers/BannerController.js b/platform/commonUI/general/src/controllers/BannerController.js
index 068b5cda81..4be9304cc6 100644
--- a/platform/commonUI/general/src/controllers/BannerController.js
+++ b/platform/commonUI/general/src/controllers/BannerController.js
@@ -52,14 +52,15 @@ define(
             };
             $scope.dismiss = function(notification, $event) {
                 $event.stopPropagation();
-                notificationService.dismissOrMinimize(notification);
+                notification.dismissOrMinimize();
             };
             $scope.maximize = function(notification) {
-                if (notification.severity !== "info"){
-                    notification.cancel = function(){
+                if (notification.model.severity !== "info"){
+
+                    notification.model.cancel = function(){
                         dialogService.dismiss();
                     };
-                    dialogService.showBlockingMessage(notification);
+                    dialogService.showBlockingMessage(notification.model);
                 }
             };
         }
diff --git a/platform/commonUI/notification/src/NotificationIndicatorController.js b/platform/commonUI/notification/src/NotificationIndicatorController.js
index 66ad825a32..e58b0a606b 100644
--- a/platform/commonUI/notification/src/NotificationIndicatorController.js
+++ b/platform/commonUI/notification/src/NotificationIndicatorController.js
@@ -48,7 +48,11 @@ define(
                 dialogService.getDialogResponse('overlay-message-list', {
                     dialog: {
                         title: "Messages",
-                        messages: notificationService.notifications
+                        //Launch the message list dialog with the models
+                        // from the notifications
+                        messages: notificationService.notifications && notificationService.notifications.map(function(notification){
+                            return notification.model;
+                        })
                     },
                     cancel: function(){
                         dialogService.dismiss();
diff --git a/platform/commonUI/notification/src/NotificationService.js b/platform/commonUI/notification/src/NotificationService.js
index 140c4537c8..cff2d57d7a 100644
--- a/platform/commonUI/notification/src/NotificationService.js
+++ b/platform/commonUI/notification/src/NotificationService.js
@@ -29,7 +29,7 @@
  * dialogs so that the same information can be provided in a dialog
  * and then minimized to a banner notification if needed.
  *
- * @namespace platform/commonUI/dialog
+ * @namespace platform/commonUI/notification
  */
 define(
     [],
@@ -54,11 +54,10 @@ define(
          * dialogs so that the same information can be provided in a dialog
          * and then minimized to a banner notification if needed.
          *
-         * @typedef {object} Notification
+         * @typedef {object} NotificationModel
          * @property {string} title The title of the message
-         * @property {string} severity The importance of the
-         * message (one of 'info', 'alert', or 'error' where info < alert <
-         * error)
+         * @property {string} severity The importance of the message (one of
+         * 'info', 'alert', or 'error' where info < alert <error)
          * @property {number} progress The completion status of a task
          * represented numerically
          * @property {boolean} unknownProgress a boolean indicating that the
@@ -78,16 +77,40 @@ define(
          * that may or may not be available from a banner.
          */
 
+        /**
+         * A wrapper object that is returned as a handle to a newly created
+         * notification. Wraps the notifications model and decorates with
+         * functions to dismiss or minimize the notification.
+         *
+         * @typedef {object} Notification
+         * @property {function} dismiss This method is added to the object
+         * returned by {@link NotificationService#notify} and can be used to
+         * dismiss this notification. Dismissing a notification will remove
+         * it completely and it will not appear in the notification indicator
+         * @property {function} minimize This method is added to the object
+         * returned by {@link NotificationService#notify} and can be used to
+         * minimize this notification. Minimizing a notification will send
+         * it to the notification indicator
+         * @property {function} dismissOrMinimize This method is added to the
+         * object returned by {@link NotificationService#notify}. It will
+         * hide the notification by either dismissing or minimizing it,
+         * depending on severity. Typically this is the method that should
+         * be used for dismissing a notification. If more control is
+         * required, then the minimize or dismiss functions can be called
+         * individually.
+         */
+
         /**
          * The notification service is responsible for informing the user of
          * events via the use of banner notifications.
+         * @memberof platform/commonUI/notification
+         * @constructor
          * @param $timeout the Angular $timeout service
          * @param DEFAULT_AUTO_DISMISS The period of time that an
          * auto-dismissed message will be displayed for.
          * @param MINIMIZE_TIMEOUT When notifications are minimized, a brief
          * animation is shown. This animation requires some time to execute,
          * so a timeout is required before the notification is hidden
-         * @constructor
          */
         function NotificationService($timeout, DEFAULT_AUTO_DISMISS, MINIMIZE_TIMEOUT) {
             this.notifications = [];
@@ -103,6 +126,83 @@ define(
             this.active = {};
         }
 
+        /*
+         * Minimize a notification. The notification will still be available
+         * from the notification list. Typically notifications with a
+         * severity of 'info' should not be minimized, but rather
+         * dismissed. If you're not sure which is appropriate,
+         * use {@link Notification#dismissOrMinimize}
+         */
+        function minimize (service, notification) {
+            //Check this is a known notification
+            var index = service.notifications.indexOf(notification);
+
+            if (service.active.timeout){
+                /*
+                 Method can be called manually (clicking dismiss) or
+                 automatically from an auto-timeout. this.active.timeout
+                 acts as a semaphore to prevent race conditions. Cancel any
+                 timeout in progress (for the case where a manual dismiss
+                 has shortcut an active auto-dismiss), and clear the
+                 semaphore.
+                 */
+                service.$timeout.cancel(service.active.timeout);
+                delete service.active.timeout;
+            }
+
+            if (index >= 0) {
+                notification.model.minimized=true;
+                //Add a brief timeout before showing the next notification
+                // in order to allow the minimize animation to run through.
+                service.$timeout(function() {
+                    service.setActiveNotification(service.selectNextNotification());
+                }, service.MINIMIZE_TIMEOUT);
+            }
+        };
+
+        /*
+         * Completely removes a notification. This will dismiss it from the
+         * message banner and remove it from the list of notifications.
+         * Typically only notifications with a severity of info should be
+         * dismissed. If you're not sure whether to dismiss or minimize a
+         * notification, use {@link Notification#dismissOrMinimize}.
+         * dismiss
+         */
+        function dismiss (service, notification) {
+            //Check this is a known notification
+            var index = service.notifications.indexOf(notification);
+
+            if (service.active.timeout){
+                /* Method can be called manually (clicking dismiss) or
+                 * automatically from an auto-timeout. this.active.timeout
+                 * acts as a semaphore to prevent race conditions. Cancel any
+                 * timeout in progress (for the case where a manual dismiss
+                 * has shortcut an active auto-dismiss), and clear the
+                 * semaphore.
+                 */
+
+                service.$timeout.cancel(service.active.timeout);
+                delete service.active.timeout;
+            }
+
+            if (index >= 0) {
+                service.notifications.splice(index, 1);
+            }
+            service.setActiveNotification(service.selectNextNotification());
+        };
+
+        /*
+         * Depending on the severity of the notification will selectively
+         * dismiss or minimize where appropriate.
+         */
+        function dismissOrMinimize (notification){
+
+            //For now minimize everything, and have discussion around which
+            //kind of messages should or should not be in the minimized
+            //notifications list
+            notification.minimize();
+        };
+
         /**
          * Returns the notification that is currently visible in the banner area
          * @returns {Notification}
@@ -115,12 +215,15 @@ define(
          * A convenience method for info notifications. Notifications
          * created via this method will be auto-dismissed after a default
          * wait period
-         * @param {Notification} notification The notification to display
+         * @param {NotificationModel} notificationModel Options describing the
+         * notification to display
+         * @returns {Notification} the provided notification decorated with
+         * functions to dismiss or minimize
          */
-        NotificationService.prototype.info = function (notification) {
-            notification.autoDismiss = notification.autoDismiss || true;
-            notification.severity = "info";
-            this.notify(notification);
+        NotificationService.prototype.info = function (notificationModel) {
+            notificationModel.autoDismiss = notificationModel.autoDismiss || true;
+            notificationModel.severity = "info";
+            return this.notify(notificationModel);
         };
 
         /**
@@ -128,25 +231,45 @@ define(
          * already active, then it will be dismissed or minimized automatically,
          * and the provided notification displayed in its place.
          *
-         * @param {Notification} notification The notification to display
+         * @param {NotificationModel} notificationModel The notification to
+         * display
+         * @returns {Notification} the provided notification decorated with
+         * functions to {@link Notification#dismiss} or {@link Notification#minimize}
          */
-        NotificationService.prototype.notify = function (notification) {
+        NotificationService.prototype.notify = function (notificationModel) {
             var self = this,
+                notification,
                 ordinality = {
                     "info": 1,
                     "alert": 2,
                     "error": 3
-                };
-            notification.severity = notification.severity || "info";
-            if (notification.autoDismiss === true){
-                notification.autoDismiss = this.DEFAULT_AUTO_DISMISS;
+                },
+                activeNotification = self.active.notification;
+
+            notification = {
+                model: notificationModel,
+                minimize: function() {
+                    minimize(self, notification)
+                },
+                dismiss: function(){
+                    dismiss(self, notification)
+                },
+                dismissOrMinimize: function(){
+                    dismissOrMinimize(notification)
+                }
+            },
+
+            notificationModel.severity = notificationModel.severity || "info";
+            if (notificationModel.autoDismiss === true){
+                notificationModel.autoDismiss = this.DEFAULT_AUTO_DISMISS;
             }
 
-            if (ordinality[notification.severity.toLowerCase()] > ordinality[this.highest.severity.toLowerCase()]){
-                this.highest.severity = notification.severity;
+            if (ordinality[notificationModel.severity.toLowerCase()] > ordinality[this.highest.severity.toLowerCase()]){
+                this.highest.severity = notificationModel.severity;
             }
 
             this.notifications.push(notification);
+
             /*
             Check if there is already an active (ie. visible) notification
              */
@@ -165,10 +288,12 @@ define(
                   serviced as soon as possible.
                  */
                 this.active.timeout = this.$timeout(function () {
-                    self.dismissOrMinimize(self.active.notification);
+                    activeNotification.dismissOrMinimize();
                 }, this.DEFAULT_AUTO_DISMISS);
             }
 
+            return notification;
+
         };
 
         /**
@@ -186,12 +311,12 @@ define(
                  notifications queued for display, setup a timeout to
                   dismiss the dialog.
                  */
-                if (notification && (notification.autoDismiss
+                if (notification && (notification.model.autoDismiss
                     || this.selectNextNotification())) {
 
-                    timeout = notification.autoDismiss || this.DEFAULT_AUTO_DISMISS;
+                    timeout = notification.model.autoDismiss || this.DEFAULT_AUTO_DISMISS;
                     this.active.timeout = this.$timeout(function () {
-                        self.dismissOrMinimize(notification);
+                        notification.dismissOrMinimize();
                     }, timeout);
                 } else {
                     delete this.active.timeout;
@@ -214,7 +339,7 @@ define(
             for (; i< this.notifications.length; i++) {
                 notification = this.notifications[i];
 
-                if (!notification.minimized
+                if (!notification.model.minimized
                     && notification!== this.active.notification) {
 
                     return notification;
@@ -222,64 +347,6 @@ define(
             }
         };
 
-        /**
-         * Minimize a notification. The notification will still be available
-         * from the notification list. Typically notifications with a
-         * severity of 'info' should not be minimized, but rather
-         * dismissed. If you're not sure which is appropriate,
-         * use {@link NotificationService#dismissOrMinimize}
-         * @see dismiss
-         * @see dismissOrMinimize
-         * @param notification
-         */
-        NotificationService.prototype.minimize = function (notification) {
-            //Check this is a known notification
-            var index = this.notifications.indexOf(notification),
-                self = this;
-            if (index >= 0) {
-                notification.minimized=true;
-                //Add a brief timeout before showing the next notification
-                // in order to allow the minimize animation to run through.
-                this.$timeout(function() {
-                    self.setActiveNotification(self.selectNextNotification());
-                }, this.MINIMIZE_TIMEOUT);
-            }
-        };
-
-        /**
-         * Completely removes a notification. This will dismiss it from the
-         * message banner and remove it from the list of notifications.
-         * Typically only notifications with a severity of info should be
-         * dismissed. If you're not sure whether to dismiss or minimize a
-         * notification, use {@link NotificationService#dismissOrMinimize}.
-         * dismiss
-         * @see dismissOrMinimize
-         * @param notification The notification to dismiss
-         */
-        NotificationService.prototype.dismiss = function (notification) {
-            //Check this is a known notification
-            var index = this.notifications.indexOf(notification);
-            if (index >= 0) {
-                this.notifications.splice(index, 1);
-            }
-            this.setActiveNotification(this.selectNextNotification());
-        };
-
-        /**
-         * Depending on the severity of the notification will selectively
-         * dismiss or minimize where appropriate.
-         * @see dismiss
-         * @see minimize
-         * @param notification
-         */
-        NotificationService.prototype.dismissOrMinimize = function (notification){
-
-            //For now minimize everything, and have discussion around which
-            //kind of messages should or should not be in the minimized
-            //notifications list
-            this.minimize(notification);
-        };
-
         return NotificationService;
     }
-);
+);
\ No newline at end of file
diff --git a/platform/commonUI/notification/test/NotificationServiceSpec.js b/platform/commonUI/notification/test/NotificationServiceSpec.js
index 695275bc70..b2fb622f52 100644
--- a/platform/commonUI/notification/test/NotificationServiceSpec.js
+++ b/platform/commonUI/notification/test/NotificationServiceSpec.js
@@ -54,7 +54,7 @@ define(
                 var activeNotification;
                 notificationService.notify(successModel);
                 activeNotification = notificationService.getActiveNotification();
-                expect(activeNotification).toBe(successModel);
+                expect(activeNotification.model).toBe(successModel);
             });
 
             it("gets a new success notification with" +
@@ -63,7 +63,7 @@ define(
                 successModel.autoDismiss = 1000;
                 notificationService.notify(successModel);
                 activeNotification = notificationService.getActiveNotification();
-                expect(activeNotification).toBe(successModel);
+                expect(activeNotification.model).toBe(successModel);
                 mockTimeout.mostRecentCall.args[0]();
                 expect(mockTimeout.calls.length).toBe(2);
                 mockTimeout.mostRecentCall.args[0]();
@@ -77,7 +77,7 @@ define(
                 successModel.autoDismiss = true;
                 notificationService.notify(successModel);
                 activeNotification = notificationService.getActiveNotification();
-                expect(activeNotification).toBe(successModel);
+                expect(activeNotification.model).toBe(successModel);
                 mockTimeout.mostRecentCall.args[0]();
                 expect(mockTimeout.calls.length).toBe(2);
                 mockTimeout.mostRecentCall.args[0]();
@@ -85,6 +85,37 @@ define(
                 expect(activeNotification).toBeUndefined();
             });
 
+            it("allows minimization of notifications", function() {
+                var notification,
+                    activeNotification;
+
+                successModel.autoDismiss = false;
+                notification = notificationService.notify(successModel);
+
+                activeNotification = notificationService.getActiveNotification();
+                expect(activeNotification.model).toBe(successModel);
+                notification.minimize();
+                mockTimeout.mostRecentCall.args[0]();
+                activeNotification = notificationService.getActiveNotification();
+                expect(activeNotification).toBeUndefined();
+                expect(notificationService.notifications.length).toBe(1);
+            });
+
+            it("allows dismissal of notifications", function() {
+                var notification,
+                    activeNotification;
+
+                successModel.autoDismiss = false;
+                notification = notificationService.notify(successModel);
+
+                activeNotification = notificationService.getActiveNotification();
+                expect(activeNotification.model).toBe(successModel);
+                notification.dismiss();
+                activeNotification = notificationService.getActiveNotification();
+                expect(activeNotification).toBeUndefined();
+                expect(notificationService.notifications.length).toBe(0);
+            });
+
             describe(" gets called with multiple notifications", function(){
                 it("auto-dismisses the previously active notification, making" +
                     " the new notification active", function() {
@@ -94,7 +125,7 @@ define(
                     activeNotification =
                         notificationService.getActiveNotification();
                     //Initially expect the active notification to be info
-                    expect(activeNotification).toBe(successModel);
+                    expect(activeNotification.model).toBe(successModel);
                     //Then notify of an error
                     notificationService.notify(errorModel);
                     //But it should be auto-dismissed and replaced with the
@@ -105,7 +136,7 @@ define(
                     // second is to allow minimization animation to take place.
                     mockTimeout.mostRecentCall.args[0]();
                     activeNotification = notificationService.getActiveNotification();
-                    expect(activeNotification).toBe(errorModel);
+                    expect(activeNotification.model).toBe(errorModel);
                 });
                 it("auto-minimizes an active error notification", function() {
                     var activeNotification;
@@ -125,7 +156,7 @@ define(
                     expect(notificationService.notifications.length).toEqual(2);
                     activeNotification =
                         notificationService.getActiveNotification();
-                    expect(activeNotification).toBe(successModel);
+                    expect(activeNotification.model).toBe(successModel);
                     expect(errorModel.minimized).toEqual(true);
                 });
                 it("auto-minimizes errors when a number of them arrive in" +
@@ -157,7 +188,7 @@ define(
                     expect(notificationService.notifications.length).toEqual(3);
                     activeNotification =
                         notificationService.getActiveNotification();
-                    expect(activeNotification).toBe(error2);
+                    expect(activeNotification.model).toBe(error2);
                     expect(errorModel.minimized).toEqual(true);
 
                     //Mock the second auto-minimize
@@ -168,7 +199,7 @@ define(
                     mockTimeout.mostRecentCall.args[0]();
                     activeNotification =
                         notificationService.getActiveNotification();
-                    expect(activeNotification).toBe(error3);
+                    expect(activeNotification.model).toBe(error3);
                     expect(error2.minimized).toEqual(true);
 
                 });