Merge pull request #1635 from velesin/header_notifications_refactoring

refactors header notifications (renders them via separate controller)

app/assets/javascripts/discourse/controllers/header_controller.js

@@ -9,6 +9,7 @@
 Discourse.HeaderController = Discourse.Controller.extend({
   topic: null,
   showExtraInfo: null,
+  notifications: null,
 
   categories: function() {
     return Discourse.Category.list();
@@ -39,6 +40,16 @@ Discourse.HeaderController = Discourse.Controller.extend({
 
     toggleMobileView: function() {
       Discourse.Mobile.toggleMobileView();
+    },
+
+    showNotifications: function(headerView) {
+      var self = this;
+
+      Discourse.ajax("/notifications").then(function(result) {
+        self.set("notifications", result);
+        self.set("currentUser.unread_notifications", 0);
+        headerView.showDropdownBySelector("#user-notifications");
+      });
     }
   }
 

app/assets/javascripts/discourse/controllers/notification_controller.js

@@ -0,0 +1,17 @@
+Discourse.NotificationController = Discourse.ObjectController.extend({
+  scope: function() {
+    return "notifications." + Discourse.Site.currentProp("notificationLookup")[this.get("notification_type")];
+  }.property(),
+
+  username: function() {
+    return this.get("data.display_username");
+  }.property(),
+
+  link: function() {
+    if (this.blank("data.topic_title")) {
+      return "";
+    }
+    var url = Discourse.Utilities.postUrl(this.get("slug"), this.get("topic_id"), this.get("post_number"));
+    return '<a href="' + url + '">' + this.get("data.topic_title") + '</a>';
+  }.property()
+});

app/assets/javascripts/discourse/controllers/notifications_controller.js

@@ -0,0 +1,3 @@
+Discourse.NotificationsController = Ember.ArrayController.extend(Discourse.HasCurrentUser, {
+  itemController: "notification"
+});

app/assets/javascripts/discourse/helpers/i18n_helpers.js

@@ -34,6 +34,13 @@ Ember.Handlebars.registerHelper('i18n', function(property, options) {
   return I18n.t(property, params);
 });
 
+/**
+ Bound version of i18n helper.
+ **/
+Ember.Handlebars.registerBoundHelper("boundI18n", function(property, options) {
+  return new Handlebars.SafeString(I18n.t(property, options.hash));
+});
+
 /**
   Set up an i18n binding that will update as a count changes, complete with pluralization.
 

app/assets/javascripts/discourse/models/notification.js

@@ -1,42 +0,0 @@
-/**
-  A data model representing a notification a user receives
-
-  @class Notification
-  @extends Discourse.Model
-  @namespace Discourse
-  @module Discourse
-**/
-Discourse.Notification = Discourse.Model.extend({
-
-  readClass: (function() {
-    if (this.read) return 'read';
-    return '';
-  }).property('read'),
-
-  url: function() {
-    if (this.blank('data.topic_title')) return "";
-    return Discourse.Utilities.postUrl(this.get('slug'), this.get('topic_id'), this.get('post_number'));
-  }.property(),
-
-  rendered: function() {
-    var notificationName = Discourse.Site.currentProp('notificationLookup')[this.notification_type];
-    return I18n.t("notifications." + notificationName, {
-      username: this.data.display_username,
-      link: "<a href='" + (this.get('url')) + "'>" + this.data.topic_title + "</a>"
-    });
-  }.property()
-
-});
-
-Discourse.Notification.reopenClass({
-  create: function(obj) {
-    obj = obj || {};
-
-    if (obj.data) {
-      obj.data = Em.Object.create(obj.data);
-    }
-    return this._super(obj);
-  }
-});
-
-

app/assets/javascripts/discourse/templates/header.js.handlebars

@@ -94,20 +94,7 @@
 
       {{render search}}
 
-      <section class='d-dropdown' id='notifications-dropdown'>
-        {{#if view.notifications}}
-          <ul>
-            {{#each view.notifications}}
-            <li class="{{unbound readClass}}">{{{unbound rendered}}}</li>
-            {{/each}}
-            <li class='read last'>
-              <a {{bindAttr href="currentUser.path"}}>{{i18n notifications.more}} &hellip;</a>
-            </li>
-          </ul>
-        {{else}}
-          <div class='none'>{{i18n notifications.none}}</div>
-        {{/if}}
-      </section>
+      {{render notifications notifications}}
 
       <section class='d-dropdown' id='site-map-dropdown'>
       <ul class="location-links">

app/assets/javascripts/discourse/templates/notifications.js.handlebars

@@ -0,0 +1,14 @@
+<section class="d-dropdown" id="notifications-dropdown">
+  {{#if content}}
+    <ul>
+      {{#each}}
+        <li {{bind-attr class="read"}}>{{unbound boundI18n scope linkBinding="link" usernameBinding="username"}}</li>
+      {{/each}}
+      <li class="read last">
+        <a {{bind-attr href="currentUser.path"}}>{{i18n notifications.more}} &hellip;</a>
+      </li>
+    </ul>
+  {{else}}
+    <div class="none">{{i18n notifications.none}}</div>
+  {{/if}}
+</section>

app/assets/javascripts/discourse/views/header_view.js

@@ -48,19 +48,12 @@ Discourse.HeaderView = Discourse.View.extend({
     return false;
   },
 
+  showDropdownBySelector: function(selector) {
+    this.showDropdown($(selector));
+  },
+
   showNotifications: function() {
-
-    var headerView = this;
-    Discourse.ajax('/notifications').then(function(result) {
-      headerView.set('notifications', result.map(function(n) {
-        return Discourse.Notification.create(n);
-      }));
-
-      // We've seen all the notifications now
-      Discourse.User.current().set('unread_notifications', 0);
-      headerView.showDropdown($('#user-notifications'));
-    });
-    return false;
+    this.get("controller").send("showNotifications", this);
   },
 
   examineDockHeader: function() {
@@ -106,7 +99,8 @@ Discourse.HeaderView = Discourse.View.extend({
       return headerView.showDropdown($(e.currentTarget));
     });
     this.$('a.unread-private-messages, a.unread-notifications, a[data-notifications]').on('click.notifications', function(e) {
-      return headerView.showNotifications(e);
+      headerView.showNotifications(e);
+      return false;
     });
     $(window).bind('scroll.discourse-dock', function() {
       headerView.examineDockHeader();

test/javascripts/controllers/header_controller_test.js

@@ -0,0 +1,36 @@
+var server;
+
+module("Discourse.HeaderController", {
+  setup: function() {
+    server = sinon.fakeServer.create();
+  },
+
+  teardown: function() {
+    server.restore();
+  }
+});
+
+test("showNotifications action", function() {
+  var controller = Discourse.HeaderController.create();
+  var viewSpy = {
+    showDropdownBySelector: sinon.spy()
+  };
+  Discourse.User.current().set("unread_notifications", 1);
+  server.respondWith("/notifications", [200, { "Content-Type": "application/json" }, '["notification"]']);
+
+
+  Ember.run(function() {
+    controller.send("showNotifications", viewSpy);
+  });
+
+  equal(controller.get("notifications"), null, "notifications are null before data has finished loading");
+  equal(Discourse.User.current().get("unread_notifications"), 1, "current user's unread notifications count is not zeroed before data has finished loading");
+  ok(viewSpy.showDropdownBySelector.notCalled, "dropdown with notifications is not shown before data has finished loading");
+
+
+  server.respond();
+
+  deepEqual(controller.get("notifications"), ["notification"], "notifications are set correctly after data has finished loading");
+  equal(Discourse.User.current().get("unread_notifications"), 0, "current user's unread notifications count is zeroed after data has finished loading");
+  ok(viewSpy.showDropdownBySelector.calledWith("#user-notifications"), "dropdown with notifications is shown after data has finished loading");
+});

test/javascripts/controllers/notification_controller_test.js

@@ -0,0 +1,51 @@
+var controller;
+var notificationFixture = {
+  notification_type: 1, //mentioned
+  post_number: 1,
+  topic_id: 1234,
+  slug: "a-slug",
+  data: {
+  topic_title: "some title",
+    display_username: "velesin"
+  }
+};
+var postUrlStub = "post-url-stub";
+
+module("Discourse.NotificationController", {
+  setup: function() {
+    sinon.stub(Discourse.Utilities, "postUrl").returns(postUrlStub);
+
+    controller = Discourse.NotificationController.create({
+      content: notificationFixture
+    });
+  },
+
+  teardown: function() {
+    Discourse.Utilities.postUrl.restore();
+  }
+});
+
+test("scope property is correct", function() {
+  equal(controller.get("scope"), "notifications.mentioned");
+});
+
+test("username property is correct", function() {
+  equal(controller.get("username"), "velesin");
+});
+
+test("link property returns empty string when there is no topic title", function() {
+  var fixtureWithEmptyTopicTitle = _.extend({}, notificationFixture, {data: {topic_title: ""}});
+  Ember.run(function() {
+    controller.set("content", fixtureWithEmptyTopicTitle);
+  });
+
+  equal(controller.get("link"), "");
+});
+
+test("link property returns correctly built link when there is a topic title", function() {
+  var $link = $(controller.get("link"));
+
+  ok(Discourse.Utilities.postUrl.calledWithExactly("a-slug", 1234, 1), "URL is generated with the correct slug, topic ID and post number");
+  equal($link.attr("href"), postUrlStub, "generated link points to a correct URL");
+  equal($link.text(), "some title", "generated link has correct text");
+});

test/javascripts/controllers/notifications_controller_test.js

@@ -0,0 +1,82 @@
+var controller, view;
+
+var appendView = function() {
+  Ember.run(function() {
+    view.appendTo(fixture());
+  });
+};
+
+var noItemsMessageSelector = "div.none";
+var itemListSelector = "ul";
+var itemSelector = "li";
+
+module("Discourse.NotificationsController", {
+  setup: function() {
+    sinon.stub(I18n, "t", function (scope, options) {
+      options = options || {};
+      return [scope, options.username, options.link].join(" ").trim();
+    });
+
+    controller = Discourse.NotificationsController.create();
+
+    view = Ember.View.create({
+      controller: controller,
+      templateName: "notifications"
+    });
+  },
+
+  teardown: function() {
+    I18n.t.restore();
+  }
+});
+
+test("mixes in HasCurrentUser", function() {
+  ok(Discourse.HasCurrentUser.detect(controller));
+});
+
+test("by default uses NotificationController as its item controller", function() {
+  equal(controller.get("itemController"), "notification");
+});
+
+test("shows proper info when there are no notifications", function() {
+  controller.set("content", null);
+
+  appendView();
+
+  ok(exists(fixture(noItemsMessageSelector)), "special 'no notifications' message is displayed");
+  equal(fixture(noItemsMessageSelector).text(), "notifications.none", "'no notifications' message contains proper internationalized text");
+  equal(count(fixture(itemListSelector)), 0, "a list of notifications is not displayed");
+});
+
+test("displays a list of notifications and a 'more' link when there are notifications", function() {
+  controller.set("itemController", null);
+  controller.set("content", [
+    {
+      read: false,
+      scope: "scope_1",
+      username: "username_1",
+      link: "link_1"
+    },
+    {
+      read: true,
+      scope: "scope_2",
+      username: "username_2",
+      link: "link_2"
+    }
+  ]);
+
+  appendView();
+
+  var items = fixture(itemSelector);
+  equal(count(items), 3, "number of list items is correct");
+
+  equal(items.eq(0).attr("class"), "", "first (unread) item has proper class");
+  equal(items.eq(0).text(), "scope_1 username_1 link_1", "first item has correct content");
+
+  equal(items.eq(1).attr("class"), "read", "second (read) item has proper class");
+  equal(items.eq(1).text(), "scope_2 username_2 link_2", "second item has correct content");
+
+  var moreLink = items.eq(2).find("> a");
+  equal(moreLink.attr("href"), Discourse.User.current().get("path"), "'more' link points to a correct URL");
+  equal(moreLink.text(), "notifications.more …", "'more' link has correct text");
+});

test/javascripts/integration/header_test.js

@@ -144,7 +144,7 @@ test("notifications: content", function() {
       equal(notificationsDropdown().find("li").eq(1).attr("class"), "read", "list item for read notification has correct class");
       equal(notificationsDropdown().find("li").eq(1).html(), 'notifications.replied velesin <a href="/t/topic/1234/2">Some topic title</a>', "notification without a slug and for a non-first post in a topic is rendered correctly");
 
-      equal(notificationsDropdown().find("li").eq(2).html(), 'notifications.liked velesin <a href=""></a>', "notification without topic title is rendered correctly");
+      equal(notificationsDropdown().find("li").eq(2).html(), 'notifications.liked velesin', "notification without topic title is rendered correctly");
 
       equal(notificationsDropdown().find("li").eq(3).attr("class"), "read last", "list item for 'more' link has correct class");
       equal(notificationsDropdown().find("li").eq(3).find("a").attr("href"), Discourse.User.current().get("path"), "'more' link points to a correct URL");

test/javascripts/models/notification_test.js

@@ -1,5 +0,0 @@
-module("Discourse.Notification");
-
-test("create", function() {
-  ok(Discourse.Notification.create(), "it can be created without arguments");
-});

test/javascripts/views/header_view_test.js

@@ -0,0 +1,14 @@
+module("Discourse.HeaderView");
+
+test("showNotifications", function() {
+  var controllerSpy = {
+    send: sinon.spy()
+  };
+  var view = Discourse.HeaderView.create({
+    controller: controllerSpy
+  });
+
+  view.showNotifications();
+
+  ok(controllerSpy.send.calledWith("showNotifications", view), "sends showNotifications message to the controller, passing header view as a param");
+});