Merge branch 'feature/subscribe-to-group-level-labels' into 'master'

Support subscribing to group labels

https://gitlab.com/gitlab-org/gitlab-ce/issues/23586

See merge request !7215

app/assets/javascripts/group_label_subscription.js.es6

+/* eslint-disable */
+(function(global) {
+  class GroupLabelSubscription {
+    constructor(container) {
+      const $container = $(container);
+      this.$dropdown = $container.find('.dropdown');
+      this.$subscribeButtons = $container.find('.js-subscribe-button');
+      this.$unsubscribeButtons = $container.find('.js-unsubscribe-button');
+
+      this.$subscribeButtons.on('click', this.subscribe.bind(this));
+      this.$unsubscribeButtons.on('click', this.unsubscribe.bind(this));
+    }
+
+    unsubscribe(event) {
+      event.preventDefault();
+
+      const url = this.$unsubscribeButtons.attr('data-url');
+
+      $.ajax({
+        type: 'POST',
+        url: url
+      }).done(() => {
+        this.toggleSubscriptionButtons();
+        this.$unsubscribeButtons.removeAttr('data-url');
+      });
+    }
+
+    subscribe(event) {
+      event.preventDefault();
+
+      const $btn = $(event.currentTarget);
+      const url = $btn.attr('data-url');
+
+      this.$unsubscribeButtons.attr('data-url', url);
+
+      $.ajax({
+        type: 'POST',
+        url: url
+      }).done(() => {
+        this.toggleSubscriptionButtons();
+      });
+    }
+
+    toggleSubscriptionButtons() {
+      this.$dropdown.toggleClass('hidden');
+      this.$subscribeButtons.toggleClass('hidden');
+      this.$unsubscribeButtons.toggleClass('hidden');
+    }
+  }
+
+  global.GroupLabelSubscription = GroupLabelSubscription;
+
+})(window.gl || (window.gl = {}));

app/assets/javascripts/project_label_subscription.js.es6

+/* eslint-disable */
+(function(global) {
+  class ProjectLabelSubscription {
+    constructor(container) {
+      this.$container = $(container);
+      this.$buttons = this.$container.find('.js-subscribe-button');
+
+      this.$buttons.on('click', this.toggleSubscription.bind(this));
+    }
+
+    toggleSubscription(event) {
+      event.preventDefault();
+
+      const $btn = $(event.currentTarget);
+      const $span = $btn.find('span');
+      const url = $btn.attr('data-url');
+      const oldStatus = $btn.attr('data-status');
+
+      $btn.addClass('disabled');
+      $span.toggleClass('hidden');
+
+      $.ajax({
+        type: 'POST',
+        url: url
+      }).done(() => {
+        let newStatus, newAction;
+
+        if (oldStatus === 'unsubscribed') {
+          [newStatus, newAction] = ['subscribed', 'Unsubscribe'];
+        } else {
+          [newStatus, newAction] = ['unsubscribed', 'Subscribe'];
+        }
+
+        $span.toggleClass('hidden');
+        $btn.removeClass('disabled');
+
+        this.$buttons.attr('data-status', newStatus);
+        this.$buttons.find('> span').text(newAction);
+
+        for (let button of this.$buttons) {
+          let $button = $(button);
+
+          if ($button.attr('data-original-title')) {
+            $button.tooltip('hide').attr('data-original-title', newAction).tooltip('fixTitle');
+          }
+        }
+      });
+    }
+  }
+
+  global.ProjectLabelSubscription = ProjectLabelSubscription;
+
+})(window.gl || (window.gl = {}));

app/assets/stylesheets/pages/labels.scss

@@ -90,7 +90,7 @@
 
     @media (min-width: $screen-sm-min) {
       display: inline-block;
-      width: 40%;
+      width: 30%;
       margin-left: 10px;
       margin-bottom: 0;
       vertical-align: middle;
@@ -222,6 +222,14 @@
   width: 100%;
 }
 
