FIX: if user undos like, and relikes notify

FEATURE: roll up likes for notify on all likes

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

@@ -1,3 +1,4 @@
+const LIKED_TYPE = 5;
 const INVITED_TYPE = 8;
 const GROUP_SUMMARY_TYPE = 16;
 
@@ -80,7 +81,16 @@ export default Ember.Component.extend({
       const count = notification.get('data.inbox_count');
       const group_name = notification.get('data.group_name');
       text = I18n.t(this.get('scope'), {count, group_name});
-    } else {
+    } else if (notification.get('notification_type') === LIKED_TYPE && notification.get("data.count") > 1)  {
+      const count = notification.get('data.count') - 2;
+      const username2 = notification.get('data.username2');
+      if (count===0) {
+        text = I18n.t('notifications.liked_2', {description, username, username2});
+      } else {
+        text = I18n.t('notifications.liked_many', {description, username, username2, count});
+      }
+    }
+    else {
       text = I18n.t(this.get('scope'), {description, username});
     }
     text = Discourse.Emoji.unescape(text);

app/models/post_action.rb

@@ -147,10 +147,10 @@ SQL
       # so callback is called
       action.save
       action.add_moderator_post_if_needed(moderator, :agreed, delete_post)
-      @trigger_spam = true if action.post_action_type_id == PostActionType.types[:spam]
+      trigger_spam = true if action.post_action_type_id == PostActionType.types[:spam]
     end
 
-    DiscourseEvent.trigger(:confirmed_spam_post, post) if @trigger_spam
+    DiscourseEvent.trigger(:confirmed_spam_post, post) if trigger_spam
 
     update_flagged_posts_count
   end
@@ -272,6 +272,7 @@ SQL
       post_action.recover!
       action_attrs.each { |attr, val| post_action.send("#{attr}=", val) }
       post_action.save
+      PostAlertObserver.after_create_post_action(post_action)
     else
       post_action = create(where_attrs.merge(action_attrs))
       if post_action && post_action.errors.count == 0
@@ -480,7 +481,7 @@ SQL
     elsif PostActionType.auto_action_flag_types.include?(post_action_type) &&
           SiteSetting.flags_required_to_hide_post > 0
 
-      old_flags, new_flags = PostAction.flag_counts_for(post.id)
+      _old_flags, new_flags = PostAction.flag_counts_for(post.id)
 
       if new_flags >= SiteSetting.flags_required_to_hide_post
         hide_post!(post, post_action_type, guess_hide_reason(post))

app/models/post_alert_observer.rb

@@ -1,10 +1,14 @@
 class PostAlertObserver < ActiveRecord::Observer
   observe :post_action, :post_revision
 
-  def alerter
+  def self.alerter
     @alerter ||= PostAlerter.new
   end
 
+  def alerter
+    self.class.alerter
+  end
+
   # Dispatch to an after_save_#{class_name} method
   def after_save(model)
     method_name = callback_for('after_save', model)
@@ -17,13 +21,60 @@ class PostAlertObserver < ActiveRecord::Observer
     send(method_name, model) if respond_to?(method_name)
   end
 
+  def refresh_like_notification(post, read)
+    return unless post && post.user_id
+
+    usernames = post.post_actions.where(post_action_type_id: PostActionType.types[:like])
+      .joins(:user)
+      .order('post_actions.created_at desc')
+      .where('post_actions.created_at > ?', 1.day.ago)
+      .pluck(:username)
+
+    if usernames.length > 0
+      data = {
+        topic_title: post.topic.title,
+        username: usernames[0],
+        display_username: usernames[0],
+        username2: usernames[1],
+        count: usernames.length
+      }
+      Notification.create(
+        notification_type: Notification.types[:liked],
+        topic_id: post.topic_id,
+        post_number: post.post_number,
+        user_id: post.user_id,
+        read: read,
+        data: data.to_json
+      )
+    end
+  end
+
   def after_save_post_action(post_action)
     # We only care about deleting post actions for now
     return if post_action.deleted_at.blank?
-    Notification.where(post_action_id: post_action.id).each(&:destroy)
+
+    if post_action.post_action_type_id == PostActionType.types[:like] && post_action.post
+
+      read = true
+
+      Notification.where(
+        topic_id: post_action.post.topic_id,
+        user_id: post_action.post.user_id,
+        post_number: post_action.post.post_number,
+        notification_type: Notification.types[:liked]
+      ).each do |notification|
+        read = false unless notification.read
+        notification.destroy
+      end
+
+      refresh_like_notification(post_action.post, read)
+    else
+      # not using destroy_all cause we want stuff to trigger
+      Notification.where(post_action_id: post_action.id).each(&:destroy)
+    end
   end
 
-  def after_create_post_action(post_action)
+  def self.after_create_post_action(post_action)
     # We only notify on likes for now
     return unless post_action.is_like?
 
@@ -40,6 +91,10 @@ class PostAlertObserver < ActiveRecord::Observer
     )
   end
 
+  def after_create_post_action(post_action)
+    self.class.after_create_post_action(post_action)
+  end
+
   def after_create_post_revision(post_revision)
     post = post_revision.post
 

app/services/post_alerter.rb

@@ -251,6 +251,19 @@ class PostAlerter
 
     return if existing_notification && !should_notify_previous?(user, existing_notification, opts)
 
