Merge branch 'issue_subscription' into 'master'

Subscription to issue/mr

Fixes #1911 and #1909

![joxi_screenshot_1426601822159](https://dev.gitlab.org/gitlab/gitlabhq/uploads/53021bc5783271322ab2dfba7598eaa3/joxi_screenshot_1426601822159.png)

![joxi_screenshot_1426601836423](https://dev.gitlab.org/gitlab/gitlabhq/uploads/244ff360fbd6f30980f8dad699400814/joxi_screenshot_1426601836423.png)

See merge request !1702

CHANGELOG

@@ -74,6 +74,7 @@ v 7.9.0 (unreleased)
   - Raise recommended number of unicorn workers from 2 to 3
   - Use same layout and interactivity for project members as group members.
   - Prevent gitlab-shell character encoding issues by receiving its changes as raw data.
+  - Ability to unsubscribe/subscribe to issue or merge request
 
 v 7.8.4
   - Fix issue_tracker_id substitution in custom issue trackers

app/assets/javascripts/subscription.js.coffee

+class @Subscription
+  constructor: (url) ->
+    $(".subscribe-button").unbind("click").click (event)=>
+      btn = $(event.currentTarget)
+      action = btn.find("span").text()
+      current_status = $(".subscription-status").attr("data-status")
+      btn.prop("disabled", true)
+      
+      $.post url, =>
+        btn.prop("disabled", false)
+        status = if current_status == "subscribed" then "unsubscribed" else "subscribed"
+        $(".subscription-status").attr("data-status", status)
+        action = if status == "subscribed" then "Unsubscribe" else "Subscribe"
+        btn.find("span").text(action)
+        $(".subscription-status>div").toggleClass("hidden")
+
+    

app/controllers/projects/issues_controller.rb

 class Projects::IssuesController < Projects::ApplicationController
   before_filter :module_enabled
-  before_filter :issue, only: [:edit, :update, :show]
+  before_filter :issue, only: [:edit, :update, :show, :toggle_subscription]
 
   # Allow read any issue
   before_filter :authorize_read_issue!
@@ -97,6 +97,12 @@ class Projects::IssuesController < Projects::ApplicationController
     redirect_to :back, notice: "#{result[:count]} issues updated"
   end
 
+  def toggle_subscription
+    @issue.toggle_subscription(current_user)
+    
+    render nothing: true
+  end
+
   protected
 
   def issue

app/controllers/projects/merge_requests_controller.rb

@@ -2,7 +2,7 @@ require 'gitlab/satellite/satellite'
 
 class Projects::MergeRequestsController < Projects::ApplicationController
   before_filter :module_enabled
-  before_filter :merge_request, only: [:edit, :update, :show, :diffs, :automerge, :automerge_check, :ci_status]
+  before_filter :merge_request, only: [:edit, :update, :show, :diffs, :automerge, :automerge_check, :ci_status, :toggle_subscription]
   before_filter :closes_issues, only: [:edit, :update, :show, :diffs]
   before_filter :validates_merge_request, only: [:show, :diffs]
   before_filter :define_show_vars, only: [:show, :diffs]
@@ -174,6 +174,12 @@ class Projects::MergeRequestsController < Projects::ApplicationController
     render json: response
   end
 
+  def toggle_subscription
+    @merge_request.toggle_subscription(current_user)
+    
+    render nothing: true
+  end
+
   protected
 
   def selected_target_project

app/models/concerns/issuable.rb

@@ -15,6 +15,7 @@ module Issuable
     has_many :notes, as: :noteable, dependent: :destroy
     has_many :label_links, as: :target, dependent: :destroy
     has_many :labels, through: :label_links
+    has_many :subscriptions, dependent: :destroy, as: :subscribable
 
     validates :author, presence: true
     validates :title, presence: true, length: { within: 0..255 }
@@ -132,6 +133,22 @@ module Issuable
     users.concat(mentions.reduce([], :|)).uniq
   end
 
+  def subscribed?(user)
+    subscription = subscriptions.find_by_user_id(user.id)
+
+    if subscription
+      return subscription.subscribed
+    end
+
+    participants.include?(user)
+  end
+
+  def toggle_subscription(user)
+    subscriptions.
+      find_or_initialize_by(user_id: user.id).
+      update(subscribed: !subscribed?(user))
+  end
+
   def to_hook_data(user)
     {
       object_kind: self.class.name.underscore,

app/models/subscription.rb

+# == Schema Information
+#
+# Table name: subscriptions
+#
+#  id                :integer          not null, primary key
+#  user_id           :integer
+#  subscribable_id   :integer
+#  subscribable_type :string(255)
+#  subscribed        :boolean
+#  created_at        :datetime
+#  updated_at        :datetime
+#
+
+class Subscription < ActiveRecord::Base
+  belongs_to :user
+  belongs_to :subscribable, polymorphic: true
+
+  validates :user_id, 
+            uniqueness: { scope: [:subscribable_id, :subscribable_type] },
+            presence: true
+end

app/services/notification_service.rb

@@ -92,6 +92,8 @@ class NotificationService
   #
   def merge_mr(merge_request, current_user)
     recipients = reject_muted_users([merge_request.author, merge_request.assignee], merge_request.target_project)
+    recipients = add_subscribed_users(recipients, merge_request)
+    recipients = reject_unsubscribed_users(recipients, merge_request)
     recipients = recipients.concat(project_watchers(merge_request.target_project)).uniq
     recipients.delete(current_user)
 
@@ -151,6 +153,10 @@ class NotificationService
     # Reject mutes users
     recipients = reject_muted_users(recipients, note.project)
 
+    recipients = add_subscribed_users(recipients, note.noteable)
+    
+    recipients = reject_unsubscribed_users(recipients, note.noteable)
+
     # Reject author
     recipients.delete(note.author)
 
@@ -314,6 +320,27 @@ class NotificationService
     end
   end
 
+  def reject_unsubscribed_users(recipients, target)
+    return recipients unless target.respond_to? :subscriptions
+    
+    recipients.reject do |user|
+      subscription = target.subscriptions.find_by_user_id(user.id)
+      subscription && !subscription.subscribed
+    end
+  end
+
+  def add_subscribed_users(recipients, target)
+    return recipients unless target.respond_to? :subscriptions
+
+    subscriptions = target.subscriptions
+
+    if subscriptions.any?
+      recipients + subscriptions.where(subscribed: true).map(&:user)
+    else
+      recipients
+    end
+  end
+  
   def new_resource_email(target, project, method)
     recipients = build_recipients(target, project)
     recipients.delete(target.author)
@@ -361,7 +388,9 @@ class NotificationService
 
     recipients = reject_muted_users(recipients, project)
     recipients = reject_mention_users(recipients, project)
+    recipients = add_subscribed_users(recipients, target)
     recipients = recipients.concat(project_watchers(project)).uniq
+    recipients = reject_unsubscribed_users(recipients, target)
     recipients
   end
 

app/views/projects/issues/_issue_context.html.haml

@@ -26,3 +26,23 @@
       = f.select(:milestone_id, milestone_options(@issue), { include_blank: "Select milestone" }, {class: 'select2 select2-compact js-select2 js-milestone'})
       = hidden_field_tag :issue_context
       = f.submit class: 'btn'
+
+  %div.prepend-top-20.clearfix
+    .issuable-context-title
+      %label
+        Subscription:
+    %button.btn.btn-block.subscribe-button
+      %i.fa.fa-eye
+      %span= @issue.subscribed?(current_user) ? "Unsubscribe" : "Subscribe"
+    - subscribtion_status = @issue.subscribed?(current_user) ? "subscribed" : "unsubscribed"
+    .subscription-status{"data-status" => subscribtion_status}
+      .description-block.unsubscribed{class: ( "hidden" if @issue.subscribed?(current_user) )}
+        You're not receiving notifications from this thread.
+      .description-block.subscribed{class: ( "hidden" unless @issue.subscribed?(current_user) )}
+        You're receiving notifications because you're subscribed to this thread.
+
+:coffeescript
+  $ ->
+    new Subscription("#{toggle_subscription_namespace_project_issue_path(@issue.project.namespace, @project, @issue)}")
+    
+      
\ No newline at end of file

app/views/projects/merge_requests/show/_context.html.haml

@@ -28,3 +28,23 @@
         = f.select(:milestone_id, milestone_options(@merge_request), { include_blank: "Select milestone" }, {class: 'select2 select2-compact js-select2 js-milestone'})
         = hidden_field_tag :merge_request_context
         = f.submit class: 'btn'
+
+  %div.prepend-top-20.clearfix
+    .issuable-context-title
+      %label
+        Subscription:
+    %button.btn.btn-block.subscribe-button
+      %i.fa.fa-eye
+      %span= @merge_request.subscribed?(current_user) ? "Unsubscribe" : "Subscribe"
+    - subscribtion_status = @merge_request.subscribed?(current_user) ? "subscribed" : "unsubscribed"
+    .subscription-status{"data-status" => subscribtion_status}
+      .description-block.unsubscribed{class: ( "hidden" if @merge_request.subscribed?(current_user) )}
+        You're not receiving notifications from this thread.
+      .description-block.subscribed{class: ( "hidden" unless @merge_request.subscribed?(current_user) )}
+        You're receiving notifications because you're subscribed to this thread.
+
+:coffeescript
+  $ ->
+    new Subscription("#{toggle_subscription_namespace_project_merge_request_path(@merge_request.project.namespace, @project, @merge_request)}")
+    
+      
\ No newline at end of file

config/routes.rb

@@ -404,6 +404,7 @@ Gitlab::Application.routes.draw do
             post :automerge
             get :automerge_check
             get :ci_status
+            post :toggle_subscription
           end
 
           collection do
@@ -437,6 +438,9 @@ Gitlab::Application.routes.draw do
         end
 
         resources :issues, constraints: { id: /\d+/ }, except: [:destroy] do
+          member do
+            post :toggle_subscription
+          end
           collection do
             post  :bulk_update
           end

db/migrate/20150313012111_create_subscriptions_table.rb

+class CreateSubscriptionsTable < ActiveRecord::Migration
+  def change
+    create_table :subscriptions do |t|
+      t.integer :user_id
+      t.references :subscribable, polymorphic: true
+      t.boolean :subscribed
+      
+      t.timestamps
+    end
+
+    add_index :subscriptions, 
+              [:subscribable_id, :subscribable_type, :user_id],
+              unique: true,
+              name: 'subscriptions_user_id_and_ref_fields'
+  end
+end

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20150306023112) do
+ActiveRecord::Schema.define(version: 20150313012111) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -335,12 +335,12 @@ ActiveRecord::Schema.define(version: 20150306023112) do
     t.string   "import_url"
     t.integer  "visibility_level",       default: 0,        null: false
     t.boolean  "archived",               default: false,    null: false
+    t.string   "avatar"
     t.string   "import_status"
     t.float    "repository_size",        default: 0.0
     t.integer  "star_count",             default: 0,        null: false
     t.string   "import_type"
     t.string   "import_source"
-    t.string   "avatar"
   end
 
   add_index "projects", ["created_at", "id"], name: "index_projects_on_created_at_and_id", using: :btree
@@ -398,6 +398,17 @@ ActiveRecord::Schema.define(version: 20150306023112) do
   add_index "snippets", ["project_id"], name: "index_snippets_on_project_id", using: :btree
   add_index "snippets", ["visibility_level"], name: "index_snippets_on_visibility_level", using: :btree
 
+  create_table "subscriptions", force: true do |t|
+    t.integer  "user_id"
+    t.integer  "subscribable_id"
+    t.string   "subscribable_type"
+    t.boolean  "subscribed"
+    t.datetime "created_at"
+    t.datetime "updated_at"
+  end
+
+  add_index "subscriptions", ["subscribable_id", "subscribable_type", "user_id"], name: "subscriptions_user_id_and_ref_fields", unique: true, using: :btree
+
   create_table "taggings", force: true do |t|
     t.integer  "tag_id"
     t.integer  "taggable_id"

features/project/issues/issues.feature

@@ -202,3 +202,11 @@ Feature: Project Issues
     And I click link "Edit" for the issue
     And I preview a description text like "Bug fixed :smile:"
     Then I should see the Markdown write tab
+
+  @javascript
+  Scenario: I can unsubscribe from issue
+    Given project "Shop" has "Tasks-open" open issue with task markdown
+    When I visit issue page "Tasks-open"
+    Then I should see that I am subscribed
+    When I click button "Unsubscribe"
+    Then I should see that I am unsubscribed

features/project/merge_requests.feature

@@ -225,3 +225,10 @@ Feature: Project Merge Requests
     When I fill in merge request search with "Fe"
     Then I should see "Feature NS-03" in merge requests
     And I should not see "Bug NS-04" in merge requests
+
+  @javascript
+  Scenario: I can unsubscribe from merge request
+    Given I visit merge request page "Bug NS-04"
+    Then I should see that I am subscribed
+    When I click button "Unsubscribe"
+    Then I should see that I am unsubscribed

features/steps/project/issues/issues.rb

@@ -18,10 +18,23 @@ class Spinach::Features::ProjectIssues < Spinach::FeatureSteps
     page.should_not have_content "Tweet control"
   end
 
+  step 'I should see that I am subscribed' do
+    find(".subscribe-button span").text.should == "Unsubscribe"
+  end
+
+  step 'I should see that I am unsubscribed' do
+    sleep 0.2
+    find(".subscribe-button span").text.should == "Subscribe"
+  end
+
   step 'I click link "Closed"' do
     click_link "Closed"
   end
 
+  step 'I click button "Unsubscribe"' do
+    click_on "Unsubscribe"
+  end
+
   step 'I should see "Release 0.3" in issues' do
     page.should have_content "Release 0.3"
   end

features/steps/project/merge_requests.rb

@@ -56,6 +56,19 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps
     page.should_not have_content "Bug NS-04"
   end
 
+  step 'I should see that I am subscribed' do
+    find(".subscribe-button span").text.should == "Unsubscribe"
+  end
+
+  step 'I should see that I am unsubscribed' do
+    sleep 0.2
+    find(".subscribe-button span").text.should == "Subscribe"
+  end
+
+  step 'I click button "Unsubscribe"' do
+    click_on "Unsubscribe"
+  end
+
   step 'I click link "Close"' do
     first(:css, '.close-mr-link').click
   end

spec/services/notification_service_spec.rb

@@ -41,13 +41,18 @@ describe NotificationService do
 
       describe :new_note do
         it do
+          add_users_with_subscription(note.project, issue)
+
           should_email(@u_watcher.id)
           should_email(note.noteable.author_id)
           should_email(note.noteable.assignee_id)
           should_email(@u_mentioned.id)
+          should_email(@subscriber.id)
           should_not_email(note.author_id)
           should_not_email(@u_participating.id)
           should_not_email(@u_disabled.id)
+          should_not_email(@unsubscriber.id)
+
           notification.new_note(note)
         end
 
@@ -191,6 +196,7 @@ describe NotificationService do
 
     before do
       build_team(issue.project)
+      add_users_with_subscription(issue.project, issue)
     end
 
     describe :new_issue do
@@ -224,6 +230,8 @@ describe NotificationService do
         should_email(issue.assignee_id)
         should_email(@u_watcher.id)
         should_email(@u_participant_mentioned.id)
+        should_email(@subscriber.id)
+        should_not_email(@unsubscriber.id)
         should_not_email(@u_participating.id)
         should_not_email(@u_disabled.id)
 
@@ -245,6 +253,8 @@ describe NotificationService do
         should_email(issue.author_id)
         should_email(@u_watcher.id)
         should_email(@u_participant_mentioned.id)
+        should_email(@subscriber.id)
+        should_not_email(@unsubscriber.id)
         should_not_email(@u_participating.id)
         should_not_email(@u_disabled.id)
 
@@ -266,6 +276,8 @@ describe NotificationService do
         should_email(issue.author_id)
         should_email(@u_watcher.id)
         should_email(@u_participant_mentioned.id)
+        should_email(@subscriber.id)
+        should_not_email(@unsubscriber.id)
         should_not_email(@u_participating.id)
         should_not_email(@u_disabled.id)
 
@@ -287,6 +299,7 @@ describe NotificationService do
 
     before do
       build_team(merge_request.target_project)
+      add_users_with_subscription(merge_request.target_project, merge_request)
     end
 
     describe :new_merge_request do
@@ -311,6 +324,8 @@ describe NotificationService do
       it do
         should_email(merge_request.assignee_id)
         should_email(@u_watcher.id)
+        should_email(@subscriber.id)
+        should_not_email(@unsubscriber.id)
         should_not_email(@u_participating.id)
         should_not_email(@u_disabled.id)
         notification.reassigned_merge_request(merge_request, merge_request.author)
@@ -329,6 +344,8 @@ describe NotificationService do
       it do
         should_email(merge_request.assignee_id)
         should_email(@u_watcher.id)
+        should_email(@subscriber.id)
+        should_not_email(@unsubscriber.id)
         should_not_email(@u_participating.id)
         should_not_email(@u_disabled.id)
         notification.close_mr(merge_request, @u_disabled)
@@ -347,6 +364,8 @@ describe NotificationService do
       it do
         should_email(merge_request.assignee_id)
         should_email(@u_watcher.id)
+        should_email(@subscriber.id)
+        should_not_email(@unsubscriber.id)
         should_not_email(@u_participating.id)
         should_not_email(@u_disabled.id)
         notification.merge_mr(merge_request, @u_disabled)
@@ -365,6 +384,8 @@ describe NotificationService do
       it do
         should_email(merge_request.assignee_id)
         should_email(@u_watcher.id)
+        should_email(@subscriber.id)
+        should_not_email(@unsubscriber.id)
         should_not_email(@u_participating.id)
         should_not_email(@u_disabled.id)
         notification.reopen_mr(merge_request, @u_disabled)
@@ -420,4 +441,15 @@ describe NotificationService do
     project.team << [@u_mentioned, :master]
     project.team << [@u_committer, :master]
   end
+
+  def add_users_with_subscription(project, issuable)
+    @subscriber = create :user
+    @unsubscriber = create :user
+
+    project.team << [@subscriber, :master]
+    project.team << [@unsubscriber, :master]
+
+    issuable.subscriptions.create(user: @subscriber, subscribed: true)
+    issuable.subscriptions.create(user: @unsubscriber, subscribed: false)
+  end
 end