From 4dfc1907acb8c11cc5493decbaaf4f525c58caaa Mon Sep 17 00:00:00 2001 From: Sam Date: Mon, 4 May 2015 13:49:32 +1000 Subject: [PATCH] Simplify desktop notifications, only include if mention/reply/pm/link Include post excerpt in the notification --- .../discourse/controllers/notification.js.es6 | 22 ++- .../subscribe-user-notifications.js.es6 | 6 +- .../lib/desktop-notifications.js.es6 | 169 ++++-------------- app/services/post_alerter.rb | 25 ++- config/locales/client.en.yml | 24 +-- 5 files changed, 87 insertions(+), 159 deletions(-) diff --git a/app/assets/javascripts/discourse/controllers/notification.js.es6 b/app/assets/javascripts/discourse/controllers/notification.js.es6 index c0245002b..e13377d69 100644 --- a/app/assets/javascripts/discourse/controllers/notification.js.es6 +++ b/app/assets/javascripts/discourse/controllers/notification.js.es6 @@ -1,8 +1,26 @@ import ObjectController from 'discourse/controllers/object'; -import { notificationUrl } from 'discourse/lib/desktop-notifications'; + +const INVITED_TYPE = 8; export default ObjectController.extend({ + notificationUrl: function(it) { + var badgeId = it.get("data.badge_id"); + if (badgeId) { + var badgeName = it.get("data.badge_name"); + return Discourse.getURL('/badges/' + badgeId + '/' + badgeName.replace(/[^A-Za-z0-9_]+/g, '-').toLowerCase()); + } + + var topicId = it.get('topic_id'); + if (topicId) { + return Discourse.Utilities.postUrl(it.get("slug"), topicId, it.get("post_number")); + } + + if (it.get('notification_type') === INVITED_TYPE) { + return Discourse.getURL('/my/invited'); + } + }, + scope: function() { return "notifications." + this.site.get("notificationLookup")[this.get("notification_type")]; }.property("notification_type"), @@ -10,7 +28,7 @@ export default ObjectController.extend({ username: Em.computed.alias("data.display_username"), url: function() { - return notificationUrl(this); + return this.notificationUrl(this); }.property("data.{badge_id,badge_name}", "slug", "topic_id", "post_number"), description: function() { diff --git a/app/assets/javascripts/discourse/initializers/subscribe-user-notifications.js.es6 b/app/assets/javascripts/discourse/initializers/subscribe-user-notifications.js.es6 index aef792098..d9f5dfd29 100644 --- a/app/assets/javascripts/discourse/initializers/subscribe-user-notifications.js.es6 +++ b/app/assets/javascripts/discourse/initializers/subscribe-user-notifications.js.es6 @@ -37,6 +37,11 @@ export default { user.set('post_queue_new_count', data.post_queue_new_count); }); } + + bus.subscribe("/notification-alert/" + user.get('id'), function(data){ + onNotification(data, user); + }); + bus.subscribe("/notification/" + user.get('id'), function(data) { const oldUnread = user.get('unread_notifications'); const oldPM = user.get('unread_private_messages'); @@ -46,7 +51,6 @@ export default { if (oldUnread !== data.unread_notifications || oldPM !== data.unread_private_messages) { user.set('lastNotificationChange', new Date()); - onNotification(user); } }, user.notification_channel_position); diff --git a/app/assets/javascripts/discourse/lib/desktop-notifications.js.es6 b/app/assets/javascripts/discourse/lib/desktop-notifications.js.es6 index 8dfedc2a1..27cdb9091 100644 --- a/app/assets/javascripts/discourse/lib/desktop-notifications.js.es6 +++ b/app/assets/javascripts/discourse/lib/desktop-notifications.js.es6 @@ -6,10 +6,7 @@ let mbClientId = "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx"; let lastAction = -1; const focusTrackerKey = "focus-tracker"; -const seenDataKey = "seen-notifications"; -const recentUpdateThreshold = 1000 * 60 * 2; // 2 minutes const idleThresholdTime = 1000 * 10; // 10 seconds -const INVITED_TYPE = 8; let notificationTagName; // "discourse-notification-popup-" + Discourse.SiteSettings.title; // Called from an initializer @@ -57,28 +54,9 @@ function init(messageBus) { // This function is only called if permission was granted function setupNotifications() { - // Load up the current state of the notifications - const seenData = JSON.parse(localStorage.getItem(seenDataKey)); - let markAllSeen = true; - if (seenData) { - const lastUpdatedAt = new Date(seenData.updated_at); - if (lastUpdatedAt.getTime() + recentUpdateThreshold > new Date().getTime()) { - // The following conditions are met: - // - This is a new Discourse tab - // - The seen notification data was updated in the last 2 minutes - // Therefore, there is no need to reset the data. - markAllSeen = false; - } - } - if (markAllSeen) { - Discourse.ajax("/notifications.json?silent=true").then(function(result) { - updateSeenNotificationDatesFrom(result); - }); - } notificationTagName = "discourse-notification-popup-" + Discourse.SiteSettings.title; - window.addEventListener("storage", function(e) { // note: This event only fires when other tabs setItem() const key = e.key; @@ -117,96 +95,41 @@ function isIdle() { } // Call-in point from message bus -function onNotification(currentUser) { +function onNotification(data) { if (!liveEnabled) { return; } if (!primaryTab) { return; } + if (!isIdle()) { return; } - const blueNotifications = currentUser.get('unread_notifications'); - const greenNotifications = currentUser.get('unread_private_messages'); - - if (blueNotifications > 0 || greenNotifications > 0) { - Discourse.ajax("/notifications.json?silent=true").then(function(result) { - - const unread = result.filter(n => !n.read); - const unseen = updateSeenNotificationDatesFrom(result); - const unreadCount = unread.length; - const unseenCount = unseen.length; - - - // If all notifications are seen, don't display - if (unreadCount === 0 || unseenCount === 0) { - return; - } - // If active in last 10 seconds, don't display - if (!isIdle()) { - return; - } - - let bodyParts = []; - - unread.forEach(function(n) { - const i18nOpts = { - username: n.data['display_username'], - topic: n.data['topic_title'], - badge: n.data['badge_name'] - }; - - bodyParts.push(I18n.t(i18nKey(n), i18nOpts)); - }); - - const notificationTitle = I18n.t('notifications.popup_title', { count: unreadCount, site_title: Discourse.SiteSettings.title }); - const notificationBody = bodyParts.join("\n"); - const notificationIcon = Discourse.SiteSettings.logo_small_url || Discourse.SiteSettings.logo_url; - - requestPermission().then(function() { - // This shows the notification! - const notification = new Notification(notificationTitle, { - body: notificationBody, - icon: notificationIcon, - tag: notificationTagName - }); - - const firstUnseen = unseen[0]; - - function clickEventHandler() { - Discourse.URL.routeTo(_notificationUrl(firstUnseen)); - // Cannot delay this until the page renders - // due to trigger-based permissions - window.focus(); - } - - notification.addEventListener('click', clickEventHandler); - setTimeout(function() { - notification.close(); - notification.removeEventListener('click', clickEventHandler); - }, 10 * 1000); - }); - }); - } -} - -const DATA_VERSION = 2; -function updateSeenNotificationDatesFrom(notifications) { - const oldSeenData = JSON.parse(localStorage.getItem(seenDataKey)); - const oldSeenNotificationDates = (oldSeenData && oldSeenData.v === DATA_VERSION) ? oldSeenData.data : []; - let newSeenNotificationDates = []; - let previouslyUnseenNotifications = []; - - notifications.forEach(function(notification) { - const dateString = new Date(notification.created_at).toUTCString(); - - if (oldSeenNotificationDates.indexOf(dateString) === -1) { - previouslyUnseenNotifications.push(notification); - } - newSeenNotificationDates.push(dateString); + const notificationTitle = I18n.t(i18nKey(data.notification_type), { + site_title: Discourse.SiteSettings.title, + topic: data.topic_title, + username: data.username }); - localStorage.setItem(seenDataKey, JSON.stringify({ - data: newSeenNotificationDates, - updated_at: new Date(), - v: DATA_VERSION - })); - return previouslyUnseenNotifications; + const notificationBody = data.excerpt; + const notificationIcon = Discourse.SiteSettings.logo_small_url || Discourse.SiteSettings.logo_url; + + requestPermission().then(function() { + // This shows the notification! + const notification = new Notification(notificationTitle, { + body: notificationBody, + icon: notificationIcon, + tag: notificationTagName + }); + + function clickEventHandler() { + Discourse.URL.routeTo(data.post_url); + // Cannot delay this until the page renders + // due to trigger-based permissions + window.focus(); + } + + notification.addEventListener('click', clickEventHandler); + setTimeout(function() { + notification.close(); + notification.removeEventListener('click', clickEventHandler); + }, 10 * 1000); + }); } // Utility function @@ -229,36 +152,10 @@ function requestPermission() { } } -function i18nKey(notification) { - let key = "notifications.popup." + Discourse.Site.current().get("notificationLookup")[notification.notification_type]; - if (notification.data.display_username && notification.data.original_username && - notification.data.display_username !== notification.data.original_username) { - key += "_mul"; - } - return key; +function i18nKey(notification_type) { + return "notifications.popup." + Discourse.Site.current().get("notificationLookup")[notification_type]; } // Exported for controllers/notification.js.es6 -function notificationUrl(it) { - var badgeId = it.get("data.badge_id"); - if (badgeId) { - var badgeName = it.get("data.badge_name"); - return Discourse.getURL('/badges/' + badgeId + '/' + badgeName.replace(/[^A-Za-z0-9_]+/g, '-').toLowerCase()); - } - var topicId = it.get('topic_id'); - if (topicId) { - return Discourse.Utilities.postUrl(it.get("slug"), topicId, it.get("post_number")); - } - - if (it.get('notification_type') === INVITED_TYPE) { - return Discourse.getURL('/my/invited'); - } -} - -function _notificationUrl(notificationJson) { - const it = Em.Object.create(notificationJson); - return notificationUrl(it); -} - -export { init, notificationUrl, onNotification }; +export { init, onNotification }; diff --git a/app/services/post_alerter.rb b/app/services/post_alerter.rb index 819e4b2a0..5115e1c2f 100644 --- a/app/services/post_alerter.rb +++ b/app/services/post_alerter.rb @@ -80,6 +80,10 @@ class PostAlerter user.reload end + NOTIFIABLE_TYPES = [:mentioned, :replied, :quoted, :posted, :linked, :private_message].map{ |t| + Notification.types[t] + } + def create_notification(user, type, post, opts={}) return if user.blank? return if user.id == Discourse::SYSTEM_USER_ID @@ -101,9 +105,9 @@ class PostAlerter # Don't notify the same user about the same notification on the same post existing_notification = user.notifications .order("notifications.id desc") - .find_by(notification_type: type, topic_id: post.topic_id, post_number: post.post_number) + .find_by(topic_id: post.topic_id, post_number: post.post_number) - if existing_notification + if existing_notification && existing_notification.notification_type == type return unless existing_notification.notification_type == Notification.types[:edited] && existing_notification.data_hash["display_username"] = opts[:display_username] end @@ -143,6 +147,23 @@ class PostAlerter original_post_id: original_post.id, original_username: original_username, display_username: opts[:display_username] || post.user.username }.to_json) + + if (!existing_notification) && NOTIFIABLE_TYPES.include?(type) + + # we may have an invalid post somehow, dont blow up + post_url = original_post.url rescue nil + if post_url + MessageBus.publish("/notification-alert/#{user.id}", { + notification_type: type, + post_number: original_post.post_number, + topic_title: original_post.topic.title, + excerpt: original_post.excerpt(400, text_entities: true, strip_links: true), + username: original_username, + post_url: post_url + }, user_ids: [user.id]) + end + end + end # TODO: Move to post-analyzer? diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 87feb5795..3f48bb93a 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -831,25 +831,13 @@ en: linked: "

{{username}} {{description}}

" granted_badge: "

Earned '{{description}}'

" - popup_title: - one: "New notification on {{site_title}}" - other: "{{count}} new notifications on {{site_title}}" popup: - mentioned: '{{username}} mentioned you in "{{topic}}"' - quoted: '{{username}} quoted you in "{{topic}}"' - replied: '{{username}} replied to you in "{{topic}}"' - replied_mul: '{{username}} in "{{topic}}"' - posted: '{{username}} posted in "{{topic}}"' - posted_mul: '{{username}} posted in "{{topic}}"' - edited: '{{username}} edited your post in "{{topic}}"' - liked: '{{username}} liked your post in "{{topic}}"' - private_message: '{{username}} sent you a private message in "{{topic}}"' - private_message_mul: '{{username}} in "{{topic}}"' - invited_to_private_message: '{{username}} invited you to a private message: "{{topic}}"' - invitee_accepted: '{{username}} joined the forum!' - moved_post: '{{username}} moved your post in "{{topic}}"' - linked: '{{username}} linked to your post from "{{topic}}"' - granted_badge: 'You earned the "{{badge}}" badge!' + mentioned: '{{username}} mentioned you in "{{topic}}" - {{site_title}}' + quoted: '{{username}} quoted you in "{{topic}}" - {{site_title}}' + replied: '{{username}} replied to you in "{{topic}}" - {{site_title}}' + posted: '{{username}} posted in "{{topic}}" - {{site_title}}' + private_message: '{{username}} sent you a private message in "{{topic}}" - {{site_title}}' + linked: '{{username}} linked to your post from "{{topic}}" - {{site_title}}' upload_selector: title: "Add an image"