Refactoring Subscribable concern to accept a project

3 files changed, 46 insertions, 38 deletions

diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb
index 664bb594aa9..ec9e7e5ae2b 100644
--- a/app/models/concerns/issuable.rb
+++ b/app/models/concerns/issuable.rb
@@ -215,7 +215,7 @@ module Issuable
     end
   end
 
-  def subscribed_without_subscriptions?(user)
+  def subscribed_without_subscriptions?(user, project)
     participants(user).include?(user)
   end
 
diff --git a/app/models/concerns/subscribable.rb b/app/models/concerns/subscribable.rb
index 083257f1005..2210a210f33 100644
--- a/app/models/concerns/subscribable.rb
+++ b/app/models/concerns/subscribable.rb
@@ -12,39 +12,45 @@ module Subscribable
     has_many :subscriptions, dependent: :destroy, as: :subscribable
   end
 
-  def subscribed?(user)
-    if subscription = subscriptions.find_by_user_id(user.id)
+  def subscribed?(user, to_project = nil)
+    if subscription = subscriptions.find_by(user: user, project: (to_project || project))
       subscription.subscribed
     else
-      subscribed_without_subscriptions?(user)
+      subscribed_without_subscriptions?(user, to_project)
     end
   end
 
   # Override this method to define custom logic to consider a subscribable as
   # subscribed without an explicit subscription record.
-  def subscribed_without_subscriptions?(user)
+  def subscribed_without_subscriptions?(user, to_project = nil)
     false
   end
 
-  def subscribers
-    subscriptions.where(subscribed: true).map(&:user)
+  def subscribers(to_project = nil)
+    subscriptions.where(project: (to_project || project), subscribed: true).map(&:user)
   end
 
-  def toggle_subscription(user)
-    subscriptions.
-      find_or_initialize_by(user_id: user.id).
-      update(subscribed: !subscribed?(user))
+  def toggle_subscription(user, to_project = nil)
+    subscribed = subscribed?(user, (to_project || project))
+
+    find_or_initialize_subscription(user, to_project).
+      update(subscribed: !subscribed)
   end
 
-  def subscribe(user)
-    subscriptions.
-      find_or_initialize_by(user_id: user.id).
+  def subscribe(user, to_project = nil)
+    find_or_initialize_subscription(user, to_project).
       update(subscribed: true)
   end
 
-  def unsubscribe(user)
-    subscriptions.
-      find_or_initialize_by(user_id: user.id).
+  def unsubscribe(user, to_project = nil)
+    find_or_initialize_subscription(user, to_project).
       update(subscribed: false)
   end
+
+  private
+
+  def find_or_initialize_subscription(user, to_project = nil)
+    subscriptions.
+      find_or_initialize_by(user_id: user.id, project_id: (to_project || project).id)
+  end
 end
diff --git a/spec/models/concerns/subscribable_spec.rb b/spec/models/concerns/subscribable_spec.rb
index b7fc5a92497..0a96cbbe166 100644
--- a/spec/models/concerns/subscribable_spec.rb
+++ b/spec/models/concerns/subscribable_spec.rb
@@ -1,67 +1,69 @@
 require 'spec_helper'
 
 describe Subscribable, 'Subscribable' do
-  let(:resource) { create(:issue) }
-  let(:user) { create(:user) }
+  let(:project)  { create(:empty_project) }
+  let(:resource) { create(:issue, project: project) }
+  let(:user)     { create(:user) }
 
   describe '#subscribed?' do
     it 'returns false when no subcription exists' do
-      expect(resource.subscribed?(user)).to be_falsey
+      expect(resource.subscribed?(user, project)).to be_falsey
     end
 
     it 'returns true when a subcription exists and subscribed is true' do
-      resource.subscriptions.create(user: user, subscribed: true)
+      resource.subscriptions.create(user: user, project: project, subscribed: true)
 
-      expect(resource.subscribed?(user)).to be_truthy
+      expect(resource.subscribed?(user, project)).to be_truthy
     end
 
     it 'returns false when a subcription exists and subscribed is false' do
-      resource.subscriptions.create(user: user, subscribed: false)
+      resource.subscriptions.create(user: user, project: project, subscribed: false)
 
-      expect(resource.subscribed?(user)).to be_falsey
+      expect(resource.subscribed?(user, project)).to be_falsey
     end
   end
+
   describe '#subscribers' do
     it 'returns [] when no subcribers exists' do
-      expect(resource.subscribers).to be_empty
+      expect(resource.subscribers(project)).to be_empty
     end
 
     it 'returns the subscribed users' do
-      resource.subscriptions.create(user: user, subscribed: true)
-      resource.subscriptions.create(user: create(:user), subscribed: false)
+      resource.subscriptions.create(user: user, project: project, subscribed: true)
+      resource.subscriptions.create(user: create(:user), project: project, subscribed: false)
 
-      expect(resource.subscribers).to eq [user]
+      expect(resource.subscribers(project)).to eq [user]
     end
   end
 
   describe '#toggle_subscription' do
     it 'toggles the current subscription state for the given user' do
-      expect(resource.subscribed?(user)).to be_falsey
+      expect(resource.subscribed?(user, project)).to be_falsey
 
-      resource.toggle_subscription(user)
+      resource.toggle_subscription(user, project)
 
-      expect(resource.subscribed?(user)).to be_truthy
+      expect(resource.subscribed?(user, project)).to be_truthy
     end
   end
 
   describe '#subscribe' do
     it 'subscribes the given user' do
-      expect(resource.subscribed?(user)).to be_falsey
+      expect(resource.subscribed?(user, project)).to be_falsey
 
-      resource.subscribe(user)
+      resource.subscribe(user, project)
 
-      expect(resource.subscribed?(user)).to be_truthy
+      expect(resource.subscribed?(user, project)).to be_truthy
     end
   end
 
   describe '#unsubscribe' do
     it 'unsubscribes the given current user' do
-      resource.subscriptions.create(user: user, subscribed: true)
-      expect(resource.subscribed?(user)).to be_truthy
+      resource.subscriptions.create(user: user, project: project, subscribed: true)
+      expect(resource.subscribed?(user, project)).to be_truthy
 
-      resource.unsubscribe(user)
+      resource.unsubscribe(user, project)
 
-      expect(resource.subscribed?(user)).to be_falsey
+      expect(resource.subscribed?(user, project)).to be_falsey
     end
   end
 end