Merge branch 'master' into vdom

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});

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() {
+  findStale(store, type, findArgs, options) {
-    return new StaleResult();
+    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) {

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'),

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;

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
+    // clear old cached notifications, we used to store in local storage
-    // they could be a week old for all we know
+    // 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);
+          oldNotifications.splice(insertPosition, 0, Em.Object.create(lastNotification));
-          keyValueStore.setItem('recent-notifications', JSON.stringify(stale));
+
+          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);

app/assets/javascripts/discourse/lib/stale-result.js.es6

@@ -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;

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

app/assets/javascripts/discourse/mixins/stale-local-storage.js.es6

@@ -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;
-    });
-  }
-};

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) {
+    return {
-      stale.results = this._hydrateFindResults(stale.results, type, findArgs);
+      hasResults: (stale !== undefined),
-    }
+      results: stale,
-    stale.refresh = () => this.find(type, findArgs, opts);
+      refresh: () => this.find(type, findArgs, opts)
-    return stale;
+    };
   },
 
   find(type, findArgs, opts) {
-    return this.adapterFor(type).find(this, type, findArgs, opts).then(result => {
+    var adapter = this.adapterFor(type);
-      return this._hydrateFindResults(result, type, findArgs, opts);
+    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 => {

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

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!

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

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?

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
     )

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

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

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

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)

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
+