diff --git a/ee/app/assets/javascripts/subscriptions/new/components/app.vue b/ee/app/assets/javascripts/subscriptions/new/components/app.vue index 139e50a82b98e1faf4bfd5c8f6da810d4dcfe1ef..10709c8c2e6fc42dd91e232bab2a1324676be3a8 100644 --- a/ee/app/assets/javascripts/subscriptions/new/components/app.vue +++ b/ee/app/assets/javascripts/subscriptions/new/components/app.vue @@ -35,7 +35,7 @@ export default { }, handleVuexActionDispatch(action) { if (action.type === 'confirmOrderError') { - this.handleError(new Error(action.payload)); + this.handleError(action.payload); } }, hideError() { diff --git a/ee/app/assets/javascripts/subscriptions/new/store/actions.js b/ee/app/assets/javascripts/subscriptions/new/store/actions.js index 5afdd29682002b64eedd3aab095b46030ef06637..b61f31e78ad25f151c0a4abf0d4b44d96e3e3bd2 100644 --- a/ee/app/assets/javascripts/subscriptions/new/store/actions.js +++ b/ee/app/assets/javascripts/subscriptions/new/store/actions.js @@ -7,6 +7,7 @@ import { redirectTo } from '~/lib/utils/url_utility'; import { s__, sprintf } from '~/locale'; import { trackCheckout, trackTransaction } from '~/google_tag_manager'; import { isInvalidPromoCodeError } from 'ee/subscriptions/new/utils'; +import { ActiveModelError } from 'ee/vue_shared/purchase_flow/utils/purchase_errors'; import defaultClient from '../graphql'; import * as types from './mutation_types'; @@ -60,7 +61,10 @@ export const fetchCountriesSuccess = ({ commit }, data = []) => { }; export const fetchCountriesError = ({ dispatch }) => { - dispatch('confirmOrderError', s__('Checkout|Failed to load countries. Please try again.')); + dispatch( + 'confirmOrderError', + new Error(s__('Checkout|Failed to load countries. Please try again.')), + ); }; export const fetchStates = ({ state, dispatch }) => { @@ -82,7 +86,10 @@ export const fetchStatesSuccess = ({ commit }, data = {}) => { }; export const fetchStatesError = ({ dispatch }) => { - dispatch('confirmOrderError', s__('Checkout|Failed to load states. Please try again.')); + dispatch( + 'confirmOrderError', + new Error(s__('Checkout|Failed to load states. Please try again.')), + ); }; export const resetStates = ({ commit }) => { @@ -139,14 +146,17 @@ export const fetchPaymentFormParamsSuccess = ({ commit, dispatch }, data) => { }, false, ); - dispatch('confirmOrderError', message); + dispatch('confirmOrderError', new Error(message)); } else { commit(types.UPDATE_PAYMENT_FORM_PARAMS, data); } }; export const fetchPaymentFormParamsError = ({ dispatch }) => { - dispatch('confirmOrderError', s__('Checkout|Credit card form failed to load. Please try again.')); + dispatch( + 'confirmOrderError', + new Error(s__('Checkout|Credit card form failed to load. Please try again.')), + ); }; export const zuoraIframeRendered = ({ commit }) => @@ -174,7 +184,7 @@ export const paymentFormSubmittedError = ({ dispatch }, response) => { response, false, ); - dispatch('confirmOrderError', message); + dispatch('confirmOrderError', new Error(message)); }; export const fetchPaymentMethodDetails = ({ state, dispatch, commit }) => @@ -191,12 +201,15 @@ export const fetchPaymentMethodDetailsSuccess = ({ commit, dispatch }, creditCar mutation: activateNextStepMutation, }) .catch((error) => { - dispatch('confirmOrderError', error.message); + dispatch('confirmOrderError', error); }); }; export const fetchPaymentMethodDetailsError = ({ dispatch }) => { - dispatch('confirmOrderError', s__('Checkout|Failed to register credit card. Please try again.')); + dispatch( + 'confirmOrderError', + new Error(s__('Checkout|Failed to register credit card. Please try again.')), + ); }; const shouldShowErrorMessageOnly = (errors) => { @@ -228,26 +241,29 @@ export const confirmOrder = ({ getters, dispatch, commit }) => { location: data.location, }); } else { - let errors; + let errorMessage; if (data.name) { - errors = sprintf( - s__('Checkout|Name: %{errors}'), - { errors: data.name.join(', ') }, + errorMessage = sprintf( + s__('Checkout|Name: %{errorMessage}'), + { errorMessage: data.name.join(', ') }, false, ); } else if (shouldShowErrorMessageOnly(data.errors)) { - errors = data.errors?.message; + errorMessage = data.errors?.message; } else { - errors = data.errors; + errorMessage = data.errors; } - trackConfirmOrder(errors); - dispatch('confirmOrderError', JSON.stringify(errors)); + trackConfirmOrder(errorMessage); + dispatch( + 'confirmOrderError', + new ActiveModelError(data.error_attribute_map, JSON.stringify(errorMessage)), + ); } }) - .catch(({ message }) => { - trackConfirmOrder(message); - dispatch('confirmOrderError', message); + .catch((error) => { + trackConfirmOrder(error.message); + dispatch('confirmOrderError', error); }); }; diff --git a/ee/app/assets/javascripts/vue_shared/purchase_flow/components/checkout/error_alert.vue b/ee/app/assets/javascripts/vue_shared/purchase_flow/components/checkout/error_alert.vue index 553f0cf3ff06f0fb0a4233f61124aca731e48b94..18785b71a6ac8f50f21149a22b187aa1082b0dc1 100644 --- a/ee/app/assets/javascripts/vue_shared/purchase_flow/components/checkout/error_alert.vue +++ b/ee/app/assets/javascripts/vue_shared/purchase_flow/components/checkout/error_alert.vue @@ -21,7 +21,7 @@ export default { }, computed: { friendlyError() { - return mapSystemToFriendlyError(this.error?.message); + return mapSystemToFriendlyError(this.error); }, friendlyErrorMessage() { return generateHelpTextWithLinks(this.friendlyError); diff --git a/ee/app/assets/javascripts/vue_shared/purchase_flow/utils/purchase_errors.js b/ee/app/assets/javascripts/vue_shared/purchase_flow/utils/purchase_errors.js index 4e1206a7adc8a3758feefb52aea280e614f8fef2..a33c7f040b9aa0244fce16970c1b5fea05733bf0 100644 --- a/ee/app/assets/javascripts/vue_shared/purchase_flow/utils/purchase_errors.js +++ b/ee/app/assets/javascripts/vue_shared/purchase_flow/utils/purchase_errors.js @@ -1,4 +1,4 @@ -import { isEmpty, isString } from 'lodash'; +import { isEmpty, isString, isObject } from 'lodash'; import { sprintf, s__ } from '~/locale'; import { GENERAL_ERROR_MESSAGE, @@ -9,6 +9,22 @@ import { } from 'ee/vue_shared/purchase_flow/constants'; import { convertObjectPropsToLowerCase } from '~/lib/utils/common_utils'; +export class ActiveModelError extends Error { + constructor(errorAttributeMap = {}, ...params) { + // Pass remaining arguments (including vendor specific ones) to parent constructor + super(...params); + + // Maintains proper stack trace for where our error was thrown (only available on V8) + if (Error.captureStackTrace) { + Error.captureStackTrace(this, ActiveModelError); + } + + this.name = 'ActiveModelError'; + // Custom debugging information + this.errorAttributeMap = errorAttributeMap; + } +} + export const CONTACT_SUPPORT_DEFAULT_MESSAGE = { message: GENERAL_ERROR_MESSAGE, links: {}, @@ -62,45 +78,122 @@ export const UNLINKED_ACCOUNT_ERROR = { }, }; -/* eslint-disable-next-line @gitlab/require-i18n-strings */ +/* eslint-disable @gitlab/require-i18n-strings */ const cannotBeBlank = "can't be blank"; -/* eslint-disable-next-line @gitlab/require-i18n-strings */ const alreadyTaken = 'has already been taken'; export const CONTRACT_EFFECTIVE_ERROR = 'The Contract effective date should not be later than the term end date of the basic subscription'; export const GENERIC_DECLINE_ERROR = 'Transaction declined.generic_decline - Your card was declined'; -export const EMAIL_TAKEN_ERROR = JSON.stringify({ email: [alreadyTaken] }); +export const EMAIL_TAKEN_ERROR = `Email ${alreadyTaken}`; +export const EMAIL_TAKEN_ERROR_TYPE = 'email:taken'; export const INSUFFICIENT_FUNDS_ERROR = 'Your card has insufficient funds.'; +export const FIRST_NAME_BLANK_ERROR = `First name ${cannotBeBlank}`; +export const LAST_NAME_BLANK_ERROR = `Last name ${cannotBeBlank}`; + +// The following messages can be removed after this MR has landed: +// https://gitlab.com/gitlab-org/customers-gitlab-com/-/merge_requests/6970 export const FIRST_NAME_BLANK_ERROR_VARIATION = JSON.stringify({ first_name: [cannotBeBlank] }); -export const LAST_NAME_BLANK_ERROR = JSON.stringify({ last_name: [cannotBeBlank] }); -export const LAST_NAME_BLANK_ERROR_VARIATION = `{${LAST_NAME_BLANK_ERROR}}`; +export const LAST_NAME_BLANK_ERROR_VARIATION_1 = JSON.stringify({ last_name: [cannotBeBlank] }); +export const LAST_NAME_BLANK_ERROR_VARIATION_2 = `{${LAST_NAME_BLANK_ERROR}}`; +/* eslint-enable @gitlab/require-i18n-strings */ export const errorDictionary = convertObjectPropsToLowerCase({ [CONTRACT_EFFECTIVE_ERROR]: EXPIRED_SUBSCRIPTION_ERROR, [GENERIC_DECLINE_ERROR]: DECLINED_CARD_GENERIC_ERROR, [EMAIL_TAKEN_ERROR]: UNLINKED_ACCOUNT_ERROR, + [EMAIL_TAKEN_ERROR_TYPE]: UNLINKED_ACCOUNT_ERROR, [INSUFFICIENT_FUNDS_ERROR]: DECLINED_CARD_FUNDS_ERROR, + [FIRST_NAME_BLANK_ERROR]: FULL_NAME_REQUIRED_ERROR, [FIRST_NAME_BLANK_ERROR_VARIATION]: FULL_NAME_REQUIRED_ERROR, [LAST_NAME_BLANK_ERROR]: FULL_NAME_REQUIRED_ERROR, - [LAST_NAME_BLANK_ERROR_VARIATION]: FULL_NAME_REQUIRED_ERROR, + [LAST_NAME_BLANK_ERROR_VARIATION_1]: FULL_NAME_REQUIRED_ERROR, + [LAST_NAME_BLANK_ERROR_VARIATION_2]: FULL_NAME_REQUIRED_ERROR, }); -export const mapSystemToFriendlyError = (systemError) => { - if (systemError && isString(systemError)) { - return ( - errorDictionary[systemError.toLowerCase()] || { - message: systemError, - links: {}, - } +/** + * @typedef {Object<ErrorAttribute,ErrorType[]>} ErrorAttributeMap - Map of attributes to error details + * @typedef {string} ErrorAttribute - the error attribute https://api.rubyonrails.org/v7.0.4.2/classes/ActiveModel/Error.html + * @typedef {string} ErrorType - the error type https://api.rubyonrails.org/v7.0.4.2/classes/ActiveModel/Error.html + * + * @example { "email": ["taken", ...] } + * // returns `${UNLINKED_ACCOUNT_ERROR}`, i.e. the `EMAIL_TAKEN_ERROR_TYPE` error message + * + * @param {ErrorAttributeMap} errorAttributeMap + * @returns {(null|string)} null or error message if found + */ +function getMessageFromType(errorAttributeMap = {}) { + if (!isObject(errorAttributeMap)) { + return null; + } + + return Object.keys(errorAttributeMap).reduce((_, attribute) => { + const errorType = errorAttributeMap[attribute].find( + (type) => errorDictionary[`${attribute}:${type}`.toLowerCase()], ); + if (errorType) { + return errorDictionary[`${attribute}:${errorType}`.toLowerCase()]; + } + + return null; + }, null); +} + +/** + * @example "Email has already been taken, Email is invalid" + * // returns `${UNLINKED_ACCOUNT_ERROR}`, i.e. the `EMAIL_TAKEN_ERROR_TYPE` error message + * + * @param {string} errorString + * @returns {(null|string)} null or error message if found + */ +function getMessageFromErrorString(errorString) { + if (isEmpty(errorString) || !isString(errorString)) { + return null; + } + + const messages = errorString.split(', '); + const errorMessage = messages.find((message) => errorDictionary[message.toLowerCase()]); + if (errorMessage) { + return errorDictionary[errorMessage.toLowerCase()]; + } + + return { + message: errorString, + links: {}, + }; +} + +/** + * Receives an Error and attempts to extract the `errorAttributeMap` in + * case it is an `ActiveModelError` and return the message if it exists. + * If a match is not found it will attempt to map a message from the + * Error.message to be returned. + * Otherwise, it will return a general error message + * + * @param {Error} systemError + * @returns error message + */ +export function mapSystemToFriendlyError(systemError) { + if (!(systemError instanceof Error)) { + return CONTACT_SUPPORT_DEFAULT_MESSAGE; + } + + const { errorAttributeMap, message } = systemError; + const messageFromType = getMessageFromType(errorAttributeMap); + if (messageFromType) { + return messageFromType; + } + + const messageFromErrorString = getMessageFromErrorString(message); + if (messageFromErrorString) { + return messageFromErrorString; } return CONTACT_SUPPORT_DEFAULT_MESSAGE; -}; +} -const generateLinks = (links) => { +function generateLinks(links) { return Object.keys(links).reduce((allLinks, link) => { /* eslint-disable-next-line @gitlab/require-i18n-strings */ const linkStart = `${link}Start`; @@ -113,7 +206,7 @@ const generateLinks = (links) => { [linkEnd]: '</a>', }; }, {}); -}; +} export const generateHelpTextWithLinks = (error) => { if (isString(error)) { diff --git a/ee/lib/gitlab/subscription_portal/client.rb b/ee/lib/gitlab/subscription_portal/client.rb index c65d528a7c18ccd0fc6bd9eeef4b87a679ecc3af..efa95f0f931e41184621476513fc03748c39e6e7 100644 --- a/ee/lib/gitlab/subscription_portal/client.rb +++ b/ee/lib/gitlab/subscription_portal/client.rb @@ -44,7 +44,7 @@ def parse_response(http_response) { success: true, data: parsed_response } when Net::HTTPUnprocessableEntity log_error(http_response) - { success: false, data: { errors: parsed_response['errors'] } } + { success: false, data: parsed_response.slice('errors', 'error_attribute_map') } else log_error(http_response) { success: false, data: { errors: "HTTP status code: #{http_response.code}" } } diff --git a/ee/spec/frontend/subscriptions/new/components/app_spec.js b/ee/spec/frontend/subscriptions/new/components/app_spec.js index 64c3726f5446733657de7ddebd4765e0bc3ae5b2..27d7c5dda2ecaee14780d04beb4a9d8839c4fa4f 100644 --- a/ee/spec/frontend/subscriptions/new/components/app_spec.js +++ b/ee/spec/frontend/subscriptions/new/components/app_spec.js @@ -16,6 +16,7 @@ Vue.use(Vuex); describe('App component', () => { let wrapper; + let store; const findModalComponent = () => wrapper.findComponent(Modal); const findCheckout = () => wrapper.findComponent(Checkout); @@ -25,7 +26,7 @@ describe('App component', () => { const findOrderSummary = () => wrapper.findComponent(OrderSummary); const createComponent = () => { - const store = new Vuex.Store({ + store = new Vuex.Store({ ...initialStore, actions: { confirmOrderError: jest.fn(), @@ -117,7 +118,7 @@ describe('App component', () => { beforeEach(() => { createComponent(); - wrapper.vm.$store.dispatch('confirmOrderError', errorMessage); + store.dispatch('confirmOrderError', error); }); it('passes the correct props', () => { @@ -133,7 +134,7 @@ describe('App component', () => { beforeEach(() => { createComponent(); - wrapper.vm.$store.dispatch('fakeAction'); + store.dispatch('fakeAction'); }); it('does not the alert', () => { diff --git a/ee/spec/frontend/subscriptions/new/store/actions_spec.js b/ee/spec/frontend/subscriptions/new/store/actions_spec.js index 1db9c45cea3f2938e776d73df89a670f945c7742..367a8cc3f824dc8de4a9707e57ea2f89154ee264 100644 --- a/ee/spec/frontend/subscriptions/new/store/actions_spec.js +++ b/ee/spec/frontend/subscriptions/new/store/actions_spec.js @@ -4,6 +4,7 @@ import * as constants from 'ee/subscriptions/constants'; import defaultClient from 'ee/subscriptions/new/graphql'; import * as actions from 'ee/subscriptions/new/store/actions'; import activateNextStepMutation from 'ee/vue_shared/purchase_flow/graphql/mutations/activate_next_step.mutation.graphql'; +import { ActiveModelError } from 'ee/vue_shared/purchase_flow/utils/purchase_errors'; import { useMockLocationHelper } from 'helpers/mock_window_location_helper'; import testAction from 'helpers/vuex_action_helper'; import Tracking from '~/tracking'; @@ -171,7 +172,12 @@ describe('Subscriptions Actions', () => { null, {}, [], - [{ type: 'confirmOrderError', payload: 'Failed to load countries. Please try again.' }], + [ + { + type: 'confirmOrderError', + payload: new Error('Failed to load countries. Please try again.'), + }, + ], ); }); }); @@ -239,7 +245,12 @@ describe('Subscriptions Actions', () => { null, {}, [], - [{ type: 'confirmOrderError', payload: 'Failed to load states. Please try again.' }], + [ + { + type: 'confirmOrderError', + payload: new Error('Failed to load states. Please try again.'), + }, + ], ); }); }); @@ -416,7 +427,12 @@ describe('Subscriptions Actions', () => { { errors: 'error message' }, {}, [], - [{ type: 'confirmOrderError', payload: 'Credit card form failed to load: error message' }], + [ + { + type: 'confirmOrderError', + payload: new Error('Credit card form failed to load: error message'), + }, + ], ); }); }); @@ -431,7 +447,7 @@ describe('Subscriptions Actions', () => { [ { type: 'confirmOrderError', - payload: 'Credit card form failed to load. Please try again.', + payload: new Error('Credit card form failed to load. Please try again.'), }, ], ); @@ -498,8 +514,9 @@ describe('Subscriptions Actions', () => { [ { type: 'confirmOrderError', - payload: + payload: new Error( 'Submitting the credit card form failed with code codeFromResponse: messageFromResponse', + ), }, ], ); @@ -574,7 +591,7 @@ describe('Subscriptions Actions', () => { payload: creditCardDetails, }, ], - [{ type: 'confirmOrderError', payload: error.message }], + [{ type: 'confirmOrderError', payload: error }], ); }); }); @@ -589,7 +606,7 @@ describe('Subscriptions Actions', () => { [ { type: 'confirmOrderError', - payload: 'Failed to register credit card. Please try again.', + payload: new Error('Failed to register credit card. Please try again.'), }, ], ); @@ -646,7 +663,7 @@ describe('Subscriptions Actions', () => { null, {}, [{ type: 'UPDATE_IS_CONFIRMING_ORDER', payload: true }], - [{ type: 'confirmOrderError', payload: '"errors"' }], + [{ type: 'confirmOrderError', payload: new ActiveModelError(null, '"errors"') }], ); expect(spy).toHaveBeenCalledWith('default', 'click_button', { @@ -663,7 +680,12 @@ describe('Subscriptions Actions', () => { null, {}, [{ type: 'UPDATE_IS_CONFIRMING_ORDER', payload: true }], - [{ type: 'confirmOrderError', payload: '"Name: Error_1, Error \' 2"' }], + [ + { + type: 'confirmOrderError', + payload: new ActiveModelError(null, '"Name: Error_1, Error \' 2"'), + }, + ], ); }); @@ -681,7 +703,34 @@ describe('Subscriptions Actions', () => { null, {}, [{ type: 'UPDATE_IS_CONFIRMING_ORDER', payload: true }], - [{ type: 'confirmOrderError', payload: '"Promo code is invalid"' }], + [ + { + type: 'confirmOrderError', + payload: new ActiveModelError(null, '"Promo code is invalid"'), + }, + ], + ); + }); + + it('sends the error_attribute_map in the paylod', async () => { + const errors = { email: ["can't be blank"] }; + const errorAttributeMap = { email: ['taken'] }; + mock.onPost(confirmOrderPath).replyOnce(HTTP_STATUS_OK, { + errors, + error_attribute_map: errorAttributeMap, + }); + + await testAction( + actions.confirmOrder, + null, + {}, + [{ type: 'UPDATE_IS_CONFIRMING_ORDER', payload: true }], + [ + { + type: 'confirmOrderError', + payload: new ActiveModelError(errorAttributeMap, JSON.stringify(errors)), + }, + ], ); }); }); @@ -697,7 +746,12 @@ describe('Subscriptions Actions', () => { null, {}, [{ type: 'UPDATE_IS_CONFIRMING_ORDER', payload: true }], - [{ type: 'confirmOrderError', payload: 'Request failed with status code 500' }], + [ + { + type: 'confirmOrderError', + payload: new Error('Request failed with status code 500'), + }, + ], ); expect(spy).toHaveBeenCalledWith('default', 'click_button', { diff --git a/ee/spec/frontend/vue_shared/purchase_flow/components/checkout/error_alert_spec.js b/ee/spec/frontend/vue_shared/purchase_flow/components/checkout/error_alert_spec.js index f1832a6fb622ee227bcbf8a96e3068d3b1e77731..c977b63837c94f3f3dac9e8a147cea1d7fdf552f 100644 --- a/ee/spec/frontend/vue_shared/purchase_flow/components/checkout/error_alert_spec.js +++ b/ee/spec/frontend/vue_shared/purchase_flow/components/checkout/error_alert_spec.js @@ -53,7 +53,7 @@ describe('Purchase Error Alert', () => { }); it('invokes mapSystemToFriendlyError', () => { - expect(mapSystemToFriendlyError).toHaveBeenCalledWith(error.message); + expect(mapSystemToFriendlyError).toHaveBeenCalledWith(error); }); it('passes the correct html', () => { diff --git a/ee/spec/frontend/vue_shared/purchase_flow/utils/purchase_errors_spec.js b/ee/spec/frontend/vue_shared/purchase_flow/utils/purchase_errors_spec.js index dfe3345bb2aee8b80875491e60fc9792184d31d7..fdfdfce99f4a6ae56b33aecbe6404aa22da98816 100644 --- a/ee/spec/frontend/vue_shared/purchase_flow/utils/purchase_errors_spec.js +++ b/ee/spec/frontend/vue_shared/purchase_flow/utils/purchase_errors_spec.js @@ -1,5 +1,8 @@ import { + ActiveModelError, errorDictionary, + EMAIL_TAKEN_ERROR, + EMAIL_TAKEN_ERROR_TYPE, CONTACT_SUPPORT_DEFAULT_MESSAGE, generateHelpTextWithLinks, mapSystemToFriendlyError, @@ -8,7 +11,7 @@ import { describe('Purchase Dynamic Errors', () => { describe('errorDictionary', () => { it('contains all the declared errors', () => { - expect(Object.keys(errorDictionary)).toHaveLength(7); + expect(Object.keys(errorDictionary)).toHaveLength(10); }); }); @@ -17,38 +20,82 @@ describe('Purchase Dynamic Errors', () => { const friendlyError = errorDictionary[systemError]; it('maps the system error to the friendly one', () => { - expect(mapSystemToFriendlyError(systemError)).toEqual(friendlyError); + expect(mapSystemToFriendlyError(new Error(systemError))).toEqual(friendlyError); }); it('maps the system error to the friendly one from uppercase', () => { - expect(mapSystemToFriendlyError(systemError.toUpperCase())).toEqual(friendlyError); + expect(mapSystemToFriendlyError(new Error(systemError.toUpperCase()))).toEqual( + friendlyError, + ); }); }); - describe.each(['', {}, [], undefined, null])('when system error is %s', (systemError) => { + describe.each(['', {}, [], undefined, null, new Error()])( + 'when system error is %s', + (systemError) => { + it('defaults to the general error message', () => { + expect(mapSystemToFriendlyError(systemError)).toEqual(CONTACT_SUPPORT_DEFAULT_MESSAGE); + }); + }, + ); + + describe('when system error is a non-existent key', () => { + const message = 'a non-existent key'; + const nonExistentKeyError = { message, links: {} }; + it('maps the system error to the friendly one', () => { - expect(mapSystemToFriendlyError(systemError)).toEqual(CONTACT_SUPPORT_DEFAULT_MESSAGE); + expect(mapSystemToFriendlyError(new Error(message))).toEqual(nonExistentKeyError); }); }); - describe('when system error is a non existent key', () => { - const message = 'a non existent key'; + describe('when system error consists of multiple non-existent keys', () => { + const message = 'a non-existent key, another non-existent key'; const nonExistentKeyError = { message, links: {} }; it('maps the system error to the friendly one', () => { - expect(mapSystemToFriendlyError(message)).toEqual(nonExistentKeyError); + expect(mapSystemToFriendlyError(new Error(message))).toEqual(nonExistentKeyError); }); }); - describe('when error is email already taken', () => { - const EMAIL_TAKEN_ERROR = JSON.stringify({ email: ['has already been taken'] }); + describe('when system error consists of multiple messages with one matching key', () => { + const message = `a non-existent key, ${EMAIL_TAKEN_ERROR}`; + + it('maps the system error to the friendly one', () => { + expect(mapSystemToFriendlyError(new Error(message))).toEqual( + errorDictionary[EMAIL_TAKEN_ERROR.toLowerCase()], + ); + }); + }); + describe('when error is email already taken message', () => { it('maps the email friendly error', () => { - expect(mapSystemToFriendlyError(EMAIL_TAKEN_ERROR)).toEqual( - errorDictionary[EMAIL_TAKEN_ERROR], + expect(mapSystemToFriendlyError(new Error(EMAIL_TAKEN_ERROR))).toEqual( + errorDictionary[EMAIL_TAKEN_ERROR.toLowerCase()], ); }); }); + + describe('when error is email:taken error_attribute_map', () => { + const errorAttributeMap = { email: ['taken'] }; + + it('maps the email friendly error', () => { + expect( + mapSystemToFriendlyError(new ActiveModelError(errorAttributeMap, EMAIL_TAKEN_ERROR)), + ).toEqual(errorDictionary[EMAIL_TAKEN_ERROR_TYPE.toLowerCase()]); + }); + }); + + describe('when there are multiple errors in the error_attribute_map', () => { + const errorAttributeMap = { email: ['taken', 'invalid'] }; + + it('maps the email friendly error', () => { + expect( + mapSystemToFriendlyError( + new ActiveModelError(errorAttributeMap, `${EMAIL_TAKEN_ERROR}, Email is invalid`), + ), + ).toEqual(errorDictionary[EMAIL_TAKEN_ERROR_TYPE.toLowerCase()]); + }); + }); }); describe('generateHelpTextWithLinks', () => { diff --git a/ee/spec/lib/gitlab/subscription_portal/clients/rest_spec.rb b/ee/spec/lib/gitlab/subscription_portal/clients/rest_spec.rb index 51cf1339a3a19714e39101afc3c6fb422f562b18..ea2474a2774e5b8b0034179ebd186056a1902737 100644 --- a/ee/spec/lib/gitlab/subscription_portal/clients/rest_spec.rb +++ b/ee/spec/lib/gitlab/subscription_portal/clients/rest_spec.rb @@ -4,21 +4,22 @@ RSpec.describe Gitlab::SubscriptionPortal::Clients::Rest do let(:client) { Gitlab::SubscriptionPortal::Client } - let(:http_response) { nil } + let(:message) { nil } let(:http_method) { :post } - let(:error_message) { 'Our team has been notified. Please try again.' } + let(:response) { nil } + let(:parsed_response) { nil } let(:gitlab_http_response) do double( - code: http_response.code, - response: http_response, + code: response.code, + response: response, body: {}, - parsed_response: {}, - message: 'message' + parsed_response: parsed_response, + message: message ) end shared_examples 'when response is successful' do - let(:http_response) { Net::HTTPSuccess.new(1.0, '201', 'OK') } + let(:response) { Net::HTTPSuccess.new(1.0, '201', 'OK') } it 'has a successful status' do allow(Gitlab::HTTP).to receive(http_method).and_return(gitlab_http_response) @@ -28,19 +29,22 @@ end shared_examples 'when http call raises an exception' do + let(:message) { 'Our team has been notified. Please try again.' } + it 'overrides the error message' do exception = Gitlab::HTTP::HTTP_ERRORS.first.new allow(Gitlab::HTTP).to receive(http_method).and_raise(exception) - result = subject - - expect(result[:success]).to eq(false) - expect(result[:data][:errors]).to eq(error_message) + expect(subject[:success]).to eq(false) + expect(subject[:data][:errors]).to eq(message) end end shared_examples 'when response code is 422' do - let(:http_response) { Net::HTTPUnprocessableEntity.new(1.0, '422', 'Error') } + let(:response) { Net::HTTPUnprocessableEntity.new(1.0, '422', 'Error') } + let(:message) { 'Email has already been taken' } + let(:error_attribute_map) { { email: ["taken"] } } + let(:parsed_response) { { errors: message, error_attribute_map: error_attribute_map }.stringify_keys } it 'has a unprocessable entity status' do allow(Gitlab::ErrorTracking).to receive(:log_exception) @@ -50,13 +54,22 @@ expect(Gitlab::ErrorTracking).to have_received(:log_exception).with( instance_of(::Gitlab::SubscriptionPortal::Client::ResponseError), - { status: '422', message: 'message', body: {} } + { status: '422', message: message, body: {} } ) end + + it 'returns the error message along with the error_attribute_map' do + allow(Gitlab::ErrorTracking).to receive(:log_exception) + allow(Gitlab::HTTP).to receive(http_method).and_return(gitlab_http_response) + + expect(subject[:success]).to eq(false) + expect(subject[:data]['errors']).to eq(message) + expect(subject[:data]['error_attribute_map']).to eq(error_attribute_map) + end end shared_examples 'when response code is 500' do - let(:http_response) { Net::HTTPServerError.new(1.0, '500', 'Error') } + let(:response) { Net::HTTPServerError.new(1.0, '500', 'Error') } it 'has a server error status' do allow(Gitlab::ErrorTracking).to receive(:log_exception) @@ -66,7 +79,7 @@ expect(Gitlab::ErrorTracking).to have_received(:log_exception).with( instance_of(::Gitlab::SubscriptionPortal::Client::ResponseError), - { status: '500', message: 'message', body: {} } + { status: '500', message: message, body: {} } ) end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 492c0060b50e9e83c7cc4c753c43eff2fcd43770..1620f986cd80cdf6e65d3028055ee9ce66104270 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -8867,7 +8867,7 @@ msgstr "" msgid "Checkout|Name of company or organization using GitLab" msgstr "" -msgid "Checkout|Name: %{errors}" +msgid "Checkout|Name: %{errorMessage}" msgstr "" msgid "Checkout|Need more users? Purchase GitLab for your %{company}."