diff options
-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 | 75 |
6 files changed, 68 insertions, 37 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 a5839630657..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, @@ -12,13 +11,17 @@ import service from '~/ide/services'; 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; beforeEach(() => { mock = new MockAdapter(axios); - store.state.projects.abcproject = { + store.state.projects[TEST_PROJECT] = { + id: TEST_PROJECT_ID, mergeRequests: {}, }; }); @@ -41,10 +44,11 @@ describe('IDE store merge request actions', () => { it('calls getProjectMergeRequests service method', done => { store - .dispatch('getMergeRequestsForBranch', { projectId: 'abcproject', branchId: 'bar' }) + .dispatch('getMergeRequestsForBranch', { projectId: TEST_PROJECT, branchId: 'bar' }) .then(() => { - expect(service.getProjectMergeRequests).toHaveBeenCalledWith('abcproject', { + expect(service.getProjectMergeRequests).toHaveBeenCalledWith(TEST_PROJECT, { source_branch: 'bar', + source_project_id: TEST_PROJECT_ID, order_by: 'created_at', per_page: 1, }); @@ -56,13 +60,11 @@ describe('IDE store merge request actions', () => { it('sets the "Merge Request" Object', done => { store - .dispatch('getMergeRequestsForBranch', { projectId: 'abcproject', branchId: 'bar' }) + .dispatch('getMergeRequestsForBranch', { projectId: TEST_PROJECT, branchId: 'bar' }) .then(() => { - expect(Object.keys(store.state.projects.abcproject.mergeRequests).length).toEqual(1); - expect(Object.keys(store.state.projects.abcproject.mergeRequests)[0]).toEqual('2'); - expect(store.state.projects.abcproject.mergeRequests[2]).toEqual( - jasmine.objectContaining(mrData), - ); + expect(store.state.projects.abcproject.mergeRequests).toEqual({ + '2': jasmine.objectContaining(mrData), + }); done(); }) .catch(done.fail); @@ -70,7 +72,7 @@ describe('IDE store merge request actions', () => { it('sets "Current Merge Request" object to the most recent MR', done => { store - .dispatch('getMergeRequestsForBranch', { projectId: 'abcproject', branchId: 'bar' }) + .dispatch('getMergeRequestsForBranch', { projectId: TEST_PROJECT, branchId: 'bar' }) .then(() => { expect(store.state.currentMergeRequestId).toEqual('2'); done(); @@ -87,9 +89,9 @@ describe('IDE store merge request actions', () => { it('does not fail if there are no merge requests for current branch', done => { store - .dispatch('getMergeRequestsForBranch', { projectId: 'abcproject', branchId: 'foo' }) + .dispatch('getMergeRequestsForBranch', { projectId: TEST_PROJECT, branchId: 'foo' }) .then(() => { - expect(Object.keys(store.state.projects.abcproject.mergeRequests).length).toEqual(0); + expect(store.state.projects[TEST_PROJECT].mergeRequests).toEqual({}); expect(store.state.currentMergeRequestId).toEqual(''); done(); }) @@ -106,7 +108,8 @@ describe('IDE store merge request actions', () => { it('flashes message, if error', done => { const flashSpy = spyOnDependency(actions, 'flash'); - getMergeRequestsForBranch({ commit() {} }, { projectId: 'abcproject', branchId: 'bar' }) + store + .dispatch('getMergeRequestsForBranch', { projectId: TEST_PROJECT, branchId: 'bar' }) .then(() => { fail('Expected getMergeRequestsForBranch to throw an error'); }) @@ -132,9 +135,9 @@ describe('IDE store merge request actions', () => { it('calls getProjectMergeRequestData service method', done => { store - .dispatch('getMergeRequestData', { projectId: 'abcproject', mergeRequestId: 1 }) + .dispatch('getMergeRequestData', { projectId: TEST_PROJECT, mergeRequestId: 1 }) .then(() => { - expect(service.getProjectMergeRequestData).toHaveBeenCalledWith('abcproject', 1, { + expect(service.getProjectMergeRequestData).toHaveBeenCalledWith(TEST_PROJECT, 1, { render_html: true, }); @@ -145,10 +148,12 @@ describe('IDE store merge request actions', () => { it('sets the Merge Request Object', done => { store - .dispatch('getMergeRequestData', { projectId: 'abcproject', mergeRequestId: 1 }) + .dispatch('getMergeRequestData', { projectId: TEST_PROJECT, mergeRequestId: 1 }) .then(() => { - expect(store.state.projects.abcproject.mergeRequests['1'].title).toBe('mergerequest'); expect(store.state.currentMergeRequestId).toBe(1); + expect(store.state.projects[TEST_PROJECT].mergeRequests['1'].title).toBe( + 'mergerequest', + ); done(); }) @@ -170,7 +175,7 @@ describe('IDE store merge request actions', () => { dispatch, state: store.state, }, - { projectId: 'abcproject', mergeRequestId: 1 }, + { projectId: TEST_PROJECT, mergeRequestId: 1 }, ) .then(done.fail) .catch(() => { @@ -179,7 +184,7 @@ describe('IDE store merge request actions', () => { action: jasmine.any(Function), actionText: 'Please try again', actionPayload: { - projectId: 'abcproject', + projectId: TEST_PROJECT, mergeRequestId: 1, force: false, }, @@ -193,7 +198,7 @@ describe('IDE store merge request actions', () => { describe('getMergeRequestChanges', () => { beforeEach(() => { - store.state.projects.abcproject.mergeRequests['1'] = { changes: [] }; + store.state.projects[TEST_PROJECT].mergeRequests['1'] = { changes: [] }; }); describe('success', () => { @@ -207,9 +212,9 @@ describe('IDE store merge request actions', () => { it('calls getProjectMergeRequestChanges service method', done => { store - .dispatch('getMergeRequestChanges', { projectId: 'abcproject', mergeRequestId: 1 }) + .dispatch('getMergeRequestChanges', { projectId: TEST_PROJECT, mergeRequestId: 1 }) .then(() => { - expect(service.getProjectMergeRequestChanges).toHaveBeenCalledWith('abcproject', 1); + expect(service.getProjectMergeRequestChanges).toHaveBeenCalledWith(TEST_PROJECT, 1); done(); }) @@ -218,9 +223,9 @@ describe('IDE store merge request actions', () => { it('sets the Merge Request Changes Object', done => { store - .dispatch('getMergeRequestChanges', { projectId: 'abcproject', mergeRequestId: 1 }) + .dispatch('getMergeRequestChanges', { projectId: TEST_PROJECT, mergeRequestId: 1 }) .then(() => { - expect(store.state.projects.abcproject.mergeRequests['1'].changes.title).toBe( + expect(store.state.projects[TEST_PROJECT].mergeRequests['1'].changes.title).toBe( 'mergerequest', ); done(); @@ -243,7 +248,7 @@ describe('IDE store merge request actions', () => { dispatch, state: store.state, }, - { projectId: 'abcproject', mergeRequestId: 1 }, + { projectId: TEST_PROJECT, mergeRequestId: 1 }, ) .then(done.fail) .catch(() => { @@ -252,7 +257,7 @@ describe('IDE store merge request actions', () => { action: jasmine.any(Function), actionText: 'Please try again', actionPayload: { - projectId: 'abcproject', + projectId: TEST_PROJECT, mergeRequestId: 1, force: false, }, @@ -266,7 +271,7 @@ describe('IDE store merge request actions', () => { describe('getMergeRequestVersions', () => { beforeEach(() => { - store.state.projects.abcproject.mergeRequests['1'] = { versions: [] }; + store.state.projects[TEST_PROJECT].mergeRequests['1'] = { versions: [] }; }); describe('success', () => { @@ -279,9 +284,9 @@ describe('IDE store merge request actions', () => { it('calls getProjectMergeRequestVersions service method', done => { store - .dispatch('getMergeRequestVersions', { projectId: 'abcproject', mergeRequestId: 1 }) + .dispatch('getMergeRequestVersions', { projectId: TEST_PROJECT, mergeRequestId: 1 }) .then(() => { - expect(service.getProjectMergeRequestVersions).toHaveBeenCalledWith('abcproject', 1); + expect(service.getProjectMergeRequestVersions).toHaveBeenCalledWith(TEST_PROJECT, 1); done(); }) @@ -290,9 +295,9 @@ describe('IDE store merge request actions', () => { it('sets the Merge Request Versions Object', done => { store - .dispatch('getMergeRequestVersions', { projectId: 'abcproject', mergeRequestId: 1 }) + .dispatch('getMergeRequestVersions', { projectId: TEST_PROJECT, mergeRequestId: 1 }) .then(() => { - expect(store.state.projects.abcproject.mergeRequests['1'].versions.length).toBe(1); + expect(store.state.projects[TEST_PROJECT].mergeRequests['1'].versions.length).toBe(1); done(); }) .catch(done.fail); @@ -313,7 +318,7 @@ describe('IDE store merge request actions', () => { dispatch, state: store.state, }, - { projectId: 'abcproject', mergeRequestId: 1 }, + { projectId: TEST_PROJECT, mergeRequestId: 1 }, ) .then(done.fail) .catch(() => { @@ -322,7 +327,7 @@ describe('IDE store merge request actions', () => { action: jasmine.any(Function), actionText: 'Please try again', actionPayload: { - projectId: 'abcproject', + projectId: TEST_PROJECT, mergeRequestId: 1, force: false, }, @@ -336,7 +341,7 @@ describe('IDE store merge request actions', () => { describe('openMergeRequest', () => { const mr = { - projectId: 'abcproject', + projectId: TEST_PROJECT, targetProjectId: 'defproject', mergeRequestId: 2, }; |