diff --git a/app/assets/javascripts/lib/utils/error_message.js b/app/assets/javascripts/lib/utils/error_message.js new file mode 100644 index 0000000000000000000000000000000000000000..4cea4257e7b6ee276719fa9238fc6bd03767a0d5 --- /dev/null +++ b/app/assets/javascripts/lib/utils/error_message.js @@ -0,0 +1,20 @@ +export const USER_FACING_ERROR_MESSAGE_PREFIX = 'UF:'; + +const getMessageFromError = (error = '') => { + return error.message || error; +}; + +export const parseErrorMessage = (error = '') => { + const messageString = getMessageFromError(error); + + if (messageString.startsWith(USER_FACING_ERROR_MESSAGE_PREFIX)) { + return { + message: messageString.replace(USER_FACING_ERROR_MESSAGE_PREFIX, '').trim(), + userFacing: true, + }; + } + return { + message: messageString, + userFacing: false, + }; +}; diff --git a/app/assets/javascripts/security_configuration/components/app.vue b/app/assets/javascripts/security_configuration/components/app.vue index e96f71981e5e20f1a5afa238989a7a1765a46244..ccfaa678201a0d91d2285d3d5ba1e5c90c1961c4 100644 --- a/app/assets/javascripts/security_configuration/components/app.vue +++ b/app/assets/javascripts/security_configuration/components/app.vue @@ -1,6 +1,7 @@ @@ -111,8 +127,9 @@ export default { variant="danger" :dismissible="false" data-testid="error-alert" - >{{ $options.i18n.loadingErrorText }} + + diff --git a/ee/spec/frontend/security_configuration/sast/components/app_spec.js b/ee/spec/frontend/security_configuration/sast/components/app_spec.js index aa46dda52284cc001bc14063a440b0f276f6e7bd..9be3820fae80a450459304132038f79db6372360 100644 --- a/ee/spec/frontend/security_configuration/sast/components/app_spec.js +++ b/ee/spec/frontend/security_configuration/sast/components/app_spec.js @@ -3,13 +3,15 @@ import { merge } from 'lodash'; import Vue from 'vue'; import VueApollo from 'vue-apollo'; import ConfigurationPageLayout from 'ee/security_configuration/components/configuration_page_layout.vue'; -import SASTConfigurationApp from 'ee/security_configuration/sast/components/app.vue'; +import SASTConfigurationApp, { i18n } from 'ee/security_configuration/sast/components/app.vue'; import ConfigurationForm from 'ee/security_configuration/sast/components/configuration_form.vue'; import sastCiConfigurationQuery from 'ee/security_configuration/sast/graphql/sast_ci_configuration.query.graphql'; import createMockApollo from 'helpers/mock_apollo_helper'; import waitForPromises from 'helpers/wait_for_promises'; import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; +import { USER_FACING_ERROR_MESSAGE_PREFIX } from '~/lib/utils/error_message'; import { sastCiConfigurationQueryResponse } from '../mock_data'; +import { specificErrorMessage, technicalErrorMessage } from '../constants'; Vue.use(VueApollo); @@ -20,8 +22,14 @@ describe('SAST Configuration App', () => { let wrapper; const pendingHandler = () => new Promise(() => {}); - const successHandler = async () => sastCiConfigurationQueryResponse; - const failureHandler = async () => ({ errors: [{ message: 'some error' }] }); + const successHandler = () => sastCiConfigurationQueryResponse; + // Prefixed with USER_FACING_ERROR_MESSAGE_PREFIX as used in lib/gitlab/utils/error_message.rb to indicate a user facing error + const failureHandlerSpecific = () => ({ + errors: [{ message: `${USER_FACING_ERROR_MESSAGE_PREFIX} ${specificErrorMessage}` }], + }); + const failureHandlerGeneric = async () => ({ + errors: [{ message: technicalErrorMessage }], + }); const createMockApolloProvider = (handler) => createMockApollo([[sastCiConfigurationQuery, handler]]); @@ -108,10 +116,10 @@ describe('SAST Configuration App', () => { }); }); - describe('when loading failed', () => { + describe('when loading failed with Error Message including user facing keyword', () => { beforeEach(() => { createComponent({ - apolloProvider: createMockApolloProvider(failureHandler), + apolloProvider: createMockApolloProvider(failureHandlerSpecific), }); return waitForPromises(); }); @@ -127,6 +135,25 @@ describe('SAST Configuration App', () => { it('displays an alert message', () => { expect(findErrorAlert().exists()).toBe(true); }); + + it('shows specific error message without keyword when defined', () => { + expect(findErrorAlert().exists()).toBe(true); + expect(findErrorAlert().text()).toContain('some specific error'); + }); + }); + + describe('when loading failed with Error Message without user facing keyword', () => { + beforeEach(() => { + createComponent({ + apolloProvider: createMockApolloProvider(failureHandlerGeneric), + }); + return waitForPromises(); + }); + + it('shows generic error message when no specific message is defined', () => { + expect(findErrorAlert().exists()).toBe(true); + expect(findErrorAlert().text()).toContain(i18n.genericErrorText); + }); }); describe('when loaded', () => { diff --git a/ee/spec/frontend/security_configuration/sast/constants.js b/ee/spec/frontend/security_configuration/sast/constants.js new file mode 100644 index 0000000000000000000000000000000000000000..f9754b94950312ce15e48a398887f5261699e7a8 --- /dev/null +++ b/ee/spec/frontend/security_configuration/sast/constants.js @@ -0,0 +1,2 @@ +export const specificErrorMessage = 'some specific error'; +export const technicalErrorMessage = 'some non-userfacing technical error'; diff --git a/lib/gitlab/utils/error_message.rb b/lib/gitlab/utils/error_message.rb new file mode 100644 index 0000000000000000000000000000000000000000..e9c6f8a58472a7b2a305a4ab7255c6098f40bd79 --- /dev/null +++ b/lib/gitlab/utils/error_message.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module Gitlab + module Utils + module ErrorMessage + extend self + + def to_user_facing(message) + "UF: #{message}" + end + end + end +end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index cbf42c8e9b352169075d7f41a9583939d81bf4d4..f486a52479ae2bc504f7766596e88e950bb9d240 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -38848,6 +38848,9 @@ msgstr "" msgid "SecurityConfiguration|Security training" msgstr "" +msgid "SecurityConfiguration|Something went wrong. Please refresh the page, or try again later." +msgstr "" + msgid "SecurityConfiguration|The status of the tools only applies to the default branch and is based on the %{linkStart}latest pipeline%{linkEnd}." msgstr "" diff --git a/spec/frontend/lib/utils/error_message_spec.js b/spec/frontend/lib/utils/error_message_spec.js new file mode 100644 index 0000000000000000000000000000000000000000..17b5168c32f417403f7e2466847430f0845c25c5 --- /dev/null +++ b/spec/frontend/lib/utils/error_message_spec.js @@ -0,0 +1,65 @@ +import { parseErrorMessage, USER_FACING_ERROR_MESSAGE_PREFIX } from '~/lib/utils/error_message'; + +const defaultErrorMessage = 'Something caused this error'; +const userFacingErrorMessage = 'User facing error message'; +const nonUserFacingErrorMessage = 'NonUser facing error message'; +const genericErrorMessage = 'Some error message'; + +describe('error message', () => { + describe('when given an errormessage object', () => { + const errorMessageObject = { + options: { + cause: defaultErrorMessage, + }, + filename: 'error.js', + linenumber: 7, + }; + + it('returns the correct values for userfacing errors', () => { + const userFacingObject = errorMessageObject; + userFacingObject.message = `${USER_FACING_ERROR_MESSAGE_PREFIX} ${userFacingErrorMessage}`; + + expect(parseErrorMessage(userFacingObject)).toEqual({ + message: userFacingErrorMessage, + userFacing: true, + }); + }); + + it('returns the correct values for non userfacing errors', () => { + const nonUserFacingObject = errorMessageObject; + nonUserFacingObject.message = nonUserFacingErrorMessage; + + expect(parseErrorMessage(nonUserFacingObject)).toEqual({ + message: nonUserFacingErrorMessage, + userFacing: false, + }); + }); + }); + + describe('when given an errormessage string', () => { + it('returns the correct values for userfacing errors', () => { + expect( + parseErrorMessage(`${USER_FACING_ERROR_MESSAGE_PREFIX} ${genericErrorMessage}`), + ).toEqual({ + message: genericErrorMessage, + userFacing: true, + }); + }); + + it('returns the correct values for non userfacing errors', () => { + expect(parseErrorMessage(genericErrorMessage)).toEqual({ + message: genericErrorMessage, + userFacing: false, + }); + }); + }); + + describe('when given nothing', () => { + it('returns an empty error message', () => { + expect(parseErrorMessage()).toEqual({ + message: '', + userFacing: false, + }); + }); + }); +}); diff --git a/spec/frontend/security_configuration/components/app_spec.js b/spec/frontend/security_configuration/components/app_spec.js index 5ef387adf39aa62fb16bb5763a13a944c168f2f1..0ca350f9ed7cb889897845f1597d3425678c2205 100644 --- a/spec/frontend/security_configuration/components/app_spec.js +++ b/spec/frontend/security_configuration/components/app_spec.js @@ -26,6 +26,8 @@ import { REPORT_TYPE_LICENSE_COMPLIANCE, REPORT_TYPE_SAST, } from '~/vue_shared/security_reports/constants'; +import { USER_FACING_ERROR_MESSAGE_PREFIX } from '~/lib/utils/error_message'; +import { manageViaMRErrorMessage } from '../constants'; const upgradePath = '/upgrade'; const autoDevopsHelpPagePath = '/autoDevopsHelpPagePath'; @@ -200,18 +202,21 @@ describe('App component', () => { }); }); - describe('when error occurs', () => { + describe('when user facing error occurs', () => { it('should show Alert with error Message', async () => { expect(findManageViaMRErrorAlert().exists()).toBe(false); - findFeatureCards().at(1).vm.$emit('error', 'There was a manage via MR error'); + // Prefixed with USER_FACING_ERROR_MESSAGE_PREFIX as used in lib/gitlab/utils/error_message.rb to indicate a user facing error + findFeatureCards() + .at(1) + .vm.$emit('error', `${USER_FACING_ERROR_MESSAGE_PREFIX} ${manageViaMRErrorMessage}`); await nextTick(); expect(findManageViaMRErrorAlert().exists()).toBe(true); - expect(findManageViaMRErrorAlert().text()).toEqual('There was a manage via MR error'); + expect(findManageViaMRErrorAlert().text()).toEqual(manageViaMRErrorMessage); }); it('should hide Alert when it is dismissed', async () => { - findFeatureCards().at(1).vm.$emit('error', 'There was a manage via MR error'); + findFeatureCards().at(1).vm.$emit('error', manageViaMRErrorMessage); await nextTick(); expect(findManageViaMRErrorAlert().exists()).toBe(true); @@ -221,6 +226,17 @@ describe('App component', () => { expect(findManageViaMRErrorAlert().exists()).toBe(false); }); }); + + describe('when non-user facing error occurs', () => { + it('should show Alert with generic error Message', async () => { + expect(findManageViaMRErrorAlert().exists()).toBe(false); + findFeatureCards().at(1).vm.$emit('error', manageViaMRErrorMessage); + + await nextTick(); + expect(findManageViaMRErrorAlert().exists()).toBe(true); + expect(findManageViaMRErrorAlert().text()).toEqual(i18n.genericErrorText); + }); + }); }); describe('Auto DevOps hint alert', () => { diff --git a/spec/frontend/security_configuration/components/feature_card_spec.js b/spec/frontend/security_configuration/components/feature_card_spec.js index 7c91c13c6d70f3044360033bbc778a6d83294733..23edd8a69de858dba76e21c278f1220b17e45fe3 100644 --- a/spec/frontend/security_configuration/components/feature_card_spec.js +++ b/spec/frontend/security_configuration/components/feature_card_spec.js @@ -5,6 +5,7 @@ import FeatureCard from '~/security_configuration/components/feature_card.vue'; import FeatureCardBadge from '~/security_configuration/components/feature_card_badge.vue'; import ManageViaMr from '~/vue_shared/security_configuration/components/manage_via_mr.vue'; import { REPORT_TYPE_SAST } from '~/vue_shared/security_reports/constants'; +import { manageViaMRErrorMessage } from '../constants'; import { makeFeature } from './utils'; describe('FeatureCard component', () => { @@ -106,8 +107,8 @@ describe('FeatureCard component', () => { }); it('should catch and emit manage-via-mr-error', () => { - findManageViaMr().vm.$emit('error', 'There was a manage via MR error'); - expect(wrapper.emitted('error')).toEqual([['There was a manage via MR error']]); + findManageViaMr().vm.$emit('error', manageViaMRErrorMessage); + expect(wrapper.emitted('error')).toEqual([[manageViaMRErrorMessage]]); }); }); diff --git a/spec/frontend/security_configuration/constants.js b/spec/frontend/security_configuration/constants.js new file mode 100644 index 0000000000000000000000000000000000000000..d31036a2534402d2ee3aff2bb0cfbed8e6c952c9 --- /dev/null +++ b/spec/frontend/security_configuration/constants.js @@ -0,0 +1 @@ +export const manageViaMRErrorMessage = 'There was a manage via MR error'; diff --git a/spec/graphql/types/project_type_spec.rb b/spec/graphql/types/project_type_spec.rb index 7f26190830e3cf4fe1a2d7a607a22ae0754d1f51..0bfca9a290b91b12f6f78e28f6e632e066f7e170 100644 --- a/spec/graphql/types/project_type_spec.rb +++ b/spec/graphql/types/project_type_spec.rb @@ -291,7 +291,7 @@ let_it_be(:project) { create(:project_empty_repo) } it 'raises an error' do - expect(subject['errors'][0]['message']).to eq('You must add at least one file to the ' \ 'repository before using Security features.') diff --git a/spec/lib/gitlab/utils/error_message_spec.rb b/spec/lib/gitlab/utils/error_message_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..2c2d16656e8763d7c6a9605871af2da6d7b6e048 --- /dev/null +++ b/spec/lib/gitlab/utils/error_message_spec.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' + +RSpec.describe Gitlab::Utils::ErrorMessage, feature_category: :error_tracking do + let(:klass) do + Class.new do + include Gitlab::Utils::ErrorMessage + end + end + + subject(:object) { klass.new } + + describe 'error message' do + subject { object.to_user_facing(string) } + + let(:string) { 'Error Message' } + + it "returns input prefixed with UF:" do + is_expected.to eq 'UF: Error Message' + end + end +end diff --git a/spec/support/shared_examples/services/security/ci_configuration/create_service_shared_examples.rb b/spec/support/shared_examples/services/security/ci_configuration/create_service_shared_examples.rb index 8bfe57f45496ae7b65aafc36c5da667513bab8e3..9fcdd296ebecac3e0a7186ba0e3a1af3317b4a47 100644 --- a/spec/support/shared_examples/services/security/ci_configuration/create_service_shared_examples.rb +++ b/spec/support/shared_examples/services/security/ci_configuration/create_service_shared_examples.rb @@ -168,7 +168,7 @@ it 'returns an error' do expect { result }.to raise_error { |error| expect(error).to be_a(Gitlab::Graphql::Errors::MutationError) - expect(error.message).to eq('You must add at least one file to the repository' \ ' before using Security features.')