Commit 55e31c56 authored by Paul Slaughter's avatar Paul Slaughter

Fix IDE when MRs are not available

- Skips MR detection
- Hide MRs from IDE nav dropdown when not available
- Disable new merge request option if can't create MR
parent 46f2ae5b
<script>
import { createNamespacedHelpers } from 'vuex';
import { GlTooltipDirective } from '@gitlab/ui';
import { s__ } from '~/locale';
const {
mapState: mapCommitState,
mapActions: mapCommitActions,
mapGetters: mapCommitGetters,
} = createNamespacedHelpers('commit');
const { mapActions: mapCommitActions, mapGetters: mapCommitGetters } = createNamespacedHelpers(
'commit',
);
export default {
directives: {
GlTooltip: GlTooltipDirective,
},
computed: {
...mapCommitState(['shouldCreateMR']),
...mapCommitGetters(['shouldHideNewMrOption']),
...mapCommitGetters(['shouldHideNewMrOption', 'shouldDisableNewMrOption', 'shouldCreateMR']),
tooltipText() {
if (this.shouldDisableNewMrOption) {
return s__(
'IDE|This option is disabled because you are not allowed to create merge requests in this project.',
);
}
return '';
},
},
methods: {
...mapCommitActions(['toggleShouldCreateMR']),
......@@ -21,14 +32,19 @@ export default {
<template>
<fieldset v-if="!shouldHideNewMrOption">
<hr class="my-2" />
<label class="mb-0 js-ide-commit-new-mr">
<label
v-gl-tooltip="tooltipText"
class="mb-0 js-ide-commit-new-mr"
:class="{ 'is-disabled': shouldDisableNewMrOption }"
>
<input
:disabled="shouldDisableNewMrOption"
:checked="shouldCreateMR"
type="checkbox"
data-qa-selector="start_new_mr_checkbox"
@change="toggleShouldCreateMR"
/>
<span class="prepend-left-10">
<span class="prepend-left-10 ide-radio-label">
{{ __('Start a new merge request') }}
</span>
</label>
......
<script>
import $ from 'jquery';
import { mapGetters } from 'vuex';
import NavForm from './nav_form.vue';
import NavDropdownButton from './nav_dropdown_button.vue';
......@@ -13,6 +14,9 @@ export default {
isVisibleDropdown: false,
};
},
computed: {
...mapGetters(['canReadMergeRequests']),
},
mounted() {
this.addDropdownListeners();
},
......@@ -42,7 +46,9 @@ export default {
<template>
<div ref="dropdown" class="btn-group ide-nav-dropdown dropdown">
<nav-dropdown-button />
<div class="dropdown-menu dropdown-menu-left p-0"><nav-form v-if="isVisibleDropdown" /></div>
<nav-dropdown-button :show-merge-requests="canReadMergeRequests" />
<div class="dropdown-menu dropdown-menu-left p-0">
<nav-form v-if="isVisibleDropdown" :show-merge-requests="canReadMergeRequests" />
</div>
</div>
</template>
......@@ -10,6 +10,13 @@ export default {
Icon,
DropdownButton,
},
props: {
showMergeRequests: {
type: Boolean,
required: false,
default: true,
},
},
computed: {
...mapState(['currentBranchId', 'currentMergeRequestId']),
mergeRequestLabel() {
......@@ -25,10 +32,10 @@ export default {
<template>
<dropdown-button>
<span class="row">
<span class="col-7 text-truncate">
<span class="col-auto text-truncate" :class="{ 'col-7': showMergeRequests }">
<icon :size="16" :aria-label="__('Current Branch')" name="branch" /> {{ branchLabel }}
</span>
<span class="col-5 pl-0 text-truncate">
<span v-if="showMergeRequests" class="col-5 pl-0 text-truncate">
<icon :size="16" :aria-label="__('Merge Request')" name="merge-request" />
{{ mergeRequestLabel }}
</span>
......
......@@ -11,12 +11,19 @@ export default {
BranchesSearchList,
MergeRequestSearchList,
},
props: {
showMergeRequests: {
type: Boolean,
required: false,
default: true,
},
},
};
</script>
<template>
<div class="ide-nav-form p-0">
<tabs stop-propagation>
<tabs v-if="showMergeRequests" stop-propagation>
<tab active>
<template slot="title">
{{ __('Branches') }}
......@@ -30,5 +37,6 @@ export default {
<merge-request-search-list />
</tab>
</tabs>
<branches-search-list v-else />
</div>
</template>
......@@ -2,10 +2,17 @@ import flash from '~/flash';
import { __ } from '~/locale';
import service from '../../services';
import * as types from '../mutation_types';
import { activityBarViews } from '../../constants';
import { activityBarViews, PERMISSION_READ_MR } from '../../constants';
export const getMergeRequestsForBranch = ({ commit, state }, { projectId, branchId } = {}) =>
service
export const getMergeRequestsForBranch = (
{ commit, state, getters },
{ projectId, branchId } = {},
) => {
if (!getters.findProjectPermissions(projectId)[PERMISSION_READ_MR]) {
return Promise.resolve();
}
return service
.getProjectMergeRequests(`${projectId}`, {
source_branch: branchId,
source_project_id: state.projects[projectId].id,
......@@ -36,6 +43,7 @@ export const getMergeRequestsForBranch = ({ commit, state }, { projectId, branch
);
throw e;
});
};
export const getMergeRequestData = (
{ commit, dispatch, state },
......
......@@ -158,7 +158,7 @@ export const commitChanges = ({ commit, state, getters, dispatch, rootState, roo
commit(rootTypes.SET_LAST_COMMIT_MSG, '', { root: true });
}, 5000);
if (state.shouldCreateMR) {
if (getters.shouldCreateMR) {
const { currentProject } = rootGetters;
const targetBranch = getters.isCreatingNewBranch
? rootState.currentBranchId
......
......@@ -54,5 +54,11 @@ export const shouldHideNewMrOption = (_state, getters, _rootState, rootGetters)
(!rootGetters.hasMergeRequest && rootGetters.isOnDefaultBranch)) &&
rootGetters.canPushToBranch;
export const shouldDisableNewMrOption = (state, getters, rootState, rootGetters) =>
!rootGetters.canCreateMergeRequests;
export const shouldCreateMR = (state, getters) =>
state.shouldCreateMR && !getters.shouldDisableNewMrOption;
// prevent babel-plugin-rewire from generating an invalid default during karma tests
export default () => {};
---
title: Enable Web IDE on projects without Merge Requests
merge_request: 24508
author:
type: fixed
......@@ -10121,6 +10121,9 @@ msgstr ""
msgid "IDE|Successful commit"
msgstr ""
msgid "IDE|This option is disabled because you are not allowed to create merge requests in this project."
msgstr ""
msgid "IDE|This option is disabled because you don't have write permissions for the current branch."
msgstr ""
......
......@@ -18,6 +18,7 @@ export const projectData = {
},
mergeRequests: {},
merge_requests_enabled: true,
userPermissions: {},
default_branch: 'master',
};
......
import Vue from 'vue';
import { createComponentWithStore } from 'spec/helpers/vue_mount_component_helper';
import { projectData, branches } from 'spec/ide/mock_data';
import { resetStore } from 'spec/ide/helpers';
import NewMergeRequestOption from '~/ide/components/commit_sidebar/new_merge_request_option.vue';
import store from '~/ide/stores';
import consts from '../../../../../app/assets/javascripts/ide/stores/modules/commit/constants';
import { createStore } from '~/ide/stores';
import { PERMISSION_CREATE_MR } from '~/ide/constants';
import consts from '~/ide/stores/modules/commit/constants';
describe('create new MR checkbox', () => {
let store;
let vm;
const setMR = () => {
vm.$store.state.currentMergeRequestId = '1';
vm.$store.state.projects[store.state.currentProjectId].mergeRequests[
......@@ -15,6 +17,10 @@ describe('create new MR checkbox', () => {
] = { foo: 'bar' };
};
const setPermissions = permissions => {
store.state.projects[store.state.currentProjectId].userPermissions = permissions;
};
const createComponent = ({ currentBranchId = 'master', createNewBranch = false } = {}) => {
const Component = Vue.extend(NewMergeRequestOption);
......@@ -25,20 +31,29 @@ describe('create new MR checkbox', () => {
: consts.COMMIT_TO_CURRENT_BRANCH;
vm.$store.state.currentBranchId = currentBranchId;
vm.$store.state.currentProjectId = 'abcproject';
const proj = JSON.parse(JSON.stringify(projectData));
proj.branches[currentBranchId] = branches.find(branch => branch.name === currentBranchId);
Vue.set(vm.$store.state.projects, 'abcproject', proj);
store.state.projects.abcproject.branches[currentBranchId] = branches.find(
branch => branch.name === currentBranchId,
);
return vm.$mount();
};
const findInput = () => vm.$el.querySelector('input[type="checkbox"]');
const findLabel = () => vm.$el.querySelector('.js-ide-commit-new-mr');
beforeEach(() => {
store = createStore();
store.state.currentProjectId = 'abcproject';
const proj = JSON.parse(JSON.stringify(projectData));
proj.userPermissions[PERMISSION_CREATE_MR] = true;
Vue.set(store.state.projects, 'abcproject', proj);
});
afterEach(() => {
vm.$destroy();
resetStore(vm.$store);
});
describe('for default branch', () => {
......@@ -160,6 +175,24 @@ describe('create new MR checkbox', () => {
.then(done)
.catch(done.fail);
});
it('shows enablded checkbox', () => {
expect(findLabel().classList.contains('is-disabled')).toBe(false);
expect(findInput().disabled).toBe(false);
});
});
describe('when user cannot create MR', () => {
beforeEach(() => {
setPermissions({ [PERMISSION_CREATE_MR]: false });
createComponent({ currentBranchId: 'regular' });
});
it('disabled checkbox', () => {
expect(findLabel().classList.contains('is-disabled')).toBe(true);
expect(findInput().disabled).toBe(true);
});
});
it('dispatches toggleShouldCreateMR when clicking checkbox', () => {
......
......@@ -2,62 +2,92 @@ import Vue from 'vue';
import { trimText } from 'spec/helpers/text_helper';
import { mountComponentWithStore } from 'spec/helpers/vue_mount_component_helper';
import NavDropdownButton from '~/ide/components/nav_dropdown_button.vue';
import store from '~/ide/stores';
import { resetStore } from '../helpers';
import { createStore } from '~/ide/stores';
describe('NavDropdown', () => {
const TEST_BRANCH_ID = 'lorem-ipsum-dolar';
const TEST_MR_ID = '12345';
const Component = Vue.extend(NavDropdownButton);
let store;
let vm;
beforeEach(() => {
vm = mountComponentWithStore(Component, { store });
vm.$mount();
store = createStore();
});
afterEach(() => {
vm.$destroy();
resetStore(store);
});
it('renders empty placeholders, if state is falsey', () => {
expect(trimText(vm.$el.textContent)).toEqual('- -');
});
const createComponent = (props = {}) => {
vm = mountComponentWithStore(Vue.extend(NavDropdownButton), { props, store });
vm.$mount();
};
it('renders branch name, if state has currentBranchId', done => {
vm.$store.state.currentBranchId = TEST_BRANCH_ID;
const findIcon = name => vm.$el.querySelector(`.ic-${name}`);
const findMRIcon = () => findIcon('merge-request');
const findBranchIcon = () => findIcon('branch');
vm.$nextTick()
.then(() => {
expect(trimText(vm.$el.textContent)).toEqual(`${TEST_BRANCH_ID} -`);
})
.then(done)
.catch(done.fail);
});
describe('normal', () => {
beforeEach(() => {
createComponent();
});
it('renders empty placeholders, if state is falsey', () => {
expect(trimText(vm.$el.textContent)).toEqual('- -');
});
it('renders mr id, if state has currentMergeRequestId', done => {
vm.$store.state.currentMergeRequestId = TEST_MR_ID;
it('renders branch name, if state has currentBranchId', done => {
vm.$store.state.currentBranchId = TEST_BRANCH_ID;
vm.$nextTick()
.then(() => {
expect(trimText(vm.$el.textContent)).toEqual(`- !${TEST_MR_ID}`);
})
.then(done)
.catch(done.fail);
vm.$nextTick()
.then(() => {
expect(trimText(vm.$el.textContent)).toEqual(`${TEST_BRANCH_ID} -`);
})
.then(done)
.catch(done.fail);
});
it('renders mr id, if state has currentMergeRequestId', done => {
vm.$store.state.currentMergeRequestId = TEST_MR_ID;
vm.$nextTick()
.then(() => {
expect(trimText(vm.$el.textContent)).toEqual(`- !${TEST_MR_ID}`);
})
.then(done)
.catch(done.fail);
});
it('renders branch and mr, if state has both', done => {
vm.$store.state.currentBranchId = TEST_BRANCH_ID;
vm.$store.state.currentMergeRequestId = TEST_MR_ID;
vm.$nextTick()
.then(() => {
expect(trimText(vm.$el.textContent)).toEqual(`${TEST_BRANCH_ID} !${TEST_MR_ID}`);
})
.then(done)
.catch(done.fail);
});
it('shows icons', () => {
expect(findBranchIcon()).toBeTruthy();
expect(findMRIcon()).toBeTruthy();
});
});
it('renders branch and mr, if state has both', done => {
vm.$store.state.currentBranchId = TEST_BRANCH_ID;
vm.$store.state.currentMergeRequestId = TEST_MR_ID;
describe('with showMergeRequests false', () => {
beforeEach(() => {
createComponent({ showMergeRequests: false });
});
it('shows single empty placeholder, if state is falsey', () => {
expect(trimText(vm.$el.textContent)).toEqual('-');
});
vm.$nextTick()
.then(() => {
expect(trimText(vm.$el.textContent)).toEqual(`${TEST_BRANCH_ID} !${TEST_MR_ID}`);
})
.then(done)
.catch(done.fail);
it('shows only branch icon', () => {
expect(findBranchIcon()).toBeTruthy();
expect(findMRIcon()).toBe(null);
});
});
});
......@@ -3,6 +3,9 @@ import Vue from 'vue';
import { mountComponentWithStore } from 'spec/helpers/vue_mount_component_helper';
import store from '~/ide/stores';
import NavDropdown from '~/ide/components/nav_dropdown.vue';
import { PERMISSION_READ_MR } from '~/ide/constants';
const TEST_PROJECT_ID = 'lorem-ipsum';
describe('IDE NavDropdown', () => {
const Component = Vue.extend(NavDropdown);
......@@ -10,6 +13,12 @@ describe('IDE NavDropdown', () => {
let $dropdown;
beforeEach(() => {
store.state.currentProjectId = TEST_PROJECT_ID;
Vue.set(store.state.projects, TEST_PROJECT_ID, {
userPermissions: {
[PERMISSION_READ_MR]: true,
},
});
vm = mountComponentWithStore(Component, { store });
$dropdown = $(vm.$el);
......@@ -21,6 +30,9 @@ describe('IDE NavDropdown', () => {
vm.$destroy();
});
const findIcon = name => vm.$el.querySelector(`.ic-${name}`);
const findMRIcon = () => findIcon('merge-request');
it('renders nothing initially', () => {
expect(vm.$el).not.toContainElement('.ide-nav-form');
});
......@@ -47,4 +59,22 @@ describe('IDE NavDropdown', () => {
.then(done)
.catch(done.fail);
});
it('renders merge request icon', () => {
expect(findMRIcon()).not.toBeNull();
});
describe('when user cannot read merge requests', () => {
beforeEach(done => {
store.state.projects[TEST_PROJECT_ID].userPermissions = {};
vm.$nextTick()
.then(done)
.catch(done.fail);
});
it('does not render merge requests', () => {
expect(findMRIcon()).toBeNull();
});
});
});
......@@ -8,7 +8,7 @@ import actions, {
openMergeRequest,
} from '~/ide/stores/actions/merge_request';
import service from '~/ide/services';
import { activityBarViews } from '~/ide/constants';
import { activityBarViews, PERMISSION_READ_MR } from '~/ide/constants';
import { resetStore } from '../../helpers';
const TEST_PROJECT = 'abcproject';
......@@ -23,6 +23,9 @@ describe('IDE store merge request actions', () => {
store.state.projects[TEST_PROJECT] = {
id: TEST_PROJECT_ID,
mergeRequests: {},
userPermissions: {
[PERMISSION_READ_MR]: true,
},
};
});
......@@ -79,6 +82,19 @@ describe('IDE store merge request actions', () => {
})
.catch(done.fail);
});
it('does nothing if user cannot read MRs', done => {
store.state.projects[TEST_PROJECT].userPermissions[PERMISSION_READ_MR] = false;
store
.dispatch('getMergeRequestsForBranch', { projectId: TEST_PROJECT, branchId: 'bar' })
.then(() => {
expect(service.getProjectMergeRequests).not.toHaveBeenCalled();
expect(store.state.currentMergeRequestId).toBe('');
})
.then(done)
.catch(done.fail);
});
});
describe('no merge requests for branch available case', () => {
......
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