diff options
Diffstat (limited to 'spec')
41 files changed, 341 insertions, 478 deletions
diff --git a/spec/controllers/projects/usage_ping_controller_spec.rb b/spec/controllers/projects/service_ping_controller_spec.rb index 9ace072d561..e6afaadc75f 100644 --- a/spec/controllers/projects/usage_ping_controller_spec.rb +++ b/spec/controllers/projects/service_ping_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Projects::UsagePingController do +RSpec.describe Projects::ServicePingController do let_it_be(:project) { create(:project) } let_it_be(:user) { create(:user) } diff --git a/spec/factories/integrations.rb b/spec/factories/integrations.rb index 8e0f5b50db5..ed8a562b331 100644 --- a/spec/factories/integrations.rb +++ b/spec/factories/integrations.rb @@ -97,7 +97,7 @@ FactoryBot.define do issue_tracker end - factory :youtrack_service, class: 'Integrations::Youtrack' do + factory :youtrack_integration, class: 'Integrations::Youtrack' do project active { true } issue_tracker diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb index fa538551987..84686c58a8e 100644 --- a/spec/factories/projects.rb +++ b/spec/factories/projects.rb @@ -402,7 +402,7 @@ FactoryBot.define do factory :youtrack_project, parent: :project do has_external_issue_tracker { true } - youtrack_service + youtrack_integration end factory :jira_project, parent: :project do diff --git a/spec/features/calendar_spec.rb b/spec/features/calendar_spec.rb index 21da92c9f43..a8aa3f0b36a 100644 --- a/spec/features/calendar_spec.rb +++ b/spec/features/calendar_spec.rb @@ -145,7 +145,7 @@ RSpec.describe 'Contributions Calendar', :js do describe '1 issue creation calendar activity' do before do - Issues::CreateService.new(project: contributed_project, current_user: user, params: issue_params).execute + Issues::CreateService.new(project: contributed_project, current_user: user, params: issue_params, spam_params: nil).execute end it_behaves_like 'a day with activity', contribution_count: 1 @@ -180,7 +180,7 @@ RSpec.describe 'Contributions Calendar', :js do push_code_contribution travel_to(Date.yesterday) do - Issues::CreateService.new(project: contributed_project, current_user: user, params: issue_params).execute + Issues::CreateService.new(project: contributed_project, current_user: user, params: issue_params, spam_params: nil).execute end end include_context 'visit user page' diff --git a/spec/features/unsubscribe_links_spec.rb b/spec/features/unsubscribe_links_spec.rb index b2d0f29808c..b7471720008 100644 --- a/spec/features/unsubscribe_links_spec.rb +++ b/spec/features/unsubscribe_links_spec.rb @@ -9,7 +9,7 @@ RSpec.describe 'Unsubscribe links', :sidekiq_might_not_need_inline do let(:author) { create(:user) } let(:project) { create(:project, :public) } let(:params) { { title: 'A bug!', description: 'Fix it!', assignees: [recipient] } } - let(:issue) { Issues::CreateService.new(project: project, current_user: author, params: params).execute } + let(:issue) { Issues::CreateService.new(project: project, current_user: author, params: params, spam_params: nil).execute } let(:mail) { ActionMailer::Base.deliveries.last } let(:body) { Capybara::Node::Simple.new(mail.default_part_body.to_s) } diff --git a/spec/features/users/user_browses_projects_on_user_page_spec.rb b/spec/features/users/user_browses_projects_on_user_page_spec.rb index ded90be3924..5e7d7b76843 100644 --- a/spec/features/users/user_browses_projects_on_user_page_spec.rb +++ b/spec/features/users/user_browses_projects_on_user_page_spec.rb @@ -125,7 +125,7 @@ RSpec.describe 'Users > User browses projects on user page', :js do end before do - Issues::CreateService.new(project: contributed_project, current_user: user, params: { title: 'Bug in old browser' }).execute + Issues::CreateService.new(project: contributed_project, current_user: user, params: { title: 'Bug in old browser' }, spam_params: nil).execute event = create(:push_event, project: contributed_project, author: user) create(:push_event_payload, event: event, commit_count: 3) end diff --git a/spec/frontend/environments/environments_app_spec.js b/spec/frontend/environments/environments_app_spec.js index 542cf58b079..5403d5d06c0 100644 --- a/spec/frontend/environments/environments_app_spec.js +++ b/spec/frontend/environments/environments_app_spec.js @@ -1,3 +1,4 @@ +import { GlTabs } from '@gitlab/ui'; import { mount, shallowMount } from '@vue/test-utils'; import MockAdapter from 'axios-mock-adapter'; import { extendedWrapper } from 'helpers/vue_test_utils_helper'; @@ -7,6 +8,7 @@ import EmptyState from '~/environments/components/empty_state.vue'; import EnableReviewAppModal from '~/environments/components/enable_review_app_modal.vue'; import EnvironmentsApp from '~/environments/components/environments_app.vue'; import axios from '~/lib/utils/axios_utils'; +import * as utils from '~/lib/utils/common_utils'; import { environment, folder } from './mock_data'; describe('Environment', () => { @@ -264,4 +266,18 @@ describe('Environment', () => { }); }); }); + + describe('tabs', () => { + beforeEach(() => { + mockRequest(200, { environments: [] }); + jest + .spyOn(utils, 'getParameterByName') + .mockImplementation((param) => (param === 'scope' ? 'stopped' : null)); + return createWrapper(true); + }); + + it('selects the tab for the parameter', () => { + expect(wrapper.findComponent(GlTabs).attributes('value')).toBe('1'); + }); + }); }); diff --git a/spec/frontend/issues_list/components/issues_list_app_spec.js b/spec/frontend/issues_list/components/issues_list_app_spec.js index a3ac57ee1bb..00b03488f0c 100644 --- a/spec/frontend/issues_list/components/issues_list_app_spec.js +++ b/spec/frontend/issues_list/components/issues_list_app_spec.js @@ -21,7 +21,6 @@ import IssuableList from '~/issuable_list/components/issuable_list_root.vue'; import { IssuableListTabs, IssuableStates } from '~/issuable_list/constants'; import IssuesListApp from '~/issues_list/components/issues_list_app.vue'; import { - apiSortParams, CREATED_DESC, DUE_DATE_OVERDUE, PARAM_DUE_DATE, @@ -148,8 +147,8 @@ describe('IssuesListApp component', () => { hasPreviousPage: getIssuesQueryResponse.data.project.issues.pageInfo.hasPreviousPage, hasNextPage: getIssuesQueryResponse.data.project.issues.pageInfo.hasNextPage, urlParams: { + sort: urlSortParams[CREATED_DESC], state: IssuableStates.Opened, - ...urlSortParams[CREATED_DESC], }, }); }); @@ -178,7 +177,7 @@ describe('IssuesListApp component', () => { describe('csv import/export component', () => { describe('when user is signed in', () => { - const search = '?search=refactor&state=opened&sort=created_date'; + const search = '?search=refactor&sort=created_date&state=opened'; beforeEach(() => { global.jsdom.reconfigure({ url: `${TEST_HOST}${search}` }); @@ -273,13 +272,17 @@ describe('IssuesListApp component', () => { describe('sort', () => { it.each(Object.keys(urlSortParams))('is set as %s from the url params', (sortKey) => { - global.jsdom.reconfigure({ url: setUrlParams(urlSortParams[sortKey], TEST_HOST) }); + global.jsdom.reconfigure({ + url: setUrlParams({ sort: urlSortParams[sortKey] }, TEST_HOST), + }); wrapper = mountComponent(); expect(findIssuableList().props()).toMatchObject({ initialSortBy: sortKey, - urlParams: urlSortParams[sortKey], + urlParams: { + sort: urlSortParams[sortKey], + }, }); }); }); @@ -640,7 +643,7 @@ describe('IssuesListApp component', () => { }); describe('when "sort" event is emitted by IssuableList', () => { - it.each(Object.keys(apiSortParams))( + it.each(Object.keys(urlSortParams))( 'updates to the new sort when payload is `%s`', async (sortKey) => { wrapper = mountComponent(); @@ -650,7 +653,9 @@ describe('IssuesListApp component', () => { jest.runOnlyPendingTimers(); await nextTick(); - expect(findIssuableList().props('urlParams')).toMatchObject(urlSortParams[sortKey]); + expect(findIssuableList().props('urlParams')).toMatchObject({ + sort: urlSortParams[sortKey], + }); }, ); }); diff --git a/spec/frontend/issues_list/mock_data.js b/spec/frontend/issues_list/mock_data.js index 6c669e02070..e6ee038c688 100644 --- a/spec/frontend/issues_list/mock_data.js +++ b/spec/frontend/issues_list/mock_data.js @@ -9,7 +9,7 @@ export const getIssuesQueryResponse = { issues: { count: 1, pageInfo: { - hasNextPage: false, + hasNextPage: true, hasPreviousPage: false, startCursor: 'startcursor', endCursor: 'endcursor', @@ -86,10 +86,10 @@ export const locationSearch = [ 'not[label_name][]=drama', 'my_reaction_emoji=thumbsup', 'confidential=no', - 'iteration_title=season:+%234', - 'not[iteration_title]=season:+%2320', - 'epic_id=gitlab-org%3A%3A%2612', - 'not[epic_id]=gitlab-org%3A%3A%2634', + 'iteration_id=4', + 'not[iteration_id]=20', + 'epic_id=12', + 'not[epic_id]=34', 'weight=1', 'not[weight]=3', ].join('&'); @@ -118,10 +118,10 @@ export const filteredTokens = [ { type: 'labels', value: { data: 'drama', operator: OPERATOR_IS_NOT } }, { type: 'my_reaction_emoji', value: { data: 'thumbsup', operator: OPERATOR_IS } }, { type: 'confidential', value: { data: 'no', operator: OPERATOR_IS } }, - { type: 'iteration', value: { data: 'season: #4', operator: OPERATOR_IS } }, - { type: 'iteration', value: { data: 'season: #20', operator: OPERATOR_IS_NOT } }, - { type: 'epic_id', value: { data: 'gitlab-org::&12', operator: OPERATOR_IS } }, - { type: 'epic_id', value: { data: 'gitlab-org::&34', operator: OPERATOR_IS_NOT } }, + { type: 'iteration', value: { data: '4', operator: OPERATOR_IS } }, + { type: 'iteration', value: { data: '20', operator: OPERATOR_IS_NOT } }, + { type: 'epic_id', value: { data: '12', operator: OPERATOR_IS } }, + { type: 'epic_id', value: { data: '34', operator: OPERATOR_IS_NOT } }, { type: 'weight', value: { data: '1', operator: OPERATOR_IS } }, { type: 'weight', value: { data: '3', operator: OPERATOR_IS_NOT } }, { type: 'filtered-search-term', value: { data: 'find' } }, @@ -138,30 +138,32 @@ export const filteredTokensWithSpecialValues = [ ]; export const apiParams = { - author_username: 'homer', - 'not[author_username]': 'marge', - assignee_username: ['bart', 'lisa'], - 'not[assignee_username]': ['patty', 'selma'], - milestone: 'season 4', - 'not[milestone]': 'season 20', - labels: ['cartoon', 'tv'], - 'not[labels]': ['live action', 'drama'], - my_reaction_emoji: 'thumbsup', + authorUsername: 'homer', + assigneeUsernames: ['bart', 'lisa'], + milestoneTitle: 'season 4', + labelName: ['cartoon', 'tv'], + myReactionEmoji: 'thumbsup', confidential: 'no', - iteration_title: 'season: #4', - 'not[iteration_title]': 'season: #20', - epic_id: '12', - 'not[epic_id]': 'gitlab-org::&34', + iterationId: '4', + epicId: '12', weight: '1', - 'not[weight]': '3', + not: { + authorUsername: 'marge', + assigneeUsernames: ['patty', 'selma'], + milestoneTitle: 'season 20', + labelName: ['live action', 'drama'], + iterationId: '20', + epicId: '34', + weight: '3', + }, }; export const apiParamsWithSpecialValues = { - assignee_id: '123', - assignee_username: 'bart', - my_reaction_emoji: 'None', - iteration_id: 'Current', - epic_id: 'None', + assigneeId: '123', + assigneeUsernames: 'bart', + myReactionEmoji: 'None', + iterationWildcardId: 'CURRENT', + epicId: 'None', weight: 'None', }; @@ -176,10 +178,10 @@ export const urlParams = { 'not[label_name][]': ['live action', 'drama'], my_reaction_emoji: 'thumbsup', confidential: 'no', - iteration_title: 'season: #4', - 'not[iteration_title]': 'season: #20', - epic_id: 'gitlab-org%3A%3A%2612', - 'not[epic_id]': 'gitlab-org::&34', + iteration_id: '4', + 'not[iteration_id]': '20', + epic_id: '12', + 'not[epic_id]': '34', weight: '1', 'not[weight]': '3', }; diff --git a/spec/frontend/issues_list/utils_spec.js b/spec/frontend/issues_list/utils_spec.js index e377c35a0aa..b7863068570 100644 --- a/spec/frontend/issues_list/utils_spec.js +++ b/spec/frontend/issues_list/utils_spec.js @@ -8,10 +8,11 @@ import { urlParams, urlParamsWithSpecialValues, } from 'jest/issues_list/mock_data'; -import { API_PARAM, DUE_DATE_VALUES, URL_PARAM, urlSortParams } from '~/issues_list/constants'; +import { DUE_DATE_VALUES, urlSortParams } from '~/issues_list/constants'; import { - convertToParams, + convertToApiParams, convertToSearchQuery, + convertToUrlParams, getDueDateValue, getFilterTokens, getSortKey, @@ -20,7 +21,7 @@ import { describe('getSortKey', () => { it.each(Object.keys(urlSortParams))('returns %s given the correct inputs', (sortKey) => { - const { sort } = urlSortParams[sortKey]; + const sort = urlSortParams[sortKey]; expect(getSortKey(sort)).toBe(sortKey); }); }); @@ -80,31 +81,23 @@ describe('getFilterTokens', () => { }); }); -describe('convertToParams', () => { +describe('convertToApiParams', () => { it('returns api params given filtered tokens', () => { - expect(convertToParams(filteredTokens, API_PARAM)).toEqual({ - ...apiParams, - epic_id: 'gitlab-org::&12', - }); + expect(convertToApiParams(filteredTokens)).toEqual(apiParams); }); it('returns api params given filtered tokens with special values', () => { - expect(convertToParams(filteredTokensWithSpecialValues, API_PARAM)).toEqual( - apiParamsWithSpecialValues, - ); + expect(convertToApiParams(filteredTokensWithSpecialValues)).toEqual(apiParamsWithSpecialValues); }); +}); +describe('convertToUrlParams', () => { it('returns url params given filtered tokens', () => { - expect(convertToParams(filteredTokens, URL_PARAM)).toEqual({ - ...urlParams, - epic_id: 'gitlab-org::&12', - }); + expect(convertToUrlParams(filteredTokens)).toEqual(urlParams); }); it('returns url params given filtered tokens with special values', () => { - expect(convertToParams(filteredTokensWithSpecialValues, URL_PARAM)).toEqual( - urlParamsWithSpecialValues, - ); + expect(convertToUrlParams(filteredTokensWithSpecialValues)).toEqual(urlParamsWithSpecialValues); }); }); diff --git a/spec/frontend/vue_shared/components/filtered_search_bar/tokens/iteration_token_spec.js b/spec/frontend/vue_shared/components/filtered_search_bar/tokens/iteration_token_spec.js index ca5dc984ae0..bd654c5a9cb 100644 --- a/spec/frontend/vue_shared/components/filtered_search_bar/tokens/iteration_token_spec.js +++ b/spec/frontend/vue_shared/components/filtered_search_bar/tokens/iteration_token_spec.js @@ -7,7 +7,7 @@ import { mockIterationToken } from '../mock_data'; jest.mock('~/flash'); describe('IterationToken', () => { - const title = 'gitlab-org: #1'; + const id = 123; let wrapper; const createComponent = ({ config = mockIterationToken, value = { data: '' } } = {}) => @@ -28,14 +28,14 @@ describe('IterationToken', () => { }); it('renders iteration value', async () => { - wrapper = createComponent({ value: { data: title } }); + wrapper = createComponent({ value: { data: id } }); await wrapper.vm.$nextTick(); const tokenSegments = wrapper.findAllComponents(GlFilteredSearchTokenSegment); expect(tokenSegments).toHaveLength(3); // `Iteration` `=` `gitlab-org: #1` - expect(tokenSegments.at(2).text()).toBe(title); + expect(tokenSegments.at(2).text()).toBe(id.toString()); }); it('fetches initial values', () => { @@ -43,10 +43,10 @@ describe('IterationToken', () => { wrapper = createComponent({ config: { ...mockIterationToken, fetchIterations: fetchIterationsSpy }, - value: { data: title }, + value: { data: id }, }); - expect(fetchIterationsSpy).toHaveBeenCalledWith(title); + expect(fetchIterationsSpy).toHaveBeenCalledWith(id); }); it('fetches iterations on user input', () => { diff --git a/spec/frontend/vue_shared/components/user_select_spec.js b/spec/frontend/vue_shared/components/user_select_spec.js index 0fabc6525ea..b777ac0a0a4 100644 --- a/spec/frontend/vue_shared/components/user_select_spec.js +++ b/spec/frontend/vue_shared/components/user_select_spec.js @@ -275,48 +275,4 @@ describe('User select dropdown', () => { expect(findEmptySearchResults().exists()).toBe(true); }); }); - - // TODO Remove this test after the following issue is resolved in the backend - // https://gitlab.com/gitlab-org/gitlab/-/issues/329750 - describe('temporary error suppression', () => { - beforeEach(() => { - jest.spyOn(console, 'error').mockImplementation(); - }); - - const nullError = { message: 'Cannot return null for non-nullable field GroupMember.user' }; - - it.each` - mockErrors - ${[nullError]} - ${[nullError, nullError]} - `('does not emit errors', async ({ mockErrors }) => { - createComponent({ - searchQueryHandler: jest.fn().mockResolvedValue({ - errors: mockErrors, - }), - }); - await waitForSearch(); - - expect(wrapper.emitted()).toEqual({}); - // eslint-disable-next-line no-console - expect(console.error).toHaveBeenCalled(); - }); - - it.each` - mockErrors - ${[{ message: 'serious error' }]} - ${[nullError, { message: 'serious error' }]} - `('emits error when non-null related errors are included', async ({ mockErrors }) => { - createComponent({ - searchQueryHandler: jest.fn().mockResolvedValue({ - errors: mockErrors, - }), - }); - await waitForSearch(); - - expect(wrapper.emitted('error')).toEqual([[]]); - // eslint-disable-next-line no-console - expect(console.error).not.toHaveBeenCalled(); - }); - }); }); diff --git a/spec/graphql/mutations/issues/create_spec.rb b/spec/graphql/mutations/issues/create_spec.rb index b32f0991959..0e7ef0e55b9 100644 --- a/spec/graphql/mutations/issues/create_spec.rb +++ b/spec/graphql/mutations/issues/create_spec.rb @@ -50,6 +50,7 @@ RSpec.describe Mutations::Issues::Create do stub_licensed_features(multiple_issue_assignees: false, issue_weights: false) project.add_guest(assignee1) project.add_guest(assignee2) + stub_spam_services end subject { mutation.resolve(**mutation_params) } diff --git a/spec/graphql/mutations/issues/set_confidential_spec.rb b/spec/graphql/mutations/issues/set_confidential_spec.rb index c3269e5c0c0..495b8442d95 100644 --- a/spec/graphql/mutations/issues/set_confidential_spec.rb +++ b/spec/graphql/mutations/issues/set_confidential_spec.rb @@ -17,6 +17,10 @@ RSpec.describe Mutations::Issues::SetConfidential do subject { mutation.resolve(project_path: project.full_path, iid: issue.iid, confidential: confidential) } + before do + stub_spam_services + end + it_behaves_like 'permission level for issue mutation is correctly verified' context 'when the user can update the issue' do diff --git a/spec/graphql/mutations/issues/update_spec.rb b/spec/graphql/mutations/issues/update_spec.rb index bd780477658..80f43338bb5 100644 --- a/spec/graphql/mutations/issues/update_spec.rb +++ b/spec/graphql/mutations/issues/update_spec.rb @@ -35,6 +35,10 @@ RSpec.describe Mutations::Issues::Update do subject { mutation.resolve(**mutation_params) } + before do + stub_spam_services + end + it_behaves_like 'permission level for issue mutation is correctly verified' context 'when the user can update the issue' do diff --git a/spec/lib/banzai/filter/references/external_issue_reference_filter_spec.rb b/spec/lib/banzai/filter/references/external_issue_reference_filter_spec.rb index 503477b2115..dbb8bfc82f3 100644 --- a/spec/lib/banzai/filter/references/external_issue_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/references/external_issue_reference_filter_spec.rb @@ -140,7 +140,9 @@ RSpec.describe Banzai::Filter::References::ExternalIssueReferenceFilter do end context "youtrack project" do - let_it_be(:service) { create(:youtrack_service, project: project) } + before_all do + create(:youtrack_integration, project: project) + end before do project.update!(issues_enabled: false) diff --git a/spec/lib/gitlab/email/handler/service_desk_handler_spec.rb b/spec/lib/gitlab/email/handler/service_desk_handler_spec.rb index 3a60564d8d2..f6b618163f0 100644 --- a/spec/lib/gitlab/email/handler/service_desk_handler_spec.rb +++ b/spec/lib/gitlab/email/handler/service_desk_handler_spec.rb @@ -50,6 +50,15 @@ RSpec.describe Gitlab::Email::Handler::ServiceDeskHandler do it 'sends thank you email' do expect { receiver.execute }.to have_enqueued_job.on_queue('mailers') end + + it 'adds metric events for incoming and reply emails' do + metric_transaction = double('Gitlab::Metrics::WebTransaction', increment: true, observe: true) + allow(::Gitlab::Metrics::BackgroundTransaction).to receive(:current).and_return(metric_transaction) + expect(metric_transaction).to receive(:add_event).with(:receive_email_service_desk, anything) + expect(metric_transaction).to receive(:add_event).with(:service_desk_thank_you_email) + + receiver.execute + end end context 'when everything is fine' do diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index f84f06da9cf..a7642d5e3c3 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -381,14 +381,14 @@ project: - microsoft_teams_integration - mattermost_integration - hangouts_chat_integration -- unify_circuit_service +- unify_circuit_integration - buildkite_integration - bamboo_integration - teamcity_integration - pushover_integration - jira_integration - redmine_integration -- youtrack_service +- youtrack_integration - custom_issue_tracker_integration - bugzilla_integration - ewm_integration @@ -557,7 +557,7 @@ project: - alert_management_alerts - repository_storage_moves - freeze_periods -- webex_teams_service +- webex_teams_integration - build_report_results - vulnerability_statistic - vulnerability_historical_statistics diff --git a/spec/mailers/emails/service_desk_spec.rb b/spec/mailers/emails/service_desk_spec.rb index 995e6c006cd..57fa990d399 100644 --- a/spec/mailers/emails/service_desk_spec.rb +++ b/spec/mailers/emails/service_desk_spec.rb @@ -115,16 +115,6 @@ RSpec.describe Emails::ServiceDesk do end end - shared_examples 'notification with metric event' do |event_type| - it 'adds metric event' do - metric_transaction = double('Gitlab::Metrics::WebTransaction', increment: true, observe: true) - allow(::Gitlab::Metrics::BackgroundTransaction).to receive(:current).and_return(metric_transaction) - expect(metric_transaction).to receive(:add_event).with(event_type) - - subject.content_type - end - end - describe '.service_desk_thank_you_email' do let_it_be(:reply_in_subject) { true } let_it_be(:default_text) do @@ -134,7 +124,6 @@ RSpec.describe Emails::ServiceDesk do subject { ServiceEmailClass.service_desk_thank_you_email(issue.id) } it_behaves_like 'read template from repository', 'thank_you' - it_behaves_like 'notification with metric event', :service_desk_thank_you_email context 'handling template markdown' do context 'with a simple text' do @@ -175,7 +164,6 @@ RSpec.describe Emails::ServiceDesk do subject { ServiceEmailClass.service_desk_new_note_email(issue.id, note.id, email) } it_behaves_like 'read template from repository', 'new_note' - it_behaves_like 'notification with metric event', :service_desk_new_note_email context 'handling template markdown' do context 'with a simple text' do diff --git a/spec/migrations/20190924152703_migrate_issue_trackers_data_spec.rb b/spec/migrations/20190924152703_migrate_issue_trackers_data_spec.rb index ea5192375f3..dad95760306 100644 --- a/spec/migrations/20190924152703_migrate_issue_trackers_data_spec.rb +++ b/spec/migrations/20190924152703_migrate_issue_trackers_data_spec.rb @@ -26,11 +26,11 @@ RSpec.describe MigrateIssueTrackersData do services.create!(type: 'BugzillaService', properties: properties, category: 'issue_tracker') end - let!(:youtrack_service) do + let!(:youtrack_integration) do services.create!(type: 'YoutrackService', properties: properties, category: 'issue_tracker') end - let!(:youtrack_service_empty) do + let!(:youtrack_integration_empty) do services.create!(type: 'YoutrackService', properties: '', category: 'issue_tracker') end @@ -56,7 +56,7 @@ RSpec.describe MigrateIssueTrackersData do migrate! expect(migration_name).to be_scheduled_delayed_migration(3.minutes, jira_integration.id, bugzilla_integration.id) - expect(migration_name).to be_scheduled_delayed_migration(6.minutes, youtrack_service.id, gitlab_service.id) + expect(migration_name).to be_scheduled_delayed_migration(6.minutes, youtrack_integration.id, gitlab_service.id) expect(BackgroundMigrationWorker.jobs.size).to eq(2) end end diff --git a/spec/migrations/20200130145430_reschedule_migrate_issue_trackers_data_spec.rb b/spec/migrations/20200130145430_reschedule_migrate_issue_trackers_data_spec.rb index 90bbdca4d9c..cf8bc608483 100644 --- a/spec/migrations/20200130145430_reschedule_migrate_issue_trackers_data_spec.rb +++ b/spec/migrations/20200130145430_reschedule_migrate_issue_trackers_data_spec.rb @@ -26,11 +26,11 @@ RSpec.describe RescheduleMigrateIssueTrackersData do services.create!(id: 12, type: 'BugzillaService', properties: properties, category: 'issue_tracker') end - let!(:youtrack_service) do + let!(:youtrack_integration) do services.create!(id: 13, type: 'YoutrackService', properties: properties, category: 'issue_tracker') end - let!(:youtrack_service_empty) do + let!(:youtrack_integration_empty) do services.create!(id: 14, type: 'YoutrackService', properties: '', category: 'issue_tracker') end @@ -57,7 +57,7 @@ RSpec.describe RescheduleMigrateIssueTrackersData do migrate! expect(migration_name).to be_scheduled_delayed_migration(3.minutes, jira_integration.id, bugzilla_integration.id) - expect(migration_name).to be_scheduled_delayed_migration(6.minutes, youtrack_service.id, gitlab_service.id) + expect(migration_name).to be_scheduled_delayed_migration(6.minutes, youtrack_integration.id, gitlab_service.id) expect(BackgroundMigrationWorker.jobs.size).to eq(2) end end diff --git a/spec/models/integrations/microsoft_teams_spec.rb b/spec/models/integrations/microsoft_teams_spec.rb index 7341905e71b..a7b45ebec87 100644 --- a/spec/models/integrations/microsoft_teams_spec.rb +++ b/spec/models/integrations/microsoft_teams_spec.rb @@ -74,7 +74,7 @@ RSpec.describe Integrations::MicrosoftTeams do context 'with issue events' do let(:opts) { { title: 'Awesome issue', description: 'please fix' } } let(:issues_sample_data) do - service = Issues::CreateService.new(project: project, current_user: user, params: opts) + service = Issues::CreateService.new(project: project, current_user: user, params: opts, spam_params: nil) issue = service.execute service.hook_data(issue, 'open') end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 00fee3a3104..1db8e1d81a4 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -39,8 +39,8 @@ RSpec.describe Project, factory_default: :keep do it { is_expected.to have_one(:microsoft_teams_integration) } it { is_expected.to have_one(:mattermost_integration) } it { is_expected.to have_one(:hangouts_chat_integration) } - it { is_expected.to have_one(:unify_circuit_service) } - it { is_expected.to have_one(:webex_teams_service) } + it { is_expected.to have_one(:unify_circuit_integration) } + it { is_expected.to have_one(:webex_teams_integration) } it { is_expected.to have_one(:packagist_integration) } it { is_expected.to have_one(:pushover_integration) } it { is_expected.to have_one(:asana_integration) } @@ -62,7 +62,7 @@ RSpec.describe Project, factory_default: :keep do it { is_expected.to have_one(:teamcity_integration) } it { is_expected.to have_one(:jira_integration) } it { is_expected.to have_one(:redmine_integration) } - it { is_expected.to have_one(:youtrack_service) } + it { is_expected.to have_one(:youtrack_integration) } it { is_expected.to have_one(:custom_issue_tracker_integration) } it { is_expected.to have_one(:bugzilla_integration) } it { is_expected.to have_one(:ewm_integration) } @@ -5870,26 +5870,6 @@ RSpec.describe Project, factory_default: :keep do end end - describe '#disabled_services' do - subject { build(:project).disabled_services } - - context 'without datadog_ci_integration' do - before do - stub_feature_flags(datadog_ci_integration: false) - end - - it { is_expected.to include('datadog') } - end - - context 'with datadog_ci_integration' do - before do - stub_feature_flags(datadog_ci_integration: true) - end - - it { is_expected.not_to include('datadog') } - end - end - describe '#find_or_initialize_service' do it 'avoids N+1 database queries' do allow(Integration).to receive(:available_services_names).and_return(%w[prometheus pushover]) diff --git a/spec/requests/api/graphql/mutations/snippets/create_spec.rb b/spec/requests/api/graphql/mutations/snippets/create_spec.rb index 214c804c519..d329b9aea6a 100644 --- a/spec/requests/api/graphql/mutations/snippets/create_spec.rb +++ b/spec/requests/api/graphql/mutations/snippets/create_spec.rb @@ -17,7 +17,6 @@ RSpec.describe 'Creating a Snippet' do let(:actions) { [{ action: action }.merge(file_1), { action: action }.merge(file_2)] } let(:project_path) { nil } let(:uploaded_files) { nil } - let(:spam_mutation_vars) { {} } let(:mutation_vars) do { description: description, @@ -26,7 +25,7 @@ RSpec.describe 'Creating a Snippet' do project_path: project_path, uploaded_files: uploaded_files, blob_actions: actions - }.merge(spam_mutation_vars) + } end let(:mutation) do @@ -77,21 +76,6 @@ RSpec.describe 'Creating a Snippet' do expect(mutation_response['snippet']).to be_nil end - - context 'when snippet_spam flag is disabled' do - before do - stub_feature_flags(snippet_spam: false) - end - - it 'passes disable_spam_action_service param to service' do - expect(::Snippets::CreateService) - .to receive(:new) - .with(project: anything, current_user: anything, params: hash_including(disable_spam_action_service: true)) - .and_call_original - - subject - end - end end shared_examples 'creates snippet' do @@ -121,15 +105,6 @@ RSpec.describe 'Creating a Snippet' do it_behaves_like 'snippet edit usage data counters' it_behaves_like 'a mutation which can mutate a spammable' do - let(:captcha_response) { 'abc123' } - let(:spam_log_id) { 1234 } - let(:spam_mutation_vars) do - { - captcha_response: captcha_response, - spam_log_id: spam_log_id - } - end - let(:service) { Snippets::CreateService } end end @@ -190,7 +165,7 @@ RSpec.describe 'Creating a Snippet' do it do expect(::Snippets::CreateService).to receive(:new) - .with(project: nil, current_user: user, params: hash_including(files: expected_value)) + .with(project: nil, current_user: user, params: hash_including(files: expected_value), spam_params: instance_of(::Spam::SpamParams)) .and_return(double(execute: creation_response)) subject diff --git a/spec/requests/api/graphql/mutations/snippets/update_spec.rb b/spec/requests/api/graphql/mutations/snippets/update_spec.rb index 77efb786dcb..a3a6ce65e9f 100644 --- a/spec/requests/api/graphql/mutations/snippets/update_spec.rb +++ b/spec/requests/api/graphql/mutations/snippets/update_spec.rb @@ -16,7 +16,6 @@ RSpec.describe 'Updating a Snippet' do let(:updated_file) { 'CHANGELOG' } let(:deleted_file) { 'README' } let(:snippet_gid) { GitlabSchema.id_from_object(snippet).to_s } - let(:spam_mutation_vars) { {} } let(:mutation_vars) do { id: snippet_gid, @@ -27,7 +26,7 @@ RSpec.describe 'Updating a Snippet' do { action: :update, filePath: updated_file, content: updated_content }, { action: :delete, filePath: deleted_file } ] - }.merge(spam_mutation_vars) + } end let(:mutation) do @@ -82,21 +81,6 @@ RSpec.describe 'Updating a Snippet' do end end - context 'when snippet_spam flag is disabled' do - before do - stub_feature_flags(snippet_spam: false) - end - - it 'passes disable_spam_action_service param to service' do - expect(::Snippets::UpdateService) - .to receive(:new) - .with(project: anything, current_user: anything, params: hash_including(disable_spam_action_service: true)) - .and_call_original - - subject - end - end - context 'when there are ActiveRecord validation errors' do let(:updated_title) { '' } @@ -125,15 +109,6 @@ RSpec.describe 'Updating a Snippet' do end it_behaves_like 'a mutation which can mutate a spammable' do - let(:captcha_response) { 'abc123' } - let(:spam_log_id) { 1234 } - let(:spam_mutation_vars) do - { - captcha_response: captcha_response, - spam_log_id: spam_log_id - } - end - let(:service) { Snippets::UpdateService } end diff --git a/spec/routing/project_routing_spec.rb b/spec/routing/project_routing_spec.rb index fe04a1d7c4a..7b194ef8122 100644 --- a/spec/routing/project_routing_spec.rb +++ b/spec/routing/project_routing_spec.rb @@ -770,13 +770,23 @@ RSpec.describe 'project routing' do end end - describe Projects::UsagePingController, 'routing' do - it 'routes to usage_ping#web_ide_clientside_preview' do - expect(post('/gitlab/gitlabhq/usage_ping/web_ide_clientside_preview')).to route_to('projects/usage_ping#web_ide_clientside_preview', namespace_id: 'gitlab', project_id: 'gitlabhq') + describe Projects::ServicePingController, 'routing' do + describe 'deprecated routing' do + it 'routes to service_ping#web_ide_clientside_preview' do + expect(post('/gitlab/gitlabhq/usage_ping/web_ide_clientside_preview')).to route_to('projects/service_ping#web_ide_clientside_preview', namespace_id: 'gitlab', project_id: 'gitlabhq') + end + + it 'routes to service_ping#web_ide_pipelines_count' do + expect(post('/gitlab/gitlabhq/usage_ping/web_ide_pipelines_count')).to route_to('projects/service_ping#web_ide_pipelines_count', namespace_id: 'gitlab', project_id: 'gitlabhq') + end + end + + it 'routes to service_ping#web_ide_clientside_preview' do + expect(post('/gitlab/gitlabhq/service_ping/web_ide_clientside_preview')).to route_to('projects/service_ping#web_ide_clientside_preview', namespace_id: 'gitlab', project_id: 'gitlabhq') end - it 'routes to usage_ping#web_ide_pipelines_count' do - expect(post('/gitlab/gitlabhq/usage_ping/web_ide_pipelines_count')).to route_to('projects/usage_ping#web_ide_pipelines_count', namespace_id: 'gitlab', project_id: 'gitlabhq') + it 'routes to service_ping#web_ide_pipelines_count' do + expect(post('/gitlab/gitlabhq/service_ping/web_ide_pipelines_count')).to route_to('projects/service_ping#web_ide_pipelines_count', namespace_id: 'gitlab', project_id: 'gitlabhq') end end diff --git a/spec/services/captcha/captcha_verification_service_spec.rb b/spec/services/captcha/captcha_verification_service_spec.rb index 245e06703f5..fe2199fb53e 100644 --- a/spec/services/captcha/captcha_verification_service_spec.rb +++ b/spec/services/captcha/captcha_verification_service_spec.rb @@ -4,21 +4,31 @@ require 'spec_helper' RSpec.describe Captcha::CaptchaVerificationService do describe '#execute' do - let(:captcha_response) { nil } - let(:request) { double(:request) } - let(:service) { described_class.new } + let(:captcha_response) { 'abc123' } + let(:fake_ip) { '1.2.3.4' } + let(:spam_params) do + ::Spam::SpamParams.new( + captcha_response: captcha_response, + spam_log_id: double, + ip_address: fake_ip, + user_agent: double, + referer: double + ) + end + + let(:service) { described_class.new(spam_params: spam_params) } - subject { service.execute(captcha_response: captcha_response, request: request) } + subject { service.execute } context 'when there is no captcha_response' do + let(:captcha_response) { nil } + it 'returns false' do expect(subject).to eq(false) end end context 'when there is a captcha_response' do - let(:captcha_response) { 'abc123' } - before do expect(Gitlab::Recaptcha).to receive(:load_configurations!) end @@ -29,10 +39,12 @@ RSpec.describe Captcha::CaptchaVerificationService do expect(subject).to eq(true) end - it 'has a request method which returns the request' do + it 'has a request method which returns an object with the ip address #remote_ip' do subject - expect(service.send(:request)).to eq(request) + request_struct = service.send(:request) + expect(request_struct).to respond_to(:remote_ip) + expect(request_struct.remote_ip).to eq(fake_ip) end end end diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb index 94810d6134a..8ace5f08988 100644 --- a/spec/services/issues/create_service_spec.rb +++ b/spec/services/issues/create_service_spec.rb @@ -8,11 +8,17 @@ RSpec.describe Issues::CreateService do let_it_be_with_reload(:project) { create(:project) } let_it_be(:user) { create(:user) } + let(:spam_params) { double } + describe '#execute' do let_it_be(:assignee) { create(:user) } let_it_be(:milestone) { create(:milestone, project: project) } - let(:issue) { described_class.new(project: project, current_user: user, params: opts).execute } + let(:issue) { described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute } + + before do + stub_spam_services + end context 'when params are valid' do let_it_be(:labels) { create_pair(:label, project: project) } @@ -44,7 +50,7 @@ RSpec.describe Issues::CreateService do end context 'when skip_system_notes is true' do - let(:issue) { described_class.new(project: project, current_user: user, params: opts).execute(skip_system_notes: true) } + let(:issue) { described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute(skip_system_notes: true) } it 'does not call Issuable::CommonSystemNotesService' do expect(Issuable::CommonSystemNotesService).not_to receive(:new) @@ -92,7 +98,7 @@ RSpec.describe Issues::CreateService do let_it_be(:non_member) { create(:user) } it 'filters out params that cannot be set without the :set_issue_metadata permission' do - issue = described_class.new(project: project, current_user: non_member, params: opts).execute + issue = described_class.new(project: project, current_user: non_member, params: opts, spam_params: spam_params).execute expect(issue).to be_persisted expect(issue.title).to eq('Awesome issue') @@ -104,7 +110,7 @@ RSpec.describe Issues::CreateService do end it 'can create confidential issues' do - issue = described_class.new(project: project, current_user: non_member, params: { confidential: true }).execute + issue = described_class.new(project: project, current_user: non_member, params: { confidential: true }, spam_params: spam_params).execute expect(issue.confidential).to be_truthy end @@ -113,7 +119,7 @@ RSpec.describe Issues::CreateService do it 'moves the issue to the end, in an asynchronous worker' do expect(IssuePlacementWorker).to receive(:perform_async).with(be_nil, Integer) - described_class.new(project: project, current_user: user, params: opts).execute + described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute end context 'when label belongs to project group' do @@ -200,7 +206,7 @@ RSpec.describe Issues::CreateService do it 'invalidates open issues counter for assignees when issue is assigned' do project.add_maintainer(assignee) - described_class.new(project: project, current_user: user, params: opts).execute + described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute expect(assignee.assigned_open_issues_count).to eq 1 end @@ -226,7 +232,7 @@ RSpec.describe Issues::CreateService do expect(project).to receive(:execute_hooks).with(an_instance_of(Hash), :issue_hooks) expect(project).to receive(:execute_services).with(an_instance_of(Hash), :issue_hooks) - described_class.new(project: project, current_user: user, params: opts).execute + described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute end it 'executes confidential issue hooks when issue is confidential' do @@ -235,7 +241,7 @@ RSpec.describe Issues::CreateService do expect(project).to receive(:execute_hooks).with(an_instance_of(Hash), :confidential_issue_hooks) expect(project).to receive(:execute_services).with(an_instance_of(Hash), :confidential_issue_hooks) - described_class.new(project: project, current_user: user, params: opts).execute + described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute end context 'after_save callback to store_mentions' do @@ -279,7 +285,7 @@ RSpec.describe Issues::CreateService do it 'removes assignee when user id is invalid' do opts = { title: 'Title', description: 'Description', assignee_ids: [-1] } - issue = described_class.new(project: project, current_user: user, params: opts).execute + issue = described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute expect(issue.assignees).to be_empty end @@ -287,7 +293,7 @@ RSpec.describe Issues::CreateService do it 'removes assignee when user id is 0' do opts = { title: 'Title', description: 'Description', assignee_ids: [0] } - issue = described_class.new(project: project, current_user: user, params: opts).execute + issue = described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute expect(issue.assignees).to be_empty end @@ -296,7 +302,7 @@ RSpec.describe Issues::CreateService do project.add_maintainer(assignee) opts = { title: 'Title', description: 'Description', assignee_ids: [assignee.id] } - issue = described_class.new(project: project, current_user: user, params: opts).execute + issue = described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute expect(issue.assignees).to eq([assignee]) end @@ -314,7 +320,7 @@ RSpec.describe Issues::CreateService do project.update!(visibility_level: level) opts = { title: 'Title', description: 'Description', assignee_ids: [assignee.id] } - issue = described_class.new(project: project, current_user: user, params: opts).execute + issue = described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute expect(issue.assignees).to be_empty end @@ -324,7 +330,7 @@ RSpec.describe Issues::CreateService do end it_behaves_like 'issuable record that supports quick actions' do - let(:issuable) { described_class.new(project: project, current_user: user, params: params).execute } + let(:issuable) { described_class.new(project: project, current_user: user, params: params, spam_params: spam_params).execute } end context 'Quick actions' do @@ -364,14 +370,14 @@ RSpec.describe Issues::CreateService do let(:opts) { { discussion_to_resolve: discussion.id, merge_request_to_resolve_discussions_of: merge_request.iid } } it 'resolves the discussion' do - described_class.new(project: project, current_user: user, params: opts).execute + described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute discussion.first_note.reload expect(discussion.resolved?).to be(true) end it 'added a system note to the discussion' do - described_class.new(project: project, current_user: user, params: opts).execute + described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute reloaded_discussion = MergeRequest.find(merge_request.id).discussions.first @@ -379,7 +385,7 @@ RSpec.describe Issues::CreateService do end it 'assigns the title and description for the issue' do - issue = described_class.new(project: project, current_user: user, params: opts).execute + issue = described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute expect(issue.title).not_to be_nil expect(issue.description).not_to be_nil @@ -391,7 +397,8 @@ RSpec.describe Issues::CreateService do merge_request_to_resolve_discussions_of: merge_request, description: nil, title: nil - }).execute + }, + spam_params: spam_params).execute expect(issue.description).to be_nil expect(issue.title).to be_nil @@ -402,14 +409,14 @@ RSpec.describe Issues::CreateService do let(:opts) { { merge_request_to_resolve_discussions_of: merge_request.iid } } it 'resolves the discussion' do - described_class.new(project: project, current_user: user, params: opts).execute + described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute discussion.first_note.reload expect(discussion.resolved?).to be(true) end it 'added a system note to the discussion' do - described_class.new(project: project, current_user: user, params: opts).execute + described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute reloaded_discussion = MergeRequest.find(merge_request.id).discussions.first @@ -417,7 +424,7 @@ RSpec.describe Issues::CreateService do end it 'assigns the title and description for the issue' do - issue = described_class.new(project: project, current_user: user, params: opts).execute + issue = described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute expect(issue.title).not_to be_nil expect(issue.description).not_to be_nil @@ -429,7 +436,8 @@ RSpec.describe Issues::CreateService do merge_request_to_resolve_discussions_of: merge_request, description: nil, title: nil - }).execute + }, + spam_params: spam_params).execute expect(issue.description).to be_nil expect(issue.title).to be_nil @@ -438,47 +446,27 @@ RSpec.describe Issues::CreateService do end context 'checking spam' do - let(:request) { double(:request, headers: nil) } - let(:api) { true } - let(:captcha_response) { 'abc123' } - let(:spam_log_id) { 1 } - let(:params) do { - title: 'Spam issue', - request: request, - api: api, - captcha_response: captcha_response, - spam_log_id: spam_log_id + title: 'Spam issue' } end subject do - described_class.new(project: project, current_user: user, params: params) - end - - before do - allow_next_instance_of(UserAgentDetailService) do |instance| - allow(instance).to receive(:create) - end + described_class.new(project: project, current_user: user, params: params, spam_params: spam_params) end it 'executes SpamActionService' do - spam_params = Spam::SpamParams.new( - api: api, - captcha_response: captcha_response, - spam_log_id: spam_log_id - ) expect_next_instance_of( Spam::SpamActionService, { - spammable: an_instance_of(Issue), - request: request, - user: user, + spammable: kind_of(Issue), + spam_params: spam_params, + user: an_instance_of(User), action: :create } ) do |instance| - expect(instance).to receive(:execute).with(spam_params: spam_params) + expect(instance).to receive(:execute) end subject.execute diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index c3a0766cb17..63ef007350d 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -376,41 +376,31 @@ RSpec.describe NotificationService, :mailer do let(:subject) { NotificationService.new } let(:mailer) { double(deliver_later: true) } + let(:issue) { create(:issue, author: User.support_bot) } + let(:project) { issue.project } + let(:note) { create(:note, noteable: issue, project: project) } - def should_email! - expect(Notify).to receive(:service_desk_new_note_email) - .with(issue.id, note.id, issue.external_author) - end + shared_examples 'notification with exact metric events' do |number_of_events| + it 'adds metric event' do + metric_transaction = double('Gitlab::Metrics::WebTransaction', increment: true, observe: true) + allow(::Gitlab::Metrics::BackgroundTransaction).to receive(:current).and_return(metric_transaction) + expect(metric_transaction).to receive(:add_event).with(:service_desk_new_note_email).exactly(number_of_events).times - def should_not_email! - expect(Notify).not_to receive(:service_desk_new_note_email) + subject.new_note(note) + end end - def execute! - subject.new_note(note) - end + shared_examples 'no participants are notified' do + it 'does not send the email' do + expect(Notify).not_to receive(:service_desk_new_note_email) - def self.it_should_email! - it 'sends the email' do - should_email! - execute! + subject.new_note(note) end - end - def self.it_should_not_email! - it 'doesn\'t send the email' do - should_not_email! - execute! - end + it_behaves_like 'notification with exact metric events', 0 end - let(:issue) { create(:issue, author: User.support_bot) } - let(:project) { issue.project } - let(:note) { create(:note, noteable: issue, project: project) } - - context 'do not exist' do - it_should_not_email! - end + it_behaves_like 'no participants are notified' context 'do exist and note not confidential' do let!(:issue_email_participant) { issue.issue_email_participants.create!(email: 'service.desk@example.com') } @@ -420,7 +410,14 @@ RSpec.describe NotificationService, :mailer do project.update!(service_desk_enabled: true) end - it_should_email! + it 'sends the email' do + expect(Notify).to receive(:service_desk_new_note_email) + .with(issue.id, note.id, issue.external_author) + + subject.new_note(note) + end + + it_behaves_like 'notification with exact metric events', 1 end context 'do exist and note is confidential' do @@ -432,7 +429,7 @@ RSpec.describe NotificationService, :mailer do project.update!(service_desk_enabled: true) end - it_should_not_email! + it_behaves_like 'no participants are notified' end end diff --git a/spec/services/snippets/create_service_spec.rb b/spec/services/snippets/create_service_spec.rb index eb6e85eb408..0b73808433f 100644 --- a/spec/services/snippets/create_service_spec.rb +++ b/spec/services/snippets/create_service_spec.rb @@ -19,8 +19,9 @@ RSpec.describe Snippets::CreateService do let(:extra_opts) { {} } let(:creator) { admin } + let(:spam_params) { double } - subject { described_class.new(project: project, current_user: creator, params: opts).execute } + subject { described_class.new(project: project, current_user: creator, params: opts, spam_params: spam_params).execute } let(:snippet) { subject.payload[:snippet] } @@ -301,6 +302,10 @@ RSpec.describe Snippets::CreateService do end end + before do + stub_spam_services + end + context 'when ProjectSnippet' do let_it_be(:project) { create(:project) } diff --git a/spec/services/snippets/update_service_spec.rb b/spec/services/snippets/update_service_spec.rb index 46bc62e11ef..7fac500f224 100644 --- a/spec/services/snippets/update_service_spec.rb +++ b/spec/services/snippets/update_service_spec.rb @@ -20,7 +20,9 @@ RSpec.describe Snippets::UpdateService do let(:extra_opts) { {} } let(:options) { base_opts.merge(extra_opts) } let(:updater) { user } - let(:service) { Snippets::UpdateService.new(project: project, current_user: updater, params: options) } + let(:spam_params) { double } + + let(:service) { Snippets::UpdateService.new(project: project, current_user: updater, params: options, spam_params: spam_params) } subject { service.execute(snippet) } @@ -721,6 +723,10 @@ RSpec.describe Snippets::UpdateService do end end + before do + stub_spam_services + end + context 'when Project Snippet' do let_it_be(:project) { create(:project) } let!(:snippet) { create(:project_snippet, :repository, author: user, project: project) } diff --git a/spec/services/spam/akismet_service_spec.rb b/spec/services/spam/akismet_service_spec.rb index 1cd049da592..d9f62258a53 100644 --- a/spec/services/spam/akismet_service_spec.rb +++ b/spec/services/spam/akismet_service_spec.rb @@ -59,7 +59,7 @@ RSpec.describe Spam::AkismetService do it_behaves_like 'no activity if Akismet is not enabled', :spam?, :check context 'if Akismet is enabled' do - it 'correctly transforms options for the akismet client' do + it 'correctly transforms options for the akismet client, including spelling of referrer key' do expected_check_params = { type: 'comment', text: text, diff --git a/spec/services/spam/spam_action_service_spec.rb b/spec/services/spam/spam_action_service_spec.rb index 9ca52b92267..3a92e5acb5a 100644 --- a/spec/services/spam/spam_action_service_spec.rb +++ b/spec/services/spam/spam_action_service_spec.rb @@ -5,15 +5,20 @@ require 'spec_helper' RSpec.describe Spam::SpamActionService do include_context 'includes Spam constants' - let(:request) { double(:request, env: env, headers: {}) } let(:issue) { create(:issue, project: project, author: user) } let(:fake_ip) { '1.2.3.4' } let(:fake_user_agent) { 'fake-user-agent' } let(:fake_referer) { 'fake-http-referer' } - let(:env) do - { 'action_dispatch.remote_ip' => fake_ip, - 'HTTP_USER_AGENT' => fake_user_agent, - 'HTTP_REFERER' => fake_referer } + let(:captcha_response) { 'abc123' } + let(:spam_log_id) { existing_spam_log.id } + let(:spam_params) do + ::Spam::SpamParams.new( + captcha_response: captcha_response, + spam_log_id: spam_log_id, + ip_address: fake_ip, + user_agent: fake_user_agent, + referer: fake_referer + ) end let_it_be(:project) { create(:project, :public) } @@ -23,32 +28,33 @@ RSpec.describe Spam::SpamActionService do issue.spam = false end - shared_examples 'only checks for spam if a request is provided' do - context 'when request is missing' do - let(:request) { nil } + describe 'constructor argument validation' do + subject do + described_service = described_class.new(spammable: issue, spam_params: spam_params, user: user, action: :create) + described_service.execute + end - it "doesn't check as spam" do - expect(fake_verdict_service).not_to receive(:execute) + context 'when spam_params is nil' do + let(:spam_params) { nil } + let(:expected_service_params_not_present_message) do + /Skipped spam check because spam_params was not present/ + end + it "returns success with a messaage" do response = subject - expect(response.message).to match(/request was not present/) + expect(response.message).to match(expected_service_params_not_present_message) expect(issue).not_to be_spam end end - - context 'when request exists' do - it 'creates a spam log' do - expect { subject } - .to log_spam(title: issue.title, description: issue.description, noteable_type: 'Issue') - end - end end shared_examples 'creates a spam log' do it do - expect { subject }.to change(SpamLog, :count).by(1) + expect { subject } + .to log_spam(title: issue.title, description: issue.description, noteable_type: 'Issue') + # TODO: These checks should be incorporated into the `log_spam` RSpec matcher above new_spam_log = SpamLog.last expect(new_spam_log.user_id).to eq(user.id) expect(new_spam_log.title).to eq(issue.title) @@ -56,25 +62,14 @@ RSpec.describe Spam::SpamActionService do expect(new_spam_log.source_ip).to eq(fake_ip) expect(new_spam_log.user_agent).to eq(fake_user_agent) expect(new_spam_log.noteable_type).to eq('Issue') - expect(new_spam_log.via_api).to eq(false) + expect(new_spam_log.via_api).to eq(true) end end describe '#execute' do - let(:request) { double(:request, env: env, headers: nil) } let(:fake_captcha_verification_service) { double(:captcha_verification_service) } let(:fake_verdict_service) { double(:spam_verdict_service) } let(:allowlisted) { false } - let(:api) { nil } - let(:captcha_response) { 'abc123' } - let(:spam_log_id) { existing_spam_log.id } - let(:spam_params) do - ::Spam::SpamParams.new( - api: api, - captcha_response: captcha_response, - spam_log_id: spam_log_id - ) - end let(:verdict_service_opts) do { @@ -88,7 +83,6 @@ RSpec.describe Spam::SpamActionService do { target: issue, user: user, - request: request, options: verdict_service_opts, context: { action: :create, @@ -100,40 +94,20 @@ RSpec.describe Spam::SpamActionService do let_it_be(:existing_spam_log) { create(:spam_log, user: user, recaptcha_verified: false) } subject do - described_service = described_class.new(spammable: issue, request: request, user: user, action: :create) + described_service = described_class.new(spammable: issue, spam_params: spam_params, user: user, action: :create) allow(described_service).to receive(:allowlisted?).and_return(allowlisted) - described_service.execute(spam_params: spam_params) + described_service.execute end before do - allow(Captcha::CaptchaVerificationService).to receive(:new) { fake_captcha_verification_service } + allow(Captcha::CaptchaVerificationService).to receive(:new).with(spam_params: spam_params) { fake_captcha_verification_service } allow(Spam::SpamVerdictService).to receive(:new).with(verdict_service_args).and_return(fake_verdict_service) end - context 'when the captcha params are passed in the headers' do - let(:request) { double(:request, env: env, headers: headers) } - let(:spam_params) { Spam::SpamActionService.filter_spam_params!({ api: api }, request) } - let(:headers) do - { - 'X-GitLab-Captcha-Response' => captcha_response, - 'X-GitLab-Spam-Log-Id' => spam_log_id - } - end - - it 'extracts the headers correctly' do - expect(fake_captcha_verification_service) - .to receive(:execute).with(captcha_response: captcha_response, request: request).and_return(true) - expect(SpamLog) - .to receive(:verify_recaptcha!).with(user_id: user.id, id: spam_log_id) - - subject - end - end - context 'when captcha response verification returns true' do before do allow(fake_captcha_verification_service) - .to receive(:execute).with(captcha_response: captcha_response, request: request).and_return(true) + .to receive(:execute).and_return(true) end it "doesn't check with the SpamVerdictService" do @@ -156,7 +130,7 @@ RSpec.describe Spam::SpamActionService do context 'when captcha response verification returns false' do before do allow(fake_captcha_verification_service) - .to receive(:execute).with(captcha_response: captcha_response, request: request).and_return(false) + .to receive(:execute).and_return(false) end context 'when spammable attributes have not changed' do @@ -200,8 +174,6 @@ RSpec.describe Spam::SpamActionService do stub_feature_flags(allow_possible_spam: false) end - it_behaves_like 'only checks for spam if a request is provided' - it 'marks as spam' do response = subject @@ -211,8 +183,6 @@ RSpec.describe Spam::SpamActionService do end context 'when allow_possible_spam feature flag is true' do - it_behaves_like 'only checks for spam if a request is provided' - it 'does not mark as spam' do response = subject @@ -232,8 +202,6 @@ RSpec.describe Spam::SpamActionService do stub_feature_flags(allow_possible_spam: false) end - it_behaves_like 'only checks for spam if a request is provided' - it 'marks as spam' do response = subject @@ -243,8 +211,6 @@ RSpec.describe Spam::SpamActionService do end context 'when allow_possible_spam feature flag is true' do - it_behaves_like 'only checks for spam if a request is provided' - it 'does not mark as spam' do response = subject @@ -264,8 +230,6 @@ RSpec.describe Spam::SpamActionService do stub_feature_flags(allow_possible_spam: false) end - it_behaves_like 'only checks for spam if a request is provided' - it_behaves_like 'creates a spam log' it 'does not mark as spam' do @@ -284,8 +248,6 @@ RSpec.describe Spam::SpamActionService do end context 'when allow_possible_spam feature flag is true' do - it_behaves_like 'only checks for spam if a request is provided' - it_behaves_like 'creates a spam log' it 'does not mark as needing reCAPTCHA' do @@ -334,37 +296,10 @@ RSpec.describe Spam::SpamActionService do allow(fake_verdict_service).to receive(:execute).and_return(ALLOW) end - context 'when the request is nil' do - let(:request) { nil } - let(:issue_ip_address) { '1.2.3.4' } - let(:issue_user_agent) { 'lynx' } - let(:verdict_service_opts) do - { - ip_address: issue_ip_address, - user_agent: issue_user_agent - } - end - - before do - allow(issue).to receive(:ip_address) { issue_ip_address } - allow(issue).to receive(:user_agent) { issue_user_agent } - end - - it 'assembles the options with information from the spammable' do - # TODO: This code untestable, because we do not perform a verification if there is not a - # request. See corresponding comment in code - # expect(Spam::SpamVerdictService).to receive(:new).with(verdict_service_args) - - subject - end - end - - context 'when the request is present' do - it 'assembles the options with information from the request' do - expect(Spam::SpamVerdictService).to receive(:new).with(verdict_service_args) + it 'assembles the options with information from the request' do + expect(Spam::SpamVerdictService).to receive(:new).with(verdict_service_args) - subject - end + subject end end end diff --git a/spec/services/spam/spam_params_spec.rb b/spec/services/spam/spam_params_spec.rb new file mode 100644 index 00000000000..e7e8b468adb --- /dev/null +++ b/spec/services/spam/spam_params_spec.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Spam::SpamParams do + describe '.new_from_request' do + let(:captcha_response) { 'abc123' } + let(:spam_log_id) { 42 } + let(:ip_address) { '0.0.0.0' } + let(:user_agent) { 'Lynx' } + let(:referer) { 'http://localhost' } + let(:headers) do + { + 'X-GitLab-Captcha-Response' => captcha_response, + 'X-GitLab-Spam-Log-Id' => spam_log_id + } + end + + let(:env) do + { + 'action_dispatch.remote_ip' => ip_address, + 'HTTP_USER_AGENT' => user_agent, + 'HTTP_REFERER' => referer + } + end + + let(:request) {double(:request, headers: headers, env: env)} + + it 'constructs from a request' do + expected = ::Spam::SpamParams.new( + captcha_response: captcha_response, + spam_log_id: spam_log_id, + ip_address: ip_address, + user_agent: user_agent, + referer: referer + ) + expect(described_class.new_from_request(request: request)).to eq(expected) + end + end +end diff --git a/spec/services/spam/spam_verdict_service_spec.rb b/spec/services/spam/spam_verdict_service_spec.rb index 215df81de63..9ff2e0ddf55 100644 --- a/spec/services/spam/spam_verdict_service_spec.rb +++ b/spec/services/spam/spam_verdict_service_spec.rb @@ -14,13 +14,11 @@ RSpec.describe Spam::SpamVerdictService do 'HTTP_REFERER' => fake_referer } end - let(:request) { double(:request, env: env) } - let(:check_for_spam) { true } let_it_be(:user) { create(:user) } let_it_be(:issue) { create(:issue, author: user) } let(:service) do - described_class.new(user: user, target: issue, request: request, options: {}) + described_class.new(user: user, target: issue, options: {}) end let(:attribs) do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index a71fc172ebd..e94ff5bcb45 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -190,6 +190,7 @@ RSpec.configure do |config| config.include RailsHelpers config.include SidekiqMiddleware config.include StubActionCableConnection, type: :channel + config.include StubSpamServices include StubFeatureFlags diff --git a/spec/support/helpers/stub_spam_services.rb b/spec/support/helpers/stub_spam_services.rb new file mode 100644 index 00000000000..841e8366845 --- /dev/null +++ b/spec/support/helpers/stub_spam_services.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +module StubSpamServices + def stub_spam_services + allow(::Spam::SpamParams).to receive(:new_from_request) do + ::Spam::SpamParams.new( + captcha_response: double(:captcha_response), + spam_log_id: double(:spam_log_id), + ip_address: double(:ip_address), + user_agent: double(:user_agent), + referer: double(:referer) + ) + end + + allow_next_instance_of(::Spam::SpamActionService) do |service| + allow(service).to receive(:execute) + end + + allow_next_instance_of(::UserAgentDetailService) do |service| + allow(service).to receive(:create) + end + end +end diff --git a/spec/support/shared_examples/graphql/mutations/can_mutate_spammable_examples.rb b/spec/support/shared_examples/graphql/mutations/can_mutate_spammable_examples.rb index 5e15c91cd41..011a2157f24 100644 --- a/spec/support/shared_examples/graphql/mutations/can_mutate_spammable_examples.rb +++ b/spec/support/shared_examples/graphql/mutations/can_mutate_spammable_examples.rb @@ -3,17 +3,13 @@ require 'spec_helper' RSpec.shared_examples 'a mutation which can mutate a spammable' do - describe "#additional_spam_params" do - it 'passes additional spam params to the service' do + describe "#spam_params" do + it 'passes spam params to the service constructor' do args = [ project: anything, current_user: anything, - params: hash_including( - api: true, - request: instance_of(ActionDispatch::Request), - captcha_response: captcha_response, - spam_log_id: spam_log_id - ) + params: anything, + spam_params: instance_of(::Spam::SpamParams) ] expect(service).to receive(:new).with(*args).and_call_original diff --git a/spec/support/shared_examples/graphql/spam_protection_shared_examples.rb b/spec/support/shared_examples/graphql/spam_protection_shared_examples.rb index 8fb89a4f80e..c0b71a494d0 100644 --- a/spec/support/shared_examples/graphql/spam_protection_shared_examples.rb +++ b/spec/support/shared_examples/graphql/spam_protection_shared_examples.rb @@ -57,7 +57,7 @@ RSpec.shared_examples 'has spam protection' do context 'and no CAPTCHA is required' do let(:render_captcha) { false } - it 'does not return a to-level error' do + it 'does not return a top-level error' do send_request expect(graphql_errors).to be_blank diff --git a/spec/support/shared_examples/models/chat_integration_shared_examples.rb b/spec/support/shared_examples/models/chat_integration_shared_examples.rb index 9f3be3e2e06..b5d73350124 100644 --- a/spec/support/shared_examples/models/chat_integration_shared_examples.rb +++ b/spec/support/shared_examples/models/chat_integration_shared_examples.rb @@ -163,7 +163,7 @@ RSpec.shared_examples "chat integration" do |integration_name| context "with issue events" do let(:opts) { { title: "Awesome issue", description: "please fix" } } let(:sample_data) do - service = Issues::CreateService.new(project: project, current_user: user, params: opts) + service = Issues::CreateService.new(project: project, current_user: user, params: opts, spam_params: nil) issue = service.execute service.hook_data(issue, "open") end diff --git a/spec/support/shared_examples/services/snippets_shared_examples.rb b/spec/support/shared_examples/services/snippets_shared_examples.rb index 0c4db7ded69..56b2ce3353d 100644 --- a/spec/support/shared_examples/services/snippets_shared_examples.rb +++ b/spec/support/shared_examples/services/snippets_shared_examples.rb @@ -1,23 +1,6 @@ # frozen_string_literal: true RSpec.shared_examples 'checking spam' do - let(:request) { double(:request, headers: headers) } - let(:headers) { nil } - let(:api) { true } - let(:captcha_response) { 'abc123' } - let(:spam_log_id) { 1 } - let(:disable_spam_action_service) { false } - - let(:extra_opts) do - { - request: request, - api: api, - captcha_response: captcha_response, - spam_log_id: spam_log_id, - disable_spam_action_service: disable_spam_action_service - } - end - before do allow_next_instance_of(UserAgentDetailService) do |instance| allow(instance).to receive(:create) @@ -25,67 +8,26 @@ RSpec.shared_examples 'checking spam' do end it 'executes SpamActionService' do - spam_params = Spam::SpamParams.new( - api: api, - captcha_response: captcha_response, - spam_log_id: spam_log_id - ) expect_next_instance_of( Spam::SpamActionService, { spammable: kind_of(Snippet), - request: request, + spam_params: spam_params, user: an_instance_of(User), action: action } ) do |instance| - expect(instance).to receive(:execute).with(spam_params: spam_params) + expect(instance).to receive(:execute) end subject end - context 'when CAPTCHA arguments are passed in the headers' do - let(:headers) do - { - 'X-GitLab-Spam-Log-Id' => spam_log_id, - 'X-GitLab-Captcha-Response' => captcha_response - } + context 'when snippet_spam flag is disabled' do + before do + stub_feature_flags(snippet_spam: false) end - let(:extra_opts) do - { - request: request, - api: api, - disable_spam_action_service: disable_spam_action_service - } - end - - it 'executes the SpamActionService correctly' do - spam_params = Spam::SpamParams.new( - api: api, - captcha_response: captcha_response, - spam_log_id: spam_log_id - ) - expect_next_instance_of( - Spam::SpamActionService, - { - spammable: kind_of(Snippet), - request: request, - user: an_instance_of(User), - action: action - } - ) do |instance| - expect(instance).to receive(:execute).with(spam_params: spam_params) - end - - subject - end - end - - context 'when spam action service is disabled' do - let(:disable_spam_action_service) { true } - it 'request parameter is not passed to the service' do expect(Spam::SpamActionService).not_to receive(:new) |