tests

8 files changed, 36 insertions, 22 deletions

diff --git a/app/assets/javascripts/subscription.js.coffee b/app/assets/javascripts/subscription.js.coffee
index f457622fc3a..a009969e4dc 100644
--- a/app/assets/javascripts/subscription.js.coffee
+++ b/app/assets/javascripts/subscription.js.coffee
@@ -1,14 +1,13 @@
 class @Subscription
   constructor: (url) ->
     $(".subscribe-button").click (event)=>
-      self = @
       btn = $(event.currentTarget)
       action = btn.prop("value")
       current_status = $(".sub_status").text().trim()
       $(".fa-spinner.subscription").removeClass("hidden")
       $(".sub_status").empty()
       
-      $.post url, subscription: action, =>
+      $.post url, =>
         $(".fa-spinner.subscription").addClass("hidden")
         status = if current_status == "subscribed" then "unsubscribed" else "subscribed"
         $(".sub_status").text(status)
diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb
index 903b7a68dc9..88302276b5e 100644
--- a/app/controllers/projects/issues_controller.rb
+++ b/app/controllers/projects/issues_controller.rb
@@ -1,6 +1,6 @@
 class Projects::IssuesController < Projects::ApplicationController
   before_filter :module_enabled
-  before_filter :issue, only: [:edit, :update, :show, :set_subscription]
+  before_filter :issue, only: [:edit, :update, :show, :toggle_subscription]
 
   # Allow read any issue
   before_filter :authorize_read_issue!
@@ -97,11 +97,8 @@ class Projects::IssuesController < Projects::ApplicationController
     redirect_to :back, notice: "#{result[:count]} issues updated"
   end
 
-  def set_subscription
-    subscribed = params[:subscription] == "Subscribe"
-
-    sub = @issue.subscriptions.find_or_create_by(user_id: current_user.id)
-    sub.update(subscribed: subscribed)
+  def toggle_subscription
+    @issue.toggle_subscription(current_user)
     
     render nothing: true
   end
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index 51ac61c3273..c63a9b0cd44 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/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, :set_subscription]
+  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,11 +174,8 @@ class Projects::MergeRequestsController < Projects::ApplicationController
     render json: response
   end
 
-  def set_subscription
-    subscribed = params[:subscription] == "Subscribe"
-
-    sub = @merge_request.subscriptions.find_or_create_by(user_id: current_user.id)
-    sub.update(subscribed: subscribed)
+  def toggle_subscription
+    @merge_request.toggle_subscription(current_user)
     
     render nothing: true
   end
diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb
index d1a35ca5294..88ac83744df 100644
--- a/app/models/concerns/issuable.rb
+++ b/app/models/concerns/issuable.rb
@@ -133,7 +133,7 @@ module Issuable
     users.concat(mentions.reduce([], :|)).uniq
   end
 
-  def subscription_status(user)
+  def subscribed?(user)
     subscription = subscriptions.find_by_user_id(user.id)
 
     if subscription
@@ -143,6 +143,12 @@ module Issuable
     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,
diff --git a/app/models/subscription.rb b/app/models/subscription.rb
index 276cf0e9465..dd75d3ab8ba 100644
--- a/app/models/subscription.rb
+++ b/app/models/subscription.rb
@@ -1,3 +1,16 @@
+# == 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
diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb
index 5ebde8fea84..3e1f4e62f10 100644
--- a/app/services/notification_service.rb
+++ b/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)
 
@@ -333,7 +335,7 @@ class NotificationService
     subscriptions = target.subscriptions
 
     if subscriptions.any?
-      recipients + subscriptions.where("subscribed is true").map(&:user)
+      recipients + subscriptions.where(subscribed: true).map(&:user)
     else
       recipients
     end
diff --git a/app/views/projects/issues/_issue_context.html.haml b/app/views/projects/issues/_issue_context.html.haml
index 24bfbdd4c58..85937e7bf42 100644
--- a/app/views/projects/issues/_issue_context.html.haml
+++ b/app/views/projects/issues/_issue_context.html.haml
@@ -33,12 +33,12 @@
         Subscription:
         %i.fa.fa-spinner.fa-spin.hidden.subscription
         %span.sub_status
-          = @issue.subscription_status(current_user) ? "subscribed" : "unsubscribed"
-    - subscribe_action = @issue.subscription_status(current_user) ? "Unsubscribe" : "Subscribe"
+          = @issue.subscribed?(current_user) ? "subscribed" : "unsubscribed"
+    - subscribe_action = @issue.subscribed?(current_user) ? "Unsubscribe" : "Subscribe"
     %input.btn.subscribe-button{:type => "button", :value => subscribe_action}
 
 :coffeescript
   $ ->
-    new Subscription("#{set_subscription_namespace_project_issue_path(@issue.project.namespace, @project, @issue)}")
+    new Subscription("#{toggle_subscription_namespace_project_issue_path(@issue.project.namespace, @project, @issue)}")
     
       
\ No newline at end of file
diff --git a/app/views/projects/merge_requests/show/_context.html.haml b/app/views/projects/merge_requests/show/_context.html.haml
index d0c00c1aeaf..79b0e7799a9 100644
--- a/app/views/projects/merge_requests/show/_context.html.haml
+++ b/app/views/projects/merge_requests/show/_context.html.haml
@@ -35,12 +35,12 @@
         Subscription:
         %i.fa.fa-spinner.fa-spin.hidden.subscription
         %span.sub_status
-          = @merge_request.subscription_status(current_user) ? "subscribed" : "unsubscribed"
-    - subscribe_action = @merge_request.subscription_status(current_user) ? "Unsubscribe" : "Subscribe"
+          = @merge_request.subscribed?(current_user) ? "subscribed" : "unsubscribed"
+    - subscribe_action = @merge_request.subscribed?(current_user) ? "Unsubscribe" : "Subscribe"
     %input.btn.subscribe-button{:type => "button", :value => subscribe_action}
 
 :coffeescript
   $ ->
-    new Subscription("#{set_subscription_namespace_project_issue_path(@merge_request.project.namespace, @project, @merge_request)}")
+    new Subscription("#{toggle_subscription_namespace_project_merge_request_path(@merge_request.project.namespace, @project, @merge_request)}")
     
       
\ No newline at end of file