From 91b46214333f7cd89e8a3bf5d0ea54eb56141c8b Mon Sep 17 00:00:00 2001 From: Mattermost Build Date: Thu, 1 Apr 2021 21:58:03 +0200 Subject: [PATCH] Add parsing error to suggestions, add more information to text suggestions and avoid empty suggestions (#5251) (#5264) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add parsing error to suggestions and avoid empty suggestions * Fix merge issue * Address feedback and send fetch form errors as suggestion errors * Fix test (cherry picked from commit 777b35bda3717e3bee15127ea9775e33b3dcaa81) Co-authored-by: Daniel Espino García --- .../app_command_parser.test.ts | 25 ++-- .../app_command_parser/app_command_parser.ts | 111 ++++++++++++------ .../slash_suggestion/slash_suggestion.tsx | 1 + .../slash_suggestion_item.tsx | 36 +++++- assets/base/i18n/en.json | 5 +- .../autocomplete/slash_command_error.png | Bin 0 -> 288 bytes .../autocomplete/slash_command_error@2x.png | Bin 0 -> 419 bytes .../autocomplete/slash_command_error@3x.png | Bin 0 -> 625 bytes 8 files changed, 120 insertions(+), 58 deletions(-) create mode 100644 assets/base/images/autocomplete/slash_command_error.png create mode 100644 assets/base/images/autocomplete/slash_command_error@2x.png create mode 100644 assets/base/images/autocomplete/slash_command_error@3x.png 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 a0fd06857..3ac4cba8c 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 @@ -416,7 +416,7 @@ describe('AppCommandParser', () => { { title: 'error: unexpected positional', command: '/jira issue create wrong', - submit: {expectError: 'Command does not accept {positionX} positional arguments.'}, + submit: {expectError: 'Unable to identify argument.'}, }, { title: 'error: multiple equal signs', @@ -443,11 +443,6 @@ describe('AppCommandParser', () => { }); describe('getSuggestions', () => { - test('just the app command', async () => { - const suggestions = await parser.getSuggestions('/jira'); - expect(suggestions).toEqual([]); - }); - test('subcommand 1', async () => { const suggestions = await parser.getSuggestions('/jira '); expect(suggestions).toEqual([ @@ -574,7 +569,7 @@ describe('AppCommandParser', () => { Description: 'The Jira issue key', Hint: '', IconData: '', - Suggestion: '', + Suggestion: 'issue: ""', }, ]); }); @@ -726,7 +721,7 @@ describe('AppCommandParser', () => { Description: 'The Jira issue summary', Hint: '', IconData: 'Create icon', - Suggestion: '', + Suggestion: 'summary: ""', }, ]); }); @@ -739,7 +734,7 @@ describe('AppCommandParser', () => { Description: 'The Jira issue summary', Hint: '', IconData: 'Create icon', - Suggestion: 'Sum', + Suggestion: 'summary: "Sum"', }, ]); }); @@ -752,7 +747,7 @@ describe('AppCommandParser', () => { Description: 'The Jira issue summary', Hint: '', IconData: 'Create icon', - Suggestion: 'Sum', + Suggestion: 'summary: "Sum"', }, ]); }); @@ -765,7 +760,7 @@ describe('AppCommandParser', () => { Description: 'The Jira issue summary', Hint: '', IconData: 'Create icon', - Suggestion: 'Sum', + Suggestion: 'summary: `Sum`', }, ]); }); @@ -778,7 +773,7 @@ describe('AppCommandParser', () => { Description: 'The Jira issue summary', Hint: '', IconData: 'Create icon', - Suggestion: '', + Suggestion: 'summary: ""', }, ]); }); @@ -836,9 +831,9 @@ describe('AppCommandParser', () => { { Complete: 'jira issue create --project KT --summary "great feature" --epic', Suggestion: '', - Description: 'No matching options.', - Hint: '', - IconData: '', + Description: '', + Hint: 'No matching options.', + IconData: 'error', }, ]); }); 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 875bd6ffb..45df306b7 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 @@ -66,7 +66,7 @@ export const ParseState = keyMirror({ }); interface FormsCache { - getForm: (location: string, binding: AppBinding) => Promise; + getForm: (location: string, binding: AppBinding) => Promise<{form?: AppForm, error?: string} | undefined>; } interface Intl { @@ -215,7 +215,11 @@ export class ParsedCommand { this.form = this.binding.form; if (!this.form) { - this.form = await this.formsCache.getForm(this.location, this.binding); + const fetched = await this.formsCache.getForm(this.location, this.binding); + if (fetched?.error) { + return this.asError(fetched.error); + } + this.form = fetched?.form; } return this; @@ -263,9 +267,7 @@ export class ParsedCommand { if (!field) { return this.asError(this.intl.formatMessage({ id: 'apps.error.parser.no_argument_pos_x', - defaultMessage: 'Command does not accept {positionX} positional arguments.', - }, { - positionX: this.position, + defaultMessage: 'Unable to identify argument.', })); } this.field = field; @@ -608,6 +610,7 @@ export class AppCommandParser { // getSuggestions returns suggestions for subcommands and/or form arguments public getSuggestions = async (pretext: string): Promise => { let parsed = new ParsedCommand(pretext, this, this.intl); + let suggestions: AutocompleteSuggestion[] = []; const commandBindings = this.getCommandBindings(); if (!commandBindings) { @@ -615,13 +618,19 @@ export class AppCommandParser { } parsed = await parsed.matchBinding(commandBindings, true); - let suggestions: AutocompleteSuggestion[] = []; + if (parsed.state === ParseState.Error) { + suggestions = this.getErrorSuggestion(parsed); + } + if (parsed.state === ParseState.Command) { suggestions = this.getCommandSuggestions(parsed); } if (parsed.form || parsed.incomplete) { parsed = parsed.parseForm(true); + if (parsed.state === ParseState.Error) { + suggestions = this.getErrorSuggestion(parsed); + } const argSuggestions = await this.getParameterSuggestions(parsed); suggestions = suggestions.concat(argSuggestions); } @@ -644,11 +653,38 @@ export class AppCommandParser { if (execute) { suggestions = [execute, ...suggestions]; } + } else if (suggestions.length === 0 && (parsed.field?.type !== AppFieldTypes.USER && parsed.field?.type !== AppFieldTypes.CHANNEL)) { + suggestions = this.getNoMatchingSuggestion(); } - return suggestions.map((suggestion) => this.decorateSuggestionComplete(parsed, suggestion)); } + getNoMatchingSuggestion = () => { + return [{ + Complete: '', + Suggestion: '', + Hint: this.intl.formatMessage({ + id: 'apps.suggestion.no_suggestion', + defaultMessage: 'No matching suggestions.', + }), + IconData: 'error', + Description: '', + }]; + } + + getErrorSuggestion = (parsed: ParsedCommand) => { + return [{ + Complete: '', + Suggestion: '', + Hint: this.intl.formatMessage({ + id: 'apps.suggestion.errors.parser_error', + defaultMessage: 'Parsing error', + }), + IconData: 'error', + Description: parsed.error, + }]; + } + // composeCallFromParsed creates the form submission call composeCallFromParsed = async (parsed: ParsedCommand): Promise<{call: AppCallRequest | null, errorMessage?: string}> => { if (!parsed.binding) { @@ -779,7 +815,7 @@ export class AppCommandParser { goBackSpace = 1; } let complete = parsed.command.substring(0, parsed.incompleteStart - goBackSpace); - complete += choice.Complete || choice.Suggestion; + complete += choice.Complete === undefined ? choice.Suggestion : choice.Complete; choice.Hint = choice.Hint || ''; complete = complete.substring(1); @@ -847,7 +883,7 @@ export class AppCommandParser { } // fetchForm unconditionaly retrieves the form for the given binding (subcommand) - fetchForm = async (binding: AppBinding): Promise => { + fetchForm = async (binding: AppBinding): Promise<{form?: AppForm, error?: string} | undefined> => { if (!binding.call) { return undefined; } @@ -860,11 +896,10 @@ export class AppCommandParser { const res = await this.store.dispatch(doAppCall(payload, AppCallTypes.FORM, this.intl)); if (res.error) { const errorResponse = res.error as AppCallResponse; - this.displayError(errorResponse.error || this.intl.formatMessage({ + return {error: errorResponse.error || this.intl.formatMessage({ id: 'apps.error.unknown', defaultMessage: 'Unknown error.', - })); - return undefined; + })}; } const callResponse = res.data as AppCallResponse; @@ -873,35 +908,33 @@ export class AppCommandParser { break; case AppCallResponseTypes.NAVIGATE: case AppCallResponseTypes.OK: - this.displayError(this.intl.formatMessage({ + return {error: this.intl.formatMessage({ id: 'apps.error.responses.unexpected_type', defaultMessage: 'App response type was not expected. Response type: {type}', }, { type: callResponse.type, - })); - return undefined; + })}; default: - this.displayError(this.intl.formatMessage({ + return {error: this.intl.formatMessage({ id: 'apps.error.responses.unknown_type', defaultMessage: 'App response type not supported. Response type: {type}.', }, { type: callResponse.type, - })); - return undefined; + })}; } - return callResponse.form; + return {form: callResponse.form}; } - getForm = async (location: string, binding: AppBinding): Promise => { + getForm = async (location: string, binding: AppBinding): Promise<{form?: AppForm, error?: string} | undefined> => { const form = this.forms[location]; if (form) { - return form; + return {form}; } const fetched = await this.fetchForm(binding); - if (fetched) { - this.forms[location] = fetched; + if (fetched?.form) { + this.forms[location] = fetched.form; } return fetched; } @@ -1044,7 +1077,7 @@ export class AppCommandParser { return [{ Complete: complete, - Suggestion: parsed.incomplete, + Suggestion: `${parsed.field.label || parsed.field.name}: ${delimiter || '"'}${parsed.incomplete}${delimiter || '"'}`, Description: f.description || '', Hint: '', IconData: parsed.binding?.icon || '', @@ -1059,12 +1092,12 @@ export class AppCommandParser { return [{ Complete: '', Suggestion: '', - Hint: '', - Description: this.intl.formatMessage({ + Hint: this.intl.formatMessage({ id: 'apps.suggestion.no_static', defaultMessage: 'No matching options.', }), - IconData: '', + Description: '', + IconData: 'error', }]; } return opts.map((opt) => { @@ -1089,7 +1122,7 @@ export class AppCommandParser { const f = parsed.field; if (!f) { // Should never happen - return this.makeSuggestionError(this.intl.formatMessage({ + return this.makeDynamicSelectSuggestionError(this.intl.formatMessage({ id: 'apps.error.parser.unexpected_error', defaultMessage: 'Unexpected error.', })); @@ -1097,7 +1130,7 @@ export class AppCommandParser { const {call, errorMessage} = await this.composeCallFromParsed(parsed); if (!call) { - return this.makeSuggestionError(this.intl.formatMessage({ + return this.makeDynamicSelectSuggestionError(this.intl.formatMessage({ id: 'apps.error.lookup.error_preparing_request', defaultMessage: 'Error preparing lookup request: {errorMessage}', }, { @@ -1111,7 +1144,7 @@ export class AppCommandParser { const res = await this.store.dispatch(doAppCall(call, AppCallTypes.LOOKUP, this.intl)); if (res.error) { const errorResponse = res.error as AppCallResponse; - return this.makeSuggestionError(errorResponse.error || this.intl.formatMessage({ + return this.makeDynamicSelectSuggestionError(errorResponse.error || this.intl.formatMessage({ id: 'apps.error.unknown', defaultMessage: 'Unknown error.', })); @@ -1123,14 +1156,14 @@ export class AppCommandParser { break; case AppCallResponseTypes.NAVIGATE: case AppCallResponseTypes.FORM: - return this.makeSuggestionError(this.intl.formatMessage({ + return this.makeDynamicSelectSuggestionError(this.intl.formatMessage({ id: 'apps.error.responses.unexpected_type', defaultMessage: 'App response type was not expected. Response type: {type}', }, { type: callResponse.type, })); default: - return this.makeSuggestionError(this.intl.formatMessage({ + return this.makeDynamicSelectSuggestionError(this.intl.formatMessage({ id: 'apps.error.responses.unknown_type', defaultMessage: 'App response type not supported. Response type: {type}.', }, { @@ -1143,7 +1176,10 @@ export class AppCommandParser { return [{ Complete: '', Suggestion: '', - Hint: '', + Hint: this.intl.formatMessage({ + id: 'apps.suggestion.no_static', + defaultMessage: 'No matching options.', + }), IconData: '', Description: this.intl.formatMessage({ id: 'apps.suggestion.no_dynamic', @@ -1169,7 +1205,7 @@ export class AppCommandParser { }); } - makeSuggestionError = (message: string): AutocompleteSuggestion[] => { + makeDynamicSelectSuggestionError = (message: string): AutocompleteSuggestion[] => { const errMsg = this.intl.formatMessage({ id: 'apps.error', defaultMessage: 'Error: {error}', @@ -1178,9 +1214,12 @@ export class AppCommandParser { }); return [{ Complete: '', - Suggestion: '', + Suggestion: this.intl.formatMessage({ + id: 'apps.suggestion.dynamic.error', + defaultMessage: 'Dynamic select error', + }), Hint: '', - IconData: '', + IconData: 'error', Description: errMsg, }]; } diff --git a/app/components/autocomplete/slash_suggestion/slash_suggestion.tsx b/app/components/autocomplete/slash_suggestion/slash_suggestion.tsx index a31714d12..e472be408 100644 --- a/app/components/autocomplete/slash_suggestion/slash_suggestion.tsx +++ b/app/components/autocomplete/slash_suggestion/slash_suggestion.tsx @@ -243,6 +243,7 @@ export default class SlashSuggestion extends PureComponent { theme={this.props.theme} suggestion={item.Suggestion} complete={item.Complete} + icon={item.IconData} /> ) diff --git a/app/components/autocomplete/slash_suggestion/slash_suggestion_item.tsx b/app/components/autocomplete/slash_suggestion/slash_suggestion_item.tsx index 11c0f4834..d2382f23a 100644 --- a/app/components/autocomplete/slash_suggestion/slash_suggestion_item.tsx +++ b/app/components/autocomplete/slash_suggestion/slash_suggestion_item.tsx @@ -9,7 +9,9 @@ import {Theme} from '@mm-redux/types/preferences'; import TouchableWithFeedback from '@components/touchable_with_feedback'; import {changeOpacity, makeStyleSheetFromTheme} from '@utils/theme'; +import FastImage from 'react-native-fast-image'; const slashIcon = require('@assets/images/autocomplete/slash_command.png'); +const bangIcon = require('@assets/images/autocomplete/slash_command_error.png'); const getStyleFromTheme = makeStyleSheetFromTheme((theme: Theme) => { return { @@ -55,6 +57,7 @@ type Props = { hint: string; onPress: (complete: string) => void; suggestion: string; + icon: string; theme: Theme; } @@ -88,6 +91,32 @@ const SlashSuggestionItem = (props: Props) => { } } + let image = ( + + ); + if (props.icon === 'error') { + image = ( + + ); + } else if (props.icon && props.icon.startsWith('http')) { + image = ( + + ); + } + return ( { > - + {image} {`${suggestionText}`} diff --git a/assets/base/i18n/en.json b/assets/base/i18n/en.json index e0530fe53..fcd05e3dc 100644 --- a/assets/base/i18n/en.json +++ b/assets/base/i18n/en.json @@ -32,7 +32,7 @@ "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.", "apps.error.parser.multiple_equal": "Multiple `=` signs are not allowed.", - "apps.error.parser.no_argument_pos_x": "Command does not accept {positionX} positional arguments.", + "apps.error.parser.no_argument_pos_x": "Unable to identify argument.", "apps.error.parser.no_bindings": "No command bindings.", "apps.error.parser.no_match": "`{command}`: No matching command found in this workspace.", "apps.error.parser.no_slash_start": "Command must start with a `/`.", @@ -47,8 +47,11 @@ "apps.error.responses.unexpected_type": "App response type was not expected. Response type: {type}.", "apps.error.responses.unknown_type": "App response type not supported. Response type: {type}.", "apps.error.unknown": "Unknown error occurred.", + "apps.suggestion.dynamic.error": "Dynamic select error", + "apps.suggestion.errors.parser_error": "Parsing error", "apps.suggestion.no_dynamic": "No data was returned for dynamic suggestions", "apps.suggestion.no_static": "No matching options.", + "apps.suggestion.no_suggestion": "No matching suggestions.", "archivedChannelMessage": "You are viewing an **archived channel**. New messages cannot be posted.", "center_panel.archived.closeChannel": "Close Channel", "channel_header.addMembers": "Add Members", diff --git a/assets/base/images/autocomplete/slash_command_error.png b/assets/base/images/autocomplete/slash_command_error.png new file mode 100644 index 0000000000000000000000000000000000000000..c1dd93d83cb146031fdffd376beed8c7dc97463e GIT binary patch literal 288 zcmeAS@N?(olHy`uVBq!ia0vp^AhsX}8<2dWZ2J^QaTa()7Bet#3xhBt!>l*8o|0J>k`CC0*978G?_f9nAJES1d`u|bO^0@p4Ehc6&jc`cJXW z_vy~|9~_VJvw7&{_)GD+-dgJQ{`7Y`3@( z1Jm{s0OJ-j@|I&3OW7_9>~*+~=^Z`_NJPJoob8SR9@;NwyG~cR3A-K4{E1xjqkPL3 z4HxDn`0MoEIrsQ8T()hy5Cg~Kr)K~+`IGWCpMT7_9QGX6b$t`UI=P9DU7rlBs_GV4 zRd*)?CBLpDP^tHoa}c~=y!}?(yCw;C5%u zyI8>2ksao@GwgosF2KG?Py-u{+eZLXain|g^`0MqAPGcit=Fy?AC@OIM86 z;G7&AthI51vF75zLI_H$36o=jY1NcaaU5%BHB##0D?U1MR})3%4TvIx!%z~oiY&g> z0BcuLgVQTxak=;Ca5yX;j7it_RZCydEN~*bZEEtuVuNkAX^bGZnGM$ZpIVLR%;ZYx z^1wpK%yw!m%WJjGHYtB1xxqGVHt*y^02_>cZAmW6t2hiZEskwgos~&DDrl~ZL+LZYxpzn zT)Ef~X!rYc7a@Za&1TcF@1L*m4pqSkoTNk;