diff options
18 files changed, 62 insertions, 219 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 76411d59853..a147bd438b4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,6 @@ entry. ## 12.7.1 -- No changes. ### Fixed (6 changes) - Fix loading of sub-epics caused by wrong subscription check. !23184 @@ -1 +1 @@ -12.7.1 +12.7.1-ee diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index 2789152e175..e38eef527be 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -222,7 +222,6 @@ 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 4fe01716704..430d9952332 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 = perform + response = fetch rescue Sentry::Client::Error => e return error(e.message, :bad_request) rescue Sentry::Client::MissingKeysError => e @@ -22,7 +22,7 @@ module ErrorTracking private - def perform + def fetch raise NotImplementedError, "#{self.class} does not implement #{__method__}" end @@ -62,9 +62,5 @@ 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 31fb6a9618c..368cd4517fc 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 perform + def fetch 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 dd6b7f8285f..b6ad8f8028b 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 perform + def fetch 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 db754d54fef..e433b4a11f2 100644 --- a/app/services/error_tracking/issue_update_service.rb +++ b/app/services/error_tracking/issue_update_service.rb @@ -4,16 +4,6 @@ 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], @@ -21,58 +11,12 @@ 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?, - 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? + { updated: response[:updated].present? } end end end diff --git a/app/services/error_tracking/list_issues_service.rb b/app/services/error_tracking/list_issues_service.rb index d34ea8aa3b0..132e9dfa7bd 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 perform + def fetch 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 6523a66bbed..09a0b952e84 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 perform + def fetch 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 033c80fd8ed..38e0a7d34ad 100644 --- a/app/services/system_note_service.rb +++ b/app/services/system_note_service.rb @@ -99,12 +99,6 @@ 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/locale/gitlab.pot b/locale/gitlab.pot index bc0717560d0..af51d4af448 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -22723,9 +22723,6 @@ 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 22826938de2..588c4b05528 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, closed_issue_iid: 1234) + .and_return(status: :success, updated: true) 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 75f2c1152d9..72514ce647d 100644 --- a/spec/fixtures/api/schemas/error_tracking/update_issue.json +++ b/spec/fixtures/api/schemas/error_tracking/update_issue.json @@ -6,15 +6,9 @@ "properties" : { "result": { "type": "object", - "required" : [ - "status", - "updated", - "closed_issue_iid" - ], "properties": { "status": { "type": "string" }, - "updated": { "type": "boolean" }, - "closed_issue_iid": { "type": ["integer", "null"] } + "updated": { "type": "boolean" } } } }, diff --git a/spec/services/error_tracking/issue_details_service_spec.rb b/spec/services/error_tracking/issue_details_service_spec.rb index 26bb3b44126..4d5505bb5a9 100644 --- a/spec/services/error_tracking/issue_details_service_spec.rb +++ b/spec/services/error_tracking/issue_details_service_spec.rb @@ -3,7 +3,24 @@ require 'spec_helper' describe ErrorTracking::IssueDetailsService do - include_context 'sentry error tracking context' + 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 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 7f53eabd717..cda15042814 100644 --- a/spec/services/error_tracking/issue_latest_event_service_spec.rb +++ b/spec/services/error_tracking/issue_latest_event_service_spec.rb @@ -3,7 +3,24 @@ require 'spec_helper' describe ErrorTracking::IssueLatestEventService do - include_context 'sentry error tracking context' + 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 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 deleted file mode 100644 index ad1dafe6ccc..00000000000 --- a/spec/services/error_tracking/issue_update_service_spec.rb +++ /dev/null @@ -1,106 +0,0 @@ -# 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 1e39146fd18..ecb6bcc541b 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 - include_context 'sentry error tracking context' - + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project) } let(:params) { { search_term: 'something', sort: 'last_seen', cursor: 'some-cursor' } } let(:list_sentry_issues_args) do { @@ -16,8 +16,22 @@ 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 deleted file mode 100644 index ba729f21e58..00000000000 --- a/spec/support/shared_contexts/sentry_error_tracking_shared_context.rb +++ /dev/null @@ -1,22 +0,0 @@ -# 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 |