Commit a7f89757 authored by Arturo Herrero's avatar Arturo Herrero

Merge Request: Close JIRA issues when issues are disabled

We close external issues even when the user is not authorized by
DeclarativePolicy framework.
parent 25918ea2
...@@ -4,7 +4,7 @@ module Issues ...@@ -4,7 +4,7 @@ module Issues
class CloseService < Issues::BaseService class CloseService < Issues::BaseService
# Closes the supplied issue if the current user is able to do so. # Closes the supplied issue if the current user is able to do so.
def execute(issue, commit: nil, notifications: true, system_note: true) def execute(issue, commit: nil, notifications: true, system_note: true)
return issue unless can?(current_user, :update_issue, issue) return issue unless can?(current_user, :update_issue, issue) || issue.is_a?(ExternalIssue)
close_issue(issue, close_issue(issue,
closed_via: commit, closed_via: commit,
......
...@@ -29,9 +29,7 @@ module MergeRequests ...@@ -29,9 +29,7 @@ module MergeRequests
closed_issues = merge_request.visible_closing_issues_for(current_user) closed_issues = merge_request.visible_closing_issues_for(current_user)
closed_issues.each do |issue| closed_issues.each do |issue|
if can?(current_user, :update_issue, issue) Issues::CloseService.new(project, current_user).execute(issue, commit: merge_request)
Issues::CloseService.new(project, current_user, {}).execute(issue, commit: merge_request)
end
end end
end end
......
...@@ -8,6 +8,7 @@ describe Issues::CloseService do ...@@ -8,6 +8,7 @@ describe Issues::CloseService do
let(:user2) { create(:user, email: "user2@example.com") } let(:user2) { create(:user, email: "user2@example.com") }
let(:guest) { create(:user) } let(:guest) { create(:user) }
let(:issue) { create(:issue, title: "My issue", project: project, assignees: [user2], author: create(:user)) } let(:issue) { create(:issue, title: "My issue", project: project, assignees: [user2], author: create(:user)) }
let(:external_issue) { ExternalIssue.new('JIRA-123', project) }
let(:closing_merge_request) { create(:merge_request, source_project: project) } let(:closing_merge_request) { create(:merge_request, source_project: project) }
let(:closing_commit) { create(:commit, project: project) } let(:closing_commit) { create(:commit, project: project) }
let!(:todo) { create(:todo, :assigned, user: user, project: project, target: issue, author: user2) } let!(:todo) { create(:todo, :assigned, user: user, project: project, target: issue, author: user2) }
...@@ -36,6 +37,16 @@ describe Issues::CloseService do ...@@ -36,6 +37,16 @@ describe Issues::CloseService do
expect(service.execute(issue)).to eq(issue) expect(service.execute(issue)).to eq(issue)
end end
it 'closes the external issue even when the user is not authorized to do so' do
allow(service).to receive(:can?).with(user, :update_issue, external_issue)
.and_return(false)
expect(service).to receive(:close_issue)
.with(external_issue, closed_via: nil, notifications: true, system_note: true)
service.execute(external_issue)
end
it 'closes the issue when the user is authorized to do so' do it 'closes the issue when the user is authorized to do so' do
allow(service).to receive(:can?).with(user, :update_issue, issue) allow(service).to receive(:can?).with(user, :update_issue, issue)
.and_return(true) .and_return(true)
......
...@@ -56,9 +56,10 @@ describe MergeRequests::PostMergeService do ...@@ -56,9 +56,10 @@ describe MergeRequests::PostMergeService do
issue = create(:issue, project: project) issue = create(:issue, project: project)
allow(merge_request).to receive(:visible_closing_issues_for).and_return([issue]) allow(merge_request).to receive(:visible_closing_issues_for).and_return([issue])
allow_any_instance_of(Issues::CloseService).to receive(:execute).with(issue, commit: merge_request).and_raise allow_any_instance_of(Issues::CloseService).to receive(:execute)
.with(issue, commit: merge_request).and_raise(RuntimeError)
expect { described_class.new(project, user, {}).execute(merge_request) }.to raise_error expect { described_class.new(project, user).execute(merge_request) }.to raise_error(RuntimeError)
expect(merge_request.reload).to be_merged expect(merge_request.reload).to be_merged
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment