diff --git a/CHANGELOG b/CHANGELOG
index f92fcff951a631201bcb36b487f8a475d4a7dfba..4512d7d0e7592c2bd68039bc7b8c75ade0f8adbd 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 fbce46769f7648f8687aeb87d9704803f3346303..57d521f2fead02ec54ce8a7d311f28fed3325aca 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 9ac1124abc15f6ff4e32f8d1695014f98de0d85f..ffcad5b3a8782c1df1da70819c2e4261544c84aa 100644
--- a/app/services/slash_commands/interpret_service.rb
+++ b/app/services/slash_commands/interpret_service.rb
@@ -195,7 +195,7 @@ module SlashCommands
params '<in 2 days | this Friday | December 31st>'
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 105629c485a8ec29273e62e305b3b470b170f563..bf2b93c92fb68578f1af1f5d90b8449846c2c1cb 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 58569ba96c3ae70f0f82df97fd8f94331307b51e..1050502fa197bc7c525045735d1029abb36bc0bc 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 4f5375a3583a2fbbecf73301d07181171e3bdabc..1638a46ed51d34d23ae2cbd16c7cfeb8250bedbb 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 a616275e8834c30342b69ce6cce5cd233890a009..5b1edba87a1d3166bb3b24daae02159105b29258 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/issuable_slash_commands_shared_examples.rb b/spec/support/features/issuable_slash_commands_shared_examples.rb
similarity index 100%
rename from spec/support/issuable_slash_commands_shared_examples.rb
rename to spec/support/features/issuable_slash_commands_shared_examples.rb
diff --git a/spec/support/issuable_create_service_slash_commands_shared_examples.rb b/spec/support/services/issuable_create_service_slash_commands_shared_examples.rb
similarity index 100%
rename from spec/support/issuable_create_service_slash_commands_shared_examples.rb
rename to spec/support/services/issuable_create_service_slash_commands_shared_examples.rb