Commit 6717643c authored by Filipa Lacerda's avatar Filipa Lacerda

Merge branch 'ide-merge-requests-forks' into 'master'

Allow merge requests from forks to be opened in Web IDE

Closes #47460

See merge request gitlab-org/gitlab-ce!20521
parents 16b867d8 9b9cbb4a
...@@ -101,6 +101,7 @@ router.beforeEach((to, from, next) => { ...@@ -101,6 +101,7 @@ router.beforeEach((to, from, next) => {
store store
.dispatch('getMergeRequestData', { .dispatch('getMergeRequestData', {
projectId: fullProjectId, projectId: fullProjectId,
targetProjectId: to.query.target_project,
mergeRequestId: to.params.mrid, mergeRequestId: to.params.mrid,
}) })
.then(mr => { .then(mr => {
...@@ -119,12 +120,14 @@ router.beforeEach((to, from, next) => { ...@@ -119,12 +120,14 @@ router.beforeEach((to, from, next) => {
.then(() => .then(() =>
store.dispatch('getMergeRequestVersions', { store.dispatch('getMergeRequestVersions', {
projectId: fullProjectId, projectId: fullProjectId,
targetProjectId: to.query.target_project,
mergeRequestId: to.params.mrid, mergeRequestId: to.params.mrid,
}), }),
) )
.then(() => .then(() =>
store.dispatch('getMergeRequestChanges', { store.dispatch('getMergeRequestChanges', {
projectId: fullProjectId, projectId: fullProjectId,
targetProjectId: to.query.target_project,
mergeRequestId: to.params.mrid, mergeRequestId: to.params.mrid,
}), }),
) )
......
...@@ -4,12 +4,14 @@ import * as types from '../mutation_types'; ...@@ -4,12 +4,14 @@ import * as types from '../mutation_types';
export const getMergeRequestData = ( export const getMergeRequestData = (
{ commit, dispatch, state }, { commit, dispatch, state },
{ projectId, mergeRequestId, force = false } = {}, { projectId, mergeRequestId, targetProjectId = null, force = false } = {},
) => ) =>
new Promise((resolve, reject) => { new Promise((resolve, reject) => {
if (!state.projects[projectId].mergeRequests[mergeRequestId] || force) { if (!state.projects[projectId].mergeRequests[mergeRequestId] || force) {
service service
.getProjectMergeRequestData(projectId, mergeRequestId, { render_html: true }) .getProjectMergeRequestData(targetProjectId || projectId, mergeRequestId, {
render_html: true,
})
.then(({ data }) => { .then(({ data }) => {
commit(types.SET_MERGE_REQUEST, { commit(types.SET_MERGE_REQUEST, {
projectPath: projectId, projectPath: projectId,
...@@ -38,12 +40,12 @@ export const getMergeRequestData = ( ...@@ -38,12 +40,12 @@ export const getMergeRequestData = (
export const getMergeRequestChanges = ( export const getMergeRequestChanges = (
{ commit, dispatch, state }, { commit, dispatch, state },
{ projectId, mergeRequestId, force = false } = {}, { projectId, mergeRequestId, targetProjectId = null, force = false } = {},
) => ) =>
new Promise((resolve, reject) => { new Promise((resolve, reject) => {
if (!state.projects[projectId].mergeRequests[mergeRequestId].changes.length || force) { if (!state.projects[projectId].mergeRequests[mergeRequestId].changes.length || force) {
service service
.getProjectMergeRequestChanges(projectId, mergeRequestId) .getProjectMergeRequestChanges(targetProjectId || projectId, mergeRequestId)
.then(({ data }) => { .then(({ data }) => {
commit(types.SET_MERGE_REQUEST_CHANGES, { commit(types.SET_MERGE_REQUEST_CHANGES, {
projectPath: projectId, projectPath: projectId,
...@@ -71,12 +73,12 @@ export const getMergeRequestChanges = ( ...@@ -71,12 +73,12 @@ export const getMergeRequestChanges = (
export const getMergeRequestVersions = ( export const getMergeRequestVersions = (
{ commit, dispatch, state }, { commit, dispatch, state },
{ projectId, mergeRequestId, force = false } = {}, { projectId, mergeRequestId, targetProjectId = null, force = false } = {},
) => ) =>
new Promise((resolve, reject) => { new Promise((resolve, reject) => {
if (!state.projects[projectId].mergeRequests[mergeRequestId].versions.length || force) { if (!state.projects[projectId].mergeRequests[mergeRequestId].versions.length || force) {
service service
.getProjectMergeRequestVersions(projectId, mergeRequestId) .getProjectMergeRequestVersions(targetProjectId || projectId, mergeRequestId)
.then(res => res.data) .then(res => res.data)
.then(data => { .then(data => {
commit(types.SET_MERGE_REQUEST_VERSIONS, { commit(types.SET_MERGE_REQUEST_VERSIONS, {
......
<script> <script>
import tooltip from '~/vue_shared/directives/tooltip'; import tooltip from '~/vue_shared/directives/tooltip';
import { n__ } from '~/locale'; import { n__ } from '~/locale';
import { webIDEUrl } from '~/lib/utils/url_utility'; import { mergeUrlParams, webIDEUrl } from '~/lib/utils/url_utility';
import Icon from '~/vue_shared/components/icon.vue'; import Icon from '~/vue_shared/components/icon.vue';
import clipboardButton from '~/vue_shared/components/clipboard_button.vue'; import clipboardButton from '~/vue_shared/components/clipboard_button.vue';
...@@ -43,7 +43,10 @@ export default { ...@@ -43,7 +43,10 @@ export default {
return this.isBranchTitleLong(this.mr.targetBranch); return this.isBranchTitleLong(this.mr.targetBranch);
}, },
webIdePath() { webIdePath() {
return webIDEUrl(this.mr.statusPath.replace('.json', '')); return mergeUrlParams({
target_project: this.mr.sourceProjectFullPath !== this.mr.targetProjectFullPath ?
this.mr.targetProjectFullPath : '',
}, webIDEUrl(`/${this.mr.sourceProjectFullPath}/merge_requests/${this.mr.iid}`));
}, },
}, },
methods: { methods: {
......
...@@ -16,10 +16,11 @@ export default class MergeRequestStore { ...@@ -16,10 +16,11 @@ export default class MergeRequestStore {
const pipelineStatus = data.pipeline ? data.pipeline.details.status : null; const pipelineStatus = data.pipeline ? data.pipeline.details.status : null;
this.squash = data.squash; this.squash = data.squash;
this.squashBeforeMergeHelpPath = this.squashBeforeMergeHelpPath || this.squashBeforeMergeHelpPath =
data.squash_before_merge_help_path; this.squashBeforeMergeHelpPath || data.squash_before_merge_help_path;
this.enableSquashBeforeMerge = this.enableSquashBeforeMerge || true; this.enableSquashBeforeMerge = this.enableSquashBeforeMerge || true;
this.iid = data.iid;
this.title = data.title; this.title = data.title;
this.targetBranch = data.target_branch; this.targetBranch = data.target_branch;
this.sourceBranch = data.source_branch; this.sourceBranch = data.source_branch;
...@@ -85,6 +86,8 @@ export default class MergeRequestStore { ...@@ -85,6 +86,8 @@ export default class MergeRequestStore {
this.isMergeAllowed = data.mergeable || false; this.isMergeAllowed = data.mergeable || false;
this.mergeOngoing = data.merge_ongoing; this.mergeOngoing = data.merge_ongoing;
this.allowCollaboration = data.allow_collaboration; this.allowCollaboration = data.allow_collaboration;
this.targetProjectFullPath = data.target_project_full_path;
this.sourceProjectFullPath = data.source_project_full_path;
// Cherry-pick and Revert actions related // Cherry-pick and Revert actions related
this.canCherryPickInCurrentMR = currentUser.can_cherry_pick_on_current_merge_request || false; this.canCherryPickInCurrentMR = currentUser.can_cherry_pick_on_current_merge_request || false;
...@@ -97,7 +100,8 @@ export default class MergeRequestStore { ...@@ -97,7 +100,8 @@ export default class MergeRequestStore {
this.hasCI = data.has_ci; this.hasCI = data.has_ci;
this.ciStatus = data.ci_status; this.ciStatus = data.ci_status;
this.isPipelineFailed = this.ciStatus === 'failed' || this.ciStatus === 'canceled'; this.isPipelineFailed = this.ciStatus === 'failed' || this.ciStatus === 'canceled';
this.isPipelinePassing = this.ciStatus === 'success' || this.ciStatus === 'success_with_warnings'; this.isPipelinePassing =
this.ciStatus === 'success' || this.ciStatus === 'success_with_warnings';
this.isPipelineSkipped = this.ciStatus === 'skipped'; this.isPipelineSkipped = this.ciStatus === 'skipped';
this.pipelineDetailedStatus = pipelineStatus; this.pipelineDetailedStatus = pipelineStatus;
this.isPipelineActive = data.pipeline ? data.pipeline.active : false; this.isPipelineActive = data.pipeline ? data.pipeline.active : false;
......
...@@ -10,9 +10,15 @@ class MergeRequestWidgetEntity < IssuableEntity ...@@ -10,9 +10,15 @@ class MergeRequestWidgetEntity < IssuableEntity
expose :merge_when_pipeline_succeeds expose :merge_when_pipeline_succeeds
expose :source_branch expose :source_branch
expose :source_project_id expose :source_project_id
expose :source_project_full_path do |merge_request|
merge_request.source_project&.full_path
end
expose :squash expose :squash
expose :target_branch expose :target_branch
expose :target_project_id expose :target_project_id
expose :target_project_full_path do |merge_request|
merge_request.project&.full_path
end
expose :allow_collaboration expose :allow_collaboration
expose :should_be_rebased?, as: :should_be_rebased expose :should_be_rebased?, as: :should_be_rebased
......
...@@ -6,6 +6,18 @@ class Commits < Grape::API ...@@ -6,6 +6,18 @@ class Commits < Grape::API
before { authorize! :download_code, user_project } before { authorize! :download_code, user_project }
helpers do
def user_access
@user_access ||= Gitlab::UserAccess.new(current_user, project: user_project)
end
def authorize_push_to_branch!(branch)
unless user_access.can_push_to_branch?(branch)
forbidden!("You are not allowed to push into this branch")
end
end
end
params do params do
requires :id, type: String, desc: 'The ID of a project' requires :id, type: String, desc: 'The ID of a project'
end end
...@@ -67,7 +79,7 @@ class Commits < Grape::API ...@@ -67,7 +79,7 @@ class Commits < Grape::API
optional :author_name, type: String, desc: 'Author name for commit' optional :author_name, type: String, desc: 'Author name for commit'
end end
post ':id/repository/commits' do post ':id/repository/commits' do
authorize! :push_code, user_project authorize_push_to_branch!(params[:branch])
attrs = declared_params attrs = declared_params
attrs[:branch_name] = attrs.delete(:branch) attrs[:branch_name] = attrs.delete(:branch)
...@@ -142,7 +154,7 @@ class Commits < Grape::API ...@@ -142,7 +154,7 @@ class Commits < Grape::API
requires :branch, type: String, desc: 'The name of the branch' requires :branch, type: String, desc: 'The name of the branch'
end end
post ':id/repository/commits/:sha/cherry_pick', requirements: API::COMMIT_ENDPOINT_REQUIREMENTS do post ':id/repository/commits/:sha/cherry_pick', requirements: API::COMMIT_ENDPOINT_REQUIREMENTS do
authorize! :push_code, user_project authorize_push_to_branch!(params[:branch])
commit = user_project.commit(params[:sha]) commit = user_project.commit(params[:sha])
not_found!('Commit') unless commit not_found!('Commit') unless commit
......
...@@ -29,8 +29,10 @@ ...@@ -29,8 +29,10 @@
"merge_when_pipeline_succeeds": { "type": "boolean" }, "merge_when_pipeline_succeeds": { "type": "boolean" },
"source_branch": { "type": "string" }, "source_branch": { "type": "string" },
"source_project_id": { "type": "integer" }, "source_project_id": { "type": "integer" },
"source_project_full_path": { "type": ["string", "null"]},
"target_branch": { "type": "string" }, "target_branch": { "type": "string" },
"target_project_id": { "type": "integer" }, "target_project_id": { "type": "integer" },
"target_project_full_path": { "type": ["string", "null"]},
"allow_collaboration": { "type": "boolean"}, "allow_collaboration": { "type": "boolean"},
"metrics": { "metrics": {
"oneOf": [ "oneOf": [
......
...@@ -119,6 +119,7 @@ describe('MRWidgetHeader', () => { ...@@ -119,6 +119,7 @@ describe('MRWidgetHeader', () => {
beforeEach(() => { beforeEach(() => {
vm = mountComponent(Component, { vm = mountComponent(Component, {
mr: { mr: {
iid: 1,
divergedCommitsCount: 12, divergedCommitsCount: 12,
sourceBranch: 'mr-widget-refactor', sourceBranch: 'mr-widget-refactor',
sourceBranchLink: '<a href="/foo/bar/mr-widget-refactor">mr-widget-refactor</a>', sourceBranchLink: '<a href="/foo/bar/mr-widget-refactor">mr-widget-refactor</a>',
...@@ -130,6 +131,8 @@ describe('MRWidgetHeader', () => { ...@@ -130,6 +131,8 @@ describe('MRWidgetHeader', () => {
emailPatchesPath: '/mr/email-patches', emailPatchesPath: '/mr/email-patches',
plainDiffPath: '/mr/plainDiffPath', plainDiffPath: '/mr/plainDiffPath',
statusPath: 'abc', statusPath: 'abc',
sourceProjectFullPath: 'root/gitlab-ce',
targetProjectFullPath: 'gitlab-org/gitlab-ce',
}, },
}); });
}); });
...@@ -146,16 +149,40 @@ describe('MRWidgetHeader', () => { ...@@ -146,16 +149,40 @@ describe('MRWidgetHeader', () => {
const button = vm.$el.querySelector('.js-web-ide'); const button = vm.$el.querySelector('.js-web-ide');
expect(button.textContent.trim()).toEqual('Open in Web IDE'); expect(button.textContent.trim()).toEqual('Open in Web IDE');
expect(button.getAttribute('href')).toEqual('/-/ide/projectabc'); expect(button.getAttribute('href')).toEqual(
'/-/ide/project/root/gitlab-ce/merge_requests/1?target_project=gitlab-org%2Fgitlab-ce',
);
}); });
it('renders web ide button with relative URL', () => { it('renders web ide button with blank query string if target & source project branch', done => {
vm.mr.targetProjectFullPath = 'root/gitlab-ce';
vm.$nextTick(() => {
const button = vm.$el.querySelector('.js-web-ide');
expect(button.textContent.trim()).toEqual('Open in Web IDE');
expect(button.getAttribute('href')).toEqual(
'/-/ide/project/root/gitlab-ce/merge_requests/1?target_project=',
);
done();
});
});
it('renders web ide button with relative URL', done => {
gon.relative_url_root = '/gitlab'; gon.relative_url_root = '/gitlab';
vm.mr.iid = 2;
const button = vm.$el.querySelector('.js-web-ide'); vm.$nextTick(() => {
const button = vm.$el.querySelector('.js-web-ide');
expect(button.textContent.trim()).toEqual('Open in Web IDE'); expect(button.textContent.trim()).toEqual('Open in Web IDE');
expect(button.getAttribute('href')).toEqual('/-/ide/projectabc'); expect(button.getAttribute('href')).toEqual(
'/gitlab/-/ide/project/root/gitlab-ce/merge_requests/2?target_project=gitlab-org%2Fgitlab-ce',
);
done();
});
}); });
it('renders download dropdown with links', () => { it('renders download dropdown with links', () => {
......
...@@ -29,8 +29,10 @@ export default { ...@@ -29,8 +29,10 @@ export default {
source_branch: 'daaaa', source_branch: 'daaaa',
source_branch_link: 'daaaa', source_branch_link: 'daaaa',
source_project_id: 19, source_project_id: 19,
source_project_full_path: '/group1/project1',
target_branch: 'master', target_branch: 'master',
target_project_id: 19, target_project_id: 19,
target_project_full_path: '/group2/project2',
metrics: { metrics: {
merged_by: { merged_by: {
name: 'Administrator', name: 'Administrator',
......
...@@ -514,6 +514,38 @@ ...@@ -514,6 +514,38 @@
expect(response).to have_gitlab_http_status(400) expect(response).to have_gitlab_http_status(400)
end end
end end
context 'when committing into a fork as a maintainer' do
include_context 'merge request allowing collaboration'
let(:project_id) { forked_project.id }
def push_params(branch_name)
{
branch: branch_name,
commit_message: 'Hello world',
actions: [
{
action: 'create',
file_path: 'foo/bar/baz.txt',
content: 'puts 8'
}
]
}
end
it 'allows pushing to the source branch of the merge request' do
post api(url, user), push_params('feature')
expect(response).to have_gitlab_http_status(:created)
end
it 'denies pushing to another branch' do
post api(url, user), push_params('other-branch')
expect(response).to have_gitlab_http_status(:forbidden)
end
end
end end
describe 'GET /projects/:id/repository/commits/:sha/refs' do describe 'GET /projects/:id/repository/commits/:sha/refs' do
...@@ -1065,11 +1097,29 @@ ...@@ -1065,11 +1097,29 @@
it 'returns 400 if you are not allowed to push to the target branch' do it 'returns 400 if you are not allowed to push to the target branch' do
post api(route, current_user), branch: 'feature' post api(route, current_user), branch: 'feature'
expect(response).to have_gitlab_http_status(400) expect(response).to have_gitlab_http_status(:forbidden)
expect(json_response['message']).to eq('You are not allowed to push into this branch') expect(json_response['message']).to match(/You are not allowed to push into this branch/)
end end
end end
end end
context 'when cherry picking to a fork as a maintainer' do
include_context 'merge request allowing collaboration'
let(:project_id) { forked_project.id }
it 'allows access from a maintainer that to the source branch' do
post api(route, user), branch: 'feature'
expect(response).to have_gitlab_http_status(:created)
end
it 'denies cherry picking to another branch' do
post api(route, user), branch: 'master'
expect(response).to have_gitlab_http_status(:forbidden)
end
end
end end
describe 'POST /projects/:id/repository/commits/:sha/comments' do describe 'POST /projects/:id/repository/commits/:sha/comments' do
......
...@@ -11,6 +11,21 @@ ...@@ -11,6 +11,21 @@
described_class.new(resource, request: request).as_json described_class.new(resource, request: request).as_json
end end
describe 'source_project_full_path' do
it 'includes the full path of the source project' do
expect(subject[:source_project_full_path]).to be_present
end
context 'when the source project is missing' do
it 'returns `nil` for the source project' do
resource.allow_broken = true
resource.update!(source_project: nil)
expect(subject[:source_project_full_path]).to be_nil
end
end
end
describe 'pipeline' do describe 'pipeline' do
let(:pipeline) { create(:ci_empty_pipeline, project: project, ref: resource.source_branch, sha: resource.source_branch_sha, head_pipeline_of: resource) } let(:pipeline) { create(:ci_empty_pipeline, project: project, ref: resource.source_branch, sha: resource.source_branch_sha, head_pipeline_of: resource) }
......
shared_context 'merge request allowing collaboration' do
include ProjectForksHelper
let(:canonical) { create(:project, :public, :repository) }
let(:forked_project) { fork_project(canonical, nil, repository: true) }
before do
canonical.add_maintainer(user)
create(:merge_request,
target_project: canonical,
source_project: forked_project,
source_branch: 'feature',
allow_collaboration: true)
end
end
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment