diff options
| -rw-r--r-- | app/controllers/dashboard/groups_controller.rb | 2 | ||||
| -rw-r--r-- | app/models/sent_notification.rb | 6 | ||||
| -rw-r--r-- | app/views/events/event/_push.html.haml | 3 | ||||
| -rw-r--r-- | changelogs/unreleased/23888-fix-unsubscription-link-for-snippet-notification.yml | 5 | ||||
| -rw-r--r-- | changelogs/unreleased/37483-activity-log-show-wrong-number-of-commits-per-push.yml | 5 | ||||
| -rw-r--r-- | changelogs/unreleased/group-sort-dropdown-blank.yml | 5 | ||||
| -rw-r--r-- | qa/qa.rb | 9 | ||||
| -rw-r--r-- | qa/qa/page/dashboard/groups.rb | 23 | ||||
| -rw-r--r-- | qa/qa/page/group/new.rb | 23 | ||||
| -rw-r--r-- | qa/qa/page/group/show.rb | 18 | ||||
| -rw-r--r-- | qa/qa/runtime/namespace.rb | 4 | ||||
| -rw-r--r-- | qa/qa/scenario/gitlab/group/create.rb | 27 | ||||
| -rw-r--r-- | qa/qa/scenario/gitlab/project/create.rb | 20 | ||||
| -rw-r--r-- | qa/qa/scenario/gitlab/sandbox/prepare.rb | 28 | ||||
| -rw-r--r-- | spec/features/dashboard/group_spec.rb | 6 | ||||
| -rw-r--r-- | spec/mailers/notify_spec.rb | 491 | ||||
| -rw-r--r-- | spec/models/sent_notification_spec.rb | 122 |
17 files changed, 546 insertions, 251 deletions
diff --git a/app/controllers/dashboard/groups_controller.rb b/app/controllers/dashboard/groups_controller.rb index 8057a0b455c..7ed18fb481c 100644 --- a/app/controllers/dashboard/groups_controller.rb +++ b/app/controllers/dashboard/groups_controller.rb @@ -1,6 +1,6 @@ class Dashboard::GroupsController < Dashboard::ApplicationController def index - @sort = params[:sort] || 'id_desc' + @sort = params[:sort] || 'created_desc' @groups = if params[:parent_id] && Group.supports_nested_groups? diff --git a/app/models/sent_notification.rb b/app/models/sent_notification.rb index 298569cb7a6..6e311806be1 100644 --- a/app/models/sent_notification.rb +++ b/app/models/sent_notification.rb @@ -53,13 +53,17 @@ class SentNotification < ActiveRecord::Base end def unsubscribable? - !for_commit? + !(for_commit? || for_snippet?) end def for_commit? noteable_type == "Commit" end + def for_snippet? + noteable_type.end_with?('Snippet') + end + def noteable if for_commit? project.commit(commit_id) rescue nil diff --git a/app/views/events/event/_push.html.haml b/app/views/events/event/_push.html.haml index 53ebdd6d2ff..9a763887b30 100644 --- a/app/views/events/event/_push.html.haml +++ b/app/views/events/event/_push.html.haml @@ -19,8 +19,7 @@ - create_mr = event.new_ref? && create_mr_button?(project.default_branch, event.ref_name, project) && event.authored_by?(current_user) - if event.commits_count > 1 %li.commits-stat - - if event.commits_count > 2 - %span ... and #{event.commits_count - 2} more commits. + %span ... and #{pluralize(event.commits_count - 1, 'more commit')}. - if event.md_ref? - from = event.commit_from diff --git a/changelogs/unreleased/23888-fix-unsubscription-link-for-snippet-notification.yml b/changelogs/unreleased/23888-fix-unsubscription-link-for-snippet-notification.yml new file mode 100644 index 00000000000..36bed037160 --- /dev/null +++ b/changelogs/unreleased/23888-fix-unsubscription-link-for-snippet-notification.yml @@ -0,0 +1,5 @@ +--- +title: Don't show an "Unsubscribe" link in snippet comment notifications +merge_request: 14764 +author: +type: fixed diff --git a/changelogs/unreleased/37483-activity-log-show-wrong-number-of-commits-per-push.yml b/changelogs/unreleased/37483-activity-log-show-wrong-number-of-commits-per-push.yml new file mode 100644 index 00000000000..225ab9acc44 --- /dev/null +++ b/changelogs/unreleased/37483-activity-log-show-wrong-number-of-commits-per-push.yml @@ -0,0 +1,5 @@ +--- +title: Fix the number representing the amount of commits related to a push event +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/group-sort-dropdown-blank.yml b/changelogs/unreleased/group-sort-dropdown-blank.yml new file mode 100644 index 00000000000..dd16892be4d --- /dev/null +++ b/changelogs/unreleased/group-sort-dropdown-blank.yml @@ -0,0 +1,5 @@ +--- +title: Fixed group sort dropdown defaulting to empty +merge_request: +author: +type: fixed @@ -31,9 +31,17 @@ module QA # GitLab instance scenarios. # module Gitlab + module Group + autoload :Create, 'qa/scenario/gitlab/group/create' + end + module Project autoload :Create, 'qa/scenario/gitlab/project/create' end + + module Sandbox + autoload :Prepare, 'qa/scenario/gitlab/sandbox/prepare' + end end end @@ -55,6 +63,7 @@ module QA end module Group + autoload :New, 'qa/page/group/new' autoload :Show, 'qa/page/group/show' end diff --git a/qa/qa/page/dashboard/groups.rb b/qa/qa/page/dashboard/groups.rb index 3690f40dcfe..083d2e1ab16 100644 --- a/qa/qa/page/dashboard/groups.rb +++ b/qa/qa/page/dashboard/groups.rb @@ -2,19 +2,22 @@ module QA module Page module Dashboard class Groups < Page::Base - def prepare_test_namespace - if page.has_content?(Runtime::Namespace.name) - return click_link(Runtime::Namespace.name) - end + def filter_by_name(name) + fill_in 'Filter by name...', with: name + end - click_on 'New group' + def has_group?(name) + filter_by_name(name) + + page.has_link?(name) + end - fill_in 'group_path', with: Runtime::Namespace.name - fill_in 'group_description', - with: "QA test run at #{Runtime::Namespace.time}" - choose 'Private' + def go_to_group(name) + click_link name + end - click_button 'Create group' + def go_to_new_group + click_on 'New group' end end end diff --git a/qa/qa/page/group/new.rb b/qa/qa/page/group/new.rb new file mode 100644 index 00000000000..cb743a7bf11 --- /dev/null +++ b/qa/qa/page/group/new.rb @@ -0,0 +1,23 @@ +module QA + module Page + module Group + class New < Page::Base + def set_path(path) + fill_in 'group_path', with: path + end + + def set_description(description) + fill_in 'group_description', with: description + end + + def set_visibility(visibility) + choose visibility + end + + def create + click_button 'Create group' + end + end + end + end +end diff --git a/qa/qa/page/group/show.rb b/qa/qa/page/group/show.rb index 296c311d7c6..6987c1f8f85 100644 --- a/qa/qa/page/group/show.rb +++ b/qa/qa/page/group/show.rb @@ -2,8 +2,24 @@ module QA module Page module Group class Show < Page::Base + def go_to_subgroups + click_link 'Subgroups' + end + + def go_to_subgroup(name) + click_link name + end + + def has_subgroup?(name) + page.has_link?(name) + end + + def go_to_new_subgroup + click_on 'New Subgroup' + end + def go_to_new_project - click_link 'New Project' + click_on 'New Project' end end end diff --git a/qa/qa/runtime/namespace.rb b/qa/qa/runtime/namespace.rb index e4910b63a14..996286430b9 100644 --- a/qa/qa/runtime/namespace.rb +++ b/qa/qa/runtime/namespace.rb @@ -10,6 +10,10 @@ module QA def name 'qa_test_' + time.strftime('%d_%m_%Y_%H-%M-%S') end + + def sandbox_name + 'gitlab-qa-sandbox' + end end end end diff --git a/qa/qa/scenario/gitlab/group/create.rb b/qa/qa/scenario/gitlab/group/create.rb new file mode 100644 index 00000000000..8e6c7c7ad80 --- /dev/null +++ b/qa/qa/scenario/gitlab/group/create.rb @@ -0,0 +1,27 @@ +require 'securerandom' + +module QA + module Scenario + module Gitlab + module Group + class Create < Scenario::Template + attr_writer :path, :description + + def initialize + @path = Runtime::Namespace.name + @description = "QA test run at #{Runtime::Namespace.time}" + end + + def perform + Page::Group::New.perform do |group| + group.set_path(@path) + group.set_description(@description) + group.set_visibility('Private') + group.create + end + end + end + end + end + end +end diff --git a/qa/qa/scenario/gitlab/project/create.rb b/qa/qa/scenario/gitlab/project/create.rb index b860701c304..7b614bfdd94 100644 --- a/qa/qa/scenario/gitlab/project/create.rb +++ b/qa/qa/scenario/gitlab/project/create.rb @@ -12,9 +12,23 @@ module QA end def perform - Page::Main::Menu.act { go_to_groups } - Page::Dashboard::Groups.act { prepare_test_namespace } - Page::Group::Show.act { go_to_new_project } + Scenario::Gitlab::Sandbox::Prepare.perform + + Page::Group::Show.perform do |page| + page.go_to_subgroups + + if page.has_subgroup?(Runtime::Namespace.name) + page.go_to_subgroup(Runtime::Namespace.name) + else + page.go_to_new_subgroup + + Scenario::Gitlab::Group::Create.perform do |group| + group.path = Runtime::Namespace.name + end + end + + page.go_to_new_project + end Page::Project::New.perform do |page| page.choose_test_namespace diff --git a/qa/qa/scenario/gitlab/sandbox/prepare.rb b/qa/qa/scenario/gitlab/sandbox/prepare.rb new file mode 100644 index 00000000000..990de456e20 --- /dev/null +++ b/qa/qa/scenario/gitlab/sandbox/prepare.rb @@ -0,0 +1,28 @@ +module QA + module Scenario + module Gitlab + module Sandbox + # Ensure we're in our sandbox namespace, either by navigating to it or + # by creating it if it doesn't yet exist + class Prepare < Scenario::Template + def perform + Page::Main::Menu.act { go_to_groups } + + Page::Dashboard::Groups.perform do |page| + if page.has_group?(Runtime::Namespace.sandbox_name) + page.go_to_group(Runtime::Namespace.sandbox_name) + else + page.go_to_new_group + + Scenario::Gitlab::Group::Create.perform do |group| + group.path = Runtime::Namespace.sandbox_name + group.description = 'QA sandbox' + end + end + end + end + end + end + end + end +end diff --git a/spec/features/dashboard/group_spec.rb b/spec/features/dashboard/group_spec.rb index 970173c7aaf..1213f8c32eb 100644 --- a/spec/features/dashboard/group_spec.rb +++ b/spec/features/dashboard/group_spec.rb @@ -5,6 +5,12 @@ RSpec.describe 'Dashboard Group' do sign_in(create(:user)) end + it 'defaults sort dropdown to last created' do + visit dashboard_groups_path + + expect(page).to have_button('Last created') + end + it 'creates new group', :js do visit dashboard_groups_path find('.btn-new').trigger('click') diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index 932e2fd8c95..c832cee965b 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -28,319 +28,334 @@ describe Notify do end def have_referable_subject(referable, reply: false) - prefix = referable.project.name if referable.project - prefix = "Re: #{prefix}" if reply + prefix = referable.project ? "#{referable.project.name} | " : '' + prefix.prepend('Re: ') if reply suffix = "#{referable.title} (#{referable.to_reference})" - have_subject [prefix, suffix].compact.join(' | ') + have_subject [prefix, suffix].compact.join end context 'for a project' do - describe 'items that are assignable, the email' do - let(:previous_assignee) { create(:user, name: 'Previous Assignee') } + shared_examples 'an assignee email' do + it 'is sent to the assignee as the author' do + sender = subject.header[:from].addrs.first + + aggregate_failures do + expect(sender.display_name).to eq(current_user.name) + expect(sender.address).to eq(gitlab_sender) + expect(subject).to deliver_to(assignee.email) + end + end + end - shared_examples 'an assignee email' do - it 'is sent to the assignee as the author' do - sender = subject.header[:from].addrs.first + context 'for issues' do + describe 'that are new' do + subject { described_class.new_issue_email(issue.assignees.first.id, issue.id) } + it_behaves_like 'an assignee email' + it_behaves_like 'an email starting a new thread with reply-by-email enabled' do + let(:model) { issue } + end + it_behaves_like 'it should show Gmail Actions View Issue link' + it_behaves_like 'an unsubscribeable thread' + + it 'has the correct subject and body' do aggregate_failures do - expect(sender.display_name).to eq(current_user.name) - expect(sender.address).to eq(gitlab_sender) - expect(subject).to deliver_to(assignee.email) + is_expected.to have_referable_subject(issue) + is_expected.to have_body_text(project_issue_path(project, issue)) end end - end - context 'for issues' do - describe 'that are new' do - subject { described_class.new_issue_email(issue.assignees.first.id, issue.id) } + it 'contains the description' do + is_expected.to have_html_escaped_body_text issue.description + end - it_behaves_like 'an assignee email' - it_behaves_like 'an email starting a new thread with reply-by-email enabled' do - let(:model) { issue } - end - it_behaves_like 'it should show Gmail Actions View Issue link' - it_behaves_like 'an unsubscribeable thread' - - it 'has the correct subject and body' do - aggregate_failures do - is_expected.to have_referable_subject(issue) - is_expected.to have_body_text(project_issue_path(project, issue)) - end + context 'when enabled email_author_in_body' do + before do + stub_application_setting(email_author_in_body: true) end - it 'contains the description' do - is_expected.to have_html_escaped_body_text issue.description + it 'contains a link to note author' do + is_expected.to have_html_escaped_body_text(issue.author_name) + is_expected.to have_body_text 'created an issue:' end + end + end - context 'when enabled email_author_in_body' do - before do - stub_application_setting(email_author_in_body: true) - end + describe 'that are reassigned' do + let(:previous_assignee) { create(:user, name: 'Previous Assignee') } + subject { described_class.reassigned_issue_email(recipient.id, issue.id, [previous_assignee.id], current_user.id) } - it 'contains a link to note author' do - is_expected.to have_html_escaped_body_text(issue.author_name) - is_expected.to have_body_text 'created an issue:' - end - end + it_behaves_like 'a multiple recipients email' + it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do + let(:model) { issue } end + it_behaves_like 'it should show Gmail Actions View Issue link' + it_behaves_like 'an unsubscribeable thread' - describe 'that have been reassigned' do - subject { described_class.reassigned_issue_email(recipient.id, issue.id, [previous_assignee.id], current_user.id) } + it 'is sent as the author' do + sender = subject.header[:from].addrs[0] + expect(sender.display_name).to eq(current_user.name) + expect(sender.address).to eq(gitlab_sender) + end - it_behaves_like 'a multiple recipients email' - it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do - let(:model) { issue } + it 'has the correct subject and body' do + aggregate_failures do + is_expected.to have_referable_subject(issue, reply: true) + is_expected.to have_html_escaped_body_text(previous_assignee.name) + is_expected.to have_html_escaped_body_text(assignee.name) + is_expected.to have_body_text(project_issue_path(project, issue)) end - it_behaves_like 'it should show Gmail Actions View Issue link' - it_behaves_like 'an unsubscribeable thread' + end + end - it 'is sent as the author' do - sender = subject.header[:from].addrs[0] - expect(sender.display_name).to eq(current_user.name) - expect(sender.address).to eq(gitlab_sender) - end + describe 'that have been relabeled' do + subject { described_class.relabeled_issue_email(recipient.id, issue.id, %w[foo bar baz], current_user.id) } - it 'has the correct subject and body' do - aggregate_failures do - is_expected.to have_referable_subject(issue, reply: true) - is_expected.to have_html_escaped_body_text(previous_assignee.name) - is_expected.to have_html_escaped_body_text(assignee.name) - is_expected.to have_body_text(project_issue_path(project, issue)) - end - end + it_behaves_like 'a multiple recipients email' + it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do + let(:model) { issue } end + it_behaves_like 'it should show Gmail Actions View Issue link' + it_behaves_like 'a user cannot unsubscribe through footer link' + it_behaves_like 'an email with a labels subscriptions link in its footer' - describe 'that have been relabeled' do - subject { described_class.relabeled_issue_email(recipient.id, issue.id, %w[foo bar baz], current_user.id) } + it 'is sent as the author' do + sender = subject.header[:from].addrs[0] + expect(sender.display_name).to eq(current_user.name) + expect(sender.address).to eq(gitlab_sender) + end - it_behaves_like 'a multiple recipients email' - it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do - let(:model) { issue } + it 'has the correct subject and body' do + aggregate_failures do + is_expected.to have_referable_subject(issue, reply: true) + is_expected.to have_body_text('foo, bar, and baz') + is_expected.to have_body_text(project_issue_path(project, issue)) end - it_behaves_like 'it should show Gmail Actions View Issue link' - it_behaves_like 'a user cannot unsubscribe through footer link' - it_behaves_like 'an email with a labels subscriptions link in its footer' + end - it 'is sent as the author' do - sender = subject.header[:from].addrs[0] - expect(sender.display_name).to eq(current_user.name) - expect(sender.address).to eq(gitlab_sender) + context 'with a preferred language' do + before do + Gitlab::I18n.locale = :es end - it 'has the correct subject and body' do - aggregate_failures do - is_expected.to have_referable_subject(issue, reply: true) - is_expected.to have_body_text('foo, bar, and baz') - is_expected.to have_body_text(project_issue_path(project, issue)) - end + after do + Gitlab::I18n.use_default_locale end - context 'with a preferred language' do - before do - Gitlab::I18n.locale = :es - end - - after do - Gitlab::I18n.use_default_locale - end - - it 'always generates the email using the default language' do - is_expected.to have_body_text('foo, bar, and baz') - end + it 'always generates the email using the default language' do + is_expected.to have_body_text('foo, bar, and baz') end end + end - describe 'status changed' do - let(:status) { 'closed' } - subject { described_class.issue_status_changed_email(recipient.id, issue.id, status, current_user.id) } + describe 'status changed' do + let(:status) { 'closed' } + subject { described_class.issue_status_changed_email(recipient.id, issue.id, status, current_user.id) } - it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do - let(:model) { issue } - end - it_behaves_like 'it should show Gmail Actions View Issue link' - it_behaves_like 'an unsubscribeable thread' + it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do + let(:model) { issue } + end + it_behaves_like 'it should show Gmail Actions View Issue link' + it_behaves_like 'an unsubscribeable thread' - it 'is sent as the author' do - sender = subject.header[:from].addrs[0] - expect(sender.display_name).to eq(current_user.name) - expect(sender.address).to eq(gitlab_sender) - end + it 'is sent as the author' do + sender = subject.header[:from].addrs[0] + expect(sender.display_name).to eq(current_user.name) + expect(sender.address).to eq(gitlab_sender) + end - it 'has the correct subject and body' do - aggregate_failures do - is_expected.to have_referable_subject(issue, reply: true) - is_expected.to have_body_text(status) - is_expected.to have_html_escaped_body_text(current_user.name) - is_expected.to have_body_text(project_issue_path project, issue) - end + it 'has the correct subject and body' do + aggregate_failures do + is_expected.to have_referable_subject(issue, reply: true) + is_expected.to have_body_text(status) + is_expected.to have_html_escaped_body_text(current_user.name) + is_expected.to have_body_text(project_issue_path project, issue) end end + end - describe 'moved to another project' do - let(:new_issue) { create(:issue) } - subject { described_class.issue_moved_email(recipient, issue, new_issue, current_user) } + describe 'moved to another project' do + let(:new_issue) { create(:issue) } + subject { described_class.issue_moved_email(recipient, issue, new_issue, current_user) } - it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do - let(:model) { issue } - end - it_behaves_like 'it should show Gmail Actions View Issue link' - it_behaves_like 'an unsubscribeable thread' + it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do + let(:model) { issue } + end + it_behaves_like 'it should show Gmail Actions View Issue link' + it_behaves_like 'an unsubscribeable thread' - it 'contains description about action taken' do - is_expected.to have_body_text 'Issue was moved to another project' - end + it 'contains description about action taken' do + is_expected.to have_body_text 'Issue was moved to another project' + end - it 'has the correct subject and body' do - new_issue_url = project_issue_path(new_issue.project, new_issue) + it 'has the correct subject and body' do + new_issue_url = project_issue_path(new_issue.project, new_issue) - aggregate_failures do - is_expected.to have_referable_subject(issue, reply: true) - is_expected.to have_body_text(new_issue_url) - is_expected.to have_body_text(project_issue_path(project, issue)) - end + aggregate_failures do + is_expected.to have_referable_subject(issue, reply: true) + is_expected.to have_body_text(new_issue_url) + is_expected.to have_body_text(project_issue_path(project, issue)) end end end + end - context 'for merge requests' do - describe 'that are new' do - subject { described_class.new_merge_request_email(merge_request.assignee_id, merge_request.id) } + context 'for merge requests' do + describe 'that are new' do + subject { described_class.new_merge_request_email(merge_request.assignee_id, merge_request.id) } + + it_behaves_like 'an assignee email' + it_behaves_like 'an email starting a new thread with reply-by-email enabled' do + let(:model) { merge_request } + end + it_behaves_like 'it should show Gmail Actions View Merge request link' + it_behaves_like 'an unsubscribeable thread' - it_behaves_like 'an assignee email' - it_behaves_like 'an email starting a new thread with reply-by-email enabled' do - let(:model) { merge_request } + it 'has the correct subject and body' do + aggregate_failures do + is_expected.to have_referable_subject(merge_request) + is_expected.to have_body_text(project_merge_request_path(project, merge_request)) + is_expected.to have_body_text(merge_request.source_branch) + is_expected.to have_body_text(merge_request.target_branch) end - it_behaves_like 'it should show Gmail Actions View Merge request link' - it_behaves_like 'an unsubscribeable thread' - - it 'has the correct subject and body' do - aggregate_failures do - is_expected.to have_referable_subject(merge_request) - is_expected.to have_body_text(project_merge_request_path(project, merge_request)) - is_expected.to have_body_text(merge_request.source_branch) - is_expected.to have_body_text(merge_request.target_branch) - end + end + + it 'contains the description' do + is_expected.to have_html_escaped_body_text merge_request.description + end + + context 'when enabled email_author_in_body' do + before do + stub_application_setting(email_author_in_body: true) end - it 'contains the description' do - is_expected.to have_html_escaped_body_text merge_request.description + it 'contains a link to note author' do + is_expected.to have_html_escaped_body_text merge_request.author_name + is_expected.to have_body_text 'created a merge request:' end + end + end - context 'when enabled email_author_in_body' do - before do - stub_application_setting(email_author_in_body: true) - end + describe 'that are reassigned' do + let(:previous_assignee) { create(:user, name: 'Previous Assignee') } + subject { described_class.reassigned_merge_request_email(recipient.id, merge_request.id, previous_assignee.id, current_user.id) } - it 'contains a link to note author' do - is_expected.to have_html_escaped_body_text merge_request.author_name - is_expected.to have_body_text 'created a merge request:' - end - end + it_behaves_like 'a multiple recipients email' + it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do + let(:model) { merge_request } end + it_behaves_like 'it should show Gmail Actions View Merge request link' + it_behaves_like "an unsubscribeable thread" - describe 'that are reassigned' do - subject { described_class.reassigned_merge_request_email(recipient.id, merge_request.id, previous_assignee.id, current_user.id) } + it 'is sent as the author' do + sender = subject.header[:from].addrs[0] + expect(sender.display_name).to eq(current_user.name) + expect(sender.address).to eq(gitlab_sender) + end - it_behaves_like 'a multiple recipients email' - it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do - let(:model) { merge_request } + it 'has the correct subject and body' do + aggregate_failures do + is_expected.to have_referable_subject(merge_request, reply: true) + is_expected.to have_html_escaped_body_text(previous_assignee.name) + is_expected.to have_body_text(project_merge_request_path(project, merge_request)) + is_expected.to have_html_escaped_body_text(assignee.name) end - it_behaves_like 'it should show Gmail Actions View Merge request link' - it_behaves_like "an unsubscribeable thread" + end + end - it 'is sent as the author' do - sender = subject.header[:from].addrs[0] - expect(sender.display_name).to eq(current_user.name) - expect(sender.address).to eq(gitlab_sender) - end + describe 'that have been relabeled' do + subject { described_class.relabeled_merge_request_email(recipient.id, merge_request.id, %w[foo bar baz], current_user.id) } - it 'has the correct subject and body' do - aggregate_failures do - is_expected.to have_referable_subject(merge_request, reply: true) - is_expected.to have_html_escaped_body_text(previous_assignee.name) - is_expected.to have_body_text(project_merge_request_path(project, merge_request)) - is_expected.to have_html_escaped_body_text(assignee.name) - end - end + it_behaves_like 'a multiple recipients email' + it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do + let(:model) { merge_request } end + it_behaves_like 'it should show Gmail Actions View Merge request link' + it_behaves_like 'a user cannot unsubscribe through footer link' + it_behaves_like 'an email with a labels subscriptions link in its footer' - describe 'that have been relabeled' do - subject { described_class.relabeled_merge_request_email(recipient.id, merge_request.id, %w[foo bar baz], current_user.id) } + it 'is sent as the author' do + sender = subject.header[:from].addrs[0] + expect(sender.display_name).to eq(current_user.name) + expect(sender.address).to eq(gitlab_sender) + end - it_behaves_like 'a multiple recipients email' - it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do - let(:model) { merge_request } - end - it_behaves_like 'it should show Gmail Actions View Merge request link' - it_behaves_like 'a user cannot unsubscribe through footer link' - it_behaves_like 'an email with a labels subscriptions link in its footer' + it 'has the correct subject and body' do + is_expected.to have_referable_subject(merge_request, reply: true) + is_expected.to have_body_text('foo, bar, and baz') + is_expected.to have_body_text(project_merge_request_path(project, merge_request)) + end + end - it 'is sent as the author' do - sender = subject.header[:from].addrs[0] - expect(sender.display_name).to eq(current_user.name) - expect(sender.address).to eq(gitlab_sender) - end + describe 'status changed' do + let(:status) { 'reopened' } + subject { described_class.merge_request_status_email(recipient.id, merge_request.id, status, current_user.id) } - it 'has the correct subject and body' do + it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do + let(:model) { merge_request } + end + it_behaves_like 'it should show Gmail Actions View Merge request link' + it_behaves_like 'an unsubscribeable thread' + + it 'is sent as the author' do + sender = subject.header[:from].addrs[0] + expect(sender.display_name).to eq(current_user.name) + expect(sender.address).to eq(gitlab_sender) + end + + it 'has the correct subject and body' do + aggregate_failures do is_expected.to have_referable_subject(merge_request, reply: true) - is_expected.to have_body_text('foo, bar, and baz') + is_expected.to have_body_text(status) + is_expected.to have_html_escaped_body_text(current_user.name) is_expected.to have_body_text(project_merge_request_path(project, merge_request)) end end + end - describe 'status changed' do - let(:status) { 'reopened' } - subject { described_class.merge_request_status_email(recipient.id, merge_request.id, status, current_user.id) } + describe 'that are merged' do + let(:merge_author) { create(:user) } + subject { described_class.merged_merge_request_email(recipient.id, merge_request.id, merge_author.id) } - it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do - let(:model) { merge_request } - end - it_behaves_like 'it should show Gmail Actions View Merge request link' - it_behaves_like 'an unsubscribeable thread' + it_behaves_like 'a multiple recipients email' + it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do + let(:model) { merge_request } + end + it_behaves_like 'it should show Gmail Actions View Merge request link' + it_behaves_like 'an unsubscribeable thread' - it 'is sent as the author' do - sender = subject.header[:from].addrs[0] - expect(sender.display_name).to eq(current_user.name) - expect(sender.address).to eq(gitlab_sender) - end + it 'is sent as the merge author' do + sender = subject.header[:from].addrs[0] + expect(sender.display_name).to eq(merge_author.name) + expect(sender.address).to eq(gitlab_sender) + end - it 'has the correct subject and body' do - aggregate_failures do - is_expected.to have_referable_subject(merge_request, reply: true) - is_expected.to have_body_text(status) - is_expected.to have_html_escaped_body_text(current_user.name) - is_expected.to have_body_text(project_merge_request_path(project, merge_request)) - end + it 'has the correct subject and body' do + aggregate_failures do + is_expected.to have_referable_subject(merge_request, reply: true) + is_expected.to have_body_text('merged') + is_expected.to have_body_text(project_merge_request_path(project, merge_request)) end end + end + end - describe 'that are merged' do - let(:merge_author) { create(:user) } - subject { described_class.merged_merge_request_email(recipient.id, merge_request.id, merge_author.id) } + context 'for snippet notes' do + let(:project_snippet) { create(:project_snippet, project: project) } + let(:project_snippet_note) { create(:note_on_project_snippet, project: project, noteable: project_snippet) } - it_behaves_like 'a multiple recipients email' - it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do - let(:model) { merge_request } - end - it_behaves_like 'it should show Gmail Actions View Merge request link' - it_behaves_like 'an unsubscribeable thread' + subject { described_class.note_snippet_email(project_snippet_note.author_id, project_snippet_note.id) } - it 'is sent as the merge author' do - sender = subject.header[:from].addrs[0] - expect(sender.display_name).to eq(merge_author.name) - expect(sender.address).to eq(gitlab_sender) - end + it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do + let(:model) { project_snippet } + end + it_behaves_like 'a user cannot unsubscribe through footer link' - it 'has the correct subject and body' do - aggregate_failures do - is_expected.to have_referable_subject(merge_request, reply: true) - is_expected.to have_body_text('merged') - is_expected.to have_body_text(project_merge_request_path(project, merge_request)) - end - end - end + it 'has the correct subject and body' do + is_expected.to have_referable_subject(project_snippet, reply: true) + is_expected.to have_html_escaped_body_text project_snippet_note.note end end @@ -1239,4 +1254,18 @@ describe Notify do end end end + + context 'for personal snippet notes' do + let(:personal_snippet) { create(:personal_snippet) } + let(:personal_snippet_note) { create(:note_on_personal_snippet, noteable: personal_snippet) } + + subject { described_class.note_personal_snippet_email(personal_snippet_note.author_id, personal_snippet_note.id) } + + it_behaves_like 'a user cannot unsubscribe through footer link' + + it 'has the correct subject and body' do + is_expected.to have_referable_subject(personal_snippet, reply: true) + is_expected.to have_html_escaped_body_text personal_snippet_note.note + end + end end diff --git a/spec/models/sent_notification_spec.rb b/spec/models/sent_notification_spec.rb index 8f05deb8b15..5ec04b99957 100644 --- a/spec/models/sent_notification_spec.rb +++ b/spec/models/sent_notification_spec.rb @@ -1,6 +1,9 @@ require 'spec_helper' describe SentNotification do + set(:user) { create(:user) } + set(:project) { create(:project) } + describe 'validation' do describe 'note validity' do context "when the project doesn't match the noteable's project" do @@ -34,7 +37,6 @@ describe SentNotification do end describe '.record' do - let(:user) { create(:user) } let(:issue) { create(:issue) } it 'creates a new SentNotification' do @@ -43,7 +45,6 @@ describe SentNotification do end describe '.record_note' do - let(:user) { create(:user) } let(:note) { create(:diff_note_on_merge_request) } it 'creates a new SentNotification' do @@ -51,6 +52,123 @@ describe SentNotification do end end + describe '#unsubscribable?' do + shared_examples 'an unsubscribable notification' do |noteable_type| + subject { described_class.record(noteable, user.id) } + + context "for #{noteable_type}" do + it { expect(subject).to be_unsubscribable } + end + end + + shared_examples 'a non-unsubscribable notification' do |noteable_type| + subject { described_class.record(noteable, user.id) } + + context "for a #{noteable_type}" do + it { expect(subject).not_to be_unsubscribable } + end + end + + it_behaves_like 'an unsubscribable notification', 'issue' do + let(:noteable) { create(:issue, project: project) } + end + + it_behaves_like 'an unsubscribable notification', 'merge request' do + let(:noteable) { create(:merge_request, source_project: project) } + end + + it_behaves_like 'a non-unsubscribable notification', 'commit' do + let(:project) { create(:project, :repository) } + let(:noteable) { project.commit } + end + + it_behaves_like 'a non-unsubscribable notification', 'personal snippet' do + let(:noteable) { create(:personal_snippet, project: project) } + end + + it_behaves_like 'a non-unsubscribable notification', 'project snippet' do + let(:noteable) { create(:project_snippet, project: project) } + end + end + + describe '#for_commit?' do + shared_examples 'a commit notification' do |noteable_type| + subject { described_class.record(noteable, user.id) } + + context "for #{noteable_type}" do + it { expect(subject).to be_for_commit } + end + end + + shared_examples 'a non-commit notification' do |noteable_type| + subject { described_class.record(noteable, user.id) } + + context "for a #{noteable_type}" do + it { expect(subject).not_to be_for_commit } + end + end + + it_behaves_like 'a non-commit notification', 'issue' do + let(:noteable) { create(:issue, project: project) } + end + + it_behaves_like 'a non-commit notification', 'merge request' do + let(:noteable) { create(:merge_request, source_project: project) } + end + + it_behaves_like 'a commit notification', 'commit' do + let(:project) { create(:project, :repository) } + let(:noteable) { project.commit } + end + + it_behaves_like 'a non-commit notification', 'personal snippet' do + let(:noteable) { create(:personal_snippet, project: project) } + end + + it_behaves_like 'a non-commit notification', 'project snippet' do + let(:noteable) { create(:project_snippet, project: project) } + end + end + + describe '#for_snippet?' do + shared_examples 'a snippet notification' do |noteable_type| + subject { described_class.record(noteable, user.id) } + + context "for #{noteable_type}" do + it { expect(subject).to be_for_snippet } + end + end + + shared_examples 'a non-snippet notification' do |noteable_type| + subject { described_class.record(noteable, user.id) } + + context "for a #{noteable_type}" do + it { expect(subject).not_to be_for_snippet } + end + end + + it_behaves_like 'a non-snippet notification', 'issue' do + let(:noteable) { create(:issue, project: project) } + end + + it_behaves_like 'a non-snippet notification', 'merge request' do + let(:noteable) { create(:merge_request, source_project: project) } + end + + it_behaves_like 'a non-snippet notification', 'commit' do + let(:project) { create(:project, :repository) } + let(:noteable) { project.commit } + end + + it_behaves_like 'a snippet notification', 'personal snippet' do + let(:noteable) { create(:personal_snippet, project: project) } + end + + it_behaves_like 'a snippet notification', 'project snippet' do + let(:noteable) { create(:project_snippet, project: project) } + end + end + describe '#create_reply' do context 'for issue' do let(:issue) { create(:issue) } |