diff --git a/CHANGELOG b/CHANGELOG index 1194a2bf91..6de445f17d 100644 --- a/CHANGELOG +++ b/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 diff --git a/app/assets/javascripts/subscription.js.coffee b/app/assets/javascripts/subscription.js.coffee new file mode 100644 index 0000000000..7f41616d4e --- /dev/null +++ b/app/assets/javascripts/subscription.js.coffee @@ -0,0 +1,17 @@ +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") + + diff --git a/app/controllers/projects/deploy_keys_controller.rb b/app/controllers/projects/deploy_keys_controller.rb index b7cc305899..679a5d76ec 100644 --- a/app/controllers/projects/deploy_keys_controller.rb +++ b/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) diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index 4266bcaef1..88302276b5 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] + 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 diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 93d79d8166..c63a9b0cd4 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] + 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 diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index f5e23e9dc2..88ac83744d 100644 --- a/app/models/concerns/issuable.rb +++ b/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, diff --git a/app/models/deploy_keys_project.rb b/app/models/deploy_keys_project.rb index f23d8205dd..7e88903b9a 100644 --- a/app/models/deploy_keys_project.rb +++ b/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 diff --git a/app/models/subscription.rb b/app/models/subscription.rb new file mode 100644 index 0000000000..dd75d3ab8b --- /dev/null +++ b/app/models/subscription.rb @@ -0,0 +1,21 @@ +# == 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 diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index fb411c3e23..848ed77ebf 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) @@ -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 diff --git a/app/views/events/_event_issue.atom.haml b/app/views/events/_event_issue.atom.haml index eba2b63797..0edb61ea24 100644 --- a/app/views/events/_event_issue.atom.haml +++ b/app/views/events/_event_issue.atom.haml @@ -1,3 +1,3 @@ %div{xmlns: "http://www.w3.org/1999/xhtml"} - if issue.description.present? - = markdown issue.description + = markdown(issue.description, xhtml: true) diff --git a/app/views/events/_event_merge_request.atom.haml b/app/views/events/_event_merge_request.atom.haml index 0aea2d17d6..1a8b62abea 100644 --- a/app/views/events/_event_merge_request.atom.haml +++ b/app/views/events/_event_merge_request.atom.haml @@ -1,3 +1,3 @@ %div{xmlns: "http://www.w3.org/1999/xhtml"} - if merge_request.description.present? - = markdown merge_request.description + = markdown(merge_request.description, xhtml: true) diff --git a/app/views/events/_event_note.atom.haml b/app/views/events/_event_note.atom.haml index be0e05481e..b49c331ccf 100644 --- a/app/views/events/_event_note.atom.haml +++ b/app/views/events/_event_note.atom.haml @@ -1,2 +1,2 @@ %div{xmlns: "http://www.w3.org/1999/xhtml"} - = markdown note.note + = markdown(note.note, xhtml: true) diff --git a/app/views/events/_event_push.atom.haml b/app/views/events/_event_push.atom.haml index 0ffd2aa0b9..5d14def8f7 100644 --- a/app/views/events/_event_push.atom.haml +++ b/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 diff --git a/app/views/profiles/show.html.haml b/app/views/profiles/show.html.haml index e6b204451c..409b6b5a19 100644 --- a/app/views/profiles/show.html.haml +++ b/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" diff --git a/app/views/projects/issues/_issue_context.html.haml b/app/views/projects/issues/_issue_context.html.haml index 4c7654354f..cb4846a41d 100644 --- a/app/views/projects/issues/_issue_context.html.haml +++ b/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 diff --git a/app/views/projects/merge_requests/show/_context.html.haml b/app/views/projects/merge_requests/show/_context.html.haml index a74f3fb24e..753c7e0e61 100644 --- a/app/views/projects/merge_requests/show/_context.html.haml +++ b/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 diff --git a/config/routes.rb b/config/routes.rb index 9e3e687b03..7ac8533818 100644 --- a/config/routes.rb +++ b/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 diff --git a/db/migrate/20150313012111_create_subscriptions_table.rb b/db/migrate/20150313012111_create_subscriptions_table.rb new file mode 100644 index 0000000000..a1d4d9dedc --- /dev/null +++ b/db/migrate/20150313012111_create_subscriptions_table.rb @@ -0,0 +1,16 @@ +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 diff --git a/db/schema.rb b/db/schema.rb index 11204b4f65..fc3d6da360 100644 --- a/db/schema.rb +++ b/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" diff --git a/features/project/issues/issues.feature b/features/project/issues/issues.feature index 283979204d..b9031f6f32 100644 --- a/features/project/issues/issues.feature +++ b/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 diff --git a/features/project/merge_requests.feature b/features/project/merge_requests.feature index eae37a9ea6..f494e65c08 100644 --- a/features/project/merge_requests.feature +++ b/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 diff --git a/features/steps/project/issues/issues.rb b/features/steps/project/issues/issues.rb index 6d72c93ad1..e8ca3f7c17 100644 --- a/features/steps/project/issues/issues.rb +++ b/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 diff --git a/features/steps/project/merge_requests.rb b/features/steps/project/merge_requests.rb index e3f5c38363..17aaee32f8 100644 --- a/features/steps/project/merge_requests.rb +++ b/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 diff --git a/lib/gitlab/markdown.rb b/lib/gitlab/markdown.rb index c3a8d90ef5..e02e5b9fc3 100644 --- a/lib/gitlab/markdown.rb +++ b/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) - end - # 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_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 diff --git a/lib/redcarpet/render/gitlab_html.rb b/lib/redcarpet/render/gitlab_html.rb index 1cd3933e4b..10efff2ae9 100644 --- a/lib/redcarpet/render/gitlab_html.rb +++ b/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 diff --git a/spec/features/atom/users_spec.rb b/spec/features/atom/users_spec.rb index c0316b073a..770ac04c2c 100644 --- a/spec/features/atom/users_spec.rb +++ b/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
\n\n

I guess/ + end + + it 'should have XHTML summaries in notes' do + expect(body).to match /Bug confirmed ]*\/>/ + end + + it 'should have XHTML summaries in merge request descriptions' do + expect(body).to match /Here is the fix: ]*\/>/ + 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 diff --git a/spec/models/deploy_keys_project_spec.rb b/spec/models/deploy_keys_project_spec.rb index aacd9bf38b..f351aab923 100644 --- a/spec/models/deploy_keys_project_spec.rb +++ b/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 diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 34737348d4..bfca2c8826 100644 --- a/spec/services/notification_service_spec.rb +++ b/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