Reduce number of supervisor subscriptions (#802)

Reduce number of supervisor subscriptions

Gemfile.lock

@@ -217,7 +217,7 @@ GEM
     net-ssh (5.0.2)
     netrc (0.11.0)
     newrelic_rpm (5.2.0.345)
-    nokogiri (1.10.3)
+    nokogiri (1.10.4)
       mini_portile2 (~> 2.4.0)
     notiffany (0.1.1)
       nenv (~> 0.1)

app/models/concerns/message_subscription.rb

@@ -16,7 +16,8 @@ def subscribe_users_to_message
     #   - Admin users processing the offer/order
     user_ids << obj.try(:created_by_id)
     user_ids << self.sender_id
-    user_ids += Subscription.where("#{klass}_id": obj.id).pluck(:user_id)
+    user_ids += public_subscribers_to(klass, obj.id)
+    user_ids += private_subscribers_to(klass, obj.id)
     admin_user_fields.each{|field| user_ids << obj.try(field)}
 
     # Remove the following users
@@ -26,8 +27,10 @@ def subscribe_users_to_message
     user_ids -= [User.system_user.try(:id), User.stockit_user.try(:id)]
     user_ids -= [obj.try(:created_by_id)] if self.is_private or obj.try('cancelled?')
 
-    # For private messages, subscribe all supervisors
-    user_ids += User.supervisors.pluck(:id) if self.is_private
+    # For private messages, subscribe all supervisors ONLY for the first message
+    if self.is_private && is_first_message_for(klass, obj.id)
+      user_ids += User.supervisors.pluck(:id)
+    end
 
     # If donor sends a message but no one else is listening, subscribe all reviewers.
     user_ids += User.reviewers.pluck(:id) if [self.sender_id] == user_ids
@@ -40,6 +43,29 @@ def subscribe_users_to_message
 
   private
 
+  # A public subscriber is defined as :
+  #   > Anyone who has a subscription to that record
+  def public_subscribers_to(klass, id)
+    Subscription
+      .joins(:message)
+      .where("#{klass}_id": id, messages: { is_private: false })
+      .pluck(:user_id)
+  end
+
+  # A private subscriber is defined as :
+  #   > A supervisor who has answered the private thread
+  def private_subscribers_to(klass, id)
+    User.supervisors
+        .joins(subscriptions: [:message])
+        .where(subscriptions: { "#{klass}_id": id })
+        .where(messages: { is_private: true })
+        .pluck(:id)
+  end
+
+  def is_first_message_for(klass, id)
+    Message.where(is_private: is_private, "#{klass}_id": id).count.eql? 1
+  end
+
   def admin_user_fields
     [:reviewed_by_id, :processed_by_id, :process_completed_by_id,  :cancelled_by_id, :process_completed_by,
       :dispatch_started_by, :closed_by, :submitted_by]

spec/models/concerns/message_subscription_spec.rb

@@ -29,7 +29,7 @@
     context "should subscribe users who have sent previous messages" do
       let(:user_id) { reviewer.id }
       before(:each) do
-        expect(Subscription).to receive_message_chain('where.pluck').and_return([user_id])
+        create :subscription, user_id: user_id, message: message, offer: message.offer
       end
       it do
         expect(message).to receive(:add_subscription).with('unread', user_id)
@@ -51,7 +51,7 @@
       let(:message) { create :message, sender: offer.created_by, offer: offer }
       it do
         expect(message.offer.reviewed_by_id).to eql(nil)
-        # expect(message).to receive(:add_subscription).with('read', message.offer.created_by_id)
+        expect(message).to receive(:add_subscription).with('read', message.offer.created_by_id)
         expect(message).to receive(:add_subscription).with('unread', reviewer.id)
         message.subscribe_users_to_message
       end
@@ -76,22 +76,87 @@
     end
 
     context "private messages" do
-      let(:message) { create :message, is_private: true, sender: reviewer }
-
+      let(:offer) { create :offer }
+      let(:message) { build :message, is_private: true, sender: reviewer, offer: offer }
+      let(:message2) { build :message, is_private: true, sender: reviewer, offer: offer }
+
       context "should not subscribe donor" do
         let(:user_id) { message.offer.created_by_id }
         it do
           expect(message).to_not receive(:add_subscription).with(anything, user_id)
-          message.subscribe_users_to_message
+          message.save
         end
       end
 
-      context "should subscribe all supervisors" do
+      context "should subscribe all supervisors if it's the first private message of the thread" do
         let!(:supervisor) { create :user, :supervisor }
+
+        before { User.current_user = supervisor }
+
         it do
           expect(message).to receive(:add_subscription).with('read', reviewer.id) # sender
           expect(message).to receive(:add_subscription).with('unread', supervisor.id) # unsubscribed supervisor
-          message.subscribe_users_to_message
+          message.save
+        end
+      end
+
+      context "should not subscribe other supervisors for subsequent messages" do
+        let!(:supervisor) { create :user, :supervisor }
+
+        before { User.current_user = supervisor }
+
+        it do
+          # The unrelated supervisor receives the first message of the thread
+          expect(message).to receive(:add_subscription).with('read', reviewer.id)
+          expect(message).to receive(:add_subscription).with('unread', supervisor.id)
+          message.save
+
+          # The unrelated supervisor doesn't receive subsequent message of the thread
+          expect(message2).to receive(:add_subscription).with('read', reviewer.id) # sender
+          expect(message2).not_to receive(:add_subscription).with('unread', supervisor.id)
+          message2.save
+        end
+      end
+
+      context "should subscribe a supervisor for subsequent messages if he/she posted something in the private thread" do
+        let!(:supervisor) { create :user, :supervisor }
+
+        before { User.current_user = supervisor }
+
+        it do
+          # The unrelated supervisor receives the first message of the thread
+          expect(message).to receive(:add_subscription).with('read', reviewer.id)
+          expect(message).to receive(:add_subscription).with('unread', supervisor.id)
+          message.save
+
+          # The supervisor answers on the the private thread
+          create :message, sender: supervisor, offer: message.offer, is_private: true
+
+          # The supervisor receives subsequent message of the thread
+          expect(message2).to receive(:add_subscription).with('read', reviewer.id) # sender
+          expect(message2).to receive(:add_subscription).with('unread', supervisor.id)
+          message2.save
+        end
+      end
+
+      context "should subscribe a supervisor for subsequent messages if he/she posted something in the public thread" do
+        let!(:supervisor) { create :user, :supervisor }
+
+        before { User.current_user = supervisor }
+
+        it do
+          # The unrelated supervisor receives the first message of the thread
+          expect(message).to receive(:add_subscription).with('read', reviewer.id)
+          expect(message).to receive(:add_subscription).with('unread', supervisor.id)
+          message.save
+
+          # The supervisor answers on the the private thread
+          create :message, sender: supervisor, offer: message.offer, is_private: false
+
+          # The supervisor receives subsequent message of the thread
+          expect(message2).to receive(:add_subscription).with('read', reviewer.id) # sender
+          expect(message2).to receive(:add_subscription).with('unread', supervisor.id)
+          message2.save
         end
       end