+.label-subscription {
+  vertical-align: middle;
+
+  .dropdown-group-label a {
+    cursor: pointer;
+  }
+}
+
 .label-subscribe-button {
   .label-subscribe-button-icon {
     &[disabled] {

app/controllers/concerns/toggle_subscription_action.rb

@@ -4,13 +4,17 @@ module ToggleSubscriptionAction
   def toggle_subscription
     return unless current_user
 
-    subscribable_resource.toggle_subscription(current_user)
+    subscribable_resource.toggle_subscription(current_user, subscribable_project)
 
     head :ok
   end
 
   private
 
+  def subscribable_project
+    @project || raise(NotImplementedError)
+  end
+
   def subscribable_resource
     raise NotImplementedError
   end

app/controllers/groups/labels_controller.rb

 class Groups::LabelsController < Groups::ApplicationController
+  include ToggleSubscriptionAction
+
   before_action :label, only: [:edit, :update, :destroy]
   before_action :authorize_admin_labels!, only: [:new, :create, :edit, :update, :destroy]
   before_action :save_previous_label_path, only: [:edit]
@@ -69,6 +71,11 @@ class Groups::LabelsController < Groups::ApplicationController
   def label
     @label ||= @group.labels.find(params[:id])
   end
+  alias_method :subscribable_resource, :label
+
+  def subscribable_project
+    nil
+  end
 
   def label_params
     params.require(:label).permit(:title, :description, :color)

app/controllers/projects/labels_controller.rb

@@ -3,7 +3,7 @@ class Projects::LabelsController < Projects::ApplicationController
 
   before_action :module_enabled
   before_action :label, only: [:edit, :update, :destroy]
-  before_action :find_labels, only: [:index, :set_priorities, :remove_priority]
+  before_action :find_labels, only: [:index, :set_priorities, :remove_priority, :toggle_subscription]
   before_action :authorize_read_label!
   before_action :authorize_admin_labels!, only: [:new, :create, :edit, :update,
                                                  :generate, :destroy, :remove_priority,
@@ -123,7 +123,10 @@ class Projects::LabelsController < Projects::ApplicationController
   def label
     @label ||= @project.labels.find(params[:id])
   end
-  alias_method :subscribable_resource, :label
+
+  def subscribable_resource
+    @available_labels.find(params[:id])
+  end
 
   def find_labels
     @available_labels ||= LabelsFinder.new(current_user, project_id: @project.id).execute

app/controllers/sent_notifications_controller.rb

@@ -12,7 +12,7 @@ class SentNotificationsController < ApplicationController
 
   def unsubscribe_and_redirect
     noteable = @sent_notification.noteable
-    noteable.unsubscribe(@sent_notification.recipient)
+    noteable.unsubscribe(@sent_notification.recipient, @sent_notification.project)
 
     flash[:notice] = "You have been unsubscribed from this thread."
 

app/helpers/labels_helper.rb

@@ -68,14 +68,6 @@ module LabelsHelper
     end
   end
 
-  def toggle_subscription_data(label)
-    return unless label.is_a?(ProjectLabel)
-
-    {
-      url: toggle_subscription_namespace_project_label_path(label.project.namespace, label.project, label)
-    }
-  end
-
   def render_colored_label(label, label_suffix = '', tooltip: true)
     label_color = label.color || Label::DEFAULT_COLOR
     text_color = text_color_for_bg(label_color)
@@ -148,20 +140,24 @@ module LabelsHelper
     end
   end
 
-  def label_subscription_status(label)
-    case label
-    when GroupLabel then 'Subscribing to group labels is currently not supported.'
-    when ProjectLabel then label.subscribed?(current_user) ? 'subscribed' : 'unsubscribed'
-    end
+  def label_subscription_status(label, project)
+    return 'project-level' if label.subscribed?(current_user, project)
+    return 'group-level' if label.subscribed?(current_user)
+
+    'unsubscribed'
   end
 
-  def label_subscription_toggle_button_text(label)
-    case label
-    when GroupLabel then 'Subscribing to group labels is currently not supported.'
-    when ProjectLabel then label.subscribed?(current_user) ? 'Unsubscribe' : 'Subscribe'
+  def group_label_unsubscribe_path(label, project)
+    case label_subscription_status(label, project)
+    when 'project-level' then toggle_subscription_namespace_project_label_path(@project.namespace, @project, label)
+    when 'group-level' then toggle_subscription_group_label_path(label.group, label)
     end
   end
 
+  def label_subscription_toggle_button_text(label, project)
+    label.subscribed?(current_user, project) ? 'Unsubscribe' : 'Subscribe'
+  end
+
   def label_deletion_confirm_text(label)
     case label
     when GroupLabel then 'Remove this label? This will affect all projects within the group. Are you sure?'

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
 

app/models/concerns/subscribable.rb

@@ -12,39 +12,71 @@ 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, project = nil)
+    if subscription = subscriptions.find_by(user: user, project: project)
       subscription.subscribed
     else
-      subscribed_without_subscriptions?(user)
+      subscribed_without_subscriptions?(user, 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, project)
     false
   end
 
-  def subscribers
-    subscriptions.where(subscribed: true).map(&:user)
+  def subscribers(project)
+    subscriptions_available(project).
+      where(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, project = nil)
+    unsubscribe_from_other_levels(user, project)
+
+    find_or_initialize_subscription(user, project).
+      update(subscribed: !subscribed?(user, project))
+  end
+
+  def subscribe(user, project = nil)
+    unsubscribe_from_other_levels(user, project)
+
+    find_or_initialize_subscription(user, project)
+      .update(subscribed: true)
+  end
+
+  def unsubscribe(user, project = nil)
+    unsubscribe_from_other_levels(user, project)
+
+    find_or_initialize_subscription(user, project)
+      .update(subscribed: false)
   end
 
-  def subscribe(user)
+  private
+
+  def unsubscribe_from_other_levels(user, project)
+    other_subscriptions = subscriptions.where(user: user)
+
+    other_subscriptions =
+      if project.blank?
+        other_subscriptions.where.not(project: nil)
+      else
+        other_subscriptions.where(project: nil)
+      end
+
+    other_subscriptions.update_all(subscribed: false)
+  end
+
+  def find_or_initialize_subscription(user, project)
     subscriptions.
-      find_or_initialize_by(user_id: user.id).
-      update(subscribed: true)
+      find_or_initialize_by(user_id: user.id, project_id: project.try(:id))
   end
 
-  def unsubscribe(user)
+  def subscriptions_available(project)
+    t = Subscription.arel_table
+
     subscriptions.
-      find_or_initialize_by(user_id: user.id).
-      update(subscribed: false)
+      where(t[:project_id].eq(nil).or(t[:project_id].eq(project.try(:id))))
   end
 end

app/models/issue.rb

@@ -266,7 +266,7 @@ class Issue < ActiveRecord::Base
 
   def as_json(options = {})
     super(options).tap do |json|
-      json[:subscribed] = subscribed?(options[:user]) if options.has_key?(:user) && options[:user]
+      json[:subscribed] = subscribed?(options[:user], project) if options.has_key?(:user) && options[:user]
 
       if options.has_key?(:labels)
         json[:labels] = labels.as_json(

app/models/subscription.rb

 class Subscription < ActiveRecord::Base
   belongs_to :user
+  belongs_to :project
   belongs_to :subscribable, polymorphic: true
 
-  validates :user_id,
-            uniqueness: { scope: [:subscribable_id, :subscribable_type] },
-            presence: true
+  validates :user, :subscribable, presence: true
+
+  validates :project_id, uniqueness: { scope: [:subscribable_id, :subscribable_type, :user_id] }
 end

app/services/issuable_base_service.rb

@@ -212,9 +212,9 @@ class IssuableBaseService < BaseService
   def change_subscription(issuable)
     case params.delete(:subscription_event)
     when 'subscribe'
-      issuable.subscribe(current_user)
+      issuable.subscribe(current_user, project)
     when 'unsubscribe'
-      issuable.unsubscribe(current_user)
+      issuable.unsubscribe(current_user, project)
     end
   end
 

app/services/notification_service.rb

@@ -75,7 +75,7 @@ class NotificationService
   #  * watchers of the issue's labels
   #
   def relabeled_issue(issue, added_labels, current_user)
-    relabeled_resource_email(issue, added_labels, current_user, :relabeled_issue_email)
+    relabeled_resource_email(issue, issue.project, added_labels, current_user, :relabeled_issue_email)
   end
 
   # When create a merge request we should send an email to:
@@ -118,7 +118,7 @@ class NotificationService
   #  * watchers of the mr's labels
   #
   def relabeled_merge_request(merge_request, added_labels, current_user)
-    relabeled_resource_email(merge_request, added_labels, current_user, :relabeled_merge_request_email)
+    relabeled_resource_email(merge_request, merge_request.target_project, added_labels, current_user, :relabeled_merge_request_email)
   end
 
   def close_mr(merge_request, current_user)
@@ -205,7 +205,7 @@ class NotificationService
 
     recipients = reject_muted_users(recipients, note.project)
 
-    recipients = add_subscribed_users(recipients, note.noteable)
+    recipients = add_subscribed_users(recipients, note.project, note.noteable)
     recipients = reject_unsubscribed_users(recipients, note.noteable)
     recipients = reject_users_without_access(recipients, note.noteable)
 
@@ -393,7 +393,7 @@ class NotificationService
     )
   end
 
-  # Build a list of users based on project notifcation settings
+  # Build a list of users based on project notification settings
   def select_project_member_setting(project, global_setting, users_global_level_watch)
     users = notification_settings_for(project, :watch)
 
@@ -505,17 +505,17 @@ class NotificationService
     end
   end
 
-  def add_subscribed_users(recipients, target)
+  def add_subscribed_users(recipients, project, target)
     return recipients unless target.respond_to? :subscribers
 
-    recipients + target.subscribers
+    recipients + target.subscribers(project)
   end
 
-  def add_labels_subscribers(recipients, target, labels: nil)
+  def add_labels_subscribers(recipients, project, target, labels: nil)
     return recipients unless target.respond_to? :labels
 
     (labels || target.labels).each do |label|
-      recipients += label.subscribers
+      recipients += label.subscribers(project)
     end
 
     recipients
@@ -571,8 +571,8 @@ class NotificationService
     end
   end
 
-  def relabeled_resource_email(target, labels, current_user, method)
-    recipients = build_relabeled_recipients(target, current_user, labels: labels)
+  def relabeled_resource_email(target, project, labels, current_user, method)
+    recipients = build_relabeled_recipients(target, project, current_user, labels: labels)
     label_names = labels.map(&:name)
 
     recipients.each do |recipient|
@@ -608,10 +608,10 @@ class NotificationService
     end
 
     recipients = reject_muted_users(recipients, project)
-    recipients = add_subscribed_users(recipients, target)
+    recipients = add_subscribed_users(recipients, project, target)
 
     if [:new_issue, :new_merge_request].include?(custom_action)
-      recipients = add_labels_subscribers(recipients, target)
+      recipients = add_labels_subscribers(recipients, project, target)
     end
 
     recipients = reject_unsubscribed_users(recipients, target)
@@ -622,8 +622,8 @@ class NotificationService
     recipients.uniq
   end
 
-  def build_relabeled_recipients(target, current_user, labels:)
-    recipients = add_labels_subscribers([], target, labels: labels)
+  def build_relabeled_recipients(target, project, current_user, labels:)
+    recipients = add_labels_subscribers([], project, target, labels: labels)
     recipients = reject_unsubscribed_users(recipients, target)
     recipients = reject_users_without_access(recipients, target)
     recipients.delete(current_user)

app/services/slash_commands/interpret_service.rb

@@ -193,7 +193,7 @@ module SlashCommands
     desc 'Subscribe'
     condition do
       issuable.persisted? &&
-        !issuable.subscribed?(current_user)
+        !issuable.subscribed?(current_user, project)
     end
     command :subscribe do
       @updates[:subscription_event] = 'subscribe'
@@ -202,7 +202,7 @@ module SlashCommands
     desc 'Unsubscribe'
     condition do
       issuable.persisted? &&
-        issuable.subscribed?(current_user)
+        issuable.subscribed?(current_user, project)
     end
     command :unsubscribe do
       @updates[:subscription_event] = 'unsubscribe'

app/views/shared/_label.html.haml

 - label_css_id = dom_id(label)
 - open_issues_count = label.open_issues_count(current_user)
 - open_merge_requests_count = label.open_merge_requests_count(current_user)
+- status = label_subscription_status(label, @project).inquiry if current_user
 - subject = local_assigns[:subject]
 
 %li{id: label_css_id, data: { id: label.id } }
@@ -18,10 +19,19 @@
         %li
           = link_to_label(label, subject: subject) do
             = pluralize open_issues_count, 'open issue'
-        - if current_user
-          %li.label-subscription{ data: toggle_subscription_data(label) }
-            %a.js-subscribe-button.label-subscribe-button.subscription-status{ role: "button", href: "#", data: { toggle: "tooltip", status: label_subscription_status(label) } }
-              %span= label_subscription_toggle_button_text(label)
+        - if current_user && defined?(@project)
+          %li.label-subscription
+            - if label.is_a?(ProjectLabel)
+              %a.js-subscribe-button.label-subscribe-button{ role: 'button', href: '#', data: { status: status, url: toggle_subscription_namespace_project_label_path(@project.namespace, @project, label) } }
+                %span= label_subscription_toggle_button_text(label, @project)
+            - else
+              %a.js-unsubscribe-button.label-subscribe-button{ role: 'button', href: '#', class: ('hidden' if status.unsubscribed?), data: { url: group_label_unsubscribe_path(label, @project) } }
+                %span Unsubscribe
+              %a.js-subscribe-button.label-subscribe-button{ role: 'button', href: '#', class: ('hidden' unless status.unsubscribed?), data: { url: toggle_subscription_namespace_project_label_path(@project.namespace, @project, label) } }
+                %span Subscribe at project level
+              %a.js-subscribe-button.label-subscribe-button{ role: 'button', href: '#', class: ('hidden' unless status.unsubscribed?), data: { url: toggle_subscription_group_label_path(label.group, label) } }
+                %span Subscribe at group level
+
         - if can?(current_user, :admin_label, label)
           %li
             = link_to 'Edit', edit_label_path(label)
@@ -34,12 +44,27 @@
     = link_to_label(label, subject: subject, css_class: 'btn btn-transparent btn-action') do
       = pluralize open_issues_count, 'open issue'
 
-    - if current_user
-      .label-subscription.inline{ data: toggle_subscription_data(label) }
-        %button.js-subscribe-button.label-subscribe-button.btn.btn-transparent.btn-action.subscription-status{ type: "button", title: label_subscription_toggle_button_text(label), data: { toggle: "tooltip", status: label_subscription_status(label) } }
-          %span.sr-only= label_subscription_toggle_button_text(label)
-          = icon('eye', class: 'label-subscribe-button-icon', disabled: label.is_a?(GroupLabel))
-          = icon('spinner spin', class: 'label-subscribe-button-loading')
+    - if current_user && defined?(@project)
+      .label-subscription.inline
+        - if label.is_a?(ProjectLabel)
+          %button.js-subscribe-button.label-subscribe-button.btn.btn-default.btn-action{ type: 'button', title: label_subscription_toggle_button_text(label, @project), data: { toggle: 'tooltip', status: status, url: toggle_subscription_namespace_project_label_path(@project.namespace, @project, label) } }
+            %span= label_subscription_toggle_button_text(label, @project)
+            = icon('spinner spin', class: 'label-subscribe-button-loading')
+        - else
+          %button.js-unsubscribe-button.label-subscribe-button.btn.btn-default.btn-action{ type: 'button', class: ('hidden' if status.unsubscribed?), title: 'Unsubscribe', data: { toggle: 'tooltip', url: group_label_unsubscribe_path(label, @project) } }
+            %span Unsubscribe
+            = icon('spinner spin', class: 'label-subscribe-button-loading')
+
+          .dropdown.dropdown-group-label{ class: ('hidden' unless status.unsubscribed?) }
+            %button.dropdown-menu-toggle{type: 'button', 'data-toggle' => 'dropdown'}
+              %span Subscribe
+              = icon('chevron-down')
+            %ul.dropdown-menu
+              %li
+                %a.js-subscribe-button{ data: { url: toggle_subscription_namespace_project_label_path(@project.namespace, @project, label) } }
+                  Project level
+                %a.js-subscribe-button{ data: { url: toggle_subscription_group_label_path(label.group, label) } }
+                  Group level
 
     - if can?(current_user, :admin_label, label)
       = link_to edit_label_path(label), title: "Edit", class: 'btn btn-transparent btn-action', data: {toggle: "tooltip"} do
@@ -49,6 +74,10 @@
         %span.sr-only Delete
         = icon('trash-o')
 
-  - if current_user && label.is_a?(ProjectLabel)
-    :javascript
-      new Subscription('##{dom_id(label)} .label-subscription');
+  - if current_user && defined?(@project)
+    - if label.is_a?(ProjectLabel)
+      :javascript
+        new gl.ProjectLabelSubscription('##{dom_id(label)} .label-subscription');
+    - else
+      :javascript
+        new gl.GroupLabelSubscription('##{dom_id(label)} .label-subscription');

app/views/shared/issuable/_sidebar.html.haml

@@ -140,7 +140,7 @@
 
       = render "shared/issuable/participants", participants: issuable.participants(current_user)
       - if current_user
-        - subscribed = issuable.subscribed?(current_user)
+        - subscribed = issuable.subscribed?(current_user, @project)
         .block.light.subscription{data: {url: toggle_subscription_path(issuable)}}
           .sidebar-collapsed-icon
             = icon('rss')

changelogs/unreleased/feature-subscribe-to-group-level-labels.yml

+---
+title: Allow users to subscribe to group labels
+merge_request: 7215
+author: 

config/routes/group.rb

@@ -30,7 +30,10 @@ scope(path: 'groups/:group_id', module: :groups, as: :group) do
 
   resource :avatar, only: [:destroy]
   resources :milestones, constraints: { id: /[^\/]+/ }, only: [:index, :show, :update, :new, :create]
-  resources :labels, except: [:show], constraints: { id: /\d+/ }
+
+  resources :labels, except: [:show], constraints: { id: /\d+/ } do
+    post :toggle_subscription, on: :member
+  end
 end
 
 # Must be last route in this file

db/migrate/20161031171301_add_project_id_to_subscriptions.rb

+class AddProjectIdToSubscriptions < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def up
+    add_column :subscriptions, :project_id, :integer
+    add_foreign_key :subscriptions, :projects, column: :project_id, on_delete: :cascade
+  end
+
+  def down
+    remove_column :subscriptions, :project_id
+  end
+end

db/migrate/20161031174110_migrate_subscriptions_project_id.rb

+class MigrateSubscriptionsProjectId < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = true
+  DOWNTIME_REASON = 'Subscriptions will not work as expected until this migration is complete.'
+
+  def up
+    execute <<-EOF.strip_heredoc
+      UPDATE subscriptions
+      SET project_id = (
+        SELECT issues.project_id
+        FROM issues
+        WHERE issues.id = subscriptions.subscribable_id
+      )
+      WHERE subscriptions.subscribable_type = 'Issue';
+    EOF
+
+    execute <<-EOF.strip_heredoc
+      UPDATE subscriptions
+      SET project_id = (
+        SELECT merge_requests.target_project_id
+        FROM merge_requests
+        WHERE merge_requests.id = subscriptions.subscribable_id
+      )
+      WHERE subscriptions.subscribable_type = 'MergeRequest';
+    EOF
+
+    execute <<-EOF.strip_heredoc
+      UPDATE subscriptions
+      SET project_id = (
+        SELECT projects.id
+        FROM labels INNER JOIN projects ON projects.id = labels.project_id
+        WHERE labels.id = subscriptions.subscribable_id
+      )
+      WHERE subscriptions.subscribable_type = 'Label';
+    EOF
+  end
+
+  def down
+    execute <<-EOF.strip_heredoc
+      UPDATE subscriptions SET project_id = NULL;
+    EOF
+  end
+end

db/migrate/20161031181638_add_unique_index_to_subscriptions.rb

+class AddUniqueIndexToSubscriptions < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = true
+  DOWNTIME_REASON = 'This migration requires downtime because it changes a column to not accept null values.'
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index :subscriptions, [:subscribable_id, :subscribable_type, :user_id, :project_id], { unique: true, name: 'index_subscriptions_on_subscribable_and_user_id_and_project_id' }
+    remove_index :subscriptions, name: 'subscriptions_user_id_and_ref_fields' if index_name_exists?(:subscriptions, 'subscriptions_user_id_and_ref_fields', false)
+  end
+
+  def down
+    add_concurrent_index :subscriptions, [:subscribable_id, :subscribable_type, :user_id], { unique: true, name: 'subscriptions_user_id_and_ref_fields' }
+    remove_index :subscriptions, name: 'index_subscriptions_on_subscribable_and_user_id_and_project_id' if index_name_exists?(:subscriptions, 'index_subscriptions_on_subscribable_and_user_id_and_project_id', false)
+  end
+end

db/schema.rb

@@ -1067,9 +1067,10 @@ ActiveRecord::Schema.define(version: 20161113184239) do
     t.boolean "subscribed"
     t.datetime "created_at"
     t.datetime "updated_at"
+    t.integer "project_id"
   end
 
-  add_index "subscriptions", ["subscribable_id", "subscribable_type", "user_id"], name: "subscriptions_user_id_and_ref_fields", unique: true, using: :btree
+  add_index "subscriptions", ["subscribable_id", "subscribable_type", "user_id", "project_id"], name: "index_subscriptions_on_subscribable_and_user_id_and_project_id", unique: true, using: :btree
 
   create_table "taggings", force: :cascade do |t|
     t.integer "tag_id"
@@ -1265,6 +1266,7 @@ ActiveRecord::Schema.define(version: 20161113184239) do
   add_foreign_key "personal_access_tokens", "users"
   add_foreign_key "protected_branch_merge_access_levels", "protected_branches"
   add_foreign_key "protected_branch_push_access_levels", "protected_branches"
+  add_foreign_key "subscriptions", "projects", on_delete: :cascade
   add_foreign_key "trending_projects", "projects", on_delete: :cascade
   add_foreign_key "u2f_registrations", "users"
 end

lib/api/entities.rb

@@ -218,7 +218,7 @@ module API
       expose :assignee, :author, using: Entities::UserBasic
 
       expose :subscribed do |issue, options|
-        issue.subscribed?(options[:current_user])
+        issue.subscribed?(options[:current_user], options[:project] || issue.project)
       end
       expose :user_notes_count
       expose :upvotes, :downvotes
@@ -248,7 +248,7 @@ module API
       expose :diff_head_sha, as: :sha
       expose :merge_commit_sha
       expose :subscribed do |merge_request, options|
-        merge_request.subscribed?(options[:current_user])
+        merge_request.subscribed?(options[:current_user], options[:project])
       end
       expose :user_notes_count
       expose :should_remove_source_branch?, as: :should_remove_source_branch
@@ -454,7 +454,7 @@ module API
       end
 
       expose :subscribed do |label, options|
-        label.subscribed?(options[:current_user])
+        label.subscribed?(options[:current_user], options[:project])
       end
     end
 

lib/api/issues.rb

@@ -120,7 +120,7 @@ module API
 
         issues = issues.reorder(issuable_order_by => issuable_sort)
 
-        present paginate(issues), with: Entities::Issue, current_user: current_user
+        present paginate(issues), with: Entities::Issue, current_user: current_user, project: user_project
       end
 
       # Get a single project issue
@@ -132,7 +132,7 @@ module API
       #   GET /projects/:id/issues/:issue_id
       get ":id/issues/:issue_id" do
         @issue = find_project_issue(params[:issue_id])
-        present @issue, with: Entities::Issue, current_user: current_user
+        present @issue, with: Entities::Issue, current_user: current_user, project: user_project
       end
 
       # Create a new project issue
@@ -174,7 +174,7 @@ module API
         end
 
         if issue.valid?
-          present issue, with: Entities::Issue, current_user: current_user
+          present issue, with: Entities::Issue, current_user: current_user, project: user_project
         else
           render_validation_error!(issue)
         end
@@ -217,7 +217,7 @@ module API
         issue = ::Issues::UpdateService.new(user_project, current_user, attrs).execute(issue)
 
         if issue.valid?
-          present issue, with: Entities::Issue, current_user: current_user
+          present issue, with: Entities::Issue, current_user: current_user, project: user_project
         else
           render_validation_error!(issue)
         end
@@ -239,7 +239,7 @@ module API
 
         begin
           issue = ::Issues::MoveService.new(user_project, current_user).execute(issue, new_project)
-          present issue, with: Entities::Issue, current_user: current_user
+          present issue, with: Entities::Issue, current_user: current_user, project: user_project
         rescue ::Issues::MoveService::MoveError => error
           render_api_error!(error.message, 400)
         end

lib/api/merge_requests.rb

@@ -60,7 +60,7 @@ module API
           end
 
         merge_requests = merge_requests.reorder(issuable_order_by => issuable_sort)
-        present paginate(merge_requests), with: Entities::MergeRequest, current_user: current_user
+        present paginate(merge_requests), with: Entities::MergeRequest, current_user: current_user, project: user_project
       end
 
       desc 'Create a merge request' do
@@ -87,7 +87,7 @@ module API
         merge_request = ::MergeRequests::CreateService.new(user_project, current_user, mr_params).execute
 
         if merge_request.valid?
-          present merge_request, with: Entities::MergeRequest, current_user: current_user
+          present merge_request, with: Entities::MergeRequest, current_user: current_user, project: user_project
         else
           handle_merge_request_errors! merge_request.errors
         end
@@ -120,7 +120,7 @@ module API
         get path do
           merge_request = user_project.merge_requests.find(params[:merge_request_id])
           authorize! :read_merge_request, merge_request
-          present merge_request, with: Entities::MergeRequest, current_user: current_user
+          present merge_request, with: Entities::MergeRequest, current_user: current_user, project: user_project
         end
 
         desc 'Get the commits of a merge request' do
@@ -167,7 +167,7 @@ module API
           merge_request = ::MergeRequests::UpdateService.new(user_project, current_user, mr_params).execute(merge_request)
 
           if merge_request.valid?
-            present merge_request, with: Entities::MergeRequest, current_user: current_user
+            present merge_request, with: Entities::MergeRequest, current_user: current_user, project: user_project
           else
             handle_merge_request_errors! merge_request.errors
           end
@@ -212,7 +212,7 @@ module API
               execute(merge_request)
           end
 
-          present merge_request, with: Entities::MergeRequest, current_user: current_user
+          present merge_request, with: Entities::MergeRequest, current_user: current_user, project: user_project
         end
 
         desc 'Cancel merge if "Merge when build succeeds" is enabled' do

lib/api/milestones.rb

@@ -114,7 +114,7 @@ module API
         }
 
         issues = IssuesFinder.new(current_user, finder_params).execute
-        present paginate(issues), with: Entities::Issue, current_user: current_user
+        present paginate(issues), with: Entities::Issue, current_user: current_user, project: user_project
       end
     end
   end

lib/api/subscriptions.rb

@@ -24,11 +24,11 @@ module API
         post ":id/#{type}/:subscribable_id/subscription" do
           resource = instance_exec(params[:subscribable_id], &finder)
 
-          if resource.subscribed?(current_user)
+          if resource.subscribed?(current_user, user_project)
             not_modified!
           else
-            resource.subscribe(current_user)
-            present resource, with: entity_class, current_user: current_user
+            resource.subscribe(current_user, user_project)
+            present resource, with: entity_class, current_user: current_user, project: user_project
           end
         end
 
@@ -38,11 +38,11 @@ module API
         delete ":id/#{type}/:subscribable_id/subscription" do
           resource = instance_exec(params[:subscribable_id], &finder)
 
-          if !resource.subscribed?(current_user)
+          if !resource.subscribed?(current_user, user_project)
             not_modified!
           else
-            resource.unsubscribe(current_user)
-            present resource, with: entity_class, current_user: current_user
+            resource.unsubscribe(current_user, user_project)
+            present resource, with: entity_class, current_user: current_user, project: user_project
           end
         end
       end

spec/controllers/groups/labels_controller_spec.rb

+require 'spec_helper'
+
+describe Groups::LabelsController do
+  let(:group) { create(:group) }
+  let(:user)  { create(:user) }
+
+  before do
+    group.add_owner(user)
+
+    sign_in(user)
+  end
+
+  describe 'POST #toggle_subscription' do
+    it 'allows user to toggle subscription on group labels' do
+      label = create(:group_label, group: group)
+
+      post :toggle_subscription, group_id: group.to_param, id: label.to_param
+
+      expect(response).to have_http_status(200)
+    end
+  end
+end

spec/controllers/projects/boards/issues_controller_spec.rb

@@ -25,7 +25,7 @@ describe Projects::Boards::IssuesController do
         create(:labeled_issue, project: project, labels: [planning])
         create(:labeled_issue, project: project, labels: [development], due_date: Date.tomorrow)
         create(:labeled_issue, project: project, labels: [development], assignee: johndoe)
-        issue.subscribe(johndoe)
+        issue.subscribe(johndoe, project)
 
         list_issues user: user, board: board, list: list2
 

spec/controllers/projects/labels_controller_spec.rb

@@ -72,14 +72,8 @@ describe Projects::LabelsController do
   end
 
   describe 'POST #generate' do
-    let(:admin) { create(:admin) }
-
-    before do
-      sign_in(admin)
-    end
-
     context 'personal project' do
-      let(:personal_project) { create(:empty_project) }
+      let(:personal_project) { create(:empty_project, namespace: user.namespace) }
 
       it 'creates labels' do
         post :generate, namespace_id: personal_project.namespace.to_param, project_id: personal_project.to_param
@@ -96,4 +90,26 @@ describe Projects::LabelsController do
       end
     end
   end
+
+  describe 'POST #toggle_subscription' do
+    it 'allows user to toggle subscription on project labels' do
+      label = create(:label, project: project)
+
+      toggle_subscription(label)
+
+      expect(response).to have_http_status(200)
+    end
+
+    it 'allows user to toggle subscription on group labels' do
+      group_label = create(:group_label, group: group)
+
+      toggle_subscription(group_label)
+
+      expect(response).to have_http_status(200)
+    end
+
+    def toggle_subscription(label)
+      post :toggle_subscription, namespace_id: project.namespace.to_param, project_id: project.to_param, id: label.to_param
+    end
+  end
 end

spec/controllers/sent_notifications_controller_spec.rb

@@ -3,11 +3,11 @@ require 'rails_helper'
 describe SentNotificationsController, type: :controller do
   let(:user) { create(:user) }
   let(:project) { create(:empty_project) }
-  let(:sent_notification) { create(:sent_notification, noteable: issue, recipient: user) }
+  let(:sent_notification) { create(:sent_notification, project: project, noteable: issue, recipient: user) }
 
   let(:issue) do
     create(:issue, project: project, author: user) do |issue|
-      issue.subscriptions.create(user: user, subscribed: true)
+      issue.subscriptions.create(user: user, project: project, subscribed: true)
     end
   end
 
@@ -17,7 +17,7 @@ describe SentNotificationsController, type: :controller do
         before { get(:unsubscribe, id: sent_notification.reply_key, force: true) }
 
         it 'unsubscribes the user' do
-          expect(issue.subscribed?(user)).to be_falsey
+          expect(issue.subscribed?(user, project)).to be_falsey
         end
 
         it 'sets the flash message' do
@@ -33,7 +33,7 @@ describe SentNotificationsController, type: :controller do
         before { get(:unsubscribe, id: sent_notification.reply_key) }
 
         it 'does not unsubscribe the user' do
-          expect(issue.subscribed?(user)).to be_truthy
+          expect(issue.subscribed?(user, project)).to be_truthy
         end
 
         it 'does not set the flash message' do
@@ -53,7 +53,7 @@ describe SentNotificationsController, type: :controller do
         before { get(:unsubscribe, id: sent_notification.reply_key.reverse) }
 
         it 'does not unsubscribe the user' do
-          expect(issue.subscribed?(user)).to be_truthy
+          expect(issue.subscribed?(user, project)).to be_truthy
         end
 
         it 'does not set the flash message' do
@@ -69,7 +69,7 @@ describe SentNotificationsController, type: :controller do
         before { get(:unsubscribe, id: sent_notification.reply_key, force: true) }
 
         it 'unsubscribes the user' do
-          expect(issue.subscribed?(user)).to be_falsey
+          expect(issue.subscribed?(user, project)).to be_falsey
         end
 
         it 'sets the flash message' do
@@ -85,14 +85,14 @@ describe SentNotificationsController, type: :controller do
       context 'when the force param is not passed' do
         let(:merge_request) do
           create(:merge_request, source_project: project, author: user) do |merge_request|
-            merge_request.subscriptions.create(user: user, subscribed: true)
+            merge_request.subscriptions.create(user: user, project: project, subscribed: true)
           end
         end
-        let(:sent_notification) { create(:sent_notification, noteable: merge_request, recipient: user) }
+        let(:sent_notification) { create(:sent_notification, project: project, noteable: merge_request, recipient: user) }
         before { get(:unsubscribe, id: sent_notification.reply_key) }
 
         it 'unsubscribes the user' do
-          expect(merge_request.subscribed?(user)).to be_falsey
+          expect(merge_request.subscribed?(user, project)).to be_falsey
         end
 
         it 'sets the flash message' do

spec/factories/subscriptions.rb

+FactoryGirl.define do
+  factory :subscription do
+    user
+    project factory: :empty_project
+    subscribable factory: :issue
+  end
+end

spec/features/projects/labels/subscription_spec.rb

+require 'spec_helper'
+
+feature 'Labels subscription', feature: true do
+  let(:user)     { create(:user) }
+  let(:group)    { create(:group) }
+  let(:project)  { create(:empty_project, :public, namespace: group) }
+  let!(:bug)     { create(:label, project: project, title: 'bug') }
+  let!(:feature) { create(:group_label, group: group, title: 'feature') }
+
+  context 'when signed in' do
+    before do
+      project.team << [user, :developer]
+      login_as user
+    end
+
+    scenario 'users can subscribe/unsubscribe to labels', js: true do
+      visit namespace_project_labels_path(project.namespace, project)
+
+      expect(page).to have_content('bug')
+      expect(page).to have_content('feature')
+
+      within "#project_label_#{bug.id}" do
+        expect(page).not_to have_button 'Unsubscribe'
+
+        click_button 'Subscribe'
+
+        expect(page).not_to have_button 'Subscribe'
+        expect(page).to have_button 'Unsubscribe'
+
+        click_button 'Unsubscribe'
+
+        expect(page).to have_button 'Subscribe'
+        expect(page).not_to have_button 'Unsubscribe'
+      end
+
+      within "#group_label_#{feature.id}" do
+        expect(page).not_to have_button 'Unsubscribe'
+
+        click_link_on_dropdown('Group level')
+
+        expect(page).not_to have_selector('.dropdown-group-label')
+        expect(page).to have_button 'Unsubscribe'
+
+        click_button 'Unsubscribe'
+
+        expect(page).to have_selector('.dropdown-group-label')
+
+        click_link_on_dropdown('Project level')
+
+        expect(page).not_to have_selector('.dropdown-group-label')
+        expect(page).to have_button 'Unsubscribe'
+      end
+    end
+  end
+
+  context 'when not signed in' do
+    it 'users can not subscribe/unsubscribe to labels' do
+      visit namespace_project_labels_path(project.namespace, project)
+
+      expect(page).to have_content 'bug'
+      expect(page).to have_content 'feature'
+      expect(page).not_to have_button('Subscribe')
+      expect(page).not_to have_selector('.dropdown-group-label')
+    end
+  end
+
+  def click_link_on_dropdown(text)
+    find('.dropdown-group-label').click
+
+    page.within('.dropdown-group-label') do
+      find('a.js-subscribe-button', text: text).click
+    end
+  end
+end

spec/features/unsubscribe_links_spec.rb

@@ -26,11 +26,11 @@ describe 'Unsubscribe links', feature: true do
         expect(current_path).to eq unsubscribe_sent_notification_path(SentNotification.last)
         expect(page).to have_text(%(Unsubscribe from issue #{issue.title} (#{issue.to_reference})))
         expect(page).to have_text(%(Are you sure you want to unsubscribe from issue #{issue.title} (#{issue.to_reference})?))
-        expect(issue.subscribed?(recipient)).to be_truthy
+        expect(issue.subscribed?(recipient, project)).to be_truthy
 
         click_link 'Unsubscribe'
 
-        expect(issue.subscribed?(recipient)).to be_falsey
+        expect(issue.subscribed?(recipient, project)).to be_falsey
         expect(current_path).to eq new_user_session_path
       end
 
@@ -38,11 +38,11 @@ describe 'Unsubscribe links', feature: true do
         visit body_link
 
         expect(current_path).to eq unsubscribe_sent_notification_path(SentNotification.last)
-        expect(issue.subscribed?(recipient)).to be_truthy
+        expect(issue.subscribed?(recipient, project)).to be_truthy
 
         click_link 'Cancel'
 
-        expect(issue.subscribed?(recipient)).to be_truthy
+        expect(issue.subscribed?(recipient, project)).to be_truthy
         expect(current_path).to eq new_user_session_path
       end
     end
@@ -51,7 +51,7 @@ describe 'Unsubscribe links', feature: true do
       visit header_link
 
       expect(page).to have_text('unsubscribed')
-      expect(issue.subscribed?(recipient)).to be_falsey
+      expect(issue.subscribed?(recipient, project)).to be_falsey
     end
   end
 
@@ -62,14 +62,14 @@ describe 'Unsubscribe links', feature: true do
       visit body_link
 
       expect(page).to have_text('unsubscribed')
-      expect(issue.subscribed?(recipient)).to be_falsey
+      expect(issue.subscribed?(recipient, project)).to be_falsey
     end
 
     it 'unsubscribes from the issue when visiting the link from the header' do
       visit header_link
 
       expect(page).to have_text('unsubscribed')
-      expect(issue.subscribed?(recipient)).to be_falsey
+      expect(issue.subscribed?(recipient, project)).to be_falsey
     end
   end
 end

spec/models/concerns/issuable_spec.rb

@@ -176,23 +176,25 @@ describe Issue, "Issuable" do
   end
 
   describe '#subscribed?' do
+    let(:project) { issue.project }
+
     context 'user is not a participant in the issue' do
       before { allow(issue).to receive(:participants).with(user).and_return([]) }
 
       it 'returns false when no subcription exists' do
-        expect(issue.subscribed?(user)).to be_falsey
+        expect(issue.subscribed?(user, project)).to be_falsey
       end
 
       it 'returns true when a subcription exists and subscribed is true' do
-        issue.subscriptions.create(user: user, subscribed: true)
+        issue.subscriptions.create(user: user, project: project, subscribed: true)
 
-        expect(issue.subscribed?(user)).to be_truthy
+        expect(issue.subscribed?(user, project)).to be_truthy
       end
 
       it 'returns false when a subcription exists and subscribed is false' do
-        issue.subscriptions.create(user: user, subscribed: false)
+        issue.subscriptions.create(user: user, project: project, subscribed: false)
 
-        expect(issue.subscribed?(user)).to be_falsey
+        expect(issue.subscribed?(user, project)).to be_falsey
       end
     end
 
@@ -200,19 +202,19 @@ describe Issue, "Issuable" do
       before { allow(issue).to receive(:participants).with(user).and_return([user]) }
 
       it 'returns false when no subcription exists' do
-        expect(issue.subscribed?(user)).to be_truthy
+        expect(issue.subscribed?(user, project)).to be_truthy
       end
 
       it 'returns true when a subcription exists and subscribed is true' do
-        issue.subscriptions.create(user: user, subscribed: true)
+        issue.subscriptions.create(user: user, project: project, subscribed: true)
 
-        expect(issue.subscribed?(user)).to be_truthy
+        expect(issue.subscribed?(user, project)).to be_truthy
       end
 
       it 'returns false when a subcription exists and subscribed is false' do
-        issue.subscriptions.create(user: user, subscribed: false)
+        issue.subscriptions.create(user: user, project: project, subscribed: false)
 
-        expect(issue.subscribed?(user)).to be_falsey
+        expect(issue.subscribed?(user, project)).to be_falsey
       end
     end
   end

spec/models/concerns/subscribable_spec.rb

 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_1)   { create(:user) }
 
   describe '#subscribed?' do
-    it 'returns false when no subcription exists' do
-      expect(resource.subscribed?(user)).to be_falsey
-    end
+    context 'without project' do
+      it 'returns false when no subscription exists' do
+        expect(resource.subscribed?(user_1)).to be_falsey
+      end
+
+      it 'returns true when a subcription exists and subscribed is true' do
+        resource.subscriptions.create(user: user_1, subscribed: true)
+
+        expect(resource.subscribed?(user_1)).to be_truthy
+      end
 
-    it 'returns true when a subcription exists and subscribed is true' do
-      resource.subscriptions.create(user: user, subscribed: true)
+      it 'returns false when a subcription exists and subscribed is false' do
+        resource.subscriptions.create(user: user_1, subscribed: false)
 
-      expect(resource.subscribed?(user)).to be_truthy
+        expect(resource.subscribed?(user_1)).to be_falsey
+      end
     end
 
-    it 'returns false when a subcription exists and subscribed is false' do
-      resource.subscriptions.create(user: user, subscribed: false)
+    context 'with project' do
+      it 'returns false when no subscription exists' do
+        expect(resource.subscribed?(user_1, project)).to be_falsey
+      end
+
+      it 'returns true when a subcription exists and subscribed is true' do
+        resource.subscriptions.create(user: user_1, project: project, subscribed: true)
+
+        expect(resource.subscribed?(user_1, project)).to be_truthy
+      end
 
-      expect(resource.subscribed?(user)).to be_falsey
+      it 'returns false when a subcription exists and subscribed is false' do
+        resource.subscriptions.create(user: user_1, project: project, subscribed: false)
+
+        expect(resource.subscribed?(user_1, project)).to be_falsey
+      end
     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)
+      user_2 = create(:user)
+      resource.subscriptions.create(user: user_1, subscribed: true)
+      resource.subscriptions.create(user: user_2, 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 contain_exactly(user_1, user_2)
     end
   end
 
   describe '#toggle_subscription' do
-    it 'toggles the current subscription state for the given user' do
-      expect(resource.subscribed?(user)).to be_falsey
+    context 'without project' do
+      it 'toggles the current subscription state for the given user' do
+        expect(resource.subscribed?(user_1)).to be_falsey
 
-      resource.toggle_subscription(user)
+        resource.toggle_subscription(user_1)
 
-      expect(resource.subscribed?(user)).to be_truthy
+        expect(resource.subscribed?(user_1)).to be_truthy
+      end
+    end
+
+    context 'with project' do
+      it 'toggles the current subscription state for the given user' do
+        expect(resource.subscribed?(user_1, project)).to be_falsey
+
+        resource.toggle_subscription(user_1, project)
+
+        expect(resource.subscribed?(user_1, project)).to be_truthy
+      end
     end
   end
 
   describe '#subscribe' do
-    it 'subscribes the given user' do
-      expect(resource.subscribed?(user)).to be_falsey
+    context 'without project' do
+      it 'subscribes the given user' do
+        expect(resource.subscribed?(user_1)).to be_falsey
+
+        resource.subscribe(user_1)
+
+        expect(resource.subscribed?(user_1)).to be_truthy
+      end
+    end
+
+    context 'with project' do
+      it 'subscribes the given user' do
+        expect(resource.subscribed?(user_1, project)).to be_falsey
 
-      resource.subscribe(user)
+        resource.subscribe(user_1, project)
 
-      expect(resource.subscribed?(user)).to be_truthy
+        expect(resource.subscribed?(user_1, project)).to be_truthy
+      end
     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
+    context 'without project' do
+      it 'unsubscribes the given current user' do
+        resource.subscriptions.create(user: user_1, subscribed: true)
+        expect(resource.subscribed?(user_1)).to be_truthy
+
+        resource.unsubscribe(user_1)
+
+        expect(resource.subscribed?(user_1)).to be_falsey
+      end
+    end
+
+    context 'with project' do
+      it 'unsubscribes the given current user' do
+        resource.subscriptions.create(user: user_1, project: project, subscribed: true)
+        expect(resource.subscribed?(user_1, project)).to be_truthy
 
-      resource.unsubscribe(user)
+        resource.unsubscribe(user_1, project)
 
-      expect(resource.subscribed?(user)).to be_falsey
+        expect(resource.subscribed?(user_1, project)).to be_falsey
+      end
     end
   end
 end

spec/models/subscription_spec.rb

+require 'spec_helper'
+
+describe Subscription, models: true do
+  describe 'relationships' do
+    it { is_expected.to belong_to(:project) }
+    it { is_expected.to belong_to(:subscribable) }
+    it { is_expected.to belong_to(:user) }
+  end
+
+  describe 'validations' do
+    it { is_expected.to validate_presence_of(:subscribable) }
+    it { is_expected.to validate_presence_of(:user) }
+
+    it 'validates uniqueness of project_id scoped to subscribable_id, subscribable_type, and user_id' do
+      create(:subscription)
+
+      expect(subject).to validate_uniqueness_of(:project_id).scoped_to([:subscribable_id, :subscribable_type, :user_id])
+    end
+  end
+end

spec/requests/api/issues_spec.rb

@@ -637,7 +637,7 @@ describe API::API, api: true  do
 
     it "sends notifications for subscribers of newly added labels" do
       label = project.labels.first
-      label.toggle_subscription(user2)
+      label.toggle_subscription(user2, project)
 
       perform_enqueued_jobs do
         post api("/projects/#{project.id}/issues", user),
@@ -828,7 +828,7 @@ describe API::API, api: true  do
 
     it "sends notifications for subscribers of newly added labels when issue is updated" do
       label = create(:label, title: 'foo', color: '#FFAABB', project: project)
-      label.toggle_subscription(user2)
+      label.toggle_subscription(user2, project)
 
       perform_enqueued_jobs do
         put api("/projects/#{project.id}/issues/#{issue.id}", user),

spec/requests/api/labels_spec.rb

@@ -339,7 +339,7 @@ describe API::API, api: true  do
     end
 
     context "when user is already subscribed to label" do
-      before { label1.subscribe(user) }
+      before { label1.subscribe(user, project) }
 
       it "returns 304" do
         post api("/projects/#{project.id}/labels/#{label1.id}/subscription", user)
@@ -358,7 +358,7 @@ describe API::API, api: true  do
   end
 
   describe "DELETE /projects/:id/labels/:label_id/subscription" do
-    before { label1.subscribe(user) }
+    before { label1.subscribe(user, project) }
 
     context "when label_id is a label title" do
       it "unsubscribes from the label" do
@@ -381,7 +381,7 @@ describe API::API, api: true  do
     end
 
     context "when user is already unsubscribed from label" do
-      before { label1.unsubscribe(user) }
+      before { label1.unsubscribe(user, project) }
 
       it "returns 304" do
         delete api("/projects/#{project.id}/labels/#{label1.id}/subscription", user)

spec/services/issuable/bulk_update_service_spec.rb

@@ -260,14 +260,14 @@ describe Issuable::BulkUpdateService, services: true do
     it 'subscribes the given user' do
       bulk_update(issues, subscription_event: 'subscribe')
 
-      expect(issues).to all(be_subscribed(user))
+      expect(issues).to all(be_subscribed(user, project))
     end
   end
 
   describe 'unsubscribe from issues' do
     let(:issues) do
       create_list(:closed_issue, 2, project: project) do |issue|
-        issue.subscriptions.create(user: user, subscribed: true)
+        issue.subscriptions.create(user: user, project: project, subscribed: true)
       end
     end
 
@@ -275,7 +275,7 @@ describe Issuable::BulkUpdateService, services: true do
       bulk_update(issues, subscription_event: 'unsubscribe')
 
       issues.each do |issue|
-        expect(issue).not_to be_subscribed(user)
+        expect(issue).not_to be_subscribed(user, project)
       end
     end
   end

spec/services/issues/update_service_spec.rb

@@ -215,7 +215,7 @@ describe Issues::UpdateService, services: true do
 
       let!(:subscriber) do
         create(:user).tap do |u|
-          label.toggle_subscription(u)
+          label.toggle_subscription(u, project)
           project.team << [u, :developer]
         end
       end

spec/services/merge_requests/update_service_spec.rb

@@ -199,7 +199,7 @@ describe MergeRequests::UpdateService, services: true do
 
     context 'when the issue is relabeled' do
       let!(:non_subscriber) { create(:user) }
-      let!(:subscriber) { create(:user).tap { |u| label.toggle_subscription(u) } }
+      let!(:subscriber) { create(:user) { |u| label.toggle_subscription(u, project) } }
 
       before do
         project.team << [non_subscriber, :developer]

spec/services/slash_commands/interpret_service_spec.rb

@@ -169,7 +169,7 @@ describe SlashCommands::InterpretService, services: true do
 
     shared_examples 'unsubscribe command' do
       it 'populates subscription_event: "unsubscribe" if content contains /unsubscribe' do
-        issuable.subscribe(developer)
+        issuable.subscribe(developer, project)
         _, updates = service.execute(content, issuable)
 
         expect(updates).to eq(subscription_event: 'unsubscribe')
@@ -321,7 +321,7 @@ describe SlashCommands::InterpretService, services: true do
     it_behaves_like 'multiple label with same argument' do
       let(:content) { %(/label ~"#{inprogress.title}" \n/label ~#{inprogress.title}) }
       let(:issuable) { issue }
-    end	
+    end
 
     it_behaves_like 'unlabel command' do
       let(:content) { %(/unlabel ~"#{inprogress.title}") }

spec/support/features/issuable_slash_commands_shared_examples.rb

@@ -230,31 +230,31 @@ shared_examples 'issuable record that supports slash commands in its description
 
     context "with a note subscribing to the #{issuable_type}" do
       it "creates a new todo for the #{issuable_type}" do
-        expect(issuable.subscribed?(master)).to be_falsy
+        expect(issuable.subscribed?(master, project)).to be_falsy
 
         write_note("/subscribe")
 
         expect(page).not_to have_content '/subscribe'
         expect(page).to have_content 'Your commands have been executed!'
 
-        expect(issuable.subscribed?(master)).to be_truthy
+        expect(issuable.subscribed?(master, project)).to be_truthy
       end
     end
 
     context "with a note unsubscribing to the #{issuable_type} as done" do
       before do
-        issuable.subscribe(master)
+        issuable.subscribe(master, project)
       end
 
       it "creates a new todo for the #{issuable_type}" do
-        expect(issuable.subscribed?(master)).to be_truthy
+        expect(issuable.subscribed?(master, project)).to be_truthy
 
         write_note("/unsubscribe")
 
         expect(page).not_to have_content '/unsubscribe'
         expect(page).to have_content 'Your commands have been executed!'
 
-        expect(issuable.subscribed?(master)).to be_falsy
+        expect(issuable.subscribed?(master, project)).to be_falsy
       end
     end
   end