diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-02-13 21:08:59 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-02-13 21:08:59 +0000 |
commit | d466ee5042520ad078fe050cb078d81dc2ebe196 (patch) | |
tree | 5648eb1aee8aeff5b5c5ff4669a184fd7676f778 /spec | |
parent | 6a9d7c009e4e5975a89bcc3e458da4b3ec484bd1 (diff) | |
download | gitlab-ce-d466ee5042520ad078fe050cb078d81dc2ebe196.tar.gz |
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'spec')
-rw-r--r-- | spec/factories/project_authorizations.rb | 9 | ||||
-rw-r--r-- | spec/frontend/ide/components/error_message_spec.js | 29 | ||||
-rw-r--r-- | spec/javascripts/ide/components/ide_spec.js | 4 | ||||
-rw-r--r-- | spec/javascripts/vue_shared/components/tooltip_on_truncate_spec.js | 211 | ||||
-rw-r--r-- | spec/lib/gitlab/background_migration/recalculate_project_authorizations_spec.rb | 243 | ||||
-rw-r--r-- | spec/lib/gitlab/diff/file_spec.rb | 2 | ||||
-rw-r--r-- | spec/lib/gitlab/gitaly_client/commit_service_spec.rb | 15 | ||||
-rw-r--r-- | spec/lib/gitlab/project_authorizations_spec.rb | 103 | ||||
-rw-r--r-- | spec/migrations/schedule_recalculate_project_authorizations_spec.rb | 57 | ||||
-rw-r--r-- | spec/models/project_spec.rb | 17 | ||||
-rw-r--r-- | spec/models/repository_spec.rb | 8 | ||||
-rw-r--r-- | spec/requests/api/commits_spec.rb | 35 | ||||
-rw-r--r-- | spec/requests/api/issues/issues_spec.rb | 9 | ||||
-rw-r--r-- | spec/requests/api/merge_requests_spec.rb | 290 | ||||
-rw-r--r-- | spec/services/users/refresh_authorized_projects_service_spec.rb | 36 | ||||
-rw-r--r-- | spec/support/helpers/api_helpers.rb | 11 |
16 files changed, 745 insertions, 334 deletions
diff --git a/spec/factories/project_authorizations.rb b/spec/factories/project_authorizations.rb new file mode 100644 index 00000000000..ffdf5576f84 --- /dev/null +++ b/spec/factories/project_authorizations.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :project_authorization do + user + project + access_level { Gitlab::Access::REPORTER } + end +end diff --git a/spec/frontend/ide/components/error_message_spec.js b/spec/frontend/ide/components/error_message_spec.js index 1de496ba3f8..3a4dcc5873d 100644 --- a/spec/frontend/ide/components/error_message_spec.js +++ b/spec/frontend/ide/components/error_message_spec.js @@ -1,4 +1,4 @@ -import { shallowMount, createLocalVue } from '@vue/test-utils'; +import { mount, createLocalVue } from '@vue/test-utils'; import Vuex from 'vuex'; import { GlLoadingIcon } from '@gitlab/ui'; import ErrorMessage from '~/ide/components/error_message.vue'; @@ -15,7 +15,7 @@ describe('IDE error message component', () => { actions: { setErrorMessage: setErrorMessageMock }, }); - wrapper = shallowMount(ErrorMessage, { + wrapper = mount(ErrorMessage, { propsData: { message: { text: 'some text', @@ -38,15 +38,18 @@ describe('IDE error message component', () => { wrapper = null; }); + const findDismissButton = () => wrapper.find('button[aria-label=Dismiss]'); + const findActionButton = () => wrapper.find('button.gl-alert-action'); + it('renders error message', () => { const text = 'error message'; createComponent({ text }); expect(wrapper.text()).toContain(text); }); - it('clears error message on click', () => { + it('clears error message on dismiss click', () => { createComponent(); - wrapper.trigger('click'); + findDismissButton().trigger('click'); expect(setErrorMessageMock).toHaveBeenCalledWith(expect.any(Object), null, undefined); }); @@ -68,29 +71,27 @@ describe('IDE error message component', () => { }); it('renders action button', () => { - const button = wrapper.find('button'); + const button = findActionButton(); expect(button.exists()).toBe(true); expect(button.text()).toContain(message.actionText); }); - it('does not clear error message on click', () => { - wrapper.trigger('click'); - - expect(setErrorMessageMock).not.toHaveBeenCalled(); + it('does not show dismiss button', () => { + expect(findDismissButton().exists()).toBe(false); }); it('dispatches action', () => { - wrapper.find('button').trigger('click'); + findActionButton().trigger('click'); expect(actionMock).toHaveBeenCalledWith(message.actionPayload); }); it('does not dispatch action when already loading', () => { - wrapper.find('button').trigger('click'); + findActionButton().trigger('click'); actionMock.mockReset(); return wrapper.vm.$nextTick(() => { - wrapper.find('button').trigger('click'); + findActionButton().trigger('click'); return wrapper.vm.$nextTick().then(() => { expect(actionMock).not.toHaveBeenCalled(); @@ -106,7 +107,7 @@ describe('IDE error message component', () => { resolveAction = resolve; }), ); - wrapper.find('button').trigger('click'); + findActionButton().trigger('click'); return wrapper.vm.$nextTick(() => { expect(wrapper.find(GlLoadingIcon).isVisible()).toBe(true); @@ -115,7 +116,7 @@ describe('IDE error message component', () => { }); it('hides loading icon when operation finishes', () => { - wrapper.find('button').trigger('click'); + findActionButton().trigger('click'); return actionMock() .then(() => wrapper.vm.$nextTick()) .then(() => { diff --git a/spec/javascripts/ide/components/ide_spec.js b/spec/javascripts/ide/components/ide_spec.js index 048db4a7533..4241b994cba 100644 --- a/spec/javascripts/ide/components/ide_spec.js +++ b/spec/javascripts/ide/components/ide_spec.js @@ -61,14 +61,14 @@ describe('ide component, non-empty repo', () => { }); it('shows error message when set', done => { - expect(vm.$el.querySelector('.flash-container')).toBe(null); + expect(vm.$el.querySelector('.gl-alert')).toBe(null); vm.$store.state.errorMessage = { text: 'error', }; vm.$nextTick(() => { - expect(vm.$el.querySelector('.flash-container')).not.toBe(null); + expect(vm.$el.querySelector('.gl-alert')).not.toBe(null); done(); }); diff --git a/spec/javascripts/vue_shared/components/tooltip_on_truncate_spec.js b/spec/javascripts/vue_shared/components/tooltip_on_truncate_spec.js index a8d39b7b5fe..5f432f2a1b5 100644 --- a/spec/javascripts/vue_shared/components/tooltip_on_truncate_spec.js +++ b/spec/javascripts/vue_shared/components/tooltip_on_truncate_spec.js @@ -1,149 +1,160 @@ -import { shallowMount, createLocalVue } from '@vue/test-utils'; +import { mount, shallowMount } from '@vue/test-utils'; import TooltipOnTruncate from '~/vue_shared/components/tooltip_on_truncate.vue'; -const TEST_TITLE = 'lorem-ipsum-dolar-sit-amit-consectur-adipiscing-elit-sed-do'; -const STYLE_TRUNCATED = 'display: inline-block; max-width: 20px;'; -const STYLE_NORMAL = 'display: inline-block; max-width: 1000px;'; +const TEXT_SHORT = 'lorem'; +const TEXT_LONG = 'lorem-ipsum-dolar-sit-amit-consectur-adipiscing-elit-sed-do'; -const localVue = createLocalVue(); +const TEXT_TRUNCATE = 'white-space: nowrap; overflow:hidden;'; +const STYLE_NORMAL = `${TEXT_TRUNCATE} display: inline-block; max-width: 1000px;`; // does not overflows +const STYLE_OVERFLOWED = `${TEXT_TRUNCATE} display: inline-block; max-width: 50px;`; // overflowed when text is long const createElementWithStyle = (style, content) => `<a href="#" style="${style}">${content}</a>`; describe('TooltipOnTruncate component', () => { let wrapper; + let parent; const createComponent = ({ propsData, ...options } = {}) => { - wrapper = shallowMount(localVue.extend(TooltipOnTruncate), { - localVue, + wrapper = shallowMount(TooltipOnTruncate, { attachToDocument: true, propsData: { - title: TEST_TITLE, ...propsData, }, + attrs: { + style: STYLE_OVERFLOWED, + }, ...options, }); }; + const createWrappedComponent = ({ propsData, ...options }) => { + // set a parent around the tested component + parent = mount( + { + props: { + title: { default: '' }, + }, + template: ` + <TooltipOnTruncate :title="title" truncate-target="child" style="${STYLE_OVERFLOWED}"> + <div>{{title}}</div> + </TooltipOnTruncate> + `, + components: { + TooltipOnTruncate, + }, + }, + { + propsData: { ...propsData }, + attachToDocument: true, + ...options, + }, + ); + + wrapper = parent.find(TooltipOnTruncate); + }; + + const hasTooltip = () => wrapper.classes('js-show-tooltip'); + afterEach(() => { wrapper.destroy(); }); - const hasTooltip = () => wrapper.classes('js-show-tooltip'); - describe('with default target', () => { - it('renders tooltip if truncated', done => { + it('renders tooltip if truncated', () => { createComponent({ - attrs: { - style: STYLE_TRUNCATED, + propsData: { + title: TEXT_LONG, }, slots: { - default: [TEST_TITLE], + default: [TEXT_LONG], }, }); - wrapper.vm - .$nextTick() - .then(() => { - expect(hasTooltip()).toBe(true); - expect(wrapper.attributes('data-original-title')).toEqual(TEST_TITLE); - expect(wrapper.attributes('data-placement')).toEqual('top'); - }) - .then(done) - .catch(done.fail); + return wrapper.vm.$nextTick().then(() => { + expect(hasTooltip()).toBe(true); + expect(wrapper.attributes('data-original-title')).toEqual(TEXT_LONG); + expect(wrapper.attributes('data-placement')).toEqual('top'); + }); }); - it('does not render tooltip if normal', done => { + it('does not render tooltip if normal', () => { createComponent({ - attrs: { - style: STYLE_NORMAL, + propsData: { + title: TEXT_SHORT, }, slots: { - default: [TEST_TITLE], + default: [TEXT_SHORT], }, }); - wrapper.vm - .$nextTick() - .then(() => { - expect(hasTooltip()).toBe(false); - }) - .then(done) - .catch(done.fail); + return wrapper.vm.$nextTick().then(() => { + expect(hasTooltip()).toBe(false); + }); }); }); describe('with child target', () => { - it('renders tooltip if truncated', done => { + it('renders tooltip if truncated', () => { createComponent({ attrs: { style: STYLE_NORMAL, }, propsData: { + title: TEXT_LONG, truncateTarget: 'child', }, slots: { - default: createElementWithStyle(STYLE_TRUNCATED, TEST_TITLE), + default: createElementWithStyle(STYLE_OVERFLOWED, TEXT_LONG), }, }); - wrapper.vm - .$nextTick() - .then(() => { - expect(hasTooltip()).toBe(true); - }) - .then(done) - .catch(done.fail); + return wrapper.vm.$nextTick().then(() => { + expect(hasTooltip()).toBe(true); + }); }); - it('does not render tooltip if normal', done => { + it('does not render tooltip if normal', () => { createComponent({ propsData: { truncateTarget: 'child', }, slots: { - default: createElementWithStyle(STYLE_NORMAL, TEST_TITLE), + default: createElementWithStyle(STYLE_NORMAL, TEXT_LONG), }, }); - wrapper.vm - .$nextTick() - .then(() => { - expect(hasTooltip()).toBe(false); - }) - .then(done) - .catch(done.fail); + return wrapper.vm.$nextTick().then(() => { + expect(hasTooltip()).toBe(false); + }); }); }); describe('with fn target', () => { - it('renders tooltip if truncated', done => { + it('renders tooltip if truncated', () => { createComponent({ attrs: { style: STYLE_NORMAL, }, propsData: { + title: TEXT_LONG, truncateTarget: el => el.childNodes[1], }, slots: { default: [ - createElementWithStyle('', TEST_TITLE), - createElementWithStyle(STYLE_TRUNCATED, TEST_TITLE), + createElementWithStyle('', TEXT_LONG), + createElementWithStyle(STYLE_OVERFLOWED, TEXT_LONG), ], }, }); - wrapper.vm - .$nextTick() - .then(() => { - expect(hasTooltip()).toBe(true); - }) - .then(done) - .catch(done.fail); + return wrapper.vm.$nextTick().then(() => { + expect(hasTooltip()).toBe(true); + }); }); }); describe('placement', () => { - it('sets data-placement when tooltip is rendered', done => { + it('sets data-placement when tooltip is rendered', () => { const placement = 'bottom'; createComponent({ @@ -151,21 +162,75 @@ describe('TooltipOnTruncate component', () => { placement, }, attrs: { - style: STYLE_TRUNCATED, + style: STYLE_OVERFLOWED, }, slots: { - default: TEST_TITLE, + default: TEXT_LONG, }, }); - wrapper.vm - .$nextTick() - .then(() => { - expect(hasTooltip()).toBe(true); - expect(wrapper.attributes('data-placement')).toEqual(placement); - }) - .then(done) - .catch(done.fail); + return wrapper.vm.$nextTick().then(() => { + expect(hasTooltip()).toBe(true); + expect(wrapper.attributes('data-placement')).toEqual(placement); + }); + }); + }); + + describe('updates when title and slot content changes', () => { + describe('is initialized with a long text', () => { + beforeEach(() => { + createWrappedComponent({ + propsData: { title: TEXT_LONG }, + }); + return parent.vm.$nextTick(); + }); + + it('renders tooltip', () => { + expect(hasTooltip()).toBe(true); + expect(wrapper.attributes('data-original-title')).toEqual(TEXT_LONG); + expect(wrapper.attributes('data-placement')).toEqual('top'); + }); + + it('does not render tooltip after updated to a short text', () => { + parent.setProps({ + title: TEXT_SHORT, + }); + + return wrapper.vm + .$nextTick() + .then(() => wrapper.vm.$nextTick()) // wait 2 times to get an updated slot + .then(() => { + expect(hasTooltip()).toBe(false); + }); + }); + }); + + describe('is initialized with a short text', () => { + beforeEach(() => { + createWrappedComponent({ + propsData: { title: TEXT_SHORT }, + }); + return wrapper.vm.$nextTick(); + }); + + it('does not render tooltip', () => { + expect(hasTooltip()).toBe(false); + }); + + it('renders tooltip after updated to a long text', () => { + parent.setProps({ + title: TEXT_LONG, + }); + + return wrapper.vm + .$nextTick() + .then(() => wrapper.vm.$nextTick()) // wait 2 times to get an updated slot + .then(() => { + expect(hasTooltip()).toBe(true); + expect(wrapper.attributes('data-original-title')).toEqual(TEXT_LONG); + expect(wrapper.attributes('data-placement')).toEqual('top'); + }); + }); }); }); }); diff --git a/spec/lib/gitlab/background_migration/recalculate_project_authorizations_spec.rb b/spec/lib/gitlab/background_migration/recalculate_project_authorizations_spec.rb new file mode 100644 index 00000000000..1ef2c451aa2 --- /dev/null +++ b/spec/lib/gitlab/background_migration/recalculate_project_authorizations_spec.rb @@ -0,0 +1,243 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::BackgroundMigration::RecalculateProjectAuthorizations, :migration, schema: 20200204113223 do + let(:users_table) { table(:users) } + let(:namespaces_table) { table(:namespaces) } + let(:projects_table) { table(:projects) } + let(:project_authorizations_table) { table(:project_authorizations) } + let(:members_table) { table(:members) } + let(:group_group_links) { table(:group_group_links) } + let(:project_group_links) { table(:project_group_links) } + + let(:user) { users_table.create!(id: 1, email: 'user@example.com', projects_limit: 10) } + let(:group) { namespaces_table.create!(type: 'Group', name: 'group', path: 'group') } + + subject { described_class.new.perform([user.id]) } + + context 'missing authorization' do + context 'personal project' do + before do + user_namespace = namespaces_table.create!(owner_id: user.id, name: 'User', path: 'user') + projects_table.create!(id: 1, + name: 'personal-project', + path: 'personal-project', + visibility_level: 0, + namespace_id: user_namespace.id) + end + + it 'creates correct authorization' do + expect { subject }.to change { project_authorizations_table.count }.from(0).to(1) + expect(project_authorizations_table.all).to( + match_array([have_attributes(user_id: 1, project_id: 1, access_level: 40)])) + end + end + + context 'group membership' do + before do + projects_table.create!(id: 1, name: 'group-project', path: 'group-project', + visibility_level: 0, namespace_id: group.id) + members_table.create!(user_id: user.id, source_id: group.id, source_type: 'Namespace', + type: 'GroupMember', access_level: 20, notification_level: 3) + end + + it 'creates correct authorization' do + expect { subject }.to change { project_authorizations_table.count }.from(0).to(1) + expect(project_authorizations_table.all).to( + match_array([have_attributes(user_id: 1, project_id: 1, access_level: 20)])) + end + end + + context 'inherited group membership' do + before do + sub_group = namespaces_table.create!(type: 'Group', name: 'subgroup', + path: 'subgroup', parent_id: group.id) + projects_table.create!(id: 1, name: 'group-project', path: 'group-project', + visibility_level: 0, namespace_id: sub_group.id) + members_table.create!(user_id: user.id, source_id: group.id, source_type: 'Namespace', + type: 'GroupMember', access_level: 20, notification_level: 3) + end + + it 'creates correct authorization' do + expect { subject }.to change { project_authorizations_table.count }.from(0).to(1) + expect(project_authorizations_table.all).to( + match_array([have_attributes(user_id: 1, project_id: 1, access_level: 20)])) + end + end + + context 'project membership' do + before do + project = projects_table.create!(id: 1, name: 'group-project', path: 'group-project', + visibility_level: 0, namespace_id: group.id) + members_table.create!(user_id: user.id, source_id: project.id, source_type: 'Project', + type: 'ProjectMember', access_level: 20, notification_level: 3) + end + + it 'creates correct authorization' do + expect { subject }.to change { project_authorizations_table.count }.from(0).to(1) + expect(project_authorizations_table.all).to( + match_array([have_attributes(user_id: 1, project_id: 1, access_level: 20)])) + end + end + + context 'shared group' do + before do + members_table.create!(user_id: user.id, source_id: group.id, source_type: 'Namespace', + type: 'GroupMember', access_level: 30, notification_level: 3) + + shared_group = namespaces_table.create!(type: 'Group', name: 'shared group', + path: 'shared-group') + projects_table.create!(id: 1, name: 'project', path: 'project', visibility_level: 0, + namespace_id: shared_group.id) + + group_group_links.create(shared_group_id: shared_group.id, shared_with_group_id: group.id, + group_access: 20) + end + + it 'creates correct authorization' do + expect { subject }.to change { project_authorizations_table.count }.from(0).to(1) + expect(project_authorizations_table.all).to( + match_array([have_attributes(user_id: 1, project_id: 1, access_level: 20)])) + end + end + + context 'shared project' do + before do + members_table.create!(user_id: user.id, source_id: group.id, source_type: 'Namespace', + type: 'GroupMember', access_level: 30, notification_level: 3) + + another_group = namespaces_table.create!(type: 'Group', name: 'another group', path: 'another-group') + shared_project = projects_table.create!(id: 1, name: 'shared project', path: 'shared-project', + visibility_level: 0, namespace_id: another_group.id) + + project_group_links.create(project_id: shared_project.id, group_id: group.id, group_access: 20) + end + + it 'creates correct authorization' do + expect { subject }.to change { project_authorizations_table.count }.from(0).to(1) + expect(project_authorizations_table.all).to( + match_array([have_attributes(user_id: 1, project_id: 1, access_level: 20)])) + end + end + end + + context 'unapproved access requests' do + context 'group membership' do + before do + projects_table.create!(id: 1, name: 'group-project', path: 'group-project', + visibility_level: 0, namespace_id: group.id) + members_table.create!(user_id: user.id, source_id: group.id, source_type: 'Namespace', + type: 'GroupMember', access_level: 20, requested_at: Time.now, notification_level: 3) + end + + it 'does not create authorization' do + expect { subject }.not_to change { project_authorizations_table.count }.from(0) + end + end + + context 'inherited group membership' do + before do + sub_group = namespaces_table.create!(type: 'Group', name: 'subgroup', path: 'subgroup', + parent_id: group.id) + projects_table.create!(id: 1, name: 'group-project', path: 'group-project', + visibility_level: 0, namespace_id: sub_group.id) + members_table.create!(user_id: user.id, source_id: group.id, source_type: 'Namespace', + type: 'GroupMember', access_level: 20, requested_at: Time.now, notification_level: 3) + end + + it 'does not create authorization' do + expect { subject }.not_to change { project_authorizations_table.count }.from(0) + end + end + + context 'project membership' do + before do + project = projects_table.create!(id: 1, name: 'group-project', path: 'group-project', + visibility_level: 0, namespace_id: group.id) + members_table.create!(user_id: user.id, source_id: project.id, source_type: 'Project', + type: 'ProjectMember', access_level: 20, requested_at: Time.now, notification_level: 3) + end + + it 'does not create authorization' do + expect { subject }.not_to change { project_authorizations_table.count }.from(0) + end + end + + context 'shared group' do + before do + members_table.create!(user_id: user.id, source_id: group.id, source_type: 'Namespace', + type: 'GroupMember', access_level: 30, requested_at: Time.now, notification_level: 3) + + shared_group = namespaces_table.create!(type: 'Group', name: 'shared group', + path: 'shared-group') + projects_table.create!(id: 1, name: 'project', path: 'project', visibility_level: 0, + namespace_id: shared_group.id) + + group_group_links.create(shared_group_id: shared_group.id, shared_with_group_id: group.id, + group_access: 20) + end + + it 'does not create authorization' do + expect { subject }.not_to change { project_authorizations_table.count }.from(0) + end + end + + context 'shared project' do + before do + members_table.create!(user_id: user.id, source_id: group.id, source_type: 'Namespace', + type: 'GroupMember', access_level: 30, requested_at: Time.now, notification_level: 3) + + another_group = namespaces_table.create!(type: 'Group', name: 'another group', path: 'another-group') + shared_project = projects_table.create!(id: 1, name: 'shared project', path: 'shared-project', + visibility_level: 0, namespace_id: another_group.id) + + project_group_links.create(project_id: shared_project.id, group_id: group.id, group_access: 20) + end + + it 'does not create authorization' do + expect { subject }.not_to change { project_authorizations_table.count }.from(0) + end + end + end + + context 'incorrect authorization' do + before do + project = projects_table.create!(id: 1, name: 'group-project', path: 'group-project', + visibility_level: 0, namespace_id: group.id) + members_table.create!(user_id: user.id, source_id: group.id, source_type: 'Namespace', + type: 'GroupMember', access_level: 30, notification_level: 3) + + project_authorizations_table.create!(user_id: user.id, project_id: project.id, + access_level: 10) + end + + it 'fixes authorization' do + expect { subject }.not_to change { project_authorizations_table.count }.from(1) + expect(project_authorizations_table.all).to( + match_array([have_attributes(user_id: 1, project_id: 1, access_level: 30)])) + end + end + + context 'unwanted authorization' do + before do + project = projects_table.create!(name: 'group-project', path: 'group-project', + visibility_level: 0, namespace_id: group.id) + + project_authorizations_table.create!(user_id: user.id, project_id: project.id, + access_level: 10) + end + + it 'deletes authorization' do + expect { subject }.to change { project_authorizations_table.count }.from(1).to(0) + end + end + + context 'deleted user' do + let(:nonexistent_user_id) { User.maximum(:id).to_i + 999 } + + it 'does not fail' do + expect { described_class.new.perform([nonexistent_user_id]) }.not_to raise_error + end + end +end diff --git a/spec/lib/gitlab/diff/file_spec.rb b/spec/lib/gitlab/diff/file_spec.rb index 4733607ccc0..61d7400b95e 100644 --- a/spec/lib/gitlab/diff/file_spec.rb +++ b/spec/lib/gitlab/diff/file_spec.rb @@ -175,7 +175,7 @@ describe Gitlab::Diff::File do [diff_file.new_content_sha, diff_file.new_path], [diff_file.old_content_sha, diff_file.old_path] ] - expect(project.repository).to receive(:blobs_at).with(items, blob_size_limit: 100 * 1024).and_call_original + expect(project.repository).to receive(:blobs_at).with(items, blob_size_limit: 10.megabytes).and_call_original old_data = diff_file.old_blob.data data = diff_file.new_blob.data diff --git a/spec/lib/gitlab/gitaly_client/commit_service_spec.rb b/spec/lib/gitlab/gitaly_client/commit_service_spec.rb index 820578dfc6e..5e1d6199c3c 100644 --- a/spec/lib/gitlab/gitaly_client/commit_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/commit_service_spec.rb @@ -279,4 +279,19 @@ describe Gitlab::GitalyClient::CommitService do expect(subject.deletions).to eq(15) end end + + describe '#find_commits' do + it 'sends an RPC request' do + request = Gitaly::FindCommitsRequest.new( + repository: repository_message, + disable_walk: true, + order: 'TOPO' + ) + + expect_any_instance_of(Gitaly::CommitService::Stub).to receive(:find_commits) + .with(request, kind_of(Hash)).and_return([]) + + client.find_commits(order: 'topo') + end + end end diff --git a/spec/lib/gitlab/project_authorizations_spec.rb b/spec/lib/gitlab/project_authorizations_spec.rb index 6e5c36172e2..1c579128223 100644 --- a/spec/lib/gitlab/project_authorizations_spec.rb +++ b/spec/lib/gitlab/project_authorizations_spec.rb @@ -97,87 +97,68 @@ describe Gitlab::ProjectAuthorizations do create(:group_group_link, shared_group: shared_group, shared_with_group: group) end - context 'when feature flag share_group_with_group is enabled' do - before do - stub_feature_flags(share_group_with_group: true) - end - - context 'group user' do - let(:user) { group_user } + context 'group user' do + let(:user) { group_user } - it 'creates proper authorizations' do - mapping = map_access_levels(authorizations) + it 'creates proper authorizations' do + mapping = map_access_levels(authorizations) - expect(mapping[project_parent.id]).to be_nil - expect(mapping[project.id]).to eq(Gitlab::Access::DEVELOPER) - expect(mapping[project_child.id]).to eq(Gitlab::Access::DEVELOPER) - end + expect(mapping[project_parent.id]).to be_nil + expect(mapping[project.id]).to eq(Gitlab::Access::DEVELOPER) + expect(mapping[project_child.id]).to eq(Gitlab::Access::DEVELOPER) end + end - context 'parent group user' do - let(:user) { parent_group_user } + context 'parent group user' do + let(:user) { parent_group_user } - it 'creates proper authorizations' do - mapping = map_access_levels(authorizations) + it 'creates proper authorizations' do + mapping = map_access_levels(authorizations) - expect(mapping[project_parent.id]).to be_nil - expect(mapping[project.id]).to be_nil - expect(mapping[project_child.id]).to be_nil - end + expect(mapping[project_parent.id]).to be_nil + expect(mapping[project.id]).to be_nil + expect(mapping[project_child.id]).to be_nil end + end - context 'child group user' do - let(:user) { child_group_user } + context 'child group user' do + let(:user) { child_group_user } - it 'creates proper authorizations' do - mapping = map_access_levels(authorizations) + it 'creates proper authorizations' do + mapping = map_access_levels(authorizations) - expect(mapping[project_parent.id]).to be_nil - expect(mapping[project.id]).to be_nil - expect(mapping[project_child.id]).to be_nil - end + expect(mapping[project_parent.id]).to be_nil + expect(mapping[project.id]).to be_nil + expect(mapping[project_child.id]).to be_nil end end - context 'when feature flag share_group_with_group is disabled' do - before do - stub_feature_flags(share_group_with_group: false) - end - - context 'group user' do - let(:user) { group_user } - - it 'creates proper authorizations' do - mapping = map_access_levels(authorizations) + context 'user without accepted access request' do + let!(:user) { create(:user) } - expect(mapping[project_parent.id]).to be_nil - expect(mapping[project.id]).to be_nil - expect(mapping[project_child.id]).to be_nil - end - end + it 'does not have access to group and its projects' do + create(:group_member, :developer, :access_request, user: user, group: group) - context 'parent group user' do - let(:user) { parent_group_user } + mapping = map_access_levels(authorizations) - it 'creates proper authorizations' do - mapping = map_access_levels(authorizations) - - expect(mapping[project_parent.id]).to be_nil - expect(mapping[project.id]).to be_nil - expect(mapping[project_child.id]).to be_nil - end + expect(mapping[project_parent.id]).to be_nil + expect(mapping[project.id]).to be_nil + expect(mapping[project_child.id]).to be_nil end + end - context 'child group user' do - let(:user) { child_group_user } + context 'unrelated project owner' do + let(:common_id) { [Project.maximum(:id).to_i, Namespace.maximum(:id).to_i].max + 999 } + let!(:group) { create(:group, id: common_id) } + let!(:unrelated_project) { create(:project, id: common_id) } + let(:user) { unrelated_project.owner } - it 'creates proper authorizations' do - mapping = map_access_levels(authorizations) + it 'does not have access to group and its projects' do + mapping = map_access_levels(authorizations) - expect(mapping[project_parent.id]).to be_nil - expect(mapping[project.id]).to be_nil - expect(mapping[project_child.id]).to be_nil - end + expect(mapping[project_parent.id]).to be_nil + expect(mapping[project.id]).to be_nil + expect(mapping[project_child.id]).to be_nil end end end diff --git a/spec/migrations/schedule_recalculate_project_authorizations_spec.rb b/spec/migrations/schedule_recalculate_project_authorizations_spec.rb new file mode 100644 index 00000000000..77ad2b2dc8e --- /dev/null +++ b/spec/migrations/schedule_recalculate_project_authorizations_spec.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20200204113223_schedule_recalculate_project_authorizations.rb') + +describe ScheduleRecalculateProjectAuthorizations, :migration do + let(:users_table) { table(:users) } + let(:namespaces_table) { table(:namespaces) } + let(:projects_table) { table(:projects) } + let(:project_authorizations_table) { table(:project_authorizations) } + + let(:user1) { users_table.create!(name: 'user1', email: 'user1@example.com', projects_limit: 1) } + let(:user2) { users_table.create!(name: 'user2', email: 'user2@example.com', projects_limit: 1) } + let(:group) { namespaces_table.create!(id: 1, type: 'Group', name: 'group', path: 'group') } + let(:project) do + projects_table.create!(id: 1, name: 'project', path: 'project', + visibility_level: 0, namespace_id: group.id) + end + + before do + stub_const("#{described_class}::BATCH_SIZE", 1) + + project_authorizations_table.create!(user_id: user1.id, project_id: project.id, access_level: 30) + project_authorizations_table.create!(user_id: user2.id, project_id: project.id, access_level: 30) + end + + it 'schedules background migration' do + Sidekiq::Testing.fake! do + Timecop.freeze do + migrate! + + expect(BackgroundMigrationWorker.jobs.size).to eq(2) + expect(described_class::MIGRATION).to be_scheduled_migration([user1.id]) + expect(described_class::MIGRATION).to be_scheduled_migration([user2.id]) + end + end + end + + it 'ignores projects with higher id than maximum group id' do + another_user = users_table.create!(name: 'another user', email: 'another-user@example.com', + projects_limit: 1) + ignored_project = projects_table.create!(id: 2, name: 'ignored-project', path: 'ignored-project', + visibility_level: 0, namespace_id: group.id) + project_authorizations_table.create!(user_id: another_user.id, project_id: ignored_project.id, + access_level: 30) + + Sidekiq::Testing.fake! do + Timecop.freeze do + migrate! + + expect(BackgroundMigrationWorker.jobs.size).to eq(2) + expect(described_class::MIGRATION).to be_scheduled_migration([user1.id]) + expect(described_class::MIGRATION).to be_scheduled_migration([user2.id]) + end + end + end +end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 5c56d1aa757..79d9e42666c 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1980,6 +1980,23 @@ describe Project do expect(project.reload.import_url).to eq('http://test.com') end + + it 'saves the url credentials percent decoded' do + url = 'http://user:pass%21%3F%40@github.com/t.git' + project = build(:project, import_url: url) + + # When the credentials are not decoded this expectation fails + expect(project.import_url).to eq(url) + expect(project.import_data.credentials).to eq(user: 'user', password: 'pass!?@') + end + + it 'saves url with no credentials' do + url = 'http://github.com/t.git' + project = build(:project, import_url: url) + + expect(project.import_url).to eq(url) + expect(project.import_data.credentials).to eq(user: nil, password: nil) + end end describe '#container_registry_url' do diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 0adf3fc8e85..00ffc3cae54 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -325,6 +325,14 @@ describe Repository do expect(repository.commits(nil, all: true, limit: 60).size).to eq(60) end end + + context "when 'order' flag is set" do + it 'passes order option to perform the query' do + expect(Gitlab::Git::Commit).to receive(:where).with(a_hash_including(order: 'topo')).and_call_original + + repository.commits('master', limit: 1, order: 'topo') + end + end end describe '#new_commits' do diff --git a/spec/requests/api/commits_spec.rb b/spec/requests/api/commits_spec.rb index e390f3945a9..170b9ccccf8 100644 --- a/spec/requests/api/commits_spec.rb +++ b/spec/requests/api/commits_spec.rb @@ -12,7 +12,6 @@ describe API::Commits do let(:project) { create(:project, :repository, creator: user, path: 'my.project') } let(:branch_with_dot) { project.repository.find_branch('ends-with.json') } let(:branch_with_slash) { project.repository.find_branch('improve/awesome') } - let(:project_id) { project.id } let(:current_user) { nil } @@ -241,6 +240,40 @@ describe API::Commits do end end end + + context 'with order parameter' do + let(:route) { "/projects/#{project_id}/repository/commits?ref_name=0031876&per_page=6&order=#{order}" } + + context 'set to topo' do + let(:order) { 'topo' } + + # git log --graph -n 6 --pretty=format:"%h" --topo-order 0031876 + # * 0031876 + # |\ + # | * 48ca272 + # | * 335bc94 + # * | bf6e164 + # * | 9d526f8 + # |/ + # * 1039376 + it 'returns project commits ordered by topo order' do + commits = project.repository.commits("0031876", limit: 6, order: 'topo') + + get api(route, current_user) + + expect(json_response.size).to eq(6) + expect(json_response.map { |entry| entry["id"] }).to eq(commits.map(&:id)) + end + end + + context 'set to blank' do + let(:order) { '' } + + it_behaves_like '400 response' do + let(:request) { get api(route, current_user) } + end + end + end end end diff --git a/spec/requests/api/issues/issues_spec.rb b/spec/requests/api/issues/issues_spec.rb index 09e005398a9..19422d4ca39 100644 --- a/spec/requests/api/issues/issues_spec.rb +++ b/spec/requests/api/issues/issues_spec.rb @@ -795,13 +795,13 @@ describe API::Issues do it 'returns issues from non archived projects only by default' do get api("/groups/#{group1.id}/issues", user), params: { scope: 'all' } - expect_response_contain_exactly(issue2, issue1) + expect_paginated_array_response([issue2.id, issue1.id]) end it 'returns issues from archived and non archived projects when non_archived is false' do get api("/groups/#{group1.id}/issues", user), params: { non_archived: false, scope: 'all' } - expect_response_contain_exactly(issue1, issue2, issue3) + expect_paginated_array_response([issue3.id, issue2.id, issue1.id]) end end end @@ -888,9 +888,4 @@ describe API::Issues do include_examples 'time tracking endpoints', 'issue' end - - def expect_response_contain_exactly(*items) - expect(json_response.length).to eq(items.size) - expect(json_response.map { |element| element['id'] }).to contain_exactly(*items.map(&:id)) - end end diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 427a361295c..00af0937dd7 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -41,8 +41,7 @@ describe API::MergeRequests do it 'returns merge requests for public projects' do get api(endpoint_path) - expect(response).to have_gitlab_http_status(200) - expect(json_response).to be_an Array + expect_successful_response_with_paginated_array end end @@ -87,10 +86,11 @@ describe API::MergeRequests do it 'returns an array of all merge_requests' do get api(endpoint_path, user) - expect(response).to have_gitlab_http_status(200) - expect(response).to include_pagination_headers - expect(json_response).to be_an Array - expect(json_response.length).to eq(4) + expect_paginated_array_response([ + merge_request_merged.id, merge_request_locked.id, + merge_request_closed.id, merge_request.id + ]) + expect(json_response.last['title']).to eq(merge_request.title) expect(json_response.last).to have_key('web_url') expect(json_response.last['sha']).to eq(merge_request.diff_head_sha) @@ -111,7 +111,7 @@ describe API::MergeRequests do get api(path, user) - expect(response).to have_gitlab_http_status(200) + expect_successful_response_with_paginated_array expect(json_response.last['labels'].pluck('name')).to eq([label2.title, label.title]) expect(json_response.last['labels'].first).to match_schema('/public_api/v4/label_basic') end @@ -139,11 +139,11 @@ describe API::MergeRequests do get api(path, user) - expect(response).to have_gitlab_http_status(200) - expect(response).to include_pagination_headers + expect_paginated_array_response([ + merge_request_merged.id, merge_request_locked.id, + merge_request_closed.id, merge_request.id + ]) expect(json_response.last.keys).to match_array(%w(id iid title web_url created_at description project_id state updated_at)) - expect(json_response).to be_an Array - expect(json_response.length).to eq(4) expect(json_response.last['iid']).to eq(merge_request.iid) expect(json_response.last['title']).to eq(merge_request.title) expect(json_response.last).to have_key('web_url') @@ -157,10 +157,10 @@ describe API::MergeRequests do get api(path, user) - expect(response).to have_gitlab_http_status(200) - expect(response).to include_pagination_headers - expect(json_response).to be_an Array - expect(json_response.length).to eq(4) + expect_paginated_array_response([ + merge_request_merged.id, merge_request_locked.id, + merge_request_closed.id, merge_request.id + ]) expect(json_response.last['title']).to eq(merge_request.title) end @@ -169,10 +169,7 @@ describe API::MergeRequests do get api(path, user) - expect(response).to have_gitlab_http_status(200) - expect(response).to include_pagination_headers - expect(json_response).to be_an Array - expect(json_response.length).to eq(1) + expect_paginated_array_response([merge_request.id]) expect(json_response.last['title']).to eq(merge_request.title) end @@ -181,10 +178,7 @@ describe API::MergeRequests do get api(path, user) - expect(response).to have_gitlab_http_status(200) - expect(response).to include_pagination_headers - expect(json_response).to be_an Array - expect(json_response.length).to eq(1) + expect_paginated_array_response([merge_request_closed.id]) expect(json_response.first['title']).to eq(merge_request_closed.title) end @@ -193,10 +187,7 @@ describe API::MergeRequests do get api(path, user) - expect(response).to have_gitlab_http_status(200) - expect(response).to include_pagination_headers - expect(json_response).to be_an Array - expect(json_response.length).to eq(1) + expect_paginated_array_response([merge_request_merged.id]) expect(json_response.first['title']).to eq(merge_request_merged.title) end @@ -210,17 +201,13 @@ describe API::MergeRequests do it 'returns an empty array if no issue matches milestone' do get api(endpoint_path, user), params: { milestone: '1.0.0' } - expect(response).to have_gitlab_http_status(200) - expect(json_response).to be_an Array - expect(json_response.length).to eq(0) + expect_empty_array_response end it 'returns an empty array if milestone does not exist' do get api(endpoint_path, user), params: { milestone: 'foo' } - expect(response).to have_gitlab_http_status(200) - expect(json_response).to be_an Array - expect(json_response.length).to eq(0) + expect_empty_array_response end it 'returns an array of merge requests in given milestone' do @@ -234,9 +221,7 @@ describe API::MergeRequests do it 'returns an array of merge requests matching state in milestone' do get api(endpoint_path, user), params: { milestone: '0.9', state: 'closed' } - expect(response).to have_gitlab_http_status(200) - expect(json_response).to be_an Array - expect(json_response.length).to eq(1) + expect_paginated_array_response([merge_request_closed.id]) expect(json_response.first['id']).to eq(merge_request_closed.id) end @@ -248,8 +233,7 @@ describe API::MergeRequests do get api(path, user) - expect(response).to have_gitlab_http_status(200) - expect(json_response).to be_an Array + expect_successful_response_with_paginated_array expect(json_response.length).to eq(1) expect(json_response.first['labels']).to eq([label2.title, label.title]) end @@ -259,9 +243,7 @@ describe API::MergeRequests do get api(path, user) - expect(response).to have_gitlab_http_status(200) - expect(json_response).to be_an Array - expect(json_response.length).to eq(0) + expect_empty_array_response end it 'returns an empty array if no merge request matches labels' do @@ -269,9 +251,7 @@ describe API::MergeRequests do get api(path, user) - expect(response).to have_gitlab_http_status(200) - expect(json_response).to be_an Array - expect(json_response.length).to eq(0) + expect_empty_array_response end it 'returns an array of labeled merge requests where all labels match' do @@ -279,8 +259,7 @@ describe API::MergeRequests do get api(path, user) - expect(response).to have_gitlab_http_status(200) - expect(json_response).to be_an Array + expect_successful_response_with_paginated_array expect(json_response.length).to eq(1) expect(json_response.first['labels']).to eq([label2.title, label.title]) end @@ -288,8 +267,7 @@ describe API::MergeRequests do it 'returns an array of merge requests with any label when filtering by any label' do get api(endpoint_path, user), params: { labels: [" #{label.title} ", " #{label2.title} "] } - expect_paginated_array_response - expect(json_response).to be_an Array + expect_successful_response_with_paginated_array expect(json_response.length).to eq(1) expect(json_response.first['labels']).to eq([label2.title, label.title]) expect(json_response.first['id']).to eq(merge_request.id) @@ -298,8 +276,7 @@ describe API::MergeRequests do it 'returns an array of merge requests with any label when filtering by any label' do get api(endpoint_path, user), params: { labels: ["#{label.title} , #{label2.title}"] } - expect_paginated_array_response - expect(json_response).to be_an Array + expect_successful_response_with_paginated_array expect(json_response.length).to eq(1) expect(json_response.first['labels']).to eq([label2.title, label.title]) expect(json_response.first['id']).to eq(merge_request.id) @@ -308,7 +285,7 @@ describe API::MergeRequests do it 'returns an array of merge requests with any label when filtering by any label' do get api(endpoint_path, user), params: { labels: IssuesFinder::FILTER_ANY } - expect_paginated_array_response + expect_successful_response_with_paginated_array expect(json_response.length).to eq(1) expect(json_response.first['id']).to eq(merge_request.id) end @@ -316,10 +293,9 @@ describe API::MergeRequests do it 'returns an array of merge requests without a label when filtering by no label' do get api(endpoint_path, user), params: { labels: IssuesFinder::FILTER_NONE } - response_ids = json_response.map { |merge_request| merge_request['id'] } - - expect_paginated_array_response - expect(response_ids).to contain_exactly(merge_request_closed.id, merge_request_merged.id, merge_request_locked.id) + expect_paginated_array_response([ + merge_request_merged.id, merge_request_locked.id, merge_request_closed.id + ]) end end @@ -339,10 +315,7 @@ describe API::MergeRequests do get api(path, user) - expect(response).to have_gitlab_http_status(200) - expect(json_response).to be_an Array - expect(json_response.length).to eq(1) - expect(json_response.first['id']).to eq(mr2.id) + expect_paginated_array_response([mr2.id]) end context 'with ordering' do @@ -356,10 +329,10 @@ describe API::MergeRequests do get api(path, user) - expect(response).to have_gitlab_http_status(200) - expect(response).to include_pagination_headers - expect(json_response).to be_an Array - expect(json_response.length).to eq(4) + expect_paginated_array_response([ + merge_request_closed.id, merge_request_locked.id, + merge_request_merged.id, merge_request.id + ]) response_dates = json_response.map { |merge_request| merge_request['created_at'] } expect(response_dates).to eq(response_dates.sort) end @@ -369,10 +342,10 @@ describe API::MergeRequests do get api(path, user) - expect(response).to have_gitlab_http_status(200) - expect(response).to include_pagination_headers - expect(json_response).to be_an Array - expect(json_response.length).to eq(4) + expect_paginated_array_response([ + merge_request.id, merge_request_merged.id, + merge_request_locked.id, merge_request_closed.id + ]) response_dates = json_response.map { |merge_request| merge_request['created_at'] } expect(response_dates).to eq(response_dates.sort.reverse) end @@ -414,10 +387,10 @@ describe API::MergeRequests do get api(path, user) - expect(response).to have_gitlab_http_status(200) - expect(response).to include_pagination_headers - expect(json_response).to be_an Array - expect(json_response.length).to eq(4) + expect_paginated_array_response([ + merge_request.id, merge_request_locked.id, + merge_request_merged.id, merge_request_closed.id + ]) response_dates = json_response.map { |merge_request| merge_request['updated_at'] } expect(response_dates).to eq(response_dates.sort.reverse) end @@ -427,10 +400,10 @@ describe API::MergeRequests do get api(path, user) - expect(response).to have_gitlab_http_status(200) - expect(response).to include_pagination_headers - expect(json_response).to be_an Array - expect(json_response.length).to eq(4) + expect_paginated_array_response([ + merge_request_closed.id, merge_request_locked.id, + merge_request_merged.id, merge_request.id + ]) response_dates = json_response.map { |merge_request| merge_request['created_at'] } expect(response_dates).to eq(response_dates.sort) end @@ -440,7 +413,9 @@ describe API::MergeRequests do it 'returns merge requests with the given source branch' do get api(endpoint_path, user), params: { source_branch: merge_request_closed.source_branch, state: 'all' } - expect_response_contain_exactly(merge_request_closed, merge_request_merged, merge_request_locked) + expect_paginated_array_response([ + merge_request_merged.id, merge_request_locked.id, merge_request_closed.id + ]) end end @@ -448,7 +423,9 @@ describe API::MergeRequests do it 'returns merge requests with the given target branch' do get api(endpoint_path, user), params: { target_branch: merge_request_closed.target_branch, state: 'all' } - expect_response_contain_exactly(merge_request_closed, merge_request_merged, merge_request_locked) + expect_paginated_array_response([ + merge_request_merged.id, merge_request_locked.id, merge_request_closed.id + ]) end end end @@ -471,7 +448,10 @@ describe API::MergeRequests do it 'returns an array of all merge requests' do get api('/merge_requests', user), params: { scope: 'all' } - expect_paginated_array_response + expect_paginated_array_response([ + merge_request_merged.id, merge_request_locked.id, + merge_request_closed.id, merge_request.id + ]) end it "returns authentication error without any scope" do @@ -507,30 +487,23 @@ describe API::MergeRequests do it 'returns an array of all merge requests except unauthorized ones' do get api('/merge_requests', user), params: { scope: :all } - expect(response).to have_gitlab_http_status(200) - expect(response).to include_pagination_headers - expect(json_response).to be_an Array - expect(json_response.map { |mr| mr['id'] }) - .to contain_exactly(merge_request.id, merge_request_closed.id, merge_request_merged.id, merge_request_locked.id, merge_request2.id) + expect_paginated_array_response([ + merge_request_merged.id, merge_request2.id, merge_request_locked.id, merge_request_closed.id, merge_request.id + ]) end it "returns an array of no merge_requests when wip=yes" do get api("/merge_requests", user), params: { wip: 'yes' } - expect(response).to have_gitlab_http_status(200) - expect(response).to include_pagination_headers - expect(json_response).to be_an Array - expect(json_response.length).to eq(0) + expect_empty_array_response end it "returns an array of no merge_requests when wip=no" do get api("/merge_requests", user), params: { wip: 'no' } - expect(response).to have_gitlab_http_status(200) - expect(response).to include_pagination_headers - expect(json_response).to be_an Array - expect(json_response.map { |mr| mr['id'] }) - .to contain_exactly(merge_request.id, merge_request_closed.id, merge_request_merged.id, merge_request_locked.id, merge_request2.id) + expect_paginated_array_response([ + merge_request_merged.id, merge_request2.id, merge_request_locked.id, merge_request_closed.id, merge_request.id + ]) end it 'does not return unauthorized merge requests' do @@ -539,7 +512,9 @@ describe API::MergeRequests do get api('/merge_requests', user), params: { scope: :all } - expect_response_contain_exactly(merge_request2, merge_request_merged, merge_request_closed, merge_request, merge_request_locked) + expect_paginated_array_response([ + merge_request_merged.id, merge_request2.id, merge_request_locked.id, merge_request_closed.id, merge_request.id + ]) expect(json_response.map { |mr| mr['id'] }).not_to include(merge_request3.id) end @@ -548,7 +523,7 @@ describe API::MergeRequests do get api('/merge_requests', user2) - expect_response_ordered_exactly(merge_request3) + expect_paginated_array_response([merge_request3.id]) end it 'returns an array of merge requests authored by the given user' do @@ -556,7 +531,7 @@ describe API::MergeRequests do get api('/merge_requests', user), params: { author_id: user2.id, scope: :all } - expect_response_ordered_exactly(merge_request3) + expect_paginated_array_response([merge_request3.id]) end it 'returns an array of merge requests assigned to the given user' do @@ -564,7 +539,7 @@ describe API::MergeRequests do get api('/merge_requests', user), params: { assignee_id: user2.id, scope: :all } - expect_response_ordered_exactly(merge_request3) + expect_paginated_array_response([merge_request3.id]) end it 'returns an array of merge requests with no assignee' do @@ -572,7 +547,7 @@ describe API::MergeRequests do get api('/merge_requests', user), params: { assignee_id: 'None', scope: :all } - expect_response_ordered_exactly(merge_request3) + expect_paginated_array_response([merge_request3.id]) end it 'returns an array of merge requests with any assignee' do @@ -581,7 +556,10 @@ describe API::MergeRequests do get api('/merge_requests', user), params: { assignee_id: 'Any', scope: :all } - expect_response_contain_exactly(merge_request, merge_request2, merge_request_closed, merge_request_merged, merge_request_locked) + expect_paginated_array_response([ + merge_request_merged.id, merge_request2.id, merge_request_locked.id, + merge_request_closed.id, merge_request.id + ]) end it 'returns an array of merge requests assigned to me' do @@ -589,7 +567,7 @@ describe API::MergeRequests do get api('/merge_requests', user2), params: { scope: 'assigned_to_me' } - expect_response_ordered_exactly(merge_request3) + expect_paginated_array_response([merge_request3.id]) end it 'returns an array of merge requests assigned to me (kebab-case)' do @@ -597,7 +575,7 @@ describe API::MergeRequests do get api('/merge_requests', user2), params: { scope: 'assigned-to-me' } - expect_response_ordered_exactly(merge_request3) + expect_paginated_array_response([merge_request3.id]) end it 'returns an array of merge requests created by me' do @@ -605,7 +583,7 @@ describe API::MergeRequests do get api('/merge_requests', user2), params: { scope: 'created_by_me' } - expect_response_ordered_exactly(merge_request3) + expect_paginated_array_response([merge_request3.id]) end it 'returns an array of merge requests created by me (kebab-case)' do @@ -613,7 +591,7 @@ describe API::MergeRequests do get api('/merge_requests', user2), params: { scope: 'created-by-me' } - expect_response_ordered_exactly(merge_request3) + expect_paginated_array_response([merge_request3.id]) end it 'returns merge requests reacted by the authenticated user by the given emoji' do @@ -622,14 +600,16 @@ describe API::MergeRequests do get api('/merge_requests', user2), params: { my_reaction_emoji: award_emoji.name, scope: 'all' } - expect_response_ordered_exactly(merge_request3) + expect_paginated_array_response([merge_request3.id]) end context 'source_branch param' do it 'returns merge requests with the given source branch' do get api('/merge_requests', user), params: { source_branch: merge_request_closed.source_branch, state: 'all' } - expect_response_contain_exactly(merge_request_closed, merge_request_merged, merge_request_locked) + expect_paginated_array_response([ + merge_request_merged.id, merge_request_locked.id, merge_request_closed.id + ]) end end @@ -637,7 +617,9 @@ describe API::MergeRequests do it 'returns merge requests with the given target branch' do get api('/merge_requests', user), params: { target_branch: merge_request_closed.target_branch, state: 'all' } - expect_response_contain_exactly(merge_request_closed, merge_request_merged, merge_request_locked) + expect_paginated_array_response([ + merge_request_merged.id, merge_request_locked.id, merge_request_closed.id + ]) end end @@ -646,7 +628,7 @@ describe API::MergeRequests do get api('/merge_requests?created_before=2000-01-02T00:00:00.060Z', user) - expect_response_ordered_exactly(merge_request2) + expect_paginated_array_response([merge_request2.id]) end it 'returns merge requests created after a specific date' do @@ -654,7 +636,7 @@ describe API::MergeRequests do get api("/merge_requests?created_after=#{merge_request2.created_at}", user) - expect_response_ordered_exactly(merge_request2) + expect_paginated_array_response([merge_request2.id]) end it 'returns merge requests updated before a specific date' do @@ -662,7 +644,7 @@ describe API::MergeRequests do get api('/merge_requests?updated_before=2000-01-02T00:00:00.060Z', user) - expect_response_ordered_exactly(merge_request2) + expect_paginated_array_response([merge_request2.id]) end it 'returns merge requests updated after a specific date' do @@ -670,7 +652,7 @@ describe API::MergeRequests do get api("/merge_requests?updated_after=#{merge_request2.updated_at}", user) - expect_response_ordered_exactly(merge_request2) + expect_paginated_array_response([merge_request2.id]) end context 'search params' do @@ -681,25 +663,25 @@ describe API::MergeRequests do it 'returns merge requests matching given search string for title' do get api("/merge_requests", user), params: { search: merge_request.title } - expect_response_ordered_exactly(merge_request) + expect_paginated_array_response([merge_request.id]) end it 'returns merge requests matching given search string for title and scoped in title' do get api("/merge_requests", user), params: { search: merge_request.title, in: 'title' } - expect_response_ordered_exactly(merge_request) + expect_paginated_array_response([merge_request.id]) end - it 'returns an empty array if no merge reques matches given search string for description and scoped in title' do + it 'returns an empty array if no merge request matches given search string for description and scoped in title' do get api("/merge_requests", user), params: { search: merge_request.description, in: 'title' } - expect_response_contain_exactly + expect_empty_array_response end it 'returns merge requests for project matching given search string for description' do get api("/merge_requests", user), params: { project_id: project.id, search: merge_request.description } - expect_response_ordered_exactly(merge_request) + expect_paginated_array_response([merge_request.id]) end end @@ -707,7 +689,7 @@ describe API::MergeRequests do it 'returns merge requests with the given state' do get api('/merge_requests', user), params: { state: 'locked' } - expect_response_contain_exactly(merge_request_locked) + expect_paginated_array_response([merge_request_locked.id]) end end end @@ -729,18 +711,13 @@ describe API::MergeRequests do it "returns an array of no merge_requests when wip=yes" do get api("/projects/#{project.id}/merge_requests", user), params: { wip: 'yes' } - expect(response).to have_gitlab_http_status(200) - expect(response).to include_pagination_headers - expect(json_response).to be_an Array - expect(json_response.length).to eq(0) + expect_empty_array_response end it 'returns merge_request by "iids" array' do get api(endpoint_path, user), params: { iids: [merge_request.iid, merge_request_closed.iid] } - expect(response).to have_gitlab_http_status(200) - expect(json_response).to be_an Array - expect(json_response.length).to eq(2) + expect_paginated_array_response([merge_request_closed.id, merge_request.id]) expect(json_response.first['title']).to eq merge_request_closed.title expect(json_response.first['id']).to eq merge_request_closed.id end @@ -815,12 +792,10 @@ describe API::MergeRequests do it 'returns an array excluding merge_requests from archived projects' do get api(endpoint_path, user) - expect_response_contain_exactly( - merge_request_merged, - merge_request_locked, - merge_request_closed, - merge_request - ) + expect_paginated_array_response([ + merge_request_merged.id, merge_request_locked.id, + merge_request_closed.id, merge_request.id + ]) end context 'with non_archived param set as false' do @@ -829,13 +804,10 @@ describe API::MergeRequests do get api(path, user) - expect_response_contain_exactly( - merge_request_merged, - merge_request_locked, - merge_request_closed, - merge_request, - merge_request_archived - ) + expect_paginated_array_response([ + merge_request_merged.id, merge_request_archived.id, merge_request_locked.id, + merge_request_closed.id, merge_request.id + ]) end end end @@ -1079,9 +1051,7 @@ describe API::MergeRequests do get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/commits", user) commit = merge_request.commits.first - expect(response).to have_gitlab_http_status(200) - expect(response).to include_pagination_headers - expect(json_response).to be_an Array + expect_successful_response_with_paginated_array expect(json_response.size).to eq(merge_request.commits.size) expect(json_response.first['id']).to eq(commit.id) expect(json_response.first['title']).to eq(commit.title) @@ -1105,9 +1075,7 @@ describe API::MergeRequests do get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/context_commits", user) - expect(response).to have_gitlab_http_status(200) - expect(response).to include_pagination_headers - expect(json_response).to be_an Array + expect_successful_response_with_paginated_array expect(json_response.size).to eq(merge_request.context_commits.size) expect(json_response.first['id']).to eq(context_commit.id) expect(json_response.first['title']).to eq(context_commit.title) @@ -1147,9 +1115,7 @@ describe API::MergeRequests do it 'returns a paginated array of corresponding pipelines' do get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/pipelines") - expect(response).to have_gitlab_http_status(200) - expect(response).to include_pagination_headers - expect(json_response).to be_an Array + expect_successful_response_with_paginated_array expect(json_response.count).to eq(1) expect(json_response.first['id']).to eq(pipeline.id) end @@ -1395,7 +1361,7 @@ describe API::MergeRequests do expect(json_response['labels']).to eq([]) end - xit 'empty label param as array, does not add any labels' do + it 'empty label param as array, does not add any labels' do params[:labels] = [] post api("/projects/#{project.id}/merge_requests", user), params: params @@ -2232,7 +2198,7 @@ describe API::MergeRequests do expect(json_response['labels']).to eq [] end - xit 'empty label as array, removes labels' do + it 'empty label as array, removes labels' do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), params: { title: 'new issue', @@ -2240,7 +2206,6 @@ describe API::MergeRequests do } expect(response.status).to eq(200) - # fails, as grape ommits for some reason empty array as optional param value, so nothing it passed along expect(json_response['labels']).to eq [] end @@ -2306,9 +2271,7 @@ describe API::MergeRequests do get api("/projects/#{project.id}/merge_requests/#{mr.iid}/closes_issues", user) - expect(response).to have_gitlab_http_status(200) - expect(response).to include_pagination_headers - expect(json_response).to be_an Array + expect_successful_response_with_paginated_array expect(json_response.length).to eq(1) expect(json_response.first['id']).to eq(issue.id) end @@ -2316,10 +2279,7 @@ describe API::MergeRequests do it 'returns an empty array when there are no issues to be closed' do get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/closes_issues", user) - expect(response).to have_gitlab_http_status(200) - expect(response).to include_pagination_headers - expect(json_response).to be_an Array - expect(json_response.length).to eq(0) + expect_empty_array_response end it 'handles external issues' do @@ -2332,9 +2292,7 @@ describe API::MergeRequests do get api("/projects/#{jira_project.id}/merge_requests/#{merge_request.iid}/closes_issues", user) - expect(response).to have_gitlab_http_status(200) - expect(response).to include_pagination_headers - expect(json_response).to be_an Array + expect_successful_response_with_paginated_array expect(json_response.length).to eq(2) expect(json_response.second['title']).to eq(ext_issue.title) expect(json_response.second['id']).to eq(ext_issue.id) @@ -2546,22 +2504,4 @@ describe API::MergeRequests do merge_request_closed.save merge_request_closed end - - def expect_response_contain_exactly(*items) - expect_paginated_array_response - expect(json_response.length).to eq(items.size) - expect(json_response.map { |element| element['id'] }).to contain_exactly(*items.map(&:id)) - end - - def expect_response_ordered_exactly(*items) - expect_paginated_array_response - expect(json_response.length).to eq(items.size) - expect(json_response.map { |element| element['id'] }).to eq(items.map(&:id)) - end - - def expect_paginated_array_response - expect(response).to have_gitlab_http_status(200) - expect(response).to include_pagination_headers - expect(json_response).to be_an Array - end end diff --git a/spec/services/users/refresh_authorized_projects_service_spec.rb b/spec/services/users/refresh_authorized_projects_service_spec.rb index f5a914bb482..d7ba7f5f69e 100644 --- a/spec/services/users/refresh_authorized_projects_service_spec.rb +++ b/spec/services/users/refresh_authorized_projects_service_spec.rb @@ -22,6 +22,42 @@ describe Users::RefreshAuthorizedProjectsService do service.execute end + + context 'callbacks' do + let(:callback) { double('callback') } + + context 'incorrect_auth_found_callback callback' do + let(:user) { create(:user) } + let(:service) do + described_class.new(user, + incorrect_auth_found_callback: callback) + end + + it 'is called' do + access_level = Gitlab::Access::DEVELOPER + create(:project_authorization, user: user, project: project, access_level: access_level) + + expect(callback).to receive(:call).with(project.id, access_level).once + + service.execute + end + end + + context 'missing_auth_found_callback callback' do + let(:service) do + described_class.new(user, + missing_auth_found_callback: callback) + end + + it 'is called' do + ProjectAuthorization.delete_all + + expect(callback).to receive(:call).with(project.id, Gitlab::Access::MAINTAINER).once + + service.execute + end + end + end end describe '#execute_without_lease' do diff --git a/spec/support/helpers/api_helpers.rb b/spec/support/helpers/api_helpers.rb index 4bf6a17c03e..44c38df71b0 100644 --- a/spec/support/helpers/api_helpers.rb +++ b/spec/support/helpers/api_helpers.rb @@ -40,6 +40,17 @@ module ApiHelpers end end + def expect_empty_array_response + expect_successful_response_with_paginated_array + expect(json_response.length).to eq(0) + end + + def expect_successful_response_with_paginated_array + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + end + def expect_paginated_array_response(items) expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers |