diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2023-05-02 21:18:39 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2023-05-02 21:18:39 +0300 |
commit | b9ce0fe1e6311105b7a748126621f9bfbe37fb2e (patch) | |
tree | c73b711a72de036cf3f48be9365038fea171c8c6 /spec | |
parent | 6f991190fe4dbb93070b090a9a31d71b25e8101d (diff) |
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'spec')
40 files changed, 1781 insertions, 270 deletions
diff --git a/spec/controllers/search_controller_spec.rb b/spec/controllers/search_controller_spec.rb index ff24b754d7a..497e2d84f4f 100644 --- a/spec/controllers/search_controller_spec.rb +++ b/spec/controllers/search_controller_spec.rb @@ -38,6 +38,41 @@ RSpec.describe SearchController, feature_category: :global_search do it_behaves_like 'with external authorization service enabled', :show, { search: 'hello' } it_behaves_like 'support for active record query timeouts', :show, { search: 'hello' }, :search_objects, :html + describe 'rate limit scope' do + it 'uses current_user and search scope' do + %w[projects blobs users issues merge_requests].each do |scope| + expect(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).with(:search_rate_limit, scope: [user, scope]) + get :show, params: { search: 'hello', scope: scope } + end + end + + it 'uses just current_user when no search scope is used' do + expect(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).with(:search_rate_limit, scope: [user]) + get :show, params: { search: 'hello' } + end + + it 'uses just current_user when search scope is abusive' do + expect(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).with(:search_rate_limit, scope: [user]) + get(:show, params: { search: 'hello', scope: 'hack-the-mainframe' }) + + expect(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).with(:search_rate_limit, scope: [user]) + get :show, params: { search: 'hello', scope: 'blobs' * 1000 } + end + + context 'when search_rate_limited_scopes feature flag is disabled' do + before do + stub_feature_flags(search_rate_limited_scopes: false) + end + + it 'uses just current_user' do + %w[projects blobs users issues merge_requests].each do |scope| + expect(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).with(:search_rate_limit, scope: [user]) + get :show, params: { search: 'hello', scope: scope } + end + end + end + end + context 'uses the right partials depending on scope' do using RSpec::Parameterized::TableSyntax render_views @@ -345,6 +380,36 @@ RSpec.describe SearchController, feature_category: :global_search do expect(json_response).to eq({ 'count' => '1' }) end + describe 'rate limit scope' do + it 'uses current_user and search scope' do + %w[projects blobs users issues merge_requests].each do |scope| + expect(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).with(:search_rate_limit, scope: [user, scope]) + get :count, params: { search: 'hello', scope: scope } + end + end + + it 'uses just current_user when search scope is abusive' do + expect(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).with(:search_rate_limit, scope: [user]) + get :count, params: { search: 'hello', scope: 'hack-the-mainframe' } + + expect(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).with(:search_rate_limit, scope: [user]) + get :count, params: { search: 'hello', scope: 'blobs' * 1000 } + end + + context 'when search_rate_limited_scopes feature flag is disabled' do + before do + stub_feature_flags(search_rate_limited_scopes: false) + end + + it 'uses just current_user' do + %w[projects blobs users issues merge_requests].each do |scope| + expect(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).with(:search_rate_limit, scope: [user]) + get :count, params: { search: 'hello', scope: scope } + end + end + end + end + it 'raises an error if search term is missing' do expect do get :count, params: { scope: 'projects' } @@ -406,6 +471,36 @@ RSpec.describe SearchController, feature_category: :global_search do expect(json_response).to match_array([]) end + describe 'rate limit scope' do + it 'uses current_user and search scope' do + %w[projects blobs users issues merge_requests].each do |scope| + expect(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).with(:search_rate_limit, scope: [user, scope]) + get :autocomplete, params: { term: 'hello', scope: scope } + end + end + + it 'uses just current_user when search scope is abusive' do + expect(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).with(:search_rate_limit, scope: [user]) + get :autocomplete, params: { term: 'hello', scope: 'hack-the-mainframe' } + + expect(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).with(:search_rate_limit, scope: [user]) + get :autocomplete, params: { term: 'hello', scope: 'blobs' * 1000 } + end + + context 'when search_rate_limited_scopes feature flag is disabled' do + before do + stub_feature_flags(search_rate_limited_scopes: false) + end + + it 'uses just current_user' do + %w[projects blobs users issues merge_requests].each do |scope| + expect(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).with(:search_rate_limit, scope: [user]) + get :autocomplete, params: { term: 'hello', scope: scope } + end + end + end + end + it_behaves_like 'rate limited endpoint', rate_limit_key: :search_rate_limit do let(:current_user) { user } @@ -525,6 +620,11 @@ RSpec.describe SearchController, feature_category: :global_search do get endpoint, params: params.merge(project_id: project.id) end end + + it 'uses request IP as rate limiting scope' do + expect(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).with(:search_rate_limit_unauthenticated, scope: [request.ip]) + get endpoint, params: params.merge(project_id: project.id) + end end end diff --git a/spec/features/admin/admin_abuse_reports_spec.rb b/spec/features/admin/admin_abuse_reports_spec.rb index 0620221051e..9fe72b981f1 100644 --- a/spec/features/admin/admin_abuse_reports_spec.rb +++ b/spec/features/admin/admin_abuse_reports_spec.rb @@ -91,51 +91,6 @@ RSpec.describe "Admin::AbuseReports", :js, feature_category: :shared do expect(report_rows[1].text).to include(report_text(open_report2)) end - it 'can be actioned on' do - open_actions_dropdown(report_rows[0]) - - expect(page).to have_content('Remove user & report') - expect(page).to have_content('Block user') - expect(page).to have_content('Remove report') - - # Remove a report - click_button('Remove report') - wait_for_requests - - expect_displayed_reports_count(1) - expect_report_shown(open_report) - - # Block reported user - open_actions_dropdown(report_rows[0]) - - click_button('Block user') - expect(page).to have_content('USER WILL BE BLOCKED! Are you sure?') - - click_button('OK') - wait_for_requests - - expect(page).to have_content('Successfully blocked') - expect(open_report.user.reload.blocked?).to eq true - - open_actions_dropdown(report_rows[0]) - - expect(page).to have_content('Already blocked') - expect(page).not_to have_content('Block user') - - # Remove user & report - click_button('Remove user & report') - expect(page).to have_content("USER #{open_report.user.name} WILL BE REMOVED! Are you sure?") - - click_button('OK') - expect_displayed_reports_count(0) - end - - def open_actions_dropdown(report_row) - within(report_row) do - find('[data-testid="base-dropdown-toggle"]').click - end - end - def report_rows page.all(abuse_report_row_selector) end diff --git a/spec/features/projects/labels/update_prioritization_spec.rb b/spec/features/projects/labels/update_prioritization_spec.rb index b527b8926a0..4af5dd380c1 100644 --- a/spec/features/projects/labels/update_prioritization_spec.rb +++ b/spec/features/projects/labels/update_prioritization_spec.rb @@ -101,7 +101,7 @@ RSpec.describe 'Prioritize labels', feature_category: :team_planning do expect(page).to have_content 'wontfix' # Sort labels - drag_to(selector: '.label-list-item', from_index: 1, to_index: 2) + drag_to(selector: '.label-list-item .label-content', from_index: 1, to_index: 2) page.within('.prioritized-labels') do expect(first('.label-list-item')).to have_content('feature') diff --git a/spec/frontend/admin/abuse_reports/components/abuse_report_details_spec.js b/spec/frontend/admin/abuse_reports/components/abuse_report_details_spec.js deleted file mode 100644 index b89bbac0196..00000000000 --- a/spec/frontend/admin/abuse_reports/components/abuse_report_details_spec.js +++ /dev/null @@ -1,53 +0,0 @@ -import { GlButton, GlCollapse } from '@gitlab/ui'; -import { nextTick } from 'vue'; -import { shallowMount } from '@vue/test-utils'; -import AbuseReportDetails from '~/admin/abuse_reports/components/abuse_report_details.vue'; -import { getTimeago } from '~/lib/utils/datetime_utility'; -import { mockAbuseReports } from '../mock_data'; - -describe('AbuseReportDetails', () => { - let wrapper; - const report = mockAbuseReports[0]; - - const findToggleButton = () => wrapper.findComponent(GlButton); - const findCollapsible = () => wrapper.findComponent(GlCollapse); - - const createComponent = () => { - wrapper = shallowMount(AbuseReportDetails, { - propsData: { - report, - }, - }); - }; - - describe('default', () => { - beforeEach(() => { - createComponent(); - }); - - it('renders toggle button with the correct text', () => { - expect(findToggleButton().text()).toEqual('Show details'); - }); - - it('renders collapsed GlCollapse containing the report details', () => { - const collapsible = findCollapsible(); - expect(collapsible.attributes('visible')).toBeUndefined(); - - const userJoinedText = `User joined ${getTimeago().format(report.reportedUser.createdAt)}`; - expect(collapsible.text()).toMatch(userJoinedText); - expect(collapsible.text()).toMatch(report.message); - }); - }); - - describe('when toggled', () => { - it('expands GlCollapse and updates toggle text', async () => { - createComponent(); - - findToggleButton().vm.$emit('click'); - await nextTick(); - - expect(findToggleButton().text()).toEqual('Hide details'); - expect(findCollapsible().attributes('visible')).toBe('true'); - }); - }); -}); diff --git a/spec/frontend/admin/abuse_reports/components/abuse_report_row_spec.js b/spec/frontend/admin/abuse_reports/components/abuse_report_row_spec.js index 9876ee70e5e..f3cced81478 100644 --- a/spec/frontend/admin/abuse_reports/components/abuse_report_row_spec.js +++ b/spec/frontend/admin/abuse_reports/components/abuse_report_row_spec.js @@ -1,9 +1,6 @@ -import { GlSprintf, GlLink } from '@gitlab/ui'; import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; import setWindowLocation from 'helpers/set_window_location_helper'; -import AbuseReportDetails from '~/admin/abuse_reports/components/abuse_report_details.vue'; import AbuseReportRow from '~/admin/abuse_reports/components/abuse_report_row.vue'; -import AbuseReportActions from '~/admin/abuse_reports/components/abuse_report_actions.vue'; import ListItem from '~/vue_shared/components/registry/list_item.vue'; import { getTimeago } from '~/lib/utils/datetime_utility'; import { SORT_UPDATED_AT } from '~/admin/abuse_reports/constants'; @@ -13,19 +10,16 @@ describe('AbuseReportRow', () => { let wrapper; const mockAbuseReport = mockAbuseReports[0]; - const findLinks = () => wrapper.findAllComponents(GlLink); - const findAbuseReportActions = () => wrapper.findComponent(AbuseReportActions); const findListItem = () => wrapper.findComponent(ListItem); const findTitle = () => wrapper.findByTestId('title'); const findDisplayedDate = () => wrapper.findByTestId('abuse-report-date'); - const findAbuseReportDetails = () => wrapper.findComponent(AbuseReportDetails); - const createComponent = () => { + const createComponent = (props = {}) => { wrapper = shallowMountExtended(AbuseReportRow, { propsData: { report: mockAbuseReport, + ...props, }, - stubs: { GlSprintf }, }); }; @@ -37,19 +31,42 @@ describe('AbuseReportRow', () => { expect(findListItem().exists()).toBe(true); }); - it('displays correctly formatted title', () => { - const { reporter, reportedUser, category, reportedUserPath, reporterPath } = mockAbuseReport; - expect(findTitle().text()).toMatchInterpolatedText( - `${reportedUser.name} reported for ${category} by ${reporter.name}`, - ); + describe('title', () => { + const { reporter, reportedUser, category, reportPath } = mockAbuseReport; - const userLink = findLinks().at(0); - expect(userLink.text()).toEqual(reportedUser.name); - expect(userLink.attributes('href')).toEqual(reportedUserPath); + it('displays correctly formatted title', () => { + expect(findTitle().text()).toMatchInterpolatedText( + `${reportedUser.name} reported for ${category} by ${reporter.name}`, + ); + }); + + it('links to the details page', () => { + expect(findTitle().attributes('href')).toEqual(reportPath); + }); + + describe('when the reportedUser is missing', () => { + beforeEach(() => { + createComponent({ report: { ...mockAbuseReport, reportedUser: null } }); + }); + + it('displays correctly formatted title', () => { + expect(findTitle().text()).toMatchInterpolatedText( + `Deleted user reported for ${category} by ${reporter.name}`, + ); + }); + }); - const reporterLink = findLinks().at(1); - expect(reporterLink.text()).toEqual(reporter.name); - expect(reporterLink.attributes('href')).toEqual(reporterPath); + describe('when the reporter is missing', () => { + beforeEach(() => { + createComponent({ report: { ...mockAbuseReport, reporter: null } }); + }); + + it('displays correctly formatted title', () => { + expect(findTitle().text()).toMatchInterpolatedText( + `${reportedUser.name} reported for ${category} by Deleted user`, + ); + }); + }); }); describe('displayed date', () => { @@ -71,16 +88,4 @@ describe('AbuseReportRow', () => { }); }); }); - - it('renders AbuseReportDetails', () => { - expect(findAbuseReportDetails().exists()).toBe(true); - expect(findAbuseReportDetails().props('report')).toEqual(mockAbuseReport); - }); - - it('renders AbuseReportRowActions with the correct props', () => { - const actions = findAbuseReportActions(); - - expect(actions.exists()).toBe(true); - expect(actions.props('report')).toMatchObject(mockAbuseReport); - }); }); diff --git a/spec/frontend/admin/abuse_reports/mock_data.js b/spec/frontend/admin/abuse_reports/mock_data.js index ee9e56d043b..1ea6ea7d131 100644 --- a/spec/frontend/admin/abuse_reports/mock_data.js +++ b/spec/frontend/admin/abuse_reports/mock_data.js @@ -4,14 +4,7 @@ export const mockAbuseReports = [ createdAt: '2018-10-03T05:46:38.977Z', updatedAt: '2022-12-07T06:45:39.977Z', reporter: { name: 'Ms. Admin' }, - reportedUser: { name: 'Mr. Abuser', createdAt: '2017-09-01T05:46:38.977Z' }, - reportedUserPath: '/mr_abuser', - reporterPath: '/admin', - userBlocked: false, - blockUserPath: '/block/user/mr_abuser/path', - removeUserAndReportPath: '/remove/user/mr_abuser/and/report/path', - removeReportPath: '/remove/report/path', - message: 'message 1', + reportedUser: { name: 'Mr. Abuser' }, reportPath: '/admin/abuse_reports/1', }, { @@ -19,14 +12,7 @@ export const mockAbuseReports = [ createdAt: '2018-10-03T05:46:38.977Z', updatedAt: '2022-12-07T06:45:39.977Z', reporter: { name: 'Ms. Reporter' }, - reportedUser: { name: 'Mr. Phisher', createdAt: '2016-09-01T05:46:38.977Z' }, - reportedUserPath: '/mr_phisher', - reporterPath: '/admin', - userBlocked: false, - blockUserPath: '/block/user/mr_phisher/path', - removeUserAndReportPath: '/remove/user/mr_phisher/and/report/path', - removeReportPath: '/remove/report/path', - message: 'message 2', + reportedUser: { name: 'Mr. Phisher' }, reportPath: '/admin/abuse_reports/2', }, ]; diff --git a/spec/frontend/ci/pipeline_editor/components/commit/commit_section_spec.js b/spec/frontend/ci/pipeline_editor/components/commit/commit_section_spec.js index f8be035d33c..8834231aaef 100644 --- a/spec/frontend/ci/pipeline_editor/components/commit/commit_section_spec.js +++ b/spec/frontend/ci/pipeline_editor/components/commit/commit_section_spec.js @@ -4,6 +4,7 @@ import { mount } from '@vue/test-utils'; import Vue from 'vue'; import createMockApollo from 'helpers/mock_apollo_helper'; import waitForPromises from 'helpers/wait_for_promises'; +import { mockTracking } from 'helpers/tracking_helper'; import CommitForm from '~/ci/pipeline_editor/components/commit/commit_form.vue'; import CommitSection from '~/ci/pipeline_editor/components/commit/commit_section.vue'; import { @@ -11,12 +12,12 @@ import { COMMIT_ACTION_UPDATE, COMMIT_SUCCESS, COMMIT_SUCCESS_WITH_REDIRECT, + pipelineEditorTrackingOptions, } from '~/ci/pipeline_editor/constants'; import { resolvers } from '~/ci/pipeline_editor/graphql/resolvers'; import commitCreate from '~/ci/pipeline_editor/graphql/mutations/commit_ci_file.mutation.graphql'; import getCurrentBranch from '~/ci/pipeline_editor/graphql/queries/client/current_branch.query.graphql'; import updatePipelineEtag from '~/ci/pipeline_editor/graphql/mutations/client/update_pipeline_etag.mutation.graphql'; - import { mockCiConfigPath, mockCiYml, @@ -280,4 +281,43 @@ describe('Pipeline Editor | Commit section', () => { createComponent({ props: { 'scroll-to-commit-form': true } }); expect(findCommitForm().props('scrollToCommitForm')).toBe(true); }); + + describe('tracking', () => { + let trackingSpy; + const { actions, label } = pipelineEditorTrackingOptions; + + beforeEach(() => { + trackingSpy = mockTracking(undefined, wrapper.element, jest.spyOn); + }); + + describe('when user commit a new file', () => { + beforeEach(async () => { + mockMutateCommitData.mockResolvedValue(mockCommitCreateResponse); + createComponentWithApollo({ props: { isNewCiConfigFile: true } }); + await submitCommit(); + }); + + it('calls tracking event with the CREATE property', () => { + expect(trackingSpy).toHaveBeenCalledWith(undefined, actions.commitCiConfig, { + label, + property: COMMIT_ACTION_CREATE, + }); + }); + }); + + describe('when user commit an update to the CI file', () => { + beforeEach(async () => { + mockMutateCommitData.mockResolvedValue(mockCommitCreateResponse); + createComponentWithApollo({ props: { isNewCiConfigFile: false } }); + await submitCommit(); + }); + + it('calls the tracking event with the UPDATE property', () => { + expect(trackingSpy).toHaveBeenCalledWith(undefined, actions.commitCiConfig, { + label, + property: COMMIT_ACTION_UPDATE, + }); + }); + }); + }); }); diff --git a/spec/frontend/jobs/components/table/job_table_app_spec.js b/spec/frontend/jobs/components/table/job_table_app_spec.js index 1ca28fefd28..30f674f5ba7 100644 --- a/spec/frontend/jobs/components/table/job_table_app_spec.js +++ b/spec/frontend/jobs/components/table/job_table_app_spec.js @@ -117,6 +117,16 @@ describe('Job table app', () => { expect(wrapper.vm.$apollo.queries.jobs.refetch).toHaveBeenCalledTimes(1); }); + it('avoids refetch jobs query when scope has not changed', async () => { + jest.spyOn(wrapper.vm.$apollo.queries.jobs, 'refetch').mockImplementation(jest.fn()); + + expect(wrapper.vm.$apollo.queries.jobs.refetch).toHaveBeenCalledTimes(0); + + await findTabs().vm.$emit('fetchJobsByStatus', null); + + expect(wrapper.vm.$apollo.queries.jobs.refetch).toHaveBeenCalledTimes(0); + }); + describe('when infinite scrolling is triggered', () => { it('does not display a skeleton loader', () => { triggerInfiniteScroll(); diff --git a/spec/frontend/merge_request_tabs_spec.js b/spec/frontend/merge_request_tabs_spec.js index 8f92ab46714..3b8c9dd3bf3 100644 --- a/spec/frontend/merge_request_tabs_spec.js +++ b/spec/frontend/merge_request_tabs_spec.js @@ -384,12 +384,12 @@ describe('MergeRequestTabs', () => { }); }); - it('scrolls to 0, if no position is stored', () => { + it('does not scroll if no position is stored', () => { testContext.class.tabShown('unknownTab'); jest.advanceTimersByTime(250); - expect(window.scrollTo.mock.calls[0][0]).toEqual({ top: 0, left: 0, behavior: 'auto' }); + expect(window.scrollTo).not.toHaveBeenCalled(); }); }); }); diff --git a/spec/frontend/pages/admin/jobs/components/table/admin_job_table_app_spec.js b/spec/frontend/pages/admin/jobs/components/table/admin_job_table_app_spec.js index 44a5878e6f2..cc6f1c27142 100644 --- a/spec/frontend/pages/admin/jobs/components/table/admin_job_table_app_spec.js +++ b/spec/frontend/pages/admin/jobs/components/table/admin_job_table_app_spec.js @@ -129,6 +129,16 @@ describe('Job table app', () => { expect(wrapper.vm.$apollo.queries.jobs.refetch).toHaveBeenCalledTimes(1); }); + it('avoids refetch jobs query when scope has not changed', async () => { + jest.spyOn(wrapper.vm.$apollo.queries.jobs, 'refetch').mockImplementation(jest.fn()); + + expect(wrapper.vm.$apollo.queries.jobs.refetch).toHaveBeenCalledTimes(0); + + await findTabs().vm.$emit('fetchJobsByStatus', null); + + expect(wrapper.vm.$apollo.queries.jobs.refetch).toHaveBeenCalledTimes(0); + }); + describe('when infinite scrolling is triggered', () => { it('does not display a skeleton loader', () => { triggerInfiniteScroll(); diff --git a/spec/frontend/pages/admin/jobs/components/table/cells/project_cell_spec.js b/spec/frontend/pages/admin/jobs/components/table/cells/project_cell_spec.js new file mode 100644 index 00000000000..3366d60d9f3 --- /dev/null +++ b/spec/frontend/pages/admin/jobs/components/table/cells/project_cell_spec.js @@ -0,0 +1,32 @@ +import { GlLink } from '@gitlab/ui'; +import { shallowMount } from '@vue/test-utils'; +import ProjectCell from '~/pages/admin/jobs/components/table/cell/project_cell.vue'; +import { mockAllJobsNodes } from '../../../../../../jobs/mock_data'; + +const mockJob = mockAllJobsNodes[0]; + +describe('Project cell', () => { + let wrapper; + + const findProjectLink = () => wrapper.findComponent(GlLink); + + const createComponent = (props = {}) => { + wrapper = shallowMount(ProjectCell, { + propsData: { + ...props, + }, + }); + }; + + describe('Project Link', () => { + beforeEach(() => { + createComponent({ job: mockJob }); + }); + + it('shows and links to the project', () => { + expect(findProjectLink().exists()).toBe(true); + expect(findProjectLink().text()).toBe(mockJob.pipeline.project.fullPath); + expect(findProjectLink().attributes('href')).toBe(mockJob.pipeline.project.webUrl); + }); + }); +}); diff --git a/spec/frontend/token_access/outbound_token_access_spec.js b/spec/frontend/token_access/outbound_token_access_spec.js index 347ea1178bc..7f321495d72 100644 --- a/spec/frontend/token_access/outbound_token_access_spec.js +++ b/spec/frontend/token_access/outbound_token_access_spec.js @@ -44,15 +44,26 @@ describe('TokenAccess component', () => { const findAddProjectBtn = () => wrapper.findByRole('button', { name: 'Add project' }); const findRemoveProjectBtn = () => wrapper.findByRole('button', { name: 'Remove access' }); const findTokenDisabledAlert = () => wrapper.findByTestId('token-disabled-alert'); + const findDeprecationAlert = () => wrapper.findByTestId('deprecation-alert'); + const findProjectPathInput = () => wrapper.findByTestId('project-path-input'); const createMockApolloProvider = (requestHandlers) => { return createMockApollo(requestHandlers); }; - const createComponent = (requestHandlers, mountFn = shallowMountExtended) => { + const createComponent = ( + requestHandlers, + mountFn = shallowMountExtended, + frozenOutboundJobTokenScopes = false, + frozenOutboundJobTokenScopesOverride = false, + ) => { wrapper = mountFn(OutboundTokenAccess, { provide: { fullPath: projectPath, + glFeatures: { + frozenOutboundJobTokenScopes, + frozenOutboundJobTokenScopesOverride, + }, }, apolloProvider: createMockApolloProvider(requestHandlers), data() { @@ -272,4 +283,59 @@ describe('TokenAccess component', () => { expect(createAlert).toHaveBeenCalledWith({ message }); }); }); + + describe('with the frozenOutboundJobTokenScopes feature flag enabled', () => { + describe('toggle', () => { + it('the toggle is off and the deprecation alert is visible', async () => { + createComponent( + [ + [getCIJobTokenScopeQuery, disabledJobTokenScopeHandler], + [getProjectsWithCIJobTokenScopeQuery, getProjectsWithScopeHandler], + ], + shallowMountExtended, + true, + ); + + await waitForPromises(); + + expect(findToggle().props('value')).toBe(false); + expect(findToggle().props('disabled')).toBe(true); + expect(findDeprecationAlert().exists()).toBe(true); + expect(findTokenDisabledAlert().exists()).toBe(false); + }); + + it('contains a warning message about disabling the current configuration', async () => { + createComponent( + [ + [getCIJobTokenScopeQuery, disabledJobTokenScopeHandler], + [getProjectsWithCIJobTokenScopeQuery, getProjectsWithScopeHandler], + ], + mountExtended, + true, + ); + + await waitForPromises(); + + expect(findToggle().text()).toContain('Disabling this feature is a permanent change.'); + }); + }); + + describe('adding a new project', () => { + it('disables the input to add new projects', async () => { + createComponent( + [ + [getCIJobTokenScopeQuery, disabledJobTokenScopeHandler], + [getProjectsWithCIJobTokenScopeQuery, getProjectsWithScopeHandler], + ], + mountExtended, + true, + false, + ); + + await waitForPromises(); + + expect(findProjectPathInput().attributes('disabled')).toBe('disabled'); + }); + }); + }); }); diff --git a/spec/helpers/avatars_helper_spec.rb b/spec/helpers/avatars_helper_spec.rb index 6eb97a99264..b7fdadbd036 100644 --- a/spec/helpers/avatars_helper_spec.rb +++ b/spec/helpers/avatars_helper_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe AvatarsHelper do +RSpec.describe AvatarsHelper, feature_category: :source_code_management do include UploadHelpers let_it_be(:user) { create(:user) } @@ -88,7 +88,7 @@ RSpec.describe AvatarsHelper do describe '#avatar_icon_for' do let!(:user) { create(:user, avatar: File.open(uploaded_image_temp_path), email: 'bar@example.com') } let(:email) { 'foo@example.com' } - let!(:another_user) { create(:user, avatar: File.open(uploaded_image_temp_path), email: email) } + let!(:another_user) { create(:user, :public_email, avatar: File.open(uploaded_image_temp_path), email: email) } it 'prefers the user to retrieve the avatar_url' do expect(helper.avatar_icon_for(user, email).to_s) @@ -102,7 +102,7 @@ RSpec.describe AvatarsHelper do end describe '#avatar_icon_for_email', :clean_gitlab_redis_cache do - let(:user) { create(:user, avatar: File.open(uploaded_image_temp_path)) } + let(:user) { create(:user, :public_email, avatar: File.open(uploaded_image_temp_path)) } subject { helper.avatar_icon_for_email(user.email).to_s } @@ -114,6 +114,14 @@ RSpec.describe AvatarsHelper do end end + context 'when a private email is used' do + it 'calls gravatar_icon' do + expect(helper).to receive(:gravatar_icon).with(user.commit_email, 20, 2) + + helper.avatar_icon_for_email(user.commit_email, 20, 2) + end + end + context 'when no user exists for the email' do it 'calls gravatar_icon' do expect(helper).to receive(:gravatar_icon).with('foo@example.com', 20, 2) @@ -136,7 +144,7 @@ RSpec.describe AvatarsHelper do it_behaves_like "returns avatar for email" it "caches the request" do - expect(User).to receive(:find_by_any_email).once.and_call_original + expect(User).to receive(:with_public_email).once.and_call_original expect(helper.avatar_icon_for_email(user.email).to_s).to eq(user.avatar.url) expect(helper.avatar_icon_for_email(user.email).to_s).to eq(user.avatar.url) diff --git a/spec/lib/banzai/filter/asset_proxy_filter_spec.rb b/spec/lib/banzai/filter/asset_proxy_filter_spec.rb index 004c70c28f1..dc6ac52a8c2 100644 --- a/spec/lib/banzai/filter/asset_proxy_filter_spec.rb +++ b/spec/lib/banzai/filter/asset_proxy_filter_spec.rb @@ -80,6 +80,15 @@ RSpec.describe Banzai::Filter::AssetProxyFilter, feature_category: :team_plannin expect(doc.at_css('img')['data-canonical-src']).to eq src end + it 'replaces invalid URLs' do + src = '///example.com/test.png' + new_src = 'https://assets.example.com/3368d2c7b9bed775bdd1e811f36a4b80a0dcd8ab/2f2f2f6578616d706c652e636f6d2f746573742e706e67' + doc = filter(image(src), @context) + + expect(doc.at_css('img')['src']).to eq new_src + expect(doc.at_css('img')['data-canonical-src']).to eq src + end + it 'skips internal images' do src = "#{Gitlab.config.gitlab.url}/test.png" doc = filter(image(src), @context) diff --git a/spec/lib/banzai/filter/commit_trailers_filter_spec.rb b/spec/lib/banzai/filter/commit_trailers_filter_spec.rb index 3ebe0798972..3d992f962ec 100644 --- a/spec/lib/banzai/filter/commit_trailers_filter_spec.rb +++ b/spec/lib/banzai/filter/commit_trailers_filter_spec.rb @@ -8,16 +8,15 @@ RSpec.describe Banzai::Filter::CommitTrailersFilter, feature_category: :source_c include CommitTrailersSpecHelper let(:secondary_email) { create(:email, :confirmed) } - let(:user) { create(:user) } + let(:user) { create(:user, :public_email) } + let(:email) { FFaker::Internet.email } let(:trailer) { "#{FFaker::Lorem.word}-by:" } - let(:commit_message) { trailer_line(trailer, user.name, user.email) } + let(:commit_message) { trailer_line(trailer, user.name, user.public_email) } let(:commit_message_html) { commit_html(commit_message) } context 'detects' do - let(:email) { FFaker::Internet.email } - context 'trailers in the form of *-by' do where(:commit_trailer) do ["#{FFaker::Lorem.word}-by:", "#{FFaker::Lorem.word}-BY:", "#{FFaker::Lorem.word}-By:"] @@ -42,7 +41,7 @@ RSpec.describe Banzai::Filter::CommitTrailersFilter, feature_category: :source_c expect_to_have_user_link_with_avatar(doc, user: user, trailer: trailer) end - it 'GitLab users via a secondary email' do + it 'does not detect GitLab users via a secondary email' do _, message_html = build_commit_message( trailer: trailer, name: secondary_email.user.name, @@ -51,9 +50,8 @@ RSpec.describe Banzai::Filter::CommitTrailersFilter, feature_category: :source_c doc = filter(message_html) - expect_to_have_user_link_with_avatar( + expect_to_have_mailto_link_with_avatar( doc, - user: secondary_email.user, trailer: trailer, email: secondary_email.email ) @@ -185,17 +183,16 @@ RSpec.describe Banzai::Filter::CommitTrailersFilter, feature_category: :source_c it 'preserves the original email used in the commit message' do message, message_html = build_commit_message( trailer: trailer, - name: secondary_email.user.name, - email: secondary_email.email + name: user.name, + email: email ) doc = filter(message_html) - expect_to_have_user_link_with_avatar( + expect_to_have_mailto_link_with_avatar( doc, - user: secondary_email.user, trailer: trailer, - email: secondary_email.email + email: email ) expect(doc.text).to match Regexp.escape(message) end @@ -218,7 +215,7 @@ RSpec.describe Banzai::Filter::CommitTrailersFilter, feature_category: :source_c # any path-only link will automatically be prefixed # with the path of its repository. # See: "build_relative_path" in "lib/banzai/filter/relative_link_filter.rb" - let(:user_with_avatar) { create(:user, :with_avatar, username: 'foobar') } + let(:user_with_avatar) { create(:user, :public_email, :with_avatar, username: 'foobar') } it 'returns a full path for avatar urls' do _, message_html = build_commit_message( diff --git a/spec/lib/gitlab/checks/branch_check_spec.rb b/spec/lib/gitlab/checks/branch_check_spec.rb index d6280d3c28c..7f535e86d69 100644 --- a/spec/lib/gitlab/checks/branch_check_spec.rb +++ b/spec/lib/gitlab/checks/branch_check_spec.rb @@ -26,8 +26,14 @@ RSpec.describe Gitlab::Checks::BranchCheck do expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, "You cannot create a branch with a 40-character hexadecimal branch name.") end + it "prohibits 40-character hexadecimal branch names as the start of a path" do + allow(subject).to receive(:branch_name).and_return("267208abfe40e546f5e847444276f7d43a39503e/test") + + expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, "You cannot create a branch with a 40-character hexadecimal branch name.") + end + it "doesn't prohibit a nested hexadecimal in a branch name" do - allow(subject).to receive(:branch_name).and_return("fix-267208abfe40e546f5e847444276f7d43a39503e") + allow(subject).to receive(:branch_name).and_return("267208abfe40e546f5e847444276f7d43a39503e-fix") expect { subject.validate! }.not_to raise_error end diff --git a/spec/lib/gitlab/database/dynamic_model_helpers_spec.rb b/spec/lib/gitlab/database/dynamic_model_helpers_spec.rb index 31486240bfa..fe423b3639b 100644 --- a/spec/lib/gitlab/database/dynamic_model_helpers_spec.rb +++ b/spec/lib/gitlab/database/dynamic_model_helpers_spec.rb @@ -49,6 +49,21 @@ RSpec.describe Gitlab::Database::DynamicModelHelpers do expect { |b| each_batch_size.call(&b) } .to yield_successive_args(1, 1) end + + context 'when a column to be batched over is specified' do + let(:projects) { Project.order(project_namespace_id: :asc) } + + it 'iterates table in batches using the given column' do + each_batch_ids = ->(&block) do + subject.each_batch(table_name, connection: connection, of: 1, column: :project_namespace_id) do |batch| + block.call(batch.pluck(:project_namespace_id)) + end + end + + expect { |b| each_batch_ids.call(&b) } + .to yield_successive_args([projects.first.project_namespace_id], [projects.last.project_namespace_id]) + end + end end context 'when transaction is open' do @@ -95,6 +110,35 @@ RSpec.describe Gitlab::Database::DynamicModelHelpers do expect { |b| each_batch_limited.call(&b) } .to yield_successive_args([first_project.id, first_project.id]) end + + context 'when primary key is not named id' do + let(:namespace_settings1) { create(:namespace_settings) } + let(:namespace_settings2) { create(:namespace_settings) } + let(:table_name) { NamespaceSetting.table_name } + let(:connection) { NamespaceSetting.connection } + let(:primary_key) { subject.define_batchable_model(table_name, connection: connection).primary_key } + + it 'iterates table in batch ranges using the correct primary key' do + expect(primary_key).to eq("namespace_id") # Sanity check the primary key is not id + expect { |b| subject.each_batch_range(table_name, connection: connection, of: 1, &b) } + .to yield_successive_args( + [namespace_settings1.namespace_id, namespace_settings1.namespace_id], + [namespace_settings2.namespace_id, namespace_settings2.namespace_id] + ) + end + end + + context 'when a column to be batched over is specified' do + it 'iterates table in batch ranges using the given column' do + expect do |b| + subject.each_batch_range(table_name, connection: connection, of: 1, column: :project_namespace_id, &b) + end + .to yield_successive_args( + [first_project.project_namespace_id, first_project.project_namespace_id], + [second_project.project_namespace_id, second_project.project_namespace_id] + ) + end + end end context 'when transaction is open' do diff --git a/spec/lib/gitlab_settings/settings_spec.rb b/spec/lib/gitlab_settings/settings_spec.rb index 55ceff4ce82..161c26dbb9f 100644 --- a/spec/lib/gitlab_settings/settings_spec.rb +++ b/spec/lib/gitlab_settings/settings_spec.rb @@ -21,18 +21,16 @@ RSpec.describe GitlabSettings::Settings, :aggregate_failures, feature_category: subject(:settings) { described_class.new(source.path, 'section1') } - it 'requires a source' do - expect { described_class.new('', '') } - .to raise_error(ArgumentError, 'config source is required') - end - - it 'requires a section' do - expect { described_class.new(source, '') } - .to raise_error(ArgumentError, 'config section is required') - end + describe '#initialize' do + it 'requires a source' do + expect { described_class.new('', '') } + .to raise_error(ArgumentError, 'config source is required') + end - it 'loads the given section config' do - expect(settings.config1.value1).to eq(1) + it 'requires a section' do + expect { described_class.new(source, '') } + .to raise_error(ArgumentError, 'config section is required') + end end describe '#reload!' do @@ -50,4 +48,20 @@ RSpec.describe GitlabSettings::Settings, :aggregate_failures, feature_category: expect(settings.config1.value1).to eq(2) end end + + it 'loads the given section config' do + expect(settings.config1.value1).to eq(1) + end + + context 'on lazy loading' do + it 'does not raise exception on initialization if source does not exists' do + settings = nil + + expect { settings = described_class.new('/tmp/any/inexisting/file.yml', 'section1') } + .not_to raise_error + + expect { settings['any key'] } + .to raise_error(Errno::ENOENT) + end + end end diff --git a/spec/lib/slack/block_kit/app_home_opened_spec.rb b/spec/lib/slack/block_kit/app_home_opened_spec.rb new file mode 100644 index 00000000000..5a5a9c6739c --- /dev/null +++ b/spec/lib/slack/block_kit/app_home_opened_spec.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Slack::BlockKit::AppHomeOpened, feature_category: :integrations do + let_it_be(:slack_installation) { create(:slack_integration) } + + let(:chat_name) { nil } + + describe '#build' do + subject(:payload) do + described_class.new(slack_installation.user_id, slack_installation.team_id, chat_name, slack_installation).build + end + + it 'generates blocks of type "home"' do + is_expected.to match({ type: 'home', blocks: kind_of(Array) }) + end + + it 'prompts the user to connect their GitLab account' do + expect(payload[:blocks]).to include( + hash_including( + { + type: 'actions', + elements: [ + hash_including( + { + type: 'button', + text: include({ text: 'Connect your GitLab account' }), + url: include(Gitlab::Routing.url_helpers.new_profile_chat_name_url) + } + ) + ] + } + ) + ) + end + + context 'when the user has linked their GitLab account' do + let_it_be(:user) { create(:user) } + let_it_be(:chat_name) do + create(:chat_name, + user: user, + team_id: slack_installation.team_id, + chat_id: slack_installation.user_id + ) + end + + it 'displays the GitLab user they are linked to' do + account = "<#{Gitlab::UrlBuilder.build(user)}|#{user.to_reference}>" + + expect(payload[:blocks]).to include( + hash_including( + { + type: 'section', + text: include({ text: "✅ Connected to GitLab account #{account}." }) + } + ) + ) + end + end + end +end diff --git a/spec/migrations/swap_system_note_metadata_note_id_to_bigint_for_gitlab_dot_com_spec.rb b/spec/migrations/swap_system_note_metadata_note_id_to_bigint_for_gitlab_dot_com_spec.rb new file mode 100644 index 00000000000..4fa5814986a --- /dev/null +++ b/spec/migrations/swap_system_note_metadata_note_id_to_bigint_for_gitlab_dot_com_spec.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_migration! + +RSpec.describe SwapSystemNoteMetadataNoteIdToBigintForGitlabDotCom, feature_category: :database do + describe '#up' do + before do + # A we call `schema_migrate_down!` before each example, and for this migration + # `#down` is same as `#up`, we need to ensure we start from the expected state. + connection = described_class.new.connection + connection.execute('ALTER TABLE system_note_metadata ALTER COLUMN note_id TYPE integer') + connection.execute('ALTER TABLE system_note_metadata ALTER COLUMN note_id_convert_to_bigint TYPE bigint') + end + + # rubocop: disable RSpec/AnyInstanceOf + it 'swaps the integer and bigint columns for GitLab.com, dev, or test' do + allow_any_instance_of(described_class).to receive(:com_or_dev_or_test_but_not_jh?).and_return(true) + + metadata = table(:system_note_metadata) + + disable_migrations_output do + reversible_migration do |migration| + migration.before -> { + metadata.reset_column_information + + expect(metadata.columns.find { |c| c.name == 'note_id' }.sql_type).to eq('integer') + expect(metadata.columns.find { |c| c.name == 'note_id_convert_to_bigint' }.sql_type).to eq('bigint') + } + + migration.after -> { + metadata.reset_column_information + + expect(metadata.columns.find { |c| c.name == 'note_id' }.sql_type).to eq('bigint') + expect(metadata.columns.find { |c| c.name == 'note_id_convert_to_bigint' }.sql_type).to eq('integer') + } + end + end + end + + it 'is a no-op for other instances' do + allow_any_instance_of(described_class).to receive(:com_or_dev_or_test_but_not_jh?).and_return(false) + + metadata = table(:system_note_metadata) + + disable_migrations_output do + reversible_migration do |migration| + migration.before -> { + metadata.reset_column_information + + expect(metadata.columns.find { |c| c.name == 'note_id' }.sql_type).to eq('integer') + expect(metadata.columns.find { |c| c.name == 'note_id_convert_to_bigint' }.sql_type).to eq('bigint') + } + + migration.after -> { + metadata.reset_column_information + + expect(metadata.columns.find { |c| c.name == 'note_id' }.sql_type).to eq('integer') + expect(metadata.columns.find { |c| c.name == 'note_id_convert_to_bigint' }.sql_type).to eq('bigint') + } + end + end + end + # rubocop: enable RSpec/AnyInstanceOf + end +end diff --git a/spec/migrations/swap_todos_note_id_to_bigint_for_gitlab_dot_com_spec.rb b/spec/migrations/swap_todos_note_id_to_bigint_for_gitlab_dot_com_spec.rb new file mode 100644 index 00000000000..e71c921998a --- /dev/null +++ b/spec/migrations/swap_todos_note_id_to_bigint_for_gitlab_dot_com_spec.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_migration! + +RSpec.describe SwapTodosNoteIdToBigintForGitlabDotCom, feature_category: :database do + describe '#up' do + before do + # A we call `schema_migrate_down!` before each example, and for this migration + # `#down` is same as `#up`, we need to ensure we start from the expected state. + connection = described_class.new.connection + connection.execute('ALTER TABLE todos ALTER COLUMN note_id TYPE integer') + connection.execute('ALTER TABLE todos ALTER COLUMN note_id_convert_to_bigint TYPE bigint') + end + + # rubocop: disable RSpec/AnyInstanceOf + it 'swaps the integer and bigint columns for GitLab.com, dev, or test' do + allow_any_instance_of(described_class).to receive(:com_or_dev_or_test_but_not_jh?).and_return(true) + + todos = table(:todos) + + disable_migrations_output do + reversible_migration do |migration| + migration.before -> { + todos.reset_column_information + + expect(todos.columns.find { |c| c.name == 'note_id' }.sql_type).to eq('integer') + expect(todos.columns.find { |c| c.name == 'note_id_convert_to_bigint' }.sql_type).to eq('bigint') + } + + migration.after -> { + todos.reset_column_information + + expect(todos.columns.find { |c| c.name == 'note_id' }.sql_type).to eq('bigint') + expect(todos.columns.find { |c| c.name == 'note_id_convert_to_bigint' }.sql_type).to eq('integer') + } + end + end + end + + it 'is a no-op for other instances' do + allow_any_instance_of(described_class).to receive(:com_or_dev_or_test_but_not_jh?).and_return(false) + + todos = table(:todos) + + disable_migrations_output do + reversible_migration do |migration| + migration.before -> { + todos.reset_column_information + + expect(todos.columns.find { |c| c.name == 'note_id' }.sql_type).to eq('integer') + expect(todos.columns.find { |c| c.name == 'note_id_convert_to_bigint' }.sql_type).to eq('bigint') + } + + migration.after -> { + todos.reset_column_information + + expect(todos.columns.find { |c| c.name == 'note_id' }.sql_type).to eq('integer') + expect(todos.columns.find { |c| c.name == 'note_id_convert_to_bigint' }.sql_type).to eq('bigint') + } + end + end + end + # rubocop: enable RSpec/AnyInstanceOf + end +end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index c994aca2991..7b1907f2a6a 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -4259,10 +4259,14 @@ RSpec.describe MergeRequest, factory_default: :keep, feature_category: :code_rev subject { create(:merge_request, source_project: project) } - it 'fetches the ref correctly' do + it 'fetches the ref and expires the ancestor cache' do expect { subject.target_project.repository.delete_refs(subject.ref_path) }.not_to raise_error + expect(project.repository).to receive(:expire_ancestor_cache).with(subject.target_branch_sha, subject.diff_head_sha).and_call_original + expect(subject).to receive(:expire_ancestor_cache).and_call_original + subject.fetch_ref! + expect(subject.target_project.repository.ref_exists?(subject.ref_path)).to be_truthy end end @@ -4273,7 +4277,8 @@ RSpec.describe MergeRequest, factory_default: :keep, feature_category: :code_rev # We use build instead of create to test that an IID is allocated subject { build(:merge_request, source_project: project) } - it 'fetches the ref correctly' do + it 'fetches the ref and expires the ancestor cache' do + expect(subject).to receive(:expire_ancestor_cache).and_call_original expect(subject.iid).to be_nil expect { subject.eager_fetch_ref! }.to change { subject.iid.to_i }.by(1) diff --git a/spec/models/namespace/package_setting_spec.rb b/spec/models/namespace/package_setting_spec.rb index 2584fa597ad..fca929600a4 100644 --- a/spec/models/namespace/package_setting_spec.rb +++ b/spec/models/namespace/package_setting_spec.rb @@ -47,28 +47,30 @@ RSpec.describe Namespace::PackageSetting do context 'package types with package_settings' do # As more package types gain settings they will be added to this list [:maven_package, :generic_package].each do |format| - let_it_be(:package) { create(format, name: 'foo', version: 'beta') } # rubocop:disable Rails/SaveBang - let_it_be(:package_type) { package.package_type } - let_it_be(:package_setting) { package.project.namespace.package_settings } - - where(:duplicates_allowed, :duplicate_exception_regex, :result) do - true | '' | true - false | '' | false - false | '.*' | true - false | 'fo.*' | true - false | 'be.*' | true - end + context "with package_type:#{format}" do + let_it_be(:package) { create(format, name: 'foo', version: 'beta') } # rubocop:disable Rails/SaveBang + let_it_be(:package_type) { package.package_type } + let_it_be(:package_setting) { package.project.namespace.package_settings } + + where(:duplicates_allowed, :duplicate_exception_regex, :result) do + true | '' | true + false | '' | false + false | '.*' | true + false | 'fo.*' | true + false | 'be.*' | true + end - with_them do - context "for #{format}" do - before do - package_setting.update!( - "#{package_type}_duplicates_allowed" => duplicates_allowed, - "#{package_type}_duplicate_exception_regex" => duplicate_exception_regex - ) - end + with_them do + context "for #{format}" do + before do + package_setting.update!( + "#{package_type}_duplicates_allowed" => duplicates_allowed, + "#{package_type}_duplicate_exception_regex" => duplicate_exception_regex + ) + end - it { is_expected.to be(result) } + it { is_expected.to be(result) } + end end end end @@ -76,11 +78,13 @@ RSpec.describe Namespace::PackageSetting do context 'package types without package_settings' do [:npm_package, :conan_package, :nuget_package, :pypi_package, :composer_package, :golang_package, :debian_package].each do |format| - let_it_be(:package) { create(format) } # rubocop:disable Rails/SaveBang - let_it_be(:package_setting) { package.project.namespace.package_settings } + context "with package_type:#{format}" do + let_it_be(:package) { create(format) } # rubocop:disable Rails/SaveBang + let_it_be(:package_setting) { package.project.namespace.package_settings } - it 'raises an error' do - expect { subject }.to raise_error(Namespace::PackageSetting::PackageSettingNotImplemented) + it 'raises an error' do + expect { subject }.to raise_error(Namespace::PackageSetting::PackageSettingNotImplemented) + end end end end diff --git a/spec/models/preloaders/user_max_access_level_in_projects_preloader_spec.rb b/spec/models/preloaders/user_max_access_level_in_projects_preloader_spec.rb index de10653d87e..a2ab59f56ab 100644 --- a/spec/models/preloaders/user_max_access_level_in_projects_preloader_spec.rb +++ b/spec/models/preloaders/user_max_access_level_in_projects_preloader_spec.rb @@ -23,8 +23,7 @@ RSpec.describe Preloaders::UserMaxAccessLevelInProjectsPreloader do # we have an existing N+1, one for each project for which user is not a member # in this spec, project_3, project_4, project_5 # https://gitlab.com/gitlab-org/gitlab/-/issues/362890 - ee_only_policy_check_queries = Gitlab.ee? ? 1 : 0 - expect { query }.to make_queries(projects.size + 3 + ee_only_policy_check_queries) + expect { query }.to make_queries(projects.size + 3) end end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 72011693e20..ea237768333 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -3132,6 +3132,18 @@ RSpec.describe Repository, feature_category: :source_code_management do 2.times { repository.ancestor?(commit.id, ancestor.id) } end + it 'calls out to Gitaly again after expiration' do + expect(repository.raw_repository).to receive(:ancestor?).once + + repository.ancestor?(commit.id, ancestor.id) + + repository.expire_ancestor_cache(commit.id, ancestor.id) + + expect(repository.raw_repository).to receive(:ancestor?).once + + 2.times { repository.ancestor?(commit.id, ancestor.id) } + end + it 'returns the value from the request store' do repository.__send__(:request_store_cache).write(cache_key, "it's apparent") diff --git a/spec/requests/api/integrations/slack/events_spec.rb b/spec/requests/api/integrations/slack/events_spec.rb new file mode 100644 index 00000000000..438715db4f0 --- /dev/null +++ b/spec/requests/api/integrations/slack/events_spec.rb @@ -0,0 +1,91 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe API::Integrations::Slack::Events, feature_category: :integrations do + describe 'POST /integrations/slack/events' do + let_it_be(:slack_installation) { create(:slack_integration) } + + let(:params) { {} } + let(:headers) do + { + ::API::Integrations::Slack::Request::VERIFICATION_TIMESTAMP_HEADER => Time.current.to_i.to_s, + ::API::Integrations::Slack::Request::VERIFICATION_SIGNATURE_HEADER => 'mock_verified_signature' + } + end + + before do + allow(ActiveSupport::SecurityUtils).to receive(:secure_compare) do |signature| + signature == 'mock_verified_signature' + end + + stub_application_setting(slack_app_signing_secret: 'mock_key') + end + + subject { post api('/integrations/slack/events'), params: params, headers: headers } + + it_behaves_like 'Slack request verification' + + context 'when type param is unknown' do + let(:params) do + { type: 'unknown_type' } + end + + it 'generates a tracked error' do + expect(Gitlab::ErrorTracking).to receive(:track_exception).once + + subject + + expect(response).to have_gitlab_http_status(:no_content) + expect(response.body).to be_empty + end + end + + context 'when type param is url_verification' do + let(:params) do + { + type: 'url_verification', + challenge: '3eZbrw1aBm2rZgRNFdxV2595E9CY3gmdALWMmHkvFXO7tYXAYM8P' + } + end + + it 'responds in-request with the challenge' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to eq({ 'challenge' => '3eZbrw1aBm2rZgRNFdxV2595E9CY3gmdALWMmHkvFXO7tYXAYM8P' }) + end + end + + context 'when event.type param is app_home_opened' do + let(:params) do + { + type: 'event_callback', + team_id: slack_installation.team_id, + event_id: 'Ev03SA75UJKB', + event: { + type: 'app_home_opened', + user: 'U0123ABCDEF' + } + } + end + + it 'calls the Slack API (integration-style test)', :sidekiq_inline, :clean_gitlab_redis_shared_state do + api_url = "#{Slack::API::BASE_URL}/views.publish" + + stub_request(:post, api_url) + .to_return( + status: 200, + body: { ok: true }.to_json, + headers: { 'Content-Type' => 'application/json' } + ) + + subject + + expect(WebMock).to have_requested(:post, api_url) + expect(response).to have_gitlab_http_status(:ok) + expect(response.body).to eq('{}') + end + end + end +end diff --git a/spec/requests/api/integrations/slack/interactions_spec.rb b/spec/requests/api/integrations/slack/interactions_spec.rb new file mode 100644 index 00000000000..35a96be75e0 --- /dev/null +++ b/spec/requests/api/integrations/slack/interactions_spec.rb @@ -0,0 +1,69 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe API::Integrations::Slack::Interactions, feature_category: :integrations do + describe 'POST /integrations/slack/interactions' do + let_it_be(:slack_installation) { create(:slack_integration) } + + let(:payload) { {} } + let(:params) { { payload: Gitlab::Json.dump(payload) } } + + let(:headers) do + { + ::API::Integrations::Slack::Request::VERIFICATION_TIMESTAMP_HEADER => Time.current.to_i.to_s, + ::API::Integrations::Slack::Request::VERIFICATION_SIGNATURE_HEADER => 'mock_verified_signature' + } + end + + before do + allow(ActiveSupport::SecurityUtils).to receive(:secure_compare) do |signature| + signature == 'mock_verified_signature' + end + + stub_application_setting(slack_app_signing_secret: 'mock_key') + end + + subject { post api('/integrations/slack/interactions'), params: params, headers: headers } + + it_behaves_like 'Slack request verification' + + context 'when type param is unknown' do + let(:payload) do + { type: 'unknown_type' } + end + + it 'generates a tracked error' do + expect(Gitlab::ErrorTracking).to receive(:track_exception).once + + subject + + expect(response).to have_gitlab_http_status(:no_content) + expect(response.body).to be_empty + end + end + + context 'when event.type param is view_closed' do + let(:payload) do + { + type: 'view_closed', + team_id: slack_installation.team_id, + event: { + type: 'view_closed', + user: 'U0123ABCDEF' + } + } + end + + it 'calls the Slack Interactivity Service' do + expect_next_instance_of(::Integrations::SlackInteractionService) do |service| + expect(service).to receive(:execute).and_return(ServiceResponse.success) + end + + subject + + expect(response).to have_gitlab_http_status(:ok) + end + end + end +end diff --git a/spec/serializers/admin/abuse_report_entity_spec.rb b/spec/serializers/admin/abuse_report_entity_spec.rb index 2101fc15dd0..003d76a172f 100644 --- a/spec/serializers/admin/abuse_report_entity_spec.rb +++ b/spec/serializers/admin/abuse_report_entity_spec.rb @@ -11,12 +11,6 @@ RSpec.describe Admin::AbuseReportEntity, feature_category: :insider_threat do described_class.new(abuse_report) end - before do - allow_next_instance_of(described_class) do |instance| - allow(instance).to receive(:markdown_field).with(abuse_report, :message).and_return(abuse_report.message) - end - end - describe '#as_json' do subject(:entity_hash) { entity.as_json } @@ -27,71 +21,20 @@ RSpec.describe Admin::AbuseReportEntity, feature_category: :insider_threat do :updated_at, :reported_user, :reporter, - :reported_user_path, - :reporter_path, - :user_blocked, - :block_user_path, - :remove_report_path, - :remove_user_and_report_path, - :message, :report_path ) end it 'correctly exposes `reported user`' do - expect(entity_hash[:reported_user].keys).to match_array([:name, :created_at]) + expect(entity_hash[:reported_user].keys).to match_array([:name]) end it 'correctly exposes `reporter`' do expect(entity_hash[:reporter].keys).to match_array([:name]) end - it 'correctly exposes :reported_user_path' do - expect(entity_hash[:reported_user_path]).to eq user_path(abuse_report.user) - end - - it 'correctly exposes :reporter_path' do - expect(entity_hash[:reporter_path]).to eq user_path(abuse_report.reporter) - end - - describe 'user_blocked' do - subject(:user_blocked) { entity_hash[:user_blocked] } - - context 'when user is blocked' do - before do - allow(abuse_report.user).to receive(:blocked?).and_return(true) - end - - it { is_expected.to be true } - end - - context 'when user is not blocked' do - before do - allow(abuse_report.user).to receive(:blocked?).and_return(false) - end - - it { is_expected.to be false } - end - end - - it 'correctly exposes :block_user_path' do - expect(entity_hash[:block_user_path]).to eq block_admin_user_path(abuse_report.user) - end - - it 'correctly exposes :remove_report_path' do - expect(entity_hash[:remove_report_path]).to eq admin_abuse_report_path(abuse_report) - end - it 'correctly exposes :report_path' do expect(entity_hash[:report_path]).to eq admin_abuse_report_path(abuse_report) end - - it 'correctly exposes :remove_user_and_report_path' do - expect(entity_hash[:remove_user_and_report_path]).to eq admin_abuse_report_path(abuse_report, remove_user: true) - end - - it 'correctly exposes :message' do - expect(entity_hash[:message]).to eq(abuse_report.message) - end end end diff --git a/spec/services/draft_notes/publish_service_spec.rb b/spec/services/draft_notes/publish_service_spec.rb index a4b1d8742d0..dab06637c1a 100644 --- a/spec/services/draft_notes/publish_service_spec.rb +++ b/spec/services/draft_notes/publish_service_spec.rb @@ -172,7 +172,12 @@ RSpec.describe DraftNotes::PublishService, feature_category: :code_review_workfl end end - it 'does not requests a lot from Gitaly', :request_store do + it 'does not request a lot from Gitaly', :request_store, :clean_gitlab_redis_cache do + merge_request + position + + Gitlab::GitalyClient.reset_counts + # NOTE: This should be reduced as we work on reducing Gitaly calls. # Gitaly requests shouldn't go above this threshold as much as possible # as it may add more to the Gitaly N+1 issue we are experiencing. diff --git a/spec/services/integrations/slack_event_service_spec.rb b/spec/services/integrations/slack_event_service_spec.rb new file mode 100644 index 00000000000..17433aee329 --- /dev/null +++ b/spec/services/integrations/slack_event_service_spec.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Integrations::SlackEventService, feature_category: :integrations do + describe '#execute' do + subject(:execute) { described_class.new(params).execute } + + let(:params) do + { + type: 'event_callback', + event: { + type: 'app_home_opened', + foo: 'bar' + } + } + end + + it 'queues a worker and returns success response' do + expect(Integrations::SlackEventWorker).to receive(:perform_async) + .with( + { + slack_event: 'app_home_opened', + params: { + event: { + foo: 'bar' + } + } + } + ) + expect(execute.payload).to eq({}) + is_expected.to be_success + end + + context 'when event a url verification request' do + let(:params) { { type: 'url_verification', foo: 'bar' } } + + it 'executes the service instead of queueing a worker and returns success response' do + expect(Integrations::SlackEventWorker).not_to receive(:perform_async) + expect_next_instance_of(Integrations::SlackEvents::UrlVerificationService, { foo: 'bar' }) do |service| + expect(service).to receive(:execute).and_return({ baz: 'qux' }) + end + expect(execute.payload).to eq({ baz: 'qux' }) + is_expected.to be_success + end + end + + context 'when event is unknown' do + let(:params) { super().merge(event: { type: 'foo' }) } + + it 'raises an error' do + expect { execute }.to raise_error(described_class::UnknownEventError) + end + end + end +end diff --git a/spec/services/integrations/slack_events/app_home_opened_service_spec.rb b/spec/services/integrations/slack_events/app_home_opened_service_spec.rb new file mode 100644 index 00000000000..0eb4c019e0a --- /dev/null +++ b/spec/services/integrations/slack_events/app_home_opened_service_spec.rb @@ -0,0 +1,113 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Integrations::SlackEvents::AppHomeOpenedService, feature_category: :integrations do + describe '#execute' do + let_it_be(:slack_installation) { create(:slack_integration) } + + let(:slack_workspace_id) { slack_installation.team_id } + let(:slack_user_id) { 'U0123ABCDEF' } + let(:api_url) { "#{Slack::API::BASE_URL}/views.publish" } + let(:api_response) { { ok: true } } + let(:params) do + { + team_id: slack_workspace_id, + event: { user: slack_user_id }, + event_id: 'Ev03SA75UJKB' + } + end + + subject(:execute) { described_class.new(params).execute } + + before do + stub_request(:post, api_url) + .to_return( + status: 200, + body: api_response.to_json, + headers: { 'Content-Type' => 'application/json' } + ) + end + + shared_examples 'there is no bot token' do + it 'does not call the Slack API, logs info, and returns a success response' do + expect(Gitlab::IntegrationsLogger).to receive(:info).with( + { + slack_user_id: slack_user_id, + slack_workspace_id: slack_workspace_id, + message: 'SlackInstallation record has no bot token' + } + ) + + is_expected.to be_success + end + end + + it 'calls the Slack API correctly and returns a success response' do + mock_view = { type: 'home', blocks: [] } + + expect_next_instance_of(Slack::BlockKit::AppHomeOpened) do |ui| + expect(ui).to receive(:build).and_return(mock_view) + end + + is_expected.to be_success + + expect(WebMock).to have_requested(:post, api_url).with( + body: { + user_id: slack_user_id, + view: mock_view + }, + headers: { + 'Authorization' => "Bearer #{slack_installation.bot_access_token}", + 'Content-Type' => 'application/json; charset=utf-8' + }) + end + + context 'when the slack installation is a legacy record' do + let_it_be(:slack_installation) { create(:slack_integration, :legacy) } + + it_behaves_like 'there is no bot token' + end + + context 'when the slack installation cannot be found' do + let(:slack_workspace_id) { non_existing_record_id } + + it_behaves_like 'there is no bot token' + end + + context 'when the Slack API call raises an HTTP exception' do + before do + allow(Gitlab::HTTP).to receive(:post).and_raise(Errno::ECONNREFUSED, 'error message') + end + + it 'tracks the exception and returns an error response' do + expect(::Gitlab::ErrorTracking).to receive(:track_exception) + .with( + Errno::ECONNREFUSED.new('HTTP exception when calling Slack API'), + { + slack_user_id: slack_user_id, + slack_workspace_id: slack_workspace_id + } + ) + is_expected.to be_error + end + end + + context 'when the Slack API returns an error' do + let(:api_response) { { ok: false, foo: 'bar' } } + + it 'tracks the exception and returns an error response' do + expect(::Gitlab::ErrorTracking).to receive(:track_exception) + .with( + StandardError.new('Slack API returned an error'), + { + slack_user_id: slack_user_id, + slack_workspace_id: slack_workspace_id, + response: api_response.with_indifferent_access + } + ) + is_expected.to be_error + end + end + end +end diff --git a/spec/services/integrations/slack_events/url_verification_service_spec.rb b/spec/services/integrations/slack_events/url_verification_service_spec.rb new file mode 100644 index 00000000000..0d668acafb9 --- /dev/null +++ b/spec/services/integrations/slack_events/url_verification_service_spec.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Integrations::SlackEvents::UrlVerificationService, feature_category: :integrations do + describe '#execute' do + it 'returns the challenge' do + expect(described_class.new({ challenge: 'foo' }).execute).to eq({ challenge: 'foo' }) + end + end +end diff --git a/spec/services/integrations/slack_interaction_service_spec.rb b/spec/services/integrations/slack_interaction_service_spec.rb new file mode 100644 index 00000000000..599320c7986 --- /dev/null +++ b/spec/services/integrations/slack_interaction_service_spec.rb @@ -0,0 +1,70 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Integrations::SlackInteractionService, feature_category: :integrations do + describe '#execute' do + subject(:execute) { described_class.new(params).execute } + + let(:params) do + { + type: slack_interaction, + foo: 'bar' + } + end + + context 'when view is closed' do + let(:slack_interaction) { 'view_closed' } + + it 'executes the correct service' do + view_closed_service = described_class::INTERACTIONS['view_closed'] + + expect_next_instance_of(view_closed_service, { foo: 'bar' }) do |service| + expect(service).to receive(:execute).and_return(ServiceResponse.success) + end + + execute + end + end + + context 'when view is submitted' do + let(:slack_interaction) { 'view_submission' } + + it 'executes the submission service' do + view_submission_service = described_class::INTERACTIONS['view_submission'] + + expect_next_instance_of(view_submission_service, { foo: 'bar' }) do |service| + expect(service).to receive(:execute).and_return(ServiceResponse.success) + end + + execute + end + end + + context 'when block action service is submitted' do + let(:slack_interaction) { 'block_actions' } + + it 'executes the block actions service' do + block_action_service = described_class::INTERACTIONS['block_actions'] + + expect_next_instance_of(block_action_service, { foo: 'bar' }) do |service| + expect(service).to receive(:execute).and_return(ServiceResponse.success) + end + + execute + end + end + + context 'when slack_interaction is not known' do + let(:slack_interaction) { 'foo' } + + it 'raises an error and does not execute a service class' do + described_class::INTERACTIONS.each_value do |service_class| + expect(service_class).not_to receive(:new) + end + + expect { execute }.to raise_error(described_class::UnknownInteractionError) + end + end + end +end diff --git a/spec/services/integrations/slack_interactions/block_action_service_spec.rb b/spec/services/integrations/slack_interactions/block_action_service_spec.rb new file mode 100644 index 00000000000..9a188ddcfe4 --- /dev/null +++ b/spec/services/integrations/slack_interactions/block_action_service_spec.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Integrations::SlackInteractions::BlockActionService, feature_category: :integrations do + describe '#execute' do + let_it_be(:slack_installation) { create(:slack_integration) } + + let(:params) do + { + view: { + team_id: slack_installation.team_id + }, + actions: [{ + action_id: action_id + }] + } + end + + subject(:execute) { described_class.new(params).execute } + + context 'when action_id is incident_management_project' do + let(:action_id) { 'incident_management_project' } + + it 'executes the correct handler' do + project_handler = described_class::ALLOWED_UPDATES_HANDLERS['incident_management_project'] + + expect_next_instance_of(project_handler, params, params[:actions].first) do |handler| + expect(handler).to receive(:execute).and_return(ServiceResponse.success) + end + + execute + end + end + + context 'when action_id is not known' do + let(:action_id) { 'random' } + + it 'does not execute the handlers' do + described_class::ALLOWED_UPDATES_HANDLERS.each_value do |handler_class| + expect(handler_class).not_to receive(:new) + end + + execute + end + end + end +end diff --git a/spec/services/integrations/slack_interactions/incident_management/incident_modal_closed_service_spec.rb b/spec/services/integrations/slack_interactions/incident_management/incident_modal_closed_service_spec.rb new file mode 100644 index 00000000000..64cddf9a66b --- /dev/null +++ b/spec/services/integrations/slack_interactions/incident_management/incident_modal_closed_service_spec.rb @@ -0,0 +1,78 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Integrations::SlackInteractions::IncidentManagement::IncidentModalClosedService, + feature_category: :integrations do + describe '#execute' do + let_it_be(:request_body) do + { + replace_original: 'true', + text: 'Incident creation cancelled.' + } + end + + let(:params) do + { + view: { + private_metadata: 'https://api.slack.com/id/1234' + } + } + end + + let(:service) { described_class.new(params) } + + before do + allow(Gitlab::HTTP).to receive(:post).and_return({ ok: true }) + end + + context 'when executed' do + it 'makes the POST call and closes the modal' do + expect(Gitlab::HTTP).to receive(:post).with( + 'https://api.slack.com/id/1234', + body: Gitlab::Json.dump(request_body), + headers: { 'Content-Type' => 'application/json' } + ) + + service.execute + end + end + + context 'when the POST call raises an HTTP exception' do + before do + allow(Gitlab::HTTP).to receive(:post).and_raise(Errno::ECONNREFUSED, 'error message') + end + + it 'tracks the exception and returns an error response' do + expect(::Gitlab::ErrorTracking).to receive(:track_exception) + .with( + Errno::ECONNREFUSED.new('HTTP exception when calling Slack API'), + { + params: params + } + ) + + service.execute + end + end + + context 'when response is not ok' do + before do + allow(Gitlab::HTTP).to receive(:post).and_return({ ok: false }) + end + + it 'returns error response and tracks the exception' do + expect(::Gitlab::ErrorTracking).to receive(:track_exception) + .with( + StandardError.new('Something went wrong while closing the incident form.'), + { + response: { ok: false }, + params: params + } + ) + + service.execute + end + end + end +end diff --git a/spec/services/integrations/slack_interactions/incident_management/incident_modal_submit_service_spec.rb b/spec/services/integrations/slack_interactions/incident_management/incident_modal_submit_service_spec.rb new file mode 100644 index 00000000000..adaeadaa997 --- /dev/null +++ b/spec/services/integrations/slack_interactions/incident_management/incident_modal_submit_service_spec.rb @@ -0,0 +1,296 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Integrations::SlackInteractions::IncidentManagement::IncidentModalSubmitService, + feature_category: :incident_management do + include Gitlab::Routing + + describe '#execute' do + let_it_be(:slack_installation) { create(:slack_integration) } + let_it_be(:project) { create(:project) } + let_it_be(:user) { create(:user) } + let_it_be(:api_url) { 'https://api.slack.com/id/1234' } + + let_it_be(:chat_name) do + create(:chat_name, + user: user, + team_id: slack_installation.team_id, + chat_id: slack_installation.user_id + ) + end + + # Setting below params as they are optional, have added values wherever required in specs + let(:zoom_link) { '' } + let(:severity) { {} } + let(:status) { '' } + let(:assignee_id) { nil } + let(:selected_label_ids) { [] } + let(:label_ids) { { selected_options: selected_label_ids } } + let(:confidential_selected_options) { [] } + let(:confidential) { { selected_options: confidential_selected_options } } + let(:title) { 'Incident title' } + + let(:zoom) do + { + link: { + value: zoom_link + } + } + end + + let(:params) do + { + team: { + id: slack_installation.team_id + }, + user: { + id: slack_installation.user_id + }, + view: { + private_metadata: api_url, + state: { + values: { + title_input: { + title: { + value: title + } + }, + incident_description: { + description: { + value: 'Incident description' + } + }, + project_and_severity_selector: { + incident_management_project: { + selected_option: { + value: project.id.to_s + } + }, + severity: severity + }, + confidentiality: { + confidential: confidential + }, + zoom: zoom, + status_and_assignee_selector: { + status: { + selected_option: { + value: status + } + }, + assignee: { + selected_option: { + value: assignee_id + } + } + }, + label_selector: { + labels: label_ids + } + } + } + } + } + end + + subject(:execute_service) { described_class.new(params).execute } + + shared_examples 'error in creation' do |error_message| + it 'returns error and raises exception' do + expect(::Gitlab::ErrorTracking).to receive(:track_exception) + .with( + described_class::IssueCreateError.new(error_message), + { + slack_workspace_id: slack_installation.team_id, + slack_user_id: slack_installation.user_id + } + ) + + expect(Gitlab::HTTP).to receive(:post) + .with( + api_url, + body: Gitlab::Json.dump( + { + replace_original: 'true', + text: 'There was a problem creating the incident. Please try again.' + } + ), + headers: { 'Content-Type' => 'application/json' } + ) + + response = execute_service + + expect(response).to be_error + expect(response.message).to eq(error_message) + end + end + + context 'when user has permissions to create incidents' do + let(:api_response) { '{"ok":true}' } + + before do + project.add_developer(user) + stub_request(:post, api_url) + .to_return(body: api_response, headers: { 'Content-Type' => 'application/json' }) + end + + context 'with markup string in title' do + let(:title) { '<a href="url">incident title</a>' } + let(:incident) { create(:incident, title: title, project: project) } + + before do + allow_next_instance_of(Issues::CreateService) do |service| + allow(service).to receive(:execute).and_return( + ServiceResponse.success(payload: { issue: incident, error: [] }) + ) + end + end + + it 'strips the markup and saves sends the title' do + expect(Gitlab::HTTP).to receive(:post) + .with( + api_url, + body: Gitlab::Json.dump( + { + replace_original: 'true', + text: "New incident has been created: " \ + "<#{issue_url(incident)}|#{incident.to_reference} - a href=\"url\"incident title/a>. " + } + ), + headers: { 'Content-Type' => 'application/json' } + ).and_return(api_response) + + execute_service + end + end + + context 'with non-optional params' do + it 'creates incident' do + response = execute_service + incident = response[:incident] + + expect(response).to be_success + expect(incident).not_to be_nil + expect(incident.description).to eq('Incident description') + expect(incident.author).to eq(user) + expect(incident.severity).to eq('unknown') + expect(incident.confidential).to be_falsey + expect(incident.escalation_status).to be_triggered + end + + it 'sends incident link to slack' do + execute_service + + expect(WebMock).to have_requested(:post, api_url) + end + end + + context 'with zoom_link' do + let(:zoom_link) { 'https://gitlab.zoom.us/j/1234' } + + it 'sets zoom link as quick action' do + incident = execute_service[:incident] + zoom_meeting = ZoomMeeting.find_by_issue_id(incident.id) + + expect(incident.description).to eq("Incident description") + expect(zoom_meeting.url).to eq(zoom_link) + end + end + + context 'with confidential and severity' do + let(:confidential_selected_options) { ['confidential'] } + let(:severity) do + { + selected_option: { + value: 'high' + } + } + end + + it 'sets confidential and severity' do + incident = execute_service[:incident] + + expect(incident.confidential).to be_truthy + expect(incident.severity).to eq('high') + end + end + + context 'with incident status' do + let(:status) { 'resolved' } + + it 'sets the incident status' do + incident = execute_service[:incident] + + expect(incident.escalation_status).to be_resolved + end + end + + context 'with assignee id' do + let(:assignee_id) { user.id.to_s } + + it 'assigns the incident to user' do + incident = execute_service[:incident] + + expect(incident.assignees).to contain_exactly(user) + end + + context 'when user is not a member of the project' do + let(:assignee_id) { create(:user).id.to_s } + + it 'does not assign the user' do + incident = execute_service[:incident] + + expect(incident.assignees).to be_empty + end + end + end + + context 'with label ids' do + let_it_be(:project_label1) { create(:label, project: project, title: 'Label 1') } + let_it_be(:project_label2) { create(:label, project: project, title: 'Label 2') } + + let(:selected_label_ids) do + [ + { value: project_label1.id.to_s }, + { value: project_label2.id.to_s } + ] + end + + it 'assigns the label to the incident' do + incident = execute_service[:incident] + + expect(incident.labels).to contain_exactly(project_label1, project_label2) + end + end + + context 'when response is not ok' do + let(:api_response) { '{"ok":false}' } + + it 'returns error response and tracks the exception' do + expect(::Gitlab::ErrorTracking).to receive(:track_exception) + .with( + StandardError.new('Something went wrong when sending the incident link to Slack.'), + { + response: { 'ok' => false }, + slack_workspace_id: slack_installation.team_id, + slack_user_id: slack_installation.user_id + } + ) + + execute_service + end + end + + context 'when incident creation fails' do + let(:title) { '' } + + it_behaves_like 'error in creation', "Title can't be blank" + end + end + + context 'when user does not have permission to create incidents' do + it_behaves_like 'error in creation', 'Operation not allowed' + end + end +end diff --git a/spec/services/integrations/slack_interactions/slack_block_actions/incident_management/project_update_handler_spec.rb b/spec/services/integrations/slack_interactions/slack_block_actions/incident_management/project_update_handler_spec.rb new file mode 100644 index 00000000000..5edffc99977 --- /dev/null +++ b/spec/services/integrations/slack_interactions/slack_block_actions/incident_management/project_update_handler_spec.rb @@ -0,0 +1,158 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Integrations::SlackInteractions::SlackBlockActions::IncidentManagement::ProjectUpdateHandler, + feature_category: :incident_management do + describe '#execute' do + let_it_be(:slack_installation) { create(:slack_integration) } + let_it_be(:old_project) { create(:project) } + let_it_be(:new_project) { create(:project) } + let_it_be(:user) { create(:user, developer_projects: [old_project, new_project]) } + let_it_be(:chat_name) { create(:chat_name, user: user) } + let_it_be(:api_url) { "#{Slack::API::BASE_URL}/views.update" } + + let(:block) do + { + block_id: 'incident_description', + element: { + initial_value: '' + } + } + end + + let(:view) do + { + id: 'V04EQH1SP27', + team_id: slack_installation.team_id, + blocks: [block] + } + end + + let(:action) do + { + selected_option: { + value: new_project.id.to_s + } + } + end + + let(:params) do + { + view: view, + user: { + id: slack_installation.user_id + } + } + end + + before do + allow_next_instance_of(ChatNames::FindUserService) do |user_service| + allow(user_service).to receive(:execute).and_return(chat_name) + end + + stub_request(:post, api_url) + .to_return( + status: 200, + body: Gitlab::Json.dump({ ok: true }), + headers: { 'Content-Type' => 'application/json' } + ) + end + + shared_examples 'does not make api call' do + it 'does not make the api call and returns nil' do + expect(Rails.cache).to receive(:read).and_return(project.id.to_s) + expect(Rails.cache).not_to receive(:write) + + expect(execute).to be_nil + expect(WebMock).not_to have_requested(:post, api_url) + end + end + + subject(:execute) { described_class.new(params, action).execute } + + context 'when project is updated' do + it 'returns success response and updates cache' do + expect(Rails.cache).to receive(:read).and_return(old_project.id.to_s) + expect(Rails.cache).to receive(:write).with( + "slack:incident_modal_opened:#{view[:id]}", + new_project.id.to_s, + expires_in: 5.minutes + ) + + expect(execute.message).to eq('Modal updated') + + updated_block = block.dup + updated_block[:block_id] = new_project.id.to_s + view[:blocks] = [updated_block] + + expect(WebMock).to have_requested(:post, api_url).with( + body: { + view_id: view[:id], + view: view.except!(:team_id, :id) + }, + headers: { + 'Authorization' => "Bearer #{slack_installation.bot_access_token}", + 'Content-Type' => 'application/json; charset=utf-8' + }) + end + end + + context 'when project is unchanged' do + it_behaves_like 'does not make api call' do + let(:project) { new_project } + end + end + + context 'when user does not have permission to read a project' do + it_behaves_like 'does not make api call' do + let(:project) { create(:project) } + end + end + + context 'when api response is not ok' do + before do + stub_request(:post, api_url) + .to_return( + status: 404, + body: Gitlab::Json.dump({ ok: false }), + headers: { 'Content-Type' => 'application/json' } + ) + end + + it 'returns error response' do + expect(Rails.cache).to receive(:read).and_return(old_project.id.to_s) + expect(::Gitlab::ErrorTracking).to receive(:track_exception) + .with( + StandardError.new('Something went wrong while updating the modal.'), + { + response: { "ok" => false }, + slack_workspace_id: slack_installation.team_id, + slack_user_id: slack_installation.user_id + } + ) + + expect(execute.message).to eq('Something went wrong while updating the modal.') + end + end + + context 'when Slack API call raises an HTTP exception' do + before do + allow(Gitlab::HTTP).to receive(:post).and_raise(Errno::ECONNREFUSED, 'error message') + end + + it 'tracks the exception and returns an error message' do + expect(Rails.cache).to receive(:read).and_return(old_project.id.to_s) + expect(::Gitlab::ErrorTracking).to receive(:track_exception) + .with( + Errno::ECONNREFUSED.new('HTTP exception when calling Slack API'), + { + slack_workspace_id: slack_installation.team_id + } + ) + + expect(execute).to be_error + end + end + end +end diff --git a/spec/services/merge_requests/reload_diffs_service_spec.rb b/spec/services/merge_requests/reload_diffs_service_spec.rb index 675b458c435..77056cbe541 100644 --- a/spec/services/merge_requests/reload_diffs_service_spec.rb +++ b/spec/services/merge_requests/reload_diffs_service_spec.rb @@ -34,8 +34,9 @@ RSpec.describe MergeRequests::ReloadDiffsService, :use_clean_rails_memory_store_ context 'cache clearing' do it 'clears the cache for older diffs on the merge request' do - expect_any_instance_of(Redis).to receive(:del).once.and_call_original - expect(Rails.cache).to receive(:delete).once.and_call_original + expect_next_instance_of(Gitlab::Diff::FileCollection::MergeRequestDiff) do |instance| + expect(instance).to receive(:clear_cache).and_call_original + end subject.execute end diff --git a/spec/views/shared/_label_row.html.haml_spec.rb b/spec/views/shared/_label_row.html.haml_spec.rb index 6fe74b6633b..eb277930c1d 100644 --- a/spec/views/shared/_label_row.html.haml_spec.rb +++ b/spec/views/shared/_label_row.html.haml_spec.rb @@ -38,7 +38,7 @@ RSpec.describe 'shared/_label_row.html.haml' do end it 'shows the path from where the label was created' do - expect(rendered).to have_css('.label-badge', text: project.full_name) + expect(rendered).to have_text(project.full_name) end end @@ -70,7 +70,7 @@ RSpec.describe 'shared/_label_row.html.haml' do end it 'shows the path from where the label was created' do - expect(rendered).to have_css('.label-badge', text: subgroup.full_name) + expect(rendered).to have_text(subgroup.full_name) end end diff --git a/spec/workers/integrations/slack_event_worker_spec.rb b/spec/workers/integrations/slack_event_worker_spec.rb new file mode 100644 index 00000000000..6754801a2bd --- /dev/null +++ b/spec/workers/integrations/slack_event_worker_spec.rb @@ -0,0 +1,129 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Integrations::SlackEventWorker, :clean_gitlab_redis_shared_state, feature_category: :integrations do + describe '.event?' do + subject { described_class.event?(event) } + + context 'when event is known' do + let(:event) { 'app_home_opened' } + + it { is_expected.to eq(true) } + end + + context 'when event is not known' do + let(:event) { 'foo' } + + it { is_expected.to eq(false) } + end + end + + describe '#perform' do + let(:worker) { described_class.new } + let(:event) { 'app_home_opened' } + let(:service_class) { ::Integrations::SlackEvents::AppHomeOpenedService } + + let(:args) do + { + slack_event: event, + params: params + } + end + + let(:params) do + { + team_id: "T0123A456BC", + event: { user: "U0123ABCDEF" }, + event_id: "Ev03SA75UJKB" + } + end + + shared_examples 'logs extra metadata on done' do + specify do + expect(worker).to receive(:log_extra_metadata_on_done).with(:slack_event, event) + expect(worker).to receive(:log_extra_metadata_on_done).with(:slack_user_id, 'U0123ABCDEF') + expect(worker).to receive(:log_extra_metadata_on_done).with(:slack_workspace_id, 'T0123A456BC') + + worker.perform(args) + end + end + + it 'executes the correct service' do + expect_next_instance_of(service_class, params) do |service| + expect(service).to receive(:execute).and_return(ServiceResponse.success) + end + + worker.perform(args) + end + + it_behaves_like 'logs extra metadata on done' + + it_behaves_like 'an idempotent worker' do + let(:job_args) { [args] } + end + + it 'ensures idempotency when called twice by only executing service once' do + expect_next_instances_of(service_class, 1, params) do |service| + expect(service).to receive(:execute).and_return(ServiceResponse.success) + end + + worker.perform(args) + worker.perform(args) + end + + it 'executes service twice if service returned an error' do + expect_next_instances_of(service_class, 2, params) do |service| + expect(service).to receive(:execute).and_return(ServiceResponse.error(message: 'foo')) + end + + worker.perform(args) + worker.perform(args) + end + + it 'executes service twice if service raised an error' do + expect_next_instances_of(service_class, 2, params) do |service| + expect(service).to receive(:execute).and_raise(ArgumentError) + end + + expect { worker.perform(args) }.to raise_error(ArgumentError) + expect { worker.perform(args) }.to raise_error(ArgumentError) + end + + it 'executes service twice when event_id is different' do + second_params = params.dup + second_args = args.dup + second_params[:event_id] = 'foo' + second_args[:params] = second_params + + expect_next_instances_of(service_class, 1, params) do |service| + expect(service).to receive(:execute).and_return(ServiceResponse.success) + end + + expect_next_instances_of(service_class, 1, second_params) do |service| + expect(service).to receive(:execute).and_return(ServiceResponse.success) + end + + worker.perform(args) + worker.perform(second_args) + end + + context 'when event is not known' do + let(:event) { 'foo' } + + it 'does not execute the service class' do + expect(service_class).not_to receive(:new) + + worker.perform(args) + end + + it 'logs an error' do + expect(Sidekiq.logger).to receive(:error).with({ message: 'Unknown slack_event', slack_event: event }) + + worker.perform(args) + end + + it_behaves_like 'logs extra metadata on done' + end + end +end |