Add parsing error to suggestions, add more information to text suggestions and avoid empty suggestions (#5251) (#5264)

* 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 777b35bda3)

Co-authored-by: Daniel Espino García <larkox@gmail.com>

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',
                 },
             ]);
         });

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<AppForm | undefined>;
+    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<AutocompleteSuggestion[]> => {
         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<AppForm | undefined> => {
+    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<AppForm | undefined> => {
+    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<ResponseType>(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,
         }];
     }

app/components/autocomplete/slash_suggestion/slash_suggestion.tsx

@@ -243,6 +243,7 @@ export default class SlashSuggestion extends PureComponent<Props, State> {
             theme={this.props.theme}
             suggestion={item.Suggestion}
             complete={item.Complete}
+            icon={item.IconData}
         />
     )
 

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 = (
+        <Image
+            style={style.iconColor}
+            width={10}
+            height={16}
+            source={slashIcon}
+        />
+    );
+    if (props.icon === 'error') {
+        image = (
+            <Image
+                style={style.iconColor}
+                width={10}
+                height={16}
+                source={bangIcon}
+            />
+        );
+    } else if (props.icon && props.icon.startsWith('http')) {
+        image = (
+            <FastImage
+                source={{uri: props.icon}}
+                style={{width: 16, height: 16}}
+            />
+        );
+    }
+
     return (
         <TouchableWithFeedback
             onPress={completeSuggestion}
@@ -97,12 +126,7 @@ const SlashSuggestionItem = (props: Props) => {
         >
             <View style={style.container}>
                 <View style={style.icon}>
-                    <Image
-                        style={style.iconColor}
-                        width={10}
-                        height={16}
-                        source={slashIcon}
-                    />
+                    {image}
                 </View>
                 <View style={style.suggestionContainer}>
                     <Text style={style.suggestionName}>{`${suggestionText}`}</Text>

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",