Add latest changes from gitlab-org/gitlab@master

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

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

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

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

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

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,

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

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

changelogs/unreleased/39825-close-issue-after-error-resolve.yml

+---
+title: Close Issue when resolving corresponding Sentry error
+merge_request: 22744
+author:
+type: added

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

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 ""

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

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"] }
       }
     }
   },

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

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

spec/services/error_tracking/issue_update_service_spec.rb

+# 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

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

spec/support/shared_contexts/sentry_error_tracking_shared_context.rb

+# 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