diff options
author | Paul Slaughter <pslaughter@gitlab.com> | 2019-04-04 04:46:12 -0500 |
---|---|---|
committer | Paul Slaughter <pslaughter@gitlab.com> | 2019-04-04 12:41:28 -0500 |
commit | 27e06caad40995baada42704f2aabdf8bee05009 (patch) | |
tree | f42ee0dad19a7c0ccee2b22a3df6b640437dfb6f | |
parent | 5e39a7cfa4e82129a60a55f9a62ccc87324cbb35 (diff) | |
download | gitlab-ce-ide-fix-detect-mr-from-fork.tar.gz |
Fix IDE detecting MR from fork branchide-fix-detect-mr-from-fork
**Why?**
Currently the IDE loads a merge request based on only the
`source_branch` name. This means it loads MR's from
forks that have the same branch name (not good).
- This required updating the BE API to accept `source_project_id`
-rw-r--r-- | app/assets/javascripts/ide/stores/actions/merge_request.js | 3 | ||||
-rw-r--r-- | app/finders/merge_requests_finder.rb | 13 | ||||
-rw-r--r-- | changelogs/unreleased/ide-fix-detect-mr-from-fork.yml | 5 | ||||
-rw-r--r-- | lib/api/merge_requests.rb | 1 | ||||
-rw-r--r-- | spec/finders/merge_requests_finder_spec.rb | 8 | ||||
-rw-r--r-- | spec/javascripts/ide/stores/actions/merge_request_spec.js | 9 |
6 files changed, 34 insertions, 5 deletions
diff --git a/app/assets/javascripts/ide/stores/actions/merge_request.js b/app/assets/javascripts/ide/stores/actions/merge_request.js index 362ced248a1..1273e375859 100644 --- a/app/assets/javascripts/ide/stores/actions/merge_request.js +++ b/app/assets/javascripts/ide/stores/actions/merge_request.js @@ -4,10 +4,11 @@ import service from '../../services'; import * as types from '../mutation_types'; import { activityBarViews } from '../../constants'; -export const getMergeRequestsForBranch = ({ commit }, { projectId, branchId } = {}) => +export const getMergeRequestsForBranch = ({ commit, state }, { projectId, branchId } = {}) => service .getProjectMergeRequests(`${projectId}`, { source_branch: branchId, + source_project_id: state.projects[projectId].id, order_by: 'created_at', per_page: 1, }) diff --git a/app/finders/merge_requests_finder.rb b/app/finders/merge_requests_finder.rb index 84689ff5dc7..29947bc94d5 100644 --- a/app/finders/merge_requests_finder.rb +++ b/app/finders/merge_requests_finder.rb @@ -40,7 +40,8 @@ class MergeRequestsFinder < IssuableFinder items = by_commit(super) items = by_source_branch(items) items = by_wip(items) - by_target_branch(items) + items = by_target_branch(items) + by_source_project_id(items) end private @@ -74,6 +75,16 @@ class MergeRequestsFinder < IssuableFinder items.where(target_branch: target_branch) end + def source_project_id + @source_project_id ||= params[:source_project_id].presence + end + + def by_source_project_id(items) + return items unless source_project_id + + items.where(source_project_id: source_project_id) + end + def by_wip(items) if params[:wip] == 'yes' items.where(wip_match(items.arel_table)) diff --git a/changelogs/unreleased/ide-fix-detect-mr-from-fork.yml b/changelogs/unreleased/ide-fix-detect-mr-from-fork.yml new file mode 100644 index 00000000000..8f4f49896d7 --- /dev/null +++ b/changelogs/unreleased/ide-fix-detect-mr-from-fork.yml @@ -0,0 +1,5 @@ +--- +title: Fix IDE detection of MR from fork with same branch name +merge_request: 26986 +author: +type: fixed diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index 98dcc388f44..e4b21b7d1c4 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -111,6 +111,7 @@ module API desc: 'Return merge requests for the given scope: `created_by_me`, `assigned_to_me` or `all`' optional :my_reaction_emoji, type: String, desc: 'Return issues reacted by the authenticated user by the given emoji' optional :source_branch, type: String, desc: 'Return merge requests with the given source branch' + optional :source_project_id, type: Integer, desc: 'Return merge requests with the given source project id' optional :target_branch, type: String, desc: 'Return merge requests with the given target branch' optional :search, type: String, desc: 'Search merge requests for text present in the title, description, or any combination of these' optional :in, type: String, desc: '`title`, `description`, or a string joining them with comma' diff --git a/spec/finders/merge_requests_finder_spec.rb b/spec/finders/merge_requests_finder_spec.rb index 56136eb84bc..f508b9bdb6f 100644 --- a/spec/finders/merge_requests_finder_spec.rb +++ b/spec/finders/merge_requests_finder_spec.rb @@ -83,6 +83,14 @@ describe MergeRequestsFinder do expect(merge_requests).to contain_exactly(merge_request2) end + it 'filters by source project id' do + params = { source_project_id: merge_request2.source_project_id } + + merge_requests = described_class.new(user, params).execute + + expect(merge_requests).to contain_exactly(merge_request1, merge_request2, merge_request3) + end + it 'filters by state' do params = { state: 'locked' } diff --git a/spec/javascripts/ide/stores/actions/merge_request_spec.js b/spec/javascripts/ide/stores/actions/merge_request_spec.js index c2ad476d0aa..4dd0c1150eb 100644 --- a/spec/javascripts/ide/stores/actions/merge_request_spec.js +++ b/spec/javascripts/ide/stores/actions/merge_request_spec.js @@ -2,7 +2,6 @@ import MockAdapter from 'axios-mock-adapter'; import axios from '~/lib/utils/axios_utils'; import store from '~/ide/stores'; import actions, { - getMergeRequestsForBranch, getMergeRequestData, getMergeRequestChanges, getMergeRequestVersions, @@ -13,6 +12,7 @@ import { activityBarViews } from '~/ide/constants'; import { resetStore } from '../../helpers'; const TEST_PROJECT = 'abcproject'; +const TEST_PROJECT_ID = 17; describe('IDE store merge request actions', () => { let mock; @@ -20,7 +20,8 @@ describe('IDE store merge request actions', () => { beforeEach(() => { mock = new MockAdapter(axios); - store.state.projects.abcproject = { + store.state.projects[TEST_PROJECT] = { + id: TEST_PROJECT_ID, mergeRequests: {}, }; }); @@ -47,6 +48,7 @@ describe('IDE store merge request actions', () => { .then(() => { expect(service.getProjectMergeRequests).toHaveBeenCalledWith(TEST_PROJECT, { source_branch: 'bar', + source_project_id: TEST_PROJECT_ID, order_by: 'created_at', per_page: 1, }); @@ -106,7 +108,8 @@ describe('IDE store merge request actions', () => { it('flashes message, if error', done => { const flashSpy = spyOnDependency(actions, 'flash'); - getMergeRequestsForBranch({ commit() {} }, { projectId: TEST_PROJECT, branchId: 'bar' }) + store + .dispatch('getMergeRequestsForBranch', { projectId: TEST_PROJECT, branchId: 'bar' }) .then(() => { fail('Expected getMergeRequestsForBranch to throw an error'); }) |