Add latest changes from gitlab-org/gitlab@master

spec/fixtures/config/mail_room_disabled.yml

@@ -9,3 +9,14 @@ test:
     ssl: true
     start_tls: false
     mailbox: "inbox"
+
+  service_desk_email:
+    enabled: false
+    address: "gitlab-incoming+%{key}@gmail.com"
+    user: "gitlab-incoming@gmail.com"
+    password: "[REDACTED]"
+    host: "imap.gmail.com"
+    port: 993
+    ssl: true
+    start_tls: false
+    mailbox: "inbox"

spec/fixtures/config/mail_room_enabled.yml

@@ -9,3 +9,14 @@ test:
     ssl: true
     start_tls: false
     mailbox: "inbox"
+
+  service_desk_email:
+    enabled: true
+    address: "gitlab-incoming+%{key}@gmail.com"
+    user: "gitlab-incoming@gmail.com"
+    password: "[REDACTED]"
+    host: "imap.gmail.com"
+    port: 993
+    ssl: true
+    start_tls: false
+    mailbox: "inbox"

spec/frontend/error_tracking/components/error_details_spec.js

 import { createLocalVue, shallowMount } from '@vue/test-utils';
 import Vuex from 'vuex';
+import { __ } from '~/locale';
 import { GlLoadingIcon, GlLink, GlBadge, GlFormInput } from '@gitlab/ui';
 import LoadingButton from '~/vue_shared/components/loading_button.vue';
 import Stacktrace from '~/error_tracking/components/stacktrace.vue';
 import ErrorDetails from '~/error_tracking/components/error_details.vue';
-import { severityLevel, severityLevelVariant } from '~/error_tracking/components/constants';
+import {
+  severityLevel,
+  severityLevelVariant,
+  errorStatus,
+} from '~/error_tracking/components/constants';
 const localVue = createLocalVue();
 localVue.use(Vuex);
