diff options
| author | GitLab Bot <gitlab-bot@gitlab.com> | 2022-07-05 18:08:43 +0000 |
|---|---|---|
| committer | GitLab Bot <gitlab-bot@gitlab.com> | 2022-07-05 18:08:43 +0000 |
| commit | e129eff88309eca18f3902afd710e2e07393fe45 (patch) | |
| tree | 2dd9399fdcfdee719d51e63cd821adc58165ccb3 /spec | |
| parent | 205b6baf2677879c35968d2b659225b58e8a1227 (diff) | |
| download | gitlab-ce-e129eff88309eca18f3902afd710e2e07393fe45.tar.gz | |
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'spec')
90 files changed, 1147 insertions, 461 deletions
diff --git a/spec/controllers/groups/variables_controller_spec.rb b/spec/controllers/groups/variables_controller_spec.rb index 8c0aa83b9c4..6dbe75bb1df 100644 --- a/spec/controllers/groups/variables_controller_spec.rb +++ b/spec/controllers/groups/variables_controller_spec.rb @@ -13,7 +13,7 @@ RSpec.describe Groups::VariablesController do before do sign_in(user) - group.add_user(user, access_level) + group.add_member(user, access_level) end describe 'GET #show' do diff --git a/spec/controllers/import/available_namespaces_controller_spec.rb b/spec/controllers/import/available_namespaces_controller_spec.rb index 0f98d649338..26ea1d92189 100644 --- a/spec/controllers/import/available_namespaces_controller_spec.rb +++ b/spec/controllers/import/available_namespaces_controller_spec.rb @@ -25,7 +25,7 @@ RSpec.describe Import::AvailableNamespacesController do it "does not include group with access level #{params[:role]} in list" do group = create(:group, project_creation_level: group_project_creation_level) - group.add_user(user, role) + group.add_member(user, role) get :index expect(response).to have_gitlab_http_status(:ok) @@ -52,7 +52,7 @@ RSpec.describe Import::AvailableNamespacesController do it "does not include group with access level #{params[:role]} in list" do group = create(:group, project_creation_level: group_project_creation_level) - group.add_user(user, role) + group.add_member(user, role) get :index expect(response).to have_gitlab_http_status(:ok) @@ -81,7 +81,7 @@ RSpec.describe Import::AvailableNamespacesController do it "#{params[:is_visible] ? 'includes' : 'does not include'} group with access level #{params[:role]} in list" do group = create(:group, project_creation_level: project_creation_level) - group.add_user(user, :developer) + group.add_member(user, :developer) get :index diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index 1305693372c..badac688229 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -1818,7 +1818,7 @@ RSpec.describe Projects::IssuesController do context 'user is allowed access' do before do - project.add_user(user, :maintainer) + project.add_member(user, :maintainer) end it 'displays all available notes' do diff --git a/spec/controllers/projects/mirrors_controller_spec.rb b/spec/controllers/projects/mirrors_controller_spec.rb index 686effd799e..d33bc215cfc 100644 --- a/spec/controllers/projects/mirrors_controller_spec.rb +++ b/spec/controllers/projects/mirrors_controller_spec.rb @@ -211,7 +211,7 @@ RSpec.describe Projects::MirrorsController do context 'data in the cache' do let(:ssh_key) { 'ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIAfuCHKVTjquxvt6CM6tdG4SLp1Btn/nOeHHE5UOzRdf' } - let(:ssh_fp) { { type: 'ed25519', bits: 256, fingerprint: '2e:65:6a:c8:cf:bf:b2:8b:9a:bd:6d:9f:11:5c:12:16', index: 0 } } + let(:ssh_fp) { { type: 'ed25519', bits: 256, fingerprint: '2e:65:6a:c8:cf:bf:b2:8b:9a:bd:6d:9f:11:5c:12:16', fingerprint_sha256: 'SHA256:eUXGGm1YGsMAS7vkcx6JOJdOGHPem5gQp4taiCfCLB8', index: 0 } } it 'returns the data with a 200 response' do stub_reactive_cache(cache, known_hosts: ssh_key) diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index d9d26a68785..34477a7bb68 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -409,7 +409,7 @@ RSpec.describe ProjectsController do before do project.update!(visibility: project_visibility.to_s) - project.team.add_user(user, :guest) if user_type == :member + project.team.add_member(user, :guest) if user_type == :member sign_in(user) unless user_type == :anonymous end diff --git a/spec/features/admin/admin_groups_spec.rb b/spec/features/admin/admin_groups_spec.rb index f903d7c2db2..040c6a65b7c 100644 --- a/spec/features/admin/admin_groups_spec.rb +++ b/spec/features/admin/admin_groups_spec.rb @@ -236,7 +236,7 @@ RSpec.describe 'Admin Groups' do it 'renders relative time' do expire_time = Time.current + 2.days current_user.update!(time_display_relative: true) - group.add_user(user, Gitlab::Access::REPORTER, expires_at: expire_time) + group.add_member(user, Gitlab::Access::REPORTER, expires_at: expire_time) visit admin_group_path(group) @@ -246,7 +246,7 @@ RSpec.describe 'Admin Groups' do it 'renders absolute time' do expire_time = Time.current.tomorrow.middle_of_day current_user.update!(time_display_relative: false) - group.add_user(user, Gitlab::Access::REPORTER, expires_at: expire_time) + group.add_member(user, Gitlab::Access::REPORTER, expires_at: expire_time) visit admin_group_path(group) @@ -257,7 +257,7 @@ RSpec.describe 'Admin Groups' do describe 'add admin himself to a group' do before do - group.add_user(:user, Gitlab::Access::OWNER) + group.add_member(:user, Gitlab::Access::OWNER) end it 'adds admin a to a group as developer', :js do @@ -274,7 +274,7 @@ RSpec.describe 'Admin Groups' do describe 'admin removes themself from a group', :js do it 'removes admin from the group' do - group.add_user(current_user, Gitlab::Access::DEVELOPER) + group.add_member(current_user, Gitlab::Access::DEVELOPER) visit group_group_members_path(group) diff --git a/spec/features/admin/admin_projects_spec.rb b/spec/features/admin/admin_projects_spec.rb index 866368af40a..6b147b01991 100644 --- a/spec/features/admin/admin_projects_spec.rb +++ b/spec/features/admin/admin_projects_spec.rb @@ -20,7 +20,7 @@ RSpec.describe "Admin::Projects" do it 'renders relative time' do expire_time = Time.current + 2.days current_user.update!(time_display_relative: true) - project.add_user(user, Gitlab::Access::REPORTER, expires_at: expire_time) + project.add_member(user, Gitlab::Access::REPORTER, expires_at: expire_time) visit admin_project_path(project) @@ -30,7 +30,7 @@ RSpec.describe "Admin::Projects" do it 'renders absolute time' do expire_time = Time.current.tomorrow.middle_of_day current_user.update!(time_display_relative: false) - project.add_user(user, Gitlab::Access::REPORTER, expires_at: expire_time) + project.add_member(user, Gitlab::Access::REPORTER, expires_at: expire_time) visit admin_project_path(project) diff --git a/spec/features/file_uploads/multipart_invalid_uploads_spec.rb b/spec/features/file_uploads/multipart_invalid_uploads_spec.rb index 91c8e100e6a..cff8b4e61a5 100644 --- a/spec/features/file_uploads/multipart_invalid_uploads_spec.rb +++ b/spec/features/file_uploads/multipart_invalid_uploads_spec.rb @@ -44,7 +44,7 @@ RSpec.describe 'Invalid uploads that must be rejected', :api, :js do # These keys are rejected directly by rack itself. # The request will not be received by multipart.rb (can't use the 'handling file uploads' shared example) - it_behaves_like 'rejecting invalid keys', key_name: 'x' * 11000, message: 'Puma caught this error: exceeded available parameter key space (RangeError)' + it_behaves_like 'rejecting invalid keys', key_name: 'x' * 11000, message: 'Puma caught this error: exceeded available parameter key space (Rack::QueryParser::ParamsTooDeepError)' it_behaves_like 'rejecting invalid keys', key_name: 'package[]test', status: 400, message: 'Bad Request' it_behaves_like 'handling file uploads', 'by rejecting uploads with an invalid key' diff --git a/spec/features/issuables/issuable_list_spec.rb b/spec/features/issuables/issuable_list_spec.rb index 0fa2d238b0a..a1e80586c05 100644 --- a/spec/features/issuables/issuable_list_spec.rb +++ b/spec/features/issuables/issuable_list_spec.rb @@ -9,7 +9,7 @@ RSpec.describe 'issuable list', :js do issuable_types = [:issue, :merge_request] before do - project.add_user(user, :developer) + project.add_member(user, :developer) sign_in(user) issuable_types.each { |type| create_issuables(type) } end diff --git a/spec/features/issues/filtered_search/visual_tokens_spec.rb b/spec/features/issues/filtered_search/visual_tokens_spec.rb index 7a367723609..c44181a60e4 100644 --- a/spec/features/issues/filtered_search/visual_tokens_spec.rb +++ b/spec/features/issues/filtered_search/visual_tokens_spec.rb @@ -15,8 +15,8 @@ RSpec.describe 'Visual tokens', :js do let_it_be(:issue) { create(:issue, project: project) } before do - project.add_user(user, :maintainer) - project.add_user(user_rock, :maintainer) + project.add_member(user, :maintainer) + project.add_member(user_rock, :maintainer) sign_in(user) visit project_issues_path(project) diff --git a/spec/features/issues/user_creates_branch_and_merge_request_spec.rb b/spec/features/issues/user_creates_branch_and_merge_request_spec.rb index ae1bce7ea4c..5ba09703852 100644 --- a/spec/features/issues/user_creates_branch_and_merge_request_spec.rb +++ b/spec/features/issues/user_creates_branch_and_merge_request_spec.rb @@ -20,7 +20,7 @@ RSpec.describe 'User creates branch and merge request on issue page', :js do context 'when signed in' do before do - project.add_user(user, membership_level) + project.add_member(user, membership_level) sign_in(user) end diff --git a/spec/features/merge_request/user_sees_deployment_widget_spec.rb b/spec/features/merge_request/user_sees_deployment_widget_spec.rb index 81034caaee2..e045f11c0d8 100644 --- a/spec/features/merge_request/user_sees_deployment_widget_spec.rb +++ b/spec/features/merge_request/user_sees_deployment_widget_spec.rb @@ -20,7 +20,7 @@ RSpec.describe 'Merge request > User sees deployment widget', :js do before do merge_request.update!(merge_commit_sha: sha) - project.add_user(user, role) + project.add_member(user, role) sign_in(user) end diff --git a/spec/features/projects/members/master_adds_member_with_expiration_date_spec.rb b/spec/features/projects/members/master_adds_member_with_expiration_date_spec.rb index bd0874316ac..c92e8bc2954 100644 --- a/spec/features/projects/members/master_adds_member_with_expiration_date_spec.rb +++ b/spec/features/projects/members/master_adds_member_with_expiration_date_spec.rb @@ -32,7 +32,7 @@ RSpec.describe 'Projects > Members > Maintainer adds member with expiration date end it 'changes expiration date' do - project.team.add_users([new_member.id], :developer, expires_at: three_days_from_now) + project.team.add_members([new_member.id], :developer, expires_at: three_days_from_now) visit project_project_members_path(project) page.within find_member_row(new_member) do @@ -46,7 +46,7 @@ RSpec.describe 'Projects > Members > Maintainer adds member with expiration date end it 'clears expiration date' do - project.team.add_users([new_member.id], :developer, expires_at: five_days_from_now) + project.team.add_members([new_member.id], :developer, expires_at: five_days_from_now) visit project_project_members_path(project) page.within find_member_row(new_member) do diff --git a/spec/features/projects/show/user_interacts_with_auto_devops_banner_spec.rb b/spec/features/projects/show/user_interacts_with_auto_devops_banner_spec.rb index 59f1bc94226..262885e09b3 100644 --- a/spec/features/projects/show/user_interacts_with_auto_devops_banner_spec.rb +++ b/spec/features/projects/show/user_interacts_with_auto_devops_banner_spec.rb @@ -7,7 +7,7 @@ RSpec.describe 'Project > Show > User interacts with auto devops implicitly enab let(:user) { create(:user) } before do - project.add_user(user, role) + project.add_member(user, role) sign_in(user) end diff --git a/spec/features/projects/show/user_sees_collaboration_links_spec.rb b/spec/features/projects/show/user_sees_collaboration_links_spec.rb index 552f068ecc7..fb2f0539558 100644 --- a/spec/features/projects/show/user_sees_collaboration_links_spec.rb +++ b/spec/features/projects/show/user_sees_collaboration_links_spec.rb @@ -90,7 +90,7 @@ RSpec.describe 'Projects > Show > Collaboration links', :js do with_them do before do project.project_feature.update!({ merge_requests_access_level: merge_requests_access_level }) - project.add_user(user, user_level) + project.add_member(user, user_level) visit project_path(project) end diff --git a/spec/features/users/login_spec.rb b/spec/features/users/login_spec.rb index efb7c98d63a..3ba3650b608 100644 --- a/spec/features/users/login_spec.rb +++ b/spec/features/users/login_spec.rb @@ -579,9 +579,9 @@ RSpec.describe 'Login', :clean_gitlab_redis_sessions do context 'group setting' do before do group1 = create :group, name: 'Group 1', require_two_factor_authentication: true - group1.add_user(user, GroupMember::DEVELOPER) + group1.add_member(user, GroupMember::DEVELOPER) group2 = create :group, name: 'Group 2', require_two_factor_authentication: true - group2.add_user(user, GroupMember::DEVELOPER) + group2.add_member(user, GroupMember::DEVELOPER) end context 'with grace period defined' do diff --git a/spec/finders/autocomplete/users_finder_spec.rb b/spec/finders/autocomplete/users_finder_spec.rb index 9b3421d1b4f..de031041e18 100644 --- a/spec/finders/autocomplete/users_finder_spec.rb +++ b/spec/finders/autocomplete/users_finder_spec.rb @@ -62,7 +62,7 @@ RSpec.describe Autocomplete::UsersFinder do let_it_be(:group) { create(:group, :public) } before_all do - group.add_users([user1], GroupMember::DEVELOPER) + group.add_members([user1], GroupMember::DEVELOPER) end it { is_expected.to match_array([user1]) } diff --git a/spec/finders/joined_groups_finder_spec.rb b/spec/finders/joined_groups_finder_spec.rb index 058db735708..feb1b11d159 100644 --- a/spec/finders/joined_groups_finder_spec.rb +++ b/spec/finders/joined_groups_finder_spec.rb @@ -45,7 +45,7 @@ RSpec.describe JoinedGroupsFinder do context 'if profile visitor is in one of the private group projects' do before do project = create(:project, :private, group: private_group, name: 'B', path: 'B') - project.add_user(profile_visitor, Gitlab::Access::DEVELOPER) + project.add_member(profile_visitor, Gitlab::Access::DEVELOPER) end it 'shows group' do diff --git a/spec/finders/packages/conan/package_finder_spec.rb b/spec/finders/packages/conan/package_finder_spec.rb index 6848786818b..f25a62225a8 100644 --- a/spec/finders/packages/conan/package_finder_spec.rb +++ b/spec/finders/packages/conan/package_finder_spec.rb @@ -45,7 +45,7 @@ RSpec.describe ::Packages::Conan::PackageFinder do before do project.update_column(:visibility_level, Gitlab::VisibilityLevel.string_options[visibility.to_s]) - project.add_user(user, role) unless role == :anonymous + project.add_member(user, role) unless role == :anonymous end it { is_expected.to eq(expected_packages) } diff --git a/spec/finders/packages/group_packages_finder_spec.rb b/spec/finders/packages/group_packages_finder_spec.rb index 954db6481cd..90a8cd3c57f 100644 --- a/spec/finders/packages/group_packages_finder_spec.rb +++ b/spec/finders/packages/group_packages_finder_spec.rb @@ -98,8 +98,8 @@ RSpec.describe Packages::GroupPackagesFinder do ) unless role == :anonymous - project.add_user(user, role) - subproject.add_user(user, role) + project.add_member(user, role) + subproject.add_member(user, role) end end diff --git a/spec/frontend/ide/components/new_dropdown/modal_spec.js b/spec/frontend/ide/components/new_dropdown/modal_spec.js index e8635444801..8c72a37ec84 100644 --- a/spec/frontend/ide/components/new_dropdown/modal_spec.js +++ b/spec/frontend/ide/components/new_dropdown/modal_spec.js @@ -1,209 +1,373 @@ -import Vue, { nextTick } from 'vue'; -import { createComponentWithStore } from 'helpers/vue_mount_component_helper'; +import { GlButton, GlModal } from '@gitlab/ui'; +import { nextTick } from 'vue'; import createFlash from '~/flash'; -import modal from '~/ide/components/new_dropdown/modal.vue'; +import Modal from '~/ide/components/new_dropdown/modal.vue'; import { createStore } from '~/ide/stores'; +import { stubComponent } from 'helpers/stub_component'; +import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; +import { createEntriesFromPaths } from '../../helpers'; jest.mock('~/flash'); +const NEW_NAME = 'babar'; + describe('new file modal component', () => { - const Component = Vue.extend(modal); - let vm; + const showModal = jest.fn(); + const toggleModal = jest.fn(); + + let store; + let wrapper; + + const findGlModal = () => wrapper.findComponent(GlModal); + const findInput = () => wrapper.findByTestId('file-name-field'); + const findTemplateButtons = () => wrapper.findAllComponents(GlButton); + const findTemplateButtonsModel = () => + findTemplateButtons().wrappers.map((x) => ({ + text: x.text(), + variant: x.props('variant'), + category: x.props('category'), + })); + + const open = (type, path) => { + // TODO: This component can not be passed props + // We have to interact with the open() method? + wrapper.vm.open(type, path); + }; + const triggerSubmit = () => { + findGlModal().vm.$emit('primary'); + }; + const triggerCancel = () => { + findGlModal().vm.$emit('cancel'); + }; + + const mountComponent = () => { + const GlModalStub = stubComponent(GlModal); + jest.spyOn(GlModalStub.methods, 'show').mockImplementation(showModal); + jest.spyOn(GlModalStub.methods, 'toggle').mockImplementation(toggleModal); + + wrapper = shallowMountExtended(Modal, { + store, + stubs: { + GlModal: GlModalStub, + }, + // We need to attach to document for "focus" to work + attachTo: document.body, + }); + }; + + beforeEach(() => { + store = createStore(); + + Object.assign( + store.state.entries, + createEntriesFromPaths([ + 'README.md', + 'src', + 'src/deleted.js', + 'src/parent_dir', + 'src/parent_dir/foo.js', + ]), + ); + Object.assign(store.state.entries['src/deleted.js'], { deleted: true }); + + jest.spyOn(store, 'dispatch').mockImplementation(); + }); afterEach(() => { - vm.$destroy(); + store = null; + wrapper.destroy(); + document.body.innerHTML = ''; }); - describe.each` - entryType | modalTitle | btnTitle | showsFileTemplates - ${'tree'} | ${'Create new directory'} | ${'Create directory'} | ${false} - ${'blob'} | ${'Create new file'} | ${'Create file'} | ${true} - `('$entryType', ({ entryType, modalTitle, btnTitle, showsFileTemplates }) => { + describe('default', () => { beforeEach(async () => { - const store = createStore(); - - vm = createComponentWithStore(Component, store).$mount(); - vm.open(entryType); - vm.name = 'testing'; + mountComponent(); + // Not necessarily needed, but used to ensure that nothing extra is happening after the tick await nextTick(); }); - afterEach(() => { - vm.close(); + it('renders modal', () => { + expect(findGlModal().props()).toMatchObject({ + actionCancel: { + attributes: [{ variant: 'default' }], + text: 'Cancel', + }, + actionPrimary: { + attributes: [{ variant: 'confirm' }], + text: 'Create file', + }, + actionSecondary: null, + size: 'lg', + modalId: 'ide-new-entry', + title: 'Create new file', + }); }); - it(`sets modal title as ${entryType}`, () => { - expect(document.querySelector('.modal-title').textContent.trim()).toBe(modalTitle); + it('renders name label', () => { + expect(wrapper.find('label').text()).toBe('Name'); }); - it(`sets button label as ${entryType}`, () => { - expect(document.querySelector('.btn-confirm').textContent.trim()).toBe(btnTitle); + it('renders template buttons', () => { + const actual = findTemplateButtonsModel(); + + expect(actual.length).toBeGreaterThan(0); + expect(actual).toEqual( + store.getters['fileTemplates/templateTypes'].map((template) => ({ + category: 'secondary', + text: template.name, + variant: 'dashed', + })), + ); }); - it(`sets form label as ${entryType}`, () => { - expect(document.querySelector('.label-bold').textContent.trim()).toBe('Name'); + // These negative ".not.toHaveBeenCalled" assertions complement the positive "toHaveBeenCalled" + // assertions that show up later in this spec. Without these, we're not guaranteed the "act" + // actually caused the change in behavior. + it('does not dispatch actions by default', () => { + expect(store.dispatch).not.toHaveBeenCalled(); }); - it(`shows file templates: ${showsFileTemplates}`, () => { - const templateFilesEl = document.querySelector('.file-templates'); - expect(Boolean(templateFilesEl)).toBe(showsFileTemplates); + it('does not trigger modal by default', () => { + expect(showModal).not.toHaveBeenCalled(); + expect(toggleModal).not.toHaveBeenCalled(); }); - }); - describe('rename entry', () => { - beforeEach(() => { - const store = createStore(); - store.state.entries = { - 'test-path': { - name: 'test', - type: 'blob', - path: 'test-path', - }, - }; - - vm = createComponentWithStore(Component, store).$mount(); + it('does not focus input by default', () => { + expect(document.activeElement).toBe(document.body); }); + }); - it.each` - entryType | modalTitle | btnTitle - ${'tree'} | ${'Rename folder'} | ${'Rename folder'} - ${'blob'} | ${'Rename file'} | ${'Rename file'} - `( - 'renders title and button for renaming $entryType', - async ({ entryType, modalTitle, btnTitle }) => { - vm.$store.state.entries['test-path'].type = entryType; - vm.open('rename', 'test-path'); + describe.each` + entryType | path | modalTitle | btnTitle | showsFileTemplates | inputValue | inputPlaceholder + ${'tree'} | ${''} | ${'Create new directory'} | ${'Create directory'} | ${false} | ${''} | ${'dir/'} + ${'blob'} | ${''} | ${'Create new file'} | ${'Create file'} | ${true} | ${''} | ${'dir/file_name'} + ${'blob'} | ${'foo/bar'} | ${'Create new file'} | ${'Create file'} | ${true} | ${'foo/bar/'} | ${'dir/file_name'} + `( + 'when opened as $entryType with path "$path"', + ({ + entryType, + path, + modalTitle, + btnTitle, + showsFileTemplates, + inputValue, + inputPlaceholder, + }) => { + beforeEach(async () => { + mountComponent(); + + open(entryType, path); await nextTick(); - expect(document.querySelector('.modal-title').textContent.trim()).toBe(modalTitle); - expect(document.querySelector('.btn-confirm').textContent.trim()).toBe(btnTitle); - }, - ); + }); - describe('entryName', () => { - it('returns entries name', () => { - vm.open('rename', 'test-path'); + it('sets modal props', () => { + expect(findGlModal().props()).toMatchObject({ + title: modalTitle, + actionPrimary: { + attributes: [{ variant: 'confirm' }], + text: btnTitle, + }, + }); + }); - expect(vm.entryName).toBe('test-path'); + it('sets input attributes', () => { + expect(findInput().element.value).toBe(inputValue); + expect(findInput().attributes('placeholder')).toBe(inputPlaceholder); }); - it('does not reset entryName to its old value if empty', () => { - vm.entryName = 'hello'; - vm.entryName = ''; + it(`shows file templates: ${showsFileTemplates}`, () => { + const actual = findTemplateButtonsModel().length > 0; - expect(vm.entryName).toBe(''); + expect(actual).toBe(showsFileTemplates); }); - }); - describe('open', () => { - it('sets entryName to path provided if modalType is rename', () => { - vm.open('rename', 'test-path'); + it('shows modal', () => { + expect(showModal).toHaveBeenCalled(); + }); - expect(vm.entryName).toBe('test-path'); + it('focus on input', () => { + expect(document.activeElement).toBe(findInput().element); }); - it("appends '/' to the path if modalType isn't rename", () => { - vm.open('blob', 'test-path'); + it('resets when canceled', async () => { + triggerCancel(); + + await nextTick(); - expect(vm.entryName).toBe('test-path/'); + // Resets input value + expect(findInput().element.value).toBe(''); + // Resets to blob mode + expect(findGlModal().props('title')).toBe('Create new file'); }); + }, + ); + + describe.each` + modalType | name | expectedName + ${'blob'} | ${'foo/bar.js'} | ${'foo/bar.js'} + ${'blob'} | ${'foo /bar.js'} | ${'foo/bar.js'} + ${'tree'} | ${'foo/dir'} | ${'foo/dir'} + ${'tree'} | ${'foo /dir'} | ${'foo/dir'} + `('when submitting as $modalType with "$name"', ({ modalType, name, expectedName }) => { + beforeEach(async () => { + mountComponent(); + + open(modalType, ''); + await nextTick(); - it('leaves entryName blank if no path is provided', () => { - vm.open('blob'); + findInput().setValue(name); + triggerSubmit(); + }); - expect(vm.entryName).toBe(''); + it('triggers createTempEntry action', () => { + expect(store.dispatch).toHaveBeenCalledWith('createTempEntry', { + name: expectedName, + type: modalType, }); }); }); - describe('createFromTemplate', () => { - let store; + describe('when creating from template type', () => { + beforeEach(async () => { + mountComponent(); - beforeEach(() => { - store = createStore(); - store.state.entries = { - 'test-path/test': { - name: 'test', - deleted: false, - }, - }; + open('blob', 'some_dir'); - vm = createComponentWithStore(Component, store).$mount(); - vm.open('blob'); + await nextTick(); - jest.spyOn(vm, 'createTempEntry').mockImplementation(); + // Set input, then trigger button + findInput().setValue('some_dir/foo.js'); + findTemplateButtons().at(1).vm.$emit('click'); }); - it.each` - entryName | newFilePath - ${''} | ${'.gitignore'} - ${'README.md'} | ${'.gitignore'} - ${'test-path/test/'} | ${'test-path/test/.gitignore'} - ${'test-path/test'} | ${'test-path/.gitignore'} - ${'test-path/test/abc.md'} | ${'test-path/test/.gitignore'} - `( - 'creates a new file with the given template name in appropriate directory for path: $path', - ({ entryName, newFilePath }) => { - vm.entryName = entryName; + it('triggers createTempEntry action', () => { + const { name: expectedName } = store.getters['fileTemplates/templateTypes'][1]; - vm.createFromTemplate({ name: '.gitignore' }); + expect(store.dispatch).toHaveBeenCalledWith('createTempEntry', { + name: `some_dir/${expectedName}`, + type: 'blob', + }); + }); - expect(vm.createTempEntry).toHaveBeenCalledWith({ - name: newFilePath, - type: 'blob', - }); - }, - ); + it('toggles modal', () => { + expect(toggleModal).toHaveBeenCalled(); + }); }); - describe('submitForm', () => { - let store; + describe.each` + origPath | title | inputValue | inputSelectionStart + ${'src/parent_dir'} | ${'Rename folder'} | ${'src/parent_dir'} | ${'src/'.length} + ${'README.md'} | ${'Rename file'} | ${'README.md'} | ${0} + `('when renaming for $origPath', ({ origPath, title, inputValue, inputSelectionStart }) => { + beforeEach(async () => { + mountComponent(); + + open('rename', origPath); + + await nextTick(); + }); - beforeEach(() => { - store = createStore(); - store.state.entries = { - 'test-path/test': { - name: 'test', - deleted: false, + it('sets modal props for renaming', () => { + expect(findGlModal().props()).toMatchObject({ + title, + actionPrimary: { + attributes: [{ variant: 'confirm' }], + text: title, }, - }; + }); + }); - vm = createComponentWithStore(Component, store).$mount(); + it('sets input value', () => { + expect(findInput().element.value).toBe(inputValue); }); - it('throws an error when target entry exists', () => { - vm.open('rename', 'test-path/test'); + it(`does not show file templates`, () => { + expect(findTemplateButtonsModel()).toHaveLength(0); + }); - expect(createFlash).not.toHaveBeenCalled(); + it('shows modal when renaming', () => { + expect(showModal).toHaveBeenCalled(); + }); + + it('focus on input when renaming', () => { + expect(document.activeElement).toBe(findInput().element); + }); + + it('selects name part of the input', () => { + expect(findInput().element.selectionStart).toBe(inputSelectionStart); + expect(findInput().element.selectionEnd).toBe(origPath.length); + }); + + describe('when renames is submitted successfully', () => { + beforeEach(() => { + findInput().setValue(NEW_NAME); + triggerSubmit(); + }); + + it('dispatches renameEntry event', () => { + expect(store.dispatch).toHaveBeenCalledWith('renameEntry', { + path: origPath, + parentPath: '', + name: NEW_NAME, + }); + }); - vm.submitForm(); + it('does not trigger flash', () => { + expect(createFlash).not.toHaveBeenCalled(); + }); + }); + }); + + describe('when renaming and file already exists', () => { + beforeEach(async () => { + mountComponent(); + + open('rename', 'src/parent_dir'); + + await nextTick(); + + // Set to something that already exists! + findInput().setValue('src'); + triggerSubmit(); + }); + it('creates flash', () => { expect(createFlash).toHaveBeenCalledWith({ - message: 'The name "test-path/test" is already taken in this directory.', + message: 'The name "src" is already taken in this directory.', fadeTransition: false, addBodyClass: true, }); }); - it('does not throw error when target entry does not exist', () => { - jest.spyOn(vm, 'renameEntry').mockImplementation(); + it('does not dispatch event', () => { + expect(store.dispatch).not.toHaveBeenCalled(); + }); + }); - vm.open('rename', 'test-path/test'); - vm.entryName = 'test-path/test2'; - vm.submitForm(); + describe('when renaming and file has been deleted', () => { + beforeEach(async () => { + mountComponent(); - expect(createFlash).not.toHaveBeenCalled(); - }); + open('rename', 'src/parent_dir/foo.js'); - it('removes leading/trailing found in the new name', () => { - vm.open('rename', 'test-path/test'); + await nextTick(); - vm.entryName = 'test-path /test'; + findInput().setValue('src/deleted.js'); + triggerSubmit(); + }); - vm.submitForm(); + it('does not create flash', () => { + expect(createFlash).not.toHaveBeenCalled(); + }); - expect(vm.entryName).toBe('test-path/test'); + it('dispatches event', () => { + expect(store.dispatch).toHaveBeenCalledWith('renameEntry', { + path: 'src/parent_dir/foo.js', + name: 'deleted.js', + parentPath: 'src', + }); }); }); }); diff --git a/spec/graphql/resolvers/ci/job_token_scope_resolver_spec.rb b/spec/graphql/resolvers/ci/job_token_scope_resolver_spec.rb index 59616815de0..ac7cef20df4 100644 --- a/spec/graphql/resolvers/ci/job_token_scope_resolver_spec.rb +++ b/spec/graphql/resolvers/ci/job_token_scope_resolver_spec.rb @@ -17,7 +17,7 @@ RSpec.describe Resolvers::Ci::JobTokenScopeResolver do describe '#resolve' do context 'with access to scope' do before do - project.add_user(current_user, :maintainer) + project.add_member(current_user, :maintainer) end it 'returns nil when scope is not enabled' do @@ -51,7 +51,7 @@ RSpec.describe Resolvers::Ci::JobTokenScopeResolver do context 'without access to scope' do before do - project.add_user(current_user, :developer) + project.add_member(current_user, :developer) end it 'generates an error' do diff --git a/spec/graphql/resolvers/container_repositories_resolver_spec.rb b/spec/graphql/resolvers/container_repositories_resolver_spec.rb index d7aa761320f..ed922259903 100644 --- a/spec/graphql/resolvers/container_repositories_resolver_spec.rb +++ b/spec/graphql/resolvers/container_repositories_resolver_spec.rb @@ -62,7 +62,7 @@ RSpec.describe Resolvers::ContainerRepositoriesResolver do context 'with authorized user' do before do - group.add_user(user, :maintainer) + group.add_member(user, :maintainer) end context 'when the object is a project' do diff --git a/spec/graphql/types/ci/job_token_scope_type_spec.rb b/spec/graphql/types/ci/job_token_scope_type_spec.rb index 43225b2089b..c1a3c4dd54d 100644 --- a/spec/graphql/types/ci/job_token_scope_type_spec.rb +++ b/spec/graphql/types/ci/job_token_scope_type_spec.rb @@ -38,7 +38,7 @@ RSpec.describe GitlabSchema.types['CiJobTokenScopeType'] do context 'with access to scope' do before do - project.add_user(current_user, :maintainer) + project.add_member(current_user, :maintainer) end context 'when multiple projects in the allow list' do @@ -46,7 +46,7 @@ RSpec.describe GitlabSchema.types['CiJobTokenScopeType'] do context 'when linked projects are readable' do before do - link.target_project.add_user(current_user, :developer) + link.target_project.add_member(current_user, :developer) end it 'returns readable projects in scope' do diff --git a/spec/helpers/diff_helper_spec.rb b/spec/helpers/diff_helper_spec.rb index cf16807723b..b6efced698d 100644 --- a/spec/helpers/diff_helper_spec.rb +++ b/spec/helpers/diff_helper_spec.rb @@ -471,21 +471,60 @@ RSpec.describe DiffHelper do describe '#conflicts' do let(:merge_request) { instance_double(MergeRequest) } + let(:merge_ref_head_diff) { true } + let(:can_be_resolved_in_ui?) { true } + let(:allow_tree_conflicts) { false } + let(:files) { [instance_double(Gitlab::Conflict::File, path: 'a')] } + let(:exception) { nil } before do allow(helper).to receive(:merge_request).and_return(merge_request) - allow(helper).to receive(:options).and_return(merge_ref_head_diff: true) + allow(helper).to receive(:options).and_return(merge_ref_head_diff: merge_ref_head_diff) + + allow_next_instance_of(MergeRequests::Conflicts::ListService, merge_request, allow_tree_conflicts: allow_tree_conflicts) do |svc| + allow(svc).to receive(:can_be_resolved_in_ui?).and_return(can_be_resolved_in_ui?) + + if exception.present? + allow(svc).to receive_message_chain(:conflicts, :files).and_raise(exception) + else + allow(svc).to receive_message_chain(:conflicts, :files).and_return(files) + end + end end - context 'when Gitlab::Git::Conflict::Resolver::ConflictSideMissing exception is raised' do - before do - allow_next_instance_of(MergeRequests::Conflicts::ListService, merge_request, allow_tree_conflicts: true) do |svc| - allow(svc).to receive_message_chain(:conflicts, :files).and_raise(Gitlab::Git::Conflict::Resolver::ConflictSideMissing) + it 'returns list of conflicts indexed by path' do + expect(helper.conflicts).to eq('a' => files.first) + end + + context 'when merge_ref_head_diff option is false' do + let(:merge_ref_head_diff) { false } + + it 'returns nil' do + expect(helper.conflicts).to be_nil + end + end + + context 'when conflicts cannot be resolved in UI' do + let(:can_be_resolved_in_ui?) { false } + + it 'returns nil' do + expect(helper.conflicts).to be_nil + end + + context 'when allow_tree_conflicts is true' do + let(:allow_tree_conflicts) { true } + + it 'returns list of conflicts' do + expect(helper.conflicts(allow_tree_conflicts: allow_tree_conflicts)).to eq('a' => files.first) end end + end + + context 'when Gitlab::Git::Conflict::Resolver::ConflictSideMissing exception is raised' do + let(:exception) { Gitlab::Git::Conflict::Resolver::ConflictSideMissing } it 'returns an empty hash' do - expect(helper.conflicts(allow_tree_conflicts: true)).to eq({}) + expect(helper.conflicts).to eq({}) end end end diff --git a/spec/helpers/namespace_storage_limit_alert_helper_spec.rb b/spec/helpers/namespace_storage_limit_alert_helper_spec.rb deleted file mode 100644 index ab3cf96edef..00000000000 --- a/spec/helpers/namespace_storage_limit_alert_helper_spec.rb +++ /dev/null @@ -1,11 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe NamespaceStorageLimitAlertHelper do - describe '#display_namespace_storage_limit_alert!' do - it 'is defined in CE' do - expect { helper.display_namespace_storage_limit_alert! }.not_to raise_error - end - end -end diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb index 4502729866c..42c97b61d19 100644 --- a/spec/helpers/projects_helper_spec.rb +++ b/spec/helpers/projects_helper_spec.rb @@ -698,7 +698,7 @@ RSpec.describe ProjectsHelper do def grant_user_access(project, user, access) case access when :developer, :maintainer - project.add_user(user, access) + project.add_member(user, access) when :owner project.namespace.update!(owner: user) end diff --git a/spec/helpers/todos_helper_spec.rb b/spec/helpers/todos_helper_spec.rb index 73a454fd9f7..bbabfedc3ee 100644 --- a/spec/helpers/todos_helper_spec.rb +++ b/spec/helpers/todos_helper_spec.rb @@ -133,6 +133,16 @@ RSpec.describe TodosHelper do expect(path).to eq("/#{todo.project.full_path}/-/work_items/#{todo.target.id}") end end + + context 'when given an issue with a note anchor' do + let(:todo) { create(:todo, project: issue.project, target: issue, note: note) } + + it 'responds with an appropriate path' do + path = helper.todo_target_path(todo) + + expect(path).to eq("/#{issue.project.full_path}/-/issues/#{issue.iid}##{dom_id(note)}") + end + end end describe '#todo_target_type_name' do diff --git a/spec/lib/banzai/reference_parser/snippet_parser_spec.rb b/spec/lib/banzai/reference_parser/snippet_parser_spec.rb index 3459784708f..e8ef4e7f6e3 100644 --- a/spec/lib/banzai/reference_parser/snippet_parser_spec.rb +++ b/spec/lib/banzai/reference_parser/snippet_parser_spec.rb @@ -24,7 +24,7 @@ RSpec.describe Banzai::ReferenceParser::SnippetParser do end before do - project.add_user(project_member, :developer) + project.add_member(project_member, :developer) end describe '#nodes_visible_to_user' do diff --git a/spec/lib/gitlab/analytics/cycle_analytics/records_fetcher_spec.rb b/spec/lib/gitlab/analytics/cycle_analytics/records_fetcher_spec.rb index ec394bb9f05..34d5158a5ab 100644 --- a/spec/lib/gitlab/analytics/cycle_analytics/records_fetcher_spec.rb +++ b/spec/lib/gitlab/analytics/cycle_analytics/records_fetcher_spec.rb @@ -25,7 +25,7 @@ RSpec.describe Gitlab::Analytics::CycleAnalytics::RecordsFetcher do describe '#serialized_records' do shared_context 'when records are loaded by maintainer' do before do - project.add_user(user, Gitlab::Access::DEVELOPER) + project.add_member(user, Gitlab::Access::DEVELOPER) end it 'returns all records' do @@ -72,7 +72,7 @@ RSpec.describe Gitlab::Analytics::CycleAnalytics::RecordsFetcher do context 'when records are loaded by guest' do before do - project.add_user(user, Gitlab::Access::GUEST) + project.add_member(user, Gitlab::Access::GUEST) end it 'filters out confidential issues' do @@ -124,7 +124,7 @@ RSpec.describe Gitlab::Analytics::CycleAnalytics::RecordsFetcher do end before do - project.add_user(user, Gitlab::Access::DEVELOPER) + project.add_member(user, Gitlab::Access::DEVELOPER) stub_const('Gitlab::Analytics::CycleAnalytics::RecordsFetcher::MAX_RECORDS', 2) end diff --git a/spec/lib/gitlab/ci/variables/builder_spec.rb b/spec/lib/gitlab/ci/variables/builder_spec.rb index b0704ad7f50..8ec0846bdca 100644 --- a/spec/lib/gitlab/ci/variables/builder_spec.rb +++ b/spec/lib/gitlab/ci/variables/builder_spec.rb @@ -166,9 +166,8 @@ RSpec.describe Gitlab::Ci::Variables::Builder do allow(builder).to receive(:secret_instance_variables) { [var('J', 10), var('K', 10)] } allow(builder).to receive(:secret_group_variables) { [var('K', 11), var('L', 11)] } allow(builder).to receive(:secret_project_variables) { [var('L', 12), var('M', 12)] } - allow(job).to receive(:trigger_request) { double(user_variables: [var('M', 13), var('N', 13)]) } - allow(pipeline).to receive(:variables) { [var('N', 14), var('O', 14)] } - allow(pipeline).to receive(:pipeline_schedule) { double(job_variables: [var('O', 15), var('P', 15)]) } + allow(pipeline).to receive(:variables) { [var('M', 13), var('N', 13)] } + allow(pipeline).to receive(:pipeline_schedule) { double(job_variables: [var('N', 14), var('O', 14)]) } end it 'returns variables in order depending on resource hierarchy' do @@ -185,8 +184,7 @@ RSpec.describe Gitlab::Ci::Variables::Builder do var('K', 11), var('L', 11), var('L', 12), var('M', 12), var('M', 13), var('N', 13), - var('N', 14), var('O', 14), - var('O', 15), var('P', 15)]) + var('N', 14), var('O', 14)]) end it 'overrides duplicate keys depending on resource hierarchy' do @@ -198,7 +196,7 @@ RSpec.describe Gitlab::Ci::Variables::Builder do 'I' => '9', 'J' => '10', 'K' => '11', 'L' => '12', 'M' => '13', 'N' => '14', - 'O' => '15', 'P' => '15') + 'O' => '14') end end diff --git a/spec/lib/gitlab/database_importers/instance_administrators/create_group_spec.rb b/spec/lib/gitlab/database_importers/instance_administrators/create_group_spec.rb index e676e5fe034..2878d72210e 100644 --- a/spec/lib/gitlab/database_importers/instance_administrators/create_group_spec.rb +++ b/spec/lib/gitlab/database_importers/instance_administrators/create_group_spec.rb @@ -109,7 +109,7 @@ RSpec.describe Gitlab::DatabaseImporters::InstanceAdministrators::CreateGroup do admin2 = create(:user, :admin) existing_group.add_owner(user) - existing_group.add_users([admin1, admin2], Gitlab::Access::MAINTAINER) + existing_group.add_members([admin1, admin2], Gitlab::Access::MAINTAINER) application_setting.instance_administrators_group_id = existing_group.id end diff --git a/spec/lib/gitlab/import_export/json/streaming_serializer_spec.rb b/spec/lib/gitlab/import_export/json/streaming_serializer_spec.rb index 03f522ae490..3f73a730744 100644 --- a/spec/lib/gitlab/import_export/json/streaming_serializer_spec.rb +++ b/spec/lib/gitlab/import_export/json/streaming_serializer_spec.rb @@ -171,4 +171,27 @@ RSpec.describe Gitlab::ImportExport::Json::StreamingSerializer do expect(described_class.batch_size(exportable)).to eq(described_class::BATCH_SIZE) end end + + describe '#serialize_relation' do + context 'when record is a merge request' do + let(:json_writer) do + Class.new do + def write_relation_array(_, _, enumerator) + enumerator.each { _1 } + end + end.new + end + + it 'removes cached external diff' do + merge_request = create(:merge_request, source_project: exportable, target_project: exportable) + cache_dir = merge_request.merge_request_diff.send(:external_diff_cache_dir) + + expect(subject).to receive(:remove_cached_external_diff).with(merge_request).twice + + subject.serialize_relation({ merge_requests: { include: [] } }) + + expect(Dir.exist?(cache_dir)).to eq(false) + end + end + end end diff --git a/spec/lib/gitlab/import_export/members_mapper_spec.rb b/spec/lib/gitlab/import_export/members_mapper_spec.rb index 87ca899a87d..d7ad34255c1 100644 --- a/spec/lib/gitlab/import_export/members_mapper_spec.rb +++ b/spec/lib/gitlab/import_export/members_mapper_spec.rb @@ -258,7 +258,7 @@ RSpec.describe Gitlab::ImportExport::MembersMapper do end before do - group.add_users([user, user2], GroupMember::DEVELOPER) + group.add_members([user, user2], GroupMember::DEVELOPER) end it 'maps the project member' do @@ -281,7 +281,7 @@ RSpec.describe Gitlab::ImportExport::MembersMapper do end before do - group.add_users([user, user2], GroupMember::DEVELOPER) + group.add_members([user, user2], GroupMember::DEVELOPER) end it 'maps the importer' do @@ -315,7 +315,7 @@ RSpec.describe Gitlab::ImportExport::MembersMapper do shared_examples_for 'it fetches the access level from parent group' do before do - group.add_users([user], group_access_level) + group.add_members([user], group_access_level) end it "and resolves it correctly" do diff --git a/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb b/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb index 95a995b1d5e..fac960d8df2 100644 --- a/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb @@ -875,7 +875,7 @@ RSpec.describe Gitlab::ImportExport::Project::TreeRestorer do context 'with group visibility' do before do group = create(:group, visibility_level: group_visibility) - group.add_users([user], GroupMember::MAINTAINER) + group.add_members([user], GroupMember::MAINTAINER) project.update!(group: group) end diff --git a/spec/lib/gitlab/quick_actions/users_extractor_spec.rb b/spec/lib/gitlab/quick_actions/users_extractor_spec.rb new file mode 100644 index 00000000000..d00f52bb056 --- /dev/null +++ b/spec/lib/gitlab/quick_actions/users_extractor_spec.rb @@ -0,0 +1,93 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::QuickActions::UsersExtractor do + subject(:extractor) { described_class.new(current_user, project: project, group: group, target: target, text: text) } + + let_it_be(:current_user) { create(:user) } + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, group: group) } + let_it_be(:target) { create(:issue, project: project) } + + let_it_be(:pancakes) { create(:user, username: 'pancakes') } + let_it_be(:waffles) { create(:user, username: 'waffles') } + let_it_be(:syrup) { create(:user, username: 'syrup') } + + before do + allow(target).to receive(:allows_multiple_assignees?).and_return(false) + end + + context 'when the text is nil' do + let(:text) { nil } + + it 'returns an empty array' do + expect(extractor.execute).to be_empty + end + end + + context 'when the text is blank' do + let(:text) { ' ' } + + it 'returns an empty array' do + expect(extractor.execute).to be_empty + end + end + + context 'when there are users to be found' do + context 'when using usernames' do + let(:text) { 'me, pancakes waffles and syrup' } + + it 'finds the users' do + expect(extractor.execute).to contain_exactly(current_user, pancakes, waffles, syrup) + end + end + + context 'when there are too many users' do + let(:text) { 'me, pancakes waffles and syrup' } + + before do + stub_const("#{described_class}::MAX_QUICK_ACTION_USERS", 2) + end + + it 'complains' do + expect { extractor.execute }.to raise_error(described_class::TooManyError) + end + end + + context 'when using references' do + let(:text) { 'me, @pancakes @waffles and @syrup' } + + it 'finds the users' do + expect(extractor.execute).to contain_exactly(current_user, pancakes, waffles, syrup) + end + end + + context 'when using a mixture of usernames and references' do + let(:text) { 'me, @pancakes waffles and @syrup' } + + it 'finds the users' do + expect(extractor.execute).to contain_exactly(current_user, pancakes, waffles, syrup) + end + end + + context 'when one or more users cannot be found' do + let(:text) { 'me, @bacon @pancakes, chicken waffles and @syrup' } + + it 'reports an error' do + expect { extractor.execute }.to raise_error(described_class::MissingError, include('bacon', 'chicken')) + end + end + + context 'when trying to find group members' do + let(:group) { create(:group, path: 'breakfast-foods') } + let(:text) { group.to_reference } + + it 'reports an error' do + [pancakes, waffles].each { group.add_developer(_1) } + + expect { extractor.execute }.to raise_error(described_class::MissingError, include('breakfast-foods')) + end + end + end +end diff --git a/spec/lib/gitlab/user_access_spec.rb b/spec/lib/gitlab/user_access_spec.rb index b1de3e21b77..1ae45d41f2d 100644 --- a/spec/lib/gitlab/user_access_spec.rb +++ b/spec/lib/gitlab/user_access_spec.rb @@ -219,19 +219,19 @@ RSpec.describe Gitlab::UserAccess do describe '#can_create_tag?' do describe 'push to none protected tag' do it 'returns true if user is a maintainer' do - project.add_user(user, :maintainer) + project.add_member(user, :maintainer) expect(access.can_create_tag?('random_tag')).to be_truthy end it 'returns true if user is a developer' do - project.add_user(user, :developer) + project.add_member(user, :developer) expect(access.can_create_tag?('random_tag')).to be_truthy end it 'returns false if user is a reporter' do - project.add_user(user, :reporter) + project.add_member(user, :reporter) expect(access.can_create_tag?('random_tag')).to be_falsey end @@ -242,19 +242,19 @@ RSpec.describe Gitlab::UserAccess do let(:not_existing_tag) { create :protected_tag, project: project } it 'returns true if user is a maintainer' do - project.add_user(user, :maintainer) + project.add_member(user, :maintainer) expect(access.can_create_tag?(tag.name)).to be_truthy end it 'returns false if user is a developer' do - project.add_user(user, :developer) + project.add_member(user, :developer) expect(access.can_create_tag?(tag.name)).to be_falsey end it 'returns false if user is a reporter' do - project.add_user(user, :reporter) + project.add_member(user, :reporter) expect(access.can_create_tag?(tag.name)).to be_falsey end @@ -266,19 +266,19 @@ RSpec.describe Gitlab::UserAccess do end it 'returns true if user is a maintainer' do - project.add_user(user, :maintainer) + project.add_member(user, :maintainer) expect(access.can_create_tag?(@tag.name)).to be_truthy end it 'returns true if user is a developer' do - project.add_user(user, :developer) + project.add_member(user, :developer) expect(access.can_create_tag?(@tag.name)).to be_truthy end it 'returns false if user is a reporter' do - project.add_user(user, :reporter) + project.add_member(user, :reporter) expect(access.can_create_tag?(@tag.name)).to be_falsey end @@ -288,19 +288,19 @@ RSpec.describe Gitlab::UserAccess do describe '#can_delete_branch?' do describe 'delete unprotected branch' do it 'returns true if user is a maintainer' do - project.add_user(user, :maintainer) + project.add_member(user, :maintainer) expect(access.can_delete_branch?('random_branch')).to be_truthy end it 'returns true if user is a developer' do - project.add_user(user, :developer) + project.add_member(user, :developer) expect(access.can_delete_branch?('random_branch')).to be_truthy end it 'returns false if user is a reporter' do - project.add_user(user, :reporter) + project.add_member(user, :reporter) expect(access.can_delete_branch?('random_branch')).to be_falsey end @@ -310,19 +310,19 @@ RSpec.describe Gitlab::UserAccess do let(:branch) { create(:protected_branch, project: project, name: "test") } it 'returns true if user is a maintainer' do - project.add_user(user, :maintainer) + project.add_member(user, :maintainer) expect(access.can_delete_branch?(branch.name)).to be_truthy end it 'returns false if user is a developer' do - project.add_user(user, :developer) + project.add_member(user, :developer) expect(access.can_delete_branch?(branch.name)).to be_falsey end it 'returns false if user is a reporter' do - project.add_user(user, :reporter) + project.add_member(user, :reporter) expect(access.can_delete_branch?(branch.name)).to be_falsey end @@ -334,7 +334,7 @@ RSpec.describe Gitlab::UserAccess do context 'when user cannot push_code to a project repository (eg. as a guest)' do it 'is false' do - project.add_user(user, :guest) + project.add_member(user, :guest) expect(access.can_push_for_ref?(ref)).to be_falsey end @@ -342,7 +342,7 @@ RSpec.describe Gitlab::UserAccess do context 'when user can push_code to a project repository (eg. as a developer)' do it 'is true' do - project.add_user(user, :developer) + project.add_member(user, :developer) expect(access.can_push_for_ref?(ref)).to be_truthy end diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index a9796c28870..f501488bb8c 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -1550,7 +1550,7 @@ RSpec.describe Notify do end describe 'invitations' do - let(:owner) { create(:user).tap { |u| group.add_user(u, Gitlab::Access::OWNER) } } + let(:owner) { create(:user).tap { |u| group.add_member(u, Gitlab::Access::OWNER) } } let(:group_member) { invite_to_group(group, inviter: inviter) } let(:inviter) { owner } @@ -1605,7 +1605,7 @@ RSpec.describe Notify do end describe 'group invitation reminders' do - let_it_be(:inviter) { create(:user).tap { |u| group.add_user(u, Gitlab::Access::OWNER) } } + let_it_be(:inviter) { create(:user).tap { |u| group.add_member(u, Gitlab::Access::OWNER) } } let(:group_member) { invite_to_group(group, inviter: inviter) } @@ -1688,7 +1688,7 @@ RSpec.describe Notify do describe 'group invitation accepted' do let(:invited_user) { create(:user, name: 'invited user') } - let(:owner) { create(:user).tap { |u| group.add_user(u, Gitlab::Access::OWNER) } } + let(:owner) { create(:user).tap { |u| group.add_member(u, Gitlab::Access::OWNER) } } let(:group_member) do invitee = invite_to_group(group, inviter: owner) invitee.accept_invite!(invited_user) @@ -1714,7 +1714,7 @@ RSpec.describe Notify do end describe 'group invitation declined' do - let(:owner) { create(:user).tap { |u| group.add_user(u, Gitlab::Access::OWNER) } } + let(:owner) { create(:user).tap { |u| group.add_member(u, Gitlab::Access::OWNER) } } let(:group_member) do invitee = invite_to_group(group, inviter: owner) invitee.decline_invite! diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index d3501d9bb1b..fef28e7c550 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -3251,10 +3251,6 @@ RSpec.describe Ci::Build do let(:trigger) { create(:ci_trigger, project: project) } let(:trigger_request) { create(:ci_trigger_request, pipeline: pipeline, trigger: trigger) } - let(:user_trigger_variable) do - { key: 'TRIGGER_KEY_1', value: 'TRIGGER_VALUE_1', public: false, masked: false } - end - let(:predefined_trigger_variable) do { key: 'CI_PIPELINE_TRIGGERED', value: 'true', public: true, masked: false } end @@ -3263,26 +3259,7 @@ RSpec.describe Ci::Build do build.trigger_request = trigger_request end - shared_examples 'returns variables for triggers' do - it { is_expected.to include(user_trigger_variable) } - it { is_expected.to include(predefined_trigger_variable) } - end - - context 'when variables are stored in trigger_request' do - before do - trigger_request.update_attribute(:variables, { 'TRIGGER_KEY_1' => 'TRIGGER_VALUE_1' } ) - end - - it_behaves_like 'returns variables for triggers' - end - - context 'when variables are stored in pipeline_variables' do - before do - create(:ci_pipeline_variable, pipeline: pipeline, key: 'TRIGGER_KEY_1', value: 'TRIGGER_VALUE_1') - end - - it_behaves_like 'returns variables for triggers' - end + it { is_expected.to include(predefined_trigger_variable) } end context 'when pipeline has a variable' do diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index d47f43a630d..91ba12a16d5 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -726,7 +726,7 @@ RSpec.describe Group do context 'when user is a member of private group' do before do - private_group.add_user(user, Gitlab::Access::DEVELOPER) + private_group.add_member(user, Gitlab::Access::DEVELOPER) end it { is_expected.to match_array([private_group, internal_group, group]) } @@ -736,7 +736,7 @@ RSpec.describe Group do let!(:private_subgroup) { create(:group, :private, parent: private_group) } before do - private_subgroup.add_user(user, Gitlab::Access::DEVELOPER) + private_subgroup.add_member(user, Gitlab::Access::DEVELOPER) end it { is_expected.to match_array([private_subgroup, internal_group, group]) } @@ -848,7 +848,7 @@ RSpec.describe Group do expect(member).to receive(:refresh_member_authorized_projects).with(blocking: true) end - group.add_user(user, GroupMember::MAINTAINER) + group.add_member(user, GroupMember::MAINTAINER) expect(group.group_members.maintainers.map(&:user)).to include(user) end @@ -858,7 +858,7 @@ RSpec.describe Group do expect(member).to receive(:refresh_member_authorized_projects).with(blocking: false) end - group.add_user(user, GroupMember::MAINTAINER, blocking_refresh: false) + group.add_member(user, GroupMember::MAINTAINER, blocking_refresh: false) end end @@ -866,12 +866,12 @@ RSpec.describe Group do let(:user) { create(:user) } before do - group.add_users([user.id], GroupMember::GUEST) + group.add_members([user.id], GroupMember::GUEST) end it "updates the group permission" do expect(group.group_members.guests.map(&:user)).to include(user) - group.add_users([user.id], GroupMember::DEVELOPER) + group.add_members([user.id], GroupMember::DEVELOPER) expect(group.group_members.developers.map(&:user)).to include(user) expect(group.group_members.guests.map(&:user)).not_to include(user) end @@ -880,7 +880,7 @@ RSpec.describe Group do let!(:project) { create(:project, group: group) } before do - group.add_users([create(:user)], :developer, tasks_to_be_done: %w(ci code), tasks_project_id: project.id) + group.add_members([create(:user)], :developer, tasks_to_be_done: %w(ci code), tasks_project_id: project.id) end it 'creates a member_task with the correct attributes', :aggregate_failures do @@ -896,7 +896,7 @@ RSpec.describe Group do let(:user) { create(:user) } before do - group.add_user(user, GroupMember::MAINTAINER) + group.add_member(user, GroupMember::MAINTAINER) end it "is true if avatar is image" do @@ -993,7 +993,7 @@ RSpec.describe Group do context 'there is also a project_bot owner' do before do - group.add_user(create(:user, :project_bot), GroupMember::OWNER) + group.add_member(create(:user, :project_bot), GroupMember::OWNER) end it { expect(group.last_owner?(@members[:owner])).to be_truthy } @@ -1024,7 +1024,7 @@ RSpec.describe Group do let(:member) { blocked_user.group_members.last } before do - group.add_user(blocked_user, GroupMember::OWNER) + group.add_member(blocked_user, GroupMember::OWNER) end context 'when last_blocked_owner is set' do @@ -1050,7 +1050,7 @@ RSpec.describe Group do context 'with another active owner' do before do - group.add_user(create(:user), GroupMember::OWNER) + group.add_member(create(:user), GroupMember::OWNER) end it { expect(group.member_last_blocked_owner?(member)).to be(false) } @@ -1058,7 +1058,7 @@ RSpec.describe Group do context 'with 2 blocked owners' do before do - group.add_user(create(:user, :blocked), GroupMember::OWNER) + group.add_member(create(:user, :blocked), GroupMember::OWNER) end it { expect(group.member_last_blocked_owner?(member)).to be(false) } @@ -1082,7 +1082,7 @@ RSpec.describe Group do describe '#single_blocked_owner?' do context 'when there is only one blocked owner' do before do - group.add_user(blocked_user, GroupMember::OWNER) + group.add_member(blocked_user, GroupMember::OWNER) end it 'returns true' do @@ -1094,8 +1094,8 @@ RSpec.describe Group do let_it_be(:blocked_user_2) { create(:user, :blocked) } before do - group.add_user(blocked_user, GroupMember::OWNER) - group.add_user(blocked_user_2, GroupMember::OWNER) + group.add_member(blocked_user, GroupMember::OWNER) + group.add_member(blocked_user_2, GroupMember::OWNER) end it 'returns true' do @@ -1114,8 +1114,8 @@ RSpec.describe Group do let_it_be(:user) { create(:user) } before do - group.add_user(blocked_user, GroupMember::OWNER) - group.add_user(user, GroupMember::OWNER) + group.add_member(blocked_user, GroupMember::OWNER) + group.add_member(user, GroupMember::OWNER) end it 'has only blocked owners' do @@ -1129,7 +1129,7 @@ RSpec.describe Group do context 'when there is only one owner' do let!(:owner) do - group.add_user(user, GroupMember::OWNER) + group.add_member(user, GroupMember::OWNER) end it 'returns the owner' do @@ -1138,7 +1138,7 @@ RSpec.describe Group do context 'and there is also a project_bot owner' do before do - group.add_user(create(:user, :project_bot), GroupMember::OWNER) + group.add_member(create(:user, :project_bot), GroupMember::OWNER) end it 'returns only the human owner' do @@ -1151,11 +1151,11 @@ RSpec.describe Group do let_it_be(:user_2) { create(:user) } let!(:owner) do - group.add_user(user, GroupMember::OWNER) + group.add_member(user, GroupMember::OWNER) end let!(:owner2) do - group.add_user(user_2, GroupMember::OWNER) + group.add_member(user_2, GroupMember::OWNER) end it 'returns both owners' do @@ -1164,7 +1164,7 @@ RSpec.describe Group do context 'and there is also a project_bot owner' do before do - group.add_user(create(:user, :project_bot), GroupMember::OWNER) + group.add_member(create(:user, :project_bot), GroupMember::OWNER) end it 'returns only the human owners' do @@ -1186,7 +1186,7 @@ RSpec.describe Group do let(:member) { group.members.last } before do - group.add_user(user, GroupMember::OWNER) + group.add_member(user, GroupMember::OWNER) end context 'when last_owner is set' do @@ -1284,11 +1284,11 @@ RSpec.describe Group do requester: create(:user) } - group.add_user(members[:owner], GroupMember::OWNER) - group.add_user(members[:maintainer], GroupMember::MAINTAINER) - group.add_user(members[:developer], GroupMember::DEVELOPER) - group.add_user(members[:reporter], GroupMember::REPORTER) - group.add_user(members[:guest], GroupMember::GUEST) + group.add_member(members[:owner], GroupMember::OWNER) + group.add_member(members[:maintainer], GroupMember::MAINTAINER) + group.add_member(members[:developer], GroupMember::DEVELOPER) + group.add_member(members[:reporter], GroupMember::REPORTER) + group.add_member(members[:guest], GroupMember::GUEST) group.request_access(members[:requester]) members @@ -1464,8 +1464,8 @@ RSpec.describe Group do describe '#direct_members' do let_it_be(:group) { create(:group, :nested) } - let_it_be(:maintainer) { group.parent.add_user(create(:user), GroupMember::MAINTAINER) } - let_it_be(:developer) { group.add_user(create(:user), GroupMember::DEVELOPER) } + let_it_be(:maintainer) { group.parent.add_member(create(:user), GroupMember::MAINTAINER) } + let_it_be(:developer) { group.add_member(create(:user), GroupMember::DEVELOPER) } it 'does not return members of the parent' do expect(group.direct_members).not_to include(maintainer) @@ -1491,8 +1491,8 @@ RSpec.describe Group do shared_examples_for 'members_with_parents' do let!(:group) { create(:group, :nested) } - let!(:maintainer) { group.parent.add_user(create(:user), GroupMember::MAINTAINER) } - let!(:developer) { group.add_user(create(:user), GroupMember::DEVELOPER) } + let!(:maintainer) { group.parent.add_member(create(:user), GroupMember::MAINTAINER) } + let!(:developer) { group.add_member(create(:user), GroupMember::DEVELOPER) } let!(:pending_maintainer) { create(:group_member, :awaiting, :maintainer, group: group.parent) } let!(:pending_developer) { create(:group_member, :awaiting, :developer, group: group) } @@ -1603,9 +1603,9 @@ RSpec.describe Group do context 'members-related methods' do let!(:group) { create(:group, :nested) } let!(:sub_group) { create(:group, parent: group) } - let!(:maintainer) { group.parent.add_user(create(:user), GroupMember::MAINTAINER) } - let!(:developer) { group.add_user(create(:user), GroupMember::DEVELOPER) } - let!(:other_developer) { group.add_user(create(:user), GroupMember::DEVELOPER) } + let!(:maintainer) { group.parent.add_member(create(:user), GroupMember::MAINTAINER) } + let!(:developer) { group.add_member(create(:user), GroupMember::DEVELOPER) } + let!(:other_developer) { group.add_member(create(:user), GroupMember::DEVELOPER) } describe '#direct_and_indirect_members' do it 'returns parents members' do @@ -1619,7 +1619,7 @@ RSpec.describe Group do end describe '#direct_and_indirect_members_with_inactive' do - let!(:maintainer_blocked) { group.parent.add_user(create(:user, :blocked), GroupMember::MAINTAINER) } + let!(:maintainer_blocked) { group.parent.add_member(create(:user, :blocked), GroupMember::MAINTAINER) } it 'returns parents members' do expect(group.direct_and_indirect_members_with_inactive).to include(developer) @@ -1795,8 +1795,8 @@ RSpec.describe Group do maintainer = create(:user) developer = create(:user) - group.add_user(maintainer, GroupMember::MAINTAINER) - group.add_user(developer, GroupMember::DEVELOPER) + group.add_member(maintainer, GroupMember::MAINTAINER) + group.add_member(developer, GroupMember::DEVELOPER) expect(group.user_ids_for_project_authorizations) .to include(maintainer.id, developer.id) @@ -1847,7 +1847,7 @@ RSpec.describe Group do context 'group membership' do before do - group.add_user(user, GroupMember::OWNER) + group.add_member(user, GroupMember::OWNER) end it 'is called when require_two_factor_authentication is changed' do @@ -1870,7 +1870,7 @@ RSpec.describe Group do it 'calls #update_two_factor_requirement on each group member' do other_user = create(:user) - group.add_user(other_user, GroupMember::OWNER) + group.add_member(other_user, GroupMember::OWNER) calls = 0 allow_any_instance_of(User).to receive(:update_two_factor_requirement) do @@ -1885,7 +1885,7 @@ RSpec.describe Group do context 'sub groups and projects' do it 'enables two_factor_requirement for group member' do - group.add_user(user, GroupMember::OWNER) + group.add_member(user, GroupMember::OWNER) group.update!(require_two_factor_authentication: true) @@ -1899,7 +1899,7 @@ RSpec.describe Group do context 'two_factor_requirement is also enabled for ancestor group' do it 'enables two_factor_requirement for subgroup member' do subgroup = create(:group, :nested, parent: group) - subgroup.add_user(indirect_user, GroupMember::OWNER) + subgroup.add_member(indirect_user, GroupMember::OWNER) group.update!(require_two_factor_authentication: true) @@ -1910,7 +1910,7 @@ RSpec.describe Group do context 'two_factor_requirement is disabled for ancestor group' do it 'enables two_factor_requirement for subgroup member' do subgroup = create(:group, :nested, parent: group, require_two_factor_authentication: true) - subgroup.add_user(indirect_user, GroupMember::OWNER) + subgroup.add_member(indirect_user, GroupMember::OWNER) group.update!(require_two_factor_authentication: false) @@ -1919,7 +1919,7 @@ RSpec.describe Group do it 'enable two_factor_requirement for ancestor group member' do ancestor_group = create(:group) - ancestor_group.add_user(indirect_user, GroupMember::OWNER) + ancestor_group.add_member(indirect_user, GroupMember::OWNER) group.update!(parent: ancestor_group) group.update!(require_two_factor_authentication: true) @@ -1933,7 +1933,7 @@ RSpec.describe Group do context 'two_factor_requirement is enabled for ancestor group' do it 'enables two_factor_requirement for subgroup member' do subgroup = create(:group, :nested, parent: group) - subgroup.add_user(indirect_user, GroupMember::OWNER) + subgroup.add_member(indirect_user, GroupMember::OWNER) group.update!(require_two_factor_authentication: true) @@ -1944,7 +1944,7 @@ RSpec.describe Group do context 'two_factor_requirement is also disabled for ancestor group' do it 'disables two_factor_requirement for subgroup member' do subgroup = create(:group, :nested, parent: group) - subgroup.add_user(indirect_user, GroupMember::OWNER) + subgroup.add_member(indirect_user, GroupMember::OWNER) group.update!(require_two_factor_authentication: false) @@ -1954,7 +1954,7 @@ RSpec.describe Group do it 'disables two_factor_requirement for ancestor group member' do ancestor_group = create(:group, require_two_factor_authentication: false) indirect_user.update!(require_two_factor_authentication_from_group: true) - ancestor_group.add_user(indirect_user, GroupMember::OWNER) + ancestor_group.add_member(indirect_user, GroupMember::OWNER) group.update!(require_two_factor_authentication: false) diff --git a/spec/models/members/group_member_spec.rb b/spec/models/members/group_member_spec.rb index f93c2d36966..94032146f51 100644 --- a/spec/models/members/group_member_spec.rb +++ b/spec/models/members/group_member_spec.rb @@ -219,7 +219,7 @@ RSpec.describe GroupMember do end context 'on create' do - let(:action) { group.add_user(user, Gitlab::Access::GUEST) } + let(:action) { group.add_member(user, Gitlab::Access::GUEST) } let(:blocking) { true } it 'changes access level', :sidekiq_inline do @@ -241,7 +241,7 @@ RSpec.describe GroupMember do context 'on update' do before do - group.add_user(user, Gitlab::Access::GUEST) + group.add_member(user, Gitlab::Access::GUEST) end let(:action) { group.members.find_by(user: user).update!(access_level: Gitlab::Access::DEVELOPER) } @@ -266,7 +266,7 @@ RSpec.describe GroupMember do context 'on destroy' do before do - group.add_user(user, Gitlab::Access::GUEST) + group.add_member(user, Gitlab::Access::GUEST) end let(:action) { group.members.find_by(user: user).destroy! } diff --git a/spec/models/members/project_member_spec.rb b/spec/models/members/project_member_spec.rb index 8c989f5aaca..48798d419e8 100644 --- a/spec/models/members/project_member_spec.rb +++ b/spec/models/members/project_member_spec.rb @@ -111,12 +111,12 @@ RSpec.describe ProjectMember do end end - describe '.add_users_to_projects' do + describe '.add_members_to_projects' do it 'adds the given users to the given projects' do projects = create_list(:project, 2) users = create_list(:user, 2) - described_class.add_users_to_projects( + described_class.add_members_to_projects( [projects.first.id, projects.second.id], [users.first.id, users.second], described_class::MAINTAINER) @@ -236,7 +236,7 @@ RSpec.describe ProjectMember do end context 'on create' do - let(:action) { project.add_user(user, Gitlab::Access::GUEST) } + let(:action) { project.add_member(user, Gitlab::Access::GUEST) } it 'changes access level' do expect { action }.to change { user.can?(:guest_access, project) }.from(false).to(true) @@ -250,7 +250,7 @@ RSpec.describe ProjectMember do let(:action) { project.members.find_by(user: user).update!(access_level: Gitlab::Access::DEVELOPER) } before do - project.add_user(user, Gitlab::Access::GUEST) + project.add_member(user, Gitlab::Access::GUEST) end it 'changes access level' do @@ -265,7 +265,7 @@ RSpec.describe ProjectMember do let(:action) { project.members.find_by(user: user).destroy! } before do - project.add_user(user, Gitlab::Access::GUEST) + project.add_member(user, Gitlab::Access::GUEST) end it 'changes access level', :sidekiq_inline do diff --git a/spec/models/merge_request_diff_file_spec.rb b/spec/models/merge_request_diff_file_spec.rb index c9bcb900eca..64203c50759 100644 --- a/spec/models/merge_request_diff_file_spec.rb +++ b/spec/models/merge_request_diff_file_spec.rb @@ -13,50 +13,53 @@ RSpec.describe MergeRequestDiffFile do let(:invalid_items_for_bulk_insertion) { [] } # class does not have any validations defined end + let(:unpacked) { 'unpacked' } + let(:packed) { [unpacked].pack('m0') } + let(:file) { create(:merge_request).merge_request_diff.merge_request_diff_files.first } + describe '#diff' do + let(:file) { build(:merge_request_diff_file) } + context 'when diff is not stored' do let(:unpacked) { 'unpacked' } let(:packed) { [unpacked].pack('m0') } before do - subject.diff = packed + file.diff = packed end context 'when the diff is marked as binary' do before do - subject.binary = true + file.binary = true end it 'unpacks from base 64' do - expect(subject.diff).to eq(unpacked) + expect(file.diff).to eq(unpacked) end context 'invalid base64' do let(:packed) { '---/dev/null' } it 'returns the raw diff' do - expect(subject.diff).to eq(packed) + expect(file.diff).to eq(packed) end end end context 'when the diff is not marked as binary' do it 'returns the raw diff' do - expect(subject.diff).to eq(packed) + expect(file.diff).to eq(packed) end end end context 'when diff is stored in DB' do - let(:file) { create(:merge_request).merge_request_diff.merge_request_diff_files.first } - it 'returns UTF-8 string' do expect(file.diff.encoding).to eq Encoding::UTF_8 end end context 'when diff is stored in external storage' do - let(:file) { create(:merge_request).merge_request_diff.merge_request_diff_files.first } let(:test_dir) { 'tmp/tests/external-diffs' } around do |example| @@ -81,17 +84,132 @@ RSpec.describe MergeRequestDiffFile do describe '#utf8_diff' do it 'does not raise error when the diff is binary' do - subject.diff = "\x05\x00\x68\x65\x6c\x6c\x6f" + file = build(:merge_request_diff_file) + file.diff = "\x05\x00\x68\x65\x6c\x6c\x6f" - expect { subject.utf8_diff }.not_to raise_error + expect { file.utf8_diff }.not_to raise_error end it 'calls #diff once' do - allow(subject).to receive(:diff).and_return('test') + allow(file).to receive(:diff).and_return('test') + + expect(file).to receive(:diff).once + + file.utf8_diff + end + end + + describe '#diff_export' do + context 'when diff is externally stored' do + let(:test_dir) { 'tmp/tests/external-diffs' } + + around do |example| + FileUtils.mkdir_p(test_dir) + + begin + example.run + ensure + FileUtils.rm_rf(test_dir) + end + end + + before do + stub_external_diffs_setting(enabled: true, storage_path: test_dir) + end + + context 'when external diff is not cached' do + it 'caches external diffs' do + expect(file.merge_request_diff).to receive(:cache_external_diff).and_call_original + + expect(file.diff_export).to eq(file.utf8_diff) + end + end + + context 'when external diff is already cached' do + it 'reads diff from cached external diff' do + file_stub = double + + allow(file.merge_request_diff).to receive(:cached_external_diff).and_yield(file_stub) + expect(file_stub).to receive(:seek).with(file.external_diff_offset) + expect(file_stub).to receive(:read).with(file.external_diff_size) + + file.diff_export + end + end + + context 'when the diff is marked as binary' do + let(:file) { build(:merge_request_diff_file) } - expect(subject).to receive(:diff).once + before do + allow(file.merge_request_diff).to receive(:stored_externally?).and_return(true) + allow(file.merge_request_diff).to receive(:cached_external_diff).and_return(packed) + end + + context 'when the diff is marked as binary' do + before do + file.binary = true + end + + it 'unpacks from base 64' do + expect(file.diff_export).to eq(unpacked) + end + + context 'invalid base64' do + let(:packed) { '---/dev/null' } + + it 'returns the raw diff' do + expect(file.diff_export).to eq(packed) + end + end + end + + context 'when the diff is not marked as binary' do + it 'returns the raw diff' do + expect(file.diff_export).to eq(packed) + end + end + end + + context 'when content responds to #encoding' do + it 'encodes content to utf8 encoding' do + expect(file.diff_export.encoding).to eq(Encoding::UTF_8) + end + end + + context 'when content is blank' do + it 'returns an empty string' do + allow(file.merge_request_diff).to receive(:cached_external_diff).and_return(nil) + + expect(file.diff_export).to eq('') + end + end - subject.utf8_diff + context 'when exception is raised' do + it 'falls back to #utf8_diff' do + allow(file).to receive(:binary?).and_raise(StandardError) + expect(file).to receive(:utf8_diff) + + file.diff_export + end + end + end + + context 'when externally_stored_diffs_caching_export feature flag is disabled' do + it 'calls #utf8_diff' do + stub_feature_flags(externally_stored_diffs_caching_export: false) + + expect(file).to receive(:utf8_diff) + + file.diff_export + end + end + + context 'when diff is not stored externally' do + it 'calls #utf8_diff' do + expect(file).to receive(:utf8_diff) + + file.diff_export + end end end end diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb index afe7251f59a..007e84164a8 100644 --- a/spec/models/merge_request_diff_spec.rb +++ b/spec/models/merge_request_diff_spec.rb @@ -1120,4 +1120,101 @@ RSpec.describe MergeRequestDiff do expect(described_class.latest_diff_for_merge_requests(nil)).to be_empty end end + + context 'external diff caching' do + let(:test_dir) { 'tmp/tests/external-diffs' } + let(:cache_dir) { File.join(Dir.tmpdir, "project-#{diff.project.id}-external-mr-#{diff.merge_request_id}-diff-#{diff.id}-cache") } + let(:cache_filepath) { File.join(cache_dir, "diff-#{diff.id}") } + let(:external_diff_content) { diff.opening_external_diff { |diff| diff.read } } + + around do |example| + FileUtils.mkdir_p(test_dir) + + begin + example.run + ensure + FileUtils.rm_rf(test_dir) + end + end + + before do + stub_external_diffs_setting(enabled: true, storage_path: test_dir) + end + + subject(:diff) { diff_with_commits } + + describe '#cached_external_diff' do + context 'when diff is externally stored' do + context 'when diff is already cached' do + it 'yields cached file' do + Dir.mkdir(cache_dir) + File.open(cache_filepath, 'wb') { |f| f.write(external_diff_content) } + + expect(diff).not_to receive(:cache_external_diff) + + expect { |b| diff.cached_external_diff(&b) }.to yield_with_args(File) + end + end + + context 'when diff is not cached' do + it 'caches external diff in tmp storage' do + expect(diff).to receive(:cache_external_diff).and_call_original + expect(File.exist?(cache_filepath)).to eq(false) + expect { |b| diff.cached_external_diff(&b) }.to yield_with_args(File) + expect(File.exist?(cache_filepath)).to eq(true) + expect(File.read(cache_filepath)).to eq(external_diff_content) + end + end + end + + context 'when diff is not externally stored' do + it 'yields nil' do + stub_external_diffs_setting(enabled: false) + + expect { |b| diff.cached_external_diff(&b) }.to yield_with_args(nil) + end + end + end + + describe '#remove_cached_external_diff' do + before do + diff.cached_external_diff { |diff| diff } + end + + it 'removes external diff cache diff' do + expect(Dir.exist?(cache_dir)).to eq(true) + + diff.remove_cached_external_diff + + expect(Dir.exist?(cache_dir)).to eq(false) + end + + context 'when path is traversed' do + it 'raises' do + allow(diff).to receive(:external_diff_cache_dir).and_return(File.join(cache_dir, '..')) + + expect { diff.remove_cached_external_diff }.to raise_error(Gitlab::Utils::PathTraversalAttackError, 'Invalid path') + end + end + + context 'when path is not allowed' do + it 'raises' do + allow(diff).to receive(:external_diff_cache_dir).and_return('/') + + expect { diff.remove_cached_external_diff }.to raise_error(StandardError, 'path / is not allowed') + end + end + + context 'when dir does not exist' do + it 'returns' do + FileUtils.rm_rf(cache_dir) + + expect(Dir.exist?(cache_dir)).to eq(false) + expect(FileUtils).not_to receive(:rm_rf).with(cache_dir) + + diff.remove_cached_external_diff + end + end + end + end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 77b81b2c310..c2f3deb6c06 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -658,7 +658,7 @@ RSpec.describe MergeRequest, factory_default: :keep do end before do - project.add_user(user, :developer) + project.add_member(user, :developer) end describe '.total_time_to_merge' do diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index b7ed7b20833..74a4a023a20 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -821,7 +821,7 @@ RSpec.describe Project, factory_default: :keep do end describe 'delegation' do - [:add_guest, :add_reporter, :add_developer, :add_maintainer, :add_user, :add_users].each do |method| + [:add_guest, :add_reporter, :add_developer, :add_maintainer, :add_member, :add_members].each do |method| it { is_expected.to delegate_method(method).to(:team) } end @@ -1862,7 +1862,7 @@ RSpec.describe Project, factory_default: :keep do describe 'when a user has access to a project' do before do - project.add_user(user, Gitlab::Access::MAINTAINER) + project.add_member(user, Gitlab::Access::MAINTAINER) end it { is_expected.to eq([project]) } diff --git a/spec/models/project_team_spec.rb b/spec/models/project_team_spec.rb index 2ddbab7779e..1fab07c1452 100644 --- a/spec/models/project_team_spec.rb +++ b/spec/models/project_team_spec.rb @@ -251,13 +251,13 @@ RSpec.describe ProjectTeam do end end - describe '#add_users' do + describe '#add_members' do let(:user1) { create(:user) } let(:user2) { create(:user) } let(:project) { create(:project) } it 'add the given users to the team' do - project.team.add_users([user1, user2], :reporter) + project.team.add_members([user1, user2], :reporter) expect(project.team.reporter?(user1)).to be(true) expect(project.team.reporter?(user2)).to be(true) @@ -265,7 +265,7 @@ RSpec.describe ProjectTeam do context 'when `tasks_to_be_done` and `tasks_project_id` are passed' do before do - project.team.add_users([user1], :developer, tasks_to_be_done: %w(ci code), tasks_project_id: project.id) + project.team.add_members([user1], :developer, tasks_to_be_done: %w(ci code), tasks_project_id: project.id) end it 'creates a member_task with the correct attributes', :aggregate_failures do @@ -277,12 +277,12 @@ RSpec.describe ProjectTeam do end end - describe '#add_user' do + describe '#add_member' do let(:user) { create(:user) } let(:project) { create(:project) } it 'add the given user to the team' do - project.team.add_user(user, :reporter) + project.team.add_member(user, :reporter) expect(project.team.reporter?(user)).to be(true) end diff --git a/spec/models/ssh_host_key_spec.rb b/spec/models/ssh_host_key_spec.rb index 4b756846598..0348aab9f97 100644 --- a/spec/models/ssh_host_key_spec.rb +++ b/spec/models/ssh_host_key_spec.rb @@ -26,6 +26,9 @@ RSpec.describe SshHostKey do 'Ebi86VjJRi2sOuYoXQU1' end + let(:ssh_key1) { Gitlab::SSHPublicKey.new(key1) } + let(:ssh_key2) { Gitlab::SSHPublicKey.new(key2) } + # Purposefully ordered so that `sort` will make changes let(:known_hosts) do <<~EOF @@ -88,10 +91,17 @@ RSpec.describe SshHostKey do it 'returns an array of indexed fingerprints when the cache is filled' do stub_reactive_cache(ssh_host_key, known_hosts: known_hosts) - expected = [key1, key2] - .map { |data| Gitlab::SSHPublicKey.new(data) } + expected = [ssh_key1, ssh_key2] .each_with_index - .map { |key, i| { bits: key.bits, fingerprint: key.fingerprint, type: key.type, index: i } } + .map do |key, i| + { + bits: key.bits, + fingerprint: key.fingerprint, + fingerprint_sha256: key.fingerprint_sha256, + type: key.type, + index: i + } + end expect(ssh_host_key.fingerprints.as_json).to eq(expected) end @@ -107,8 +117,16 @@ RSpec.describe SshHostKey do expect(ssh_host_key.fingerprints.as_json).to eq( [ - { bits: 2048, fingerprint: Gitlab::SSHPublicKey.new(key1).fingerprint, type: :rsa, index: 0 }, - { bits: 2048, fingerprint: Gitlab::SSHPublicKey.new(key2).fingerprint, type: :rsa, index: 1 } + { bits: 2048, + fingerprint: ssh_key1.fingerprint, + fingerprint_sha256: ssh_key1.fingerprint_sha256, + type: :rsa, + index: 0 }, + { bits: 2048, + fingerprint: ssh_key2.fingerprint, + fingerprint_sha256: ssh_key2.fingerprint_sha256, + type: :rsa, + index: 1 } ] ) end @@ -116,6 +134,19 @@ RSpec.describe SshHostKey do it 'returns an empty array when the cache is empty' do expect(ssh_host_key.fingerprints).to eq([]) end + + context 'when FIPS is enabled', :fips_mode do + it 'only includes SHA256 fingerprint' do + stub_reactive_cache(ssh_host_key, known_hosts: known_hosts) + + expect(ssh_host_key.fingerprints.as_json).to eq( + [ + { bits: 2048, fingerprint_sha256: ssh_key1.fingerprint_sha256, type: :rsa, index: 0 }, + { bits: 2048, fingerprint_sha256: ssh_key2.fingerprint_sha256, type: :rsa, index: 1 } + ] + ) + end + end end describe '#host_keys_changed?' do diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 7a5908fb586..626d8c8d337 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -2292,7 +2292,7 @@ RSpec.describe User do @group = create :group @group.add_owner(@user) - @group.add_user(@user2, GroupMember::OWNER) + @group.add_member(@user2, GroupMember::OWNER) end it { expect(@user2.several_namespaces?).to be_truthy } @@ -3834,7 +3834,7 @@ RSpec.describe User do let!(:project) { create(:project, group: project_group) } before do - private_group.add_user(user, Gitlab::Access::MAINTAINER) + private_group.add_member(user, Gitlab::Access::MAINTAINER) project.add_maintainer(user) end @@ -3861,7 +3861,7 @@ RSpec.describe User do let_it_be(:parent_group) do create(:group).tap do |g| - g.add_user(user, Gitlab::Access::MAINTAINER) + g.add_member(user, Gitlab::Access::MAINTAINER) end end @@ -4289,7 +4289,7 @@ RSpec.describe User do let!(:runner) { create(:ci_runner, :group, groups: [group]) } def add_user(access) - group.add_user(user, access) + group.add_member(user, access) end it_behaves_like :group_member @@ -4379,7 +4379,7 @@ RSpec.describe User do let!(:project_runner) { create(:ci_runner, :project, projects: [project]) } def add_user(access) - project.add_user(user, access) + project.add_member(user, access) end it_behaves_like :project_member @@ -4401,8 +4401,8 @@ RSpec.describe User do let!(:another_user) { create(:user) } def add_user(access) - subgroup.add_user(user, access) - group.add_user(another_user, :owner) + subgroup.add_member(user, access) + group.add_member(another_user, :owner) end it_behaves_like :group_member @@ -4759,8 +4759,8 @@ RSpec.describe User do let(:group2) { create :group, require_two_factor_authentication: true, two_factor_grace_period: 32 } before do - group1.add_user(user, GroupMember::OWNER) - group2.add_user(user, GroupMember::OWNER) + group1.add_member(user, GroupMember::OWNER) + group2.add_member(user, GroupMember::OWNER) user.update_two_factor_requirement end @@ -4779,7 +4779,7 @@ RSpec.describe User do let!(:group1a) { create :group, parent: group1 } before do - group1a.add_user(user, GroupMember::OWNER) + group1a.add_member(user, GroupMember::OWNER) user.update_two_factor_requirement end @@ -4794,7 +4794,7 @@ RSpec.describe User do let!(:group1a) { create :group, require_two_factor_authentication: true, parent: group1 } before do - group1.add_user(user, GroupMember::OWNER) + group1.add_member(user, GroupMember::OWNER) user.update_two_factor_requirement end @@ -4815,7 +4815,7 @@ RSpec.describe User do group_access: ProjectGroupLink.default_access ) - group2.add_user(user, GroupMember::OWNER) + group2.add_member(user, GroupMember::OWNER) end it 'does not require 2FA' do @@ -4829,7 +4829,7 @@ RSpec.describe User do let(:group) { create :group } before do - group.add_user(user, GroupMember::OWNER) + group.add_member(user, GroupMember::OWNER) user.update_two_factor_requirement end @@ -4858,8 +4858,8 @@ RSpec.describe User do let(:user) { create :user } before do - group.add_user(user, GroupMember::OWNER) - group_not_requiring_2FA.add_user(user, GroupMember::OWNER) + group.add_member(user, GroupMember::OWNER) + group_not_requiring_2FA.add_member(user, GroupMember::OWNER) end context 'when user is direct member of group requiring 2FA' do diff --git a/spec/policies/environment_policy_spec.rb b/spec/policies/environment_policy_spec.rb index 649b1a770c0..701fc7ac9ae 100644 --- a/spec/policies/environment_policy_spec.rb +++ b/spec/policies/environment_policy_spec.rb @@ -28,7 +28,7 @@ RSpec.describe EnvironmentPolicy do with_them do before do - project.add_user(user, access_level) unless access_level.nil? + project.add_member(user, access_level) unless access_level.nil? end it { expect(policy.allowed?(:stop_environment)).to be allowed? } @@ -49,7 +49,7 @@ RSpec.describe EnvironmentPolicy do context 'with protected branch' do with_them do before do - project.add_user(user, access_level) unless access_level.nil? + project.add_member(user, access_level) unless access_level.nil? create(:protected_branch, :no_one_can_push, name: 'master', project: project) end @@ -86,7 +86,7 @@ RSpec.describe EnvironmentPolicy do with_them do before do - project.add_user(user, access_level) unless access_level.nil? + project.add_member(user, access_level) unless access_level.nil? end it { expect(policy.allowed?(:stop_environment)).to be allowed? } @@ -120,7 +120,7 @@ RSpec.describe EnvironmentPolicy do with_them do before do - project.add_user(user, access_level) unless access_level.nil? + project.add_member(user, access_level) unless access_level.nil? end it { expect(policy).to be_disallowed :destroy_environment } diff --git a/spec/policies/namespace/root_storage_statistics_policy_spec.rb b/spec/policies/namespace/root_storage_statistics_policy_spec.rb index e6b58bca4a8..89875f83c9b 100644 --- a/spec/policies/namespace/root_storage_statistics_policy_spec.rb +++ b/spec/policies/namespace/root_storage_statistics_policy_spec.rb @@ -59,7 +59,7 @@ RSpec.describe Namespace::RootStorageStatisticsPolicy do with_them do before do - group.add_user(user, user_type) unless user_type == :non_member + group.add_member(user, user_type) unless user_type == :non_member end it { is_expected.to eq(outcome) } diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb index 5884dc3f23f..de36b755e88 100644 --- a/spec/policies/project_policy_spec.rb +++ b/spec/policies/project_policy_spec.rb @@ -1736,7 +1736,7 @@ RSpec.describe ProjectPolicy do %w(guest reporter developer maintainer).each do |role| context role do before do - project.add_user(current_user, role.to_sym) + project.add_member(current_user, role.to_sym) end if role == 'guest' @@ -1770,7 +1770,7 @@ RSpec.describe ProjectPolicy do %w(guest reporter developer maintainer).each do |role| context role do before do - project.add_user(current_user, role.to_sym) + project.add_member(current_user, role.to_sym) end it { is_expected.to be_allowed(:read_ci_cd_analytics) } @@ -1800,7 +1800,7 @@ RSpec.describe ProjectPolicy do %w(guest reporter developer maintainer).each do |role| context role do before do - project.add_user(current_user, role.to_sym) + project.add_member(current_user, role.to_sym) end if role == 'guest' diff --git a/spec/policies/project_statistics_policy_spec.rb b/spec/policies/project_statistics_policy_spec.rb index 74630dc38ad..56e6161a264 100644 --- a/spec/policies/project_statistics_policy_spec.rb +++ b/spec/policies/project_statistics_policy_spec.rb @@ -72,7 +72,7 @@ RSpec.describe ProjectStatisticsPolicy do before do unless [:unauthenticated, :non_member].include?(user_type) - project.add_user(external, user_type) + project.add_member(external, user_type) end end diff --git a/spec/requests/api/ci/runner/jobs_request_post_spec.rb b/spec/requests/api/ci/runner/jobs_request_post_spec.rb index 3f3a61949d4..746be1ccc44 100644 --- a/spec/requests/api/ci/runner/jobs_request_post_spec.rb +++ b/spec/requests/api/ci/runner/jobs_request_post_spec.rb @@ -675,14 +675,6 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do end end - context 'when variables are stored in trigger_request' do - before do - trigger_request.update_attribute(:variables, { TRIGGER_KEY_1: 'TRIGGER_VALUE_1' } ) - end - - it_behaves_like 'expected variables behavior' - end - context 'when variables are stored in pipeline_variables' do before do create(:ci_pipeline_variable, pipeline: pipeline, key: :TRIGGER_KEY_1, value: 'TRIGGER_VALUE_1') diff --git a/spec/requests/api/events_spec.rb b/spec/requests/api/events_spec.rb index 110d6e2f99e..d6c3999f22f 100644 --- a/spec/requests/api/events_spec.rb +++ b/spec/requests/api/events_spec.rb @@ -173,7 +173,7 @@ RSpec.describe API::Events do let(:second_note) { create(:note_on_issue, project: create(:project)) } before do - second_note.project.add_user(user, :developer) + second_note.project.add_member(user, :developer) [second_note].each do |note| EventCreateService.new.leave_note(note, user) diff --git a/spec/requests/api/graphql/ci/runner_spec.rb b/spec/requests/api/graphql/ci/runner_spec.rb index 446d1fb1bdb..e17a83d8e47 100644 --- a/spec/requests/api/graphql/ci/runner_spec.rb +++ b/spec/requests/api/graphql/ci/runner_spec.rb @@ -424,7 +424,7 @@ RSpec.describe 'Query.runner(id)' do let(:user) { create(:user) } before do - group.add_user(user, Gitlab::Access::OWNER) + group.add_member(user, Gitlab::Access::OWNER) end it_behaves_like 'retrieval with no admin url' do diff --git a/spec/requests/api/graphql/container_repository/container_repository_details_spec.rb b/spec/requests/api/graphql/container_repository/container_repository_details_spec.rb index 847fa72522e..14c55e61a65 100644 --- a/spec/requests/api/graphql/container_repository/container_repository_details_spec.rb +++ b/spec/requests/api/graphql/container_repository/container_repository_details_spec.rb @@ -71,7 +71,7 @@ RSpec.describe 'container repository details' do with_them do before do project.update!(visibility_level: Gitlab::VisibilityLevel.const_get(project_visibility.to_s.upcase, false)) - project.add_user(user, role) unless role == :anonymous + project.add_member(user, role) unless role == :anonymous end it 'return the proper response' do diff --git a/spec/requests/api/graphql/group/container_repositories_spec.rb b/spec/requests/api/graphql/group/container_repositories_spec.rb index be0b866af4a..8ec321c8d7c 100644 --- a/spec/requests/api/graphql/group/container_repositories_spec.rb +++ b/spec/requests/api/graphql/group/container_repositories_spec.rb @@ -82,7 +82,7 @@ RSpec.describe 'getting container repositories in a group' do group.update!(visibility_level: Gitlab::VisibilityLevel.const_get(group_visibility.to_s.upcase, false)) project.update!(visibility_level: Gitlab::VisibilityLevel.const_get(group_visibility.to_s.upcase, false)) - group.add_user(user, role) unless role == :anonymous + group.add_member(user, role) unless role == :anonymous end it 'return the proper response' do diff --git a/spec/requests/api/graphql/group/dependency_proxy_blobs_spec.rb b/spec/requests/api/graphql/group/dependency_proxy_blobs_spec.rb index cdb21512894..daa1483e956 100644 --- a/spec/requests/api/graphql/group/dependency_proxy_blobs_spec.rb +++ b/spec/requests/api/graphql/group/dependency_proxy_blobs_spec.rb @@ -75,7 +75,7 @@ RSpec.describe 'getting dependency proxy blobs in a group' do with_them do before do group.update_column(:visibility_level, Gitlab::VisibilityLevel.const_get(group_visibility.to_s.upcase, false)) - group.add_user(user, role) unless role == :anonymous + group.add_member(user, role) unless role == :anonymous end it 'return the proper response' do diff --git a/spec/requests/api/graphql/group/dependency_proxy_group_setting_spec.rb b/spec/requests/api/graphql/group/dependency_proxy_group_setting_spec.rb index d21c3046c1a..cc706c3051f 100644 --- a/spec/requests/api/graphql/group/dependency_proxy_group_setting_spec.rb +++ b/spec/requests/api/graphql/group/dependency_proxy_group_setting_spec.rb @@ -61,7 +61,7 @@ RSpec.describe 'getting dependency proxy settings for a group' do with_them do before do group.update_column(:visibility_level, Gitlab::VisibilityLevel.const_get(group_visibility.to_s.upcase, false)) - group.add_user(user, role) unless role == :anonymous + group.add_member(user, role) unless role == :anonymous end it 'return the proper response' do diff --git a/spec/requests/api/graphql/group/dependency_proxy_image_ttl_policy_spec.rb b/spec/requests/api/graphql/group/dependency_proxy_image_ttl_policy_spec.rb index 40f4b082072..3b2b04b1322 100644 --- a/spec/requests/api/graphql/group/dependency_proxy_image_ttl_policy_spec.rb +++ b/spec/requests/api/graphql/group/dependency_proxy_image_ttl_policy_spec.rb @@ -60,7 +60,7 @@ RSpec.describe 'getting dependency proxy image ttl policy for a group' do with_them do before do group.update_column(:visibility_level, Gitlab::VisibilityLevel.const_get(group_visibility.to_s.upcase, false)) - group.add_user(user, role) unless role == :anonymous + group.add_member(user, role) unless role == :anonymous end it 'return the proper response' do diff --git a/spec/requests/api/graphql/group/dependency_proxy_manifests_spec.rb b/spec/requests/api/graphql/group/dependency_proxy_manifests_spec.rb index c7149c100b2..9bb661a734e 100644 --- a/spec/requests/api/graphql/group/dependency_proxy_manifests_spec.rb +++ b/spec/requests/api/graphql/group/dependency_proxy_manifests_spec.rb @@ -73,7 +73,7 @@ RSpec.describe 'getting dependency proxy manifests in a group' do with_them do before do group.update_column(:visibility_level, Gitlab::VisibilityLevel.const_get(group_visibility.to_s.upcase, false)) - group.add_user(user, role) unless role == :anonymous + group.add_member(user, role) unless role == :anonymous end it 'return the proper response' do diff --git a/spec/requests/api/graphql/project/container_repositories_spec.rb b/spec/requests/api/graphql/project/container_repositories_spec.rb index bbab6012f3f..01b117a89d8 100644 --- a/spec/requests/api/graphql/project/container_repositories_spec.rb +++ b/spec/requests/api/graphql/project/container_repositories_spec.rb @@ -81,7 +81,7 @@ RSpec.describe 'getting container repositories in a project' do with_them do before do project.update!(visibility_level: Gitlab::VisibilityLevel.const_get(project_visibility.to_s.upcase, false)) - project.add_user(user, role) unless role == :anonymous + project.add_member(user, role) unless role == :anonymous end it 'return the proper response' do diff --git a/spec/requests/api/graphql/project/packages_cleanup_policy_spec.rb b/spec/requests/api/graphql/project/packages_cleanup_policy_spec.rb index a025c57d4b8..33e1dbcba27 100644 --- a/spec/requests/api/graphql/project/packages_cleanup_policy_spec.rb +++ b/spec/requests/api/graphql/project/packages_cleanup_policy_spec.rb @@ -61,7 +61,7 @@ RSpec.describe 'getting the packages cleanup policy linked to a project' do with_them do before do project.update!(visibility: visibility.to_s) - project.add_user(current_user, role) unless role == :anonymous + project.add_member(current_user, role) unless role == :anonymous end it 'return the proper response' do diff --git a/spec/requests/api/group_variables_spec.rb b/spec/requests/api/group_variables_spec.rb index 6d5676bbe35..a7b4bea362f 100644 --- a/spec/requests/api/group_variables_spec.rb +++ b/spec/requests/api/group_variables_spec.rb @@ -10,7 +10,7 @@ RSpec.describe API::GroupVariables do let(:access_level) {} before do - group.add_user(user, access_level) if access_level + group.add_member(user, access_level) if access_level end describe 'GET /groups/:id/variables' do diff --git a/spec/requests/api/invitations_spec.rb b/spec/requests/api/invitations_spec.rb index 64ad5733c1b..58068bf01a3 100644 --- a/spec/requests/api/invitations_spec.rb +++ b/spec/requests/api/invitations_spec.rb @@ -320,7 +320,7 @@ RSpec.describe API::Invitations do let(:source) { project } end - it 'records queries', :request_store, :use_sql_query_cache do + it 'does not exceed expected queries count for emails', :request_store, :use_sql_query_cache do post invitations_url(project, maintainer), params: { email: email, access_level: Member::DEVELOPER } control = ActiveRecord::QueryRecorder.new(skip_cached: false) do @@ -336,7 +336,25 @@ RSpec.describe API::Invitations do end.not_to exceed_all_query_limit(control.count).with_threshold(unresolved_n_plus_ones) end - it 'records queries with secondary emails', :request_store, :use_sql_query_cache do + it 'does not exceed expected queries count for user_ids', :request_store, :use_sql_query_cache do + stranger2 = create(:user) + + post invitations_url(project, maintainer), params: { user_id: stranger.id, access_level: Member::DEVELOPER } + + control = ActiveRecord::QueryRecorder.new(skip_cached: false) do + post invitations_url(project, maintainer), params: { user_id: stranger2.id, access_level: Member::DEVELOPER } + end + + users = create_list(:user, 5) + + unresolved_n_plus_ones = 116 # 51 for 1 vs 167 for 5 - currently there are 29 queries added per user + + expect do + post invitations_url(project, maintainer), params: { user_id: users.map(&:id).join(','), access_level: Member::DEVELOPER } + end.not_to exceed_all_query_limit(control.count).with_threshold(unresolved_n_plus_ones) + end + + it 'does not exceed expected queries count with secondary emails', :request_store, :use_sql_query_cache do create(:email, email: email, user: create(:user)) post invitations_url(project, maintainer), params: { email: email, access_level: Member::DEVELOPER } @@ -365,7 +383,7 @@ RSpec.describe API::Invitations do let(:source) { group } end - it 'records queries', :request_store, :use_sql_query_cache do + it 'does not exceed expected queries count for emails', :request_store, :use_sql_query_cache do post invitations_url(group, maintainer), params: { email: email, access_level: Member::DEVELOPER } control = ActiveRecord::QueryRecorder.new(skip_cached: false) do @@ -381,7 +399,7 @@ RSpec.describe API::Invitations do end.not_to exceed_all_query_limit(control.count).with_threshold(unresolved_n_plus_ones) end - it 'records queries with secondary emails', :request_store, :use_sql_query_cache do + it 'does not exceed expected queries count for secondary emails', :request_store, :use_sql_query_cache do create(:email, email: email, user: create(:user)) post invitations_url(group, maintainer), params: { email: email, access_level: Member::DEVELOPER } diff --git a/spec/requests/jira_connect/oauth_application_ids_controller_spec.rb b/spec/requests/jira_connect/oauth_application_ids_controller_spec.rb index ffeaf1075f3..b0c2eaec4e2 100644 --- a/spec/requests/jira_connect/oauth_application_ids_controller_spec.rb +++ b/spec/requests/jira_connect/oauth_application_ids_controller_spec.rb @@ -3,7 +3,40 @@ require 'spec_helper' RSpec.describe JiraConnect::OauthApplicationIdsController do + describe 'OPTIONS /-/jira_connect/oauth_application_id' do + before do + stub_application_setting(jira_connect_application_key: '123456') + + options '/-/jira_connect/oauth_application_id', headers: { 'Origin' => 'http://notgitlab.com' } + end + + it 'returns 200' do + expect(response).to have_gitlab_http_status(:ok) + end + + it 'allows cross-origin requests', :aggregate_failures do + expect(response.headers['Access-Control-Allow-Origin']).to eq '*' + expect(response.headers['Access-Control-Allow-Methods']).to eq 'GET, OPTIONS' + expect(response.headers['Access-Control-Allow-Credentials']).to be_nil + end + + context 'on GitLab.com' do + before do + allow(Gitlab).to receive(:com?).and_return(true) + end + + it 'renders not found' do + options '/-/jira_connect/oauth_application_id' + + expect(response).to have_gitlab_http_status(:not_found) + expect(response.headers['Access-Control-Allow-Origin']).not_to eq '*' + end + end + end + describe 'GET /-/jira_connect/oauth_application_id' do + let(:cors_request_headers) { { 'Origin' => 'http://notgitlab.com' } } + before do stub_application_setting(jira_connect_application_key: '123456') end @@ -15,6 +48,14 @@ RSpec.describe JiraConnect::OauthApplicationIdsController do expect(json_response).to eq({ "application_id" => "123456" }) end + it 'allows cross-origin requests', :aggregate_failures do + get '/-/jira_connect/oauth_application_id', headers: cors_request_headers + + expect(response.headers['Access-Control-Allow-Origin']).to eq '*' + expect(response.headers['Access-Control-Allow-Methods']).to eq 'GET, OPTIONS' + expect(response.headers['Access-Control-Allow-Credentials']).to be_nil + end + context 'application ID is empty' do before do stub_application_setting(jira_connect_application_key: '') @@ -38,5 +79,17 @@ RSpec.describe JiraConnect::OauthApplicationIdsController do expect(response).to have_gitlab_http_status(:not_found) end end + + context 'on GitLab.com' do + before do + allow(Gitlab).to receive(:com?).and_return(true) + end + + it 'renders not found' do + get '/-/jira_connect/oauth_application_id' + + expect(response).to have_gitlab_http_status(:not_found) + end + end end end diff --git a/spec/requests/jira_connect/subscriptions_controller_spec.rb b/spec/requests/jira_connect/subscriptions_controller_spec.rb new file mode 100644 index 00000000000..b10d07b3771 --- /dev/null +++ b/spec/requests/jira_connect/subscriptions_controller_spec.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe JiraConnect::SubscriptionsController do + describe 'GET /-/jira_connect/subscriptions' do + let_it_be(:installation) { create(:jira_connect_installation, instance_url: 'http://self-managed-gitlab.com') } + + let(:qsh) do + Atlassian::Jwt.create_query_string_hash('https://gitlab.test/subscriptions', 'GET', 'https://gitlab.test') + end + + let(:jwt) { Atlassian::Jwt.encode({ iss: installation.client_key, qsh: qsh }, installation.shared_secret) } + + before do + get '/-/jira_connect/subscriptions', params: { jwt: jwt } + end + + subject(:content_security_policy) { response.headers['Content-Security-Policy'] } + + it { is_expected.to include('http://self-managed-gitlab.com/-/jira_connect/oauth_application_ids')} + + context 'with no self-managed instance configured' do + let_it_be(:installation) { create(:jira_connect_installation, instance_url: '') } + + it { is_expected.not_to include('http://self-managed-gitlab.com')} + end + end +end diff --git a/spec/requests/openid_connect_spec.rb b/spec/requests/openid_connect_spec.rb index c647fee1564..afaa6168bfd 100644 --- a/spec/requests/openid_connect_spec.rb +++ b/spec/requests/openid_connect_spec.rb @@ -120,9 +120,9 @@ RSpec.describe 'OpenID Connect requests' do let!(:group4) { create :group, parent: group3 } before do - group1.add_user(user, GroupMember::OWNER) - group3.add_user(user, Gitlab::Access::DEVELOPER) - group4.add_user(user, Gitlab::Access::MAINTAINER) + group1.add_member(user, GroupMember::OWNER) + group3.add_member(user, Gitlab::Access::DEVELOPER) + group4.add_member(user, Gitlab::Access::MAINTAINER) request_user_info! end @@ -163,8 +163,8 @@ RSpec.describe 'OpenID Connect requests' do let!(:group4) { create :group, parent: group3 } before do - group1.add_user(user, Gitlab::Access::OWNER) - group3.add_user(user, Gitlab::Access::DEVELOPER) + group1.add_member(user, Gitlab::Access::OWNER) + group3.add_member(user, Gitlab::Access::DEVELOPER) request_access_token! @payload = JSON::JWT.decode(json_response['id_token'], :skip_verification) @@ -358,8 +358,8 @@ RSpec.describe 'OpenID Connect requests' do let!(:group4) { create :group, parent: group3 } before do - group1.add_user(user, Gitlab::Access::OWNER) - group3.add_user(user, Gitlab::Access::DEVELOPER) + group1.add_member(user, Gitlab::Access::OWNER) + group3.add_member(user, Gitlab::Access::DEVELOPER) request_access_token! @payload = JSON::JWT.decode(json_response['id_token'], :skip_verification) diff --git a/spec/requests/projects/environments_controller_spec.rb b/spec/requests/projects/environments_controller_spec.rb index 5cdf507abef..0890b0c45da 100644 --- a/spec/requests/projects/environments_controller_spec.rb +++ b/spec/requests/projects/environments_controller_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe Projects::EnvironmentsController do - let_it_be(:project) { create(:project, :repository) } + let_it_be_with_refind(:project) { create(:project, :repository) } let(:environment) { create(:environment, name: 'production', project: project) } diff --git a/spec/serializers/member_user_entity_spec.rb b/spec/serializers/member_user_entity_spec.rb index 85f29845d65..4dd6848c47b 100644 --- a/spec/serializers/member_user_entity_spec.rb +++ b/spec/serializers/member_user_entity_spec.rb @@ -51,7 +51,7 @@ RSpec.describe MemberUserEntity do shared_examples 'correctly exposes user two_factor_enabled' do context 'when the current_user has a role lower than minimum manage member role' do before do - source.add_user(current_user, Gitlab::Access::DEVELOPER) + source.add_member(current_user, Gitlab::Access::DEVELOPER) end it 'does not expose user two_factor_enabled' do @@ -65,7 +65,7 @@ RSpec.describe MemberUserEntity do context 'when the current user has a minimum manage member role or higher' do before do - source.add_user(current_user, minimum_manage_member_role) + source.add_member(current_user, minimum_manage_member_role) end it 'matches json schema' do diff --git a/spec/services/groups/destroy_service_spec.rb b/spec/services/groups/destroy_service_spec.rb index 628943e40ff..57a151efda6 100644 --- a/spec/services/groups/destroy_service_spec.rb +++ b/spec/services/groups/destroy_service_spec.rb @@ -12,7 +12,7 @@ RSpec.describe Groups::DestroyService do let(:remove_path) { group.path + "+#{group.id}+deleted" } before do - group.add_user(user, Gitlab::Access::OWNER) + group.add_member(user, Gitlab::Access::OWNER) end def destroy_group(group, user, async) @@ -168,8 +168,8 @@ RSpec.describe Groups::DestroyService do let(:group2_user) { create(:user) } before do - group1.add_user(group1_user, Gitlab::Access::OWNER) - group2.add_user(group2_user, Gitlab::Access::OWNER) + group1.add_member(group1_user, Gitlab::Access::OWNER) + group2.add_member(group2_user, Gitlab::Access::OWNER) end context 'when a project is shared with a group' do @@ -203,7 +203,7 @@ RSpec.describe Groups::DestroyService do let(:group3_user) { create(:user) } before do - group3.add_user(group3_user, Gitlab::Access::OWNER) + group3.add_member(group3_user, Gitlab::Access::OWNER) create(:group_group_link, shared_group: group2, shared_with_group: group3) group3.refresh_members_authorized_projects @@ -290,7 +290,7 @@ RSpec.describe Groups::DestroyService do let!(:shared_with_group_user) { create(:user) } before do - shared_with_group.add_user(shared_with_group_user, Gitlab::Access::MAINTAINER) + shared_with_group.add_member(shared_with_group_user, Gitlab::Access::MAINTAINER) create(:group_group_link, shared_group: shared_group, shared_with_group: shared_with_group) shared_with_group.refresh_members_authorized_projects diff --git a/spec/services/groups/update_service_spec.rb b/spec/services/groups/update_service_spec.rb index 46c5e2a9818..c0e1691fe26 100644 --- a/spec/services/groups/update_service_spec.rb +++ b/spec/services/groups/update_service_spec.rb @@ -58,7 +58,7 @@ RSpec.describe Groups::UpdateService do let!(:service) { described_class.new(public_group, user, visibility_level: Gitlab::VisibilityLevel::INTERNAL) } before do - public_group.add_user(user, Gitlab::Access::OWNER) + public_group.add_member(user, Gitlab::Access::OWNER) create(:project, :public, group: public_group) expect(TodosDestroyer::GroupPrivateWorker).not_to receive(:perform_in) @@ -119,7 +119,7 @@ RSpec.describe Groups::UpdateService do let!(:service) { described_class.new(internal_group, user, visibility_level: Gitlab::VisibilityLevel::PRIVATE) } before do - internal_group.add_user(user, Gitlab::Access::OWNER) + internal_group.add_member(user, Gitlab::Access::OWNER) create(:project, :internal, group: internal_group) expect(TodosDestroyer::GroupPrivateWorker).not_to receive(:perform_in) @@ -135,7 +135,7 @@ RSpec.describe Groups::UpdateService do let!(:service) { described_class.new(internal_group, user, visibility_level: Gitlab::VisibilityLevel::PRIVATE) } before do - internal_group.add_user(user, Gitlab::Access::OWNER) + internal_group.add_member(user, Gitlab::Access::OWNER) create(:project, :private, group: internal_group) expect(TodosDestroyer::GroupPrivateWorker).to receive(:perform_in) @@ -233,7 +233,7 @@ RSpec.describe Groups::UpdateService do let!(:service) { described_class.new(internal_group, user, visibility_level: 99) } before do - internal_group.add_user(user, Gitlab::Access::MAINTAINER) + internal_group.add_member(user, Gitlab::Access::MAINTAINER) end it "does not change permission level" do @@ -246,7 +246,7 @@ RSpec.describe Groups::UpdateService do let(:service) { described_class.new(internal_group, user, emails_disabled: true) } it 'updates the attribute' do - internal_group.add_user(user, Gitlab::Access::OWNER) + internal_group.add_member(user, Gitlab::Access::OWNER) expect { service.execute }.to change { internal_group.emails_disabled }.to(true) end @@ -280,7 +280,7 @@ RSpec.describe Groups::UpdateService do let!(:service) { described_class.new(internal_group, user, path: SecureRandom.hex) } before do - internal_group.add_user(user, Gitlab::Access::MAINTAINER) + internal_group.add_member(user, Gitlab::Access::MAINTAINER) create(:project, :internal, group: internal_group) end diff --git a/spec/services/issues/related_branches_service_spec.rb b/spec/services/issues/related_branches_service_spec.rb index 53474c61907..7a4bae7f852 100644 --- a/spec/services/issues/related_branches_service_spec.rb +++ b/spec/services/issues/related_branches_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Issues::RelatedBranchesService, :clean_gitlab_redis_cache do +RSpec.describe Issues::RelatedBranchesService do let_it_be(:developer) { create(:user) } let_it_be(:issue) { create(:issue) } diff --git a/spec/services/members/creator_service_spec.rb b/spec/services/members/creator_service_spec.rb index 8b1df2ab86d..ad4c649086b 100644 --- a/spec/services/members/creator_service_spec.rb +++ b/spec/services/members/creator_service_spec.rb @@ -11,7 +11,7 @@ RSpec.describe Members::CreatorService do describe '#execute' do it 'raises error for new member on authorization check implementation' do expect do - described_class.add_user(source, user, :maintainer, current_user: current_user) + described_class.add_member(source, user, :maintainer, current_user: current_user) end.to raise_error(NotImplementedError) end @@ -19,7 +19,7 @@ RSpec.describe Members::CreatorService do source.add_developer(user) expect do - described_class.add_user(source, user, :maintainer, current_user: current_user) + described_class.add_member(source, user, :maintainer, current_user: current_user) end.to raise_error(NotImplementedError) end end diff --git a/spec/services/members/groups/creator_service_spec.rb b/spec/services/members/groups/creator_service_spec.rb index b80b7998eac..4130fbd44fa 100644 --- a/spec/services/members/groups/creator_service_spec.rb +++ b/spec/services/members/groups/creator_service_spec.rb @@ -14,13 +14,13 @@ RSpec.describe Members::Groups::CreatorService do it_behaves_like 'owner management' - describe '.add_users' do + describe '.add_members' do it_behaves_like 'bulk member creation' do let_it_be(:member_type) { GroupMember } end end - describe '.add_user' do + describe '.add_member' do it_behaves_like 'member creation' do let_it_be(:member_type) { GroupMember } end @@ -30,7 +30,7 @@ RSpec.describe Members::Groups::CreatorService do expect(AuthorizedProjectsWorker).to receive(:bulk_perform_and_wait).once 1.upto(3) do - described_class.add_user(source, user, :maintainer) + described_class.add_member(source, user, :maintainer) end end end diff --git a/spec/services/members/projects/creator_service_spec.rb b/spec/services/members/projects/creator_service_spec.rb index 38955122ab0..8304bee2ffc 100644 --- a/spec/services/members/projects/creator_service_spec.rb +++ b/spec/services/members/projects/creator_service_spec.rb @@ -14,13 +14,13 @@ RSpec.describe Members::Projects::CreatorService do it_behaves_like 'owner management' - describe '.add_users' do + describe '.add_members' do it_behaves_like 'bulk member creation' do let_it_be(:member_type) { ProjectMember } end end - describe '.add_user' do + describe '.add_member' do it_behaves_like 'member creation' do let_it_be(:member_type) { ProjectMember } end @@ -30,7 +30,7 @@ RSpec.describe Members::Projects::CreatorService do expect(AuthorizedProjectUpdate::UserRefreshFromReplicaWorker).to receive(:bulk_perform_in).once 1.upto(3) do - described_class.add_user(source, user, :maintainer) + described_class.add_member(source, user, :maintainer) end end end diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 8e0f964d965..98fe8a40c61 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -582,8 +582,8 @@ RSpec.describe NotificationService, :mailer do before do note.project.namespace_id = group.id - group.add_user(@u_watcher, GroupMember::MAINTAINER) - group.add_user(@u_custom_global, GroupMember::MAINTAINER) + group.add_member(@u_watcher, GroupMember::MAINTAINER) + group.add_member(@u_custom_global, GroupMember::MAINTAINER) note.project.save! @u_watcher.notification_settings_for(note.project).participating! @@ -3850,7 +3850,7 @@ RSpec.describe NotificationService, :mailer do # Group member: global=watch, group=global @g_global_watcher ||= create_global_setting_for(create(:user), :watch) - group.add_users([@g_watcher, @g_global_watcher], :maintainer) + group.add_members([@g_watcher, @g_global_watcher], :maintainer) group end diff --git a/spec/services/projects/destroy_service_spec.rb b/spec/services/projects/destroy_service_spec.rb index 278ca148b74..7361b9b8e35 100644 --- a/spec/services/projects/destroy_service_spec.rb +++ b/spec/services/projects/destroy_service_spec.rb @@ -224,7 +224,7 @@ RSpec.describe Projects::DestroyService, :aggregate_failures, :event_store_publi context 'when flushing caches fail due to Redis' do before do new_user = create(:user) - project.team.add_user(new_user, Gitlab::Access::DEVELOPER) + project.team.add_member(new_user, Gitlab::Access::DEVELOPER) allow_any_instance_of(described_class).to receive(:flush_caches).and_raise(::Redis::CannotConnectError) end diff --git a/spec/services/projects/fork_service_spec.rb b/spec/services/projects/fork_service_spec.rb index e292c13c46e..b1b45e89c1b 100644 --- a/spec/services/projects/fork_service_spec.rb +++ b/spec/services/projects/fork_service_spec.rb @@ -32,14 +32,14 @@ RSpec.describe Projects::ForkService do external_authorization_classification_label: 'classification-label') @to_user = create(:user) @to_namespace = @to_user.namespace - @from_project.add_user(@to_user, :developer) + @from_project.add_member(@to_user, :developer) end context 'fork project' do context 'when forker is a guest' do before do @guest = create(:user) - @from_project.add_user(@guest, :guest) + @from_project.add_member(@guest, :guest) end subject { fork_project(@from_project, @guest, using_service: true) } @@ -261,10 +261,10 @@ RSpec.describe Projects::ForkService do description: 'Wow, such a cool project!', ci_config_path: 'debian/salsa-ci.yml') @group = create(:group) - @group.add_user(@group_owner, GroupMember::OWNER) - @group.add_user(@developer, GroupMember::DEVELOPER) - @project.add_user(@developer, :developer) - @project.add_user(@group_owner, :developer) + @group.add_member(@group_owner, GroupMember::OWNER) + @group.add_member(@developer, GroupMember::DEVELOPER) + @project.add_member(@developer, :developer) + @project.add_member(@group_owner, :developer) @opts = { namespace: @group, using_service: true } end diff --git a/spec/services/projects/update_service_spec.rb b/spec/services/projects/update_service_spec.rb index 1241491d914..f019434a4fe 100644 --- a/spec/services/projects/update_service_spec.rb +++ b/spec/services/projects/update_service_spec.rb @@ -424,7 +424,7 @@ RSpec.describe Projects::UpdateService do it 'does not update when not project owner' do maintainer = create(:user) - project.add_user(maintainer, :maintainer) + project.add_member(maintainer, :maintainer) expect { update_project(project, maintainer, emails_disabled: true) } .not_to change { project.emails_disabled } diff --git a/spec/services/quick_actions/interpret_service_spec.rb b/spec/services/quick_actions/interpret_service_spec.rb index f7ed6006099..3f11eaa7e93 100644 --- a/spec/services/quick_actions/interpret_service_spec.rb +++ b/spec/services/quick_actions/interpret_service_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe QuickActions::InterpretService do + include AfterNextHelpers + let_it_be(:group) { create(:group, :crm_enabled) } let_it_be(:public_project) { create(:project, :public, group: group) } let_it_be(:repository_project) { create(:project, :repository) } @@ -17,8 +19,9 @@ RSpec.describe QuickActions::InterpretService do let(:milestone) { create(:milestone, project: project, title: '9.10') } let(:commit) { create(:commit, project: project) } + let(:current_user) { developer } - subject(:service) { described_class.new(project, developer) } + subject(:service) { described_class.new(project, current_user) } before_all do public_project.add_developer(developer) @@ -682,6 +685,58 @@ RSpec.describe QuickActions::InterpretService do end shared_examples 'assign command' do + it 'assigns to me' do + cmd = '/assign me' + + _, updates, _ = service.execute(cmd, issuable) + + expect(updates).to eq(assignee_ids: [current_user.id]) + end + + it 'does not assign to group members' do + grp = create(:group) + grp.add_developer(developer) + grp.add_developer(developer2) + + cmd = "/assign #{grp.to_reference}" + + _, updates, message = service.execute(cmd, issuable) + + expect(updates).to be_blank + expect(message).to include('Failed to find users') + end + + context 'when there are too many references' do + before do + stub_const('Gitlab::QuickActions::UsersExtractor::MAX_QUICK_ACTION_USERS', 2) + end + + it 'says what went wrong' do + cmd = '/assign her and you, me and them' + + _, updates, message = service.execute(cmd, issuable) + + expect(updates).to be_blank + expect(message).to include('Too many references. Quick actions are limited to at most 2 user references') + end + end + + context 'when the user extractor raises an uninticipated error' do + before do + allow_next(Gitlab::QuickActions::UsersExtractor) + .to receive(:execute).and_raise(Gitlab::QuickActions::UsersExtractor::Error) + end + + it 'tracks the exception in dev, and reports a generic message in production' do + expect(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception).twice + + _, updates, message = service.execute('/assign some text', issuable) + + expect(updates).to be_blank + expect(message).to include('Something went wrong') + end + end + it 'assigns to users with escaped underscores' do user = create(:user) base = user.username diff --git a/spec/services/todo_service_spec.rb b/spec/services/todo_service_spec.rb index e4582e19416..1cb44366457 100644 --- a/spec/services/todo_service_spec.rb +++ b/spec/services/todo_service_spec.rb @@ -186,8 +186,8 @@ RSpec.describe TodoService do before do group.add_owner(author) - group.add_user(member, Gitlab::Access::DEVELOPER) - group.add_user(john_doe, Gitlab::Access::DEVELOPER) + group.add_member(member, Gitlab::Access::DEVELOPER) + group.add_member(john_doe, Gitlab::Access::DEVELOPER) service.new_issue(issue, author) end diff --git a/spec/support/helpers/project_helpers.rb b/spec/support/helpers/project_helpers.rb index ef8947ab340..2427ed2bcc9 100644 --- a/spec/support/helpers/project_helpers.rb +++ b/spec/support/helpers/project_helpers.rb @@ -13,7 +13,7 @@ module ProjectHelpers when :admin create(:user, :admin, name: 'admin') else - create(:user, name: membership).tap { |u| target.add_user(u, membership) } + create(:user, name: membership).tap { |u| target.add_member(u, membership) } end end diff --git a/spec/support/shared_examples/finders/issues_finder_shared_examples.rb b/spec/support/shared_examples/finders/issues_finder_shared_examples.rb index 622a88e8323..f3c616c7d9b 100644 --- a/spec/support/shared_examples/finders/issues_finder_shared_examples.rb +++ b/spec/support/shared_examples/finders/issues_finder_shared_examples.rb @@ -962,7 +962,7 @@ RSpec.shared_examples 'issues or work items finder' do |factory, execute_context group = create(:group) project = create(:project, group: group) item = create(factory, project: project) - group.add_user(user, :owner) + group.add_member(user, :owner) expect(items).to include(item) end diff --git a/spec/support/shared_examples/models/member_shared_examples.rb b/spec/support/shared_examples/models/member_shared_examples.rb index 75fff11cecd..9ac173341d7 100644 --- a/spec/support/shared_examples/models/member_shared_examples.rb +++ b/spec/support/shared_examples/models/member_shared_examples.rb @@ -80,7 +80,7 @@ RSpec.shared_examples_for "member creation" do let_it_be(:admin) { create(:admin) } it 'returns a Member object', :aggregate_failures do - member = described_class.add_user(source, user, :maintainer) + member = described_class.add_member(source, user, :maintainer) expect(member).to be_a member_type expect(member).to be_persisted @@ -99,7 +99,7 @@ RSpec.shared_examples_for "member creation" do end it 'does not update the member' do - member = described_class.add_user(source, project_bot, :maintainer, current_user: user) + member = described_class.add_member(source, project_bot, :maintainer, current_user: user) expect(source.users.reload).to include(project_bot) expect(member).to be_persisted @@ -110,7 +110,7 @@ RSpec.shared_examples_for "member creation" do context 'when project_bot is not already a member' do it 'adds the member' do - member = described_class.add_user(source, project_bot, :maintainer, current_user: user) + member = described_class.add_member(source, project_bot, :maintainer, current_user: user) expect(source.users.reload).to include(project_bot) expect(member).to be_persisted @@ -120,7 +120,7 @@ RSpec.shared_examples_for "member creation" do context 'when admin mode is enabled', :enable_admin_mode, :aggregate_failures do it 'sets members.created_by to the given admin current_user' do - member = described_class.add_user(source, user, :maintainer, current_user: admin) + member = described_class.add_member(source, user, :maintainer, current_user: admin) expect(member).to be_persisted expect(source.users.reload).to include(user) @@ -130,7 +130,7 @@ RSpec.shared_examples_for "member creation" do context 'when admin mode is disabled' do it 'rejects setting members.created_by to the given admin current_user', :aggregate_failures do - member = described_class.add_user(source, user, :maintainer, current_user: admin) + member = described_class.add_member(source, user, :maintainer, current_user: admin) expect(member).not_to be_persisted expect(source.users.reload).not_to include(user) @@ -139,7 +139,7 @@ RSpec.shared_examples_for "member creation" do end it 'sets members.expires_at to the given expires_at' do - member = described_class.add_user(source, user, :maintainer, expires_at: Date.new(2016, 9, 22)) + member = described_class.add_member(source, user, :maintainer, expires_at: Date.new(2016, 9, 22)) expect(member.expires_at).to eq(Date.new(2016, 9, 22)) end @@ -148,7 +148,7 @@ RSpec.shared_examples_for "member creation" do it "accepts the :#{sym_key} symbol as access level", :aggregate_failures do expect(source.users).not_to include(user) - member = described_class.add_user(source, user.id, sym_key) + member = described_class.add_member(source, user.id, sym_key) expect(member.access_level).to eq(int_access_level) expect(source.users.reload).to include(user) @@ -157,7 +157,7 @@ RSpec.shared_examples_for "member creation" do it "accepts the #{int_access_level} integer as access level", :aggregate_failures do expect(source.users).not_to include(user) - member = described_class.add_user(source, user.id, int_access_level) + member = described_class.add_member(source, user.id, int_access_level) expect(member.access_level).to eq(int_access_level) expect(source.users.reload).to include(user) @@ -169,7 +169,7 @@ RSpec.shared_examples_for "member creation" do it 'adds the user as a member' do expect(source.users).not_to include(user) - described_class.add_user(source, user.id, :maintainer) + described_class.add_member(source, user.id, :maintainer) expect(source.users.reload).to include(user) end @@ -179,7 +179,7 @@ RSpec.shared_examples_for "member creation" do it 'does not add the user as a member' do expect(source.users).not_to include(user) - described_class.add_user(source, non_existing_record_id, :maintainer) + described_class.add_member(source, non_existing_record_id, :maintainer) expect(source.users.reload).not_to include(user) end @@ -189,7 +189,7 @@ RSpec.shared_examples_for "member creation" do it 'adds the user as a member' do expect(source.users).not_to include(user) - described_class.add_user(source, user, :maintainer) + described_class.add_member(source, user, :maintainer) expect(source.users.reload).to include(user) end @@ -205,7 +205,7 @@ RSpec.shared_examples_for "member creation" do expect(source.requesters.exists?(user_id: user)).to be_truthy expect do - described_class.add_user(source, user, :maintainer) + described_class.add_member(source, user, :maintainer) end.to raise_error(Gitlab::Access::AccessDeniedError) expect(source.users.reload).not_to include(user) @@ -217,7 +217,7 @@ RSpec.shared_examples_for "member creation" do it 'adds the user as a member' do expect(source.users).not_to include(user) - described_class.add_user(source, user.email, :maintainer) + described_class.add_member(source, user.email, :maintainer) expect(source.users.reload).to include(user) end @@ -227,7 +227,7 @@ RSpec.shared_examples_for "member creation" do it 'creates an invited member' do expect(source.users).not_to include(user) - described_class.add_user(source, 'user@example.com', :maintainer) + described_class.add_member(source, 'user@example.com', :maintainer) expect(source.members.invite.pluck(:invite_email)).to include('user@example.com') end @@ -237,7 +237,7 @@ RSpec.shared_examples_for "member creation" do it 'creates an invited member', :aggregate_failures do email_starting_with_number = "#{user.id}_email@example.com" - described_class.add_user(source, email_starting_with_number, :maintainer) + described_class.add_member(source, email_starting_with_number, :maintainer) expect(source.members.invite.pluck(:invite_email)).to include(email_starting_with_number) expect(source.users.reload).not_to include(user) @@ -249,7 +249,7 @@ RSpec.shared_examples_for "member creation" do it 'creates the member' do expect(source.users).not_to include(user) - described_class.add_user(source, user, :maintainer, current_user: admin) + described_class.add_member(source, user, :maintainer, current_user: admin) expect(source.users.reload).to include(user) end @@ -263,7 +263,7 @@ RSpec.shared_examples_for "member creation" do expect(source.users).not_to include(user) expect(source.requesters.exists?(user_id: user)).to be_truthy - described_class.add_user(source, user, :maintainer, current_user: admin) + described_class.add_member(source, user, :maintainer, current_user: admin) expect(source.users.reload).to include(user) expect(source.requesters.reload.exists?(user_id: user)).to be_falsy @@ -275,7 +275,7 @@ RSpec.shared_examples_for "member creation" do it 'does not create the member', :aggregate_failures do expect(source.users).not_to include(user) - member = described_class.add_user(source, user, :maintainer, current_user: user) + member = described_class.add_member(source, user, :maintainer, current_user: user) expect(source.users.reload).not_to include(user) expect(member).not_to be_persisted @@ -290,7 +290,7 @@ RSpec.shared_examples_for "member creation" do expect(source.users).not_to include(user) expect(source.requesters.exists?(user_id: user)).to be_truthy - described_class.add_user(source, user, :maintainer, current_user: user) + described_class.add_member(source, user, :maintainer, current_user: user) expect(source.users.reload).not_to include(user) expect(source.requesters.exists?(user_id: user)).to be_truthy @@ -300,14 +300,14 @@ RSpec.shared_examples_for "member creation" do context 'when member already exists' do before do - source.add_user(user, :developer) + source.add_member(user, :developer) end context 'with no current_user' do it 'updates the member' do expect(source.users).to include(user) - described_class.add_user(source, user, :maintainer) + described_class.add_member(source, user, :maintainer) expect(source.members.find_by(user_id: user).access_level).to eq(Gitlab::Access::MAINTAINER) end @@ -317,7 +317,7 @@ RSpec.shared_examples_for "member creation" do it 'updates the member' do expect(source.users).to include(user) - described_class.add_user(source, user, :maintainer, current_user: admin) + described_class.add_member(source, user, :maintainer, current_user: admin) expect(source.members.find_by(user_id: user).access_level).to eq(Gitlab::Access::MAINTAINER) end @@ -327,7 +327,7 @@ RSpec.shared_examples_for "member creation" do it 'does not update the member' do expect(source.users).to include(user) - described_class.add_user(source, user, :maintainer, current_user: user) + described_class.add_member(source, user, :maintainer, current_user: user) expect(source.members.find_by(user_id: user).access_level).to eq(Gitlab::Access::DEVELOPER) end @@ -345,12 +345,12 @@ RSpec.shared_examples_for "bulk member creation" do # maintainers cannot add owners source.add_maintainer(user) - expect(described_class.add_users(source, [user1, user2], :owner, current_user: user)).to be_empty + expect(described_class.add_members(source, [user1, user2], :owner, current_user: user)).to be_empty end end it 'returns Member objects' do - members = described_class.add_users(source, [user1, user2], :maintainer) + members = described_class.add_members(source, [user1, user2], :maintainer) expect(members.map(&:user)).to contain_exactly(user1, user2) expect(members).to all(be_a(member_type)) @@ -358,7 +358,7 @@ RSpec.shared_examples_for "bulk member creation" do end it 'returns an empty array' do - members = described_class.add_users(source, [], :maintainer) + members = described_class.add_members(source, [], :maintainer) expect(members).to be_a Array expect(members).to be_empty @@ -367,7 +367,7 @@ RSpec.shared_examples_for "bulk member creation" do it 'supports different formats' do list = ['joe@local.test', admin, user1.id, user2.id.to_s] - members = described_class.add_users(source, list, :maintainer) + members = described_class.add_members(source, list, :maintainer) expect(members.size).to eq(4) expect(members.first).to be_invite @@ -375,7 +375,7 @@ RSpec.shared_examples_for "bulk member creation" do context 'with de-duplication' do it 'has the same user by id and user' do - members = described_class.add_users(source, [user1.id, user1, user1.id, user2, user2.id, user2], :maintainer) + members = described_class.add_members(source, [user1.id, user1, user1.id, user2, user2.id, user2], :maintainer) expect(members.map(&:user)).to contain_exactly(user1, user2) expect(members).to all(be_a(member_type)) @@ -383,7 +383,7 @@ RSpec.shared_examples_for "bulk member creation" do end it 'has the same user sent more than once' do - members = described_class.add_users(source, [user1, user1], :maintainer) + members = described_class.add_members(source, [user1, user1], :maintainer) expect(members.map(&:user)).to contain_exactly(user1) expect(members).to all(be_a(member_type)) @@ -392,7 +392,7 @@ RSpec.shared_examples_for "bulk member creation" do end it 'with the same user sent more than once by user and by email' do - members = described_class.add_users(source, [user1, user1.email], :maintainer) + members = described_class.add_members(source, [user1, user1.email], :maintainer) expect(members.map(&:user)).to contain_exactly(user1) expect(members).to all(be_a(member_type)) @@ -400,7 +400,7 @@ RSpec.shared_examples_for "bulk member creation" do end it 'with the same user sent more than once by user id and by email' do - members = described_class.add_users(source, [user1.id, user1.email], :maintainer) + members = described_class.add_members(source, [user1.id, user1.email], :maintainer) expect(members.map(&:user)).to contain_exactly(user1) expect(members).to all(be_a(member_type)) @@ -409,12 +409,12 @@ RSpec.shared_examples_for "bulk member creation" do context 'when a member already exists' do before do - source.add_user(user1, :developer) + source.add_member(user1, :developer) end it 'has the same user sent more than once with the member already existing' do expect do - members = described_class.add_users(source, [user1, user1, user2], :maintainer) + members = described_class.add_members(source, [user1, user1, user2], :maintainer) expect(members.map(&:user)).to contain_exactly(user1, user2) expect(members).to all(be_a(member_type)) expect(members).to all(be_persisted) @@ -425,7 +425,7 @@ RSpec.shared_examples_for "bulk member creation" do user3 = create(:user) expect do - members = described_class.add_users(source, [user1.id, user2, user3.id], :maintainer) + members = described_class.add_members(source, [user1.id, user2, user3.id], :maintainer) expect(members.map(&:user)).to contain_exactly(user1, user2, user3) expect(members).to all(be_a(member_type)) expect(members).to all(be_persisted) @@ -436,7 +436,7 @@ RSpec.shared_examples_for "bulk member creation" do user3 = create(:user) expect do - members = described_class.add_users(source, [user1, user2, user3], :maintainer) + members = described_class.add_members(source, [user1, user2, user3], :maintainer) expect(members.map(&:user)).to contain_exactly(user1, user2, user3) expect(members).to all(be_a(member_type)) expect(members).to all(be_persisted) @@ -448,7 +448,7 @@ RSpec.shared_examples_for "bulk member creation" do let(:task_project) { source.is_a?(Group) ? create(:project, group: source) : source } it 'creates a member_task with the correct attributes', :aggregate_failures do - members = described_class.add_users(source, [user1], :developer, tasks_to_be_done: %w(ci code), tasks_project_id: task_project.id) + members = described_class.add_members(source, [user1], :developer, tasks_to_be_done: %w(ci code), tasks_project_id: task_project.id) member = members.last expect(member.tasks_to_be_done).to match_array([:ci, :code]) @@ -457,7 +457,7 @@ RSpec.shared_examples_for "bulk member creation" do context 'with an already existing member' do before do - source.add_user(user1, :developer) + source.add_member(user1, :developer) end it 'does not update tasks to be done if tasks already exist', :aggregate_failures do @@ -465,7 +465,7 @@ RSpec.shared_examples_for "bulk member creation" do create(:member_task, member: member, project: task_project, tasks_to_be_done: %w(code ci)) expect do - described_class.add_users(source, + described_class.add_members(source, [user1.id], :developer, tasks_to_be_done: %w(issues), @@ -479,7 +479,7 @@ RSpec.shared_examples_for "bulk member creation" do it 'adds tasks to be done if they do not exist', :aggregate_failures do expect do - described_class.add_users(source, + described_class.add_members(source, [user1.id], :developer, tasks_to_be_done: %w(issues), diff --git a/spec/support/shared_examples/requests/api/conan_packages_shared_examples.rb b/spec/support/shared_examples/requests/api/conan_packages_shared_examples.rb index d7c2b645de9..bb2f8965294 100644 --- a/spec/support/shared_examples/requests/api/conan_packages_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/conan_packages_shared_examples.rb @@ -72,7 +72,7 @@ RSpec.shared_examples 'conan search endpoint' do project.update!(visibility: 'private') project.team.truncate user.project_authorizations.delete_all - project.add_user(user, role) unless role == :anonymous + project.add_member(user, role) unless role == :anonymous get api(url), params: params, headers: headers end diff --git a/spec/views/shared/deploy_tokens/_form.html.haml_spec.rb b/spec/views/shared/deploy_tokens/_form.html.haml_spec.rb index 5ac42952f78..74ad0ccb77a 100644 --- a/spec/views/shared/deploy_tokens/_form.html.haml_spec.rb +++ b/spec/views/shared/deploy_tokens/_form.html.haml_spec.rb @@ -10,7 +10,7 @@ RSpec.describe 'shared/deploy_tokens/_form.html.haml' do RSpec.shared_examples "display deploy token settings" do |role, shows_package_registry_permissions| before do - subject.add_user(user, role) + subject.add_member(user, role) allow(view).to receive(:current_user).and_return(user) stub_config(packages: { enabled: packages_enabled }) end diff --git a/spec/workers/disallow_two_factor_for_group_worker_spec.rb b/spec/workers/disallow_two_factor_for_group_worker_spec.rb index a69dd893f81..f30b12dd7f4 100644 --- a/spec/workers/disallow_two_factor_for_group_worker_spec.rb +++ b/spec/workers/disallow_two_factor_for_group_worker_spec.rb @@ -13,7 +13,7 @@ RSpec.describe DisallowTwoFactorForGroupWorker do end it "updates group members" do - group.add_user(user, GroupMember::DEVELOPER) + group.add_member(user, GroupMember::DEVELOPER) described_class.new.perform(group.id) diff --git a/spec/workers/remove_expired_members_worker_spec.rb b/spec/workers/remove_expired_members_worker_spec.rb index 6d0d4aeef89..8d7d488094f 100644 --- a/spec/workers/remove_expired_members_worker_spec.rb +++ b/spec/workers/remove_expired_members_worker_spec.rb @@ -47,7 +47,7 @@ RSpec.describe RemoveExpiredMembersWorker do let_it_be(:expired_project_bot) { create(:user, :project_bot) } before do - project.add_user(expired_project_bot, :maintainer, expires_at: 1.day.from_now) + project.add_member(expired_project_bot, :maintainer, expires_at: 1.day.from_now) travel_to(3.days.from_now) end @@ -67,7 +67,7 @@ RSpec.describe RemoveExpiredMembersWorker do let_it_be(:other_project_bot) { create(:user, :project_bot) } before do - project.add_user(other_project_bot, :maintainer, expires_at: 10.days.from_now) + project.add_member(other_project_bot, :maintainer, expires_at: 10.days.from_now) travel_to(3.days.from_now) end |