From 1502c20d04c7ff8d719175c76b0a2507ab390172 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Thu, 3 Dec 2020 00:09:53 +0000 Subject: Add latest changes from gitlab-org/gitlab@master --- .../admin/integrations_controller_spec.rb | 16 +++ spec/controllers/admin/users_controller_spec.rb | 51 ++++++++ spec/controllers/groups_controller_spec.rb | 4 +- spec/controllers/invites_controller_spec.rb | 2 +- spec/controllers/projects/jobs_controller_spec.rb | 2 +- .../experience_levels_controller_spec.rb | 10 +- spec/controllers/root_controller_spec.rb | 2 +- spec/factories/merge_requests.rb | 2 +- spec/features/admin/users/user_spec.rb | 4 +- spec/features/projects/jobs_spec.rb | 2 +- .../registrations/experience_level_spec.rb | 2 +- .../integrations/edit/store/actions_spec.js | 30 +++++ .../integrations/edit/store/mutations_spec.js | 16 +++ spec/frontend/pipelines/unwrapping_utils_spec.js | 127 +++++++++++++++++++ .../types/countable_connection_type_spec.rb | 2 +- .../types/merge_request_connection_type_spec.rb | 11 ++ spec/helpers/services_helper_spec.rb | 53 ++++++++ .../experimentation/controller_concern_spec.rb | 140 +++++++++++++++------ spec/lib/gitlab/experimentation/experiment_spec.rb | 8 +- spec/lib/gitlab/experimentation_spec.rb | 139 ++++++++++++-------- spec/mailers/notify_spec.rb | 2 +- ...smissal_information_for_vulnerabilities_spec.rb | 31 +++++ spec/models/merge_request_spec.rb | 71 +++++++++++ spec/policies/global_policy_spec.rb | 18 +++ .../invitation_reminder_email_service_spec.rb | 4 +- spec/services/notification_service_spec.rb | 14 +++ spec/services/users/reject_service_spec.rb | 54 ++++++++ spec/support/helpers/stub_experiments.rb | 16 +-- .../issuable_invite_members_shared_examples.rb | 4 +- 29 files changed, 715 insertions(+), 122 deletions(-) create mode 100644 spec/frontend/pipelines/unwrapping_utils_spec.js create mode 100644 spec/graphql/types/merge_request_connection_type_spec.rb create mode 100644 spec/migrations/populate_remaining_missing_dismissal_information_for_vulnerabilities_spec.rb create mode 100644 spec/services/users/reject_service_spec.rb (limited to 'spec') diff --git a/spec/controllers/admin/integrations_controller_spec.rb b/spec/controllers/admin/integrations_controller_spec.rb index 1a13d016b73..1b4c48d93c7 100644 --- a/spec/controllers/admin/integrations_controller_spec.rb +++ b/spec/controllers/admin/integrations_controller_spec.rb @@ -73,4 +73,20 @@ RSpec.describe Admin::IntegrationsController do end end end + + describe '#reset' do + let(:integration) { create(:jira_service, :instance) } + + before do + post :reset, params: { id: integration.class.to_param } + end + + it 'returns 200 OK' do + expected_json = {}.to_json + + expect(flash[:notice]).to eq('This integration, and inheriting projects were reset.') + expect(response).to have_gitlab_http_status(:ok) + expect(response.body).to eq(expected_json) + end + end end diff --git a/spec/controllers/admin/users_controller_spec.rb b/spec/controllers/admin/users_controller_spec.rb index d0d1fa6a6bc..f902a3d2541 100644 --- a/spec/controllers/admin/users_controller_spec.rb +++ b/spec/controllers/admin/users_controller_spec.rb @@ -102,6 +102,57 @@ RSpec.describe Admin::UsersController do end end + describe 'DELETE #reject' do + subject { put :reject, params: { id: user.username } } + + context 'when rejecting a pending user' do + let(:user) { create(:user, :blocked_pending_approval) } + + it 'hard deletes the user', :sidekiq_inline do + subject + + expect(User.exists?(user.id)).to be_falsy + end + + it 'displays the rejection message' do + subject + + expect(response).to redirect_to(admin_users_path) + expect(flash[:notice]).to eq("You've rejected #{user.name}") + end + + it 'sends the user a rejection email' do + expect_next_instance_of(NotificationService) do |notification| + allow(notification).to receive(:user_admin_rejection).with(user.name, user.notification_email) + end + + subject + end + end + + context 'when user is not pending' do + let(:user) { create(:user, state: 'active') } + + it 'does not reject and delete the user' do + subject + + expect(User.exists?(user.id)).to be_truthy + end + + it 'displays the error' do + subject + + expect(flash[:alert]).to eq('This user does not have a pending request') + end + + it 'does not email the user' do + expect(NotificationService).not_to receive(:new) + + subject + end + end + end + describe 'PUT #approve' do let(:user) { create(:user, :blocked_pending_approval) } diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb index 90b38799629..939c36a98b2 100644 --- a/spec/controllers/groups_controller_spec.rb +++ b/spec/controllers/groups_controller_spec.rb @@ -333,7 +333,7 @@ RSpec.describe GroupsController, factory_default: :keep do context 'and the user is part of the control group' do before do - stub_experiment_for_user(onboarding_issues: false) + stub_experiment_for_subject(onboarding_issues: false) end it 'tracks the event with the "created_namespace" action with the "control_group" property', :snowplow do @@ -350,7 +350,7 @@ RSpec.describe GroupsController, factory_default: :keep do context 'and the user is part of the experimental group' do before do - stub_experiment_for_user(onboarding_issues: true) + stub_experiment_for_subject(onboarding_issues: true) end it 'tracks the event with the "created_namespace" action with the "experimental_group" property', :snowplow do diff --git a/spec/controllers/invites_controller_spec.rb b/spec/controllers/invites_controller_spec.rb index 2d13b942c31..8aaed97ad04 100644 --- a/spec/controllers/invites_controller_spec.rb +++ b/spec/controllers/invites_controller_spec.rb @@ -25,7 +25,7 @@ RSpec.describe InvitesController, :snowplow do shared_examples "tracks the 'accepted' event for the invitation reminders experiment" do before do stub_experiment(invitation_reminders: true) - allow(Gitlab::Experimentation).to receive(:enabled_for_attribute?).with(:invitation_reminders, member.invite_email).and_return(experimental_group) + stub_experiment_for_subject(invitation_reminders: experimental_group) end context 'when in the control group' do diff --git a/spec/controllers/projects/jobs_controller_spec.rb b/spec/controllers/projects/jobs_controller_spec.rb index f9ad3a33fd1..bc6d2ec2ed1 100644 --- a/spec/controllers/projects/jobs_controller_spec.rb +++ b/spec/controllers/projects/jobs_controller_spec.rb @@ -18,7 +18,7 @@ RSpec.describe Projects::JobsController, :clean_gitlab_redis_shared_state do describe 'pushing tracking_data to Gon' do before do stub_experiment(jobs_empty_state: experiment_active) - stub_experiment_for_user(jobs_empty_state: in_experiment_group) + stub_experiment_for_subject(jobs_empty_state: in_experiment_group) get_index end diff --git a/spec/controllers/registrations/experience_levels_controller_spec.rb b/spec/controllers/registrations/experience_levels_controller_spec.rb index 4be67f29107..015daba8682 100644 --- a/spec/controllers/registrations/experience_levels_controller_spec.rb +++ b/spec/controllers/registrations/experience_levels_controller_spec.rb @@ -19,7 +19,7 @@ RSpec.describe Registrations::ExperienceLevelsController do context 'with an authenticated user' do before do sign_in(user) - stub_experiment_for_user(onboarding_issues: true) + stub_experiment_for_subject(onboarding_issues: true) end it { is_expected.to have_gitlab_http_status(:ok) } @@ -28,7 +28,7 @@ RSpec.describe Registrations::ExperienceLevelsController do context 'when not part of the onboarding issues experiment' do before do - stub_experiment_for_user(onboarding_issues: false) + stub_experiment_for_subject(onboarding_issues: false) end it { is_expected.to have_gitlab_http_status(:not_found) } @@ -47,12 +47,12 @@ RSpec.describe Registrations::ExperienceLevelsController do context 'with an authenticated user' do before do sign_in(user) - stub_experiment_for_user(onboarding_issues: true) + stub_experiment_for_subject(onboarding_issues: true) end context 'when not part of the onboarding issues experiment' do before do - stub_experiment_for_user(onboarding_issues: false) + stub_experiment_for_subject(onboarding_issues: false) end it { is_expected.to have_gitlab_http_status(:not_found) } @@ -90,7 +90,7 @@ RSpec.describe Registrations::ExperienceLevelsController do let(:issues_board) { build(:board, id: 123, project: project) } before do - stub_experiment_for_user( + stub_experiment_for_subject( onboarding_issues: true, default_to_issues_board: default_to_issues_board_xp? ) diff --git a/spec/controllers/root_controller_spec.rb b/spec/controllers/root_controller_spec.rb index 1db99a09404..85f9ea66c5f 100644 --- a/spec/controllers/root_controller_spec.rb +++ b/spec/controllers/root_controller_spec.rb @@ -125,7 +125,7 @@ RSpec.describe RootController do context 'when experiment is enabled' do before do - stub_experiment_for_user(customize_homepage: true) + stub_experiment_for_subject(customize_homepage: true) end it 'renders the default dashboard' do diff --git a/spec/factories/merge_requests.rb b/spec/factories/merge_requests.rb index 998d93420d1..8be7607208b 100644 --- a/spec/factories/merge_requests.rb +++ b/spec/factories/merge_requests.rb @@ -52,7 +52,7 @@ FactoryBot.define do after(:build) do |merge_request, evaluator| metrics = merge_request.build_metrics - metrics.merged_at = 1.week.ago + metrics.merged_at = 1.week.from_now metrics.merged_by = evaluator.merged_by metrics.pipeline = create(:ci_empty_pipeline) end diff --git a/spec/features/admin/users/user_spec.rb b/spec/features/admin/users/user_spec.rb index cbf5c8db169..46380218e91 100644 --- a/spec/features/admin/users/user_spec.rb +++ b/spec/features/admin/users/user_spec.rb @@ -37,9 +37,7 @@ RSpec.describe 'Admin::Users::User' do expect(page).to have_content(user.name) expect(page).to have_content('Pending approval') expect(page).to have_link('Approve user') - expect(page).to have_button('Block user') - expect(page).to have_button('Delete user') - expect(page).to have_button('Delete user and contributions') + expect(page).to have_link('Reject request') end end diff --git a/spec/features/projects/jobs_spec.rb b/spec/features/projects/jobs_spec.rb index 9b6d7df3709..4edda9febbe 100644 --- a/spec/features/projects/jobs_spec.rb +++ b/spec/features/projects/jobs_spec.rb @@ -28,7 +28,7 @@ RSpec.describe 'Jobs', :clean_gitlab_redis_shared_state do context 'with no jobs' do before do stub_experiment(jobs_empty_state: experiment_active) - stub_experiment_for_user(jobs_empty_state: in_experiment_group) + stub_experiment_for_subject(jobs_empty_state: in_experiment_group) visit project_jobs_path(project) end diff --git a/spec/features/registrations/experience_level_spec.rb b/spec/features/registrations/experience_level_spec.rb index 30f19870f69..25496e2fef1 100644 --- a/spec/features/registrations/experience_level_spec.rb +++ b/spec/features/registrations/experience_level_spec.rb @@ -9,7 +9,7 @@ RSpec.describe 'Experience level screen' do before do group.add_owner(user) gitlab_sign_in(user) - stub_experiment_for_user(onboarding_issues: true) + stub_experiment_for_subject(onboarding_issues: true) visit users_sign_up_experience_level_path(namespace_path: group.to_param) end diff --git a/spec/frontend/integrations/edit/store/actions_spec.js b/spec/frontend/integrations/edit/store/actions_spec.js index 5b5c8d6f76e..1ff881c265d 100644 --- a/spec/frontend/integrations/edit/store/actions_spec.js +++ b/spec/frontend/integrations/edit/store/actions_spec.js @@ -1,13 +1,19 @@ import testAction from 'helpers/vuex_action_helper'; +import { refreshCurrentPage } from '~/lib/utils/url_utility'; import createState from '~/integrations/edit/store/state'; import { setOverride, setIsSaving, setIsTesting, setIsResetting, + requestResetIntegration, + receiveResetIntegrationSuccess, + receiveResetIntegrationError, } from '~/integrations/edit/store/actions'; import * as types from '~/integrations/edit/store/mutation_types'; +jest.mock('~/lib/utils/url_utility'); + describe('Integration form store actions', () => { let state; @@ -40,4 +46,28 @@ describe('Integration form store actions', () => { ]); }); }); + + describe('requestResetIntegration', () => { + it('should commit REQUEST_RESET_INTEGRATION mutation', () => { + return testAction(requestResetIntegration, null, state, [ + { type: types.REQUEST_RESET_INTEGRATION }, + ]); + }); + }); + + describe('receiveResetIntegrationSuccess', () => { + it('should call refreshCurrentPage()', () => { + return testAction(receiveResetIntegrationSuccess, null, state, [], [], () => { + expect(refreshCurrentPage).toHaveBeenCalled(); + }); + }); + }); + + describe('receiveResetIntegrationError', () => { + it('should commit RECEIVE_RESET_INTEGRATION_ERROR mutation', () => { + return testAction(receiveResetIntegrationError, null, state, [ + { type: types.RECEIVE_RESET_INTEGRATION_ERROR }, + ]); + }); + }); }); diff --git a/spec/frontend/integrations/edit/store/mutations_spec.js b/spec/frontend/integrations/edit/store/mutations_spec.js index 4707b4b3714..81f39adb87f 100644 --- a/spec/frontend/integrations/edit/store/mutations_spec.js +++ b/spec/frontend/integrations/edit/store/mutations_spec.js @@ -40,4 +40,20 @@ describe('Integration form store mutations', () => { expect(state.isResetting).toBe(true); }); }); + + describe(`${types.REQUEST_RESET_INTEGRATION}`, () => { + it('sets isResetting', () => { + mutations[types.REQUEST_RESET_INTEGRATION](state); + + expect(state.isResetting).toBe(true); + }); + }); + + describe(`${types.RECEIVE_RESET_INTEGRATION_ERROR}`, () => { + it('sets isResetting', () => { + mutations[types.RECEIVE_RESET_INTEGRATION_ERROR](state); + + expect(state.isResetting).toBe(false); + }); + }); }); diff --git a/spec/frontend/pipelines/unwrapping_utils_spec.js b/spec/frontend/pipelines/unwrapping_utils_spec.js new file mode 100644 index 00000000000..34413ad3ef3 --- /dev/null +++ b/spec/frontend/pipelines/unwrapping_utils_spec.js @@ -0,0 +1,127 @@ +import { + unwrapGroups, + unwrapNodesWithName, + unwrapStagesWithNeeds, +} from '~/pipelines/components/unwrapping_utils'; + +const groupsArray = [ + { + name: 'build_a', + size: 1, + status: { + label: 'passed', + group: 'success', + icon: 'status_success', + }, + }, + { + name: 'bob_the_build', + size: 1, + status: { + label: 'passed', + group: 'success', + icon: 'status_success', + }, + }, +]; + +const basicStageInfo = { + name: 'center_stage', + status: { + action: null, + }, +}; + +const stagesAndGroups = [ + { + ...basicStageInfo, + groups: { + nodes: groupsArray, + }, + }, +]; + +const needArray = [ + { + name: 'build_b', + }, +]; + +const elephantArray = [ + { + name: 'build_b', + elephant: 'gray', + }, +]; + +const baseJobs = { + name: 'test_d', + status: { + icon: 'status_success', + tooltip: null, + hasDetails: true, + detailsPath: '/root/abcd-dag/-/pipelines/162', + group: 'success', + action: null, + }, +}; + +const jobArrayWithNeeds = [ + { + ...baseJobs, + needs: { + nodes: needArray, + }, + }, +]; + +const jobArrayWithElephant = [ + { + ...baseJobs, + needs: { + nodes: elephantArray, + }, + }, +]; + +const completeMock = [ + { + ...basicStageInfo, + groups: { + nodes: groupsArray.map(group => ({ ...group, jobs: { nodes: jobArrayWithNeeds } })), + }, + }, +]; + +describe('Shared pipeline unwrapping utils', () => { + describe('unwrapGroups', () => { + it('takes stages without nodes and returns the unwrapped groups', () => { + expect(unwrapGroups(stagesAndGroups)[0].groups).toEqual(groupsArray); + }); + + it('keeps other stage properties intact', () => { + expect(unwrapGroups(stagesAndGroups)[0]).toMatchObject(basicStageInfo); + }); + }); + + describe('unwrapNodesWithName', () => { + it('works with no field argument', () => { + expect(unwrapNodesWithName(jobArrayWithNeeds, 'needs')[0].needs).toEqual([needArray[0].name]); + }); + + it('works with custom field argument', () => { + expect(unwrapNodesWithName(jobArrayWithElephant, 'needs', 'elephant')[0].needs).toEqual([ + elephantArray[0].elephant, + ]); + }); + }); + + describe('unwrapStagesWithNeeds', () => { + it('removes nodes from groups, jobs, and needs', () => { + const firstProcessedGroup = unwrapStagesWithNeeds(completeMock)[0].groups[0]; + expect(firstProcessedGroup).toMatchObject(groupsArray[0]); + expect(firstProcessedGroup.jobs[0]).toMatchObject(baseJobs); + expect(firstProcessedGroup.jobs[0].needs[0]).toBe(needArray[0].name); + }); + }); +}); diff --git a/spec/graphql/types/countable_connection_type_spec.rb b/spec/graphql/types/countable_connection_type_spec.rb index 3b3c02baa5d..648dbacff42 100644 --- a/spec/graphql/types/countable_connection_type_spec.rb +++ b/spec/graphql/types/countable_connection_type_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe GitlabSchema.types['MergeRequestConnection'] do +RSpec.describe GitlabSchema.types['PipelineConnection'] do it 'has the expected fields' do expected_fields = %i[count page_info edges nodes] diff --git a/spec/graphql/types/merge_request_connection_type_spec.rb b/spec/graphql/types/merge_request_connection_type_spec.rb new file mode 100644 index 00000000000..f4ab6c79721 --- /dev/null +++ b/spec/graphql/types/merge_request_connection_type_spec.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe GitlabSchema.types['MergeRequestConnection'] do + it 'has the expected fields' do + expected_fields = %i[count totalTimeToMerge page_info edges nodes] + + expect(described_class).to have_graphql_fields(*expected_fields) + end +end diff --git a/spec/helpers/services_helper_spec.rb b/spec/helpers/services_helper_spec.rb index d6b48b3d565..29c83b7c4da 100644 --- a/spec/helpers/services_helper_spec.rb +++ b/spec/helpers/services_helper_spec.rb @@ -59,4 +59,57 @@ RSpec.describe ServicesHelper do end end end + + describe '#scoped_reset_integration_path' do + let(:integration) { build_stubbed(:jira_service) } + let(:group) { nil } + + subject { helper.scoped_reset_integration_path(integration, group: group) } + + context 'when no group is present' do + it 'returns instance-level path' do + is_expected.to eq(reset_admin_application_settings_integration_path(integration)) + end + end + + context 'when group is present' do + let(:group) { build_stubbed(:group) } + + it 'returns group-level path' do + is_expected.to eq(reset_group_settings_integration_path(group, integration)) + end + end + end + + describe '#reset_integrations?' do + let(:group) { nil } + + subject { helper.reset_integrations?(group: group) } + + context 'when `reset_integrations` is not enabled' do + it 'returns false' do + stub_feature_flags(reset_integrations: false) + + is_expected.to eq(false) + end + end + + context 'when `reset_integrations` is enabled' do + it 'returns true' do + stub_feature_flags(reset_integrations: true) + + is_expected.to eq(true) + end + end + + context 'when `reset_integrations` is enabled for a group' do + let(:group) { build_stubbed(:group) } + + it 'returns true' do + stub_feature_flags(reset_integrations: group) + + is_expected.to eq(true) + end + end + end end diff --git a/spec/lib/gitlab/experimentation/controller_concern_spec.rb b/spec/lib/gitlab/experimentation/controller_concern_spec.rb index 727afdd4063..3cda5d230c5 100644 --- a/spec/lib/gitlab/experimentation/controller_concern_spec.rb +++ b/spec/lib/gitlab/experimentation/controller_concern_spec.rb @@ -75,29 +75,24 @@ RSpec.describe Gitlab::Experimentation::ControllerConcern, type: :controller do describe '#push_frontend_experiment' do it 'pushes an experiment to the frontend' do gon = instance_double('gon') - experiments = { experiments: { 'myExperiment' => true } } - - stub_experiment_for_user(my_experiment: true) + stub_experiment_for_subject(my_experiment: true) allow(controller).to receive(:gon).and_return(gon) - expect(gon).to receive(:push).with(experiments, true) + expect(gon).to receive(:push).with({ experiments: { 'myExperiment' => true } }, true) controller.push_frontend_experiment(:my_experiment) end end describe '#experiment_enabled?' do - def check_experiment(exp_key = :test_experiment) - controller.experiment_enabled?(exp_key) + def check_experiment(exp_key = :test_experiment, subject = nil) + controller.experiment_enabled?(exp_key, subject: subject) end subject { check_experiment } context 'cookie is not present' do - it 'calls Gitlab::Experimentation.enabled_for_value? with the name of the experiment and an experimentation_subject_index of nil' do - expect(Gitlab::Experimentation).to receive(:enabled_for_value?).with(:test_experiment, nil) - check_experiment - end + it { is_expected.to eq(false) } end context 'cookie is present' do @@ -109,37 +104,56 @@ RSpec.describe Gitlab::Experimentation::ControllerConcern, type: :controller do end where(:experiment_key, :index_value) do - :test_experiment | 40 # Zlib.crc32('test_experimentabcd-1234') % 100 = 40 - :backwards_compatible_test_experiment | 76 # 'abcd1234'.hex % 100 = 76 + :test_experiment | 'abcd-1234' + :backwards_compatible_test_experiment | 'abcd1234' end with_them do - it 'calls Gitlab::Experimentation.enabled_for_value? with the name of the experiment and the calculated experimentation_subject_index based on the uuid' do - expect(Gitlab::Experimentation).to receive(:enabled_for_value?).with(experiment_key, index_value) + it 'calls Gitlab::Experimentation.in_experiment_group?? with the name of the experiment and the calculated experimentation_subject_index based on the uuid' do + expect(Gitlab::Experimentation).to receive(:in_experiment_group?).with(experiment_key, subject: index_value) + check_experiment(experiment_key) end end - end - it 'returns true when DNT: 0 is set in the request' do - allow(Gitlab::Experimentation).to receive(:enabled_for_value?) { true } - controller.request.headers['DNT'] = '0' + context 'when subject is given' do + let(:user) { build(:user) } - is_expected.to be_truthy + it 'uses the subject' do + expect(Gitlab::Experimentation).to receive(:in_experiment_group?).with(:test_experiment, subject: user) + + check_experiment(:test_experiment, user) + end + end end - it 'returns false when DNT: 1 is set in the request' do - allow(Gitlab::Experimentation).to receive(:enabled_for_value?) { true } - controller.request.headers['DNT'] = '1' + context 'do not track' do + before do + allow(Gitlab::Experimentation).to receive(:in_experiment_group?) { true } + end - is_expected.to be_falsy + context 'when do not track is disabled' do + before do + controller.request.headers['DNT'] = '0' + end + + it { is_expected.to eq(true) } + end + + context 'when do not track is enabled' do + before do + controller.request.headers['DNT'] = '1' + end + + it { is_expected.to eq(false) } + end end - describe 'URL parameter to force enable experiment' do + context 'URL parameter to force enable experiment' do it 'returns true unconditionally' do get :index, params: { force_experiment: :test_experiment } - is_expected.to be_truthy + is_expected.to eq(true) end end end @@ -152,7 +166,7 @@ RSpec.describe Gitlab::Experimentation::ControllerConcern, type: :controller do context 'the user is part of the experimental group' do before do - stub_experiment_for_user(test_experiment: true) + stub_experiment_for_subject(test_experiment: true) end it 'tracks the event with the right parameters' do @@ -169,7 +183,7 @@ RSpec.describe Gitlab::Experimentation::ControllerConcern, type: :controller do context 'the user is part of the control group' do before do - stub_experiment_for_user(test_experiment: false) + stub_experiment_for_subject(test_experiment: false) end it 'tracks the event with the right parameters' do @@ -212,6 +226,59 @@ RSpec.describe Gitlab::Experimentation::ControllerConcern, type: :controller do expect_no_snowplow_event end end + + context 'subject is provided' do + before do + stub_experiment_for_subject(test_experiment: false) + end + + it "provides the subject's hashed global_id as label" do + experiment_subject = double(:subject, to_global_id: 'abc') + + controller.track_experiment_event(:test_experiment, 'start', 1, subject: experiment_subject) + + expect_snowplow_event( + category: 'Team', + action: 'start', + property: 'control_group', + value: 1, + label: Digest::MD5.hexdigest('abc') + ) + end + + it "provides the subject's hashed string representation as label" do + experiment_subject = 'somestring' + + controller.track_experiment_event(:test_experiment, 'start', 1, subject: experiment_subject) + + expect_snowplow_event( + category: 'Team', + action: 'start', + property: 'control_group', + value: 1, + label: Digest::MD5.hexdigest('somestring') + ) + end + end + + context 'no subject is provided but cookie is set' do + before do + get :index + stub_experiment_for_subject(test_experiment: false) + end + + it 'uses the experimentation_subject_id as fallback' do + controller.track_experiment_event(:test_experiment, 'start', 1) + + expect_snowplow_event( + category: 'Team', + action: 'start', + property: 'control_group', + value: 1, + label: cookies.permanent.signed[:experimentation_subject_id] + ) + end + end end context 'when the experiment is disabled' do @@ -235,7 +302,7 @@ RSpec.describe Gitlab::Experimentation::ControllerConcern, type: :controller do context 'the user is part of the experimental group' do before do - stub_experiment_for_user(test_experiment: true) + stub_experiment_for_subject(test_experiment: true) end it 'pushes the right parameters to gon' do @@ -253,9 +320,7 @@ RSpec.describe Gitlab::Experimentation::ControllerConcern, type: :controller do context 'the user is part of the control group' do before do - allow_next_instance_of(described_class) do |instance| - allow(instance).to receive(:experiment_enabled?).with(:test_experiment).and_return(false) - end + stub_experiment_for_subject(test_experiment: false) end it 'pushes the right parameters to gon' do @@ -308,7 +373,7 @@ RSpec.describe Gitlab::Experimentation::ControllerConcern, type: :controller do it 'does not push data to gon' do controller.frontend_experimentation_tracking_data(:test_experiment, 'start') - expect(Gon.method_defined?(:tracking_data)).to be_falsey + expect(Gon.method_defined?(:tracking_data)).to eq(false) end end end @@ -319,7 +384,7 @@ RSpec.describe Gitlab::Experimentation::ControllerConcern, type: :controller do end it 'does not push data to gon' do - expect(Gon.method_defined?(:tracking_data)).to be_falsey + expect(Gon.method_defined?(:tracking_data)).to eq(false) controller.track_experiment_event(:test_experiment, 'start') end end @@ -336,7 +401,7 @@ RSpec.describe Gitlab::Experimentation::ControllerConcern, type: :controller do context 'the user is part of the experimental group' do before do - stub_experiment_for_user(test_experiment: true) + stub_experiment_for_subject(test_experiment: true) end it 'calls add_user on the Experiment model' do @@ -348,9 +413,7 @@ RSpec.describe Gitlab::Experimentation::ControllerConcern, type: :controller do context 'the user is part of the control group' do before do - allow_next_instance_of(described_class) do |instance| - allow(instance).to receive(:experiment_enabled?).with(:test_experiment).and_return(false) - end + stub_experiment_for_subject(test_experiment: false) end it 'calls add_user on the Experiment model' do @@ -395,6 +458,7 @@ RSpec.describe Gitlab::Experimentation::ControllerConcern, type: :controller do context 'is disabled' do before do request.headers['DNT'] = '0' + stub_experiment_for_subject(test_experiment: false) end it 'calls add_user on the Experiment model' do @@ -475,7 +539,7 @@ RSpec.describe Gitlab::Experimentation::ControllerConcern, type: :controller do it 'returns a string with the experiment tracking category & group joined with a ":"' do expect(controller).to receive(:tracking_category).with(experiment_key).and_return('Experiment::Category') - expect(controller).to receive(:tracking_group).with(experiment_key, '_group').and_return('experimental_group') + expect(controller).to receive(:tracking_group).with(experiment_key, '_group', subject: nil).and_return('experimental_group') expect(subject).to eq('Experiment::Category:experimental_group') end diff --git a/spec/lib/gitlab/experimentation/experiment_spec.rb b/spec/lib/gitlab/experimentation/experiment_spec.rb index 4af76e9e920..7b1d1763010 100644 --- a/spec/lib/gitlab/experimentation/experiment_spec.rb +++ b/spec/lib/gitlab/experimentation/experiment_spec.rb @@ -20,14 +20,14 @@ RSpec.describe Gitlab::Experimentation::Experiment do subject(:experiment) { described_class.new(:experiment_key, **params) } - describe '#enabled?' do + describe '#active?' do before do allow(Gitlab).to receive(:dev_env_or_com?).and_return(on_gitlab_com) end - subject { experiment.enabled? } + subject { experiment.active? } - where(:on_gitlab_com, :percentage, :is_enabled) do + where(:on_gitlab_com, :percentage, :is_active) do true | 0 | false true | 10 | true false | 0 | false @@ -35,7 +35,7 @@ RSpec.describe Gitlab::Experimentation::Experiment do end with_them do - it { is_expected.to eq(is_enabled) } + it { is_expected.to eq(is_active) } end end diff --git a/spec/lib/gitlab/experimentation_spec.rb b/spec/lib/gitlab/experimentation_spec.rb index 4130d5f9184..2e9daed41e4 100644 --- a/spec/lib/gitlab/experimentation_spec.rb +++ b/spec/lib/gitlab/experimentation_spec.rb @@ -28,7 +28,7 @@ RSpec.describe Gitlab::Experimentation::EXPERIMENTS do end end -RSpec.describe Gitlab::Experimentation, :snowplow do +RSpec.describe Gitlab::Experimentation do before do stub_const('Gitlab::Experimentation::EXPERIMENTS', { backwards_compatible_test_experiment: { @@ -47,92 +47,131 @@ RSpec.describe Gitlab::Experimentation, :snowplow do let(:enabled_percentage) { 10 } - describe '.enabled?' do - subject { described_class.enabled?(:test_experiment) } + describe '.get_experiment' do + subject { described_class.get_experiment(:test_experiment) } - context 'feature toggle is enabled and we are selected' do - it { is_expected.to be_truthy } + context 'returns experiment' do + it { is_expected.to be_instance_of(Gitlab::Experimentation::Experiment) } + end + + context 'experiment is not defined' do + subject { described_class.get_experiment(:missing_experiment) } + + it { is_expected.to be_nil } + end + end + + describe '.active?' do + subject { described_class.active?(:test_experiment) } + + context 'feature toggle is enabled' do + it { is_expected.to eq(true) } end describe 'experiment is not defined' do it 'returns false' do - expect(described_class.enabled?(:missing_experiment)).to be_falsey + expect(described_class.active?(:missing_experiment)).to eq(false) end end describe 'experiment is disabled' do let(:enabled_percentage) { 0 } - it { is_expected.to be_falsey } + it { is_expected.to eq(false) } end end - describe '.enabled_for_value?' do - subject { described_class.enabled_for_value?(:test_experiment, experimentation_subject_index) } + describe '.in_experiment_group?' do + context 'with new index calculation' do + let(:enabled_percentage) { 50 } + let(:experiment_subject) { 'z' } # Zlib.crc32('test_experimentz') % 100 = 33 - let(:experimentation_subject_index) { 9 } - - context 'experiment is disabled' do - before do - allow(described_class).to receive(:enabled?).and_return(false) - end + subject { described_class.in_experiment_group?(:test_experiment, subject: experiment_subject) } - it { is_expected.to be_falsey } - end + context 'when experiment is active' do + context 'when subject is part of the experiment' do + it { is_expected.to eq(true) } + end - context 'experiment is enabled' do - before do - allow(described_class).to receive(:enabled?).and_return(true) - end + context 'when subject is not part of the experiment' do + let(:experiment_subject) { 'a' } # Zlib.crc32('test_experimenta') % 100 = 61 - it { is_expected.to be_truthy } + it { is_expected.to eq(false) } + end - describe 'experimentation_subject_index' do - context 'experimentation_subject_index is not set' do - let(:experimentation_subject_index) { nil } + context 'when subject has a global_id' do + let(:experiment_subject) { double(:subject, to_global_id: 'z') } - it { is_expected.to be_falsey } + it { is_expected.to eq(true) } end - context 'experimentation_subject_index is an empty string' do - let(:experimentation_subject_index) { '' } + context 'when subject is nil' do + let(:experiment_subject) { nil } - it { is_expected.to be_falsey } + it { is_expected.to eq(false) } end - context 'experimentation_subject_index outside enabled ratio' do - let(:experimentation_subject_index) { 11 } + context 'when subject is an empty string' do + let(:experiment_subject) { '' } - it { is_expected.to be_falsey } + it { is_expected.to eq(false) } end end + + context 'when experiment is not active' do + before do + allow(described_class).to receive(:active?).and_return(false) + end + + it { is_expected.to eq(false) } + end end - end - describe '.enabled_for_attribute?' do - subject { described_class.enabled_for_attribute?(:test_experiment, attribute) } + context 'with backwards compatible index calculation' do + let(:experiment_subject) { 'abcd' } # Digest::SHA1.hexdigest('abcd').hex % 100 = 7 - let(:attribute) { 'abcd' } # Digest::SHA1.hexdigest('abcd').hex % 100 = 7 + subject { described_class.in_experiment_group?(:backwards_compatible_test_experiment, subject: experiment_subject) } - context 'experiment is disabled' do - before do - allow(described_class).to receive(:enabled?).and_return(false) - end + context 'when experiment is active' do + before do + allow(described_class).to receive(:active?).and_return(true) + end - it { is_expected.to be false } - end + context 'when subject is part of the experiment' do + it { is_expected.to eq(true) } + end - context 'experiment is enabled' do - before do - allow(described_class).to receive(:enabled?).and_return(true) - end + context 'when subject is not part of the experiment' do + let(:experiment_subject) { 'abc' } # Digest::SHA1.hexdigest('abc').hex % 100 = 17 - it { is_expected.to be true } + it { is_expected.to eq(false) } + end + + context 'when subject has a global_id' do + let(:experiment_subject) { double(:subject, to_global_id: 'abcd') } + + it { is_expected.to eq(true) } + end - context 'outside enabled ratio' do - let(:attribute) { 'abc' } # Digest::SHA1.hexdigest('abc').hex % 100 = 17 + context 'when subject is nil' do + let(:experiment_subject) { nil } + + it { is_expected.to eq(false) } + end + + context 'when subject is an empty string' do + let(:experiment_subject) { '' } + + it { is_expected.to eq(false) } + end + end + + context 'when experiment is not active' do + before do + allow(described_class).to receive(:active?).and_return(false) + end - it { is_expected.to be false } + it { is_expected.to eq(false) } end end end diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index 382f29e1bed..b4688cc02c4 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -1453,7 +1453,7 @@ RSpec.describe Notify do shared_examples "tracks the 'sent' event for the invitation reminders experiment" do before do stub_experiment(invitation_reminders: true) - allow(Gitlab::Experimentation).to receive(:enabled_for_attribute?).with(:invitation_reminders, group_member.invite_email).and_return(experimental_group) + allow(Gitlab::Experimentation).to receive(:in_experiment_group?).with(:invitation_reminders, subject: group_member.invite_email).and_return(experimental_group) end it "tracks the 'sent' event", :snowplow do diff --git a/spec/migrations/populate_remaining_missing_dismissal_information_for_vulnerabilities_spec.rb b/spec/migrations/populate_remaining_missing_dismissal_information_for_vulnerabilities_spec.rb new file mode 100644 index 00000000000..986436971ac --- /dev/null +++ b/spec/migrations/populate_remaining_missing_dismissal_information_for_vulnerabilities_spec.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_migration! + +RSpec.describe PopulateRemainingMissingDismissalInformationForVulnerabilities do + let(:users) { table(:users) } + let(:namespaces) { table(:namespaces) } + let(:projects) { table(:projects) } + let(:vulnerabilities) { table(:vulnerabilities) } + + let(:user) { users.create!(name: 'test', email: 'test@example.com', projects_limit: 5) } + let(:namespace) { namespaces.create!(name: 'gitlab', path: 'gitlab-org') } + let(:project) { projects.create!(namespace_id: namespace.id, name: 'foo') } + + let(:states) { { detected: 1, dismissed: 2, resolved: 3, confirmed: 4 } } + let!(:vulnerability_1) { vulnerabilities.create!(title: 'title', state: states[:detected], severity: 0, confidence: 5, report_type: 2, project_id: project.id, author_id: user.id) } + let!(:vulnerability_2) { vulnerabilities.create!(title: 'title', state: states[:dismissed], severity: 0, confidence: 5, report_type: 2, project_id: project.id, author_id: user.id) } + let!(:vulnerability_3) { vulnerabilities.create!(title: 'title', state: states[:resolved], severity: 0, confidence: 5, report_type: 2, project_id: project.id, author_id: user.id) } + let!(:vulnerability_4) { vulnerabilities.create!(title: 'title', state: states[:confirmed], severity: 0, confidence: 5, report_type: 2, project_id: project.id, author_id: user.id) } + + describe '#perform' do + it 'calls the background migration class instance with broken vulnerability IDs' do + expect_next_instance_of(::Gitlab::BackgroundMigration::PopulateMissingVulnerabilityDismissalInformation) do |migrator| + expect(migrator).to receive(:perform).with(vulnerability_2.id) + end + + migrate! + end + end +end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 9e1ed05f896..09bad8694fc 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -500,6 +500,77 @@ RSpec.describe MergeRequest, factory_default: :keep do end end + describe 'time to merge calculations' do + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project) } + + let!(:mr1) do + create( + :merge_request, + :with_merged_metrics, + source_project: project, + target_project: project + ) + end + + let!(:mr2) do + create( + :merge_request, + :with_merged_metrics, + source_project: project, + target_project: project + ) + end + + let!(:mr3) do + create( + :merge_request, + :with_merged_metrics, + source_project: project, + target_project: project + ) + end + + let!(:unmerged_mr) do + create( + :merge_request, + source_project: project, + target_project: project + ) + end + + before do + project.add_user(user, :developer) + end + + describe '.total_time_to_merge' do + it 'returns the sum of the time to merge for all merged MRs' do + mrs = project.merge_requests + + expect(mrs.total_time_to_merge).to be_within(1).of(expected_total_time(mrs)) + end + + context 'when merged_at is earlier than created_at' do + before do + mr1.metrics.update!(merged_at: mr1.metrics.created_at - 1.week) + end + + it 'returns nil' do + mrs = project.merge_requests.where(id: mr1.id) + + expect(mrs.total_time_to_merge).to be_nil + end + end + + def expected_total_time(mrs) + mrs = mrs.reject { |mr| mr.merged_at.nil? } + mrs.reduce(0.0) do |sum, mr| + (mr.merged_at - mr.created_at) + sum + end + end + end + end + describe '#target_branch_sha' do let(:project) { create(:project, :repository) } diff --git a/spec/policies/global_policy_spec.rb b/spec/policies/global_policy_spec.rb index c8401e4acff..e677f5558fd 100644 --- a/spec/policies/global_policy_spec.rb +++ b/spec/policies/global_policy_spec.rb @@ -150,6 +150,24 @@ RSpec.describe GlobalPolicy do end end + describe 'rejecting users' do + context 'regular user' do + it { is_expected.not_to be_allowed(:reject_user) } + end + + context 'admin' do + let(:current_user) { create(:admin) } + + context 'when admin mode is enabled', :enable_admin_mode do + it { is_expected.to be_allowed(:reject_user) } + end + + context 'when admin mode is disabled' do + it { is_expected.to be_disallowed(:reject_user) } + end + end + end + describe 'using project statistics filters' do context 'regular user' do it { is_expected.not_to be_allowed(:use_project_statistics_filters) } diff --git a/spec/services/members/invitation_reminder_email_service_spec.rb b/spec/services/members/invitation_reminder_email_service_spec.rb index 88280869476..7cb30662152 100644 --- a/spec/services/members/invitation_reminder_email_service_spec.rb +++ b/spec/services/members/invitation_reminder_email_service_spec.rb @@ -11,7 +11,7 @@ RSpec.describe Members::InvitationReminderEmailService do context 'when the experiment is disabled' do before do - allow(Gitlab::Experimentation).to receive(:enabled_for_attribute?).and_return(false) + allow(Gitlab::Experimentation).to receive(:in_experiment_group?).and_return(false) invitation.expires_at = frozen_time + 2.days end @@ -26,7 +26,7 @@ RSpec.describe Members::InvitationReminderEmailService do context 'when the experiment is enabled' do before do - allow(Gitlab::Experimentation).to receive(:enabled_for_attribute?).and_return(true) + allow(Gitlab::Experimentation).to receive(:in_experiment_group?).and_return(true) invitation.expires_at = frozen_time + expires_at_days.days if expires_at_days end diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 34a95c0505d..28577821231 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -2326,6 +2326,20 @@ RSpec.describe NotificationService, :mailer do end end + describe '#user_admin_rejection', :deliver_mails_inline do + let_it_be(:user) { create(:user, :blocked_pending_approval) } + + before do + reset_delivered_emails! + end + + it 'sends the user a rejection email' do + notification.user_admin_rejection(user.name, user.email) + + should_only_email(user) + end + end + describe 'GroupMember', :deliver_mails_inline do let(:added_user) { create(:user) } diff --git a/spec/services/users/reject_service_spec.rb b/spec/services/users/reject_service_spec.rb new file mode 100644 index 00000000000..07863d1a290 --- /dev/null +++ b/spec/services/users/reject_service_spec.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Users::RejectService do + let_it_be(:current_user) { create(:admin) } + let(:user) { create(:user, :blocked_pending_approval) } + + subject(:execute) { described_class.new(current_user).execute(user) } + + describe '#execute' do + context 'failures' do + context 'when the executor user is not allowed to reject users' do + let(:current_user) { create(:user) } + + it 'returns error result' do + expect(subject[:status]).to eq(:error) + expect(subject[:message]).to match(/You are not allowed to reject a user/) + end + end + + context 'when the executor user is an admin in admin mode', :enable_admin_mode do + context 'when user is not in pending approval state' do + let(:user) { create(:user, state: 'active') } + + it 'returns error result' do + expect(subject[:status]).to eq(:error) + expect(subject[:message]) + .to match(/This user does not have a pending request/) + end + end + end + end + + context 'success' do + context 'when the executor user is an admin in admin mode', :enable_admin_mode do + it 'deletes the user', :sidekiq_inline do + subject + + expect(subject[:status]).to eq(:success) + expect { User.find(user.id) }.to raise_error(ActiveRecord::RecordNotFound) + end + + it 'emails the user on rejection' do + expect_next_instance_of(NotificationService) do |notification| + allow(notification).to receive(:user_admin_rejection).with(user.name, user.notification_email) + end + + subject + end + end + end + end +end diff --git a/spec/support/helpers/stub_experiments.rb b/spec/support/helpers/stub_experiments.rb index 7a6154d5ef9..247692d83ee 100644 --- a/spec/support/helpers/stub_experiments.rb +++ b/spec/support/helpers/stub_experiments.rb @@ -3,15 +3,15 @@ module StubExperiments # Stub Experiment with `key: true/false` # - # @param [Hash] experiment where key is feature name and value is boolean whether enabled or not. + # @param [Hash] experiment where key is feature name and value is boolean whether active or not. # # Examples - # - `stub_experiment(signup_flow: false)` ... Disable `signup_flow` experiment globally. + # - `stub_experiment(signup_flow: false)` ... Disables `signup_flow` experiment. def stub_experiment(experiments) - allow(Gitlab::Experimentation).to receive(:enabled?).and_call_original + allow(Gitlab::Experimentation).to receive(:active?).and_call_original experiments.each do |experiment_key, enabled| - allow(Gitlab::Experimentation).to receive(:enabled?).with(experiment_key) { enabled } + allow(Gitlab::Experimentation).to receive(:active?).with(experiment_key) { enabled } end end @@ -20,12 +20,12 @@ module StubExperiments # @param [Hash] experiment where key is feature name and value is boolean whether enabled or not. # # Examples - # - `stub_experiment_for_user(signup_flow: false)` ... Disable `signup_flow` experiment for user. - def stub_experiment_for_user(experiments) - allow(Gitlab::Experimentation).to receive(:enabled_for_value?).and_call_original + # - `stub_experiment_for_subject(signup_flow: false)` ... Disable `signup_flow` experiment for user. + def stub_experiment_for_subject(experiments) + allow(Gitlab::Experimentation).to receive(:in_experiment_group?).and_call_original experiments.each do |experiment_key, enabled| - allow(Gitlab::Experimentation).to receive(:enabled_for_value?).with(experiment_key, anything) { enabled } + allow(Gitlab::Experimentation).to receive(:in_experiment_group?).with(experiment_key, anything) { enabled } end end end diff --git a/spec/support/shared_examples/features/issuable_invite_members_shared_examples.rb b/spec/support/shared_examples/features/issuable_invite_members_shared_examples.rb index ac1cc2da7e3..3fec1a56c0c 100644 --- a/spec/support/shared_examples/features/issuable_invite_members_shared_examples.rb +++ b/spec/support/shared_examples/features/issuable_invite_members_shared_examples.rb @@ -3,7 +3,7 @@ RSpec.shared_examples 'issuable invite members experiments' do context 'when invite_members_version_a experiment is enabled' do before do - stub_experiment_for_user(invite_members_version_a: true) + stub_experiment_for_subject(invite_members_version_a: true) end it 'shows a link for inviting members and follows through to the members page' do @@ -28,7 +28,7 @@ RSpec.shared_examples 'issuable invite members experiments' do context 'when invite_members_version_b experiment is enabled' do before do - stub_experiment_for_user(invite_members_version_b: true) + stub_experiment_for_subject(invite_members_version_b: true) end it 'shows a link for inviting members and follows through to modal' do -- cgit v1.2.3