From abe7ef3d18b8ed29a7098edcd3f8bc1ca0c7b511 Mon Sep 17 00:00:00 2001 From: Steffen van Bergerem Date: Wed, 14 Dec 2016 20:57:51 +0100 Subject: [PATCH] Update existing notifications in dropdown on fetch When fetching notifications this merges existing notifications and changes their appearance in the dropdown if the html or the unread status changed. This doesn't update all notifications in the dropdown but only those that are returned by the server. Related to #7247. --- .../app/collections/notifications.js | 3 ++- .../javascripts/app/models/notification.js | 9 +++++++- .../app/views/notification_dropdown_view.js | 8 +++++++ .../notifications_collection_spec.js | 20 ++++++++++++++-- .../app/models/notification_spec.js | 23 ++++++++++++++----- .../views/notification_dropdown_view_spec.js | 4 ++++ 6 files changed, 57 insertions(+), 10 deletions(-) diff --git a/app/assets/javascripts/app/collections/notifications.js b/app/assets/javascripts/app/collections/notifications.js index 151410fe0..15a3e72ad 100644 --- a/app/assets/javascripts/app/collections/notifications.js +++ b/app/assets/javascripts/app/collections/notifications.js @@ -84,7 +84,8 @@ app.collections.Notifications = Backbone.Collection.extend({ /* eslint-disable new-cap */ var model = new this.model(item); /* eslint-enable new-cap */ - model.on("change:unread", this.onChangedUnreadStatus.bind(this)); + model.on("userChangedUnreadStatus", this.onChangedUnreadStatus.bind(this)); + model.on("change:unread", function() { this.trigger("update"); }.bind(this)); return model; }.bind(this)); }, diff --git a/app/assets/javascripts/app/models/notification.js b/app/assets/javascripts/app/models/notification.js index b2e342116..632a0fb90 100644 --- a/app/assets/javascripts/app/models/notification.js +++ b/app/assets/javascripts/app/models/notification.js @@ -40,6 +40,10 @@ app.models.Notification = Backbone.Model.extend({ * } */ parse: function(response) { + if (response.id) { + // already correct format + return response; + } var result = {type: response.type}; result = $.extend(result, response[result.type]); return result; @@ -62,7 +66,10 @@ app.models.Notification = Backbone.Model.extend({ /* eslint-enable camelcase */ type: "PUT", context: this, - success: function() { this.set("unread", state); } + success: function() { + this.set("unread", state); + this.trigger("userChangedUnreadStatus", this); + } }); } } diff --git a/app/assets/javascripts/app/views/notification_dropdown_view.js b/app/assets/javascripts/app/views/notification_dropdown_view.js index a72f1e8f1..fb4490829 100644 --- a/app/assets/javascripts/app/views/notification_dropdown_view.js +++ b/app/assets/javascripts/app/views/notification_dropdown_view.js @@ -21,6 +21,7 @@ app.views.NotificationDropdown = app.views.Base.extend({ this.collection.on("pushFront", this.onPushFront.bind(this)); this.collection.on("pushBack", this.onPushBack.bind(this)); this.collection.on("finishedLoading", this.finishLoading.bind(this)); + this.collection.on("change:note_html", this.onNotificationChange.bind(this)); }, toggleDropdown: function(evt){ @@ -87,6 +88,13 @@ app.views.NotificationDropdown = app.views.Base.extend({ $(node).find(this.avatars.selector).error(this.avatars.fallback); }, + onNotificationChange: function(notification) { + var node = this.dropdownNotifications.find("[data-guid=" + notification.get("id") + "]"); + $(node).replaceWith(notification.get("note_html")); + $(node).find(".unread-toggle .entypo-eye").tooltip("destroy").tooltip(); + $(node).find(this.avatars.selector).error(this.avatars.fallback); + }, + finishLoading: function() { app.helpers.timeago(this.dropdownNotifications); this.updateScrollbar(); diff --git a/spec/javascripts/app/collections/notifications_collection_spec.js b/spec/javascripts/app/collections/notifications_collection_spec.js index c6c8eeb7a..8a5c2b981 100644 --- a/spec/javascripts/app/collections/notifications_collection_spec.js +++ b/spec/javascripts/app/collections/notifications_collection_spec.js @@ -205,7 +205,7 @@ describe("app.collections.Notifications", function() { }); it("correctly binds the change:unread event", function() { - spyOn(app.collections.Notifications.prototype, "onChangedUnreadStatus"); + spyOn(this.target, "trigger"); /* eslint-disable camelcase */ var parsed = this.target.parse({ @@ -216,8 +216,24 @@ describe("app.collections.Notifications", function() { /* eslint-enable camelcase */ parsed[0].set("unread", true); + expect(this.target.trigger).toHaveBeenCalledWith("update"); + }); - expect(app.collections.Notifications.prototype.onChangedUnreadStatus).toHaveBeenCalled(); + it("correctly binds the userChangedUnreadStatus event", function() { + spyOn(this.target, "onChangedUnreadStatus"); + + /* eslint-disable camelcase */ + var parsed = this.target.parse({ + unread_count: 15, + unread_count_by_type: {reshared: 6}, + notification_list: [{"reshared": {id: 1}, "type": "reshared"}] + }); + /* eslint-enable camelcase */ + + parsed[0].set("unread", true); + parsed[0].trigger("userChangedUnreadStatus", parsed[0]); + + expect(this.target.onChangedUnreadStatus).toHaveBeenCalled(); }); }); diff --git a/spec/javascripts/app/models/notification_spec.js b/spec/javascripts/app/models/notification_spec.js index d405c34ec..0cc5b9cdd 100644 --- a/spec/javascripts/app/models/notification_spec.js +++ b/spec/javascripts/app/models/notification_spec.js @@ -18,8 +18,8 @@ describe("app.models.Notification", function() { }); describe("parse", function() { - it("correctly parses the object", function() { - var parsed = this.model.parse({ + beforeEach(function() { + this.response = { "reshared": { "id": 45, "target_type": "Post", @@ -31,9 +31,8 @@ describe("app.models.Notification", function() { "note_html": "" }, "type": "reshared" - }); - - expect(parsed).toEqual({ + }; + this.parsedResponse = { "type": "reshared", "id": 45, "target_type": "Post", @@ -43,7 +42,17 @@ describe("app.models.Notification", function() { "created_at": "2015-10-27T19:56:30.000Z", "updated_at": "2015-10-27T19:56:30.000Z", "note_html": "" - }); + }; + }); + + it("correctly parses the object", function() { + var parsed = this.model.parse(this.response); + expect(parsed).toEqual(this.parsedResponse); + }); + + it("correctly parses the object twice", function() { + var parsed = this.model.parse(this.parsedResponse); + expect(parsed).toEqual(this.parsedResponse); }); }); @@ -67,6 +76,7 @@ describe("app.models.Notification", function() { beforeEach(function() { this.target = new app.models.Notification({"reshared": {id: 16}, "type": "reshared"}); spyOn(app.models.Notification.prototype, "set").and.callThrough(); + spyOn(app.models.Notification.prototype, "trigger"); }); it("calls calls ajax with correct parameters and sets 'unread' attribute", function() { @@ -80,6 +90,7 @@ describe("app.models.Notification", function() { /* eslint-enable camelcase */ expect(call.method).toEqual("PUT"); expect(app.models.Notification.prototype.set).toHaveBeenCalledWith("unread", true); + expect(app.models.Notification.prototype.trigger).toHaveBeenCalledWith("userChangedUnreadStatus", this.target); }); }); }); diff --git a/spec/javascripts/app/views/notification_dropdown_view_spec.js b/spec/javascripts/app/views/notification_dropdown_view_spec.js index 69dcb4a56..47c360603 100644 --- a/spec/javascripts/app/views/notification_dropdown_view_spec.js +++ b/spec/javascripts/app/views/notification_dropdown_view_spec.js @@ -15,9 +15,11 @@ describe("app.views.NotificationDropdown", function() { this.view.collection.off("pushFront"); this.view.collection.off("pushBack"); this.view.collection.off("finishedLoading"); + this.view.collection.off("change:note_html"); spyOn(this.view, "onPushFront"); spyOn(this.view, "onPushBack"); spyOn(this.view, "finishLoading"); + spyOn(this.view, "onNotificationChange"); }); it("binds collection events", function() { @@ -26,10 +28,12 @@ describe("app.views.NotificationDropdown", function() { this.collection.trigger("pushFront"); this.collection.trigger("pushBack"); this.collection.trigger("finishedLoading"); + this.collection.trigger("change:note_html"); expect(this.view.onPushFront).toHaveBeenCalled(); expect(this.view.onPushBack).toHaveBeenCalled(); expect(this.view.finishLoading).toHaveBeenCalled(); + expect(this.view.onNotificationChange).toHaveBeenCalled(); }); });