From a38adf96ad74808d6280d16ff78f4b93bc148bb6 Mon Sep 17 00:00:00 2001
From: Alexander Turinske <aturinske@gitlab.com>
Date: Wed, 13 Nov 2024 20:37:13 -0800
Subject: [PATCH 1/3] Update error handling of subscription
- previously the error part of graphql never was triggered
and was instead handled by errorMessages
- now error is triggered for some issues
- errorMessages has been deprecated for a more standard
errors
- update tests
Changelog: changed
EE: true
---
.../policy_editor/editor_wrapper.vue | 8 ++--
...olicy_project_created.subscription.graphql | 2 +-
.../policy_editor/editor_wrapper_spec.js | 38 +++++++++++++------
3 files changed, 31 insertions(+), 17 deletions(-)
diff --git a/ee/app/assets/javascripts/security_orchestration/components/policy_editor/editor_wrapper.vue b/ee/app/assets/javascripts/security_orchestration/components/policy_editor/editor_wrapper.vue
index eea25c3c87d0ba..045dc519fe6bcb 100644
--- a/ee/app/assets/javascripts/security_orchestration/components/policy_editor/editor_wrapper.vue
+++ b/ee/app/assets/javascripts/security_orchestration/components/policy_editor/editor_wrapper.vue
@@ -26,10 +26,10 @@ export default {
if (!securityPolicyProjectCreated) return;
const project = securityPolicyProjectCreated?.project;
- const e = securityPolicyProjectCreated?.errorMessage;
+ const e = securityPolicyProjectCreated?.errors;
- if (e) {
- this.setError(e);
+ if (e.length) {
+ this.setError(e.join('\n'));
this.setLoadingFlag(false);
}
@@ -41,7 +41,7 @@ export default {
}
},
error(e) {
- this.setError(e);
+ this.setError(e.message);
this.setLoadingFlag(false);
},
},
diff --git a/ee/app/assets/javascripts/security_orchestration/graphql/queries/security_policy_project_created.subscription.graphql b/ee/app/assets/javascripts/security_orchestration/graphql/queries/security_policy_project_created.subscription.graphql
index 96ea082b3b11d4..0d2cd08dc53144 100644
--- a/ee/app/assets/javascripts/security_orchestration/graphql/queries/security_policy_project_created.subscription.graphql
+++ b/ee/app/assets/javascripts/security_orchestration/graphql/queries/security_policy_project_created.subscription.graphql
@@ -9,6 +9,6 @@ subscription getSecurityPolicyProject($fullPath: String!) {
}
}
status
- errorMessage
+ errors
}
}
diff --git a/ee/spec/frontend/security_orchestration/components/policy_editor/editor_wrapper_spec.js b/ee/spec/frontend/security_orchestration/components/policy_editor/editor_wrapper_spec.js
index fe4da433c05665..05ed4f57faf3c9 100644
--- a/ee/spec/frontend/security_orchestration/components/policy_editor/editor_wrapper_spec.js
+++ b/ee/spec/frontend/security_orchestration/components/policy_editor/editor_wrapper_spec.js
@@ -30,16 +30,20 @@ Vue.use(VueApollo);
describe('EditorWrapper component', () => {
let wrapper;
- const getSecurityPolicyProjectSubscriptionErrorHandlerMock = jest.fn().mockResolvedValue({
+ const getSecurityPolicyProjectSubscriptionErrorAsDataHandlerMock = jest.fn().mockResolvedValue({
data: {
securityPolicyProjectCreated: {
project: null,
status: null,
- errorMessage: 'error',
+ errors: ['There was an error', 'error reason'],
},
},
});
+ const getSecurityPolicyProjectSubscriptionErrorHandlerMock = jest
+ .fn()
+ .mockRejectedValue({ message: 'error' });
+
const getSecurityPolicyProjectSubscriptionHandlerMock = jest.fn().mockResolvedValue({
data: {
securityPolicyProjectCreated: {
@@ -52,7 +56,7 @@ describe('EditorWrapper component', () => {
},
},
status: null,
- errorMessage: '',
+ errors: [],
},
},
});
@@ -177,16 +181,26 @@ describe('EditorWrapper component', () => {
expect(goToPolicyMR).not.toHaveBeenCalled();
});
- it('passes the errors when the the subscription fails with a project', async () => {
+ it('shows the errors when the subscription fails to create due to an SPP already existing with the same name, but not linked', async () => {
factory({
- subscriptionMock: getSecurityPolicyProjectSubscriptionErrorHandlerMock,
+ subscriptionMock: getSecurityPolicyProjectSubscriptionErrorAsDataHandlerMock,
});
await waitForPromises();
- expect(findScanExecutionPolicyEditor().props('assignedPolicyProject')).toEqual({
- branch: '',
- fullPath: '',
+ const alert = findErrorAlert();
+ expect(alert.exists()).toBe(true);
+ expect(alert.props('title')).toBe('There was an error');
+ expect(alert.text()).toBe('error reason');
+ });
+
+ it('shows the errors when the subscription fails due to a configuration issue', async () => {
+ factory({
+ subscriptionMock: getSecurityPolicyProjectSubscriptionErrorHandlerMock,
});
- expect(findScanExecutionPolicyEditor().props('errorSources')).toEqual([]);
+ await waitForPromises();
+ const alert = findErrorAlert();
+ expect(alert.exists()).toBe(true);
+ expect(alert.props('title')).toBe('error');
+ expect(alert.text()).toBe('');
});
});
@@ -194,7 +208,7 @@ describe('EditorWrapper component', () => {
describe('without an assigned policy project', () => {
it('does not make the request to create the MR without an assigned policy project', async () => {
await factory({
- subscriptionMock: getSecurityPolicyProjectSubscriptionErrorHandlerMock,
+ subscriptionMock: getSecurityPolicyProjectSubscriptionErrorAsDataHandlerMock,
});
findScanExecutionPolicyEditor().vm.$emit('save', {
action: SECURITY_POLICY_ACTIONS.APPEND,
@@ -213,7 +227,7 @@ describe('EditorWrapper component', () => {
`('makes the request to "goToPolicyMR" $status', async ({ action }) => {
factory({
provide: { assignedPolicyProject: existingAssignedPolicyProject },
- subscriptionMock: getSecurityPolicyProjectSubscriptionErrorHandlerMock,
+ subscriptionMock: getSecurityPolicyProjectSubscriptionErrorAsDataHandlerMock,
});
findScanExecutionPolicyEditor().vm.$emit('save', {
action,
@@ -236,7 +250,7 @@ describe('EditorWrapper component', () => {
it('passes extra merge request input to goToPolicyMR', async () => {
factory({
provide: { assignedPolicyProject: existingAssignedPolicyProject },
- subscriptionMock: getSecurityPolicyProjectSubscriptionErrorHandlerMock,
+ subscriptionMock: getSecurityPolicyProjectSubscriptionErrorAsDataHandlerMock,
});
const extraMergeRequestInput = {
title: 'test',
--
GitLab
From 2dd802229914381ae1ac88445a57d7033137c7b8 Mon Sep 17 00:00:00 2001
From: Alexander Turinske <aturinske@gitlab.com>
Date: Tue, 19 Nov 2024 14:53:05 -0800
Subject: [PATCH 2/3] Update integration tests for new api
- update errorMessages to errors
- update code to be more readable
---
.../components/policy_editor/editor_wrapper.vue | 7 +++----
.../security_orchestration/policy_editor/utils.js | 2 +-
2 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/ee/app/assets/javascripts/security_orchestration/components/policy_editor/editor_wrapper.vue b/ee/app/assets/javascripts/security_orchestration/components/policy_editor/editor_wrapper.vue
index 045dc519fe6bcb..49abc97074a152 100644
--- a/ee/app/assets/javascripts/security_orchestration/components/policy_editor/editor_wrapper.vue
+++ b/ee/app/assets/javascripts/security_orchestration/components/policy_editor/editor_wrapper.vue
@@ -25,11 +25,10 @@ export default {
result({ data: { securityPolicyProjectCreated } }) {
if (!securityPolicyProjectCreated) return;
- const project = securityPolicyProjectCreated?.project;
- const e = securityPolicyProjectCreated?.errors;
+ const { project, errors } = securityPolicyProjectCreated;
- if (e.length) {
- this.setError(e.join('\n'));
+ if (errors.length) {
+ this.setError(errors.join('\n'));
this.setLoadingFlag(false);
}
diff --git a/ee/spec/frontend_integration/security_orchestration/policy_editor/utils.js b/ee/spec/frontend_integration/security_orchestration/policy_editor/utils.js
index 6995252cf8b40c..04e7d4d429257e 100644
--- a/ee/spec/frontend_integration/security_orchestration/policy_editor/utils.js
+++ b/ee/spec/frontend_integration/security_orchestration/policy_editor/utils.js
@@ -43,7 +43,7 @@ export const createSppSubscriptionHandler = () =>
},
},
status: null,
- errorMessage: '',
+ errors: [],
},
},
});
--
GitLab
From 09030197bc000f4433b4fa0785a5440f73a7562a Mon Sep 17 00:00:00 2001
From: Alexander Turinske <aturinske@gitlab.com>
Date: Thu, 21 Nov 2024 08:46:32 -0800
Subject: [PATCH 3/3] Quarantine test that fails in vue 3
- skip failing test due to vue 3 behavior
---
.../policy_editor/editor_wrapper_spec.js | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/ee/spec/frontend/security_orchestration/components/policy_editor/editor_wrapper_spec.js b/ee/spec/frontend/security_orchestration/components/policy_editor/editor_wrapper_spec.js
index 05ed4f57faf3c9..2d16995501470e 100644
--- a/ee/spec/frontend/security_orchestration/components/policy_editor/editor_wrapper_spec.js
+++ b/ee/spec/frontend/security_orchestration/components/policy_editor/editor_wrapper_spec.js
@@ -40,10 +40,6 @@ describe('EditorWrapper component', () => {
},
});
- const getSecurityPolicyProjectSubscriptionErrorHandlerMock = jest
- .fn()
- .mockRejectedValue({ message: 'error' });
-
const getSecurityPolicyProjectSubscriptionHandlerMock = jest.fn().mockResolvedValue({
data: {
securityPolicyProjectCreated: {
@@ -192,7 +188,16 @@ describe('EditorWrapper component', () => {
expect(alert.text()).toBe('error reason');
});
- it('shows the errors when the subscription fails due to a configuration issue', async () => {
+ /**
+ * This fails in VUE_VERSION=3..
+ * See https://gitlab.com/gitlab-org/gitlab/-/merge_requests/172643#note_2220742867
+ */
+ // quarantine: https://gitlab.com/gitlab-org/gitlab/-/issues/458409
+ // eslint-disable-next-line jest/no-disabled-tests
+ it.skip('shows the errors when the subscription fails due to a configuration issue', async () => {
+ const getSecurityPolicyProjectSubscriptionErrorHandlerMock = jest
+ .fn()
+ .mockRejectedValue({ message: 'error' });
factory({
subscriptionMock: getSecurityPolicyProjectSubscriptionErrorHandlerMock,
});
--
GitLab