From a2ed6ba3bde35c9ddbc78129b30c05411459fb22 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Espino=20Garc=C3=ADa?= Date: Mon, 27 Sep 2021 14:54:32 +0200 Subject: [PATCH] Sync with App Command Parser with Webapp (#5631) * Sync with App Command Parser with Webapp * Add i18n * Patch flaky test --- .../app_command_parser.test.ts | 71 +++++++- .../app_command_parser/app_command_parser.ts | 166 +++++++++++------- .../app_command_parser_dependencies.ts | 9 - .../tests/app_command_parser_test_data.ts | 32 ++++ 4 files changed, 202 insertions(+), 76 deletions(-) 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 2c7d3b798..1d9c8dc7a 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 @@ -419,6 +419,60 @@ describe('AppCommandParser', () => { command: '/jira issue create --project == test', submit: {expectError: 'Multiple `=` signs are not allowed.'}, }, + { + title: 'rest field', + command: '/jira issue rest hello world', + autocomplete: {verify: (parsed: ParsedCommand): void => { + expect(parsed.state).toBe(ParseState.Rest); + expect(parsed.binding?.label).toBe('rest'); + expect(parsed.incomplete).toBe('hello world'); + expect(parsed.values?.summary).toBe(undefined); + }}, + submit: {verify: (parsed: ParsedCommand): void => { + expect(parsed.state).toBe(ParseState.Rest); + expect(parsed.binding?.label).toBe('rest'); + expect(parsed.values?.summary).toBe('hello world'); + }}, + }, + { + title: 'rest field with other field', + command: '/jira issue rest --verbose true hello world', + autocomplete: {verify: (parsed: ParsedCommand): void => { + expect(parsed.state).toBe(ParseState.Rest); + expect(parsed.binding?.label).toBe('rest'); + expect(parsed.incomplete).toBe('hello world'); + expect(parsed.values?.summary).toBe(undefined); + expect(parsed.values?.verbose).toBe('true'); + }}, + submit: {verify: (parsed: ParsedCommand): void => { + expect(parsed.state).toBe(ParseState.Rest); + expect(parsed.binding?.label).toBe('rest'); + expect(parsed.values?.summary).toBe('hello world'); + expect(parsed.values?.verbose).toBe('true'); + }}, + }, + { + title: 'rest field as flag with other field', + command: '/jira issue rest --summary "hello world" --verbose true', + autocomplete: {verify: (parsed: ParsedCommand): void => { + expect(parsed.state).toBe(ParseState.EndValue); + expect(parsed.binding?.label).toBe('rest'); + expect(parsed.incomplete).toBe('true'); + expect(parsed.values?.summary).toBe('hello world'); + expect(parsed.values?.verbose).toBe(undefined); + }}, + submit: {verify: (parsed: ParsedCommand): void => { + expect(parsed.state).toBe(ParseState.EndValue); + expect(parsed.binding?.label).toBe('rest'); + expect(parsed.values?.summary).toBe('hello world'); + expect(parsed.values?.verbose).toBe('true'); + }}, + }, + { + title: 'error: rest after rest field flag', + command: '/jira issue rest --summary "hello world" --verbose true hello world', + submit: {expectError: 'Unable to identify argument.'}, + }, ]; table.forEach((tc) => { @@ -508,6 +562,13 @@ describe('AppCommandParser', () => { IconData: 'Create icon', Description: 'Create a new Jira issue', }, + { + Suggestion: 'rest', + Complete: 'jira issue rest', + Hint: 'rest hint', + IconData: 'rest icon', + Description: 'rest description', + }, ]); }); @@ -528,6 +589,14 @@ describe('AppCommandParser', () => { IconData: 'Create icon', Description: 'Create a new Jira issue', }, + { + Suggestion: 'rest', + Complete: 'JiRa IsSuE rest', + Hint: 'rest hint', + IconData: 'rest icon', + Description: 'rest description', + }, + ]); }); @@ -859,7 +928,7 @@ describe('AppCommandParser', () => { context: { app_id: 'jira', channel_id: 'current_channel_id', - location: '/command', + location: '/command/jira/issue/create', root_id: 'root_id', team_id: 'team_id', }, 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 981621398..c4b328c72 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 @@ -33,7 +33,6 @@ import { EXECUTE_CURRENT_COMMAND_ITEM_ID, COMMAND_SUGGESTION_ERROR, getExecuteSuggestion, - displayError, createCallRequest, selectUserByUsername, getUserByUsername, @@ -46,10 +45,10 @@ import { filterEmptyOptions, autocompleteUsersInChannel, autocompleteChannels, - ExtendedAutocompleteSuggestion, getChannelSuggestions, getUserSuggestions, inTextMentionSuggestions, + ExtendedAutocompleteSuggestion, getAppCommandForm, getAppRHSCommandForm, makeRHSAppBindingSelector, @@ -78,6 +77,7 @@ export enum ParseState { EndQuotedValue = 'EndQuotedValue', EndTickedValue = 'EndTickedValue', Error = 'Error', + Rest = 'Rest', } interface FormsCache { @@ -304,12 +304,20 @@ export class ParsedCommand { // Positional parameter. this.position++; // eslint-disable-next-line no-loop-func - const field = fields.find((f: AppField) => f.position === this.position); + let field = fields.find((f: AppField) => f.position === this.position); if (!field) { - return this.asError(this.intl.formatMessage({ - id: 'apps.error.parser.no_argument_pos_x', - defaultMessage: 'Unable to identify argument.', - })); + field = fields.find((f) => f.position === -1 && f.type === AppFieldTypes.TEXT); + if (!field || this.values[field.name]) { + return this.asError(this.intl.formatMessage({ + id: 'apps.error.parser.no_argument_pos_x', + defaultMessage: 'Unable to identify argument.', + })); + } + this.incompleteStart = this.i; + this.incomplete = ''; + this.field = field; + this.state = ParseState.Rest; + break; } this.field = field; this.state = ParseState.StartValue; @@ -319,6 +327,28 @@ export class ParsedCommand { break; } + case ParseState.Rest: { + if (!this.field) { + return this.asError(this.intl.formatMessage({ + id: 'apps.error.parser.missing_field_value', + defaultMessage: 'Field value is missing.', + })); + } + + if (autocompleteMode && c === '') { + return this; + } + + if (c === '') { + this.values[this.field.name] = this.incomplete; + return this; + } + + this.i++; + this.incomplete += c; + break; + } + case ParseState.ParameterSeparator: { this.incompleteStart = this.i; switch (c) { @@ -475,7 +505,7 @@ export class ParsedCommand { if (this.incompleteStart === this.i - 1) { return this.asError(this.intl.formatMessage({ id: 'apps.error.parser.empty_value', - defaultMessage: 'empty values are not allowed', + defaultMessage: 'Empty values are not allowed.', })); } this.i++; @@ -515,7 +545,7 @@ export class ParsedCommand { if (this.incompleteStart === this.i - 1) { return this.asError(this.intl.formatMessage({ id: 'apps.error.parser.empty_value', - defaultMessage: 'empty values are not allowed', + defaultMessage: 'Empty values are not allowed.', })); } this.i++; @@ -549,13 +579,13 @@ export class ParsedCommand { (!autocompleteMode && this.incomplete !== 'true' && this.incomplete !== 'false'))) { // reset back where the value started, and treat as a new parameter this.i = this.incompleteStart; - this.values![this.field.name] = 'true'; + this.values[this.field.name] = 'true'; this.state = ParseState.StartParameter; } else { if (autocompleteMode && c === '') { return this; } - this.values![this.field.name] = this.incomplete; + this.values[this.field.name] = this.incomplete; this.incomplete = ''; this.incompleteStart = this.i; if (c === '') { @@ -622,57 +652,59 @@ export class AppCommandParser { } private async addDefaultAndReadOnlyValues(parsed: ParsedCommand) { - await Promise.all(parsed.form?.fields?.map(async (f) => { + if (!parsed.form?.fields) { + return; + } + + await Promise.all(parsed.form.fields.map(async (f) => { if (!f.value) { return; } - if (!f.readonly || f.name in parsed.values) { - return; - } - - switch (f.type) { - case AppFieldTypes.TEXT: - parsed.values[f.name] = f.value as string; - break; - case AppFieldTypes.BOOL: - parsed.values[f.name] = 'true'; - break; - case AppFieldTypes.USER: { - const userID = (f.value as AppSelectOption).value; - let user = selectUser(this.store.getState(), userID); - if (!user) { - const dispatchResult = await this.store.dispatch(getUser(userID)); - if ('error' in dispatchResult) { - // Silently fail on default value - break; + if (f.readonly || !(f.name in parsed.values)) { + switch (f.type) { + case AppFieldTypes.TEXT: + parsed.values[f.name] = f.value as string; + break; + case AppFieldTypes.BOOL: + parsed.values[f.name] = 'true'; + break; + case AppFieldTypes.USER: { + const userID = (f.value as AppSelectOption).value; + let user = selectUser(this.store.getState(), userID); + if (!user) { + const dispatchResult = await this.store.dispatch(getUser(userID)); + if ('error' in dispatchResult) { + // Silently fail on default value + break; + } + user = dispatchResult.data; } - user = dispatchResult.data; + parsed.values[f.name] = user.username; + break; } - parsed.values[f.name] = user.username; - break; - } - case AppFieldTypes.CHANNEL: { - const channelID = (f.value as AppSelectOption).label; - let channel = selectChannel(this.store.getState(), channelID); - if (!channel) { - const dispatchResult = await this.store.dispatch(getChannel(channelID)); - if ('error' in dispatchResult) { - // Silently fail on default value - break; + case AppFieldTypes.CHANNEL: { + const channelID = (f.value as AppSelectOption).label; + let channel = selectChannel(this.store.getState(), channelID); + if (!channel) { + const dispatchResult = await this.store.dispatch(getChannel(channelID)); + if ('error' in dispatchResult) { + // Silently fail on default value + break; + } + channel = dispatchResult.data; } - channel = dispatchResult.data; + parsed.values[f.name] = channel.name; + break; } - parsed.values[f.name] = channel.name; - break; - } - case AppFieldTypes.STATIC_SELECT: - case AppFieldTypes.DYNAMIC_SELECT: - parsed.values[f.name] = (f.value as AppSelectOption).value; - break; - case AppFieldTypes.MARKDOWN: + case AppFieldTypes.STATIC_SELECT: + case AppFieldTypes.DYNAMIC_SELECT: + parsed.values[f.name] = (f.value as AppSelectOption).value; + break; + case AppFieldTypes.MARKDOWN: - // Do nothing + // Do nothing + } } }) || []); } @@ -984,7 +1016,7 @@ export class AppCommandParser { } context.channel_id = channel.id; - context.team_id = this.teamID || channel.team_id || getCurrentTeamId(this.store.getState()); + context.team_id = channel.team_id || getCurrentTeamId(this.store.getState()); return context; } @@ -992,7 +1024,10 @@ export class AppCommandParser { // fetchForm unconditionaly retrieves the form for the given binding (subcommand) private fetchForm = async (binding: AppBinding): Promise<{form?: AppForm; error?: string} | undefined> => { if (!binding.call) { - return undefined; + return {error: this.intl.formatMessage({ + id: 'apps.error.parser.missing_call', + defaultMessage: 'Missing binding call.', + })}; } const payload = createCallRequest( @@ -1055,15 +1090,6 @@ export class AppCommandParser { return fetched; } - // displayError shows an error that was caught by the parser - private displayError = (err: any): void => { - let errStr = err as string; - if (err.message) { - errStr = err.message; - } - displayError(this.intl, errStr); - } - // getSuggestionsForSubCommands returns suggestions for a subcommand's name private getCommandSuggestions = (parsed: ParsedCommand): AutocompleteSuggestion[] => { if (!parsed.binding?.bindings?.length) { @@ -1113,6 +1139,14 @@ export class AppCommandParser { case ParseState.EndTickedValue: case ParseState.TickValue: return this.getValueSuggestions(parsed, '`'); + case ParseState.Rest: { + const execute = getExecuteSuggestion(parsed); + const value = await this.getValueSuggestions(parsed); + if (execute) { + return [execute, ...value]; + } + return value; + } } return []; } @@ -1341,11 +1375,11 @@ export class AppCommandParser { }); return [{ Complete: '', - Suggestion: this.intl.formatMessage({ + Suggestion: '', + Hint: this.intl.formatMessage({ id: 'apps.suggestion.dynamic.error', defaultMessage: 'Dynamic select error', }), - Hint: '', IconData: COMMAND_SUGGESTION_ERROR, Description: errMsg, }]; diff --git a/app/components/autocomplete/slash_suggestion/app_command_parser/app_command_parser_dependencies.ts b/app/components/autocomplete/slash_suggestion/app_command_parser/app_command_parser_dependencies.ts index ef423d0c6..cb602f03b 100644 --- a/app/components/autocomplete/slash_suggestion/app_command_parser/app_command_parser_dependencies.ts +++ b/app/components/autocomplete/slash_suggestion/app_command_parser/app_command_parser_dependencies.ts @@ -2,7 +2,6 @@ // See LICENSE.txt for license information. import {intlShape} from 'react-intl'; -import {Alert} from 'react-native'; import {getUserByUsername, getUser, autocompleteUsers} from '@mm-redux/actions/users'; import {getCurrentTeamId, getCurrentTeam} from '@mm-redux/selectors/entities/teams'; @@ -114,14 +113,6 @@ export const getExecuteSuggestion = (_: ParsedCommand): AutocompleteSuggestion | return null; }; -export const displayError = (intl: typeof intlShape, body: string) => { - const title = intl.formatMessage({ - id: 'mobile.general.error.title', - defaultMessage: 'Error', - }); - Alert.alert(title, body); -}; - export const errorMessage = (intl: typeof intlShape, error: string, _command: string, _position: number): string => { // eslint-disable-line @typescript-eslint/no-unused-vars return intl.formatMessage({ id: 'apps.error.parser', diff --git a/app/components/autocomplete/slash_suggestion/app_command_parser/tests/app_command_parser_test_data.ts b/app/components/autocomplete/slash_suggestion/app_command_parser/tests/app_command_parser_test_data.ts index dde57a439..115459e07 100644 --- a/app/components/autocomplete/slash_suggestion/app_command_parser/tests/app_command_parser_test_data.ts +++ b/app/components/autocomplete/slash_suggestion/app_command_parser/tests/app_command_parser_test_data.ts @@ -181,6 +181,37 @@ export const createCommand: AppBinding = { } as AppForm, }; +export const restCommand: AppBinding = { + app_id: 'jira', + label: 'rest', + location: 'rest', + description: 'rest description', + icon: 'rest icon', + hint: 'rest hint', + form: { + call: { + path: '/create-issue', + }, + fields: [ + { + name: 'summary', + label: 'summary', + description: 'The Jira issue summary', + type: AppFieldTypes.TEXT, + hint: 'The thing is working great!', + position: -1, + }, + { + name: 'verbose', + label: 'verbose', + description: 'display details', + type: AppFieldTypes.BOOL, + hint: 'yes or no!', + }, + ], + } as AppForm, +}; + export const testBindings: AppBinding[] = [ { app_id: '', @@ -204,6 +235,7 @@ export const testBindings: AppBinding[] = [ bindings: [ viewCommand, createCommand, + restCommand, ], }], },