From afe057a8ff8546f0032e439a9a200307fb6de86a Mon Sep 17 00:00:00 2001
From: GitLab Bot <gitlab-bot@gitlab.com>
Date: Tue, 21 Jan 2020 09:08:43 +0000
Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master

---
 app/policies/project_policy.rb                |   1 +
 app/services/error_tracking/base_service.rb   |   8 +-
 .../error_tracking/issue_details_service.rb   |   2 +-
 .../issue_latest_event_service.rb             |   2 +-
 .../error_tracking/issue_update_service.rb    |  58 +++++++++-
 .../error_tracking/list_issues_service.rb     |   2 +-
 .../error_tracking/list_projects_service.rb   |   2 +-
 app/services/system_note_service.rb           |   6 +
 .../39825-close-issue-after-error-resolve.yml |   5 +
 doc/development/documentation/styleguide.md   |   4 +-
 locale/gitlab.pot                             |   3 +
 .../error_tracking_controller_spec.rb         |   2 +-
 .../schemas/error_tracking/update_issue.json  |   8 +-
 .../issue_details_service_spec.rb             |  19 +---
 .../issue_latest_event_service_spec.rb        |  19 +---
 .../issue_update_service_spec.rb              | 106 ++++++++++++++++++
 .../list_issues_service_spec.rb               |  18 +--
 .../sentry_error_tracking_shared_context.rb   |  22 ++++
 18 files changed, 224 insertions(+), 63 deletions(-)
 create mode 100644 changelogs/unreleased/39825-close-issue-after-error-resolve.yml
 create mode 100644 spec/services/error_tracking/issue_update_service_spec.rb
 create mode 100644 spec/support/shared_contexts/sentry_error_tracking_shared_context.rb

diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb
index e38eef527be..2789152e175 100644
--- a/app/policies/project_policy.rb
+++ b/app/policies/project_policy.rb
@@ -222,6 +222,7 @@ class ProjectPolicy < BasePolicy
     enable :read_deployment
     enable :read_merge_request
     enable :read_sentry_issue
+    enable :update_sentry_issue
     enable :read_prometheus
   end
 
diff --git a/app/services/error_tracking/base_service.rb b/app/services/error_tracking/base_service.rb
index 430d9952332..4fe01716704 100644
--- a/app/services/error_tracking/base_service.rb
+++ b/app/services/error_tracking/base_service.rb
@@ -7,7 +7,7 @@ module ErrorTracking
       return unauthorized if unauthorized
 
       begin
-        response = fetch
+        response = perform
       rescue Sentry::Client::Error => e
         return error(e.message, :bad_request)
       rescue Sentry::Client::MissingKeysError => e
@@ -22,7 +22,7 @@ module ErrorTracking
 
     private
 
-    def fetch
+    def perform
       raise NotImplementedError,
           "#{self.class} does not implement #{__method__}"
     end
@@ -62,5 +62,9 @@ module ErrorTracking
     def can_read?
       can?(current_user, :read_sentry_issue, project)
     end
+
+    def can_update?
+      can?(current_user, :update_sentry_issue, project)
+    end
   end
 end
diff --git a/app/services/error_tracking/issue_details_service.rb b/app/services/error_tracking/issue_details_service.rb
index 368cd4517fc..31fb6a9618c 100644
--- a/app/services/error_tracking/issue_details_service.rb
+++ b/app/services/error_tracking/issue_details_service.rb
@@ -4,7 +4,7 @@ module ErrorTracking
   class IssueDetailsService < ErrorTracking::BaseService
     private
 
-    def fetch
+    def perform
       project_error_tracking_setting.issue_details(issue_id: params[:issue_id])
     end
 
diff --git a/app/services/error_tracking/issue_latest_event_service.rb b/app/services/error_tracking/issue_latest_event_service.rb
index b6ad8f8028b..dd6b7f8285f 100644
--- a/app/services/error_tracking/issue_latest_event_service.rb
+++ b/app/services/error_tracking/issue_latest_event_service.rb
@@ -4,7 +4,7 @@ module ErrorTracking
   class IssueLatestEventService < ErrorTracking::BaseService
     private
 
-    def fetch
+    def perform
       project_error_tracking_setting.issue_latest_event(issue_id: params[:issue_id])
     end
 
diff --git a/app/services/error_tracking/issue_update_service.rb b/app/services/error_tracking/issue_update_service.rb
index e433b4a11f2..db754d54fef 100644
--- a/app/services/error_tracking/issue_update_service.rb
+++ b/app/services/error_tracking/issue_update_service.rb
@@ -4,6 +4,16 @@ module ErrorTracking
   class IssueUpdateService < ErrorTracking::BaseService
     private
 
