From adab5dba43660a22118ea77038ef03fe0ded19f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Tue, 27 Sep 2016 11:29:06 +0200 Subject: Fix permission for setting an issue's due date MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- CHANGELOG | 1 + app/services/issuable_base_service.rb | 1 + app/services/slash_commands/interpret_service.rb | 4 +- .../issues/user_uses_slash_commands_spec.rb | 72 +++++- spec/services/issues/create_service_spec.rb | 36 ++- spec/services/issues/update_service_spec.rb | 97 +++++--- .../slash_commands/interpret_service_spec.rb | 71 +++++- .../issuable_slash_commands_shared_examples.rb | 261 +++++++++++++++++++++ ...reate_service_slash_commands_shared_examples.rb | 83 ------- .../issuable_slash_commands_shared_examples.rb | 261 --------------------- ...reate_service_slash_commands_shared_examples.rb | 83 +++++++ 11 files changed, 560 insertions(+), 410 deletions(-) create mode 100644 spec/support/features/issuable_slash_commands_shared_examples.rb delete mode 100644 spec/support/issuable_create_service_slash_commands_shared_examples.rb delete mode 100644 spec/support/issuable_slash_commands_shared_examples.rb create mode 100644 spec/support/services/issuable_create_service_slash_commands_shared_examples.rb diff --git a/CHANGELOG b/CHANGELOG index f92fcff951a..4512d7d0e75 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -7,6 +7,7 @@ v 8.13.0 (unreleased) - Log LDAP lookup errors and don't swallow unrelated exceptions. !6103 (Markus Koller) - Add more tests for calendar contribution (ClemMakesApps) - Avoid database queries on Banzai::ReferenceParser::BaseParser for nodes without references + - Fix permission for setting an issue's due date - Fix robots.txt disallowing access to groups starting with "s" (Matt Harrison) - Only update issuable labels if they have been changed - Revoke button in Applications Settings underlines on hover. diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index fbce46769f7..57d521f2fea 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -50,6 +50,7 @@ class IssuableBaseService < BaseService params.delete(:remove_label_ids) params.delete(:label_ids) params.delete(:assignee_id) + params.delete(:due_date) end end diff --git a/app/services/slash_commands/interpret_service.rb b/app/services/slash_commands/interpret_service.rb index 9ac1124abc1..ffcad5b3a87 100644 --- a/app/services/slash_commands/interpret_service.rb +++ b/app/services/slash_commands/interpret_service.rb @@ -195,7 +195,7 @@ module SlashCommands params '' condition do issuable.respond_to?(:due_date) && - current_user.can?(:"update_#{issuable.to_ability_name}", issuable) + current_user.can?(:"admin_#{issuable.to_ability_name}", project) end command :due do |due_date_param| due_date = Chronic.parse(due_date_param).try(:to_date) @@ -208,7 +208,7 @@ module SlashCommands issuable.persisted? && issuable.respond_to?(:due_date) && issuable.due_date? && - current_user.can?(:"update_#{issuable.to_ability_name}", issuable) + current_user.can?(:"admin_#{issuable.to_ability_name}", project) end command :remove_due_date do @updates[:due_date] = nil diff --git a/spec/features/issues/user_uses_slash_commands_spec.rb b/spec/features/issues/user_uses_slash_commands_spec.rb index 105629c485a..bf2b93c92fb 100644 --- a/spec/features/issues/user_uses_slash_commands_spec.rb +++ b/spec/features/issues/user_uses_slash_commands_spec.rb @@ -25,32 +25,78 @@ feature 'Issues > User uses slash commands', feature: true, js: true do describe 'adding a due date from note' do let(:issue) { create(:issue, project: project) } - it 'does not create a note, and sets the due date accordingly' do - write_note("/due 2016-08-28") + context 'when the current user can update the due date' do + it 'does not create a note, and sets the due date accordingly' do + write_note("/due 2016-08-28") - expect(page).not_to have_content '/due 2016-08-28' - expect(page).to have_content 'Your commands have been executed!' + expect(page).not_to have_content '/due 2016-08-28' + expect(page).to have_content 'Your commands have been executed!' - issue.reload + issue.reload - expect(issue.due_date).to eq Date.new(2016, 8, 28) + expect(issue.due_date).to eq Date.new(2016, 8, 28) + end + end + + context 'when the current user cannot update the due date' do + let(:guest) { create(:user) } + before do + project.team << [guest, :guest] + logout + login_with(guest) + visit namespace_project_issue_path(project.namespace, project, issue) + end + + it 'does not create a note, and sets the due date accordingly' do + write_note("/due 2016-08-28") + + expect(page).to have_content '/due 2016-08-28' + expect(page).not_to have_content 'Your commands have been executed!' + + issue.reload + + expect(issue.due_date).to be_nil + end end end describe 'removing a due date from note' do let(:issue) { create(:issue, project: project, due_date: Date.new(2016, 8, 28)) } - it 'does not create a note, and removes the due date accordingly' do - expect(issue.due_date).to eq Date.new(2016, 8, 28) + context 'when the current user can update the due date' do + it 'does not create a note, and removes the due date accordingly' do + expect(issue.due_date).to eq Date.new(2016, 8, 28) + + write_note("/remove_due_date") + + expect(page).not_to have_content '/remove_due_date' + expect(page).to have_content 'Your commands have been executed!' + + issue.reload + + expect(issue.due_date).to be_nil + end + end + + context 'when the current user cannot update the due date' do + let(:guest) { create(:user) } + before do + project.team << [guest, :guest] + logout + login_with(guest) + visit namespace_project_issue_path(project.namespace, project, issue) + end - write_note("/remove_due_date") + it 'does not create a note, and sets the due date accordingly' do + write_note("/remove_due_date") - expect(page).not_to have_content '/remove_due_date' - expect(page).to have_content 'Your commands have been executed!' + expect(page).to have_content '/remove_due_date' + expect(page).not_to have_content 'Your commands have been executed!' - issue.reload + issue.reload - expect(issue.due_date).to be_nil + expect(issue.due_date).to eq Date.new(2016, 8, 28) + end end end end diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb index 58569ba96c3..1050502fa19 100644 --- a/spec/services/issues/create_service_spec.rb +++ b/spec/services/issues/create_service_spec.rb @@ -20,16 +20,38 @@ describe Issues::CreateService, services: true do let(:opts) do { title: 'Awesome issue', description: 'please fix', - assignee: assignee, + assignee_id: assignee.id, label_ids: labels.map(&:id), - milestone_id: milestone.id } + milestone_id: milestone.id, + due_date: Date.tomorrow } end - it { expect(issue).to be_valid } - it { expect(issue.title).to eq('Awesome issue') } - it { expect(issue.assignee).to eq assignee } - it { expect(issue.labels).to match_array labels } - it { expect(issue.milestone).to eq milestone } + it 'creates the issue with the given params' do + expect(issue).to be_persisted + expect(issue.title).to eq('Awesome issue') + expect(issue.assignee).to eq assignee + expect(issue.labels).to match_array labels + expect(issue.milestone).to eq milestone + expect(issue.due_date).to eq Date.tomorrow + end + + context 'when current user cannot admin issues in the project' do + let(:guest) { create(:user) } + before do + project.team << [guest, :guest] + end + + it 'filters out params that cannot be set without the :admin_issue permission' do + issue = described_class.new(project, guest, opts).execute + + expect(issue).to be_persisted + expect(issue.title).to eq('Awesome issue') + expect(issue.assignee).to be_nil + expect(issue.labels).to be_empty + expect(issue.milestone).to be_nil + expect(issue.due_date).to be_nil + end + end it 'creates a pending todo for new assignee' do attributes = { diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb index 4f5375a3583..1638a46ed51 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -32,55 +32,84 @@ describe Issues::UpdateService, services: true do described_class.new(project, user, opts).execute(issue) end - context "valid params" do - before do - opts = { + context 'valid params' do + let(:opts) do + { title: 'New title', description: 'Also please fix', assignee_id: user2.id, state_event: 'close', - label_ids: [label.id] + label_ids: [label.id], + due_date: Date.tomorrow } - - perform_enqueued_jobs do - update_issue(opts) - end end - it { expect(issue).to be_valid } - it { expect(issue.title).to eq('New title') } - it { expect(issue.assignee).to eq(user2) } - it { expect(issue).to be_closed } - it { expect(issue.labels.count).to eq(1) } - it { expect(issue.labels.first.title).to eq(label.name) } - - it 'sends email to user2 about assign of new issue and email to user3 about issue unassignment' do - deliveries = ActionMailer::Base.deliveries - email = deliveries.last - recipients = deliveries.last(2).map(&:to).flatten - expect(recipients).to include(user2.email, user3.email) - expect(email.subject).to include(issue.title) + it 'updates the issue with the given params' do + update_issue(opts) + + expect(issue).to be_valid + expect(issue.title).to eq 'New title' + expect(issue.description).to eq 'Also please fix' + expect(issue.assignee).to eq user2 + expect(issue).to be_closed + expect(issue.labels).to match_array [label] + expect(issue.due_date).to eq Date.tomorrow end - it 'creates system note about issue reassign' do - note = find_note('Reassigned to') + context 'when current user cannot admin issues in the project' do + let(:guest) { create(:user) } + before do + project.team << [guest, :guest] + end - expect(note).not_to be_nil - expect(note.note).to include "Reassigned to \@#{user2.username}" + it 'filters out params that cannot be set without the :admin_issue permission' do + described_class.new(project, guest, opts).execute(issue) + + expect(issue).to be_valid + expect(issue.title).to eq 'New title' + expect(issue.description).to eq 'Also please fix' + expect(issue.assignee).to eq user3 + expect(issue.labels).to be_empty + expect(issue.milestone).to be_nil + expect(issue.due_date).to be_nil + end end - it 'creates system note about issue label edit' do - note = find_note('Added ~') + context 'with background jobs processed' do + before do + perform_enqueued_jobs do + update_issue(opts) + end + end + + it 'sends email to user2 about assign of new issue and email to user3 about issue unassignment' do + deliveries = ActionMailer::Base.deliveries + email = deliveries.last + recipients = deliveries.last(2).map(&:to).flatten + expect(recipients).to include(user2.email, user3.email) + expect(email.subject).to include(issue.title) + end - expect(note).not_to be_nil - expect(note.note).to include "Added ~#{label.id} label" - end + it 'creates system note about issue reassign' do + note = find_note('Reassigned to') - it 'creates system note about title change' do - note = find_note('Changed title:') + expect(note).not_to be_nil + expect(note.note).to include "Reassigned to \@#{user2.username}" + end - expect(note).not_to be_nil - expect(note.note).to eq 'Changed title: **{-Old-} title** → **{+New+} title**' + it 'creates system note about issue label edit' do + note = find_note('Added ~') + + expect(note).not_to be_nil + expect(note.note).to include "Added ~#{label.id} label" + end + + it 'creates system note about title change' do + note = find_note('Changed title:') + + expect(note).not_to be_nil + expect(note.note).to eq 'Changed title: **{-Old-} title** → **{+New+} title**' + end end end diff --git a/spec/services/slash_commands/interpret_service_spec.rb b/spec/services/slash_commands/interpret_service_spec.rb index a616275e883..5b1edba87a1 100644 --- a/spec/services/slash_commands/interpret_service_spec.rb +++ b/spec/services/slash_commands/interpret_service_spec.rb @@ -1,19 +1,19 @@ require 'spec_helper' describe SlashCommands::InterpretService, services: true do - let(:project) { create(:project) } - let(:user) { create(:user) } + let(:project) { create(:empty_project, :public) } + let(:developer) { create(:user) } let(:issue) { create(:issue, project: project) } let(:milestone) { create(:milestone, project: project, title: '9.10') } let(:inprogress) { create(:label, project: project, title: 'In Progress') } let(:bug) { create(:label, project: project, title: 'Bug') } before do - project.team << [user, :developer] + project.team << [developer, :developer] end describe '#execute' do - let(:service) { described_class.new(project, user) } + let(:service) { described_class.new(project, developer) } let(:merge_request) { create(:merge_request, source_project: project) } shared_examples 'reopen command' do @@ -45,13 +45,13 @@ describe SlashCommands::InterpretService, services: true do it 'fetches assignee and populates assignee_id if content contains /assign' do _, updates = service.execute(content, issuable) - expect(updates).to eq(assignee_id: user.id) + expect(updates).to eq(assignee_id: developer.id) end end shared_examples 'unassign command' do it 'populates assignee_id: nil if content contains /unassign' do - issuable.update(assignee_id: user.id) + issuable.update(assignee_id: developer.id) _, updates = service.execute(content, issuable) expect(updates).to eq(assignee_id: nil) @@ -124,7 +124,7 @@ describe SlashCommands::InterpretService, services: true do shared_examples 'done command' do it 'populates todo_event: "done" if content contains /done' do - TodoService.new.mark_todo(issuable, user) + TodoService.new.mark_todo(issuable, developer) _, updates = service.execute(content, issuable) expect(updates).to eq(todo_event: 'done') @@ -141,7 +141,7 @@ describe SlashCommands::InterpretService, services: true do shared_examples 'unsubscribe command' do it 'populates subscription_event: "unsubscribe" if content contains /unsubscribe' do - issuable.subscribe(user) + issuable.subscribe(developer) _, updates = service.execute(content, issuable) expect(updates).to eq(subscription_event: 'unsubscribe') @@ -209,12 +209,12 @@ describe SlashCommands::InterpretService, services: true do end it_behaves_like 'assign command' do - let(:content) { "/assign @#{user.username}" } + let(:content) { "/assign @#{developer.username}" } let(:issuable) { issue } end it_behaves_like 'assign command' do - let(:content) { "/assign @#{user.username}" } + let(:content) { "/assign @#{developer.username}" } let(:issuable) { merge_request } end @@ -380,5 +380,56 @@ describe SlashCommands::InterpretService, services: true do let(:content) { '/remove_due_date' } let(:issuable) { merge_request } end + + context 'when current_user cannot :admin_issue' do + let(:visitor) { create(:user) } + let(:issue) { create(:issue, project: project, author: visitor) } + let(:service) { described_class.new(project, visitor) } + + it_behaves_like 'empty command' do + let(:content) { "/assign @#{developer.username}" } + let(:issuable) { issue } + end + + it_behaves_like 'empty command' do + let(:content) { '/unassign' } + let(:issuable) { issue } + end + + it_behaves_like 'empty command' do + let(:content) { "/milestone %#{milestone.title}" } + let(:issuable) { issue } + end + + it_behaves_like 'empty command' do + let(:content) { '/remove_milestone' } + let(:issuable) { issue } + end + + it_behaves_like 'empty command' do + let(:content) { %(/label ~"#{inprogress.title}" ~#{bug.title} ~unknown) } + let(:issuable) { issue } + end + + it_behaves_like 'empty command' do + let(:content) { %(/unlabel ~"#{inprogress.title}") } + let(:issuable) { issue } + end + + it_behaves_like 'empty command' do + let(:content) { %(/relabel ~"#{inprogress.title}") } + let(:issuable) { issue } + end + + it_behaves_like 'empty command' do + let(:content) { '/due tomorrow' } + let(:issuable) { issue } + end + + it_behaves_like 'empty command' do + let(:content) { '/remove_due_date' } + let(:issuable) { issue } + end + end end end diff --git a/spec/support/features/issuable_slash_commands_shared_examples.rb b/spec/support/features/issuable_slash_commands_shared_examples.rb new file mode 100644 index 00000000000..5e3b8f2b23e --- /dev/null +++ b/spec/support/features/issuable_slash_commands_shared_examples.rb @@ -0,0 +1,261 @@ +# Specifications for behavior common to all objects with executable attributes. +# It takes a `issuable_type`, and expect an `issuable`. + +shared_examples 'issuable record that supports slash commands in its description and notes' do |issuable_type| + include SlashCommandsHelpers + include WaitForAjax + + let(:master) { create(:user) } + let(:assignee) { create(:user, username: 'bob') } + let(:guest) { create(:user) } + let(:project) { create(:project, :public) } + let!(:milestone) { create(:milestone, project: project, title: 'ASAP') } + let!(:label_bug) { create(:label, project: project, title: 'bug') } + let!(:label_feature) { create(:label, project: project, title: 'feature') } + let(:new_url_opts) { {} } + + before do + project.team << [master, :master] + project.team << [assignee, :developer] + project.team << [guest, :guest] + login_with(master) + end + + after do + # Ensure all outstanding Ajax requests are complete to avoid database deadlocks + wait_for_ajax + end + + describe "new #{issuable_type}" do + context 'with commands in the description' do + it "creates the #{issuable_type} and interpret commands accordingly" do + visit public_send("new_namespace_project_#{issuable_type}_path", project.namespace, project, new_url_opts) + fill_in "#{issuable_type}_title", with: 'bug 345' + fill_in "#{issuable_type}_description", with: "bug description\n/label ~bug\n/milestone %\"ASAP\"" + click_button "Submit #{issuable_type}".humanize + + issuable = project.public_send(issuable_type.to_s.pluralize).first + + expect(issuable.description).to eq "bug description" + expect(issuable.labels).to eq [label_bug] + expect(issuable.milestone).to eq milestone + expect(page).to have_content 'bug 345' + expect(page).to have_content 'bug description' + end + end + end + + describe "note on #{issuable_type}" do + before do + visit public_send("namespace_project_#{issuable_type}_path", project.namespace, project, issuable) + end + + context 'with a note containing commands' do + it 'creates a note without the commands and interpret the commands accordingly' do + write_note("Awesome!\n/assign @bob\n/label ~bug\n/milestone %\"ASAP\"") + + expect(page).to have_content 'Awesome!' + expect(page).not_to have_content '/assign @bob' + expect(page).not_to have_content '/label ~bug' + expect(page).not_to have_content '/milestone %"ASAP"' + + issuable.reload + note = issuable.notes.user.first + + expect(note.note).to eq "Awesome!" + expect(issuable.assignee).to eq assignee + expect(issuable.labels).to eq [label_bug] + expect(issuable.milestone).to eq milestone + end + end + + context 'with a note containing only commands' do + it 'does not create a note but interpret the commands accordingly' do + write_note("/assign @bob\n/label ~bug\n/milestone %\"ASAP\"") + + expect(page).not_to have_content '/assign @bob' + expect(page).not_to have_content '/label ~bug' + expect(page).not_to have_content '/milestone %"ASAP"' + expect(page).to have_content 'Your commands have been executed!' + + issuable.reload + + expect(issuable.notes.user).to be_empty + expect(issuable.assignee).to eq assignee + expect(issuable.labels).to eq [label_bug] + expect(issuable.milestone).to eq milestone + end + end + + context "with a note closing the #{issuable_type}" do + before do + expect(issuable).to be_open + end + + context "when current user can close #{issuable_type}" do + it "closes the #{issuable_type}" do + write_note("/close") + + expect(page).not_to have_content '/close' + expect(page).to have_content 'Your commands have been executed!' + + expect(issuable.reload).to be_closed + end + end + + context "when current user cannot close #{issuable_type}" do + before do + logout + login_with(guest) + visit public_send("namespace_project_#{issuable_type}_path", project.namespace, project, issuable) + end + + it "does not close the #{issuable_type}" do + write_note("/close") + + expect(page).not_to have_content '/close' + expect(page).not_to have_content 'Your commands have been executed!' + + expect(issuable).to be_open + end + end + end + + context "with a note reopening the #{issuable_type}" do + before do + issuable.close + expect(issuable).to be_closed + end + + context "when current user can reopen #{issuable_type}" do + it "reopens the #{issuable_type}" do + write_note("/reopen") + + expect(page).not_to have_content '/reopen' + expect(page).to have_content 'Your commands have been executed!' + + expect(issuable.reload).to be_open + end + end + + context "when current user cannot reopen #{issuable_type}" do + before do + logout + login_with(guest) + visit public_send("namespace_project_#{issuable_type}_path", project.namespace, project, issuable) + end + + it "does not reopen the #{issuable_type}" do + write_note("/reopen") + + expect(page).not_to have_content '/reopen' + expect(page).not_to have_content 'Your commands have been executed!' + + expect(issuable).to be_closed + end + end + end + + context "with a note changing the #{issuable_type}'s title" do + context "when current user can change title of #{issuable_type}" do + it "reopens the #{issuable_type}" do + write_note("/title Awesome new title") + + expect(page).not_to have_content '/title' + expect(page).to have_content 'Your commands have been executed!' + + expect(issuable.reload.title).to eq 'Awesome new title' + end + end + + context "when current user cannot change title of #{issuable_type}" do + before do + logout + login_with(guest) + visit public_send("namespace_project_#{issuable_type}_path", project.namespace, project, issuable) + end + + it "does not reopen the #{issuable_type}" do + write_note("/title Awesome new title") + + expect(page).not_to have_content '/title' + expect(page).not_to have_content 'Your commands have been executed!' + + expect(issuable.reload.title).not_to eq 'Awesome new title' + end + end + end + + context "with a note marking the #{issuable_type} as todo" do + it "creates a new todo for the #{issuable_type}" do + write_note("/todo") + + expect(page).not_to have_content '/todo' + expect(page).to have_content 'Your commands have been executed!' + + todos = TodosFinder.new(master).execute + todo = todos.first + + expect(todos.size).to eq 1 + expect(todo).to be_pending + expect(todo.target).to eq issuable + expect(todo.author).to eq master + expect(todo.user).to eq master + end + end + + context "with a note marking the #{issuable_type} as done" do + before do + TodoService.new.mark_todo(issuable, master) + end + + it "creates a new todo for the #{issuable_type}" do + todos = TodosFinder.new(master).execute + todo = todos.first + + expect(todos.size).to eq 1 + expect(todos.first).to be_pending + expect(todo.target).to eq issuable + expect(todo.author).to eq master + expect(todo.user).to eq master + + write_note("/done") + + expect(page).not_to have_content '/done' + expect(page).to have_content 'Your commands have been executed!' + + expect(todo.reload).to be_done + end + end + + context "with a note subscribing to the #{issuable_type}" do + it "creates a new todo for the #{issuable_type}" do + expect(issuable.subscribed?(master)).to be_falsy + + write_note("/subscribe") + + expect(page).not_to have_content '/subscribe' + expect(page).to have_content 'Your commands have been executed!' + + expect(issuable.subscribed?(master)).to be_truthy + end + end + + context "with a note unsubscribing to the #{issuable_type} as done" do + before do + issuable.subscribe(master) + end + + it "creates a new todo for the #{issuable_type}" do + expect(issuable.subscribed?(master)).to be_truthy + + write_note("/unsubscribe") + + expect(page).not_to have_content '/unsubscribe' + expect(page).to have_content 'Your commands have been executed!' + + expect(issuable.subscribed?(master)).to be_falsy + end + end + end +end diff --git a/spec/support/issuable_create_service_slash_commands_shared_examples.rb b/spec/support/issuable_create_service_slash_commands_shared_examples.rb deleted file mode 100644 index 5f9645ed44f..00000000000 --- a/spec/support/issuable_create_service_slash_commands_shared_examples.rb +++ /dev/null @@ -1,83 +0,0 @@ -# Specifications for behavior common to all objects with executable attributes. -# It can take a `default_params`. - -shared_examples 'new issuable record that supports slash commands' do - let!(:project) { create(:project) } - let(:user) { create(:user).tap { |u| project.team << [u, :master] } } - let(:assignee) { create(:user) } - let!(:milestone) { create(:milestone, project: project) } - let!(:labels) { create_list(:label, 3, project: project) } - let(:base_params) { { title: FFaker::Lorem.sentence(3) } } - let(:params) { base_params.merge(defined?(default_params) ? default_params : {}).merge(example_params) } - let(:issuable) { described_class.new(project, user, params).execute } - - context 'with labels in command only' do - let(:example_params) do - { - description: "/label ~#{labels.first.name} ~#{labels.second.name}\n/unlabel ~#{labels.third.name}" - } - end - - it 'attaches labels to issuable' do - expect(issuable).to be_persisted - expect(issuable.label_ids).to match_array([labels.first.id, labels.second.id]) - end - end - - context 'with labels in params and command' do - let(:example_params) do - { - label_ids: [labels.second.id], - description: "/label ~#{labels.first.name}\n/unlabel ~#{labels.third.name}" - } - end - - it 'attaches all labels to issuable' do - expect(issuable).to be_persisted - expect(issuable.label_ids).to match_array([labels.first.id, labels.second.id]) - end - end - - context 'with assignee and milestone in command only' do - let(:example_params) do - { - description: %(/assign @#{assignee.username}\n/milestone %"#{milestone.name}") - } - end - - it 'assigns and sets milestone to issuable' do - expect(issuable).to be_persisted - expect(issuable.assignee).to eq(assignee) - expect(issuable.milestone).to eq(milestone) - end - end - - context 'with assignee and milestone in params and command' do - let(:example_params) do - { - assignee: build_stubbed(:user), - milestone_id: double(:milestone), - description: %(/assign @#{assignee.username}\n/milestone %"#{milestone.name}") - } - end - - it 'assigns and sets milestone to issuable from command' do - expect(issuable).to be_persisted - expect(issuable.assignee).to eq(assignee) - expect(issuable.milestone).to eq(milestone) - end - end - - describe '/close' do - let(:example_params) do - { - description: '/close' - } - end - - it 'returns an open issue' do - expect(issuable).to be_persisted - expect(issuable).to be_open - end - end -end diff --git a/spec/support/issuable_slash_commands_shared_examples.rb b/spec/support/issuable_slash_commands_shared_examples.rb deleted file mode 100644 index 5e3b8f2b23e..00000000000 --- a/spec/support/issuable_slash_commands_shared_examples.rb +++ /dev/null @@ -1,261 +0,0 @@ -# Specifications for behavior common to all objects with executable attributes. -# It takes a `issuable_type`, and expect an `issuable`. - -shared_examples 'issuable record that supports slash commands in its description and notes' do |issuable_type| - include SlashCommandsHelpers - include WaitForAjax - - let(:master) { create(:user) } - let(:assignee) { create(:user, username: 'bob') } - let(:guest) { create(:user) } - let(:project) { create(:project, :public) } - let!(:milestone) { create(:milestone, project: project, title: 'ASAP') } - let!(:label_bug) { create(:label, project: project, title: 'bug') } - let!(:label_feature) { create(:label, project: project, title: 'feature') } - let(:new_url_opts) { {} } - - before do - project.team << [master, :master] - project.team << [assignee, :developer] - project.team << [guest, :guest] - login_with(master) - end - - after do - # Ensure all outstanding Ajax requests are complete to avoid database deadlocks - wait_for_ajax - end - - describe "new #{issuable_type}" do - context 'with commands in the description' do - it "creates the #{issuable_type} and interpret commands accordingly" do - visit public_send("new_namespace_project_#{issuable_type}_path", project.namespace, project, new_url_opts) - fill_in "#{issuable_type}_title", with: 'bug 345' - fill_in "#{issuable_type}_description", with: "bug description\n/label ~bug\n/milestone %\"ASAP\"" - click_button "Submit #{issuable_type}".humanize - - issuable = project.public_send(issuable_type.to_s.pluralize).first - - expect(issuable.description).to eq "bug description" - expect(issuable.labels).to eq [label_bug] - expect(issuable.milestone).to eq milestone - expect(page).to have_content 'bug 345' - expect(page).to have_content 'bug description' - end - end - end - - describe "note on #{issuable_type}" do - before do - visit public_send("namespace_project_#{issuable_type}_path", project.namespace, project, issuable) - end - - context 'with a note containing commands' do - it 'creates a note without the commands and interpret the commands accordingly' do - write_note("Awesome!\n/assign @bob\n/label ~bug\n/milestone %\"ASAP\"") - - expect(page).to have_content 'Awesome!' - expect(page).not_to have_content '/assign @bob' - expect(page).not_to have_content '/label ~bug' - expect(page).not_to have_content '/milestone %"ASAP"' - - issuable.reload - note = issuable.notes.user.first - - expect(note.note).to eq "Awesome!" - expect(issuable.assignee).to eq assignee - expect(issuable.labels).to eq [label_bug] - expect(issuable.milestone).to eq milestone - end - end - - context 'with a note containing only commands' do - it 'does not create a note but interpret the commands accordingly' do - write_note("/assign @bob\n/label ~bug\n/milestone %\"ASAP\"") - - expect(page).not_to have_content '/assign @bob' - expect(page).not_to have_content '/label ~bug' - expect(page).not_to have_content '/milestone %"ASAP"' - expect(page).to have_content 'Your commands have been executed!' - - issuable.reload - - expect(issuable.notes.user).to be_empty - expect(issuable.assignee).to eq assignee - expect(issuable.labels).to eq [label_bug] - expect(issuable.milestone).to eq milestone - end - end - - context "with a note closing the #{issuable_type}" do - before do - expect(issuable).to be_open - end - - context "when current user can close #{issuable_type}" do - it "closes the #{issuable_type}" do - write_note("/close") - - expect(page).not_to have_content '/close' - expect(page).to have_content 'Your commands have been executed!' - - expect(issuable.reload).to be_closed - end - end - - context "when current user cannot close #{issuable_type}" do - before do - logout - login_with(guest) - visit public_send("namespace_project_#{issuable_type}_path", project.namespace, project, issuable) - end - - it "does not close the #{issuable_type}" do - write_note("/close") - - expect(page).not_to have_content '/close' - expect(page).not_to have_content 'Your commands have been executed!' - - expect(issuable).to be_open - end - end - end - - context "with a note reopening the #{issuable_type}" do - before do - issuable.close - expect(issuable).to be_closed - end - - context "when current user can reopen #{issuable_type}" do - it "reopens the #{issuable_type}" do - write_note("/reopen") - - expect(page).not_to have_content '/reopen' - expect(page).to have_content 'Your commands have been executed!' - - expect(issuable.reload).to be_open - end - end - - context "when current user cannot reopen #{issuable_type}" do - before do - logout - login_with(guest) - visit public_send("namespace_project_#{issuable_type}_path", project.namespace, project, issuable) - end - - it "does not reopen the #{issuable_type}" do - write_note("/reopen") - - expect(page).not_to have_content '/reopen' - expect(page).not_to have_content 'Your commands have been executed!' - - expect(issuable).to be_closed - end - end - end - - context "with a note changing the #{issuable_type}'s title" do - context "when current user can change title of #{issuable_type}" do - it "reopens the #{issuable_type}" do - write_note("/title Awesome new title") - - expect(page).not_to have_content '/title' - expect(page).to have_content 'Your commands have been executed!' - - expect(issuable.reload.title).to eq 'Awesome new title' - end - end - - context "when current user cannot change title of #{issuable_type}" do - before do - logout - login_with(guest) - visit public_send("namespace_project_#{issuable_type}_path", project.namespace, project, issuable) - end - - it "does not reopen the #{issuable_type}" do - write_note("/title Awesome new title") - - expect(page).not_to have_content '/title' - expect(page).not_to have_content 'Your commands have been executed!' - - expect(issuable.reload.title).not_to eq 'Awesome new title' - end - end - end - - context "with a note marking the #{issuable_type} as todo" do - it "creates a new todo for the #{issuable_type}" do - write_note("/todo") - - expect(page).not_to have_content '/todo' - expect(page).to have_content 'Your commands have been executed!' - - todos = TodosFinder.new(master).execute - todo = todos.first - - expect(todos.size).to eq 1 - expect(todo).to be_pending - expect(todo.target).to eq issuable - expect(todo.author).to eq master - expect(todo.user).to eq master - end - end - - context "with a note marking the #{issuable_type} as done" do - before do - TodoService.new.mark_todo(issuable, master) - end - - it "creates a new todo for the #{issuable_type}" do - todos = TodosFinder.new(master).execute - todo = todos.first - - expect(todos.size).to eq 1 - expect(todos.first).to be_pending - expect(todo.target).to eq issuable - expect(todo.author).to eq master - expect(todo.user).to eq master - - write_note("/done") - - expect(page).not_to have_content '/done' - expect(page).to have_content 'Your commands have been executed!' - - expect(todo.reload).to be_done - end - end - - context "with a note subscribing to the #{issuable_type}" do - it "creates a new todo for the #{issuable_type}" do - expect(issuable.subscribed?(master)).to be_falsy - - write_note("/subscribe") - - expect(page).not_to have_content '/subscribe' - expect(page).to have_content 'Your commands have been executed!' - - expect(issuable.subscribed?(master)).to be_truthy - end - end - - context "with a note unsubscribing to the #{issuable_type} as done" do - before do - issuable.subscribe(master) - end - - it "creates a new todo for the #{issuable_type}" do - expect(issuable.subscribed?(master)).to be_truthy - - write_note("/unsubscribe") - - expect(page).not_to have_content '/unsubscribe' - expect(page).to have_content 'Your commands have been executed!' - - expect(issuable.subscribed?(master)).to be_falsy - end - end - end -end diff --git a/spec/support/services/issuable_create_service_slash_commands_shared_examples.rb b/spec/support/services/issuable_create_service_slash_commands_shared_examples.rb new file mode 100644 index 00000000000..5f9645ed44f --- /dev/null +++ b/spec/support/services/issuable_create_service_slash_commands_shared_examples.rb @@ -0,0 +1,83 @@ +# Specifications for behavior common to all objects with executable attributes. +# It can take a `default_params`. + +shared_examples 'new issuable record that supports slash commands' do + let!(:project) { create(:project) } + let(:user) { create(:user).tap { |u| project.team << [u, :master] } } + let(:assignee) { create(:user) } + let!(:milestone) { create(:milestone, project: project) } + let!(:labels) { create_list(:label, 3, project: project) } + let(:base_params) { { title: FFaker::Lorem.sentence(3) } } + let(:params) { base_params.merge(defined?(default_params) ? default_params : {}).merge(example_params) } + let(:issuable) { described_class.new(project, user, params).execute } + + context 'with labels in command only' do + let(:example_params) do + { + description: "/label ~#{labels.first.name} ~#{labels.second.name}\n/unlabel ~#{labels.third.name}" + } + end + + it 'attaches labels to issuable' do + expect(issuable).to be_persisted + expect(issuable.label_ids).to match_array([labels.first.id, labels.second.id]) + end + end + + context 'with labels in params and command' do + let(:example_params) do + { + label_ids: [labels.second.id], + description: "/label ~#{labels.first.name}\n/unlabel ~#{labels.third.name}" + } + end + + it 'attaches all labels to issuable' do + expect(issuable).to be_persisted + expect(issuable.label_ids).to match_array([labels.first.id, labels.second.id]) + end + end + + context 'with assignee and milestone in command only' do + let(:example_params) do + { + description: %(/assign @#{assignee.username}\n/milestone %"#{milestone.name}") + } + end + + it 'assigns and sets milestone to issuable' do + expect(issuable).to be_persisted + expect(issuable.assignee).to eq(assignee) + expect(issuable.milestone).to eq(milestone) + end + end + + context 'with assignee and milestone in params and command' do + let(:example_params) do + { + assignee: build_stubbed(:user), + milestone_id: double(:milestone), + description: %(/assign @#{assignee.username}\n/milestone %"#{milestone.name}") + } + end + + it 'assigns and sets milestone to issuable from command' do + expect(issuable).to be_persisted + expect(issuable.assignee).to eq(assignee) + expect(issuable.milestone).to eq(milestone) + end + end + + describe '/close' do + let(:example_params) do + { + description: '/close' + } + end + + it 'returns an open issue' do + expect(issuable).to be_persisted + expect(issuable).to be_open + end + end +end -- cgit v1.2.1