@@ -56,6 +61,8 @@ describe('ErrorDetails', () => {
     actions = {
       startPollingDetails: () => {},
       startPollingStacktrace: () => {},
+      updateIgnoreStatus: jest.fn(),
+      updateResolveStatus: jest.fn(),
     };
     getters = {
@@ -219,6 +226,96 @@ describe('ErrorDetails', () => {
       });
     });
+    describe('Status update', () => {
+      const findUpdateIgnoreStatusButton = () =>
+        wrapper.find('[data-qa-selector="update_ignore_status_button"]');
+      const findUpdateResolveStatusButton = () =>
+        wrapper.find('[data-qa-selector="update_resolve_status_button"]');
+
+      afterEach(() => {
+        actions.updateIgnoreStatus.mockClear();
+        actions.updateResolveStatus.mockClear();
+      });
+
+      describe('when error is unresolved', () => {
+        beforeEach(() => {
+          store.state.details.errorStatus = errorStatus.UNRESOLVED;
+          mountComponent();
+        });
+
+        it('displays Ignore and Resolve buttons', () => {
+          expect(findUpdateIgnoreStatusButton().text()).toBe(__('Ignore'));
+          expect(findUpdateResolveStatusButton().text()).toBe(__('Resolve'));
+        });
+
+        it('marks error as ignored when ignore button is clicked', () => {
+          findUpdateIgnoreStatusButton().trigger('click');
+          expect(actions.updateIgnoreStatus.mock.calls[0][1]).toEqual(
+            expect.objectContaining({ status: errorStatus.IGNORED }),
+          );
+        });
+
+        it('marks error as resolved when resolve button is clicked', () => {
+          findUpdateResolveStatusButton().trigger('click');
+          expect(actions.updateResolveStatus.mock.calls[0][1]).toEqual(
+            expect.objectContaining({ status: errorStatus.RESOLVED }),
+          );
+        });
+      });
+
+      describe('when error is ignored', () => {
+        beforeEach(() => {
+          store.state.details.errorStatus = errorStatus.IGNORED;
+          mountComponent();
+        });
+
+        it('displays Undo Ignore and Resolve buttons', () => {
+          expect(findUpdateIgnoreStatusButton().text()).toBe(__('Undo ignore'));
+          expect(findUpdateResolveStatusButton().text()).toBe(__('Resolve'));
+        });
+
+        it('marks error as unresolved when ignore button is clicked', () => {
+          findUpdateIgnoreStatusButton().trigger('click');
+          expect(actions.updateIgnoreStatus.mock.calls[0][1]).toEqual(
+            expect.objectContaining({ status: errorStatus.UNRESOLVED }),
+          );
+        });
+
+        it('marks error as resolved when resolve button is clicked', () => {
+          findUpdateResolveStatusButton().trigger('click');
+          expect(actions.updateResolveStatus.mock.calls[0][1]).toEqual(
+            expect.objectContaining({ status: errorStatus.RESOLVED }),
+          );
+        });
+      });
+
+      describe('when error is resolved', () => {
+        beforeEach(() => {
+          store.state.details.errorStatus = errorStatus.RESOLVED;
+          mountComponent();
+        });
+
+        it('displays Ignore and Unresolve buttons', () => {
+          expect(findUpdateIgnoreStatusButton().text()).toBe(__('Ignore'));
+          expect(findUpdateResolveStatusButton().text()).toBe(__('Unresolve'));
+        });
+
+        it('marks error as ignored when ignore button is clicked', () => {
+          findUpdateIgnoreStatusButton().trigger('click');
+          expect(actions.updateIgnoreStatus.mock.calls[0][1]).toEqual(
+            expect.objectContaining({ status: errorStatus.IGNORED }),
+          );
+        });
+
+        it('marks error as unresolved when unresolve button is clicked', () => {
+          findUpdateResolveStatusButton().trigger('click');
+          expect(actions.updateResolveStatus.mock.calls[0][1]).toEqual(
+            expect.objectContaining({ status: errorStatus.UNRESOLVED }),
+          );
+        });
+      });
+    });
+
     describe('GitLab issue link', () => {
       const gitlabIssue = 'https://gitlab.example.com/issues/1';
       const findGitLabLink = () => wrapper.find(`[href="${gitlabIssue}"]`);

spec/frontend/error_tracking/store/actions_spec.js

@@ -10,6 +10,8 @@ jest.mock('~/flash.js');
 jest.mock('~/lib/utils/url_utility');
 let mock;
+const commit = jest.fn();
+const dispatch = jest.fn().mockResolvedValue();
 describe('Sentry common store actions', () => {
   beforeEach(() => {
@@ -20,26 +22,22 @@ describe('Sentry common store actions', () => {
     mock.restore();
     createFlash.mockClear();
   });
+  const endpoint = '123/stacktrace';
+  const redirectUrl = '/list';
+  const status = 'resolved';
+  const params = { endpoint, redirectUrl, status };
   describe('updateStatus', () => {
-    const endpoint = '123/stacktrace';
-    const redirectUrl = '/list';
-    const status = 'resolved';
-
     it('should handle successful status update', done => {
       mock.onPut().reply(200, {});
       testAction(
         actions.updateStatus,
-        { endpoint, redirectUrl, status },
+        params,
         {},
         [
           {
-            payload: true,
-            type: types.SET_UPDATING_RESOLVE_STATUS,
-          },
-          {
-            payload: false,
-            type: 'SET_UPDATING_RESOLVE_STATUS',
+            payload: 'resolved',
+            type: types.SET_ERROR_STATUS,
           },
         ],
         [],
@@ -52,27 +50,29 @@ describe('Sentry common store actions', () => {
     it('should handle unsuccessful status update', done => {
       mock.onPut().reply(400, {});
-      testAction(
-        actions.updateStatus,
-        { endpoint, redirectUrl, status },
-        {},
-        [
-          {
-            payload: true,
-            type: types.SET_UPDATING_RESOLVE_STATUS,
-          },
-          {
-            payload: false,
-            type: types.SET_UPDATING_RESOLVE_STATUS,
-          },
-        ],
-        [],
-        () => {
-          expect(visitUrl).not.toHaveBeenCalled();
-          expect(createFlash).toHaveBeenCalledTimes(1);
-          done();
-        },
-      );
+      testAction(actions.updateStatus, params, {}, [], [], () => {
+        expect(visitUrl).not.toHaveBeenCalled();
+        expect(createFlash).toHaveBeenCalledTimes(1);
+        done();
+      });
     });
   });
+
+  describe('updateResolveStatus', () => {
+    it('handles status update', () =>
+      actions.updateResolveStatus({ commit, dispatch }, params).then(() => {
+        expect(commit).toHaveBeenCalledWith(types.SET_UPDATING_RESOLVE_STATUS, true);
+        expect(commit).toHaveBeenCalledWith(types.SET_UPDATING_RESOLVE_STATUS, false);
+        expect(dispatch).toHaveBeenCalledWith('updateStatus', params);
+      }));
+  });
+
+  describe('updateIgnoreStatus', () => {
+    it('handles status update', () =>
+      actions.updateIgnoreStatus({ commit, dispatch }, params).then(() => {
+        expect(commit).toHaveBeenCalledWith(types.SET_UPDATING_IGNORE_STATUS, true);
+        expect(commit).toHaveBeenCalledWith(types.SET_UPDATING_IGNORE_STATUS, false);
+        expect(dispatch).toHaveBeenCalledWith('updateStatus', params);
+      }));
+  });
 });

spec/graphql/types/error_tracking/sentry_error_collection_type_spec.rb

@@ -12,6 +12,7 @@ describe GitlabSchema.types['SentryErrorCollection'] do
       errors
       detailed_error
       external_url
+      error_stack_trace
     ]
     is_expected.to have_graphql_fields(*expected_fields)

spec/graphql/types/error_tracking/sentry_error_stack_trace_entry_type_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe GitlabSchema.types['SentryErrorStackTraceEntry'] do
+  it { expect(described_class.graphql_name).to eq('SentryErrorStackTraceEntry') }
+
+  it 'exposes the expected fields' do
+    expected_fields = %i[
+      function
+      col
+      line
+      file_name
+      trace_context
+    ]
+
+    is_expected.to have_graphql_fields(*expected_fields)
+  end
+end

spec/graphql/types/error_tracking/sentry_error_stack_trace_type_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe GitlabSchema.types['SentryErrorStackTrace'] do
+  it { expect(described_class.graphql_name).to eq('SentryErrorStackTrace') }
+
+  it { expect(described_class).to require_graphql_authorizations(:read_sentry_issue) }
+
+  it 'exposes the expected fields' do
+    expected_fields = %i[
+      issue_id
+      date_received
+      stack_trace_entries
+    ]
+
+    is_expected.to have_graphql_fields(*expected_fields)
+  end
+end

spec/helpers/projects/error_tracking_helper_spec.rb

@@ -83,7 +83,6 @@ describe Projects::ErrorTrackingHelper do
   describe '#error_details_data' do
     let(:issue_id) { 1234 }
     let(:route_params) { [project.owner, project, issue_id, { format: :json }] }
-    let(:list_path) { project_error_tracking_index_path(project) }
     let(:details_path) { details_namespace_project_error_tracking_index_path(*route_params) }
     let(:project_path) { project.full_path }
     let(:stack_trace_path) { stack_trace_namespace_project_error_tracking_index_path(*route_params) }
@@ -91,10 +90,6 @@ describe Projects::ErrorTrackingHelper do
     let(:result) { helper.error_details_data(project, issue_id) }
-    it 'returns the correct list path' do
-      expect(result['list-path']).to eq list_path
-    end
-
     it 'returns the correct issue id' do
       expect(result['issue-id']).to eq issue_id
     end

spec/lib/gitlab/mail_room/mail_room_spec.rb

@@ -4,9 +4,10 @@ require 'spec_helper'
 describe Gitlab::MailRoom do
   let(:default_port) { 143 }
-  let(:default_config) do
+  let(:yml_config) do
     {
-      enabled: false,
+      enabled: true,
+      address: 'address@example.com',
       port: default_port,
       ssl: false,
       start_tls: false,
@@ -16,71 +17,73 @@ describe Gitlab::MailRoom do
     }
   end
-  shared_examples_for 'only truthy if both enabled and address are truthy' do |target_proc|
-    context 'with both enabled and address as truthy values' do
-      it 'is truthy' do
-        stub_config(enabled: true, address: 'localhost')
-        expect(target_proc.call).to be_truthy
-      end
-    end
-
-    context 'with address only as truthy' do
-      it 'is falsey' do
-        stub_config(enabled: false, address: 'localhost')
-
-        expect(target_proc.call).to be_falsey
-      end
-    end
-    context 'with enabled only as truthy' do
-      it 'is falsey' do
-        stub_config(enabled: true, address: nil)
-        expect(target_proc.call).to be_falsey
-      end
+  let(:custom_config) { {} }
+  let(:incoming_email_config) { yml_config.merge(custom_config) }
+  let(:service_desk_email_config) { yml_config.merge(custom_config) }
+  let(:configs) do
+    {
+      incoming_email: incoming_email_config,
+      service_desk_email: service_desk_email_config
+    }
+  end
+  before do
+    described_class.instance_variable_set(:@enabled_configs, nil)
+  end
+  describe '#enabled_configs' do
+    before do
+      allow(described_class).to receive(:load_yaml).and_return(configs)
     end
-    context 'with neither address nor enabled as truthy' do
-      it 'is falsey' do
-        stub_config(enabled: false, address: nil)
-
-        expect(target_proc.call).to be_falsey
+    context 'when both email and address is set' do
+      it 'returns email configs' do
+        expect(described_class.enabled_configs.size).to eq(2)
       end
     end
-  end
-
-  before do
-    described_class.reset_config!
-    allow(File).to receive(:exist?).and_return true
-  end
-  describe '#config' do
-    context 'if the yml file cannot be found' do
+    context 'when the yml file cannot be found' do
       before do
-        allow(File).to receive(:exist?).and_return false
+        allow(described_class).to receive(:config_file).and_return('not_existing_file')
       end
-      it 'returns an empty hash' do
-        expect(described_class.config).to be_empty
+      it 'returns an empty list' do
+        expect(described_class.enabled_configs).to be_empty
       end
     end
-    before do
-      allow(described_class).to receive(:load_from_yaml).and_return(default_config)
+    context 'when email is disabled' do
+      let(:custom_config) { { enabled: false } }
+
+      it 'returns an empty list' do
+        expect(described_class.enabled_configs).to be_empty
+      end
     end
-    it 'sets up config properly' do
-      expected_result = default_config
-      expect(described_class.config).to match expected_result
+    context 'when email is enabled but address is not set' do
+      let(:custom_config) { { enabled: true, address: '' } }
+      it 'returns an empty list' do
+        expect(described_class.enabled_configs).to be_empty
+      end
     end
     context 'when a config value is missing from the yml file' do
+      let(:yml_config) { {} }
+      let(:custom_config) { { enabled: true, address: 'address@example.com' } }
+
       it 'overwrites missing values with the default' do
-        stub_config(port: nil)
-        expect(described_class.config[:port]).to eq default_port
+        expect(described_class.enabled_configs.first[:port]).to eq(Gitlab::MailRoom::DEFAULT_CONFIG[:port])
+      end
+    end
+
+    context 'when only incoming_email config is present' do
+      let(:configs) { { incoming_email: incoming_email_config } }
+      it 'returns only encoming_email' do
+        expect(described_class.enabled_configs.size).to eq(1)
+        expect(described_class.enabled_configs.first[:worker]).to eq('EmailReceiverWorker')
       end
     end
@@ -91,50 +94,31 @@ describe Gitlab::MailRoom do
         allow(Gitlab::Redis::Queues).to receive(:new).and_return(fake_redis_queues)
       end
-      target_proc = proc { described_class.config[:redis_url] }
-      it_behaves_like 'only truthy if both enabled and address are truthy', target_proc
+      it 'sets redis config' do
+        config = described_class.enabled_configs.first
+        expect(config[:redis_url]).to eq('localhost')
+        expect(config[:sentinels]).to eq('yes, them')
+      end
     end
     describe 'setting up the log path' do
       context 'if the log path is a relative path' do
-        it 'expands the log path to an absolute value' do
-          stub_config(log_path: 'tiny_log.log')
-          new_path = Pathname.new(described_class.config[:log_path])
+        let(:custom_config) { { log_path: 'tiny_log.log' } }
+        it 'expands the log path to an absolute value' do
+          new_path = Pathname.new(described_class.enabled_configs.first[:log_path])
           expect(new_path.absolute?).to be_truthy
         end
       end
       context 'if the log path is absolute path' do
-        it 'leaves the path as-is' do
-          new_path = '/dev/null'
-          stub_config(log_path: new_path)
-          expect(described_class.config[:log_path]).to eq new_path
+        let(:custom_config) { { log_path: '/dev/null' } }
+        it 'leaves the path as-is' do
+          expect(described_class.enabled_configs.first[:log_path]).to eq '/dev/null'
         end
       end
     end
   end
-
-  describe '#enabled?' do
-    target_proc = proc { described_class.enabled? }
-
-    it_behaves_like 'only truthy if both enabled and address are truthy', target_proc
-  end
-
-  describe '#reset_config?' do
-    it 'resets config' do
-      described_class.instance_variable_set(:@config, { some_stuff: 'hooray' })
-
-      described_class.reset_config!
-
-      expect(described_class.instance_variable_get(:@config)).to be_nil
-    end
-  end
-
-  def stub_config(override_values)
-    modified_config = default_config.merge(override_values)
-    allow(described_class).to receive(:load_from_yaml).and_return(modified_config)
-  end
 end

spec/requests/api/graphql/project/error_tracking/sentry_errors_request_spec.rb

@@ -40,8 +40,8 @@ describe 'sentry errors requests' do
         post_graphql(query, current_user: current_user)
       end
-      it "is expected to return an empty error" do
-        expect(error_data).to eq nil
+      it 'is expected to return an empty error' do
+        expect(error_data).to be_nil
       end
     end
@@ -49,7 +49,7 @@ describe 'sentry errors requests' do
       before do
         allow_any_instance_of(ErrorTracking::ProjectErrorTrackingSetting)
           .to receive(:issue_details)
-          .and_return({ issue: sentry_detailed_error })
+          .and_return(issue: sentry_detailed_error)
         post_graphql(query, current_user: current_user)
       end
@@ -72,8 +72,8 @@ describe 'sentry errors requests' do
       context 'user does not have permission' do
         let(:current_user) { create(:user) }
-        it "is expected to return an empty error" do
-          expect(error_data).to eq nil
+        it 'is expected to return an empty error' do
+          expect(error_data).to be_nil
         end
       end
     end
@@ -82,13 +82,13 @@ describe 'sentry errors requests' do
       before do
         expect_any_instance_of(ErrorTracking::ProjectErrorTrackingSetting)
           .to receive(:issue_details)
-          .and_return({ error: 'error message' })
+          .and_return(error: 'error message')
         post_graphql(query, current_user: current_user)
       end
       it 'is expected to handle the error and return nil' do
-        expect(error_data).to eq nil
+        expect(error_data).to be_nil
       end
     end
   end
@@ -132,8 +132,8 @@ describe 'sentry errors requests' do
         post_graphql(query, current_user: current_user)
       end
-      it "is expected to return nil" do
-        expect(error_data).to eq nil
+      it 'is expected to return nil' do
+        expect(error_data).to be_nil
       end
     end
@@ -141,7 +141,7 @@ describe 'sentry errors requests' do
       before do
         expect_any_instance_of(ErrorTracking::ProjectErrorTrackingSetting)
           .to receive(:list_sentry_issues)
-          .and_return({ issues: [sentry_error], pagination: pagination })
+          .and_return(issues: [sentry_error], pagination: pagination)
         post_graphql(query, current_user: current_user)
       end
@@ -174,17 +174,82 @@ describe 'sentry errors requests' do
       end
     end
-    context "sentry api itself errors out" do
+    context 'sentry api itself errors out' do
       before do
         expect_any_instance_of(ErrorTracking::ProjectErrorTrackingSetting)
           .to receive(:list_sentry_issues)
-          .and_return({ error: 'error message' })
+          .and_return(error: 'error message')
         post_graphql(query, current_user: current_user)
       end
       it 'is expected to handle the error and return nil' do
-        expect(error_data).to eq nil
+        expect(error_data).to be_nil
+      end
+    end
+  end
+
+  describe 'getting a stack trace' do
+    let_it_be(:sentry_stack_trace) { build(:error_tracking_error_event) }
+    let(:sentry_gid) { Gitlab::ErrorTracking::DetailedError.new(id: 1).to_global_id.to_s }
+
+    let(:stack_trace_fields) do
+      all_graphql_fields_for('SentryErrorStackTrace'.classify)
+    end
+
+    let(:fields) do
+      query_graphql_field('errorStackTrace', { id: sentry_gid }, stack_trace_fields)
+    end
+
+    let(:stack_trace_data) { graphql_data.dig('project', 'sentryErrors', 'errorStackTrace') }
+
+    it_behaves_like 'a working graphql query' do
+      before do
+        post_graphql(query, current_user: current_user)
+      end
+    end
+
+    context 'when data is loading via reactive cache' do
+      before do
+        post_graphql(query, current_user: current_user)
+      end
+
+      it 'is expected to return an empty error' do
+        expect(stack_trace_data).to be_nil
+      end
+    end
+
+    context 'reactive cache returns data' do
+      before do
+        allow_any_instance_of(ErrorTracking::ProjectErrorTrackingSetting)
+          .to receive(:issue_latest_event)
+          .and_return(latest_event: sentry_stack_trace)
+
+        post_graphql(query, current_user: current_user)
+      end
+
+      it_behaves_like 'setting stack trace error'
+
+      context 'user does not have permission' do
+        let(:current_user) { create(:user) }
+
+        it 'is expected to return an empty error' do
+          expect(stack_trace_data).to be_nil
+        end
+      end
+    end
+
+    context 'sentry api returns an error' do
+      before do
+        expect_any_instance_of(ErrorTracking::ProjectErrorTrackingSetting)
+          .to receive(:issue_latest_event)
+          .and_return(error: 'error message')
+
+        post_graphql(query, current_user: current_user)
+      end
+
+      it 'is expected to handle the error and return nil' do
+        expect(stack_trace_data).to be_nil
       end
     end
   end

spec/support/shared_examples/error_tracking_shared_examples.rb

@@ -3,11 +3,34 @@
 RSpec.shared_examples 'setting sentry error data' do
   it 'sets the sentry error data correctly' do
     aggregate_failures 'testing the sentry error is correct' do
-      expect(error['id']).to eql sentry_error.to_global_id.to_s
-      expect(error['sentryId']).to eql sentry_error.id.to_s
-      expect(error['status']).to eql sentry_error.status.upcase
-      expect(error['firstSeen']).to eql sentry_error.first_seen
-      expect(error['lastSeen']).to eql sentry_error.last_seen
+      expect(error['id']).to eq sentry_error.to_global_id.to_s
+      expect(error['sentryId']).to eq sentry_error.id.to_s
+      expect(error['status']).to eq sentry_error.status.upcase
+      expect(error['firstSeen']).to eq sentry_error.first_seen
+      expect(error['lastSeen']).to eq sentry_error.last_seen
+    end
+  end
+end
+
+RSpec.shared_examples 'setting stack trace error' do
+  it 'sets the stack trace data correctly' do
+    aggregate_failures 'testing the stack trace is correct' do
+      expect(stack_trace_data['dateReceived']).to eq(sentry_stack_trace.date_received)
+      expect(stack_trace_data['issueId']).to eq(sentry_stack_trace.issue_id)
+      expect(stack_trace_data['stackTraceEntries']).to be_an_instance_of(Array)
+      expect(stack_trace_data['stackTraceEntries'].size).to eq(sentry_stack_trace.stack_trace_entries.size)
+    end
+  end
+
+  it 'sets the stack trace entry data correctly' do
+    aggregate_failures 'testing the stack trace entry is correct' do
+      stack_trace_entry = stack_trace_data['stackTraceEntries'].first
+      model_entry = sentry_stack_trace.stack_trace_entries.first
+
+      expect(stack_trace_entry['function']).to eq model_entry['function']
+      expect(stack_trace_entry['col']).to eq model_entry['colNo']
+      expect(stack_trace_entry['line']).to eq model_entry['lineNo'].to_s
+      expect(stack_trace_entry['fileName']).to eq model_entry['filename']
     end
   end
 end