+    def perform
+      response = fetch
+
+      unless parse_errors(response).present?
+        response[:closed_issue_iid] = update_related_issue&.iid
+      end
+
+      response
+    end
+
     def fetch
       project_error_tracking_setting.update_issue(
         issue_id: params[:issue_id],
@@ -11,12 +21,58 @@ module ErrorTracking
       )
     end
 
+    def update_related_issue
+      issue = related_issue
+      return unless issue
+
+      close_and_create_note(issue)
+    end
+
+    def close_and_create_note(issue)
+      return unless resolving? && issue.opened?
+
+      processed_issue = close_issue(issue)
+      return unless processed_issue.reset.closed?
+
+      create_system_note(processed_issue)
+      processed_issue
+    end
+
+    def close_issue(issue)
+      Issues::CloseService
+        .new(project, current_user)
+        .execute(issue, system_note: false)
+    end
+
+    def create_system_note(issue)
+      SystemNoteService.close_after_error_tracking_resolve(issue, project, current_user)
+    end
+
+    def related_issue
+      SentryIssueFinder
+        .new(project, current_user: current_user)
+        .execute(params[:issue_id])
+        &.issue
+    end
+
+    def resolving?
+      update_params[:status] == 'resolved'
+    end
+
     def update_params
       params.except(:issue_id)
     end
 
     def parse_response(response)
