Commit 11df2291 authored by Sean Arnold's avatar Sean Arnold

Only run on open issues

-Add changelog file
-Add new negative flow specs
- Add translation
- Rename system note argument
- Add new update_sentry_issue permission
parent 902dbd2d
...@@ -222,6 +222,7 @@ class ProjectPolicy < BasePolicy ...@@ -222,6 +222,7 @@ class ProjectPolicy < BasePolicy
enable :read_deployment enable :read_deployment
enable :read_merge_request enable :read_merge_request
enable :read_sentry_issue enable :read_sentry_issue
enable :update_sentry_issue
enable :read_prometheus enable :read_prometheus
end end
......
...@@ -62,5 +62,9 @@ module ErrorTracking ...@@ -62,5 +62,9 @@ module ErrorTracking
def can_read? def can_read?
can?(current_user, :read_sentry_issue, project) can?(current_user, :read_sentry_issue, project)
end end
def can_update?
can?(current_user, :update_sentry_issue, project)
end
end end
end end
...@@ -21,10 +21,19 @@ module ErrorTracking ...@@ -21,10 +21,19 @@ module ErrorTracking
def update_related_issue def update_related_issue
issue = related_issue issue = related_issue
return unless issue && resolving? return unless issue
@closed_issue = close_and_create_note(issue)
end
def close_and_create_note(issue)
return unless resolving? && issue.opened?
processed_issue = close_issue(issue) processed_issue = close_issue(issue)
return unless processed_issue.reset.closed?
create_system_note(processed_issue) create_system_note(processed_issue)
processed_issue
end end
def close_issue(issue) def close_issue(issue)
...@@ -34,8 +43,6 @@ module ErrorTracking ...@@ -34,8 +43,6 @@ module ErrorTracking
end end
def create_system_note(issue) def create_system_note(issue)
return unless issue.reset.closed?
SystemNoteService.close_after_error_tracking_resolve(issue, project, current_user) SystemNoteService.close_after_error_tracking_resolve(issue, project, current_user)
end end
...@@ -55,7 +62,15 @@ module ErrorTracking ...@@ -55,7 +62,15 @@ module ErrorTracking
end end
def parse_response(response) def parse_response(response)
{ updated: response[:updated].present? } {
updated: response[:updated].present?,
closed_issue_iid: @closed_issue&.iid
}
end
def check_permissions
return error('Error Tracking is not enabled') unless enabled?
return error('Access denied', :unauthorized) unless can_update?
end end
end end
end end
...@@ -99,10 +99,10 @@ module SystemNoteService ...@@ -99,10 +99,10 @@ module SystemNoteService
::SystemNotes::TimeTrackingService.new(noteable: noteable, project: project, author: author).change_time_spent ::SystemNotes::TimeTrackingService.new(noteable: noteable, project: project, author: author).change_time_spent
end end
def close_after_error_tracking_resolve(noteable, project, author) def close_after_error_tracking_resolve(issue, project, author)
body = 'automatically closed this as a result of resolving the corresponding Sentry error.' body = _('resolved the corresponding error and closed the issue.')
create_note(NoteSummary.new(noteable, project, author, body, action: 'closed')) create_note(NoteSummary.new(issue, project, author, body, action: 'closed'))
end end
def change_status(noteable, project, author, status, source = nil) def change_status(noteable, project, author, status, source = nil)
......
---
title: Close Issue when resolving corresponding Sentry error
merge_request: 22744
author:
type: added
...@@ -22788,6 +22788,9 @@ msgstr[1] "" ...@@ -22788,6 +22788,9 @@ msgstr[1] ""
msgid "reset it." msgid "reset it."
msgstr "" msgstr ""
msgid "resolved the corresponding error and closed the issue."
msgstr ""
msgid "score" msgid "score"
msgstr "" msgstr ""
......
...@@ -301,7 +301,7 @@ describe Projects::ErrorTrackingController do ...@@ -301,7 +301,7 @@ describe Projects::ErrorTrackingController do
context 'update result is successful' do context 'update result is successful' do
before do before do
expect(issue_update_service).to receive(:execute) expect(issue_update_service).to receive(:execute)
.and_return(status: :success, updated: true) .and_return(status: :success, updated: true, closed_issue_iid: 1234)
update_issue update_issue
end end
......
...@@ -6,9 +6,15 @@ ...@@ -6,9 +6,15 @@
"properties" : { "properties" : {
"result": { "result": {
"type": "object", "type": "object",
"required" : [
"status",
"updated",
"closed_issue_iid"
],
"properties": { "properties": {
"status": { "type": "string" }, "status": { "type": "string" },
"updated": { "type": "boolean" } "updated": { "type": "boolean" },
"closed_issue_iid": { "type": ["integer", "null"] }
} }
} }
}, },
......
...@@ -9,6 +9,17 @@ describe ErrorTracking::IssueUpdateService do ...@@ -9,6 +9,17 @@ describe ErrorTracking::IssueUpdateService do
subject { described_class.new(project, user, arguments) } subject { described_class.new(project, user, arguments) }
shared_examples 'does not perform close issue flow' do
it 'does not call the close issue service' do
expect(issue_close_service)
.not_to receive(:execute)
end
it 'does not create system note' do
expect(SystemNoteService).not_to receive(:close_after_error_tracking_resolve)
end
end
describe '#execute' do describe '#execute' do
context 'with authorized user' do context 'with authorized user' do
context 'when update_issue returns success' do context 'when update_issue returns success' do
...@@ -20,7 +31,7 @@ describe ErrorTracking::IssueUpdateService do ...@@ -20,7 +31,7 @@ describe ErrorTracking::IssueUpdateService do
end end
it 'returns the response' do it 'returns the response' do
expect(result).to eq(update_issue_response.merge(status: :success)) expect(result).to eq(update_issue_response.merge(status: :success, closed_issue_iid: nil))
end end
it 'updates any related issue' do it 'updates any related issue' do
...@@ -40,6 +51,10 @@ describe ErrorTracking::IssueUpdateService do ...@@ -40,6 +51,10 @@ describe ErrorTracking::IssueUpdateService do
allow_any_instance_of(SentryIssuesFinder) allow_any_instance_of(SentryIssuesFinder)
.to receive(:find_by_identifier) .to receive(:find_by_identifier)
.and_return(sentry_issue) .and_return(sentry_issue)
allow(Issues::CloseService)
.to receive(:new)
.and_return(issue_close_service)
end end
after do after do
...@@ -47,12 +62,7 @@ describe ErrorTracking::IssueUpdateService do ...@@ -47,12 +62,7 @@ describe ErrorTracking::IssueUpdateService do
end end
it 'closes the issue' do it 'closes the issue' do
close_service = spy(:close_service) expect(issue_close_service)
expect(Issues::CloseService)
.to receive(:new)
.and_return(close_service)
expect(close_service)
.to receive(:execute) .to receive(:execute)
.with(issue, system_note: false) .with(issue, system_note: false)
.and_return(issue) .and_return(issue)
...@@ -61,6 +71,29 @@ describe ErrorTracking::IssueUpdateService do ...@@ -61,6 +71,29 @@ describe ErrorTracking::IssueUpdateService do
it 'creates a system note' do it 'creates a system note' do
expect(SystemNoteService).to receive(:close_after_error_tracking_resolve) expect(SystemNoteService).to receive(:close_after_error_tracking_resolve)
end end
it 'returns a response with closed issue' do
closed_issue = create(:issue, :closed, project: project)
expect(issue_close_service)
.to receive(:execute)
.with(issue, system_note: false)
.and_return(closed_issue)
expect(result).to eq(status: :success, updated: true, closed_issue_iid: closed_issue.iid)
end
context 'issue is already closed' do
let(:issue) { create(:issue, :closed, project: project) }
include_examples 'does not perform close issue flow'
end
context 'status is not resolving' do
let(:arguments) { { issue_id: sentry_issue.sentry_issue_identifier, status: 'ignored' } }
include_examples 'does not perform close issue flow'
end
end end
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