summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPaul Slaughter <pslaughter@gitlab.com>2019-04-04 04:46:12 -0500
committerPaul Slaughter <pslaughter@gitlab.com>2019-04-04 12:41:28 -0500
commit27e06caad40995baada42704f2aabdf8bee05009 (patch)
treef42ee0dad19a7c0ccee2b22a3df6b640437dfb6f
parent5e39a7cfa4e82129a60a55f9a62ccc87324cbb35 (diff)
downloadgitlab-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.js3
-rw-r--r--app/finders/merge_requests_finder.rb13
-rw-r--r--changelogs/unreleased/ide-fix-detect-mr-from-fork.yml5
-rw-r--r--lib/api/merge_requests.rb1
-rw-r--r--spec/finders/merge_requests_finder_spec.rb8
-rw-r--r--spec/javascripts/ide/stores/actions/merge_request_spec.js9
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');
})