+    notification_data = {}
+
+    if  existing_notification &&
+        existing_notification.created_at > 1.day.ago &&
+        user.user_option.like_notification_frequency == UserOption.like_notification_frequency_type[:always]
+
+      data = existing_notification.data_hash
+      notification_data["username2"] = data["display_username"]
+      notification_data["count"] = (data["count"] || 1).to_i + 1
+      # don't use destroy so we don't trigger a notification count refresh
+      Notification.where(id: existing_notification.id).destroy_all
+    end
+
     collapsed = false
 
     if type == Notification.types[:replied] || type == Notification.types[:posted]
@@ -287,13 +300,13 @@ class PostAlerter
       end
     end
 
-    notification_data = {
+    notification_data.merge!({
       topic_title: topic_title,
       original_post_id: original_post.id,
       original_post_type: original_post.post_type,
       original_username: original_username,
       display_username: opts[:display_username] || post.user.username
-    }
+    })
 
     if group = opts[:group]
       notification_data[:group_id] = group.id

config/locales/client.en.yml

@@ -1014,6 +1014,10 @@ en:
       posted: "<i title='replied' class='fa fa-reply'></i><p><span>{{username}}</span> {{description}}</p>"
       edited: "<i title='edited' class='fa fa-pencil'></i><p><span>{{username}}</span> {{description}}</p>"
       liked: "<i title='liked' class='fa fa-heart'></i><p><span>{{username}}</span> {{description}}</p>"
+      liked_2: "<i title='liked' class='fa fa-heart'></i><p><span>{{username}}, {{username2}}</span> {{description}}</p>"
+      liked_many:
+        one: "<i title='liked' class='fa fa-heart'></i><p><span>{{username}}, {{username2}} and 1 other</span> {{description}}</p>"
+        other: "<i title='liked' class='fa fa-heart'></i><p><span>{{username}}, {{username2}} and {{count}} others</span> {{description}}</p>"
       private_message: "<i title='private message' class='fa fa-envelope-o'></i><p><span>{{username}}</span> {{description}}</p>"
       invited_to_private_message: "<i title='private message' class='fa fa-envelope-o'></i><p><span>{{username}}</span> {{description}}</p>"
       invited_to_topic: "<i title='invited to topic' class='fa fa-hand-o-right'></i><p><span>{{username}}</span> {{description}}</p>"

spec/models/post_alert_observer_spec.rb

@@ -21,11 +21,8 @@ describe PostAlertObserver do
     end
 
     context 'when removing a liked post' do
-      before do
-        PostAction.act(evil_trout, post, PostActionType.types[:like])
-      end
-
       it 'removes a notification' do
+        PostAction.act(evil_trout, post, PostActionType.types[:like])
         expect {
           PostAction.remove_act(evil_trout, post, PostActionType.types[:like])
         }.to change(Notification, :count).by(-1)

spec/services/post_alerter_spec.rb

@@ -50,12 +50,27 @@ describe PostAlerter do
   end
 
   context 'likes' do
+
+    it 'notifies on likes after an undo' do
+      ActiveRecord::Base.observers.enable :all
+
+      post = Fabricate(:post, raw: 'I love waffles')
+
+      PostAction.act(evil_trout, post, PostActionType.types[:like])
+      PostAction.remove_act(evil_trout, post, PostActionType.types[:like])
+      PostAction.act(evil_trout, post, PostActionType.types[:like])
+
+      expect(Notification.count(post_number: 1, topic_id: post.topic_id)).to eq(1)
+    end
+
     it 'notifies on likes correctly' do
       ActiveRecord::Base.observers.enable :all
 
       post = Fabricate(:post, raw: 'I love waffles')
+
       PostAction.act(evil_trout, post, PostActionType.types[:like])
-      PostAction.act(Fabricate(:admin), post, PostActionType.types[:like])
+      admin = Fabricate(:admin)
+      PostAction.act(admin, post, PostActionType.types[:like])
 
       # one like
       expect(Notification.count(post_number: 1, topic_id: post.topic_id)).to eq(1)
@@ -66,8 +81,28 @@ describe PostAlerter do
 
       admin2 = Fabricate(:admin)
       PostAction.act(admin2, post, PostActionType.types[:like])
-      # two likes one edit
+      expect(Notification.count(post_number: 1, topic_id: post.topic_id)).to eq(1)
-      expect(Notification.count(post_number: 1, topic_id: post.topic_id)).to eq(2)
+
+      # adds info to the notification
+      notification = Notification.find_by(post_number: 1,
+                                          topic_id: post.topic_id)
+
+
+      expect(notification.data_hash["count"].to_i).to eq(2)
+      expect(notification.data_hash["username2"]).to eq(evil_trout.username)
+
+      # this is a tricky thing ... removing a like should fix up the notifications
+      PostAction.remove_act(evil_trout, post, PostActionType.types[:like])
+
+      # rebuilds the missing notification
+      expect(Notification.count(post_number: 1, topic_id: post.topic_id)).to eq(1)
+      notification = Notification.find_by(post_number: 1,
+                                          topic_id: post.topic_id)
+
+      expect(notification.data_hash["count"]).to eq(2)
+      expect(notification.data_hash["username"]).to eq(admin2.username)
+      expect(notification.data_hash["username2"]).to eq(admin.username)
+
 
       post.user.user_option.update_columns(like_notification_frequency:
                                            UserOption.like_notification_frequency_type[:first_time_and_daily])
@@ -82,7 +117,7 @@ describe PostAlerter do
       end
 
       # first happend within the same day, no need to notify
-      expect(Notification.count(post_number: 1, topic_id: post.topic_id)).to eq(3)
+      expect(Notification.count(post_number: 1, topic_id: post.topic_id)).to eq(2)
 
     end
   end