Merge branch 'master' of dev.gitlab.org:gitlab/gitlabhq into ce-to-ee

Conflicts:
	db/schema.rb

CHANGELOG

@@ -75,6 +75,9 @@ 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
+  - Delete deploy key when last connection to a project is destroyed.
+  - Fix invalid Atom feeds when using emoji, horizontal rules, or images (Christian Walther)
 
 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/deploy_keys_controller.rb

@@ -37,7 +37,7 @@ class Projects::DeployKeysController < Projects::ApplicationController
     @key.destroy
 
     respond_to do |format|
-      format.html { redirect_to project_deploy_keys_url }
+      format.html { redirect_to namespace_project_deploy_keys_path(@project.namespace, @project) }
       format.js { render nothing: true }
     end
   end
@@ -50,7 +50,7 @@ class Projects::DeployKeysController < Projects::ApplicationController
   end
 
   def disable
-    @project.deploy_keys_projects.where(deploy_key_id: params[:id]).last.destroy
+    @project.deploy_keys_projects.find_by(deploy_key_id: params[:id]).destroy
 
     redirect_to namespace_project_deploy_keys_path(@project.namespace,
                                                    @project)

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/deploy_keys_project.rb

@@ -16,4 +16,12 @@ class DeployKeysProject < ActiveRecord::Base
   validates :deploy_key_id, presence: true
   validates :deploy_key_id, uniqueness: { scope: [:project_id], message: "already exists in project" }
   validates :project_id, presence: true
+
+  after_destroy :destroy_orphaned_deploy_key
+
+  private
+
+  def destroy_orphaned_deploy_key
+    self.deploy_key.destroy if self.deploy_key.deploy_keys_projects.length == 0
+  end
 end

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/events/_event_issue.atom.haml

 %div{xmlns: "http://www.w3.org/1999/xhtml"}
   - if issue.description.present?
-    = markdown issue.description
+    = markdown(issue.description, xhtml: true)

app/views/events/_event_merge_request.atom.haml

 %div{xmlns: "http://www.w3.org/1999/xhtml"}
   - if merge_request.description.present?
-    = markdown merge_request.description
+    = markdown(merge_request.description, xhtml: true)

app/views/events/_event_note.atom.haml

 %div{xmlns: "http://www.w3.org/1999/xhtml"}
-  = markdown note.note
+  = markdown(note.note, xhtml: true)

app/views/events/_event_push.atom.haml

@@ -6,7 +6,7 @@
       %i
         at
         = commit[:timestamp].to_time.to_s(:short)
-    %blockquote= markdown(escape_once(commit[:message]))
+    %blockquote= markdown(escape_once(commit[:message]), xhtml: true)
   - if event.commits_count > 15
     %p
       %i

app/views/profiles/show.html.haml

@@ -96,5 +96,7 @@
 
   .row
     .col-md-7
-      .col-sm-2
-      = f.submit 'Save changes', class: "btn btn-success"
+      .form-group
+        .col-sm-2  
+        .col-sm-10
+          = f.submit 'Save changes', class: "btn btn-success"

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

@@ -436,6 +436,7 @@ Gitlab::Application.routes.draw do
             post :automerge
             get :automerge_check
             get :ci_status
+            post :toggle_subscription
           end
 
           collection do
@@ -469,6 +470,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: 20150312000132) do
+ActiveRecord::Schema.define(version: 20150313012111) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -458,6 +458,17 @@ ActiveRecord::Schema.define(version: 20150312000132) 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

@@ -230,3 +230,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

lib/gitlab/markdown.rb

@@ -34,17 +34,23 @@ module Gitlab
 
     attr_reader :html_options
 
-    def gfm_with_tasks(text, project = @project, html_options = {})
-      text = gfm(text, project, html_options)
-      parse_tasks(text)
+    # Public: Parse the provided text with GitLab-Flavored Markdown
+    #
+    # text         - the source text
+    # project      - extra options for the reference links as given to link_to
+    # html_options - extra options for the reference links as given to link_to
+    def gfm(text, project = @project, html_options = {})
+      gfm_with_options(text, {}, project, html_options)
     end
 
     # Public: Parse the provided text with GitLab-Flavored Markdown
     #
     # text         - the source text
+    # options      - parse_tasks: true - render tasks
+    #              - xhtml: true       - output XHTML instead of HTML
     # project      - extra options for the reference links as given to link_to
     # html_options - extra options for the reference links as given to link_to
-    def gfm(text, project = @project, html_options = {})
+    def gfm_with_options(text, options = {}, project = @project, html_options = {})
       return text if text.nil?
 
       # Duplicate the string so we don't alter the original, then call to_str
