From cfec4ed6fe77e4150b1ea83b87f407aa0cca944c Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Thu, 1 Apr 2021 09:09:05 +0000 Subject: Add latest changes from gitlab-org/gitlab@master --- GITLAB_PAGES_VERSION | 2 +- .../javascripts/diffs/components/commit_item.vue | 61 +--------- .../diffs/components/compare_versions.vue | 51 +++++++- app/assets/stylesheets/pages/projects.scss | 5 - .../merge_requests/creations_controller.rb | 7 +- .../merge_requests/after_create_service.rb | 10 ++ app/services/merge_requests/create_service.rb | 10 -- app/views/layouts/nav/sidebar/_group.html.haml | 4 +- app/views/layouts/nav/sidebar/_project.html.haml | 4 +- app/views/projects/_home_panel.html.haml | 6 +- ...j-fix-n-1-projects-endpoint-forked-projects.yml | 5 + .../mk-speed-up-merge-request-creation-action.yml | 6 + changelogs/unreleased/project-overview-copy-id.yml | 5 + changelogs/unreleased/release-pages-1-37-0.yml | 5 + changelogs/unreleased/tc-move-commit-buttons.yml | 5 + .../merge_requests/img/commit_nav_v13_11.png | Bin 0 -> 78516 bytes .../merge_requests/img/commit_nav_v13_4.png | Bin 21170 -> 0 bytes .../reviewing_and_managing_merge_requests.md | 4 +- lib/api/entities/basic_project_details.rb | 9 +- lib/api/entities/project.rb | 2 +- ...age_with_external_authorization_service_spec.rb | 4 +- spec/features/groups/merge_requests_spec.rb | 2 +- spec/features/groups/navbar_spec.rb | 2 +- spec/features/projects/active_tabs_spec.rb | 2 +- spec/features/projects/fork_spec.rb | 2 +- .../features/projects/merge_request_button_spec.rb | 4 +- spec/features/projects/user_uses_shortcuts_spec.rb | 2 +- spec/frontend/diffs/components/commit_item_spec.js | 130 -------------------- .../diffs/components/compare_versions_spec.js | 133 ++++++++++++++++++++- spec/requests/api/projects_spec.rb | 25 ++++ .../merge_requests/after_create_service_spec.rb | 88 ++++++++++++++ .../services/merge_requests/create_service_spec.rb | 81 ------------- .../services/merge_requests/update_service_spec.rb | 14 +-- .../shared_contexts/navbar_structure_context.rb | 4 +- 34 files changed, 363 insertions(+), 331 deletions(-) create mode 100644 changelogs/unreleased/21121-fj-fix-n-1-projects-endpoint-forked-projects.yml create mode 100644 changelogs/unreleased/mk-speed-up-merge-request-creation-action.yml create mode 100644 changelogs/unreleased/project-overview-copy-id.yml create mode 100644 changelogs/unreleased/release-pages-1-37-0.yml create mode 100644 changelogs/unreleased/tc-move-commit-buttons.yml create mode 100644 doc/user/project/merge_requests/img/commit_nav_v13_11.png delete mode 100644 doc/user/project/merge_requests/img/commit_nav_v13_4.png diff --git a/GITLAB_PAGES_VERSION b/GITLAB_PAGES_VERSION index 39fc130ef85..bf50e910e62 100644 --- a/GITLAB_PAGES_VERSION +++ b/GITLAB_PAGES_VERSION @@ -1 +1 @@ -1.36.0 +1.37.0 diff --git a/app/assets/javascripts/diffs/components/commit_item.vue b/app/assets/javascripts/diffs/components/commit_item.vue index 92b317eb3f0..bc0f2fb0b69 100644 --- a/app/assets/javascripts/diffs/components/commit_item.vue +++ b/app/assets/javascripts/diffs/components/commit_item.vue @@ -1,7 +1,6 @@ @@ -146,38 +119,6 @@ export default { class="input-group-text" /> -
- - - - - {{ __('Prev') }} - - - - {{ __('Next') }} - - - -
diff --git a/app/assets/javascripts/diffs/components/compare_versions.vue b/app/assets/javascripts/diffs/components/compare_versions.vue index 3fb9787ac30..7526c5347f7 100644 --- a/app/assets/javascripts/diffs/components/compare_versions.vue +++ b/app/assets/javascripts/diffs/components/compare_versions.vue @@ -1,7 +1,8 @@ @@ -92,6 +109,38 @@ export default { {{ __('Viewing commit') }} {{ commit.short_id }}
+
+ + + + + {{ __('Prev') }} + + + + {{ __('Next') }} + + + +
X and C keyboard shortcuts. -![Merge requests commit navigation](img/commit_nav_v13_4.png) +![Merge requests commit navigation](img/commit_nav_v13_11.png) ### Incrementally expand merge request diffs diff --git a/lib/api/entities/basic_project_details.rb b/lib/api/entities/basic_project_details.rb index cf0b32bed26..2de49d6ed40 100644 --- a/lib/api/entities/basic_project_details.rb +++ b/lib/api/entities/basic_project_details.rb @@ -8,11 +8,10 @@ module API expose :default_branch, if: -> (project, options) { Ability.allowed?(options[:current_user], :download_code, project) } # Avoids an N+1 query: https://github.com/mbleigh/acts-as-taggable-on/issues/91#issuecomment-168273770 expose :tag_list do |project| - # project.tags.order(:name).pluck(:name) is the most suitable option - # to avoid loading all the ActiveRecord objects but, if we use it here - # it override the preloaded associations and makes a query - # (fixed in https://github.com/rails/rails/pull/25976). - project.tags.map(&:name).sort + # Tags is a preloaded association. If we perform then sorting + # through the database, it will trigger a new query, ending up + # in an N+1 if we have several projects + project.tags.pluck(:name).sort # rubocop:disable CodeReuse/ActiveRecord end expose :ssh_url_to_repo, :http_url_to_repo, :web_url, :readme_url diff --git a/lib/api/entities/project.rb b/lib/api/entities/project.rb index 161cdb68170..b159effdde7 100644 --- a/lib/api/entities/project.rb +++ b/lib/api/entities/project.rb @@ -135,7 +135,7 @@ module API .preload(project_group_links: { group: :route }, fork_network: :root_project, fork_network_member: :forked_from_project, - forked_from_project: [:route, :forks, :tags, namespace: :route]) + forked_from_project: [:route, :forks, :tags, :group, :project_feature, namespace: [:route, :owner]]) end # rubocop: enable CodeReuse/ActiveRecord diff --git a/spec/features/groups/group_page_with_external_authorization_service_spec.rb b/spec/features/groups/group_page_with_external_authorization_service_spec.rb index 8ef1b60d8ca..187d878472e 100644 --- a/spec/features/groups/group_page_with_external_authorization_service_spec.rb +++ b/spec/features/groups/group_page_with_external_authorization_service_spec.rb @@ -19,7 +19,7 @@ RSpec.describe 'The group page' do expect(page).to have_link('Details') expect(page).to have_link('Activity') expect(page).to have_link('Issues') - expect(page).to have_link('Merge Requests') + expect(page).to have_link('Merge requests') expect(page).to have_link('Members') end end @@ -50,7 +50,7 @@ RSpec.describe 'The group page' do expect(page).not_to have_link('Contribution') expect(page).not_to have_link('Issues') - expect(page).not_to have_link('Merge Requests') + expect(page).not_to have_link('Merge requests') expect(page).to have_link('Members') end end diff --git a/spec/features/groups/merge_requests_spec.rb b/spec/features/groups/merge_requests_spec.rb index 43d4b6b23e0..f79c93157dc 100644 --- a/spec/features/groups/merge_requests_spec.rb +++ b/spec/features/groups/merge_requests_spec.rb @@ -27,7 +27,7 @@ RSpec.describe 'Group merge requests page' do end it 'ignores archived merge request count badges in navbar' do - expect(first(:link, text: 'Merge Requests').find('.badge').text).to eq("1") + expect(first(:link, text: 'Merge requests').find('.badge').text).to eq("1") end it 'ignores archived merge request count badges in state-filters' do diff --git a/spec/features/groups/navbar_spec.rb b/spec/features/groups/navbar_spec.rb index 7025874a4ff..710755ce306 100644 --- a/spec/features/groups/navbar_spec.rb +++ b/spec/features/groups/navbar_spec.rb @@ -30,7 +30,7 @@ RSpec.describe 'Group navbar' do ] }, { - nav_item: _('Merge Requests'), + nav_item: _('Merge requests'), nav_sub_items: [] }, (security_and_compliance_nav_item if Gitlab.ee?), diff --git a/spec/features/projects/active_tabs_spec.rb b/spec/features/projects/active_tabs_spec.rb index 86fe59f003f..9de43e7d18c 100644 --- a/spec/features/projects/active_tabs_spec.rb +++ b/spec/features/projects/active_tabs_spec.rb @@ -79,7 +79,7 @@ RSpec.describe 'Project active tab' do visit project_merge_requests_path(project) end - it_behaves_like 'page has active tab', 'Merge Requests' + it_behaves_like 'page has active tab', 'Merge requests' end context 'on project Wiki' do diff --git a/spec/features/projects/fork_spec.rb b/spec/features/projects/fork_spec.rb index 81751d2123a..f82dddbe38d 100644 --- a/spec/features/projects/fork_spec.rb +++ b/spec/features/projects/fork_spec.rb @@ -208,7 +208,7 @@ RSpec.describe 'Project fork' do expect(page).to have_content(/new merge request/i) page.within '.nav-sidebar' do - first(:link, 'Merge Requests').click + first(:link, 'Merge requests').click end expect(page).to have_content(/new merge request/i) diff --git a/spec/features/projects/merge_request_button_spec.rb b/spec/features/projects/merge_request_button_spec.rb index 9547ba8a390..93bbabcc3f8 100644 --- a/spec/features/projects/merge_request_button_spec.rb +++ b/spec/features/projects/merge_request_button_spec.rb @@ -14,7 +14,9 @@ RSpec.describe 'Merge Request button' do it 'does not show Create merge request button' do visit url - expect(page).not_to have_link(label) + within '.content-wrapper' do + expect(page).not_to have_link(label) + end end end diff --git a/spec/features/projects/user_uses_shortcuts_spec.rb b/spec/features/projects/user_uses_shortcuts_spec.rb index f97c8d820e3..b6fde19e0d4 100644 --- a/spec/features/projects/user_uses_shortcuts_spec.rb +++ b/spec/features/projects/user_uses_shortcuts_spec.rb @@ -151,7 +151,7 @@ RSpec.describe 'User uses shortcuts', :js do find('body').native.send_key('g') find('body').native.send_key('m') - expect(page).to have_active_navigation('Merge Requests') + expect(page).to have_active_navigation('Merge requests') end end diff --git a/spec/frontend/diffs/components/commit_item_spec.js b/spec/frontend/diffs/components/commit_item_spec.js index 8cb4fd20063..0191822d97a 100644 --- a/spec/frontend/diffs/components/commit_item_spec.js +++ b/spec/frontend/diffs/components/commit_item_spec.js @@ -13,8 +13,6 @@ const TEST_AUTHOR_EMAIL = 'test+test@gitlab.com'; const TEST_AUTHOR_GRAVATAR = `${TEST_HOST}/avatar/test?s=40`; const TEST_SIGNATURE_HTML = 'Legit commit'; const TEST_PIPELINE_STATUS_PATH = `${TEST_HOST}/pipeline/status`; -const NEXT_COMMIT_URL = `${TEST_HOST}/?commit_id=next`; -const PREV_COMMIT_URL = `${TEST_HOST}/?commit_id=prev`; describe('diffs/components/commit_item', () => { let wrapper; @@ -31,12 +29,6 @@ describe('diffs/components/commit_item', () => { const getCommitActionsElement = () => wrapper.find('.commit-actions'); const getCommitPipelineStatus = () => wrapper.find(CommitPipelineStatus); - const getCommitNavButtonsElement = () => wrapper.find('.commit-nav-buttons'); - const getNextCommitNavElement = () => - getCommitNavButtonsElement().find('.btn-group > *:last-child'); - const getPrevCommitNavElement = () => - getCommitNavButtonsElement().find('.btn-group > *:first-child'); - const mountComponent = (propsData) => { wrapper = mount(Component, { propsData: { @@ -180,126 +172,4 @@ describe('diffs/components/commit_item', () => { expect(getCommitPipelineStatus().exists()).toBe(true); }); }); - - describe('without neighbor commits', () => { - beforeEach(() => { - mountComponent({ commit: { ...commit, prev_commit_id: null, next_commit_id: null } }); - }); - - it('does not render any navigation buttons', () => { - expect(getCommitNavButtonsElement().exists()).toEqual(false); - }); - }); - - describe('with neighbor commits', () => { - let mrCommit; - - beforeEach(() => { - mrCommit = { - ...commit, - next_commit_id: 'next', - prev_commit_id: 'prev', - }; - - mountComponent({ commit: mrCommit }); - }); - - it('renders the commit navigation buttons', () => { - expect(getCommitNavButtonsElement().exists()).toEqual(true); - - mountComponent({ - commit: { ...mrCommit, next_commit_id: null }, - }); - expect(getCommitNavButtonsElement().exists()).toEqual(true); - - mountComponent({ - commit: { ...mrCommit, prev_commit_id: null }, - }); - expect(getCommitNavButtonsElement().exists()).toEqual(true); - }); - - describe('prev commit', () => { - const { location } = window; - - beforeAll(() => { - delete window.location; - window.location = { href: `${TEST_HOST}?commit_id=${mrCommit.id}` }; - }); - - beforeEach(() => { - jest.spyOn(wrapper.vm, 'moveToNeighboringCommit').mockImplementation(() => {}); - }); - - afterAll(() => { - window.location = location; - }); - - it('uses the correct href', () => { - const link = getPrevCommitNavElement(); - - expect(link.element.getAttribute('href')).toEqual(PREV_COMMIT_URL); - }); - - it('triggers the correct Vuex action on click', () => { - const link = getPrevCommitNavElement(); - - link.trigger('click'); - return wrapper.vm.$nextTick().then(() => { - expect(wrapper.vm.moveToNeighboringCommit).toHaveBeenCalledWith({ - direction: 'previous', - }); - }); - }); - - it('renders a disabled button when there is no prev commit', () => { - mountComponent({ commit: { ...mrCommit, prev_commit_id: null } }); - - const button = getPrevCommitNavElement(); - - expect(button.element.tagName).toEqual('BUTTON'); - expect(button.element.hasAttribute('disabled')).toEqual(true); - }); - }); - - describe('next commit', () => { - const { location } = window; - - beforeAll(() => { - delete window.location; - window.location = { href: `${TEST_HOST}?commit_id=${mrCommit.id}` }; - }); - - beforeEach(() => { - jest.spyOn(wrapper.vm, 'moveToNeighboringCommit').mockImplementation(() => {}); - }); - - afterAll(() => { - window.location = location; - }); - - it('uses the correct href', () => { - const link = getNextCommitNavElement(); - - expect(link.element.getAttribute('href')).toEqual(NEXT_COMMIT_URL); - }); - - it('triggers the correct Vuex action on click', () => { - const link = getNextCommitNavElement(); - - link.trigger('click'); - return wrapper.vm.$nextTick().then(() => { - expect(wrapper.vm.moveToNeighboringCommit).toHaveBeenCalledWith({ direction: 'next' }); - }); - }); - - it('renders a disabled button when there is no next commit', () => { - mountComponent({ commit: { ...mrCommit, next_commit_id: null } }); - - const button = getNextCommitNavElement(); - - expect(button.element.tagName).toEqual('BUTTON'); - expect(button.element.hasAttribute('disabled')).toEqual(true); - }); - }); - }); }); diff --git a/spec/frontend/diffs/components/compare_versions_spec.js b/spec/frontend/diffs/components/compare_versions_spec.js index c93a3771ec0..4feddb939c6 100644 --- a/spec/frontend/diffs/components/compare_versions_spec.js +++ b/spec/frontend/diffs/components/compare_versions_spec.js @@ -1,5 +1,6 @@ import { mount, createLocalVue } from '@vue/test-utils'; import Vuex from 'vuex'; +import { TEST_HOST } from 'helpers/test_constants'; import { trimText } from 'helpers/text_helper'; import CompareVersionsComponent from '~/diffs/components/compare_versions.vue'; import { createStore } from '~/mr_notes/stores'; @@ -9,12 +10,17 @@ import diffsMockData from '../mock_data/merge_request_diffs'; const localVue = createLocalVue(); localVue.use(Vuex); +const NEXT_COMMIT_URL = `${TEST_HOST}/?commit_id=next`; +const PREV_COMMIT_URL = `${TEST_HOST}/?commit_id=prev`; + describe('CompareVersions', () => { let wrapper; let store; const targetBranchName = 'tmp-wine-dev'; + const { commit } = getDiffWithCommit(); - const createWrapper = (props) => { + const createWrapper = (props = {}, commitArgs = {}) => { + store.state.diffs.commit = { ...store.state.diffs.commit, ...commitArgs }; wrapper = mount(CompareVersionsComponent, { localVue, store, @@ -28,6 +34,11 @@ describe('CompareVersions', () => { const findLimitedContainer = () => wrapper.find('.container-limited.limit-container-width'); const findCompareSourceDropdown = () => wrapper.find('.mr-version-dropdown'); const findCompareTargetDropdown = () => wrapper.find('.mr-version-compare-dropdown'); + const getCommitNavButtonsElement = () => wrapper.find('.commit-nav-buttons'); + const getNextCommitNavElement = () => + getCommitNavButtonsElement().find('.btn-group > *:last-child'); + const getPrevCommitNavElement = () => + getCommitNavButtonsElement().find('.btn-group > *:first-child'); beforeEach(() => { store = createStore(); @@ -161,4 +172,124 @@ describe('CompareVersions', () => { expect(findCompareTargetDropdown().exists()).toBe(false); }); }); + + describe('without neighbor commits', () => { + beforeEach(() => { + createWrapper({ commit: { ...commit, prev_commit_id: null, next_commit_id: null } }); + }); + + it('does not render any navigation buttons', () => { + expect(getCommitNavButtonsElement().exists()).toEqual(false); + }); + }); + + describe('with neighbor commits', () => { + let mrCommit; + + beforeEach(() => { + mrCommit = { + ...commit, + next_commit_id: 'next', + prev_commit_id: 'prev', + }; + + createWrapper({}, mrCommit); + }); + + it('renders the commit navigation buttons', () => { + expect(getCommitNavButtonsElement().exists()).toEqual(true); + + createWrapper({ + commit: { ...mrCommit, next_commit_id: null }, + }); + expect(getCommitNavButtonsElement().exists()).toEqual(true); + + createWrapper({ + commit: { ...mrCommit, prev_commit_id: null }, + }); + expect(getCommitNavButtonsElement().exists()).toEqual(true); + }); + + describe('prev commit', () => { + const { location } = window; + + beforeAll(() => { + delete window.location; + window.location = { href: `${TEST_HOST}?commit_id=${mrCommit.id}` }; + }); + + beforeEach(() => { + jest.spyOn(wrapper.vm, 'moveToNeighboringCommit').mockImplementation(() => {}); + }); + + afterAll(() => { + window.location = location; + }); + + it('uses the correct href', () => { + const link = getPrevCommitNavElement(); + + expect(link.element.getAttribute('href')).toEqual(PREV_COMMIT_URL); + }); + + it('triggers the correct Vuex action on click', () => { + const link = getPrevCommitNavElement(); + + link.trigger('click'); + return wrapper.vm.$nextTick().then(() => { + expect(wrapper.vm.moveToNeighboringCommit).toHaveBeenCalledWith({ + direction: 'previous', + }); + }); + }); + + it('renders a disabled button when there is no prev commit', () => { + createWrapper({}, { ...mrCommit, prev_commit_id: null }); + + const button = getPrevCommitNavElement(); + + expect(button.element.hasAttribute('disabled')).toEqual(true); + }); + }); + + describe('next commit', () => { + const { location } = window; + + beforeAll(() => { + delete window.location; + window.location = { href: `${TEST_HOST}?commit_id=${mrCommit.id}` }; + }); + + beforeEach(() => { + jest.spyOn(wrapper.vm, 'moveToNeighboringCommit').mockImplementation(() => {}); + }); + + afterAll(() => { + window.location = location; + }); + + it('uses the correct href', () => { + const link = getNextCommitNavElement(); + + expect(link.element.getAttribute('href')).toEqual(NEXT_COMMIT_URL); + }); + + it('triggers the correct Vuex action on click', () => { + const link = getNextCommitNavElement(); + + link.trigger('click'); + return wrapper.vm.$nextTick().then(() => { + expect(wrapper.vm.moveToNeighboringCommit).toHaveBeenCalledWith({ direction: 'next' }); + }); + }); + + it('renders a disabled button when there is no next commit', () => { + createWrapper({}, { ...mrCommit, next_commit_id: null }); + + const button = getNextCommitNavElement(); + + expect(button.element.hasAttribute('disabled')).toEqual(true); + }); + }); + }); }); diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 1f0eed96503..1850363bb72 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -810,6 +810,31 @@ RSpec.describe API::Projects do end end end + + context 'with forked projects', :use_clean_rails_memory_store_caching do + include ProjectForksHelper + + let_it_be(:admin) { create(:admin) } + + it 'avoids N+1 queries' do + get api('/projects', admin) + + base_project = create(:project, :public, namespace: admin.namespace) + + fork_project1 = fork_project(base_project, admin, namespace: create(:user).namespace) + fork_project2 = fork_project(fork_project1, admin, namespace: create(:user).namespace) + + control = ActiveRecord::QueryRecorder.new do + get api('/projects', admin) + end + + fork_project(fork_project2, admin, namespace: create(:user).namespace) + + expect do + get api('/projects', admin) + end.not_to exceed_query_limit(control.count) + end + end end describe 'POST /projects' do diff --git a/spec/services/merge_requests/after_create_service_spec.rb b/spec/services/merge_requests/after_create_service_spec.rb index dce351d8a31..4830221649f 100644 --- a/spec/services/merge_requests/after_create_service_spec.rb +++ b/spec/services/merge_requests/after_create_service_spec.rb @@ -93,5 +93,93 @@ RSpec.describe MergeRequests::AfterCreateService do expect(merge_request.reload).to be_unchecked end end + + it 'increments the usage data counter of create event' do + counter = Gitlab::UsageDataCounters::MergeRequestCounter + + expect { execute_service }.to change { counter.read(:create) }.by(1) + end + + context 'todos' do + it 'does not creates todos' do + attributes = { + project: merge_request.target_project, + target_id: merge_request.id, + target_type: merge_request.class.name + } + + expect { execute_service }.not_to change { Todo.where(attributes).count } + end + + context 'when merge request is assigned to someone' do + let_it_be(:assignee) { create(:user) } + let_it_be(:merge_request) { create(:merge_request, assignees: [assignee]) } + + it 'creates a todo for new assignee' do + attributes = { + project: merge_request.target_project, + author: merge_request.author, + user: assignee, + target_id: merge_request.id, + target_type: merge_request.class.name, + action: Todo::ASSIGNED, + state: :pending + } + + expect { execute_service }.to change { Todo.where(attributes).count }.by(1) + end + end + + context 'when reviewer is assigned' do + let_it_be(:reviewer) { create(:user) } + let_it_be(:merge_request) { create(:merge_request, reviewers: [reviewer]) } + + it 'creates a todo for new reviewer' do + attributes = { + project: merge_request.target_project, + author: merge_request.author, + user: reviewer, + target_id: merge_request.id, + target_type: merge_request.class.name, + action: Todo::REVIEW_REQUESTED, + state: :pending + } + + expect { execute_service }.to change { Todo.where(attributes).count }.by(1) + end + end + end + + context 'when saving references to issues that the created merge request closes' do + let_it_be(:first_issue) { create(:issue, project: merge_request.target_project) } + let_it_be(:second_issue) { create(:issue, project: merge_request.target_project) } + + it 'creates a `MergeRequestsClosingIssues` record for each issue' do + merge_request.description = "Closes #{first_issue.to_reference} and #{second_issue.to_reference}" + merge_request.source_branch = "feature" + merge_request.target_branch = merge_request.target_project.default_branch + merge_request.save! + + execute_service + + issue_ids = MergeRequestsClosingIssues.where(merge_request: merge_request).pluck(:issue_id) + expect(issue_ids).to match_array([first_issue.id, second_issue.id]) + end + end + + it 'tracks merge request creation in usage data' do + expect(Gitlab::UsageDataCounters::MergeRequestCounter).to receive(:count).with(:create) + + execute_service + end + + it 'calls MergeRequests::LinkLfsObjectsService#execute' do + service = instance_spy(MergeRequests::LinkLfsObjectsService) + allow(MergeRequests::LinkLfsObjectsService).to receive(:new).with(merge_request.target_project).and_return(service) + + execute_service + + expect(service).to have_received(:execute).with(merge_request) + end end end diff --git a/spec/services/merge_requests/create_service_spec.rb b/spec/services/merge_requests/create_service_spec.rb index 4f47a22b07c..4646ed866a0 100644 --- a/spec/services/merge_requests/create_service_spec.rb +++ b/spec/services/merge_requests/create_service_spec.rb @@ -47,16 +47,6 @@ RSpec.describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do .to change { project.open_merge_requests_count }.from(0).to(1) end - it 'does not creates todos' do - attributes = { - project: project, - target_id: merge_request.id, - target_type: merge_request.class.name - } - - expect(Todo.where(attributes).count).to be_zero - end - it 'creates exactly 1 create MR event', :sidekiq_might_not_need_inline do attributes = { action: :created, @@ -113,20 +103,6 @@ RSpec.describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do end it { expect(merge_request.assignees).to eq([user2]) } - - it 'creates a todo for new assignee' do - attributes = { - project: project, - author: user, - user: user2, - target_id: merge_request.id, - target_type: merge_request.class.name, - action: Todo::ASSIGNED, - state: :pending - } - - expect(Todo.where(attributes).count).to eq 1 - end end context 'when reviewer is assigned' do @@ -142,20 +118,6 @@ RSpec.describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do it { expect(merge_request.reviewers).to eq([user2]) } - it 'creates a todo for new reviewer' do - attributes = { - project: project, - author: user, - user: user2, - target_id: merge_request.id, - target_type: merge_request.class.name, - action: Todo::REVIEW_REQUESTED, - state: :pending - } - - expect(Todo.where(attributes).count).to eq 1 - end - it 'invalidates counter cache for reviewers', :use_clean_rails_memory_store_caching do expect { merge_request } .to change { user2.review_requested_open_merge_requests_count } @@ -328,12 +290,6 @@ RSpec.describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do end end - it 'increments the usage data counter of create event' do - counter = Gitlab::UsageDataCounters::MergeRequestCounter - - expect { service.execute }.to change { counter.read(:create) }.by(1) - end - context 'after_save callback to store_mentions' do let(:labels) { create_pair(:label, project: project) } let(:milestone) { create(:milestone, project: project) } @@ -494,35 +450,6 @@ RSpec.describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do end end - context 'while saving references to issues that the created merge request closes' do - let(:first_issue) { create(:issue, project: project) } - let(:second_issue) { create(:issue, project: project) } - - let(:opts) do - { - title: 'Awesome merge_request', - source_branch: 'feature', - target_branch: 'master', - force_remove_source_branch: '1' - } - end - - before do - project.add_maintainer(user) - project.add_developer(user2) - end - - it 'creates a `MergeRequestsClosingIssues` record for each issue' do - issue_closing_opts = opts.merge(description: "Closes #{first_issue.to_reference} and #{second_issue.to_reference}") - service = described_class.new(project, user, issue_closing_opts) - allow(service).to receive(:execute_hooks) - merge_request = service.execute - - issue_ids = MergeRequestsClosingIssues.where(merge_request: merge_request).pluck(:issue_id) - expect(issue_ids).to match_array([first_issue.id, second_issue.id]) - end - end - context 'when source and target projects are different' do let(:target_project) { fork_project(project, nil, repository: true) } @@ -571,14 +498,6 @@ RSpec.describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do expect(merge_request).to be_persisted end - it 'calls MergeRequests::LinkLfsObjectsService#execute', :sidekiq_might_not_need_inline do - expect_next_instance_of(MergeRequests::LinkLfsObjectsService) do |service| - expect(service).to receive(:execute).with(instance_of(MergeRequest)) - end - - described_class.new(project, user, opts).execute - end - it 'does not create the merge request when the target project is archived' do target_project.update!(archived: true) diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index 7a7f684c6d0..d27a35420aa 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -948,18 +948,8 @@ RSpec.describe MergeRequests::UpdateService, :mailer do end it 'removes `MergeRequestsClosingIssues` records when issues are not closed anymore' do - opts = { - title: 'Awesome merge_request', - description: "Closes #{first_issue.to_reference} and #{second_issue.to_reference}", - source_branch: 'feature', - target_branch: 'master', - force_remove_source_branch: '1' - } - - merge_request = MergeRequests::CreateService.new(project, user, opts).execute - - issue_ids = MergeRequestsClosingIssues.where(merge_request: merge_request).pluck(:issue_id) - expect(issue_ids).to match_array([first_issue.id, second_issue.id]) + create(:merge_requests_closing_issues, issue: first_issue, merge_request: merge_request) + create(:merge_requests_closing_issues, issue: second_issue, merge_request: merge_request) service = described_class.new(project, user, description: "not closing any issues") allow(service).to receive(:execute_hooks) diff --git a/spec/support/shared_contexts/navbar_structure_context.rb b/spec/support/shared_contexts/navbar_structure_context.rb index 460c7febc30..78d14ecb880 100644 --- a/spec/support/shared_contexts/navbar_structure_context.rb +++ b/spec/support/shared_contexts/navbar_structure_context.rb @@ -59,7 +59,7 @@ RSpec.shared_context 'project navbar structure' do ] }, { - nav_item: _('Merge Requests'), + nav_item: _('Merge requests'), nav_sub_items: [] }, { @@ -190,7 +190,7 @@ RSpec.shared_context 'group navbar structure' do ] }, { - nav_item: _('Merge Requests'), + nav_item: _('Merge requests'), nav_sub_items: [] }, security_and_compliance_nav_item, -- cgit v1.2.1