From ade9b972152fa9046812b7146e4c796d9be83acd Mon Sep 17 00:00:00 2001 From: Steffen van Bergerem Date: Sat, 7 Jan 2017 00:27:43 +0100 Subject: [PATCH 1/3] Update backbonejs, switch to rails-assets source --- Gemfile | 3 ++- Gemfile.lock | 16 ++++------------ 2 files changed, 6 insertions(+), 13 deletions(-) diff --git a/Gemfile b/Gemfile index 3ae7baf9b..f2d2289ec 100644 --- a/Gemfile +++ b/Gemfile @@ -85,7 +85,6 @@ gem "entypo-rails", "3.0.0.pre.rc2" # JavaScript -gem "backbone-on-rails", "1.2.0.0" gem "handlebars_assets", "0.23.1" gem "jquery-rails", "4.2.1" gem "jquery-ui-rails", "5.0.5" @@ -103,6 +102,8 @@ source "https://rails-assets.org" do gem "rails-assets-markdown-it-sub", "1.0.0" gem "rails-assets-markdown-it-sup", "1.0.0" gem "rails-assets-highlightjs", "9.7.0" + + gem "rails-assets-backbone", "1.3.3" gem "rails-assets-bootstrap-markdown", "2.10.0" gem "rails-assets-corejs-typeahead", "1.0.1" gem "rails-assets-fineuploader-dist", "5.11.0" diff --git a/Gemfile.lock b/Gemfile.lock index 7528defe7..597609ce8 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -58,11 +58,6 @@ GEM attr_required (1.0.1) autoprefixer-rails (6.5.1) execjs - backbone-on-rails (1.2.0.0) - eco - ejs - jquery-rails - railties bcrypt (3.1.11) bindata (2.3.1) bootstrap-sass (3.3.7) @@ -191,12 +186,6 @@ GEM docile (1.1.5) domain_name (0.5.20160615) unf (>= 0.0.5, < 1.0.0) - eco (1.0.0) - coffee-script - eco-source - execjs - eco-source (1.1.0.rc.1) - ejs (1.1.1) entypo-rails (3.0.0.pre.rc2) railties (>= 4.1, <= 5) equalizer (0.0.10) @@ -641,6 +630,8 @@ GEM railties (= 4.2.7.1) sprockets-rails rails-assets-autosize (3.0.20) + rails-assets-backbone (1.3.3) + rails-assets-underscore (>= 1.8.3) rails-assets-blueimp-gallery (2.21.3) rails-assets-bootstrap (3.3.7) rails-assets-jquery (>= 1.9.1, < 4) @@ -681,6 +672,7 @@ GEM rails-assets-markdown-it-sub (1.0.0) rails-assets-markdown-it-sup (1.0.0) rails-assets-perfect-scrollbar (0.6.12) + rails-assets-underscore (1.8.3) rails-deprecated_sanitizer (1.0.3) activesupport (>= 4.2.0.alpha) rails-dom-testing (1.0.7) @@ -922,7 +914,6 @@ DEPENDENCIES addressable (= 2.4.0) asset_sync (= 1.1.0) autoprefixer-rails (= 6.5.1) - backbone-on-rails (= 1.2.0.0) bootstrap-sass (= 3.3.7) bootstrap-switch-rails (= 3.3.3) capybara (= 2.10.1) @@ -998,6 +989,7 @@ DEPENDENCIES rack-ssl (= 1.4.1) rails (= 4.2.7.1) rails-assets-autosize (= 3.0.20)! + rails-assets-backbone (= 1.3.3)! rails-assets-blueimp-gallery (= 2.21.3)! rails-assets-bootstrap-markdown (= 2.10.0)! rails-assets-corejs-typeahead (= 1.0.1)! From abe7ef3d18b8ed29a7098edcd3f8bc1ca0c7b511 Mon Sep 17 00:00:00 2001 From: Steffen van Bergerem Date: Wed, 14 Dec 2016 20:57:51 +0100 Subject: [PATCH 2/3] 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(); }); }); From 6e7910b037624d6fd1cb7a051337a3faef3914ba Mon Sep 17 00:00:00 2001 From: Steffen van Bergerem Date: Fri, 13 Jan 2017 00:09:31 +0100 Subject: [PATCH 3/3] Deduplicate notification dropdown view closes #7270 --- Changelog.md | 1 + .../app/views/notification_dropdown_view.js | 24 +++--- .../views/notification_dropdown_view_spec.js | 73 +++++++++++++++++++ spec/javascripts/jasmine_helpers/factory.js | 17 +++++ 4 files changed, 105 insertions(+), 10 deletions(-) diff --git a/Changelog.md b/Changelog.md index ff5610316..b6462294e 100644 --- a/Changelog.md +++ b/Changelog.md @@ -9,6 +9,7 @@ * Fix background color of year on notifications page with dark theme [#7263](https://github.com/diaspora/diaspora/pull/7263) * Fix jasmine tests in firefox [#7246](https://github.com/diaspora/diaspora/pull/7246) * Prevent scroll to top when clicking 'mark all as read' in the notification dropdown [#7253](https://github.com/diaspora/diaspora/pull/7253) +* Update existing notifications in dropdown on fetch [#7270](https://github.com/diaspora/diaspora/pull/7270) ## Features * Add links to the aspects and followed tags pages on mobile [#7265](https://github.com/diaspora/diaspora/pull/7265) diff --git a/app/assets/javascripts/app/views/notification_dropdown_view.js b/app/assets/javascripts/app/views/notification_dropdown_view.js index fb4490829..c3f8f1911 100644 --- a/app/assets/javascripts/app/views/notification_dropdown_view.js +++ b/app/assets/javascripts/app/views/notification_dropdown_view.js @@ -77,22 +77,26 @@ app.views.NotificationDropdown = app.views.Base.extend({ }, onPushBack: function(notification) { - var node = this.dropdownNotifications.append(notification.get("note_html")); - $(node).find(".unread-toggle .entypo-eye").tooltip("destroy").tooltip(); - $(node).find(this.avatars.selector).error(this.avatars.fallback); + var node = $(notification.get("note_html")); + this.dropdownNotifications.append(node); + this.afterNotificationChanges(node); }, onPushFront: function(notification) { - var node = this.dropdownNotifications.prepend(notification.get("note_html")); - $(node).find(".unread-toggle .entypo-eye").tooltip("destroy").tooltip(); - $(node).find(this.avatars.selector).error(this.avatars.fallback); + var node = $(notification.get("note_html")); + this.dropdownNotifications.prepend(node); + this.afterNotificationChanges(node); }, 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); + var node = $(notification.get("note_html")); + this.dropdownNotifications.find("[data-guid=" + notification.get("id") + "]").replaceWith(node); + this.afterNotificationChanges(node); + }, + + afterNotificationChanges: function(node) { + node.find(".unread-toggle .entypo-eye").tooltip("destroy").tooltip(); + node.find(this.avatars.selector).error(this.avatars.fallback); }, finishLoading: function() { diff --git a/spec/javascripts/app/views/notification_dropdown_view_spec.js b/spec/javascripts/app/views/notification_dropdown_view_spec.js index 47c360603..5a1452672 100644 --- a/spec/javascripts/app/views/notification_dropdown_view_spec.js +++ b/spec/javascripts/app/views/notification_dropdown_view_spec.js @@ -110,4 +110,77 @@ describe("app.views.NotificationDropdown", function() { expect($.fn.perfectScrollbar).not.toHaveBeenCalled(); }); }); + + describe("notification changes", function() { + beforeEach(function() { + this.collection.fetch(); + jasmine.Ajax.requests.mostRecent().respondWith({ + status: 200, + responseText: spec.readFixture("notifications_collection") + }); + this.notification = factory.notification({ + "id": 1337, + "note_html": "
This is a notification
" + }); + expect(this.collection.length).toBeGreaterThan(0); + expect(this.view.$(".notifications .stream-element").length).toBe(this.collection.length); + }); + + describe("onPushBack", function() { + it("adds the notification at the end of the rendered list", function() { + this.view.onPushBack(this.notification); + expect(this.view.$(".notifications .stream-element").length).toBe(this.collection.length + 1); + expect(this.view.$(".notifications .stream-element").last().text()).toBe("This is a notification"); + }); + + it("calls afterNotificationChanges", function() { + spyOn(this.view, "afterNotificationChanges"); + this.view.onPushBack(this.notification); + expect(this.view.afterNotificationChanges).toHaveBeenCalled(); + var node = this.view.afterNotificationChanges.calls.mostRecent().args[0]; + expect(node.text()).toBe("This is a notification"); + }); + }); + + describe("onPushFront", function() { + it("adds the notification to the beginning of the rendered list", function() { + this.view.onPushFront(this.notification); + expect(this.view.$(".notifications .stream-element").length).toBe(this.collection.length + 1); + expect(this.view.$(".notifications .stream-element").first().text()).toBe("This is a notification"); + }); + + it("calls afterNotificationChanges", function() { + spyOn(this.view, "afterNotificationChanges"); + this.view.onPushFront(this.notification); + expect(this.view.afterNotificationChanges).toHaveBeenCalled(); + var node = this.view.afterNotificationChanges.calls.mostRecent().args[0]; + expect(node.text()).toBe("This is a notification"); + }); + }); + + describe("onNotificationChange", function() { + beforeEach(function() { + // create a notification which replaces the first in the collection + var firstNoteId = this.collection.models[0].attributes.id; + this.notification = factory.notification({ + "id": firstNoteId, + "note_html": "
This is a notification
" + }); + }); + + it("replaces the notification in the rendered list", function() { + this.view.onNotificationChange(this.notification); + expect(this.view.$(".notifications .stream-element").length).toBe(this.collection.length); + expect(this.view.$(".notifications .stream-element").first().text()).toBe("This is a notification"); + }); + + it("calls afterNotificationChanges", function() { + spyOn(this.view, "afterNotificationChanges"); + this.view.onNotificationChange(this.notification); + expect(this.view.afterNotificationChanges).toHaveBeenCalled(); + var node = this.view.afterNotificationChanges.calls.mostRecent().args[0]; + expect(node.text()).toBe("This is a notification"); + }); + }); + }); }); diff --git a/spec/javascripts/jasmine_helpers/factory.js b/spec/javascripts/jasmine_helpers/factory.js index d21435651..7732e8281 100644 --- a/spec/javascripts/jasmine_helpers/factory.js +++ b/spec/javascripts/jasmine_helpers/factory.js @@ -55,6 +55,23 @@ var factory = { return new app.models.Contact(_.extend(attrs, overrides)); }, + notification: function(overrides) { + var noteId = this.id.next(); + var defaultAttrs = { + "type": "reshared", + "id": noteId, + "target_type": "Post", + "target_id": this.id.next(), + "recipient_id": this.id.next(), + "unread": true, + "created_at": "2012-01-04T00:55:30Z", + "updated_at": "2012-01-04T00:55:30Z", + "note_html": "This is a notification!" + }; + + return new app.models.Notification(_.extend(defaultAttrs, overrides)); + }, + user : function(overrides) { return new app.models.User(factory.userAttrs(overrides)); },