diff options
Diffstat (limited to 'spec')
47 files changed, 1525 insertions, 422 deletions
diff --git a/spec/features/admin/admin_sees_project_statistics_spec.rb b/spec/features/admin/admin_sees_project_statistics_spec.rb index b5323a1c76d..ecd0aab925b 100644 --- a/spec/features/admin/admin_sees_project_statistics_spec.rb +++ b/spec/features/admin/admin_sees_project_statistics_spec.rb @@ -15,7 +15,7 @@ describe "Admin > Admin sees project statistics" do let(:project) { create(:project, :repository) } it "shows project statistics" do - expect(page).to have_content("Storage: 0 Bytes (0 Bytes repositories, 0 Bytes wikis, 0 Bytes build artifacts, 0 Bytes LFS)") + expect(page).to have_content("Storage: 0 Bytes (Repository: 0 Bytes / Wikis: 0 Bytes / Build Artifacts: 0 Bytes / LFS: 0 Bytes)") end end diff --git a/spec/features/search/user_uses_header_search_field_spec.rb b/spec/features/search/user_uses_header_search_field_spec.rb index 1cc47cd6bd1..7ddd5c12cdf 100644 --- a/spec/features/search/user_uses_header_search_field_spec.rb +++ b/spec/features/search/user_uses_header_search_field_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe 'User uses header search field' do +describe 'User uses header search field', :js do include FilteredSearchHelpers let(:project) { create(:project) } @@ -11,57 +11,12 @@ describe 'User uses header search field' do sign_in(user) end - context 'when user is in a global scope', :js do + shared_examples 'search field examples' do before do - visit(root_path) - page.find('#search').click + visit(url) end - context 'when clicking issues' do - it 'shows assigned issues' do - find('.search-input-container .dropdown-menu').click_link('Issues assigned to me') - - expect(page).to have_selector('.filtered-search') - expect_tokens([assignee_token(user.name)]) - expect_filtered_search_input_empty - end - - it 'shows created issues' do - find('.search-input-container .dropdown-menu').click_link("Issues I've created") - - expect(page).to have_selector('.filtered-search') - expect_tokens([author_token(user.name)]) - expect_filtered_search_input_empty - end - end - - context 'when clicking merge requests' do - let!(:merge_request) { create(:merge_request, source_project: project, author: user, assignees: [user]) } - - it 'shows assigned merge requests' do - find('.search-input-container .dropdown-menu').click_link('Merge requests assigned to me') - - expect(page).to have_selector('.filtered-search') - expect_tokens([assignee_token(user.name)]) - expect_filtered_search_input_empty - end - - it 'shows created merge requests' do - find('.search-input-container .dropdown-menu').click_link("Merge requests I've created") - - expect(page).to have_selector('.filtered-search') - expect_tokens([author_token(user.name)]) - expect_filtered_search_input_empty - end - end - end - - context 'when user is in a project scope' do - before do - visit(project_path(project)) - end - - it 'starts searching by pressing the enter key', :js do + it 'starts searching by pressing the enter key' do fill_in('search', with: 'gitlab') find('#search').native.send_keys(:enter) @@ -70,30 +25,31 @@ describe 'User uses header search field' do end end - context 'when clicking the search field', :js do + context 'when clicking the search field' do before do page.find('#search').click + wait_for_all_requests end it 'shows category search dropdown' do - expect(page).to have_selector('.dropdown-header', text: /#{project.name}/i) + expect(page).to have_selector('.dropdown-header', text: /#{scope_name}/i) end context 'when clicking issues' do let!(:issue) { create(:issue, project: project, author: user, assignees: [user]) } it 'shows assigned issues' do - find('.dropdown-menu').click_link('Issues assigned to me') + find('.search-input-container .dropdown-menu').click_link('Issues assigned to me') - expect(page).to have_selector('.filtered-search') + expect(page).to have_selector('.issues-list .issue') expect_tokens([assignee_token(user.name)]) expect_filtered_search_input_empty end it 'shows created issues' do - find('.dropdown-menu').click_link("Issues I've created") + find('.search-input-container .dropdown-menu').click_link("Issues I've created") - expect(page).to have_selector('.filtered-search') + expect(page).to have_selector('.issues-list .issue') expect_tokens([author_token(user.name)]) expect_filtered_search_input_empty end @@ -103,33 +59,77 @@ describe 'User uses header search field' do let!(:merge_request) { create(:merge_request, source_project: project, author: user, assignees: [user]) } it 'shows assigned merge requests' do - find('.dropdown-menu').click_link('Merge requests assigned to me') + find('.search-input-container .dropdown-menu').click_link('Merge requests assigned to me') - expect(page).to have_selector('.merge-requests-holder') + expect(page).to have_selector('.mr-list .merge-request') expect_tokens([assignee_token(user.name)]) expect_filtered_search_input_empty end it 'shows created merge requests' do - find('.dropdown-menu').click_link("Merge requests I've created") + find('.search-input-container .dropdown-menu').click_link("Merge requests I've created") - expect(page).to have_selector('.merge-requests-holder') + expect(page).to have_selector('.mr-list .merge-request') expect_tokens([author_token(user.name)]) expect_filtered_search_input_empty end end end - context 'when entering text into the search field', :js do + context 'when entering text into the search field' do before do page.within('.search-input-wrap') do - fill_in('search', with: project.name[0..3]) + fill_in('search', with: scope_name.first(4)) end end it 'does not display the category search dropdown' do - expect(page).not_to have_selector('.dropdown-header', text: /#{project.name}/i) + expect(page).not_to have_selector('.dropdown-header', text: /#{scope_name}/i) end end end + + context 'when user is in a global scope' do + include_examples 'search field examples' do + let(:url) { root_path } + let(:scope_name) { 'All GitLab' } + end + end + + context 'when user is in a project scope' do + include_examples 'search field examples' do + let(:url) { project_path(project) } + let(:scope_name) { project.name } + end + end + + context 'when user is in a group scope' do + let(:group) { create(:group) } + let(:project) { create(:project, namespace: group) } + + before do + group.add_maintainer(user) + end + + include_examples 'search field examples' do + let(:url) { group_path(group) } + let(:scope_name) { group.name } + end + end + + context 'when user is in a subgroup scope' do + let(:group) { create(:group) } + let(:subgroup) { create(:group, :public, parent: group) } + let(:project) { create(:project, namespace: subgroup) } + + before do + group.add_owner(user) + subgroup.add_owner(user) + end + + include_examples 'search field examples' do + let(:url) { group_path(subgroup) } + let(:scope_name) { subgroup.name } + end + end end diff --git a/spec/frontend/api_spec.js b/spec/frontend/api_spec.js index 0188d12a57d..7004373be0e 100644 --- a/spec/frontend/api_spec.js +++ b/spec/frontend/api_spec.js @@ -412,6 +412,22 @@ describe('Api', () => { }); }); + describe('user counts', () => { + it('fetches single user counts', done => { + const expectedUrl = `${dummyUrlRoot}/api/${dummyApiVersion}/user_counts`; + mock.onGet(expectedUrl).reply(200, { + merge_requests: 4, + }); + + Api.userCounts() + .then(({ data }) => { + expect(data.merge_requests).toBe(4); + }) + .then(done) + .catch(done.fail); + }); + }); + describe('user status', () => { it('fetches single user status', done => { const userId = '123456'; diff --git a/spec/frontend/commons/nav/user_merge_requests_spec.js b/spec/frontend/commons/nav/user_merge_requests_spec.js new file mode 100644 index 00000000000..4da6d53557a --- /dev/null +++ b/spec/frontend/commons/nav/user_merge_requests_spec.js @@ -0,0 +1,113 @@ +import { + openUserCountsBroadcast, + closeUserCountsBroadcast, + refreshUserMergeRequestCounts, +} from '~/commons/nav/user_merge_requests'; +import Api from '~/api'; + +jest.mock('~/api'); + +const TEST_COUNT = 1000; +const MR_COUNT_CLASS = 'merge-requests-count'; + +describe('User Merge Requests', () => { + let channelMock; + let newBroadcastChannelMock; + + beforeEach(() => { + global.gon.current_user_id = 123; + + channelMock = { + postMessage: jest.fn(), + close: jest.fn(), + }; + newBroadcastChannelMock = jest.fn().mockImplementation(() => channelMock); + + global.BroadcastChannel = newBroadcastChannelMock; + setFixtures(`<div class="${MR_COUNT_CLASS}">0</div>`); + }); + + const findMRCountText = () => document.body.querySelector(`.${MR_COUNT_CLASS}`).textContent; + + describe('refreshUserMergeRequestCounts', () => { + beforeEach(() => { + Api.userCounts.mockReturnValue( + Promise.resolve({ + data: { merge_requests: TEST_COUNT }, + }), + ); + }); + + describe('with open broadcast channel', () => { + beforeEach(() => { + openUserCountsBroadcast(); + + return refreshUserMergeRequestCounts(); + }); + + it('updates the top count of merge requests', () => { + expect(findMRCountText()).toEqual(TEST_COUNT.toLocaleString()); + }); + + it('calls the API', () => { + expect(Api.userCounts).toHaveBeenCalled(); + }); + + it('posts count to BroadcastChannel', () => { + expect(channelMock.postMessage).toHaveBeenCalledWith(TEST_COUNT); + }); + }); + + describe('without open broadcast channel', () => { + beforeEach(() => refreshUserMergeRequestCounts()); + + it('does not post anything', () => { + expect(channelMock.postMessage).not.toHaveBeenCalled(); + }); + }); + }); + + describe('openUserCountsBroadcast', () => { + beforeEach(() => { + openUserCountsBroadcast(); + }); + + it('creates BroadcastChannel that updates DOM on message received', () => { + expect(findMRCountText()).toEqual('0'); + + channelMock.onmessage({ data: TEST_COUNT }); + + expect(findMRCountText()).toEqual(TEST_COUNT.toLocaleString()); + }); + + it('closes if called while already open', () => { + expect(channelMock.close).not.toHaveBeenCalled(); + + openUserCountsBroadcast(); + + expect(channelMock.close).toHaveBeenCalled(); + }); + }); + + describe('closeUserCountsBroadcast', () => { + describe('when not opened', () => { + it('does nothing', () => { + expect(channelMock.close).not.toHaveBeenCalled(); + }); + }); + + describe('when opened', () => { + beforeEach(() => { + openUserCountsBroadcast(); + }); + + it('closes', () => { + expect(channelMock.close).not.toHaveBeenCalled(); + + closeUserCountsBroadcast(); + + expect(channelMock.close).toHaveBeenCalled(); + }); + }); + }); +}); diff --git a/spec/frontend/confidential_merge_request/components/__snapshots__/project_form_group_spec.js.snap b/spec/frontend/confidential_merge_request/components/__snapshots__/project_form_group_spec.js.snap index a241c764df7..fd307ce5ab3 100644 --- a/spec/frontend/confidential_merge_request/components/__snapshots__/project_form_group_spec.js.snap +++ b/spec/frontend/confidential_merge_request/components/__snapshots__/project_form_group_spec.js.snap @@ -19,7 +19,7 @@ exports[`Confidential merge request project form group component renders empty s <br /> <span> - To protect this issues confidentiality, + To protect this issue's confidentiality, <a class="help-link" href="https://test.com" @@ -28,23 +28,6 @@ exports[`Confidential merge request project form group component renders empty s </a> and set the forks visiblity to private. </span> - - <gllink-stub - class="help-link" - href="/help" - target="_blank" - > - <span - class="sr-only" - > - Read more - </span> - - <i - aria-hidden="true" - class="fa fa-question-circle" - /> - </gllink-stub> </p> </div> </div> @@ -69,7 +52,7 @@ exports[`Confidential merge request project form group component renders fork dr <br /> <span> - To protect this issues confidentiality, + To protect this issue's confidentiality, <a class="help-link" href="https://test.com" @@ -78,23 +61,6 @@ exports[`Confidential merge request project form group component renders fork dr </a> and set the forks visiblity to private. </span> - - <gllink-stub - class="help-link" - href="/help" - target="_blank" - > - <span - class="sr-only" - > - Read more - </span> - - <i - aria-hidden="true" - class="fa fa-question-circle" - /> - </gllink-stub> </p> </div> </div> diff --git a/spec/frontend/notes/components/discussion_notes_replies_wrapper_spec.js b/spec/frontend/notes/components/discussion_notes_replies_wrapper_spec.js new file mode 100644 index 00000000000..279ca017b44 --- /dev/null +++ b/spec/frontend/notes/components/discussion_notes_replies_wrapper_spec.js @@ -0,0 +1,51 @@ +import { mount } from '@vue/test-utils'; +import DiscussionNotesRepliesWrapper from '~/notes/components/discussion_notes_replies_wrapper.vue'; + +const TEST_CHILDREN = '<li>Hello!</li><li>World!</li>'; + +// We have to wrap our SUT with a TestComponent because multiple roots are possible +// because it's a functional component. +const TestComponent = { + components: { DiscussionNotesRepliesWrapper }, + template: `<ul><discussion-notes-replies-wrapper v-bind="$attrs">${TEST_CHILDREN}</discussion-notes-replies-wrapper></ul>`, +}; + +describe('DiscussionNotesRepliesWrapper', () => { + let wrapper; + + const createComponent = (props = {}) => { + wrapper = mount(TestComponent, { + propsData: props, + sync: false, + }); + }; + + afterEach(() => { + wrapper.destroy(); + }); + + describe('when normal discussion', () => { + beforeEach(() => { + createComponent(); + }); + + it('renders children directly', () => { + expect(wrapper.html()).toEqual(`<ul>${TEST_CHILDREN}</ul>`); + }); + }); + + describe('when diff discussion', () => { + beforeEach(() => { + createComponent({ + isDiffDiscussion: true, + }); + }); + + it('wraps children with notes', () => { + const notes = wrapper.find('li.discussion-collapsible ul.notes'); + + expect(notes.exists()).toBe(true); + expect(notes.html()).toEqual(`<ul class="notes">${TEST_CHILDREN}</ul>`); + }); + }); +}); diff --git a/spec/graphql/gitlab_schema_spec.rb b/spec/graphql/gitlab_schema_spec.rb index 93b86b9b812..dec6b23d72a 100644 --- a/spec/graphql/gitlab_schema_spec.rb +++ b/spec/graphql/gitlab_schema_spec.rb @@ -143,6 +143,26 @@ describe GitlabSchema do end end + context 'for classes that are not ActiveRecord subclasses and have implemented .lazy_find' do + it 'returns the correct record' do + note = create(:discussion_note_on_merge_request) + + result = described_class.object_from_id(note.to_global_id) + + expect(result.__sync).to eq(note) + end + + it 'batchloads the queries' do + note1 = create(:discussion_note_on_merge_request) + note2 = create(:discussion_note_on_merge_request) + + expect do + [described_class.object_from_id(note1.to_global_id), + described_class.object_from_id(note2.to_global_id)].map(&:__sync) + end.not_to exceed_query_limit(1) + end + end + context 'for other classes' do # We cannot use an anonymous class here as `GlobalID` expects `.name` not # to return `nil` diff --git a/spec/graphql/types/diff_refs_type_spec.rb b/spec/graphql/types/diff_refs_type_spec.rb new file mode 100644 index 00000000000..91017c827ad --- /dev/null +++ b/spec/graphql/types/diff_refs_type_spec.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe GitlabSchema.types['DiffRefs'] do + it { expect(described_class.graphql_name).to eq('DiffRefs') } + + it { expect(described_class).to have_graphql_fields(:base_sha, :head_sha, :start_sha) } +end diff --git a/spec/graphql/types/merge_request_type_spec.rb b/spec/graphql/types/merge_request_type_spec.rb index f73bd062369..59bd0123d88 100644 --- a/spec/graphql/types/merge_request_type_spec.rb +++ b/spec/graphql/types/merge_request_type_spec.rb @@ -13,7 +13,7 @@ describe GitlabSchema.types['MergeRequest'] do description_html state created_at updated_at source_project target_project project project_id source_project_id target_project_id source_branch target_branch work_in_progress merge_when_pipeline_succeeds diff_head_sha - merge_commit_sha user_notes_count should_remove_source_branch + merge_commit_sha user_notes_count should_remove_source_branch diff_refs force_remove_source_branch merge_status in_progress_merge_commit_sha merge_error allow_collaboration should_be_rebased rebase_commit_sha rebase_in_progress merge_commit_message default_merge_commit_message diff --git a/spec/graphql/types/notes/diff_position_type_spec.rb b/spec/graphql/types/notes/diff_position_type_spec.rb index 2f8724d7f0d..345bca8f702 100644 --- a/spec/graphql/types/notes/diff_position_type_spec.rb +++ b/spec/graphql/types/notes/diff_position_type_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe GitlabSchema.types['DiffPosition'] do it 'exposes the expected fields' do - expected_fields = [:head_sha, :base_sha, :start_sha, :file_path, :old_path, + expected_fields = [:diff_refs, :file_path, :old_path, :new_path, :position_type, :old_line, :new_line, :x, :y, :width, :height] diff --git a/spec/graphql/types/notes/discussion_type_spec.rb b/spec/graphql/types/notes/discussion_type_spec.rb index 2a1eb0efd35..ba7fc961212 100644 --- a/spec/graphql/types/notes/discussion_type_spec.rb +++ b/spec/graphql/types/notes/discussion_type_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe GitlabSchema.types['Discussion'] do - it { is_expected.to have_graphql_fields(:id, :created_at, :notes) } + it { is_expected.to have_graphql_fields(:id, :created_at, :notes, :reply_id) } it { is_expected.to require_graphql_authorizations(:read_note) } end diff --git a/spec/helpers/storage_helper_spec.rb b/spec/helpers/storage_helper_spec.rb index 62c00964524..4bd0fbb76ca 100644 --- a/spec/helpers/storage_helper_spec.rb +++ b/spec/helpers/storage_helper_spec.rb @@ -31,7 +31,7 @@ describe StorageHelper do build_artifacts_size: 30.megabytes)) end - let(:message) { '10 KB repositories, 10 Bytes wikis, 30 MB build artifacts, 20 GB LFS' } + let(:message) { 'Repository: 10 KB / Wikis: 10 Bytes / Build Artifacts: 30 MB / LFS: 20 GB' } it 'works on ProjectStatistics' do expect(helper.storage_counters_details(project.statistics)).to eq(message) diff --git a/spec/javascripts/diffs/components/inline_diff_view_spec.js b/spec/javascripts/diffs/components/inline_diff_view_spec.js index 0b3890b68d6..9b61dbe7975 100644 --- a/spec/javascripts/diffs/components/inline_diff_view_spec.js +++ b/spec/javascripts/diffs/components/inline_diff_view_spec.js @@ -10,6 +10,7 @@ describe('InlineDiffView', () => { let component; const getDiffFileMock = () => Object.assign({}, diffFileMockData); const getDiscussionsMockData = () => [Object.assign({}, discussionsMockData)]; + const notesLength = getDiscussionsMockData()[0].notes.length; beforeEach(done => { const diffFile = getDiffFileMock(); @@ -40,7 +41,7 @@ describe('InlineDiffView', () => { Vue.nextTick(() => { expect(el.querySelectorAll('.notes_holder').length).toEqual(1); - expect(el.querySelectorAll('.notes_holder .note-discussion li').length).toEqual(6); + expect(el.querySelectorAll('.notes_holder .note').length).toEqual(notesLength + 1); expect(el.innerText.indexOf('comment 5')).toBeGreaterThan(-1); component.$store.dispatch('setInitialNotes', []); diff --git a/spec/javascripts/issuable_spec.js b/spec/javascripts/issuable_spec.js index 25543053eba..4d57bfb1b33 100644 --- a/spec/javascripts/issuable_spec.js +++ b/spec/javascripts/issuable_spec.js @@ -2,14 +2,14 @@ import $ from 'jquery'; import MockAdaptor from 'axios-mock-adapter'; import axios from '~/lib/utils/axios_utils'; import IssuableIndex from '~/issuable_index'; +import issuableInitBulkUpdateSidebar from '~/issuable_init_bulk_update_sidebar'; describe('Issuable', () => { - let Issuable; describe('initBulkUpdate', () => { it('should not set bulkUpdateSidebar', () => { - Issuable = new IssuableIndex('issue_'); + new IssuableIndex('issue_'); // eslint-disable-line no-new - expect(Issuable.bulkUpdateSidebar).not.toBeDefined(); + expect(issuableInitBulkUpdateSidebar.bulkUpdateSidebar).toBeNull(); }); it('should set bulkUpdateSidebar', () => { @@ -17,9 +17,9 @@ describe('Issuable', () => { element.classList.add('issues-bulk-update'); document.body.appendChild(element); - Issuable = new IssuableIndex('issue_'); + new IssuableIndex('issue_'); // eslint-disable-line no-new - expect(Issuable.bulkUpdateSidebar).toBeDefined(); + expect(issuableInitBulkUpdateSidebar.bulkUpdateSidebar).toBeDefined(); }); }); @@ -36,7 +36,7 @@ describe('Issuable', () => { input.setAttribute('id', 'issuable_email'); document.body.appendChild(input); - Issuable = new IssuableIndex('issue_'); + new IssuableIndex('issue_'); // eslint-disable-line no-new mock = new MockAdaptor(axios); diff --git a/spec/javascripts/notes/components/comment_form_spec.js b/spec/javascripts/notes/components/comment_form_spec.js index 362963ddaf4..88c86746992 100644 --- a/spec/javascripts/notes/components/comment_form_spec.js +++ b/spec/javascripts/notes/components/comment_form_spec.js @@ -251,6 +251,21 @@ describe('issue_comment_form component', () => { }); }); }); + + describe('when toggling state', () => { + it('should update MR count', done => { + spyOn(vm, 'closeIssue').and.returnValue(Promise.resolve()); + + const updateMrCountSpy = spyOnDependency(CommentForm, 'refreshUserMergeRequestCounts'); + vm.toggleIssueState(); + + Vue.nextTick(() => { + expect(updateMrCountSpy).toHaveBeenCalled(); + + done(); + }); + }); + }); }); describe('issue is confidential', () => { diff --git a/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js b/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js index bb76616be56..ba3ba01944d 100644 --- a/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js +++ b/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js @@ -58,9 +58,11 @@ const createComponent = (customConfig = {}) => { describe('ReadyToMerge', () => { let vm; + let updateMrCountSpy; beforeEach(() => { vm = createComponent(); + updateMrCountSpy = spyOnDependency(ReadyToMerge, 'refreshUserMergeRequestCounts'); }); afterEach(() => { @@ -461,6 +463,7 @@ describe('ReadyToMerge', () => { expect(eventHub.$emit).toHaveBeenCalledWith('MRWidgetUpdateRequested'); expect(eventHub.$emit).toHaveBeenCalledWith('FetchActionsContent'); expect(vm.initiateRemoveSourceBranchPolling).toHaveBeenCalled(); + expect(updateMrCountSpy).toHaveBeenCalled(); expect(cpc).toBeFalsy(); expect(spc).toBeTruthy(); diff --git a/spec/lib/gitlab/cache/ci/project_pipeline_status_spec.rb b/spec/lib/gitlab/cache/ci/project_pipeline_status_spec.rb index 483c5ea9cff..972dd7e0d2b 100644 --- a/spec/lib/gitlab/cache/ci/project_pipeline_status_spec.rb +++ b/spec/lib/gitlab/cache/ci/project_pipeline_status_spec.rb @@ -26,6 +26,7 @@ describe Gitlab::Cache::Ci::ProjectPipelineStatus, :clean_gitlab_redis_cache do end it 'loads 10 projects without hitting Gitaly call limit', :request_store do + allow(Gitlab::GitalyClient).to receive(:can_access_disk?).and_return(false) projects = Gitlab::GitalyClient.allow_n_plus_1_calls do (1..10).map { create(:project, :repository) } end diff --git a/spec/lib/gitlab/cycle_analytics/test_stage_spec.rb b/spec/lib/gitlab/cycle_analytics/test_stage_spec.rb index eacde22cd56..8633a63849f 100644 --- a/spec/lib/gitlab/cycle_analytics/test_stage_spec.rb +++ b/spec/lib/gitlab/cycle_analytics/test_stage_spec.rb @@ -3,6 +3,40 @@ require 'lib/gitlab/cycle_analytics/shared_stage_spec' describe Gitlab::CycleAnalytics::TestStage do let(:stage_name) { :test } + let(:project) { create(:project) } + let(:stage) { described_class.new(project: project, options: { from: 2.days.ago, current_user: project.creator }) } it_behaves_like 'base stage' + + describe '#median' do + before do + issue_1 = create(:issue, project: project, created_at: 90.minutes.ago) + issue_2 = create(:issue, project: project, created_at: 60.minutes.ago) + issue_3 = create(:issue, project: project, created_at: 60.minutes.ago) + mr_1 = create(:merge_request, :closed, source_project: project, created_at: 60.minutes.ago) + mr_2 = create(:merge_request, :closed, source_project: project, created_at: 40.minutes.ago, source_branch: 'A') + mr_3 = create(:merge_request, source_project: project, created_at: 10.minutes.ago, source_branch: 'B') + mr_4 = create(:merge_request, source_project: project, created_at: 10.minutes.ago, source_branch: 'C') + mr_5 = create(:merge_request, source_project: project, created_at: 10.minutes.ago, source_branch: 'D') + mr_1.metrics.update!(latest_build_started_at: 32.minutes.ago, latest_build_finished_at: 2.minutes.ago) + mr_2.metrics.update!(latest_build_started_at: 62.minutes.ago, latest_build_finished_at: 32.minutes.ago) + mr_3.metrics.update!(latest_build_started_at: nil, latest_build_finished_at: nil) + mr_4.metrics.update!(latest_build_started_at: nil, latest_build_finished_at: nil) + mr_5.metrics.update!(latest_build_started_at: nil, latest_build_finished_at: nil) + + create(:merge_requests_closing_issues, merge_request: mr_1, issue: issue_1) + create(:merge_requests_closing_issues, merge_request: mr_2, issue: issue_2) + create(:merge_requests_closing_issues, merge_request: mr_3, issue: issue_3) + create(:merge_requests_closing_issues, merge_request: mr_4, issue: issue_3) + create(:merge_requests_closing_issues, merge_request: mr_5, issue: issue_3) + end + + around do |example| + Timecop.freeze { example.run } + end + + it 'counts median from issues with metrics' do + expect(stage.median).to eq(ISSUES_MEDIAN) + end + end end diff --git a/spec/lib/gitlab/git/commit_spec.rb b/spec/lib/gitlab/git/commit_spec.rb index 25052a79916..3f0e6b34291 100644 --- a/spec/lib/gitlab/git/commit_spec.rb +++ b/spec/lib/gitlab/git/commit_spec.rb @@ -408,7 +408,7 @@ describe Gitlab::Git::Commit, :seed_helper do context 'when oids is empty' do it 'makes no Gitaly request' do - expect(Gitlab::GitalyClient).not_to receive(:call) + expect(Gitlab::GitalyClient).not_to receive(:call).with(repository.storage, :commit_service, :list_commits_by_oid) described_class.batch_by_oid(repository, []) end diff --git a/spec/lib/gitlab/git/rugged_impl/use_rugged_spec.rb b/spec/lib/gitlab/git/rugged_impl/use_rugged_spec.rb new file mode 100644 index 00000000000..e7ef9d08f80 --- /dev/null +++ b/spec/lib/gitlab/git/rugged_impl/use_rugged_spec.rb @@ -0,0 +1,99 @@ +# frozen_string_literal: true + +require 'spec_helper' +require 'json' +require 'tempfile' + +describe Gitlab::Git::RuggedImpl::UseRugged, :seed_helper do + let(:project) { create(:project, :repository) } + let(:repository) { project.repository } + let(:feature_flag_name) { 'feature-flag-name' } + let(:feature_flag) { Feature.get(feature_flag_name) } + let(:temp_gitaly_metadata_file) { create_temporary_gitaly_metadata_file } + + before(:all) do + create_gitaly_metadata_file + end + + subject(:wrapper) do + klazz = Class.new { include Gitlab::Git::RuggedImpl::UseRugged } + klazz.new + end + + before do + Gitlab::GitalyClient.instance_variable_set(:@can_use_disk, {}) + end + + context 'when feature flag is not persisted' do + before do + allow(Feature).to receive(:persisted?).with(feature_flag).and_return(false) + end + + it 'returns true when gitaly matches disk' do + pending('temporary disabled because of https://gitlab.com/gitlab-org/gitlab-ce/issues/64338') + expect(subject.use_rugged?(repository, feature_flag_name)).to be true + end + + it 'returns false when disk access fails' do + allow(Gitlab::GitalyClient).to receive(:storage_metadata_file_path).and_return("/fake/path/doesnt/exist") + + expect(subject.use_rugged?(repository, feature_flag_name)).to be false + end + + it "returns false when gitaly doesn't match disk" do + allow(Gitlab::GitalyClient).to receive(:storage_metadata_file_path).and_return(temp_gitaly_metadata_file) + + expect(subject.use_rugged?(repository, feature_flag_name)).to be_falsey + + File.delete(temp_gitaly_metadata_file) + end + + it "doesn't lead to a second rpc call because gitaly client should use the cached value" do + pending('temporary disabled because of https://gitlab.com/gitlab-org/gitlab-ce/issues/64338') + expect(subject.use_rugged?(repository, feature_flag_name)).to be true + + expect(Gitlab::GitalyClient).not_to receive(:filesystem_id) + + subject.use_rugged?(repository, feature_flag_name) + end + end + + context 'when feature flag is persisted' do + before do + allow(Feature).to receive(:persisted?).with(feature_flag).and_return(true) + end + + it 'returns false when the feature flag is off' do + allow(feature_flag).to receive(:enabled?).and_return(false) + + expect(subject.use_rugged?(repository, feature_flag_name)).to be_falsey + end + + it "returns true when feature flag is on" do + allow(feature_flag).to receive(:enabled?).and_return(true) + allow(Gitlab::GitalyClient).to receive(:can_use_disk?).and_return(false) + + expect(subject.use_rugged?(repository, feature_flag_name)).to be true + end + end + + def create_temporary_gitaly_metadata_file + tmp = Tempfile.new('.gitaly-metadata') + gitaly_metadata = { + "gitaly_filesystem_id" => "some-value" + } + tmp.write(gitaly_metadata.to_json) + tmp.flush + tmp.close + tmp.path + end + + def create_gitaly_metadata_file + File.open(File.join(SEED_STORAGE_PATH, '.gitaly-metadata'), 'w+') do |f| + gitaly_metadata = { + "gitaly_filesystem_id" => SecureRandom.uuid + } + f.write(gitaly_metadata.to_json) + end + end +end diff --git a/spec/lib/gitlab/global_id_spec.rb b/spec/lib/gitlab/global_id_spec.rb new file mode 100644 index 00000000000..d35b5da0b75 --- /dev/null +++ b/spec/lib/gitlab/global_id_spec.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::GlobalId do + describe '.build' do + set(:object) { create(:issue) } + + it 'returns a standard GlobalId if only object is passed' do + expect(described_class.build(object).to_s).to eq(object.to_global_id.to_s) + end + + it 'returns a GlobalId from params' do + expect(described_class.build(model_name: 'MyModel', id: 'myid').to_s).to eq( + 'gid://gitlab/MyModel/myid' + ) + end + + it 'returns a GlobalId from object and `id` param' do + expect(described_class.build(object, id: 'myid').to_s).to eq( + 'gid://gitlab/Issue/myid' + ) + end + + it 'returns a GlobalId from object and `model_name` param' do + expect(described_class.build(object, model_name: 'MyModel').to_s).to eq( + "gid://gitlab/MyModel/#{object.id}" + ) + end + + it 'returns an error if model_name and id are not able to be determined' do + expect { described_class.build(id: 'myid') }.to raise_error(URI::InvalidComponentError) + expect { described_class.build(model_name: 'MyModel') }.to raise_error(URI::InvalidComponentError) + expect { described_class.build }.to raise_error(URI::InvalidComponentError) + end + end +end diff --git a/spec/lib/gitlab/metrics/samplers/unicorn_sampler_spec.rb b/spec/lib/gitlab/metrics/samplers/unicorn_sampler_spec.rb index 090e456644f..4b697b2ba0f 100644 --- a/spec/lib/gitlab/metrics/samplers/unicorn_sampler_spec.rb +++ b/spec/lib/gitlab/metrics/samplers/unicorn_sampler_spec.rb @@ -4,7 +4,7 @@ describe Gitlab::Metrics::Samplers::UnicornSampler do subject { described_class.new(1.second) } describe '#sample' do - let(:unicorn) { double('unicorn') } + let(:unicorn) { Module.new } let(:raindrops) { double('raindrops') } let(:stats) { double('stats') } @@ -78,19 +78,32 @@ describe Gitlab::Metrics::Samplers::UnicornSampler do end end - context 'additional metrics' do - let(:unicorn_workers) { 2 } - + context 'unicorn workers' do before do - allow(unicorn).to receive(:listener_names).and_return([""]) - allow(::Gitlab::Metrics::System).to receive(:cpu_time).and_return(3.14) - allow(subject).to receive(:unicorn_workers_count).and_return(unicorn_workers) + allow(unicorn).to receive(:listener_names).and_return([]) end - it "sets additional metrics" do - expect(subject.metrics[:unicorn_workers]).to receive(:set).with({}, unicorn_workers) + context 'without http server' do + it "does set unicorn_workers to 0" do + expect(subject.metrics[:unicorn_workers]).to receive(:set).with({}, 0) - subject.sample + subject.sample + end + end + + context 'with http server' do + let(:http_server_class) { Struct.new(:worker_processes) } + let!(:http_server) { http_server_class.new(5) } + + before do + stub_const('Unicorn::HttpServer', http_server_class) + end + + it "sets additional metrics" do + expect(subject.metrics[:unicorn_workers]).to receive(:set).with({}, 5) + + subject.sample + end end end end diff --git a/spec/lib/peek/views/redis_detailed_spec.rb b/spec/lib/peek/views/redis_detailed_spec.rb new file mode 100644 index 00000000000..da13b6df53b --- /dev/null +++ b/spec/lib/peek/views/redis_detailed_spec.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Peek::Views::RedisDetailed do + let(:redis_detailed_class) do + Class.new do + include Peek::Views::RedisDetailed + end + end + + subject { redis_detailed_class.new } + + using RSpec::Parameterized::TableSyntax + + where(:cmd, :expected) do + [:auth, 'test'] | 'auth <redacted>' + [:set, 'key', 'value'] | 'set key <redacted>' + [:set, 'bad'] | 'set bad' + [:hmset, 'key1', 'value1', 'key2', 'value2'] | 'hmset key1 <redacted>' + [:get, 'key'] | 'get key' + end + + with_them do + it 'scrubs Redis commands', :request_store do + subject.detail_store << { cmd: cmd, duration: 1.second } + + expect(subject.details.count).to eq(1) + expect(subject.details.first) + .to eq({ + cmd: expected, + duration: 1000 + }) + end + end +end diff --git a/spec/models/concerns/project_api_compatibility_spec.rb b/spec/models/concerns/project_api_compatibility_spec.rb new file mode 100644 index 00000000000..8cecd4fe7bc --- /dev/null +++ b/spec/models/concerns/project_api_compatibility_spec.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe ProjectAPICompatibility do + let(:project) { create(:project) } + + # git_strategy + it "converts build_git_strategy=fetch to build_allow_git_fetch=true" do + project.update!(build_git_strategy: 'fetch') + expect(project.build_allow_git_fetch).to eq(true) + end + + it "converts build_git_strategy=clone to build_allow_git_fetch=false" do + project.update!(build_git_strategy: 'clone') + expect(project.build_allow_git_fetch).to eq(false) + end + + # auto_devops_enabled + it "converts auto_devops_enabled=false to auto_devops_enabled?=false" do + expect(project.auto_devops_enabled?).to eq(true) + project.update!(auto_devops_enabled: false) + expect(project.auto_devops_enabled?).to eq(false) + end + + it "converts auto_devops_enabled=true to auto_devops_enabled?=true" do + expect(project.auto_devops_enabled?).to eq(true) + project.update!(auto_devops_enabled: true) + expect(project.auto_devops_enabled?).to eq(true) + end + + # auto_devops_deploy_strategy + it "converts auto_devops_deploy_strategy=timed_incremental to auto_devops.deploy_strategy=timed_incremental" do + expect(project.auto_devops).to be_nil + project.update!(auto_devops_deploy_strategy: 'timed_incremental') + expect(project.auto_devops.deploy_strategy).to eq('timed_incremental') + end +end diff --git a/spec/models/concerns/project_features_compatibility_spec.rb b/spec/models/concerns/project_features_compatibility_spec.rb index 5aa43b58217..1fe176ab5af 100644 --- a/spec/models/concerns/project_features_compatibility_spec.rb +++ b/spec/models/concerns/project_features_compatibility_spec.rb @@ -4,7 +4,8 @@ require 'spec_helper' describe ProjectFeaturesCompatibility do let(:project) { create(:project) } - let(:features) { %w(issues wiki builds merge_requests snippets) } + let(:features_except_repository) { %w(issues wiki builds merge_requests snippets) } + let(:features) { features_except_repository + ['repository'] } # We had issues_enabled, snippets_enabled, builds_enabled, merge_requests_enabled and issues_enabled fields on projects table # All those fields got moved to a new table called project_feature and are now integers instead of booleans @@ -12,30 +13,37 @@ describe ProjectFeaturesCompatibility do # So we can keep it compatible it "converts fields from 'true' to ProjectFeature::ENABLED" do - features.each do |feature| + features_except_repository.each do |feature| project.update_attribute("#{feature}_enabled".to_sym, "true") expect(project.project_feature.public_send("#{feature}_access_level")).to eq(ProjectFeature::ENABLED) end end it "converts fields from 'false' to ProjectFeature::DISABLED" do - features.each do |feature| + features_except_repository.each do |feature| project.update_attribute("#{feature}_enabled".to_sym, "false") expect(project.project_feature.public_send("#{feature}_access_level")).to eq(ProjectFeature::DISABLED) end end it "converts fields from true to ProjectFeature::ENABLED" do - features.each do |feature| + features_except_repository.each do |feature| project.update_attribute("#{feature}_enabled".to_sym, true) expect(project.project_feature.public_send("#{feature}_access_level")).to eq(ProjectFeature::ENABLED) end end it "converts fields from false to ProjectFeature::DISABLED" do - features.each do |feature| + features_except_repository.each do |feature| project.update_attribute("#{feature}_enabled".to_sym, false) expect(project.project_feature.public_send("#{feature}_access_level")).to eq(ProjectFeature::DISABLED) end end + + it "accepts private as ProjectFeature::PRIVATE" do + features.each do |feature| + project.update!("#{feature}_access_level".to_sym => 'private') + expect(project.project_feature.public_send("#{feature}_access_level")).to eq(ProjectFeature::PRIVATE) + end + end end diff --git a/spec/models/concerns/reactive_caching_spec.rb b/spec/models/concerns/reactive_caching_spec.rb index e2ab9ddd4a5..3d026932f59 100644 --- a/spec/models/concerns/reactive_caching_spec.rb +++ b/spec/models/concerns/reactive_caching_spec.rb @@ -47,30 +47,12 @@ describe ReactiveCaching, :use_clean_rails_memory_store_caching do subject(:go!) { instance.result } - context 'when cache is empty' do - it { is_expected.to be_nil } - - it 'enqueues a background worker to bootstrap the cache' do - expect(ReactiveCachingWorker).to receive(:perform_async).with(CacheTest, 666) - - go! - end - - it 'updates the cache lifespan' do - expect(reactive_cache_alive?(instance)).to be_falsy - - go! - - expect(reactive_cache_alive?(instance)).to be_truthy - end - end - - context 'when the cache is full' do + shared_examples 'a cacheable value' do |cached_value| before do - stub_reactive_cache(instance, 4) + stub_reactive_cache(instance, cached_value) end - it { is_expected.to eq(4) } + it { is_expected.to eq(cached_value) } it 'does not enqueue a background worker' do expect(ReactiveCachingWorker).not_to receive(:perform_async) @@ -90,9 +72,7 @@ describe ReactiveCaching, :use_clean_rails_memory_store_caching do end it { is_expected.to be_nil } - end - context 'when cache was invalidated' do it 'refreshes cache' do expect(ReactiveCachingWorker).to receive(:perform_async).with(CacheTest, 666) @@ -101,12 +81,34 @@ describe ReactiveCaching, :use_clean_rails_memory_store_caching do end end - context 'when cache contains non-nil but blank value' do - before do - stub_reactive_cache(instance, false) + context 'when cache is empty' do + it { is_expected.to be_nil } + + it 'enqueues a background worker to bootstrap the cache' do + expect(ReactiveCachingWorker).to receive(:perform_async).with(CacheTest, 666) + + go! end - it { is_expected.to eq(false) } + it 'updates the cache lifespan' do + expect(reactive_cache_alive?(instance)).to be_falsy + + go! + + expect(reactive_cache_alive?(instance)).to be_truthy + end + end + + context 'when the cache is full' do + it_behaves_like 'a cacheable value', 4 + end + + context 'when the cache contains non-nil but blank value' do + it_behaves_like 'a cacheable value', false + end + + context 'when the cache contains nil value' do + it_behaves_like 'a cacheable value', nil end end diff --git a/spec/models/discussion_spec.rb b/spec/models/discussion_spec.rb index 22d4dab0617..950bdec4d00 100644 --- a/spec/models/discussion_spec.rb +++ b/spec/models/discussion_spec.rb @@ -10,6 +10,20 @@ describe Discussion do let(:second_note) { create(:diff_note_on_merge_request, in_reply_to: first_note) } let(:third_note) { create(:diff_note_on_merge_request) } + describe '.lazy_find' do + let!(:note1) { create(:discussion_note_on_merge_request).to_discussion } + let!(:note2) { create(:discussion_note_on_merge_request, in_reply_to: note1).to_discussion } + + subject { [note1, note2].map { |note| described_class.lazy_find(note.discussion_id) } } + + it 'batches requests' do + expect do + [described_class.lazy_find(note1.id), + described_class.lazy_find(note2.id)].map(&:__sync) + end.not_to exceed_query_limit(1) + end + end + describe '.build' do it 'returns a discussion of the right type' do discussion = described_class.build([first_note, second_note], merge_request) diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index fe6d68aff3f..9b0c232f370 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -3220,4 +3220,34 @@ describe MergeRequest do it { is_expected.to be_truthy } end end + + describe '#cleanup_refs' do + subject { merge_request.cleanup_refs(only: only) } + + let(:merge_request) { build(:merge_request) } + + context 'when removing all refs' do + let(:only) { :all } + + it 'deletes all refs from the target project' do + expect(merge_request.target_project.repository) + .to receive(:delete_refs) + .with(merge_request.ref_path, merge_request.merge_ref_path, merge_request.train_ref_path) + + subject + end + end + + context 'when removing only train ref' do + let(:only) { :train } + + it 'deletes train ref from the target project' do + expect(merge_request.target_project.repository) + .to receive(:delete_refs) + .with(merge_request.train_ref_path) + + subject + end + end + end end diff --git a/spec/models/project_statistics_spec.rb b/spec/models/project_statistics_spec.rb index 1cb49d83ffa..db3e4902c64 100644 --- a/spec/models/project_statistics_spec.rb +++ b/spec/models/project_statistics_spec.rb @@ -135,6 +135,49 @@ describe ProjectStatistics do expect(statistics.wiki_size).to eq(0) end end + + context 'when the column is namespace relatable' do + let(:namespace) { create(:group) } + let(:project) { create(:project, namespace: namespace) } + + context 'when the feature flag is off' do + it 'does not schedule the aggregation worker' do + stub_feature_flags(update_statistics_namespace: false, namespace: namespace) + + expect(Namespaces::ScheduleAggregationWorker) + .not_to receive(:perform_async) + + statistics.refresh!(only: [:lfs_objects_size]) + end + end + + context 'when the feature flag is on' do + it 'schedules the aggregation worker' do + expect(Namespaces::ScheduleAggregationWorker) + .to receive(:perform_async) + + statistics.refresh!(only: [:lfs_objects_size]) + end + end + + context 'when no argument is passed' do + it 'schedules the aggregation worker' do + expect(Namespaces::ScheduleAggregationWorker) + .to receive(:perform_async) + + statistics.refresh! + end + end + end + + context 'when the column is not namespace relatable' do + it 'does not schedules an aggregation worker' do + expect(Namespaces::ScheduleAggregationWorker) + .not_to receive(:perform_async) + + statistics.refresh!(only: [:commit_count]) + end + end end describe '#update_commit_count' do diff --git a/spec/requests/api/graphql/mutations/notes/create/diff_note_spec.rb b/spec/requests/api/graphql/mutations/notes/create/diff_note_spec.rb new file mode 100644 index 00000000000..b04fcb9aece --- /dev/null +++ b/spec/requests/api/graphql/mutations/notes/create/diff_note_spec.rb @@ -0,0 +1,64 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Adding a DiffNote' do + include GraphqlHelpers + + set(:current_user) { create(:user) } + let(:noteable) { create(:merge_request, source_project: project, target_project: project) } + let(:project) { create(:project, :repository) } + let(:diff_refs) { noteable.diff_refs } + let(:mutation) do + variables = { + noteable_id: GitlabSchema.id_from_object(noteable).to_s, + body: 'Body text', + position: { + paths: { + old_path: 'files/ruby/popen.rb', + new_path: 'files/ruby/popen2.rb' + }, + new_line: 14, + base_sha: diff_refs.base_sha, + head_sha: diff_refs.head_sha, + start_sha: diff_refs.start_sha + } + } + + graphql_mutation(:create_diff_note, variables) + end + + def mutation_response + graphql_mutation_response(:create_diff_note) + end + + it_behaves_like 'a Note mutation when the user does not have permission' + + context 'when the user has permission' do + before do + project.add_developer(current_user) + end + + it_behaves_like 'a Note mutation that creates a Note' + + it_behaves_like 'a Note mutation when there are active record validation errors', model: DiffNote + + context do + let(:diff_refs) { build(:merge_request).diff_refs } # Allow fake diff refs so arguments are valid + + it_behaves_like 'a Note mutation when the given resource id is not for a Noteable' + end + + it 'returns the note with the correct position' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(mutation_response['note']['body']).to eq('Body text') + mutation_position_response = mutation_response['note']['position'] + expect(mutation_position_response['positionType']).to eq('text') + expect(mutation_position_response['filePath']).to eq('files/ruby/popen2.rb') + expect(mutation_position_response['oldPath']).to eq('files/ruby/popen.rb') + expect(mutation_position_response['newPath']).to eq('files/ruby/popen2.rb') + expect(mutation_position_response['newLine']).to eq(14) + end + end +end diff --git a/spec/requests/api/graphql/mutations/notes/create/image_diff_note_spec.rb b/spec/requests/api/graphql/mutations/notes/create/image_diff_note_spec.rb new file mode 100644 index 00000000000..3ba6c689024 --- /dev/null +++ b/spec/requests/api/graphql/mutations/notes/create/image_diff_note_spec.rb @@ -0,0 +1,70 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Adding an image DiffNote' do + include GraphqlHelpers + + set(:current_user) { create(:user) } + let(:noteable) { create(:merge_request, source_project: project, target_project: project) } + let(:project) { create(:project, :repository) } + let(:diff_refs) { noteable.diff_refs } + let(:mutation) do + variables = { + noteable_id: GitlabSchema.id_from_object(noteable).to_s, + body: 'Body text', + position: { + paths: { + old_path: 'files/images/any_image.png', + new_path: 'files/images/any_image2.png' + }, + width: 100, + height: 200, + x: 1, + y: 2, + base_sha: diff_refs.base_sha, + head_sha: diff_refs.head_sha, + start_sha: diff_refs.start_sha + } + } + + graphql_mutation(:create_image_diff_note, variables) + end + + def mutation_response + graphql_mutation_response(:create_image_diff_note) + end + + it_behaves_like 'a Note mutation when the user does not have permission' + + context 'when the user has permission' do + before do + project.add_developer(current_user) + end + + it_behaves_like 'a Note mutation that creates a Note' + + it_behaves_like 'a Note mutation when there are active record validation errors', model: DiffNote + + context do + let(:diff_refs) { build(:merge_request).diff_refs } # Allow fake diff refs so arguments are valid + + it_behaves_like 'a Note mutation when the given resource id is not for a Noteable' + end + + it 'returns the note with the correct position' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(mutation_response['note']['body']).to eq('Body text') + mutation_position_response = mutation_response['note']['position'] + expect(mutation_position_response['filePath']).to eq('files/images/any_image2.png') + expect(mutation_position_response['oldPath']).to eq('files/images/any_image.png') + expect(mutation_position_response['newPath']).to eq('files/images/any_image2.png') + expect(mutation_position_response['positionType']).to eq('image') + expect(mutation_position_response['width']).to eq(100) + expect(mutation_position_response['height']).to eq(200) + expect(mutation_position_response['x']).to eq(1) + expect(mutation_position_response['y']).to eq(2) + end + end +end diff --git a/spec/requests/api/graphql/mutations/notes/create/note_spec.rb b/spec/requests/api/graphql/mutations/notes/create/note_spec.rb new file mode 100644 index 00000000000..14aaa430ac9 --- /dev/null +++ b/spec/requests/api/graphql/mutations/notes/create/note_spec.rb @@ -0,0 +1,64 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Adding a Note' do + include GraphqlHelpers + + set(:current_user) { create(:user) } + let(:noteable) { create(:merge_request, source_project: project, target_project: project) } + let(:project) { create(:project) } + let(:discussion) { nil } + let(:mutation) do + variables = { + noteable_id: GitlabSchema.id_from_object(noteable).to_s, + discussion_id: (GitlabSchema.id_from_object(discussion).to_s if discussion), + body: 'Body text' + } + + graphql_mutation(:create_note, variables) + end + + def mutation_response + graphql_mutation_response(:create_note) + end + + it_behaves_like 'a Note mutation when the user does not have permission' + + context 'when the user has permission' do + before do + project.add_developer(current_user) + end + + it_behaves_like 'a Note mutation that creates a Note' + + it_behaves_like 'a Note mutation when there are active record validation errors' + + it_behaves_like 'a Note mutation when the given resource id is not for a Noteable' + + it 'returns the note' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(mutation_response['note']['body']).to eq('Body text') + end + + describe 'creating Notes in reply to a discussion' do + context 'when the user does not have permission to create notes on the discussion' do + let(:discussion) { create(:discussion_note).to_discussion } + + it_behaves_like 'a mutation that returns top-level errors', + errors: ["The discussion does not exist or you don't have permission to perform this action"] + end + + context 'when the user has permission to create notes on the discussion' do + let(:discussion) { create(:discussion_note, project: project).to_discussion } + + it 'creates a Note in a discussion' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(mutation_response['note']['discussion']['id']).to eq(discussion.to_global_id.to_s) + end + end + end + end +end diff --git a/spec/requests/api/graphql/mutations/notes/destroy_spec.rb b/spec/requests/api/graphql/mutations/notes/destroy_spec.rb new file mode 100644 index 00000000000..337a6e6f6e6 --- /dev/null +++ b/spec/requests/api/graphql/mutations/notes/destroy_spec.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Destroying a Note' do + include GraphqlHelpers + + let!(:note) { create(:note) } + let(:mutation) do + variables = { + id: GitlabSchema.id_from_object(note).to_s + } + + graphql_mutation(:destroy_note, variables) + end + + def mutation_response + graphql_mutation_response(:destroy_note) + end + + context 'when the user does not have permission' do + let(:current_user) { create(:user) } + + it_behaves_like 'a mutation that returns top-level errors', + errors: ['The resource that you are attempting to access does not exist or you don\'t have permission to perform this action'] + + it 'does not destroy the Note' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + end.not_to change { Note.count } + end + end + + context 'when the user has permission' do + let(:current_user) { note.author } + + it_behaves_like 'a Note mutation when the given resource id is not for a Note' + + it 'destroys the Note' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + end.to change { Note.count }.by(-1) + end + + it 'returns an empty Note' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(mutation_response).to have_key('note') + expect(mutation_response['note']).to be_nil + end + end +end diff --git a/spec/requests/api/graphql/mutations/notes/update_spec.rb b/spec/requests/api/graphql/mutations/notes/update_spec.rb new file mode 100644 index 00000000000..958f640995a --- /dev/null +++ b/spec/requests/api/graphql/mutations/notes/update_spec.rb @@ -0,0 +1,72 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Updating a Note' do + include GraphqlHelpers + + let!(:note) { create(:note, note: original_body) } + let(:original_body) { 'Initial body text' } + let(:updated_body) { 'Updated body text' } + let(:mutation) do + variables = { + id: GitlabSchema.id_from_object(note).to_s, + body: updated_body + } + + graphql_mutation(:update_note, variables) + end + + def mutation_response + graphql_mutation_response(:update_note) + end + + context 'when the user does not have permission' do + let(:current_user) { create(:user) } + + it_behaves_like 'a mutation that returns top-level errors', + errors: ['The resource that you are attempting to access does not exist or you don\'t have permission to perform this action'] + + it 'does not update the Note' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(note.reload.note).to eq(original_body) + end + end + + context 'when the user has permission' do + let(:current_user) { note.author } + + it_behaves_like 'a Note mutation when the given resource id is not for a Note' + + it 'updates the Note' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(note.reload.note).to eq(updated_body) + end + + it 'returns the updated Note' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(mutation_response['note']['body']).to eq(updated_body) + end + + context 'when there are ActiveRecord validation errors' do + let(:updated_body) { '' } + + it_behaves_like 'a mutation that returns errors in the response', errors: ["Note can't be blank"] + + it 'does not update the Note' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(note.reload.note).to eq(original_body) + end + + it 'returns the Note with its original body' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(mutation_response['note']['body']).to eq(original_body) + end + end + end +end diff --git a/spec/requests/api/graphql_spec.rb b/spec/requests/api/graphql_spec.rb index 67371cb35b6..54401ec4085 100644 --- a/spec/requests/api/graphql_spec.rb +++ b/spec/requests/api/graphql_spec.rb @@ -6,16 +6,6 @@ describe 'GraphQL' do let(:query) { graphql_query_for('echo', 'text' => 'Hello world' ) } - context 'graphql is disabled by feature flag' do - before do - stub_feature_flags(graphql: false) - end - - it 'does not generate a route for GraphQL' do - expect { post_graphql(query) }.to raise_error(ActionController::RoutingError) - end - end - context 'logging' do shared_examples 'logging a graphql query' do let(:expected_params) do diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index c67412a44c1..a2aae257352 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -1102,6 +1102,12 @@ describe API::Projects do expect(json_response['wiki_enabled']).to be_present expect(json_response['jobs_enabled']).to be_present expect(json_response['snippets_enabled']).to be_present + expect(json_response['snippets_access_level']).to be_present + expect(json_response['repository_access_level']).to be_present + expect(json_response['issues_access_level']).to be_present + expect(json_response['merge_requests_access_level']).to be_present + expect(json_response['wiki_access_level']).to be_present + expect(json_response['builds_access_level']).to be_present expect(json_response['resolve_outdated_diff_discussions']).to eq(project.resolve_outdated_diff_discussions) expect(json_response['container_registry_enabled']).to be_present expect(json_response['created_at']).to be_present @@ -1913,6 +1919,34 @@ describe API::Projects do end end + it 'updates builds_access_level' do + project_param = { builds_access_level: 'private' } + + put api("/projects/#{project3.id}", user), params: project_param + + expect(response).to have_gitlab_http_status(200) + + expect(json_response['builds_access_level']).to eq('private') + end + + it 'updates build_git_strategy' do + project_param = { build_git_strategy: 'clone' } + + put api("/projects/#{project3.id}", user), params: project_param + + expect(response).to have_gitlab_http_status(200) + + expect(json_response['build_git_strategy']).to eq('clone') + end + + it 'rejects to update build_git_strategy when build_git_strategy is invalid' do + project_param = { build_git_strategy: 'invalid' } + + put api("/projects/#{project3.id}", user), params: project_param + + expect(response).to have_gitlab_http_status(400) + end + it 'updates merge_method' do project_param = { merge_method: 'ff' } @@ -1946,6 +1980,26 @@ describe API::Projects do '-/system/project/avatar/'\ "#{project3.id}/banana_sample.gif") end + + it 'updates auto_devops_deploy_strategy' do + project_param = { auto_devops_deploy_strategy: 'timed_incremental' } + + put api("/projects/#{project3.id}", user), params: project_param + + expect(response).to have_gitlab_http_status(200) + + expect(json_response['auto_devops_deploy_strategy']).to eq('timed_incremental') + end + + it 'updates auto_devops_enabled' do + project_param = { auto_devops_enabled: false } + + put api("/projects/#{project3.id}", user), params: project_param + + expect(response).to have_gitlab_http_status(200) + + expect(json_response['auto_devops_enabled']).to eq(false) + end end context 'when authenticated as project maintainer' do diff --git a/spec/requests/api/user_counts_spec.rb b/spec/requests/api/user_counts_spec.rb new file mode 100644 index 00000000000..c833bd047e2 --- /dev/null +++ b/spec/requests/api/user_counts_spec.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe API::UserCounts do + let(:user) { create(:user) } + let(:project) { create(:project, :public) } + + let!(:merge_request) { create(:merge_request, :simple, author: user, assignees: [user], source_project: project, title: "Test") } + + describe 'GET /user_counts' do + context 'when unauthenticated' do + it 'returns authentication error' do + get api('/user_counts') + + expect(response.status).to eq(401) + end + end + + context 'when authenticated' do + it 'returns open counts for current user' do + get api('/user_counts', user) + + expect(response.status).to eq(200) + expect(json_response).to be_a Hash + expect(json_response['merge_requests']).to eq(1) + end + + it 'updates the mr count when a new mr is assigned' do + create(:merge_request, source_project: project, author: user, assignees: [user]) + + get api('/user_counts', user) + + expect(response.status).to eq(200) + expect(json_response).to be_a Hash + expect(json_response['merge_requests']).to eq(2) + end + end + end +end diff --git a/spec/routing/api_routing_spec.rb b/spec/routing/api_routing_spec.rb deleted file mode 100644 index 3c48ead4ff2..00000000000 --- a/spec/routing/api_routing_spec.rb +++ /dev/null @@ -1,23 +0,0 @@ -require 'spec_helper' - -describe 'api', 'routing' do - context 'when graphql is disabled' do - before do - stub_feature_flags(graphql: false) - end - - it 'does not route to the GraphqlController' do - expect(post('/api/graphql')).not_to route_to('graphql#execute') - end - end - - context 'when graphql is enabled' do - before do - stub_feature_flags(graphql: true) - end - - it 'routes to the GraphqlController' do - expect(post('/api/graphql')).to route_to('graphql#execute') - end - end -end diff --git a/spec/services/issuable/bulk_update_service_spec.rb b/spec/services/issuable/bulk_update_service_spec.rb index aebc5ba2874..3d2d4b5f216 100644 --- a/spec/services/issuable/bulk_update_service_spec.rb +++ b/spec/services/issuable/bulk_update_service_spec.rb @@ -11,343 +11,371 @@ describe Issuable::BulkUpdateService do .reverse_merge(issuable_ids: Array(issuables).map(&:id).join(',')) type = Array(issuables).first.model_name.param_key - Issuable::BulkUpdateService.new(project, user, bulk_update_params).execute(type) + Issuable::BulkUpdateService.new(user, bulk_update_params).execute(type) end - describe 'close issues' do - let(:issues) { create_list(:issue, 2, project: project) } - - it 'succeeds and returns the correct number of issues updated' do - result = bulk_update(issues, state_event: 'close') + shared_examples 'updates milestones' do + it 'succeeds' do + result = bulk_update(issues, milestone_id: milestone.id) expect(result[:success]).to be_truthy expect(result[:count]).to eq(issues.count) end - it 'closes all the issues passed' do - bulk_update(issues, state_event: 'close') + it 'updates the issues milestone' do + bulk_update(issues, milestone_id: milestone.id) - expect(project.issues.opened).to be_empty - expect(project.issues.closed).not_to be_empty + issues.each do |issue| + expect(issue.reload.milestone).to eq(milestone) + end end + end - context 'when issue for a different project is created' do - let(:private_project) { create(:project, :private) } - let(:issue) { create(:issue, project: private_project, author: user) } + context 'with project issues' do + describe 'close issues' do + let(:issues) { create_list(:issue, 2, project: project) } - context 'when user has access to the project' do - it 'closes all issues passed' do - private_project.add_maintainer(user) + it 'succeeds and returns the correct number of issues updated' do + result = bulk_update(issues, state_event: 'close') - bulk_update(issues + [issue], state_event: 'close') + expect(result[:success]).to be_truthy + expect(result[:count]).to eq(issues.count) + end - expect(project.issues.opened).to be_empty - expect(project.issues.closed).not_to be_empty - expect(private_project.issues.closed).not_to be_empty - end + it 'closes all the issues passed' do + bulk_update(issues, state_event: 'close') + + expect(project.issues.opened).to be_empty + expect(project.issues.closed).not_to be_empty end - context 'when user does not have access to project' do - it 'only closes all issues that the user has access to' do - bulk_update(issues + [issue], state_event: 'close') + context 'when issue for a different project is created' do + let(:private_project) { create(:project, :private) } + let(:issue) { create(:issue, project: private_project, author: user) } + + context 'when user has access to the project' do + it 'closes all issues passed' do + private_project.add_maintainer(user) + + bulk_update(issues + [issue], state_event: 'close') + + expect(project.issues.opened).to be_empty + expect(project.issues.closed).not_to be_empty + expect(private_project.issues.closed).not_to be_empty + end + end + + context 'when user does not have access to project' do + it 'only closes all issues that the user has access to' do + bulk_update(issues + [issue], state_event: 'close') - expect(project.issues.opened).to be_empty - expect(project.issues.closed).not_to be_empty - expect(private_project.issues.closed).to be_empty + expect(project.issues.opened).to be_empty + expect(project.issues.closed).not_to be_empty + expect(private_project.issues.closed).to be_empty + end end end end - end - describe 'reopen issues' do - let(:issues) { create_list(:closed_issue, 2, project: project) } + describe 'reopen issues' do + let(:issues) { create_list(:closed_issue, 2, project: project) } - it 'succeeds and returns the correct number of issues updated' do - result = bulk_update(issues, state_event: 'reopen') + it 'succeeds and returns the correct number of issues updated' do + result = bulk_update(issues, state_event: 'reopen') - expect(result[:success]).to be_truthy - expect(result[:count]).to eq(issues.count) - end + expect(result[:success]).to be_truthy + expect(result[:count]).to eq(issues.count) + end - it 'reopens all the issues passed' do - bulk_update(issues, state_event: 'reopen') + it 'reopens all the issues passed' do + bulk_update(issues, state_event: 'reopen') - expect(project.issues.closed).to be_empty - expect(project.issues.opened).not_to be_empty + expect(project.issues.closed).to be_empty + expect(project.issues.opened).not_to be_empty + end end - end - describe 'updating merge request assignee' do - let(:merge_request) { create(:merge_request, target_project: project, source_project: project, assignees: [user]) } + describe 'updating merge request assignee' do + let(:merge_request) { create(:merge_request, target_project: project, source_project: project, assignees: [user]) } - context 'when the new assignee ID is a valid user' do - it 'succeeds' do - new_assignee = create(:user) - project.add_developer(new_assignee) + context 'when the new assignee ID is a valid user' do + it 'succeeds' do + new_assignee = create(:user) + project.add_developer(new_assignee) - result = bulk_update(merge_request, assignee_ids: [user.id, new_assignee.id]) + result = bulk_update(merge_request, assignee_ids: [user.id, new_assignee.id]) - expect(result[:success]).to be_truthy - expect(result[:count]).to eq(1) - end + expect(result[:success]).to be_truthy + expect(result[:count]).to eq(1) + end - it 'updates the assignee to the user ID passed' do - assignee = create(:user) - project.add_developer(assignee) + it 'updates the assignee to the user ID passed' do + assignee = create(:user) + project.add_developer(assignee) - expect { bulk_update(merge_request, assignee_ids: [assignee.id]) } - .to change { merge_request.reload.assignee_ids }.from([user.id]).to([assignee.id]) + expect { bulk_update(merge_request, assignee_ids: [assignee.id]) } + .to change { merge_request.reload.assignee_ids }.from([user.id]).to([assignee.id]) + end end - end - context "when the new assignee ID is #{IssuableFinder::NONE}" do - it 'unassigns the issues' do - expect { bulk_update(merge_request, assignee_ids: [IssuableFinder::NONE]) } - .to change { merge_request.reload.assignee_ids }.to([]) + context "when the new assignee ID is #{IssuableFinder::NONE}" do + it 'unassigns the issues' do + expect { bulk_update(merge_request, assignee_ids: [IssuableFinder::NONE]) } + .to change { merge_request.reload.assignee_ids }.to([]) + end end - end - context 'when the new assignee ID is not present' do - it 'does not unassign' do - expect { bulk_update(merge_request, assignee_ids: []) } - .not_to change { merge_request.reload.assignee_ids } + context 'when the new assignee ID is not present' do + it 'does not unassign' do + expect { bulk_update(merge_request, assignee_ids: []) } + .not_to change { merge_request.reload.assignee_ids } + end end end - end - describe 'updating issue assignee' do - let(:issue) { create(:issue, project: project, assignees: [user]) } + describe 'updating issue assignee' do + let(:issue) { create(:issue, project: project, assignees: [user]) } - context 'when the new assignee ID is a valid user' do - it 'succeeds' do - new_assignee = create(:user) - project.add_developer(new_assignee) + context 'when the new assignee ID is a valid user' do + it 'succeeds' do + new_assignee = create(:user) + project.add_developer(new_assignee) - result = bulk_update(issue, assignee_ids: [new_assignee.id]) + result = bulk_update(issue, assignee_ids: [new_assignee.id]) - expect(result[:success]).to be_truthy - expect(result[:count]).to eq(1) - end + expect(result[:success]).to be_truthy + expect(result[:count]).to eq(1) + end - it 'updates the assignee to the user ID passed' do - assignee = create(:user) - project.add_developer(assignee) - expect { bulk_update(issue, assignee_ids: [assignee.id]) } - .to change { issue.reload.assignees.first }.from(user).to(assignee) + it 'updates the assignee to the user ID passed' do + assignee = create(:user) + project.add_developer(assignee) + expect { bulk_update(issue, assignee_ids: [assignee.id]) } + .to change { issue.reload.assignees.first }.from(user).to(assignee) + end end - end - context "when the new assignee ID is #{IssuableFinder::NONE}" do - it "unassigns the issues" do - expect { bulk_update(issue, assignee_ids: [IssuableFinder::NONE.to_s]) } - .to change { issue.reload.assignees.count }.from(1).to(0) + context "when the new assignee ID is #{IssuableFinder::NONE}" do + it "unassigns the issues" do + expect { bulk_update(issue, assignee_ids: [IssuableFinder::NONE.to_s]) } + .to change { issue.reload.assignees.count }.from(1).to(0) + end end - end - context 'when the new assignee ID is not present' do - it 'does not unassign' do - expect { bulk_update(issue, assignee_ids: []) } - .not_to change { issue.reload.assignees } + context 'when the new assignee ID is not present' do + it 'does not unassign' do + expect { bulk_update(issue, assignee_ids: []) } + .not_to change { issue.reload.assignees } + end end end - end - - describe 'updating milestones' do - let(:issue) { create(:issue, project: project) } - let(:milestone) { create(:milestone, project: project) } - it 'succeeds' do - result = bulk_update(issue, milestone_id: milestone.id) + describe 'updating milestones' do + let(:issues) { [create(:issue, project: project)] } + let(:milestone) { create(:milestone, project: project) } - expect(result[:success]).to be_truthy - expect(result[:count]).to eq(1) + it_behaves_like 'updates milestones' end - it 'updates the issue milestone' do - expect { bulk_update(issue, milestone_id: milestone.id) } - .to change { issue.reload.milestone }.from(nil).to(milestone) - end - end - - describe 'updating labels' do - def create_issue_with_labels(labels) - create(:labeled_issue, project: project, labels: labels) - end + describe 'updating labels' do + def create_issue_with_labels(labels) + create(:labeled_issue, project: project, labels: labels) + end - let(:bug) { create(:label, project: project) } - let(:regression) { create(:label, project: project) } - let(:merge_requests) { create(:label, project: project) } - - let(:issue_all_labels) { create_issue_with_labels([bug, regression, merge_requests]) } - let(:issue_bug_and_regression) { create_issue_with_labels([bug, regression]) } - let(:issue_bug_and_merge_requests) { create_issue_with_labels([bug, merge_requests]) } - let(:issue_no_labels) { create(:issue, project: project) } - let(:issues) { [issue_all_labels, issue_bug_and_regression, issue_bug_and_merge_requests, issue_no_labels] } - - let(:labels) { [] } - let(:add_labels) { [] } - let(:remove_labels) { [] } - - let(:bulk_update_params) do - { - label_ids: labels.map(&:id), - add_label_ids: add_labels.map(&:id), - remove_label_ids: remove_labels.map(&:id) - } - end + let(:bug) { create(:label, project: project) } + let(:regression) { create(:label, project: project) } + let(:merge_requests) { create(:label, project: project) } - before do - bulk_update(issues, bulk_update_params) - end + let(:issue_all_labels) { create_issue_with_labels([bug, regression, merge_requests]) } + let(:issue_bug_and_regression) { create_issue_with_labels([bug, regression]) } + let(:issue_bug_and_merge_requests) { create_issue_with_labels([bug, merge_requests]) } + let(:issue_no_labels) { create(:issue, project: project) } + let(:issues) { [issue_all_labels, issue_bug_and_regression, issue_bug_and_merge_requests, issue_no_labels] } - context 'when label_ids are passed' do - let(:issues) { [issue_all_labels, issue_no_labels] } - let(:labels) { [bug, regression] } + let(:labels) { [] } + let(:add_labels) { [] } + let(:remove_labels) { [] } - it 'updates the labels of all issues passed to the labels passed' do - expect(issues.map(&:reload).map(&:label_ids)).to all(match_array(labels.map(&:id))) + let(:bulk_update_params) do + { + label_ids: labels.map(&:id), + add_label_ids: add_labels.map(&:id), + remove_label_ids: remove_labels.map(&:id) + } end - it 'does not update issues not passed in' do - expect(issue_bug_and_regression.label_ids).to contain_exactly(bug.id, regression.id) + before do + bulk_update(issues, bulk_update_params) end - context 'when those label IDs are empty' do - let(:labels) { [] } + context 'when label_ids are passed' do + let(:issues) { [issue_all_labels, issue_no_labels] } + let(:labels) { [bug, regression] } - it 'updates the issues passed to have no labels' do - expect(issues.map(&:reload).map(&:label_ids)).to all(be_empty) + it 'updates the labels of all issues passed to the labels passed' do + expect(issues.map(&:reload).map(&:label_ids)).to all(match_array(labels.map(&:id))) end - end - end - context 'when add_label_ids are passed' do - let(:issues) { [issue_all_labels, issue_bug_and_merge_requests, issue_no_labels] } - let(:add_labels) { [bug, regression, merge_requests] } + it 'does not update issues not passed in' do + expect(issue_bug_and_regression.label_ids).to contain_exactly(bug.id, regression.id) + end - it 'adds those label IDs to all issues passed' do - expect(issues.map(&:reload).map(&:label_ids)).to all(include(*add_labels.map(&:id))) - end + context 'when those label IDs are empty' do + let(:labels) { [] } - it 'does not update issues not passed in' do - expect(issue_bug_and_regression.label_ids).to contain_exactly(bug.id, regression.id) + it 'updates the issues passed to have no labels' do + expect(issues.map(&:reload).map(&:label_ids)).to all(be_empty) + end + end end - end - context 'when remove_label_ids are passed' do - let(:issues) { [issue_all_labels, issue_bug_and_merge_requests, issue_no_labels] } - let(:remove_labels) { [bug, regression, merge_requests] } + context 'when add_label_ids are passed' do + let(:issues) { [issue_all_labels, issue_bug_and_merge_requests, issue_no_labels] } + let(:add_labels) { [bug, regression, merge_requests] } - it 'removes those label IDs from all issues passed' do - expect(issues.map(&:reload).map(&:label_ids)).to all(be_empty) - end + it 'adds those label IDs to all issues passed' do + expect(issues.map(&:reload).map(&:label_ids)).to all(include(*add_labels.map(&:id))) + end - it 'does not update issues not passed in' do - expect(issue_bug_and_regression.label_ids).to contain_exactly(bug.id, regression.id) + it 'does not update issues not passed in' do + expect(issue_bug_and_regression.label_ids).to contain_exactly(bug.id, regression.id) + end end - end - context 'when add_label_ids and remove_label_ids are passed' do - let(:issues) { [issue_all_labels, issue_bug_and_merge_requests, issue_no_labels] } - let(:add_labels) { [bug] } - let(:remove_labels) { [merge_requests] } + context 'when remove_label_ids are passed' do + let(:issues) { [issue_all_labels, issue_bug_and_merge_requests, issue_no_labels] } + let(:remove_labels) { [bug, regression, merge_requests] } - it 'adds the label IDs to all issues passed' do - expect(issues.map(&:reload).map(&:label_ids)).to all(include(bug.id)) - end + it 'removes those label IDs from all issues passed' do + expect(issues.map(&:reload).map(&:label_ids)).to all(be_empty) + end - it 'removes the label IDs from all issues passed' do - expect(issues.map(&:reload).map(&:label_ids).flatten).not_to include(merge_requests.id) + it 'does not update issues not passed in' do + expect(issue_bug_and_regression.label_ids).to contain_exactly(bug.id, regression.id) + end end - it 'does not update issues not passed in' do - expect(issue_bug_and_regression.label_ids).to contain_exactly(bug.id, regression.id) - end - end + context 'when add_label_ids and remove_label_ids are passed' do + let(:issues) { [issue_all_labels, issue_bug_and_merge_requests, issue_no_labels] } + let(:add_labels) { [bug] } + let(:remove_labels) { [merge_requests] } - context 'when add_label_ids and label_ids are passed' do - let(:issues) { [issue_all_labels, issue_bug_and_regression, issue_bug_and_merge_requests] } - let(:labels) { [merge_requests] } - let(:add_labels) { [regression] } + it 'adds the label IDs to all issues passed' do + expect(issues.map(&:reload).map(&:label_ids)).to all(include(bug.id)) + end - it 'adds the label IDs to all issues passed' do - expect(issues.map(&:reload).map(&:label_ids)).to all(include(regression.id)) - end + it 'removes the label IDs from all issues passed' do + expect(issues.map(&:reload).map(&:label_ids).flatten).not_to include(merge_requests.id) + end - it 'ignores the label IDs parameter' do - expect(issues.map(&:reload).map(&:label_ids)).to all(include(bug.id)) + it 'does not update issues not passed in' do + expect(issue_bug_and_regression.label_ids).to contain_exactly(bug.id, regression.id) + end end - it 'does not update issues not passed in' do - expect(issue_no_labels.label_ids).to be_empty - end - end + context 'when add_label_ids and label_ids are passed' do + let(:issues) { [issue_all_labels, issue_bug_and_regression, issue_bug_and_merge_requests] } + let(:labels) { [merge_requests] } + let(:add_labels) { [regression] } - context 'when remove_label_ids and label_ids are passed' do - let(:issues) { [issue_no_labels, issue_bug_and_regression] } - let(:labels) { [merge_requests] } - let(:remove_labels) { [regression] } + it 'adds the label IDs to all issues passed' do + expect(issues.map(&:reload).map(&:label_ids)).to all(include(regression.id)) + end - it 'removes the label IDs from all issues passed' do - expect(issues.map(&:reload).map(&:label_ids).flatten).not_to include(regression.id) - end + it 'ignores the label IDs parameter' do + expect(issues.map(&:reload).map(&:label_ids)).to all(include(bug.id)) + end - it 'ignores the label IDs parameter' do - expect(issues.map(&:reload).map(&:label_ids).flatten).not_to include(merge_requests.id) + it 'does not update issues not passed in' do + expect(issue_no_labels.label_ids).to be_empty + end end - it 'does not update issues not passed in' do - expect(issue_all_labels.label_ids).to contain_exactly(bug.id, regression.id, merge_requests.id) - end - end + context 'when remove_label_ids and label_ids are passed' do + let(:issues) { [issue_no_labels, issue_bug_and_regression] } + let(:labels) { [merge_requests] } + let(:remove_labels) { [regression] } - context 'when add_label_ids, remove_label_ids, and label_ids are passed' do - let(:issues) { [issue_bug_and_merge_requests, issue_no_labels] } - let(:labels) { [regression] } - let(:add_labels) { [bug] } - let(:remove_labels) { [merge_requests] } + it 'removes the label IDs from all issues passed' do + expect(issues.map(&:reload).map(&:label_ids).flatten).not_to include(regression.id) + end - it 'adds the label IDs to all issues passed' do - expect(issues.map(&:reload).map(&:label_ids)).to all(include(bug.id)) - end + it 'ignores the label IDs parameter' do + expect(issues.map(&:reload).map(&:label_ids).flatten).not_to include(merge_requests.id) + end - it 'removes the label IDs from all issues passed' do - expect(issues.map(&:reload).map(&:label_ids).flatten).not_to include(merge_requests.id) + it 'does not update issues not passed in' do + expect(issue_all_labels.label_ids).to contain_exactly(bug.id, regression.id, merge_requests.id) + end end - it 'ignores the label IDs parameter' do - expect(issues.map(&:reload).map(&:label_ids).flatten).not_to include(regression.id) - end + context 'when add_label_ids, remove_label_ids, and label_ids are passed' do + let(:issues) { [issue_bug_and_merge_requests, issue_no_labels] } + let(:labels) { [regression] } + let(:add_labels) { [bug] } + let(:remove_labels) { [merge_requests] } - it 'does not update issues not passed in' do - expect(issue_bug_and_regression.label_ids).to contain_exactly(bug.id, regression.id) + it 'adds the label IDs to all issues passed' do + expect(issues.map(&:reload).map(&:label_ids)).to all(include(bug.id)) + end + + it 'removes the label IDs from all issues passed' do + expect(issues.map(&:reload).map(&:label_ids).flatten).not_to include(merge_requests.id) + end + + it 'ignores the label IDs parameter' do + expect(issues.map(&:reload).map(&:label_ids).flatten).not_to include(regression.id) + end + + it 'does not update issues not passed in' do + expect(issue_bug_and_regression.label_ids).to contain_exactly(bug.id, regression.id) + end end end - end - describe 'subscribe to issues' do - let(:issues) { create_list(:issue, 2, project: project) } + describe 'subscribe to issues' do + let(:issues) { create_list(:issue, 2, project: project) } - it 'subscribes the given user' do - bulk_update(issues, subscription_event: 'subscribe') + it 'subscribes the given user' do + bulk_update(issues, subscription_event: 'subscribe') - expect(issues).to all(be_subscribed(user, project)) + expect(issues).to all(be_subscribed(user, project)) + end end - end - describe 'unsubscribe from issues' do - let(:issues) do - create_list(:closed_issue, 2, project: project) do |issue| - issue.subscriptions.create(user: user, project: project, subscribed: true) + describe 'unsubscribe from issues' do + let(:issues) do + create_list(:closed_issue, 2, project: project) do |issue| + issue.subscriptions.create(user: user, project: project, subscribed: true) + end + end + + it 'unsubscribes the given user' do + bulk_update(issues, subscription_event: 'unsubscribe') + + issues.each do |issue| + expect(issue).not_to be_subscribed(user, project) + end end end + end - it 'unsubscribes the given user' do - bulk_update(issues, subscription_event: 'unsubscribe') + context 'with group issues' do + let(:group) { create(:group) } - issues.each do |issue| - expect(issue).not_to be_subscribed(user, project) + context 'updating milestone' do + let(:milestone) { create(:milestone, group: group) } + let(:project1) { create(:project, :repository, group: group) } + let(:project2) { create(:project, :repository, group: group) } + let(:issue1) { create(:issue, project: project1) } + let(:issue2) { create(:issue, project: project2) } + let(:issues) { [issue1, issue2] } + + before do + group.add_maintainer(user) end + + it_behaves_like 'updates milestones' end end end diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb index 28fa5d12d9c..468e7c286d5 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -480,6 +480,22 @@ describe Issues::UpdateService, :mailer do update_issue(description: "- [x] Task 1\n- [X] Task 2") end + it 'does not check for spam on task status change' do + params = { + update_task: { + index: 1, + checked: false, + line_source: '- [x] Task 1', + line_number: 1 + } + } + service = described_class.new(project, user, params) + + expect(service).not_to receive(:spam_check) + + service.execute(issue) + end + it 'creates system note about task status change' do note1 = find_note('marked the task **Task 1** as completed') note2 = find_note('marked the task **Task 2** as completed') diff --git a/spec/services/merge_requests/merge_service_spec.rb b/spec/services/merge_requests/merge_service_spec.rb index aa759ac9edc..22578436c18 100644 --- a/spec/services/merge_requests/merge_service_spec.rb +++ b/spec/services/merge_requests/merge_service_spec.rb @@ -214,6 +214,19 @@ describe MergeRequests::MergeService do allow(Rails.logger).to receive(:error) end + context 'when source is missing' do + it 'logs and saves error' do + allow(merge_request).to receive(:diff_head_sha) { nil } + + error_message = 'No source for merge' + + service.execute(merge_request) + + expect(merge_request.merge_error).to eq(error_message) + expect(Rails.logger).to have_received(:error).with(a_string_matching(error_message)) + end + end + it 'logs and saves error if there is an exception' do error_message = 'error message' diff --git a/spec/services/merge_requests/merge_to_ref_service_spec.rb b/spec/services/merge_requests/merge_to_ref_service_spec.rb index e2f201677fa..758679edc45 100644 --- a/spec/services/merge_requests/merge_to_ref_service_spec.rb +++ b/spec/services/merge_requests/merge_to_ref_service_spec.rb @@ -191,6 +191,28 @@ describe MergeRequests::MergeToRefService do it { expect(todo).not_to be_done } end + context 'when source is missing' do + it 'returns error' do + allow(merge_request).to receive(:diff_head_sha) { nil } + + error_message = 'No source for merge' + + result = service.execute(merge_request) + + expect(result[:status]).to eq(:error) + expect(result[:message]).to eq(error_message) + end + end + + context 'when target ref is passed as a parameter' do + let(:params) { { commit_message: 'merge train', target_ref: target_ref } } + + it_behaves_like 'successfully merges to ref with merge method' do + let(:first_parent_ref) { 'refs/heads/master' } + let(:target_ref) { 'refs/merge-requests/1/train' } + end + end + describe 'cascading merge refs' do set(:project) { create(:project, :repository) } let(:params) { { commit_message: 'Cascading merge', first_parent_ref: first_parent_ref, target_ref: target_ref } } diff --git a/spec/support/helpers/git_http_helpers.rb b/spec/support/helpers/git_http_helpers.rb index cd49bb148f2..c83860d7b51 100644 --- a/spec/support/helpers/git_http_helpers.rb +++ b/spec/support/helpers/git_http_helpers.rb @@ -1,4 +1,8 @@ +require_relative 'workhorse_helpers' + module GitHttpHelpers + include WorkhorseHelpers + def clone_get(project, options = {}) get "/#{project}/info/refs", params: { service: 'git-upload-pack' }, headers: auth_env(*options.values_at(:user, :password, :spnego_request_token)) end diff --git a/spec/support/helpers/graphql_helpers.rb b/spec/support/helpers/graphql_helpers.rb index 1a09d48f4cd..ec3c460cd37 100644 --- a/spec/support/helpers/graphql_helpers.rb +++ b/spec/support/helpers/graphql_helpers.rb @@ -57,7 +57,8 @@ module GraphqlHelpers end def variables_for_mutation(name, input) - graphql_input = input.map { |name, value| [GraphqlHelpers.fieldnamerize(name), value] }.to_h + graphql_input = prepare_input_for_mutation(input) + result = { input_variable_name_for_mutation(name) => graphql_input } # Avoid trying to serialize multipart data into JSON @@ -68,6 +69,18 @@ module GraphqlHelpers end end + # Recursively convert a Hash with Ruby-style keys to GraphQL fieldname-style keys + # + # prepare_input_for_mutation({ 'my_key' => 1 }) + # => { 'myKey' => 1} + def prepare_input_for_mutation(input) + input.map do |name, value| + value = prepare_input_for_mutation(value) if value.is_a?(Hash) + + [GraphqlHelpers.fieldnamerize(name), value] + end.to_h + end + def input_variable_name_for_mutation(mutation_name) mutation_name = GraphqlHelpers.fieldnamerize(mutation_name) mutation_field = GitlabSchema.mutation.fields[mutation_name] diff --git a/spec/support/helpers/reactive_caching_helpers.rb b/spec/support/helpers/reactive_caching_helpers.rb index b76b53db0b9..528da37e8cf 100644 --- a/spec/support/helpers/reactive_caching_helpers.rb +++ b/spec/support/helpers/reactive_caching_helpers.rb @@ -10,7 +10,7 @@ module ReactiveCachingHelpers def stub_reactive_cache(subject = nil, data = nil, *qualifiers) allow(ReactiveCachingWorker).to receive(:perform_async) allow(ReactiveCachingWorker).to receive(:perform_in) - write_reactive_cache(subject, data, *qualifiers) unless data.nil? + write_reactive_cache(subject, data, *qualifiers) unless subject.nil? end def synchronous_reactive_cache(subject) diff --git a/spec/support/shared_examples/graphql/notes_creation_shared_examples.rb b/spec/support/shared_examples/graphql/notes_creation_shared_examples.rb new file mode 100644 index 00000000000..f2e1a95345b --- /dev/null +++ b/spec/support/shared_examples/graphql/notes_creation_shared_examples.rb @@ -0,0 +1,61 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'a Note mutation that does not create a Note' do + it do + expect do + post_graphql_mutation(mutation, current_user: current_user) + end.not_to change { Note.count } + end +end + +RSpec.shared_examples 'a Note mutation that creates a Note' do + it do + expect do + post_graphql_mutation(mutation, current_user: current_user) + end.to change { Note.count }.by(1) + end +end + +RSpec.shared_examples 'a Note mutation when the user does not have permission' do + it_behaves_like 'a Note mutation that does not create a Note' + + it_behaves_like 'a mutation that returns top-level errors', + errors: ['The resource that you are attempting to access does not exist or you don\'t have permission to perform this action'] +end + +RSpec.shared_examples 'a Note mutation when there are active record validation errors' do |model: Note| + before do + expect_next_instance_of(model) do |note| + expect(note).to receive(:valid?).at_least(:once).and_return(false) + expect(note).to receive_message_chain( + :errors, + :full_messages + ).and_return(['Error 1', 'Error 2']) + end + end + + it_behaves_like 'a Note mutation that does not create a Note' + + it_behaves_like 'a mutation that returns errors in the response', errors: ['Error 1', 'Error 2'] + + it 'returns an empty Note' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(mutation_response).to have_key('note') + expect(mutation_response['note']).to be_nil + end +end + +RSpec.shared_examples 'a Note mutation when the given resource id is not for a Noteable' do + let(:noteable) { create(:label, project: project) } + + it_behaves_like 'a Note mutation that does not create a Note' + + it_behaves_like 'a mutation that returns top-level errors', errors: ['Cannot add notes to this resource'] +end + +RSpec.shared_examples 'a Note mutation when the given resource id is not for a Note' do + let(:note) { create(:issue) } + + it_behaves_like 'a mutation that returns top-level errors', errors: ['Resource is not a note'] +end diff --git a/spec/workers/project_cache_worker_spec.rb b/spec/workers/project_cache_worker_spec.rb index 51afb076da1..edc55920b8e 100644 --- a/spec/workers/project_cache_worker_spec.rb +++ b/spec/workers/project_cache_worker_spec.rb @@ -36,6 +36,11 @@ describe ProjectCacheWorker do end context 'with an existing project' do + before do + lease_key = "namespace:namespaces_root_statistics:#{project.namespace_id}" + stub_exclusive_lease_taken(lease_key, timeout: Namespace::AggregationSchedule::DEFAULT_LEASE_TIMEOUT) + end + it 'refreshes the method caches' do expect_any_instance_of(Repository).to receive(:refresh_method_caches) .with(%i(readme)) @@ -81,6 +86,10 @@ describe ProjectCacheWorker do expect(UpdateProjectStatisticsWorker).not_to receive(:perform_in) + expect(Namespaces::ScheduleAggregationWorker) + .not_to receive(:perform_async) + .with(project.namespace_id) + worker.update_statistics(project, statistics) end end @@ -98,6 +107,11 @@ describe ProjectCacheWorker do .with(lease_timeout, project.id, statistics) .and_call_original + expect(Namespaces::ScheduleAggregationWorker) + .to receive(:perform_async) + .with(project.namespace_id) + .twice + worker.update_statistics(project, statistics) end end |