FEATURE: Consolidate likes notifications. (#6879)

app/assets/javascripts/discourse-common/lib/icon-library.js.es6

@@ -24,6 +24,7 @@ const REPLACEMENTS = {
   "notification.liked": "heart",
   "notification.liked_2": "heart",
   "notification.liked_many": "heart",
+  "notification.liked_consolidated": "heart",
   "notification.private_message": "far-envelope",
   "notification.invited_to_private_message": "far-envelope",
   "notification.invited_to_topic": "hand-point-right",

app/assets/javascripts/discourse/models/user-stream.js.es6

@@ -30,14 +30,16 @@ export default RestModel.extend({
     "/user_actions.json?offset=%@&username=%@"
   ),
 
-  filterBy(filter, noContentHelpKey) {
+  filterBy(filter, noContentHelpKey, actingUsername) {
     this.setProperties({
       filter,
       itemsLoaded: 0,
       content: [],
-      noContentHelpKey: noContentHelpKey,
+      noContentHelpKey,
-      lastLoadedUrl: null
+      lastLoadedUrl: null,
+      actingUsername
     });
+
     return this.findItems();
   },
 
@@ -77,6 +79,10 @@ export default RestModel.extend({
       findUrl += "&no_results_help_key=" + this.get("noContentHelpKey");
     }
 
+    if (this.get("actingUsername")) {
+      findUrl += `&acting_username=${this.get("actingUsername")}`;
+    }
+
     // Don't load the same stream twice. We're probably at the end.
     const lastLoadedUrl = this.get("lastLoadedUrl");
     if (lastLoadedUrl === findUrl) {

app/assets/javascripts/discourse/routes/user-activity-stream.js.es6

@@ -1,17 +1,20 @@
 import ViewingActionType from "discourse/mixins/viewing-action-type";
 
 export default Discourse.Route.extend(ViewingActionType, {
+  queryParams: {
+    acting_username: { refreshModel: true }
+  },
+
   model() {
     return this.modelFor("user").get("stream");
   },
 
-  afterModel() {
+  afterModel(model, transition) {
-    return this.modelFor("user")
+    return model.filterBy(
-      .get("stream")
+      this.get("userActionType"),
-      .filterBy(
+      this.get("noContentHelpKey") || "user_activity.no_default",
-        this.get("userActionType"),
+      transition.queryParams.acting_username
-        this.get("noContentHelpKey") || "user_activity.no_default"
+    );
-      );
   },
 
   renderTemplate() {

app/assets/javascripts/discourse/widgets/notification-item.js.es6

@@ -16,6 +16,7 @@ import { iconNode } from "discourse-common/lib/icon-library";
 const LIKED_TYPE = 5;
 const INVITED_TYPE = 8;
 const GROUP_SUMMARY_TYPE = 16;
+export const LIKED_CONSOLIDATED_TYPE = 19;
 
 createWidget("notification-item", {
   tagName: "li",
@@ -61,6 +62,14 @@ createWidget("notification-item", {
       return userPath(data.display_username);
     }
 
+    if (attrs.notification_type === LIKED_CONSOLIDATED_TYPE) {
+      return userPath(
+        `${
+          this.currentUser.username
+        }/notifications/likes-received?acting_username=${data.display_username}`
+      );
+    }
+
     if (data.group_id) {
       return userPath(data.username + "/messages/group/" + data.group_name);
     }
@@ -77,7 +86,16 @@ createWidget("notification-item", {
       return this.attrs.fancy_title;
     }
 
-    const title = data.topic_title;
+    let title;
+
+    if (this.attrs.notification_type === LIKED_CONSOLIDATED_TYPE) {
+      title = I18n.t("notifications.liked_consolidated_description", {
+        count: parseInt(data.count)
+      });
+    } else {
+      title = data.topic_title;
+    }
+
     return Ember.isEmpty(title) ? "" : escapeExpression(title);
   },
 
@@ -95,9 +113,11 @@ createWidget("notification-item", {
 
     const username = formatUsername(data.display_username);
     const description = this.description();
+
     if (notificationType === LIKED_TYPE && data.count > 1) {
       const count = data.count - 2;
       const username2 = formatUsername(data.username2);
+
       if (count === 0) {
         return I18n.t("notifications.liked_2", {
           description,

app/controllers/user_actions_controller.rb

@@ -2,7 +2,7 @@ class UserActionsController < ApplicationController
 
   def index
     params.require(:username)
-    params.permit(:filter, :offset)
+    params.permit(:filter, :offset, :acting_username)
 
     per_chunk = 30
 
@@ -11,13 +11,16 @@ class UserActionsController < ApplicationController
 
     action_types = (params[:filter] || "").split(",").map(&:to_i)
 
-    opts = { user_id: user.id,
+    opts = {
-             user: user,
+      user_id: user.id,
-             offset: params[:offset].to_i,
+      user: user,
-             limit: per_chunk,
+      offset: params[:offset].to_i,
-             action_types: action_types,
+      limit: per_chunk,
-             guardian: guardian,
+      action_types: action_types,
-             ignore_private_messages: params[:filter] ? false : true }
+      guardian: guardian,
+      ignore_private_messages: params[:filter] ? false : true,
+      acting_username: params[:acting_username]
+    }
 
     # Pending is restricted
     stream = if opts[:action_types].include?(UserAction::PENDING)

app/models/notification.rb

@@ -13,6 +13,12 @@ class Notification < ActiveRecord::Base
   scope :visible , lambda { joins('LEFT JOIN topics ON notifications.topic_id = topics.id')
     .where('topics.id IS NULL OR topics.deleted_at IS NULL') }
 
+  scope :get_liked_by, ->(user) {
+    where("data::json ->> 'original_username' = ?", user.username_lower)
+      .where(notification_type: Notification.types[:liked])
+      .order(created_at: :desc)
+  }
+
   attr_accessor :skip_send_email
 
   after_commit :send_email, on: :create
@@ -53,7 +59,8 @@ class Notification < ActiveRecord::Base
                         group_mentioned: 15,
                         group_message_summary: 16,
                         watching_first_post: 17,
-                        topic_reminder: 18
+                        topic_reminder: 18,
+                        liked_consolidated: 19,
                        )
   end
 

app/models/user_action.rb

@@ -201,6 +201,7 @@ class UserAction < ActiveRecord::Base
     ignore_private_messages = opts[:ignore_private_messages]
     offset = opts[:offset] || 0
     limit = opts[:limit] || 60
+    acting_username = opts[:acting_username]
 
     # Acting user columns. Can be extended by plugins to include custom avatar
     # columns
@@ -258,6 +259,12 @@ class UserAction < ActiveRecord::Base
       builder.where("a.user_id = :user_id", user_id: user_id.to_i)
       builder.where("a.action_type in (:action_types)", action_types: action_types) if action_types && action_types.length > 0
 
+      if acting_username
+        builder.where("u.username_lower = :acting_username",
+          acting_username: acting_username.downcase
+        )
+      end
+
       unless SiteSetting.enable_mentions?
         builder.where("a.action_type <> :mention_type", mention_type: UserAction::MENTION)
       end

app/services/post_action_notifier.rb

@@ -76,16 +76,87 @@ class PostActionNotifier
     post = post_action.post
     return if post_action.user.blank?
 
-    alerter.create_notification(
+    user_notifications = post.user.notifications
-      post.user,
+
-      Notification.types[:liked],
+    consolidation_window =
-      post,
+      SiteSetting.likes_notification_consolidation_window_mins.minutes.ago
-      display_username: post_action.user.username,
+
-      post_action_id: post_action.id,
+    liked_by_user_notifications =
-      user_id: post_action.user_id
+      user_notifications
+        .get_liked_by(post_action.user)
+        .where("created_at > ?", consolidation_window)
+
+    user_liked_consolidated_notification =
+      user_notifications
+        .where(
+          "created_at > ? AND notification_type = ?",
+          consolidation_window,
+          Notification.types[:liked_consolidated]
+        )
+        .first
+
+    if user_liked_consolidated_notification
+      update_consolidated_liked_notification_count!(
+        user_liked_consolidated_notification
+      )
+    elsif (
+      liked_by_user_notifications.count >=
+      SiteSetting.likes_notification_consolidation_threshold
     )
+      create_consolidated_liked_notification!(
+        liked_by_user_notifications,
+        post,
+        post_action
+      )
+    else
+      alerter.create_notification(
+        post.user,
+        Notification.types[:liked],
+        post,
+        display_username: post_action.user.username,
+        post_action_id: post_action.id,
+        user_id: post_action.user_id
+      )
+    end
   end
 
+  def self.update_consolidated_liked_notification_count!(notification)
+    Notification.transaction do
+      data = JSON.parse(notification.data)
+      data["count"] += 1
+
+      notification.update!(
+        data: data.to_json,
+        read: false
+      )
+    end
+  end
+  private_class_method :update_consolidated_liked_notification_count!
+
+  def self.create_consolidated_liked_notification!(notifications,
+                                                  post,
+                                                  post_action)
+
+    Notification.transaction do
+      timestamp = notifications.last.created_at
+
+      Notification.create!(
+        notification_type: Notification.types[:liked_consolidated],
+        user_id: post.user_id,
+        data: {
+          username: post_action.user.username,
+          display_username: post_action.user.username,
+          count: notifications.count + 1
+        }.to_json,
+        updated_at: timestamp,
+        created_at: timestamp
+      )
+
+      notifications.delete_all
+    end
+  end
+  private_class_method :create_consolidated_liked_notification!
+
   def self.after_create_post_revision(post_revision)
     return if @disabled
 

config/locales/client.en.yml

@@ -1522,6 +1522,10 @@ en:
       liked_many:
         one: "<span>{{username}}, {{username2}} and 1 other</span> {{description}}"
         other: "<span>{{username}}, {{username2}} and {{count}} others</span> {{description}}"
+      liked_consolidated_description:
+        one: "liked {{count}} of your posts"
+        other: "liked {{count}} of your posts"
+      liked_consolidated: "<span>{{username}}</span> {{description}}"
       private_message: "<span>{{username}}</span> {{description}}"
       invited_to_private_message: "<p><span>{{username}}</span> {{description}}"
       invited_to_topic: "<span>{{username}}</span> {{description}}"

config/locales/server.en.yml

@@ -1789,6 +1789,10 @@ en:
 
     disable_edit_notifications: "Disables edit notifications by the system user when 'download_remote_images_to_local' is active."
 
+    likes_notification_consolidation_threshold: "Number of liked notifications received before the notifications are consolidated into a single one. The window can be configured via `SiteSetting.likes_notification_consolidation_window_mins`."
+
+    likes_notification_consolidation_window_mins: "Duration in minutes where liked notifications are consolidated into a single notification once the threshold has been reaced. The threshold can be configured via `SiteSetting.likes_notification_consolidation_threshold`."
+
     automatically_unpin_topics: "Automatically unpin topics when the user reaches the bottom."
 
     read_time_word_count: "Word count per minute for calculating estimated reading time."

config/site_settings.yml

@@ -1700,6 +1700,14 @@ uncategorized:
 
   disable_edit_notifications: false
 
+  likes_notification_consolidation_threshold:
+    default: 5
+    min: 3
+
+  likes_notification_consolidation_window_mins:
+    default: 120
+    min: 1
+
   delete_drafts_older_than_n_days:
     default: 180
 

spec/fabricators/post_fabricator.rb

@@ -29,14 +29,14 @@ Fabricator(:moderator_post, from: :post) do
 end
 
 Fabricator(:basic_reply, from: :post) do
-  user(:coding_horror)
+  user(fabricator: :coding_horror)
   reply_to_post_number 1
   topic
   raw 'this reply has no quotes'
 end
 
 Fabricator(:reply, from: :post) do
-  user(:coding_horror)
+  user(fabricator: :coding_horror)
   topic
   raw '
     [quote="Evil Trout, post:1"]hello[/quote]

spec/models/notification_spec.rb

@@ -250,6 +250,32 @@ describe Notification do
     end
   end
 
+  describe '.get_liked_by' do
+    let(:post) { Fabricate(:post) }
+    let(:user) { Fabricate(:user) }
+
+    before do
+      PostActionNotifier.enable
+    end
+
+    it 'should return the right notifications' do
+      expect(Notification.get_liked_by(user)).to eq([])
+
+      expect do
+        PostAlerter.post_created(Fabricate(:basic_reply,
+          user: user,
+          topic: post.topic
+        ))
+
+        PostAction.act(user, post, PostActionType.types[:like])
+      end.to change { Notification.count }.by(2)
+
+      expect(Notification.get_liked_by(user)).to contain_exactly(
+        Notification.find_by(notification_type: Notification.types[:liked])
+      )
+    end
+  end
+
 end
 
 # pulling this out cause I don't want an observer

spec/models/post_action_spec.rb

@@ -296,6 +296,74 @@ describe PostAction do
       expect(Notification.exists?(id: notification.id)).to eq(false)
     end
 
+    describe 'likes consolidation' do
+      let(:liker) { Fabricate(:user) }
+      let(:likee) { Fabricate(:user) }
+
+      before do
+        SiteSetting.likes_notification_consolidation_threshold = 3
+      end
+
+      it 'should consolidate likes notification when the threshold is reached' do
+        freeze_time
+
+        expect do
+          4.times do
+            PostAction.act(
+              liker,
+              Fabricate(:post, user: likee),
+              PostActionType.types[:like]
+            )
+          end
+        end.to change { likee.reload.notifications.count }.by(1)
+
+        notification = likee.notifications.last
+
+        expect(notification.notification_type).to eq(
+          Notification.types[:liked_consolidated]
+        )
+
+        data = JSON.parse(notification.data)
+
+        expect(data["username"]).to eq(liker.username)
+        expect(data["display_username"]).to eq(liker.username)
+        expect(data["count"]).to eq(4)
+
+        notification.update!(read: true)
+
+        expect do
+          2.times do
+            PostAction.act(
+              liker,
+              Fabricate(:post, user: likee),
+              PostActionType.types[:like]
+            )
+          end
+        end.to_not change { likee.reload.notifications.count }
+
+        data = JSON.parse(notification.reload.data)
+
+        expect(notification.read).to eq(false)
+        expect(data["count"]).to eq(6)
+
+        freeze_time(
+          SiteSetting.likes_notification_consolidation_window_mins.minutes.since
+        )
+
+        expect do
+          PostAction.act(
+            liker,
+            Fabricate(:post, user: likee),
+            PostActionType.types[:like]
+          )
+        end.to change { likee.reload.notifications.count }.by(1)
+
+        notification = likee.notifications.last
+
+        expect(notification.notification_type).to eq(Notification.types[:liked])
+      end
+    end
+
     it "should not generate a notification if liker has been muted" do
       mutee = Fabricate(:user)
       MutedUser.create!(user_id: post.user.id, muted_user_id: mutee.id)

spec/requests/user_actions_controller_spec.rb

@@ -34,6 +34,29 @@ describe UserActionsController do
       expect(action["post_number"]).to eq(1)
     end
 
+    it 'can be filtered by acting_username' do
+      UserActionCreator.enable
+      PostActionNotifier.enable
+
+      post = Fabricate(:post)
+      user = Fabricate(:user)
+      PostAction.act(user, post, PostActionType.types[:like])
+
+      get "/user_actions.json", params: {
+        username: post.user.username,
+        acting_username: user.username
+      }
+
+      expect(response.status).to eq(200)
+
+      response_body = JSON.parse(response.body)
+
+      expect(response_body["user_actions"].count).to eq(1)
+
+      expect(response_body["user_actions"].first["acting_username"])
+        .to eq(user.username)
+    end
+
     it 'renders help text if provided for self' do
       logged_in = sign_in(Fabricate(:user))
 

test/javascripts/fixtures/notification_fixtures.js.es6

@@ -1,4 +1,6 @@
 /*jshint maxlen:10000000 */
+import { LIKED_CONSOLIDATED_TYPE } from "discourse/widgets/notification-item";
+
 export default {
   "/notifications": {
     notifications: [
@@ -10,7 +12,13 @@ export default {
         topic_id: 1234,
         slug: "a-slug",
         data: { topic_title: "some title", display_username: "velesin" }
-      }
+      },
+      {
+        id: 456,
+        notification_type: LIKED_CONSOLIDATED_TYPE,
+        read: false,
+        data: { display_username: "aquaman", count: "5"  }
+      },
     ]
   }
 };

test/javascripts/fixtures/site-fixtures.js.es6

@@ -1,3 +1,5 @@
+import { LIKED_CONSOLIDATED_TYPE } from "discourse/widgets/notification-item";
+
 export default {
   "site.json": {
     site: {
@@ -16,7 +18,8 @@ export default {
         posted: 9,
         moved_post: 10,
         linked: 11,
-        granted_badge: 12
+        granted_badge: 12,
+        liked_consolidated: LIKED_CONSOLIDATED_TYPE,
       },
       post_types: {
         regular: 1,

test/javascripts/widgets/user-menu-test.js.es6

@@ -15,6 +15,30 @@ widgetTest("basics", {
   }
 });
 
+widgetTest("notifications", {
+  template: '{{mount-widget widget="user-menu"}}',
+
+  test(assert) {
+    const $links = find(".notifications li a");
+
+    assert.equal($links.length, 2);
+    assert.ok($links[0].href.includes("/t/a-slug/123"));
+
+    assert.ok(
+      $links[1].href.includes(
+        "/u/eviltrout/notifications/likes-received?acting_username=aquaman"
+      )
+    );
+
+    assert.equal(
+      $links[1].text,
+      `aquaman ${I18n.t("notifications.liked_consolidated_description", {
+        count: 5
+      })}`
+    );
+  }
+});
+
 widgetTest("log out", {
   template: '{{mount-widget widget="user-menu" logout=(action "logout")}}',