Merge branch '213581-ide-alert-user-when-reject-unsigned-commits' into 'master'

Web IDE show alert when project rejects unsigned commits

See merge request gitlab-org/gitlab!54166

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

 <script>
 import { GlModal, GlSafeHtmlDirective, GlButton, GlTooltipDirective } from '@gitlab/ui';
 import { mapState, mapActions, mapGetters } from 'vuex';
-import { n__, s__ } from '~/locale';
+import { n__ } from '~/locale';
 import { leftSidebarViews, MAX_WINDOW_HEIGHT_COMPACT } from '../../constants';
 import { createUnexpectedCommitError } from '../../lib/errors';
 import Actions from './actions.vue';
 import CommitMessageField from './message_field.vue';
 import SuccessMessage from './success_message.vue';
 
-const MSG_CANNOT_PUSH_CODE = s__(
-  'WebIDE|You need permission to edit files directly in this project.',
-);
-
 export default {
   components: {
     Actions,
@@ -35,14 +31,14 @@ export default {
   computed: {
     ...mapState(['changedFiles', 'stagedFiles', 'currentActivityView', 'lastCommitMsg']),
     ...mapState('commit', ['commitMessage', 'submitCommitLoading', 'commitError']),
-    ...mapGetters(['someUncommittedChanges', 'canPushCode']),
+    ...mapGetters(['someUncommittedChanges', 'canPushCodeStatus']),
     ...mapGetters('commit', ['discardDraftButtonDisabled', 'preBuiltCommitMessage']),
     commitButtonDisabled() {
-      return !this.canPushCode || !this.someUncommittedChanges;
+      return !this.canPushCodeStatus.isAllowed || !this.someUncommittedChanges;
     },
     commitButtonTooltip() {
-      if (!this.canPushCode) {
-        return MSG_CANNOT_PUSH_CODE;
+      if (!this.canPushCodeStatus.isAllowed) {
+        return this.canPushCodeStatus.messageShort;
       }
 
       return '';
@@ -86,7 +82,7 @@ export default {
     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) {
+      if (!this.canPushCodeStatus.isAllowed) {
         return false;
       }
 
@@ -130,8 +126,6 @@ export default {
       this.componentHeight = null;
     },
   },
-  // Expose for tests
-  MSG_CANNOT_PUSH_CODE,
 };
 </script>
 

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

 <script>
 import { GlAlert, GlButton, GlLoadingIcon } from '@gitlab/ui';
 import { mapActions, mapGetters, mapState } from 'vuex';
-import { __, s__ } from '~/locale';
+import { __ } from '~/locale';
 import {
   WEBIDE_MARK_APP_START,
   WEBIDE_MARK_FILE_FINISH,
@@ -25,10 +25,6 @@ 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,
@@ -63,7 +59,7 @@ export default {
       'loading',
     ]),
     ...mapGetters([
-      'canPushCode',
+      'canPushCodeStatus',
       'activeFile',
       'someUncommittedChanges',
       'isCommitModeActive',
@@ -116,7 +112,6 @@ export default {
       this.loadDeferred = true;
     },
   },
-  MSG_CANNOT_PUSH_CODE,
 };
 </script>
 
@@ -125,8 +120,8 @@ 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 v-if="!canPushCodeStatus.isAllowed" :dismissible="false">{{
+      canPushCodeStatus.message
     }}</gl-alert>
     <error-message v-if="errorMessage" :message="errorMessage" />
     <div class="ide-view flex-grow d-flex">

app/assets/javascripts/ide/constants.js

@@ -15,6 +15,7 @@ export const FILE_VIEW_MODE_PREVIEW = 'preview';
 export const PERMISSION_CREATE_MR = 'createMergeRequestIn';
 export const PERMISSION_READ_MR = 'readMergeRequest';
 export const PERMISSION_PUSH_CODE = 'pushCode';
+export const PUSH_RULE_REJECT_UNSIGNED_COMMITS = 'rejectUnsignedCommits';
 
 // The default permission object to use when the project data isn't available yet.
 // This helps us encapsulate checks like `canPushCode` without requiring an

app/assets/javascripts/ide/messages.js

+import { s__ } from '~/locale';
+
+export 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 const MSG_CANNOT_PUSH_CODE_SHORT = s__(
+  'WebIDE|You need permission to edit files directly in this project.',
+);
+
+export const MSG_CANNOT_PUSH_UNSIGNED = s__(
+  'WebIDE|This project does not accept unsigned commits. You will not be able to commit your changes through the Web IDE.',
+);
+
+export const MSG_CANNOT_PUSH_UNSIGNED_SHORT = s__(
+  'WebIDE|This project does not accept unsigned commits.',
+);

app/assets/javascripts/ide/queries/getUserPermissions.query.graphql

-query getUserPermissions($projectPath: ID!) {
-  project(fullPath: $projectPath) {
-    userPermissions {
-      createMergeRequestIn
-      readMergeRequest
-      pushCode
-    }
-  }
-}

app/assets/javascripts/ide/queries/get_ide_project.query.graphql

+#import "~/ide/queries/ide_project.fragment.graphql"
+
+query getIdeProject($projectPath: ID!) {
+  project(fullPath: $projectPath) {
+    ...IdeProject
+  }
+}

app/assets/javascripts/ide/queries/ide_project.fragment.graphql

+fragment IdeProject on Project {
+  userPermissions {
+    createMergeRequestIn
+    readMergeRequest
+    pushCode
+  }
+}

app/assets/javascripts/ide/services/index.js

+import getIdeProject from 'ee_else_ce/ide/queries/get_ide_project.query.graphql';
 import Api from '~/api';
 import axios from '~/lib/utils/axios_utils';
 import { joinPaths, escapeFileUrl } from '~/lib/utils/url_utility';
-import getUserPermissions from '../queries/getUserPermissions.query.graphql';
 import { query } from './gql';
 
 const fetchApiProjectData = (projectPath) => Api.project(projectPath).then(({ data }) => data);
 
 const fetchGqlProjectData = (projectPath) =>
   query({
-    query: getUserPermissions,
+    query: getIdeProject,
     variables: { projectPath },
   }).then(({ data }) => data.project);
 

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

@@ -7,7 +7,14 @@ import {
   PERMISSION_READ_MR,
   PERMISSION_CREATE_MR,
   PERMISSION_PUSH_CODE,
+  PUSH_RULE_REJECT_UNSIGNED_COMMITS,
 } from '../constants';
+import {
+  MSG_CANNOT_PUSH_CODE,
+  MSG_CANNOT_PUSH_CODE_SHORT,
+  MSG_CANNOT_PUSH_UNSIGNED,
+  MSG_CANNOT_PUSH_UNSIGNED_SHORT,
+} from '../messages';
 import { getChangesCountForFiles, filePathMatches } from './utils';
 
 export const activeFile = (state) => state.openFiles.find((file) => file.active) || null;
@@ -153,14 +160,47 @@ export const getDiffInfo = (state, getters) => (path) => {
 export const findProjectPermissions = (state, getters) => (projectId) =>
   getters.findProject(projectId)?.userPermissions || DEFAULT_PERMISSIONS;
 
+export const findPushRules = (state, getters) => (projectId) =>
+  getters.findProject(projectId)?.pushRules || {};
+
 export const canReadMergeRequests = (state, getters) =>
   Boolean(getters.findProjectPermissions(state.currentProjectId)[PERMISSION_READ_MR]);
 
 export const canCreateMergeRequests = (state, getters) =>
   Boolean(getters.findProjectPermissions(state.currentProjectId)[PERMISSION_CREATE_MR]);
 
-export const canPushCode = (state, getters) =>
-  Boolean(getters.findProjectPermissions(state.currentProjectId)[PERMISSION_PUSH_CODE]);
+/**
+ * Returns an object with `isAllowed` and `message` based on why the user cant push code
+ */
+export const canPushCodeStatus = (state, getters) => {
+  const canPushCode = getters.findProjectPermissions(state.currentProjectId)[PERMISSION_PUSH_CODE];
+  const rejectUnsignedCommits = getters.findPushRules(state.currentProjectId)[
+    PUSH_RULE_REJECT_UNSIGNED_COMMITS
+  ];
+
+  if (rejectUnsignedCommits) {
+    return {
+      isAllowed: false,
+      message: MSG_CANNOT_PUSH_UNSIGNED,
+      messageShort: MSG_CANNOT_PUSH_UNSIGNED_SHORT,
+    };
+  }
+  if (!canPushCode) {
+    return {
+      isAllowed: false,
+      message: MSG_CANNOT_PUSH_CODE,
+      messageShort: MSG_CANNOT_PUSH_CODE_SHORT,
+    };
+  }
+
+  return {
+    isAllowed: true,
+    message: '',
+    messageShort: '',
+  };
+};
+
+export const canPushCode = (state, getters) => getters.canPushCodeStatus.isAllowed;
 
 export const entryExists = (state) => (path) =>
   Boolean(state.entries[path] && !state.entries[path].deleted);

changelogs/unreleased/213581-ide-alert-user-when-reject-unsigned-commits.yml

+---
+title: Web IDE disallow commit when project has 'reject unsigned commits' rule
+merge_request: 54166
+author:
+type: changed

ee/app/assets/javascripts/ide/queries/get_ide_project.query.graphql

+#import "~/ide/queries/ide_project.fragment.graphql"
+
+query getIdeProject($projectPath: ID!) {
+  project(fullPath: $projectPath) {
+    ...IdeProject
+    pushRules {
+      rejectUnsignedCommits
+    }
+  }
+}

ee/spec/features/ide/user_opens_ide_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe 'EE user opens IDE', :js do
+  include WebIdeSpecHelpers
+
+  let_it_be(:unsigned_commits_warning) { 'This project does not accept unsigned commits.' }
+
+  let(:project) { create(:project, :repository) }
+  let(:user) { project.owner }
+
+  before do
+    stub_licensed_features(push_rules: true)
+    stub_licensed_features(reject_unsigned_commits: true)
+    sign_in(user)
+  end
+
+  context 'default' do
+    before do
+      ide_visit(project)
+    end
+
+    it 'does not show warning' do
+      expect(page).not_to have_text(unsigned_commits_warning)
+    end
+  end
+
+  context 'when has reject_unsigned_commit push rule' do
+    before do
+      create(:push_rule, project: project, reject_unsigned_commits: true)
+
+      ide_visit(project)
+    end
+
+    it 'shows warning' do
+      expect(page).to have_text(unsigned_commits_warning)
+    end
+  end
+end

locale/gitlab.pot

@@ -33691,6 +33691,12 @@ msgstr ""
 msgid "WebIDE|Merge request"
 msgstr ""
 
+msgid "WebIDE|This project does not accept unsigned commits."
+msgstr ""
+
+msgid "WebIDE|This project does not accept unsigned commits. You will not be able to commit your changes through the Web IDE."
+msgstr ""
+
 msgid "WebIDE|You need permission to edit files directly in this project."
 msgstr ""
 

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

@@ -14,6 +14,7 @@ import {
   createBranchChangedCommitError,
   branchAlreadyExistsCommitError,
 } from '~/ide/lib/errors';
+import { MSG_CANNOT_PUSH_CODE_SHORT } from '~/ide/messages';
 import { createStore } from '~/ide/stores';
 import { COMMIT_TO_NEW_BRANCH } from '~/ide/stores/modules/commit/constants';
 
@@ -84,8 +85,8 @@ describe('IDE commit form', () => {
     ${'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}
+    ${'when user cannot push'}     | ${['test']} | ${{ pushCode: false }} | ${goToEditView}   | ${findBeginCommitButtonData} | ${true}  | ${MSG_CANNOT_PUSH_CODE_SHORT}
+    ${'when user cannot push'}     | ${['test']} | ${{ pushCode: false }} | ${goToCommitView} | ${findCommitButtonData}      | ${true}  | ${MSG_CANNOT_PUSH_CODE_SHORT}
   `('$desc', ({ stagedFiles, userPermissions, viewFn, buttonFn, disabled, tooltip }) => {
     beforeEach(async () => {
       store.state.stagedFiles = stagedFiles;

spec/frontend/ide/components/ide_spec.js

@@ -4,6 +4,7 @@ import Vuex from 'vuex';
 import waitForPromises from 'helpers/wait_for_promises';
 import ErrorMessage from '~/ide/components/error_message.vue';
 import Ide from '~/ide/components/ide.vue';
+import { MSG_CANNOT_PUSH_CODE } from '~/ide/messages';
 import { createStore } from '~/ide/stores';
 import { file } from '../helpers';
 import { projectData } from '../mock_data';
@@ -158,7 +159,7 @@ describe('WebIDE', () => {
     expect(findAlert().props()).toMatchObject({
       dismissible: false,
     });
-    expect(findAlert().text()).toBe(Ide.MSG_CANNOT_PUSH_CODE);
+    expect(findAlert().text()).toBe(MSG_CANNOT_PUSH_CODE);
   });
 
   it.each`

spec/frontend/ide/services/index_spec.js

 import axios from 'axios';
 import MockAdapter from 'axios-mock-adapter';
+import getIdeProject from 'ee_else_ce/ide/queries/get_ide_project.query.graphql';
 import Api from '~/api';
-import getUserPermissions from '~/ide/queries/getUserPermissions.query.graphql';
 import services from '~/ide/services';
 import { query } from '~/ide/services/gql';
 import { escapeFileUrl } from '~/lib/utils/url_utility';
@@ -228,7 +228,7 @@ describe('IDE services', () => {
         expect(response).toEqual({ data: { ...projectData, ...gqlProjectData } });
         expect(Api.project).toHaveBeenCalledWith(TEST_PROJECT_ID);
         expect(query).toHaveBeenCalledWith({
-          query: getUserPermissions,
+          query: getIdeProject,
           variables: {
             projectPath: TEST_PROJECT_ID,
           },

spec/frontend/ide/stores/getters_spec.js

 import { TEST_HOST } from 'helpers/test_constants';
+import {
+  DEFAULT_PERMISSIONS,
+  PERMISSION_PUSH_CODE,
+  PUSH_RULE_REJECT_UNSIGNED_COMMITS,
+} from '~/ide/constants';
+import {
+  MSG_CANNOT_PUSH_CODE,
+  MSG_CANNOT_PUSH_CODE_SHORT,
+  MSG_CANNOT_PUSH_UNSIGNED,
+  MSG_CANNOT_PUSH_UNSIGNED_SHORT,
+} from '~/ide/messages';
 import { createStore } from '~/ide/stores';
 import * as getters from '~/ide/stores/getters';
-import { DEFAULT_PERMISSIONS } from '../../../../app/assets/javascripts/ide/constants';
 import { file } from '../helpers';
 
 const TEST_PROJECT_ID = 'test_project';
@@ -385,22 +395,23 @@ describe('IDE store getters', () => {
     );
   });
 
-  describe('findProjectPermissions', () => {
-    it('returns false if project not found', () => {
-      expect(localStore.getters.findProjectPermissions(TEST_PROJECT_ID)).toEqual(
-        DEFAULT_PERMISSIONS,
-      );
+  describe.each`
+    getterName                  | projectField         | defaultValue
+    ${'findProjectPermissions'} | ${'userPermissions'} | ${DEFAULT_PERMISSIONS}
+    ${'findPushRules'}          | ${'pushRules'}       | ${{}}
+  `('$getterName', ({ getterName, projectField, defaultValue }) => {
+    const callGetter = (...args) => localStore.getters[getterName](...args);
+
+    it('returns default if project not found', () => {
+      expect(callGetter(TEST_PROJECT_ID)).toEqual(defaultValue);
     });
 
-    it('finds permission in given project', () => {
-      const userPermissions = {
-        readMergeRequest: true,
-        createMergeRequestsIn: false,
-      };
+    it('finds field in given project', () => {
+      const obj = { test: 'foo' };
 
-      localState.projects[TEST_PROJECT_ID] = { userPermissions };
+      localState.projects[TEST_PROJECT_ID] = { [projectField]: obj };
 
-      expect(localStore.getters.findProjectPermissions(TEST_PROJECT_ID)).toBe(userPermissions);
+      expect(callGetter(TEST_PROJECT_ID)).toBe(obj);
     });
   });
 
@@ -408,7 +419,6 @@ describe('IDE store getters', () => {
     getterName                  | permissionKey
     ${'canReadMergeRequests'}   | ${'readMergeRequest'}
     ${'canCreateMergeRequests'} | ${'createMergeRequestIn'}
-    ${'canPushCode'}            | ${'pushCode'}
   `('$getterName', ({ getterName, permissionKey }) => {
     it.each([true, false])('finds permission for current project (%s)', (val) => {
       localState.projects[TEST_PROJECT_ID] = {
@@ -422,6 +432,38 @@ describe('IDE store getters', () => {
     });
   });
 
+  describe('canPushCodeStatus', () => {
+    it.each`
+      pushCode | rejectUnsignedCommits | expected
+      ${true}  | ${false}              | ${{ isAllowed: true, message: '', messageShort: '' }}
+      ${false} | ${false}              | ${{ isAllowed: false, message: MSG_CANNOT_PUSH_CODE, messageShort: MSG_CANNOT_PUSH_CODE_SHORT }}
+      ${false} | ${true}               | ${{ isAllowed: false, message: MSG_CANNOT_PUSH_UNSIGNED, messageShort: MSG_CANNOT_PUSH_UNSIGNED_SHORT }}
+    `(
+      'with pushCode="$pushCode" and rejectUnsignedCommits="$rejectUnsignedCommits"',
+      ({ pushCode, rejectUnsignedCommits, expected }) => {
+        localState.projects[TEST_PROJECT_ID] = {
+          pushRules: {
+            [PUSH_RULE_REJECT_UNSIGNED_COMMITS]: rejectUnsignedCommits,
+          },
+          userPermissions: {
+            [PERMISSION_PUSH_CODE]: pushCode,
+          },
+        };
+        localState.currentProjectId = TEST_PROJECT_ID;
+
+        expect(localStore.getters.canPushCodeStatus).toEqual(expected);
+      },
+    );
+  });
+
+  describe('canPushCode', () => {
+    it.each([true, false])('with canPushCodeStatus.isAllowed = $s', (isAllowed) => {
+      const canPushCodeStatus = { isAllowed };
+
+      expect(getters.canPushCode({}, { canPushCodeStatus })).toBe(isAllowed);
+    });
+  });
+
   describe('entryExists', () => {
     beforeEach(() => {
       localState.entries = {