From e75d3673cc4327fa3dcc183fa6445c21fbddf455 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Espino=20Garc=C3=ADa?= Date: Wed, 24 Mar 2021 11:52:54 +0100 Subject: [PATCH] Make createCall function and related functions return an error message to be shown at a higher level (#5239) --- app/actions/views/command.ts | 7 +- .../app_command_parser.test.ts | 4 +- .../app_command_parser/app_command_parser.ts | 88 +++++++++++-------- assets/base/i18n/en.json | 4 +- 4 files changed, 58 insertions(+), 45 deletions(-) diff --git a/app/actions/views/command.ts b/app/actions/views/command.ts index a6c57892e..882a81a7e 100644 --- a/app/actions/views/command.ts +++ b/app/actions/views/command.ts @@ -43,16 +43,13 @@ export function executeCommand(message: string, channelId: string, rootId: strin if (appsAreEnabled) { const parser = new AppCommandParser({dispatch, getState}, intl, args.channel_id, args.root_id); if (parser.isAppCommand(msg)) { - const call = await parser.composeCallFromCommand(message); + const {call, errorMessage} = await parser.composeCallFromCommand(message); const createErrorMessage = (errMessage: string) => { return {error: {message: errMessage}}; }; if (!call) { - return createErrorMessage(intl.formatMessage({ - id: 'mobile.commands.error_title', - defaultMessage: 'Error Executing Command', - })); + return createErrorMessage(errorMessage!); } const res = await dispatch(doAppCall(call, AppCallTypes.SUBMIT, intl)); diff --git a/app/components/autocomplete/slash_suggestion/app_command_parser/app_command_parser.test.ts b/app/components/autocomplete/slash_suggestion/app_command_parser/app_command_parser.test.ts index b2802ea37..a0fd06857 100644 --- a/app/components/autocomplete/slash_suggestion/app_command_parser/app_command_parser.test.ts +++ b/app/components/autocomplete/slash_suggestion/app_command_parser/app_command_parser.test.ts @@ -879,7 +879,7 @@ describe('AppCommandParser', () => { const cmd = '/jira issue create'; const values = {}; - const call = await parser.composeCallFromCommand(cmd); + const {call} = await parser.composeCallFromCommand(cmd); expect(call).toEqual({ ...base, raw_command: cmd, @@ -902,7 +902,7 @@ describe('AppCommandParser', () => { project: '', }; - const call = await parser.composeCallFromCommand(cmd); + const {call} = await parser.composeCallFromCommand(cmd); expect(call).toEqual({ ...base, expand: {}, diff --git a/app/components/autocomplete/slash_suggestion/app_command_parser/app_command_parser.ts b/app/components/autocomplete/slash_suggestion/app_command_parser/app_command_parser.ts index 1a520c168..885d351a9 100644 --- a/app/components/autocomplete/slash_suggestion/app_command_parser/app_command_parser.ts +++ b/app/components/autocomplete/slash_suggestion/app_command_parser/app_command_parser.ts @@ -553,35 +553,34 @@ export class AppCommandParser { } // composeCallFromCommand creates the form submission call - public composeCallFromCommand = async (command: string): Promise => { + public composeCallFromCommand = async (command: string): Promise<{call: AppCallRequest | null, errorMessage?: string}> => { let parsed = new ParsedCommand(command, this, this.intl); const commandBindings = this.getCommandBindings(); if (!commandBindings) { - this.displayError(this.intl.formatMessage({ - id: 'apps.error.parser.no_bindings', - defaultMessage: 'No command bindings.', - })); - return null; + return {call: null, + errorMessage: this.intl.formatMessage({ + id: 'apps.error.parser.no_bindings', + defaultMessage: 'No command bindings.', + })}; } parsed = await parsed.matchBinding(commandBindings, false); parsed = parsed.parseForm(false); if (parsed.state === ParseState.Error) { - this.displayError(parsed.errorMessage()); - return null; + return {call: null, errorMessage: parsed.errorMessage()}; } const missing = this.getMissingFields(parsed); if (missing.length > 0) { const missingStr = missing.map((f) => f.label).join(', '); - this.displayError(this.intl.formatMessage({ - id: 'apps.error.command.field_missing', - defaultMessage: 'Required fields missing: `{fieldName}`.', - }, { - fieldName: missingStr, - })); - return null; + return {call: null, + errorMessage: this.intl.formatMessage({ + id: 'apps.error.command.field_missing', + defaultMessage: 'Required fields missing: `{fieldName}`.', + }, { + fieldName: missingStr, + })}; } return this.composeCallFromParsed(parsed); @@ -661,33 +660,41 @@ export class AppCommandParser { } // composeCallFromParsed creates the form submission call - composeCallFromParsed = async (parsed: ParsedCommand): Promise => { + composeCallFromParsed = async (parsed: ParsedCommand): Promise<{call: AppCallRequest | null, errorMessage?: string}> => { if (!parsed.binding) { - return null; + return {call: null, + errorMessage: this.intl.formatMessage({ + id: 'apps.error.parser.missing_binding', + defaultMessage: 'Missing command bindings.', + })}; } const call = parsed.form?.call || parsed.binding.call; if (!call) { - return null; + return {call: null, + errorMessage: this.intl.formatMessage({ + id: 'apps.error.parser.missing_call', + defaultMessage: 'Missing binding call.', + })}; } const values: AppCallValues = parsed.values; - const ok = await this.expandOptions(parsed, values); + const {errorMessage} = await this.expandOptions(parsed, values); - if (!ok) { - return null; + if (errorMessage) { + return {call: null, errorMessage}; } const context = this.getAppContext(parsed.binding.app_id); - return createCallRequest(call, context, {}, values, parsed.command); + return {call: createCallRequest(call, context, {}, values, parsed.command)}; } - expandOptions = async (parsed: ParsedCommand, values: AppCallValues) => { + expandOptions = async (parsed: ParsedCommand, values: AppCallValues): Promise<{errorMessage?: string}> => { if (!parsed.form?.fields) { - return true; + return {}; } - let ok = true; + const errors: {[key: string]: string} = {}; await Promise.all(parsed.form.fields.map(async (f) => { if (!values[f.name]) { return; @@ -699,14 +706,13 @@ export class AppCommandParser { case AppFieldTypes.STATIC_SELECT: { const option = f.options?.find((o) => (o.value === values[f.name])); if (!option) { - ok = false; - this.displayError(this.intl.formatMessage({ + errors[f.name] = this.intl.formatMessage({ id: 'apps.error.command.unknown_option', defaultMessage: 'Unknown option for field `{fieldName}`: `{option}`.', }, { fieldName: f.name, option: values[f.name], - })); + }); return; } values[f.name] = option; @@ -721,14 +727,13 @@ export class AppCommandParser { if (!user) { const dispatchResult = await this.store.dispatch(getUserByUsername(userName) as any); if ('error' in dispatchResult) { - ok = false; - this.displayError(this.intl.formatMessage({ + errors[f.name] = this.intl.formatMessage({ id: 'apps.error.command.unknown_user', defaultMessage: 'Unknown user for field `{fieldName}`: `{option}`.', }, { fieldName: f.name, option: values[f.name], - })); + }); return; } user = dispatchResult.data; @@ -745,14 +750,13 @@ export class AppCommandParser { if (!channel) { const dispatchResult = await this.store.dispatch(getChannelByNameAndTeamName(getCurrentTeam(this.store.getState()).name, channelName) as any); if ('error' in dispatchResult) { - ok = false; - this.displayError(this.intl.formatMessage({ + errors[f.name] = this.intl.formatMessage({ id: 'apps.error.command.unknown_channel', defaultMessage: 'Unknown channel for field `{fieldName}`: `{option}`.', }, { fieldName: f.name, option: values[f.name], - })); + }); return; } channel = dispatchResult.data; @@ -763,7 +767,15 @@ export class AppCommandParser { } })); - return ok; + if (Object.keys(errors).length === 0) { + return {}; + } + + let errorMessage = ''; + Object.keys(errors).forEach((v) => { + errorMessage = errorMessage + errors[v] + '\n'; + }); + return {errorMessage}; } // decorateSuggestionComplete applies the necessary modifications for a suggestion to be processed @@ -1093,11 +1105,13 @@ export class AppCommandParser { })); } - const call = await this.composeCallFromParsed(parsed); + const {call, errorMessage} = await this.composeCallFromParsed(parsed); if (!call) { return this.makeSuggestionError(this.intl.formatMessage({ id: 'apps.error.lookup.error_preparing_request', - defaultMessage: 'Error preparing lookup request.', + defaultMessage: 'Error preparing lookup request: {errorMessage}', + }, { + errorMessage, })); } call.selected_field = f.name; diff --git a/assets/base/i18n/en.json b/assets/base/i18n/en.json index 84899fd7a..7cd3160ea 100644 --- a/assets/base/i18n/en.json +++ b/assets/base/i18n/en.json @@ -23,9 +23,11 @@ "apps.error.form.refresh": "There has been an error fetching the select fields. Contact the app developer. Details: {details}", "apps.error.form.refresh_no_refresh": "Called refresh on no refresh field.", "apps.error.form.submit.pretext": "There has been an error submitting the modal. Contact the app developer. Details: {details}", - "apps.error.lookup.error_preparing_request": "Error preparing lookup request.", + "apps.error.lookup.error_preparing_request": "Error preparing lookup request: {errorMessage}", "apps.error.parser": "Parsing error: {error}.\n```\n{command}\n{space}^\n```", "apps.error.parser.empty_value": "empty values are not allowed", + "apps.error.parser.missing_binding": "Missing command bindings.", + "apps.error.parser.missing_call": "Missing binding call.", "apps.error.parser.missing_field_value": "Field value is missing.", "apps.error.parser.missing_quote": "Matching double quote expected before end of input.", "apps.error.parser.missing_tick": "Matching tick quote expected before end of input.",