Added more tests, some refactoring

platform/commonUI/notification/src/NotificationService.js

@@ -23,13 +23,61 @@
 
 /**
  * This bundle implements the notification service, which can be used to
- * show banner notifications to the user.
+ * show banner notifications to the user. Banner notifications
+ * are used to inform users of events in a non-intrusive way. As
+ * much as possible, notifications share a model with blocking
+ * 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
  */
 define(
     ["./MessageSeverity"],
     function (MessageSeverity) {
         "use strict";
+
+        /**
+         * A representation of a user action. Actions are provided to
+         * dialogs and notifications and are shown as buttons.
+         *
+         * @typedef {object} NotificationAction
+         * @property {string} label the label to appear on the button for
+         * this action
+         * @property {function} action a callback function to be invoked
+         * when the button is clicked
+        */
+
+        /**
+         * A representation of a banner notification. Banner notifications
+         * are used to inform users of events in a non-intrusive way. As
+         * much as possible, notifications share a model with blocking
+         * dialogs so that the same information can be provided in a dialog
+         * and then minimized to a banner notification if needed.
+         *
+         * @typedef {object} Notification
+         * @property {string} title The title of the message
+         * @property {number} progress The completion status of a task
+         * represented numerically
+         * @property {MessageSeverity} messageSeverity The importance of the
+         * message (eg. error, success)
+         * @property {boolean} unknownProgress a boolean indicating that the
+         * progress of the underlying task is unknown. This will result in a
+         * visually distinct progress bar.
+         * @property {boolean | number} autoDismiss If truthy, dialog will
+         * be automatically minimized or dismissed (depending on severity).
+         * Additionally, if the provided value is a number, it will be used
+         * as the delay period before being dismissed.
+         * @property {NotificationAction} primaryAction the default user
+         * response to
+         * this message. Will be represented as a button with the provided
+         * label and action. May be used by banner notifications to display
+         * only the most important option to users.
+         * @property {NotificationAction[]} additionalActions any additional
+         * actions
+         * that the user can take. Will be represented as additional buttons
+         * that may or may not be available from a banner.
+         */
+
         /**
          * The notification service is responsible for informing the user of
          * events via the use of banner notifications.
@@ -41,69 +89,51 @@ define(
             this.$timeout = $timeout;
             this.DEFAULT_AUTO_DISMISS = DEFAULT_AUTO_DISMISS;
 
-            /**
+            /*
-             * Exposes the current "active" notification. This is a
+             * A context in which to hold the active notification and a
-             * notification that is of current highest importance that has
+             * handle to its timeout.
-             * not been dismissed. The deinition of what is of highest
-             * importance might be a little nuanced and require tweaking.
-             * For example, if an important error message is visible and a
-             * success message is triggered, it may be desirable to
-             * temporarily show the success message and then auto-dismiss it.
-             * @type {{notification: undefined}}
              */
             this.active = {
             };
         }
-        /**
-        var model = {
-            title: string,
-            progress: number,
-            severity: MessageSeverity,
-            unknownProgress: boolean,
-            minimized: boolean,
-            autoDismiss: boolean | number,
-            actions: {
-                label: string,
-                action: function
-            }
-        }
-        */
 
         /**
-         * Possibly refactor this out to a provider?
+         * Returns the notification that is currently visible in the banner area
-         * @constructor
+         * @returns {Notification}
          */
-        function Notification (model) {
-            this.model = model;
-        }
-
-        Notification.prototype.minimize = function (setValue) {
-            if (typeof setValue !== undefined){
-                this.model.minimized = setValue;
-            } else {
-                return this.model.minimized;
-            }
-        };
-
         NotificationService.prototype.getActiveNotification = function (){
             return this.active.notification;
         }
 
         /**
-         * model = {
+         * A convenience method for success notifications. Notifications
-         *
+         * created via this method will be auto-dismissed after a default
-         * }
+         * wait period
-         * @param model
+         * @param {Notification} notification The notification to display
          */
-        NotificationService.prototype.notify = function (model) {
+        NotificationService.prototype.success = function (notification) {
-            var notification = new Notification(model),
+            notification.autoDismiss = notification.autoDismiss || true;
-                that=this;
+            NotificationService.prototype.notify(notification);
+        }
+
+        /**
+         * Notifies the user of an event. If there is a banner notification
+         * 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
+         */
+        NotificationService.prototype.notify = function (notification) {
+            /*var notification = new Notification(model),
+                that=this; */
+            var that = this;
+
             this.notifications.push(notification);
             /*
             Check if there is already an active (ie. visible) notification
              */
             if (!this.active.notification){
-                setActiveNotification.call(this, notification);
+                this.setActiveNotification(notification);
 
             } else if (!this.active.timeout){
                 /*
@@ -122,28 +152,42 @@ define(
 
         };
 
-        function setActiveNotification (notification) {
+        /**
-            var that = this;
+         * Used internally by the NotificationService
-            this.active.notification = notification;
+         * @private
-            /*
+         */
-            If autoDismiss has been specified, setup a timeout to
+        NotificationService.prototype.setActiveNotification =
-            dismiss the dialog.
+            function (notification) {
 
-            If there are other notifications pending in the queue, set this
+                var that = this;
-            one to auto-dismiss
+                this.active.notification = notification;
-             */
+                /*
-            if (notification.model.autoDismiss
+                If autoDismiss has been specified, setup a timeout to
-                || selectNextNotification.call(this)) {
+                dismiss the dialog.
-                var timeout = isNaN(notification.model.autoDismiss) ?
-                    this.DEFAULT_AUTO_DISMISS : notification.model.autoDismiss;
 
-                this.active.timeout = this.$timeout(function () {
+                If there are other notifications pending in the queue, set this
-                    that.dismissOrMinimize(notification);
+                one to auto-dismiss
-                }, timeout);
+                 */
-            }
+                if (notification && (notification.autoDismiss
-        }
+                    || this.selectNextNotification())) {
+                    var timeout = isNaN(notification.autoDismiss) ?
+                        this.DEFAULT_AUTO_DISMISS :
+                        notification.autoDismiss;
 
-        function selectNextNotification () {
+                    this.active.timeout = this.$timeout(function () {
+                        that.dismissOrMinimize(notification);
+                    }, timeout);
+                } else {
+                    delete this.active.timeout;
+                }
+        };
+
+        /**
+         * Used internally by the NotificationService
+         *
+         * @private
+         */
+        NotificationService.prototype.selectNextNotification = function () {
             /*
             Loop through the notifications queue and find the first one that
             has not already been minimized (manually or otherwise).
@@ -151,7 +195,7 @@ define(
             for (var i=0; i< this.notifications.length; i++) {
                 var notification = this.notifications[i];
 
-                if (!notification.model.minimized
+                if (!notification.minimized
                     && notification!= this.activeNotification) {
 
                     return notification;
@@ -162,8 +206,9 @@ define(
         /**
          * Minimize a notification. The notification will still be available
          * from the notification list. Typically notifications with a
-         * severity of SUCCESS should not be minimized, but rather
+         * severity of 'success' should not be minimized, but rather
-         * dismissed.
+         * dismissed. If you're not sure which is appropriate,
+         * use {@link NotificationService#dismissOrMinimize}
          * @see dismiss
          * @see dismissOrMinimize
          * @param notification
@@ -172,19 +217,17 @@ define(
             //Check this is a known notification
             var index = this.notifications.indexOf(notification);
             if (index >= 0) {
-                notification.minimize(true);
+                notification.minimized=true;
-                delete this.active.notification;
+                this.setActiveNotification(this.selectNextNotification());
-                delete this.active.timeout;
-                setActiveNotification.call(this, selectNextNotification.call(this));
             }
-        }
+        };
 
         /**
-         * Completely remove a notification. This will dismiss it from the
+         * 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 SUCCESS should be
+         * Typically only notifications with a severity of success should be
          * dismissed. If you're not sure whether to dismiss or minimize a
-         * notification, use the dismissOrMinimize method.
+         * notification, use {@link NotificationService#dismissOrMinimize}.
          * dismiss
          * @see dismissOrMinimize
          * @param notification The notification to dismiss
@@ -194,11 +237,7 @@ define(
             var index = this.notifications.indexOf(notification);
             if (index >= 0) {
                 this.notifications.splice(index, 1);
-
+                this.setActiveNotification(this.selectNextNotification());
-                delete this.active.notification;
-                delete this.active.timeout;
-
-                setActiveNotification.call(this, selectNextNotification.call(this));
             }
         }
 
@@ -210,7 +249,7 @@ define(
          * @param notification
          */
         NotificationService.prototype.dismissOrMinimize = function (notification){
-            if (notification.model.severity > MessageSeverity.SUCCESS){
+            if (notification.severity > MessageSeverity.SUCCESS){
                 this.minimize(notification);
             } else {
                 this.dismiss(notification);

platform/commonUI/notification/test/NotificationServiceSpec.js

@@ -30,24 +30,18 @@ define(
             var notificationService,
                 mockTimeout,
                 mockAutoDismiss,
-                successModel = {
+                successModel,
-                    title: "Mock Success Notification",
+                errorModel;
-                    severity: MessageSeverity.SUCCESS
-                },
-                errorModel = {
-                    title: "Mock Error Notification",
-                    severity: MessageSeverity.ERROR
-                };
 
         /**
          * 1) Calling .notify results in a new notification being created
-         * with the provided model and set to the active notification
+         * with the provided model and set to the active notification. DONE
          *
          * 2) Calling .notify with autoDismiss results in a SUCCESS notification
-         * becoming dismissed after timeout has elapsed
+         * becoming dismissed after timeout has elapsed DONE
          *
          * 3) Calling .notify with autoDismiss results in an ERROR notification
-         * being MINIMIZED after a timeout has elapsed
+         * being MINIMIZED after a timeout has elapsed DONE
          * 
          * 4) Calling .notify with an active success notification results in that
          * notification being auto-dismissed, and the new notification becoming
@@ -84,17 +78,49 @@ define(
                 mockAutoDismiss = 0;
                 notificationService = new NotificationService(
                     mockTimeout, mockAutoDismiss);
+                successModel = {
+                    title: "Mock Success Notification",
+                    severity: MessageSeverity.SUCCESS
+                };
+                errorModel = {
+                    title: "Mock Error Notification",
+                    severity: MessageSeverity.ERROR
+                };
             });
 
-            it("Calls the notification service with a new notification, making" +
+            it("gets a new success notification, making" +
                 " the notification active", function() {
                 var activeNotification;
                 notificationService.notify(successModel);
                 activeNotification = notificationService.getActiveNotification();
-                expect(activeNotification.model).toBe(successModel);
+                expect(activeNotification).toBe(successModel);
             });
 
-            describe(" called with multiple notifications", function(){
+            it("gets a new success notification with" +
+                " numerical auto-dismiss specified. ", function() {
+                var activeNotification;
+                successModel.autoDismiss = 1000;
+                notificationService.notify(successModel);
+                activeNotification = notificationService.getActiveNotification();
+                expect(activeNotification).toBe(successModel);
+                mockTimeout.mostRecentCall.args[0]();
+                activeNotification = notificationService.getActiveNotification();
+                expect(activeNotification).toBeUndefined();
+            });
+
+            it("gets a new notification with" +
+                " boolean auto-dismiss specified. ", function() {
+                var activeNotification;
+                successModel.autoDismiss = true;
+                notificationService.notify(successModel);
+                activeNotification = notificationService.getActiveNotification();
+                expect(activeNotification).toBe(successModel);
+                mockTimeout.mostRecentCall.args[0]();
+                activeNotification = notificationService.getActiveNotification();
+                expect(activeNotification).toBeUndefined();
+            });
+
+            describe(" gets called with multiple notifications", function(){
                 it("auto-dismisses the previously active notification, making" +
                     " the new notification active", function() {
                     var activeNotification;
@@ -103,14 +129,14 @@ define(
                     activeNotification =
                         notificationService.getActiveNotification();
                     //Initially expect the active notification to be success
-                    expect(activeNotification.model).toBe(successModel);
+                    expect(activeNotification).toBe(successModel);
                     //Then notify of an error
                     notificationService.notify(errorModel);
                     //But it should be auto-dismissed and replaced with the
                     // error notification
                     mockTimeout.mostRecentCall.args[0]();
                     activeNotification = notificationService.getActiveNotification();
-                    expect(activeNotification.model).toBe(errorModel);
+                    expect(activeNotification).toBe(errorModel);
                 });
                 it("auto-dismisses an active success notification, removing" +
                     " it completely", function() {
@@ -125,9 +151,9 @@ define(
                 });
                 it("auto-minimizes an active error notification", function() {
                     var activeNotification;
-                    //First pre-load with a success message
+                    //First pre-load with an error message
                     notificationService.notify(errorModel);
-                    //Then notify of an error
+                    //Then notify of success
                     notificationService.notify(successModel);
                     expect(notificationService.notifications.length).toEqual(2);
                     //Mock the auto-minimize
@@ -137,8 +163,42 @@ define(
                     expect(notificationService.notifications.length).toEqual(2);
                     activeNotification =
                         notificationService.getActiveNotification();
-                    expect(activeNotification.model).toBe(successModel);
+                    expect(activeNotification).toBe(successModel);
                     expect(errorModel.minimized).toEqual(true);
+                });
+                it("auto-minimizes errors when a number of them arrive in" +
+                    " short succession ", function() {
+                    var activeNotification;
+                    var error2 = {
+                        title: "Second Mock Error Notification",
+                        severity: MessageSeverity.ERROR
+                    }
+                    var error3 = {
+                        title: "Third Mock Error Notification",
+                        severity: MessageSeverity.ERROR
+                    }
+                    //First pre-load with a success message
+                    notificationService.notify(errorModel);
+                    //Then notify of a third error
+                    notificationService.notify(error2);
+                    notificationService.notify(error3);
+                    expect(notificationService.notifications.length).toEqual(3);
+                    //Mock the auto-minimize
+                    mockTimeout.mostRecentCall.args[0]();
+                    //Previous error message should be minimized, not
+                    // dismissed
+                    expect(notificationService.notifications.length).toEqual(3);
+                    activeNotification =
+                        notificationService.getActiveNotification();
+                    expect(activeNotification).toBe(error2);
+                    expect(errorModel.minimized).toEqual(true);
+
+                    //Mock the second auto-minimize
+                    mockTimeout.mostRecentCall.args[0]();
+                    activeNotification =
+                        notificationService.getActiveNotification();
+                    expect(activeNotification).toBe(error3);
+                    expect(error2.minimized).toEqual(true);
 
                 });
             });