diff --git a/app/assets/javascripts/discourse/adapters/notification.js.es6 b/app/assets/javascripts/discourse/adapters/notification.js.es6
index f8298a430..ddee883f6 100644
--- a/app/assets/javascripts/discourse/adapters/notification.js.es6
+++ b/app/assets/javascripts/discourse/adapters/notification.js.es6
@@ -1,4 +1,3 @@
import RestAdapter from 'discourse/adapters/rest';
-import StaleLocalStorage from 'discourse/mixins/stale-local-storage';
-export default RestAdapter.extend(StaleLocalStorage);
+export default RestAdapter.extend({cache: true});
diff --git a/app/assets/javascripts/discourse/adapters/rest.js.es6 b/app/assets/javascripts/discourse/adapters/rest.js.es6
index 1404e76d5..5faaf6d0b 100644
--- a/app/assets/javascripts/discourse/adapters/rest.js.es6
+++ b/app/assets/javascripts/discourse/adapters/rest.js.es6
@@ -1,6 +1,8 @@
-import StaleResult from 'discourse/lib/stale-result';
+import { hashString } from 'discourse/lib/hash';
+
const ADMIN_MODELS = ['plugin', 'site-customization', 'embeddable-host'];
+
export function Result(payload, responseJson) {
this.payload = payload;
this.responseJson = responseJson;
@@ -19,6 +21,15 @@ function rethrow(error) {
export default Ember.Object.extend({
+
+ storageKey(type, findArgs, options) {
+ if (options && options.cacheKey) {
+ return options.cacheKey;
+ }
+ const hashedArgs = Math.abs(hashString(JSON.stringify(findArgs)));
+ return `${type}_${hashedArgs}`;
+ },
+
basePath(store, type) {
if (ADMIN_MODELS.indexOf(type.replace('_', '-')) !== -1) { return "/admin/"; }
return "/";
@@ -56,8 +67,15 @@ export default Ember.Object.extend({
return ajax(this.pathFor(store, type, findArgs)).catch(rethrow);
},
- findStale() {
- return new StaleResult();
+ findStale(store, type, findArgs, options) {
+ if (this.cached) {
+ return this.cached[this.storageKey(type, findArgs, options)];
+ }
+ },
+
+ cacheFind(store, type, findArgs, opts, hydrated) {
+ this.cached = this.cached || {};
+ this.cached[this.storageKey(type,findArgs,opts)] = hydrated;
},
update(store, type, id, attrs) {
diff --git a/app/assets/javascripts/discourse/components/notification-item.js.es6 b/app/assets/javascripts/discourse/components/notification-item.js.es6
index 6818ba5a3..f900c1b27 100644
--- a/app/assets/javascripts/discourse/components/notification-item.js.es6
+++ b/app/assets/javascripts/discourse/components/notification-item.js.es6
@@ -61,6 +61,10 @@ export default Ember.Component.extend({
_markRead: function(){
this.$('a').click(() => {
this.set('notification.read', true);
+ Discourse.setTransientHeader("Discourse-Clear-Notifications", this.get('notification.id'));
+ if (document && document.cookie) {
+ document.cookie = `cn=${this.get('notification.id')}; expires=Fri, 31 Dec 9999 23:59:59 GMT`;
+ }
return true;
});
}.on('didInsertElement'),
diff --git a/app/assets/javascripts/discourse/components/user-menu.js.es6 b/app/assets/javascripts/discourse/components/user-menu.js.es6
index 506fc98d7..d864f23ba 100644
--- a/app/assets/javascripts/discourse/components/user-menu.js.es6
+++ b/app/assets/javascripts/discourse/components/user-menu.js.es6
@@ -54,7 +54,7 @@ export default Ember.Component.extend({
// TODO: It's a bit odd to use the store in a component, but this one really
// wants to reach out and grab notifications
const store = this.container.lookup('store:main');
- const stale = store.findStale('notification', {recent: true, limit }, {storageKey: 'recent-notifications'});
+ const stale = store.findStale('notification', {recent: true, limit }, {cacheKey: 'recent-notifications'});
if (stale.hasResults) {
const results = stale.results;
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 79157a123..5923dfca8 100644
--- a/app/assets/javascripts/discourse/initializers/subscribe-user-notifications.js.es6
+++ b/app/assets/javascripts/discourse/initializers/subscribe-user-notifications.js.es6
@@ -9,10 +9,11 @@ export default {
site = container.lookup('site:main'),
siteSettings = container.lookup('site-settings:main'),
bus = container.lookup('message-bus:main'),
- keyValueStore = container.lookup('key-value-store:main');
+ keyValueStore = container.lookup('key-value-store:main'),
+ store = container.lookup('store:main');
- // clear old cached notifications
- // they could be a week old for all we know
+ // clear old cached notifications, we used to store in local storage
+ // TODO 2017 delete this line
keyValueStore.remove('recent-notifications');
if (user) {
@@ -40,12 +41,12 @@ export default {
user.set('lastNotificationChange', new Date());
}
- var stale = keyValueStore.getObject('recent-notifications');
+ const stale = store.findStale('notification', {}, {cacheKey: 'recent-notifications'});
const lastNotification = data.last_notification && data.last_notification.notification;
- if (stale && stale.notifications && lastNotification) {
+ if (stale && stale.hasResults && lastNotification) {
- const oldNotifications = stale.notifications;
+ const oldNotifications = stale.results.get('content');
const staleIndex = _.findIndex(oldNotifications, {id: lastNotification.id});
if (staleIndex > -1) {
@@ -61,8 +62,21 @@ export default {
insertPosition = insertPosition === -1 ? oldNotifications.length - 1 : insertPosition;
}
- oldNotifications.splice(insertPosition, 0, lastNotification);
- keyValueStore.setItem('recent-notifications', JSON.stringify(stale));
+ oldNotifications.splice(insertPosition, 0, Em.Object.create(lastNotification));
+
+ var idx=0;
+ data.recent.forEach((info)=> {
+ var old = oldNotifications[idx];
+ if (old) {
+ if (old.get('id') !== info[0]) {
+ oldNotifications.splice(idx, 1);
+ return;
+ } else if (old.get('read') !== info[1]) {
+ old.set('read', info[1]);
+ }
+ }
+ idx += 1;
+ });
}
}, user.notification_channel_position);
diff --git a/app/assets/javascripts/discourse/lib/stale-result.js.es6 b/app/assets/javascripts/discourse/lib/stale-result.js.es6
deleted file mode 100644
index 1ae29e583..000000000
--- a/app/assets/javascripts/discourse/lib/stale-result.js.es6
+++ /dev/null
@@ -1,12 +0,0 @@
-const StaleResult = function() {
- this.hasResults = false;
-};
-
-StaleResult.prototype.setResults = function(results) {
- if (results) {
- this.results = results;
- this.hasResults = true;
- }
-};
-
-export default StaleResult;
diff --git a/app/assets/javascripts/discourse/mixins/ajax.js b/app/assets/javascripts/discourse/mixins/ajax.js
index fdd58cf2f..36e42f512 100644
--- a/app/assets/javascripts/discourse/mixins/ajax.js
+++ b/app/assets/javascripts/discourse/mixins/ajax.js
@@ -3,9 +3,14 @@
respect Discourse paths and the run loop.
**/
var _trackView = false;
+var _transientHeader = null;
Discourse.Ajax = Em.Mixin.create({
+ setTransientHeader: function(k, v) {
+ _transientHeader = {key: k, value: v};
+ },
+
viewTrackingRequired: function() {
_trackView = true;
},
@@ -43,6 +48,11 @@ Discourse.Ajax = Em.Mixin.create({
args.headers = args.headers || {};
+ if (_transientHeader) {
+ args.headers[_transientHeader.key] = _transientHeader.value;
+ _transientHeader = null;
+ }
+
if (_trackView && (!args.type || args.type === "GET")) {
_trackView = false;
// DON'T CHANGE: rack is prepending "HTTP_" in the header's name
diff --git a/app/assets/javascripts/discourse/mixins/stale-local-storage.js.es6 b/app/assets/javascripts/discourse/mixins/stale-local-storage.js.es6
deleted file mode 100644
index 19d849516..000000000
--- a/app/assets/javascripts/discourse/mixins/stale-local-storage.js.es6
+++ /dev/null
@@ -1,34 +0,0 @@
-import StaleResult from 'discourse/lib/stale-result';
-import { hashString } from 'discourse/lib/hash';
-
-// Mix this in to an adapter to provide stale caching in our key value store
-export default {
- storageKey(type, findArgs) {
- const hashedArgs = Math.abs(hashString(JSON.stringify(findArgs)));
- return `${type}_${hashedArgs}`;
- },
-
- findStale(store, type, findArgs, opts) {
- const staleResult = new StaleResult();
- const key = (opts && opts.storageKey) || this.storageKey(type, findArgs);
- try {
- const stored = this.keyValueStore.getItem(key);
- if (stored) {
- const parsed = JSON.parse(stored);
- staleResult.setResults(parsed);
- }
- } catch(e) {
- // JSON parsing error
- }
- return staleResult;
- },
-
- find(store, type, findArgs, opts) {
- const key = (opts && opts.storageKey) || this.storageKey(type, findArgs);
-
- return this._super(store, type, findArgs).then((results) => {
- this.keyValueStore.setItem(key, JSON.stringify(results));
- return results;
- });
- }
-};
diff --git a/app/assets/javascripts/discourse/models/store.js.es6 b/app/assets/javascripts/discourse/models/store.js.es6
index 52246ff34..af365c7ba 100644
--- a/app/assets/javascripts/discourse/models/store.js.es6
+++ b/app/assets/javascripts/discourse/models/store.js.es6
@@ -75,19 +75,43 @@ export default Ember.Object.extend({
// refresh it in the background.
findStale(type, findArgs, opts) {
const stale = this.adapterFor(type).findStale(this, type, findArgs, opts);
- if (stale.hasResults) {
- stale.results = this._hydrateFindResults(stale.results, type, findArgs);
- }
- stale.refresh = () => this.find(type, findArgs, opts);
- return stale;
+ return {
+ hasResults: (stale !== undefined),
+ results: stale,
+ refresh: () => this.find(type, findArgs, opts)
+ };
},
find(type, findArgs, opts) {
- return this.adapterFor(type).find(this, type, findArgs, opts).then(result => {
- return this._hydrateFindResults(result, type, findArgs, opts);
+ var adapter = this.adapterFor(type);
+ return adapter.find(this, type, findArgs, opts).then(result => {
+ var hydrated = this._hydrateFindResults(result, type, findArgs, opts);
+ if (adapter.cache) {
+ const stale = adapter.findStale(this, type, findArgs, opts);
+ hydrated = this._updateStale(stale, hydrated);
+ adapter.cacheFind(this, type, findArgs, opts, hydrated);
+ }
+ return hydrated;
});
},
+ _updateStale(stale, hydrated) {
+ if (!stale) {
+ return hydrated;
+ }
+
+ hydrated.set('content', hydrated.get('content').map((item) => {
+ var staleItem = stale.content.findBy('id', item.get('id'));
+ if (staleItem) {
+ staleItem.setProperties(item);
+ } else {
+ staleItem = item;
+ }
+ return staleItem;
+ }));
+ return hydrated;
+ },
+
refreshResults(resultSet, type, url) {
const self = this;
return Discourse.ajax(url).then(result => {
diff --git a/app/assets/javascripts/main_include.js b/app/assets/javascripts/main_include.js
index dd6a27e13..0c11c4398 100644
--- a/app/assets/javascripts/main_include.js
+++ b/app/assets/javascripts/main_include.js
@@ -7,7 +7,6 @@
//= require ./ember-addons/macro-alias
//= require ./ember-addons/ember-computed-decorators
//= require ./discourse/lib/hash
-//= require ./discourse/lib/stale-result
//= require ./discourse/lib/load-script
//= require ./discourse/lib/notification-levels
//= require ./discourse/lib/app-events
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 05f850e30..6dd3f81ad 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -33,6 +33,7 @@ class ApplicationController < ActionController::Base
end
before_filter :set_current_user_for_logs
+ before_filter :clear_notifications
before_filter :set_locale
before_filter :set_mobile_view
before_filter :inject_preview_style
@@ -137,6 +138,28 @@ class ApplicationController < ActionController::Base
response.headers["X-Discourse-Route"] = "#{controller_name}/#{action_name}"
end
+ def clear_notifications
+ if current_user && !Discourse.readonly_mode?
+
+ cookie_notifications = cookies['cn'.freeze]
+ notifications = request.headers['Discourse-Clear-Notifications'.freeze]
+
+ if cookie_notifications
+ if notifications.present?
+ notifications += "," << cookie_notifications
+ else
+ notifications = cookie_notifications
+ end
+ end
+
+ if notifications.present?
+ notification_ids = notifications.split(",").map(&:to_i)
+ Notification.where(user_id: current_user.id, id: notification_ids).update_all(read: true)
+ cookies.delete('cn')
+ end
+ end
+ end
+
def set_locale
I18n.locale = current_user.try(:effective_locale) || SiteSetting.default_locale
I18n.ensure_all_loaded!
diff --git a/app/models/category_user.rb b/app/models/category_user.rb
index e7138d91d..7f0ab17a8 100644
--- a/app/models/category_user.rb
+++ b/app/models/category_user.rb
@@ -95,7 +95,14 @@ class CategoryUser < ActiveRecord::Base
end
def self.ensure_consistency!
- exec_sql("DELETE FROM category_users WHERE user_id NOT IN (SELECT id FROM users)")
+ exec_sql <<SQL
+ DELETE FROM category_users
+ WHERE user_id IN (
+ SELECT cu.user_id FROM category_users cu
+ LEFT JOIN users u ON u.id = cu.user_id
+ WHERE u.id IS NULL
+ )
+SQL
end
private_class_method :apply_default_to_topic, :remove_default_from_topic
diff --git a/app/models/notification.rb b/app/models/notification.rb
index bffb70dce..d21875c8b 100644
--- a/app/models/notification.rb
+++ b/app/models/notification.rb
@@ -130,15 +130,29 @@ class Notification < ActiveRecord::Base
.to_a
if notifications.present?
- notifications += user
- .notifications
- .order('notifications.created_at DESC')
- .where(read: false, notification_type: Notification.types[:private_message])
- .joins(:topic)
- .where('notifications.id < ?', notifications.last.id)
- .limit(count)
- notifications.sort do |x,y|
+ ids = Notification.exec_sql("
+ SELECT n.id FROM notifications n
+ WHERE
+ n.notification_type = 6 AND
+ n.user_id = #{user.id.to_i} AND
+ NOT read
+ ORDER BY n.id ASC
+ LIMIT #{count.to_i}
+ ").values.map do |x,_|
+ x.to_i
+ end
+
+ if ids.length > 0
+ notifications += user
+ .notifications
+ .order('notifications.created_at DESC')
+ .where(id: ids)
+ .joins(:topic)
+ .limit(count)
+ end
+
+ notifications.uniq(&:id).sort do |x,y|
if x.unread_pm? && !y.unread_pm?
-1
elsif y.unread_pm? && !x.unread_pm?
diff --git a/app/models/user.rb b/app/models/user.rb
index 639846ee2..a1be4c16b 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -293,15 +293,6 @@ class User < ActiveRecord::Base
def saw_notification_id(notification_id)
User.where("id = ? and seen_notification_id < ?", id, notification_id)
.update_all ["seen_notification_id = ?", notification_id]
-
- # some notifications are considered read once seen
- Notification.where('user_id = ? AND NOT read AND notification_type IN (?)', id, [
- Notification.types[:granted_badge],
- Notification.types[:invitee_accepted],
- Notification.types[:group_message_summary],
- Notification.types[:liked]
- ])
- .update_all ["read = ?", true]
end
def publish_notifications_state
@@ -310,11 +301,43 @@ class User < ActiveRecord::Base
notification = notifications.visible.order('notifications.id desc').first
json = NotificationSerializer.new(notification).as_json if notification
+
+ sql = "
+ SELECT * FROM (
+ SELECT n.id, n.read FROM notifications n
+ LEFT JOIN topics t ON n.topic_id = t.id
+ WHERE
+ t.deleted_at IS NULL AND
+ n.notification_type = :type AND
+ n.user_id = :user_id AND
+ NOT read
+ ORDER BY n.id DESC
+ LIMIT 20
+ ) AS x
+ UNION ALL
+ SELECT * FROM (
+ SELECT n.id, n.read FROM notifications n
+ LEFT JOIN topics t ON n.topic_id = t.id
+ WHERE
+ t.deleted_at IS NULL AND
+ (n.notification_type <> :type OR read) AND
+ n.user_id = :user_id
+ ORDER BY n.id DESC
+ LIMIT 20
+ ) AS y
+ "
+
+ recent = User.exec_sql(sql, user_id: id,
+ type: Notification.types[:private_message]).values.map do |id, read|
+ [id.to_i, read == 't'.freeze]
+ end
+
MessageBus.publish("/notification/#{id}",
{unread_notifications: unread_notifications,
unread_private_messages: unread_private_messages,
total_unread_notifications: total_unread_notifications,
- last_notification: json
+ last_notification: json,
+ recent: recent
},
user_ids: [id] # only publish the notification to this user
)
diff --git a/app/models/user_archived_message.rb b/app/models/user_archived_message.rb
index 3a54b6ea6..0617c76f0 100644
--- a/app/models/user_archived_message.rb
+++ b/app/models/user_archived_message.rb
@@ -3,6 +3,12 @@ class UserArchivedMessage < ActiveRecord::Base
belongs_to :topic
def self.move_to_inbox!(user_id, topic_id)
+ return if (TopicUser.where(
+ user_id: user_id,
+ topic_id: topic_id,
+ notification_level: TopicUser.notification_levels[:muted]
+ ).exists?)
+
UserArchivedMessage.where(user_id: user_id, topic_id: topic_id).destroy_all
MessageBus.publish("/topic/#{topic_id}", {type: "move_to_inbox"}, user_ids: [user_id])
end
diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml
index 7c1a72820..78d65d384 100644
--- a/config/locales/server.en.yml
+++ b/config/locales/server.en.yml
@@ -1287,7 +1287,7 @@ en:
moved_post: "%{display_username} moved your post to %{link}"
private_message: "%{display_username} sent you a message: %{link}"
invited_to_private_message: "%{display_username} invited you to a message: %{link}"
- invited_to_topic: "%{display_username} invited you to a topic: %{link}"
+ invited_to_topic: "%{display_username} invited you to %{link}"
invitee_accepted: "%{display_username} accepted your invitation"
linked: "%{display_username} linked you in %{link}"
granted_badge: "You earned %{link}"
@@ -2041,7 +2041,7 @@ en:
Please visit this link to view the message: %{base_url}%{url}
user_invited_to_topic:
- subject_template: "[%{site_name}] %{username} invited you to a topic '%{topic_title}'"
+ subject_template: "[%{site_name}] %{username} invited you to '%{topic_title}'"
text_body_template: |
%{username} invited you to a discussion
diff --git a/db/migrate/20160215075528_add_unread_pm_index_to_notifications.rb b/db/migrate/20160215075528_add_unread_pm_index_to_notifications.rb
new file mode 100644
index 000000000..382489e2f
--- /dev/null
+++ b/db/migrate/20160215075528_add_unread_pm_index_to_notifications.rb
@@ -0,0 +1,6 @@
+class AddUnreadPmIndexToNotifications < ActiveRecord::Migration
+ def change
+ # create index idxtmp on notifications(user_id, id) where notification_type = 6 AND NOT read
+ add_index :notifications, [:user_id, :id], unique: true, where: 'notification_type = 6 AND NOT read'
+ end
+end
diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb
index fc1911efd..48a217039 100644
--- a/spec/controllers/application_controller_spec.rb
+++ b/spec/controllers/application_controller_spec.rb
@@ -85,6 +85,35 @@ describe TopicsController do
end
+ describe 'clear_notifications' do
+ it 'correctly clears notifications if specified via cookie' do
+ notification = Fabricate(:notification)
+ log_in_user(notification.user)
+
+ request.cookies['cn'] = "2828,100,#{notification.id}"
+
+ get :show, {topic_id: 100}
+
+ expect(response.cookies['cn']).to eq nil
+
+ notification.reload
+ expect(notification.read).to eq true
+
+ end
+
+ it 'correctly clears notifications if specified via header' do
+ notification = Fabricate(:notification)
+ log_in_user(notification.user)
+
+ request.headers['Discourse-Clear-Notifications'] = "2828,100,#{notification.id}"
+
+ get :show, {topic_id: 100}
+
+ notification.reload
+ expect(notification.read).to eq true
+ end
+ end
+
describe 'set_locale' do
it 'sets the one the user prefers' do
SiteSetting.stubs(:allow_user_locale).returns(true)
diff --git a/spec/models/user_archived_message_spec.rb b/spec/models/user_archived_message_spec.rb
new file mode 100644
index 000000000..cb3567b5d
--- /dev/null
+++ b/spec/models/user_archived_message_spec.rb
@@ -0,0 +1,21 @@
+require 'rails_helper'
+
+describe UserArchivedMessage do
+ it 'Does not move archived muted messages back to inbox' do
+ user = Fabricate(:admin)
+ user2 = Fabricate(:admin)
+
+ topic = create_post(user: user,
+ skip_validations: true,
+ target_usernames: [user2.username,user.username].join(","),
+ archetype: Archetype.private_message).topic
+
+ UserArchivedMessage.archive!(user.id, topic.id)
+ expect(topic.message_archived?(user)).to eq(true)
+
+ TopicUser.change(user.id, topic.id, notification_level: TopicUser.notification_levels[:muted])
+ UserArchivedMessage.move_to_inbox!(user.id, topic.id)
+ expect(topic.message_archived?(user)).to eq(true)
+ end
+end
+