Merge branch '213581-ide-alert-when-cannot-push-code' into 'master'

Step 2 - Add alert and disable when cannot push code in IDE

See merge request gitlab-org/gitlab!51710

app/assets/javascripts/ide/components/commit_sidebar/form.vue

 <script>
 import { mapState, mapActions, mapGetters } from 'vuex';
-import { GlModal, GlSafeHtmlDirective, GlButton } from '@gitlab/ui';
-import { n__ } from '~/locale';
+import { GlModal, GlSafeHtmlDirective, GlButton, GlTooltipDirective } from '@gitlab/ui';
+import { n__, s__ } from '~/locale';
 import CommitMessageField from './message_field.vue';
 import Actions from './actions.vue';
 import SuccessMessage from './success_message.vue';
 import { leftSidebarViews, MAX_WINDOW_HEIGHT_COMPACT } from '../../constants';
 import { createUnexpectedCommitError } from '../../lib/errors';
 
+const MSG_CANNOT_PUSH_CODE = s__(
+  'WebIDE|You need permission to edit files directly in this project.',
+);
+
 export default {
   components: {
     Actions,
@@ -18,6 +22,7 @@ export default {
   },
   directives: {
     SafeHtml: GlSafeHtmlDirective,
+    GlTooltip: GlTooltipDirective,
   },
   data() {
     return {
@@ -30,8 +35,18 @@ export default {
   computed: {
     ...mapState(['changedFiles', 'stagedFiles', 'currentActivityView', 'lastCommitMsg']),
     ...mapState('commit', ['commitMessage', 'submitCommitLoading', 'commitError']),
-    ...mapGetters(['someUncommittedChanges']),
+    ...mapGetters(['someUncommittedChanges', 'canPushCode']),
     ...mapGetters('commit', ['discardDraftButtonDisabled', 'preBuiltCommitMessage']),
+    commitButtonDisabled() {
+      return !this.canPushCode || !this.someUncommittedChanges;
+    },
+    commitButtonTooltip() {
+      if (!this.canPushCode) {
+        return MSG_CANNOT_PUSH_CODE;
+      }
+
+      return '';
+    },
     overviewText() {
       return n__('%d changed file', '%d changed files', this.stagedFiles.length);
     },
@@ -69,6 +84,12 @@ export default {
       'updateCommitAction',
     ]),
     commit() {
+      // Even though the submit button will be disabled, we need to disable the submission
+      // since hitting enter on the branch name text input also submits the form.
+      if (!this.canPushCode) {
+        return false;
+      }
+
       return this.commitChanges();
     },
     handleCompactState() {
@@ -109,6 +130,8 @@ export default {
       this.componentHeight = null;
     },
   },
+  // Expose for tests
+  MSG_CANNOT_PUSH_CODE,
 };
 </script>
 
@@ -130,17 +153,22 @@ export default {
       @after-enter="afterEndTransition"
     >
       <div v-if="isCompact" ref="compactEl" class="commit-form-compact">
-        <gl-button
-          :disabled="!someUncommittedChanges"
-          category="primary"
-          variant="info"
-          block
-          class="qa-begin-commit-button"
-          data-testid="begin-commit-button"
-          @click="beginCommit"
+        <div
+          v-gl-tooltip="{ title: commitButtonTooltip }"
+          data-testid="begin-commit-button-tooltip"
         >
-          {{ __('Commit…') }}
-        </gl-button>
+          <gl-button
+            :disabled="commitButtonDisabled"
+            category="primary"
+            variant="info"
+            block
+            class="qa-begin-commit-button"
+            data-testid="begin-commit-button"
+            @click="beginCommit"
+          >
+            {{ __('Commit…') }}
+          </gl-button>
+        </div>
         <p class="text-center bold">{{ overviewText }}</p>
       </div>
       <form v-else ref="formEl" @submit.prevent.stop="commit">
@@ -153,16 +181,23 @@ export default {
         />
         <div class="clearfix gl-mt-5">
           <actions />
-          <gl-button
-            :loading="submitCommitLoading"
-            class="float-left qa-commit-button"
-            data-testid="commit-button"
-            category="primary"
-            variant="success"
-            @click="commit"
+          <div
+            v-gl-tooltip="{ title: commitButtonTooltip }"
+            class="float-left"
+            data-testid="commit-button-tooltip"
           >
-            {{ __('Commit') }}
-          </gl-button>
+            <gl-button
+              :disabled="commitButtonDisabled"
+              :loading="submitCommitLoading"
+              data-testid="commit-button"
+              class="qa-commit-button"
+              category="primary"
+              variant="success"
+              @click="commit"
+            >
+              {{ __('Commit') }}
+            </gl-button>
+          </div>
           <gl-button
             v-if="!discardDraftButtonDisabled"
             class="float-right"

app/assets/javascripts/ide/components/ide.vue

 <script>
 import { mapActions, mapGetters, mapState } from 'vuex';
-import { GlButton, GlLoadingIcon } from '@gitlab/ui';
-import { __ } from '~/locale';
+import { GlAlert, GlButton, GlLoadingIcon } from '@gitlab/ui';
+import { __, s__ } from '~/locale';
 import {
   WEBIDE_MARK_APP_START,
   WEBIDE_MARK_FILE_FINISH,
@@ -26,10 +26,15 @@ eventHub.$on(WEBIDE_MEASURE_FILE_AFTER_INTERACTION, () =>
   ),
 );
 
+const MSG_CANNOT_PUSH_CODE = s__(
+  'WebIDE|You need permission to edit files directly in this project. Fork this project to make your changes and submit a merge request.',
+);
+
 export default {
   components: {
     IdeSidebar,
     RepoEditor,
+    GlAlert,
     GlButton,
     GlLoadingIcon,
     ErrorMessage: () => import(/* webpackChunkName: 'ide_runtime' */ './error_message.vue'),
@@ -59,12 +64,14 @@ export default {
       'loading',
     ]),
     ...mapGetters([
+      'canPushCode',
       'activeFile',
       'someUncommittedChanges',
       'isCommitModeActive',
       'allBlobs',
       'emptyRepo',
       'currentTree',
+      'hasCurrentProject',
       'editorTheme',
       'getUrlForPath',
     ]),
@@ -110,6 +117,7 @@ export default {
       this.loadDeferred = true;
     },
   },
+  MSG_CANNOT_PUSH_CODE,
 };
 </script>
 
@@ -118,6 +126,9 @@ export default {
     class="ide position-relative d-flex flex-column align-items-stretch"
     :class="{ [`theme-${themeName}`]: themeName }"
   >
+    <gl-alert v-if="!canPushCode" :dismissible="false">{{
+      $options.MSG_CANNOT_PUSH_CODE
+    }}</gl-alert>
     <error-message v-if="errorMessage" :message="errorMessage" />
     <div class="ide-view flex-grow d-flex">
       <template v-if="loadDeferred">

app/assets/javascripts/ide/constants.js

@@ -16,6 +16,13 @@ export const PERMISSION_CREATE_MR = 'createMergeRequestIn';
 export const PERMISSION_READ_MR = 'readMergeRequest';
 export const PERMISSION_PUSH_CODE = 'pushCode';
 
+// The default permission object to use when the project data isn't available yet.
+// This helps us encapsulate checks like `canPushCode` without requiring an
+// additional check like `currentProject && canPushCode`.
+export const DEFAULT_PERMISSIONS = {
+  [PERMISSION_PUSH_CODE]: true,
+};
+
 export const viewerTypes = {
   mr: 'mrdiff',
   edit: 'editor',

app/assets/javascripts/ide/stores/getters.js

@@ -2,6 +2,7 @@ import { getChangesCountForFiles, filePathMatches } from './utils';
 import {
   leftSidebarViews,
   packageJsonPath,
+  DEFAULT_PERMISSIONS,
   PERMISSION_READ_MR,
   PERMISSION_CREATE_MR,
   PERMISSION_PUSH_CODE,
@@ -150,7 +151,7 @@ export const getDiffInfo = (state, getters) => (path) => {
 };
 
 export const findProjectPermissions = (state, getters) => (projectId) =>
-  getters.findProject(projectId)?.userPermissions || {};
+  getters.findProject(projectId)?.userPermissions || DEFAULT_PERMISSIONS;
 
 export const canReadMergeRequests = (state, getters) =>
   Boolean(getters.findProjectPermissions(state.currentProjectId)[PERMISSION_READ_MR]);

changelogs/unreleased/213581-ide-alert-when-cannot-push-code.yml

+---
+title: Web IDE shows alert and disable buttons when user cannot push code
+merge_request: 51710
+author:
+type: changed

locale/gitlab.pot

@@ -31713,6 +31713,12 @@ msgstr ""
 msgid "WebIDE|Merge request"
 msgstr ""
 
+msgid "WebIDE|You need permission to edit files directly in this project."
+msgstr ""
+
+msgid "WebIDE|You need permission to edit files directly in this project. Fork this project to make your changes and submit a merge request."
+msgstr ""
+
 msgid "Webhook"
 msgstr ""
 

spec/frontend/ide/components/commit_sidebar/form_spec.js

@@ -4,6 +4,7 @@ import { GlModal } from '@gitlab/ui';
 import { projectData } from 'jest/ide/mock_data';
 import { stubComponent } from 'helpers/stub_component';
 import waitForPromises from 'helpers/wait_for_promises';
+import { createMockDirective, getBinding } from 'helpers/vue_mock_directive';
 import { createStore } from '~/ide/stores';
 import { COMMIT_TO_NEW_BRANCH } from '~/ide/stores/modules/commit/constants';
 import CommitForm from '~/ide/components/commit_sidebar/form.vue';
@@ -23,6 +24,9 @@ describe('IDE commit form', () => {
   const createComponent = () => {
     wrapper = shallowMount(CommitForm, {
       store,
+      directives: {
+        GlTooltip: createMockDirective(),
+      },
       stubs: {
         GlModal: stubComponent(GlModal),
       },
@@ -39,8 +43,21 @@ describe('IDE commit form', () => {
     store.state.currentActivityView = leftSidebarViews.edit.name;
   };
   const findBeginCommitButton = () => wrapper.find('[data-testid="begin-commit-button"]');
+  const findBeginCommitButtonTooltip = () =>
+    wrapper.find('[data-testid="begin-commit-button-tooltip"]');
+  const findBeginCommitButtonData = () => ({
+    disabled: findBeginCommitButton().props('disabled'),
+    tooltip: getBinding(findBeginCommitButtonTooltip().element, 'gl-tooltip').value.title,
+  });
   const findCommitButton = () => wrapper.find('[data-testid="commit-button"]');
+  const findCommitButtonTooltip = () => wrapper.find('[data-testid="commit-button-tooltip"]');
+  const findCommitButtonData = () => ({
+    disabled: findCommitButton().props('disabled'),
+    tooltip: getBinding(findCommitButtonTooltip().element, 'gl-tooltip').value.title,
+  });
+  const clickCommitButton = () => findCommitButton().vm.$emit('click');
   const findForm = () => wrapper.find('form');
+  const submitForm = () => findForm().trigger('submit');
   const findCommitMessageInput = () => wrapper.find(CommitMessageField);
   const setCommitMessageInput = (val) => findCommitMessageInput().vm.$emit('input', val);
   const findDiscardDraftButton = () => wrapper.find('[data-testid="discard-draft"]');
@@ -52,27 +69,40 @@ describe('IDE commit form', () => {
     store.state.currentBranchId = 'master';
     Vue.set(store.state.projects, 'abcproject', {
       ...projectData,
+      userPermissions: { pushCode: true },
     });
   });
 
   afterEach(() => {
     wrapper.destroy();
-    wrapper = null;
   });
 
+  // Notes:
+  // - When there are no changes, there is no commit button so there's nothing to test :)
   describe.each`
-    desc                           | stagedFiles | disabled
-    ${'when there are changes'}    | ${['test']} | ${false}
-    ${'when there are no changes'} | ${[]}       | ${true}
-  `('$desc', ({ stagedFiles, disabled }) => {
+    desc                           | stagedFiles | userPermissions        | viewFn            | buttonFn                     | disabled | tooltip
+    ${'when there are no changes'} | ${[]}       | ${{ pushCode: true }}  | ${goToEditView}   | ${findBeginCommitButtonData} | ${true}  | ${''}
+    ${'when there are changes'}    | ${['test']} | ${{ pushCode: true }}  | ${goToEditView}   | ${findBeginCommitButtonData} | ${false} | ${''}
+    ${'when there are changes'}    | ${['test']} | ${{ pushCode: true }}  | ${goToCommitView} | ${findCommitButtonData}      | ${false} | ${''}
+    ${'when user cannot push'}     | ${['test']} | ${{ pushCode: false }} | ${goToEditView}   | ${findBeginCommitButtonData} | ${true}  | ${CommitForm.MSG_CANNOT_PUSH_CODE}
+    ${'when user cannot push'}     | ${['test']} | ${{ pushCode: false }} | ${goToCommitView} | ${findCommitButtonData}      | ${true}  | ${CommitForm.MSG_CANNOT_PUSH_CODE}
+  `('$desc', ({ stagedFiles, userPermissions, viewFn, buttonFn, disabled, tooltip }) => {
     beforeEach(async () => {
       store.state.stagedFiles = stagedFiles;
+      store.state.projects.abcproject.userPermissions = userPermissions;
 
       createComponent();
     });
 
-    it(`begin button disabled=${disabled}`, async () => {
-      expect(findBeginCommitButton().props('disabled')).toBe(disabled);
+    it(`at view=${viewFn.name}, ${buttonFn.name} has disabled=${disabled} tooltip=${tooltip}`, async () => {
+      viewFn();
+
+      await wrapper.vm.$nextTick();
+
+      expect(buttonFn()).toEqual({
+        disabled,
+        tooltip,
+      });
     });
   });
 
@@ -252,12 +282,21 @@ describe('IDE commit form', () => {
         jest.spyOn(store, 'dispatch').mockResolvedValue();
       });
 
-      it('calls commitChanges', () => {
-        findCommitButton().vm.$emit('click');
+      it.each([clickCommitButton, submitForm])('when %p, commits changes', (fn) => {
+        fn();
 
         expect(store.dispatch).toHaveBeenCalledWith('commit/commitChanges', undefined);
       });
 
+      it('when cannot push code, submitting does nothing', async () => {
+        store.state.projects.abcproject.userPermissions.pushCode = false;
+        await wrapper.vm.$nextTick();
+
+        submitForm();
+
+        expect(store.dispatch).not.toHaveBeenCalled();
+      });
+
       it.each`
         createError                                          | props
         ${() => createCodeownersCommitError('test message')} | ${{ actionPrimary: { text: 'Create new branch' } }}

spec/frontend/ide/components/ide_spec.js

 import Vuex from 'vuex';
 import { createLocalVue, shallowMount } from '@vue/test-utils';
+import { GlAlert } from '@gitlab/ui';
 import waitForPromises from 'helpers/wait_for_promises';
 import { createStore } from '~/ide/stores';
 import ErrorMessage from '~/ide/components/error_message.vue';
-import ide from '~/ide/components/ide.vue';
+import Ide from '~/ide/components/ide.vue';
 import { file } from '../helpers';
 import { projectData } from '../mock_data';
 
@@ -15,12 +16,12 @@ describe('WebIDE', () => {
 
   let wrapper;
 
-  function createComponent({ projData = emptyProjData, state = {} } = {}) {
+  const createComponent = ({ projData = emptyProjData, state = {} } = {}) => {
     const store = createStore();
 
     store.state.currentProjectId = 'abcproject';
     store.state.currentBranchId = 'master';
-    store.state.projects.abcproject = { ...projData };
+    store.state.projects.abcproject = projData && { ...projData };
     store.state.trees['abcproject/master'] = {
       tree: [],
       loading: false,
@@ -29,11 +30,13 @@ describe('WebIDE', () => {
       store.state[key] = state[key];
     });
 
-    return shallowMount(ide, {
+    wrapper = shallowMount(Ide, {
       store,
       localVue,
     });
-  }
+  };
+
+  const findAlert = () => wrapper.find(GlAlert);
 
   afterEach(() => {
     wrapper.destroy();
@@ -42,7 +45,7 @@ describe('WebIDE', () => {
 
   describe('ide component, empty repo', () => {
     beforeEach(() => {
-      wrapper = createComponent({
+      createComponent({
         projData: {
           empty_repo: true,
         },
@@ -63,7 +66,7 @@ describe('WebIDE', () => {
       `(
         'should error message exists=$exists when errorMessage=$errorMessage',
         async ({ errorMessage, exists }) => {
-          wrapper = createComponent({
+          createComponent({
             state: {
               errorMessage,
             },
@@ -78,12 +81,12 @@ describe('WebIDE', () => {
 
     describe('onBeforeUnload', () => {
       it('returns undefined when no staged files or changed files', () => {
-        wrapper = createComponent();
+        createComponent();
         expect(wrapper.vm.onBeforeUnload()).toBe(undefined);
       });
 
       it('returns warning text when their are changed files', () => {
-        wrapper = createComponent({
+        createComponent({
           state: {
             changedFiles: [file()],
           },
@@ -93,7 +96,7 @@ describe('WebIDE', () => {
       });
 
       it('returns warning text when their are staged files', () => {
-        wrapper = createComponent({
+        createComponent({
           state: {
             stagedFiles: [file()],
           },
@@ -104,7 +107,7 @@ describe('WebIDE', () => {
 
       it('updates event object', () => {
         const event = {};
-        wrapper = createComponent({
+        createComponent({
           state: {
             stagedFiles: [file()],
           },
@@ -118,7 +121,7 @@ describe('WebIDE', () => {
 
     describe('non-existent branch', () => {
       it('does not render "New file" button for non-existent branch when repo is not empty', () => {
-        wrapper = createComponent({
+        createComponent({
           state: {
             projects: {},
           },
@@ -130,7 +133,7 @@ describe('WebIDE', () => {
 
     describe('branch with files', () => {
       beforeEach(() => {
-        wrapper = createComponent({
+        createComponent({
           projData: {
             empty_repo: false,
           },
@@ -142,4 +145,31 @@ describe('WebIDE', () => {
       });
     });
   });
+
+  it('when user cannot push code, shows alert', () => {
+    createComponent({
+      projData: {
+        userPermissions: {
+          pushCode: false,
+        },
+      },
+    });
+
+    expect(findAlert().props()).toMatchObject({
+      dismissible: false,
+    });
+    expect(findAlert().text()).toBe(Ide.MSG_CANNOT_PUSH_CODE);
+  });
+
+  it.each`
+    desc                           | projData
+    ${'when user can push code'}   | ${{ userPermissions: { pushCode: true } }}
+    ${'when project is not ready'} | ${null}
+  `('$desc, no alert is shown', ({ projData }) => {
+    createComponent({
+      projData,
+    });
+
+    expect(findAlert().exists()).toBe(false);
+  });
 });

spec/frontend/ide/stores/getters_spec.js

@@ -2,6 +2,7 @@ import { TEST_HOST } from 'helpers/test_constants';
 import * as getters from '~/ide/stores/getters';
 import { createStore } from '~/ide/stores';
 import { file } from '../helpers';
+import { DEFAULT_PERMISSIONS } from '../../../../app/assets/javascripts/ide/constants';
 
 const TEST_PROJECT_ID = 'test_project';
 
@@ -386,7 +387,9 @@ describe('IDE store getters', () => {
 
   describe('findProjectPermissions', () => {
     it('returns false if project not found', () => {
-      expect(localStore.getters.findProjectPermissions(TEST_PROJECT_ID)).toEqual({});
+      expect(localStore.getters.findProjectPermissions(TEST_PROJECT_ID)).toEqual(
+        DEFAULT_PERMISSIONS,
+      );
     });
 
     it('finds permission in given project', () => {