@@ -87,14 +93,22 @@ module Gitlab
       markdown_pipeline = HTML::Pipeline::Gitlab.new(filters).pipeline
 
       result = markdown_pipeline.call(text, markdown_context)
-      text = result[:output].to_html(save_with: 0)
+      saveoptions = 0
+      if options[:xhtml]
+        saveoptions |= Nokogiri::XML::Node::SaveOptions::AS_XHTML
+      end
+      text = result[:output].to_html(save_with: saveoptions)
 
       allowed_attributes = ActionView::Base.sanitized_allowed_attributes
       allowed_tags = ActionView::Base.sanitized_allowed_tags
 
-      sanitize text.html_safe,
-               attributes: allowed_attributes + %w(id class style),
-               tags: allowed_tags + %w(table tr td th)
+      text = sanitize text.html_safe,
+                      attributes: allowed_attributes + %w(id class style),
+                      tags: allowed_tags + %w(table tr td th)
+      if options[:parse_tasks]
+        text = parse_tasks(text)
+      end
+      text
     end
 
     private

lib/redcarpet/render/gitlab_html.rb

@@ -67,10 +67,6 @@ class Redcarpet::Render::GitlabHTML < Redcarpet::Render::HTML
     unless @template.instance_variable_get("@project_wiki") || @project.nil?
       full_document = h.create_relative_links(full_document)
     end
-    if @options[:parse_tasks]
-      h.gfm_with_tasks(full_document)
-    else
-      h.gfm(full_document)
-    end
+    h.gfm_with_options(full_document, @options)
   end
 end

spec/features/atom/users_spec.rb

@@ -15,17 +15,24 @@ describe "User Feed", feature: true  do
       let(:project) { create(:project) }
       let(:issue) do
         create(:issue, project: project,
-               author: user, description: '')
+               author: user, description: "Houston, we have a bug!\n\n***\n\nI guess.")
       end
       let(:note) do
         create(:note, noteable: issue, author: user,
-               note: 'Bug confirmed', project: project)
+               note: 'Bug confirmed :+1:', project: project)
+      end
+      let(:merge_request) do
+        create(:merge_request,
+               title: 'Fix bug', author: user,
+               source_project: project, target_project: project,
+               description: "Here is the fix: ![an image](image.png)")
       end
 
       before do
         project.team << [user, :master]
         issue_event(issue, user)
         note_event(note, user)
+        merge_request_event(merge_request, user)
         visit user_path(user, :atom, private_token: user.private_token)
       end
 
@@ -37,6 +44,18 @@ describe "User Feed", feature: true  do
         expect(body).
           to have_content("#{safe_name} commented on issue ##{issue.iid}")
       end
+
+      it 'should have XHTML summaries in issue descriptions' do
+        expect(body).to match /we have a bug!<\/p>\n\n<hr ?\/>\n\n<p>I guess/
+      end
+
+      it 'should have XHTML summaries in notes' do
+        expect(body).to match /Bug confirmed <img[^>]*\/>/
+      end
+
+      it 'should have XHTML summaries in merge request descriptions' do
+        expect(body).to match /Here is the fix: <img[^>]*\/>/
+      end
     end
   end
 
@@ -48,6 +67,10 @@ describe "User Feed", feature: true  do
     EventCreateService.new.leave_note(note, user)
   end
 
+  def merge_request_event(request, user)
+    EventCreateService.new.open_mr(request, user)
+  end
+
   def safe_name
     html_escape(user.name)
   end

spec/models/deploy_keys_project_spec.rb

@@ -21,4 +21,37 @@ describe DeployKeysProject do
     it { is_expected.to validate_presence_of(:project_id) }
     it { is_expected.to validate_presence_of(:deploy_key_id) }
   end
+
+  describe "Destroying" do
+    let(:project)     { create(:project) }
+    subject           { create(:deploy_keys_project, project: project) }
+    let(:deploy_key)  { subject.deploy_key }
+
+    context "when the deploy key is only used by this project" do
+      it "destroys the deploy key" do
+        subject.destroy
+
+        expect {
+          deploy_key.reload
+        }.to raise_error(ActiveRecord::RecordNotFound)
+      end
+    end
+
+    context "when the deploy key is used by more than one project" do
+
+      let!(:other_project) { create(:project) }
+
+      before do
+        other_project.deploy_keys << deploy_key
+      end
+
+      it "doesn't destroy the deploy key" do
+        subject.destroy
+
+        expect {
+          deploy_key.reload
+        }.not_to raise_error(ActiveRecord::RecordNotFound)
+      end
+    end
+  end
 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