-      { updated: response[:updated].present? }
+      {
+        updated: response[:updated].present?,
+        closed_issue_iid: response[: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
diff --git a/app/services/error_tracking/list_issues_service.rb b/app/services/error_tracking/list_issues_service.rb
index 132e9dfa7bd..d34ea8aa3b0 100644
--- a/app/services/error_tracking/list_issues_service.rb
+++ b/app/services/error_tracking/list_issues_service.rb
@@ -12,7 +12,7 @@ module ErrorTracking
 
     private
 
-    def fetch
+    def perform
       project_error_tracking_setting.list_sentry_issues(
         issue_status: issue_status,
         limit: limit,
diff --git a/app/services/error_tracking/list_projects_service.rb b/app/services/error_tracking/list_projects_service.rb
index 09a0b952e84..6523a66bbed 100644
--- a/app/services/error_tracking/list_projects_service.rb
+++ b/app/services/error_tracking/list_projects_service.rb
@@ -12,7 +12,7 @@ module ErrorTracking
 
     private
 
-    def fetch
+    def perform
       project_error_tracking_setting.list_sentry_projects
     end
 
diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb
index 38e0a7d34ad..033c80fd8ed 100644
--- a/app/services/system_note_service.rb
+++ b/app/services/system_note_service.rb
@@ -99,6 +99,12 @@ module SystemNoteService
     ::SystemNotes::TimeTrackingService.new(noteable: noteable, project: project, author: author).change_time_spent
   end
 
+  def close_after_error_tracking_resolve(issue, project, author)
+    body = _('resolved the corresponding error and closed the issue.')
+
+    create_note(NoteSummary.new(issue, project, author, body, action: 'closed'))
+  end
+
   def change_status(noteable, project, author, status, source = nil)
     ::SystemNotes::IssuablesService.new(noteable: noteable, project: project, author: author).change_status(status, source)
   end
diff --git a/changelogs/unreleased/39825-close-issue-after-error-resolve.yml b/changelogs/unreleased/39825-close-issue-after-error-resolve.yml
new file mode 100644
index 00000000000..9a59c812290
--- /dev/null
+++ b/changelogs/unreleased/39825-close-issue-after-error-resolve.yml
@@ -0,0 +1,5 @@
+---
+title: Close Issue when resolving corresponding Sentry error
+merge_request: 22744
+author:
+type: added
diff --git a/doc/development/documentation/styleguide.md b/doc/development/documentation/styleguide.md
index 173ca324f59..c7c59a72e9c 100644
--- a/doc/development/documentation/styleguide.md
+++ b/doc/development/documentation/styleguide.md
@@ -275,10 +275,10 @@ Do not include the same information in multiple places. [Link to a SSOT instead.
 - Do not use "may" and "might" interchangeably:
   - Use "might" to indicate the probability of something occurring. "If you skip this step, the import process might fail."
   - Use "may" to indicate giving permission for someone to do something, or consider using "can" instead. "You may select either option on this screen." Or, "you can select either option on this screen."
-- We recommend avoiding Latin abbreviations, such as "e.g.," "i.e.," or "etc.,"
+- We discourage use of Latin abbreviations, such as "e.g.," "i.e.," or "etc.,"
 as even native users of English might misunderstand them.
   - Instead of "i.e.", use "that is."
-  - Instead of "e.g.", use "for example."
+  - Instead of "e.g.", use "for example," "such as," "for instance," or "like."
   - Instead of "etc.", either use "and so on" or consider editing it out, since it can be vague.
 
 ## Text
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 942c81aafd5..f905986bf0d 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -22788,6 +22788,9 @@ msgstr[1] ""
 msgid "reset it."
 msgstr ""
 
+msgid "resolved the corresponding error and closed the issue."
+msgstr ""
+
 msgid "score"
 msgstr ""
 
diff --git a/spec/controllers/projects/error_tracking_controller_spec.rb b/spec/controllers/projects/error_tracking_controller_spec.rb
index 588c4b05528..22826938de2 100644
--- a/spec/controllers/projects/error_tracking_controller_spec.rb
+++ b/spec/controllers/projects/error_tracking_controller_spec.rb
@@ -301,7 +301,7 @@ describe Projects::ErrorTrackingController do
       context 'update result is successful' do
         before do
           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
         end
diff --git a/spec/fixtures/api/schemas/error_tracking/update_issue.json b/spec/fixtures/api/schemas/error_tracking/update_issue.json
index 72514ce647d..75f2c1152d9 100644
--- a/spec/fixtures/api/schemas/error_tracking/update_issue.json
+++ b/spec/fixtures/api/schemas/error_tracking/update_issue.json
@@ -6,9 +6,15 @@
   "properties" : {
     "result": {
       "type": "object",
+      "required" : [
+        "status",
+        "updated",
+        "closed_issue_iid"
+      ],
       "properties": {
         "status": { "type": "string" },
-        "updated": { "type": "boolean" }
+        "updated": { "type": "boolean" },
+        "closed_issue_iid": { "type": ["integer", "null"] }
       }
     }
   },
diff --git a/spec/services/error_tracking/issue_details_service_spec.rb b/spec/services/error_tracking/issue_details_service_spec.rb
index 4d5505bb5a9..26bb3b44126 100644
--- a/spec/services/error_tracking/issue_details_service_spec.rb
+++ b/spec/services/error_tracking/issue_details_service_spec.rb
@@ -3,24 +3,7 @@
 require 'spec_helper'
 
 describe ErrorTracking::IssueDetailsService do
-  let_it_be(:user) { create(:user) }
-  let_it_be(:project) { create(:project) }
-
-  let(:sentry_url) { 'https://sentrytest.gitlab.com/api/0/projects/sentry-org/sentry-project' }
-  let(:token) { 'test-token' }
-  let(:result) { subject.execute }
-
-  let(:error_tracking_setting) do
-    create(:project_error_tracking_setting, api_url: sentry_url, token: token, project: project)
-  end
-
-  subject { described_class.new(project, user) }
-
-  before do
-    expect(project).to receive(:error_tracking_setting).at_least(:once).and_return(error_tracking_setting)
-
-    project.add_reporter(user)
-  end
+  include_context 'sentry error tracking context'
 
   describe '#execute' do
     context 'with authorized user' do
diff --git a/spec/services/error_tracking/issue_latest_event_service_spec.rb b/spec/services/error_tracking/issue_latest_event_service_spec.rb
index cda15042814..7f53eabd717 100644
--- a/spec/services/error_tracking/issue_latest_event_service_spec.rb
+++ b/spec/services/error_tracking/issue_latest_event_service_spec.rb
@@ -3,24 +3,7 @@
 require 'spec_helper'
 
 describe ErrorTracking::IssueLatestEventService do
-  let_it_be(:user) { create(:user) }
-  let_it_be(:project) { create(:project) }
-
-  let(:sentry_url) { 'https://sentrytest.gitlab.com/api/0/projects/sentry-org/sentry-project' }
-  let(:token) { 'test-token' }
-  let(:result) { subject.execute }
-
-  let(:error_tracking_setting) do
-    create(:project_error_tracking_setting, api_url: sentry_url, token: token, project: project)
-  end
-
-  subject { described_class.new(project, user) }
-
-  before do
-    expect(project).to receive(:error_tracking_setting).at_least(:once).and_return(error_tracking_setting)
-
-    project.add_reporter(user)
-  end
+  include_context 'sentry error tracking context'
 
   describe '#execute' do
     context 'with authorized user' do
diff --git a/spec/services/error_tracking/issue_update_service_spec.rb b/spec/services/error_tracking/issue_update_service_spec.rb
new file mode 100644
index 00000000000..ad1dafe6ccc
--- /dev/null
+++ b/spec/services/error_tracking/issue_update_service_spec.rb
@@ -0,0 +1,106 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe ErrorTracking::IssueUpdateService do
+  include_context 'sentry error tracking context'
+
+  let(:arguments) { { issue_id: 1234, status: 'resolved' } }
+
+  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
+    context 'with authorized user' do
+      context 'when update_issue returns success' do
+        let(:update_issue_response) { { updated: true } }
+
+        before do
+          expect(error_tracking_setting)
+            .to receive(:update_issue).and_return(update_issue_response)
+        end
+
+        it 'returns the response' do
+          expect(result).to eq(update_issue_response.merge(status: :success, closed_issue_iid: nil))
+        end
+
+        it 'updates any related issue' do
+          expect(subject).to receive(:update_related_issue)
+
+          result
+        end
+
+        context 'related issue and resolving' do
+          let(:issue) { create(:issue, project: project) }
+          let(:sentry_issue) { create(:sentry_issue, issue: issue) }
+          let(:arguments) { { issue_id: sentry_issue.sentry_issue_identifier, status: 'resolved' } }
+
+          let(:issue_close_service) { spy(:issue_close_service) }
+
+          before do
+            allow_any_instance_of(SentryIssueFinder)
+              .to receive(:execute)
+              .and_return(sentry_issue)
+
+            allow(Issues::CloseService)
+              .to receive(:new)
+              .and_return(issue_close_service)
+          end
+
+          after do
+            result
+          end
+
+          it 'closes the issue' do
+            expect(issue_close_service)
+              .to receive(:execute)
+              .with(issue, system_note: false)
+              .and_return(issue)
+          end
+
+          it 'creates a system note' do
+            expect(SystemNoteService).to receive(:close_after_error_tracking_resolve)
+          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
+
+      include_examples 'error tracking service sentry error handling', :update_issue
+    end
+
+    include_examples 'error tracking service unauthorized user'
+    include_examples 'error tracking service disabled'
+  end
+end
diff --git a/spec/services/error_tracking/list_issues_service_spec.rb b/spec/services/error_tracking/list_issues_service_spec.rb
index ecb6bcc541b..1e39146fd18 100644
--- a/spec/services/error_tracking/list_issues_service_spec.rb
+++ b/spec/services/error_tracking/list_issues_service_spec.rb
@@ -3,8 +3,8 @@
 require 'spec_helper'
 
 describe ErrorTracking::ListIssuesService do
-  let_it_be(:user) { create(:user) }
-  let_it_be(:project) { create(:project) }
+  include_context 'sentry error tracking context'
+
   let(:params) { { search_term: 'something', sort: 'last_seen', cursor: 'some-cursor' } }
   let(:list_sentry_issues_args) do
     {
@@ -16,22 +16,8 @@ describe ErrorTracking::ListIssuesService do
     }
   end
 
-  let(:sentry_url) { 'https://sentrytest.gitlab.com/api/0/projects/sentry-org/sentry-project' }
-  let(:token) { 'test-token' }
-  let(:result) { subject.execute }
-
-  let(:error_tracking_setting) do
-    create(:project_error_tracking_setting, api_url: sentry_url, token: token, project: project)
-  end
-
   subject { described_class.new(project, user, params) }
 
-  before do
-    expect(project).to receive(:error_tracking_setting).at_least(:once).and_return(error_tracking_setting)
-
-    project.add_reporter(user)
-  end
-
   describe '#execute' do
     context 'with authorized user' do
       context 'when list_sentry_issues returns issues' do
diff --git a/spec/support/shared_contexts/sentry_error_tracking_shared_context.rb b/spec/support/shared_contexts/sentry_error_tracking_shared_context.rb
new file mode 100644
index 00000000000..ba729f21e58
--- /dev/null
+++ b/spec/support/shared_contexts/sentry_error_tracking_shared_context.rb
@@ -0,0 +1,22 @@
+# frozen_string_literal: true
+
+shared_context 'sentry error tracking context' do
+  let_it_be(:user) { create(:user) }
+  let_it_be(:project) { create(:project) }
+
+  let(:sentry_url) { 'https://sentrytest.gitlab.com/api/0/projects/sentry-org/sentry-project' }
+  let(:token) { 'test-token' }
+  let(:result) { subject.execute }
+
+  subject { described_class.new(project, user) }
+
+  let(:error_tracking_setting) do
+    create(:project_error_tracking_setting, api_url: sentry_url, token: token, project: project)
+  end
+
+  before do
+    expect(project).to receive(:error_tracking_setting).at_least(:once).and_return(error_tracking_setting)
+
+    project.add_reporter(user)
+  end
+end