{{
- $options.MSG_CANNOT_PUSH_CODE
+ {{
+ canPushCodeStatus.message
}}
diff --git a/app/assets/javascripts/ide/constants.js b/app/assets/javascripts/ide/constants.js
index ed6b750480b2aa5cb8b79b93f9feb359fd45cee5..056df3739eec91961acd02240f19b0e1a00c4917 100644
--- a/app/assets/javascripts/ide/constants.js
+++ b/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
diff --git a/app/assets/javascripts/ide/messages.js b/app/assets/javascripts/ide/messages.js
new file mode 100644
index 0000000000000000000000000000000000000000..4298d4c627c9189b49e9ecd47fccd2af40c13427
--- /dev/null
+++ b/app/assets/javascripts/ide/messages.js
@@ -0,0 +1,17 @@
+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.',
+);
diff --git a/app/assets/javascripts/ide/queries/getUserPermissions.query.graphql b/app/assets/javascripts/ide/queries/getUserPermissions.query.graphql
deleted file mode 100644
index f0b50793226ef2c9bf7f1d5a7e9726432f1f9e7b..0000000000000000000000000000000000000000
--- a/app/assets/javascripts/ide/queries/getUserPermissions.query.graphql
+++ /dev/null
@@ -1,9 +0,0 @@
-query getUserPermissions($projectPath: ID!) {
- project(fullPath: $projectPath) {
- userPermissions {
- createMergeRequestIn
- readMergeRequest
- pushCode
- }
- }
-}
diff --git a/app/assets/javascripts/ide/queries/get_ide_project.query.graphql b/app/assets/javascripts/ide/queries/get_ide_project.query.graphql
new file mode 100644
index 0000000000000000000000000000000000000000..6ceb36909f3d4be585ac414fdf7a4c538457e96f
--- /dev/null
+++ b/app/assets/javascripts/ide/queries/get_ide_project.query.graphql
@@ -0,0 +1,7 @@
+#import "~/ide/queries/ide_project.fragment.graphql"
+
+query getIdeProject($projectPath: ID!) {
+ project(fullPath: $projectPath) {
+ ...IdeProject
+ }
+}
diff --git a/app/assets/javascripts/ide/queries/ide_project.fragment.graphql b/app/assets/javascripts/ide/queries/ide_project.fragment.graphql
new file mode 100644
index 0000000000000000000000000000000000000000..c107f2376f9165fd5973d1e2415e1db580348b73
--- /dev/null
+++ b/app/assets/javascripts/ide/queries/ide_project.fragment.graphql
@@ -0,0 +1,7 @@
+fragment IdeProject on Project {
+ userPermissions {
+ createMergeRequestIn
+ readMergeRequest
+ pushCode
+ }
+}
diff --git a/app/assets/javascripts/ide/services/index.js b/app/assets/javascripts/ide/services/index.js
index e98653aedc2f4cb5091e048fd1507d5b39282a13..0aa08323d13ba99abb6e6ac09e849e0e48c42199 100644
--- a/app/assets/javascripts/ide/services/index.js
+++ b/app/assets/javascripts/ide/services/index.js
@@ -1,14 +1,14 @@
+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);
diff --git a/app/assets/javascripts/ide/stores/getters.js b/app/assets/javascripts/ide/stores/getters.js
index 9b93fc74f761fdf4e411d4a82889f0c53078b911..a5bb32ec44a02e180bb146f3ca185217a640e853 100644
--- a/app/assets/javascripts/ide/stores/getters.js
+++ b/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);
diff --git a/changelogs/unreleased/213581-ide-alert-user-when-reject-unsigned-commits.yml b/changelogs/unreleased/213581-ide-alert-user-when-reject-unsigned-commits.yml
new file mode 100644
index 0000000000000000000000000000000000000000..62ba5d7828d0f6aab2e9641e19b3e9d810f5d320
--- /dev/null
+++ b/changelogs/unreleased/213581-ide-alert-user-when-reject-unsigned-commits.yml
@@ -0,0 +1,5 @@
+---
+title: Web IDE disallow commit when project has 'reject unsigned commits' rule
+merge_request: 54166
+author:
+type: changed
diff --git a/ee/app/assets/javascripts/ide/queries/get_ide_project.query.graphql b/ee/app/assets/javascripts/ide/queries/get_ide_project.query.graphql
new file mode 100644
index 0000000000000000000000000000000000000000..41fefa9240ccc85bf08719b7bdab0bee06433e85
--- /dev/null
+++ b/ee/app/assets/javascripts/ide/queries/get_ide_project.query.graphql
@@ -0,0 +1,10 @@
+#import "~/ide/queries/ide_project.fragment.graphql"
+
+query getIdeProject($projectPath: ID!) {
+ project(fullPath: $projectPath) {
+ ...IdeProject
+ pushRules {
+ rejectUnsignedCommits
+ }
+ }
+}
diff --git a/ee/spec/features/ide/user_opens_ide_spec.rb b/ee/spec/features/ide/user_opens_ide_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..1ecaa6eadb529ebac6ee7849ee1baffd9760a59d
--- /dev/null
+++ b/ee/spec/features/ide/user_opens_ide_spec.rb
@@ -0,0 +1,40 @@
+# 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
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 9d3e6adb99bf497533508dde07b67c7e9b66e7f6..3b9ccefa390590ff7946a42620e3f1f6226539e1 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -33622,6 +33622,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 ""
diff --git a/spec/frontend/ide/components/commit_sidebar/form_spec.js b/spec/frontend/ide/components/commit_sidebar/form_spec.js
index 2b567816ce895cab75971aa3b637985af7ad3509..083a2a73b2459576250b98b3b9db88e3f79ca685 100644
--- a/spec/frontend/ide/components/commit_sidebar/form_spec.js
+++ b/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;
diff --git a/spec/frontend/ide/components/ide_spec.js b/spec/frontend/ide/components/ide_spec.js
index c9d19c18d0398d5c1b950d11feea1c5768b69d64..bd251f78654484b3accb8d856ece8736844d912c 100644
--- a/spec/frontend/ide/components/ide_spec.js
+++ b/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`
diff --git a/spec/frontend/ide/services/index_spec.js b/spec/frontend/ide/services/index_spec.js
index 678d58cba349032b9f1bee6afd42d9355e749a02..3503834e24b764738b5f7fbb993c723afe018d1d 100644
--- a/spec/frontend/ide/services/index_spec.js
+++ b/spec/frontend/ide/services/index_spec.js
@@ -1,7 +1,7 @@
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,
},
diff --git a/spec/frontend/ide/stores/getters_spec.js b/spec/frontend/ide/stores/getters_spec.js
index 450f559202665ccaa3d5ae833571ea3c07379cf1..6b66c87e205a08d2780a9f5b84836c724a19cd7e 100644
--- a/spec/frontend/ide/stores/getters_spec.js
+++ b/spec/frontend/ide/stores/getters_spec.js
@@ -1,7 +1,17 @@
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 = {