Improving the original label-subscribing implementation

1. Make the "subscribed" text in Issuable sidebar reflect the labels
   subscription status

2. Current user mut be logged-in to toggle issue/MR/label subscription

2 files changed, 17 insertions, 18 deletions

diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb
index affc4a842a7..86ab84615ba 100644
--- a/app/models/concerns/issuable.rb
+++ b/app/models/concerns/issuable.rb
@@ -149,6 +149,10 @@ module Issuable
     notes.awards.where(note: "thumbsup").count
   end
 
+  def subscribed_without_subscriptions?(user)
+    participants(user).include?(user)
+  end
+
   def to_hook_data(user)
     hook_data = {
       object_kind: self.class.name.underscore,
@@ -179,12 +183,6 @@ module Issuable
     end
   end
 
-  # Labels that are currently applied to this object
-  # that are not present in `old_labels`
-  def added_labels(old_labels)
-    self.labels - old_labels
-  end
-
   # Convert this Issuable class name to a format usable by Ability definitions
   #
   # Examples:
diff --git a/app/models/concerns/subscribable.rb b/app/models/concerns/subscribable.rb
index cab9241ac3d..d5a881b2445 100644
--- a/app/models/concerns/subscribable.rb
+++ b/app/models/concerns/subscribable.rb
@@ -13,20 +13,21 @@ module Subscribable
   end
 
   def subscribed?(user)
-    subscription = subscriptions.find_by_user_id(user.id)
-
-    if subscription
-      return subscription.subscribed
+    if subscription = subscriptions.find_by_user_id(user.id)
+      subscription.subscribed
+    else
+      subscribed_without_subscriptions?(user)
     end
+  end
 
-    # FIXME
-    # Issue/MergeRequest has participants, but Label doesn't.
-    # Ideally, subscriptions should be separate from participations,
-    # but that seems like a larger change with farther-reaching
-    # consequences, so this is a compromise for the time being.
-    if respond_to?(:participants)
-      participants(user).include?(user)
-    end
+  # Override this method to define custom logic to consider a subscribable as
+  # subscribed without an explicit subscription record.
+  def subscribed_without_subscriptions?(user)
+    false
+  end
+
+  def subscribers
+    subscriptions.where(subscribed: true).map(&:user)
   end
 
   def toggle_subscription(user)