Sync binding filtering with webapp (#5655)

* Sync binding filtering with webapp

* Add return types and use variable instead of updating actions

* Allow bindings with no call but form with calls

app/actions/apps.ts

@@ -8,7 +8,7 @@ import {ActionFunc, DispatchFunc} from '@mm-redux/types/actions';
 import {AppCallResponse, AppCallRequest, AppCallType, AppContext} from '@mm-redux/types/apps';
 import {CommandArgs} from '@mm-redux/types/integrations';
 import {Post} from '@mm-redux/types/posts';
-import {makeCallErrorResponse} from '@utils/apps';
+import {cleanForm, makeCallErrorResponse} from '@utils/apps';
 
 export function doAppCall<Res=unknown>(call: AppCallRequest, type: AppCallType, intl: any): ActionFunc {
     return async () => {
@@ -30,6 +30,8 @@ export function doAppCall<Res=unknown>(call: AppCallRequest, type: AppCallType,
                     return {error: makeCallErrorResponse(errMsg)};
                 }
 
+                cleanForm(res.form);
+
                 return {data: res};
             }
             case AppCallResponseTypes.NAVIGATE:

app/components/autocomplete/slash_suggestion/app_command_parser/app_command_parser.ts

@@ -42,6 +42,7 @@ import {
     getCurrentTeam,
     selectChannelByName,
     errorMessage as parserErrorMessage,
+    filterEmptyOptions,
     autocompleteUsersInChannel,
     autocompleteChannels,
     ExtendedAutocompleteSuggestion,
@@ -1287,7 +1288,8 @@ export class AppCommandParser {
             }));
         }
 
-        const items = callResponse?.data?.items;
+        let items = callResponse?.data?.items;
+        items = items?.filter(filterEmptyOptions);
         if (!items?.length) {
             return [{
                 Complete: '',
@@ -1313,7 +1315,7 @@ export class AppCommandParser {
             }
             return ({
                 Complete: complete,
-                Description: s.label,
+                Description: s.label || s.value,
                 Suggestion: s.value,
                 Hint: '',
                 IconData: s.icon_data || parsed.binding?.icon || '',

app/components/autocomplete/slash_suggestion/app_command_parser/app_command_parser_dependencies.ts

@@ -86,7 +86,10 @@ export {
 export {getChannelByNameAndTeamName, getChannel, autocompleteChannels} from '@mm-redux/actions/channels';
 
 export {doAppCall} from '@actions/apps';
-export {createCallRequest} from '@utils/apps';
+export {
+    createCallRequest,
+    filterEmptyOptions,
+} from '@utils/apps';
 
 export const getStore = () => Store.redux;
 

app/components/post_list/post/body/content/embedded_bindings/embedded_binding.tsx

@@ -4,7 +4,8 @@
 import React from 'react';
 import {View} from 'react-native';
 
-import {copyAndFillBindings} from '@utils/apps';
+import {AppBindingLocations} from '@mm-redux/constants/apps';
+import {cleanBinding} from '@utils/apps';
 import {changeOpacity, makeStyleSheetFromTheme} from '@utils/theme';
 
 import EmbedText from './embed_text';
@@ -40,7 +41,7 @@ const getStyleSheet = makeStyleSheetFromTheme((theme:Theme) => {
 const EmbeddedBinding = ({embed, postId, theme}: Props) => {
     const style = getStyleSheet(theme);
 
-    const bindings = copyAndFillBindings(embed)?.bindings;
+    const bindings = cleanBinding(embed, AppBindingLocations.IN_POST)?.bindings;
 
     return (
         <>

app/mm-redux/reducers/entities/__snapshots__/apps.test.js.snap

@@ -7,13 +7,11 @@ Array [
     "bindings": Array [
       Object {
         "app_id": "1",
-        "bindings": undefined,
         "call": Object {},
         "label": "a",
         "location": "/post_menu/locA",
       },
     ],
-    "call": Object {},
     "location": "/post_menu",
   },
   Object {
@@ -21,13 +19,11 @@ Array [
     "bindings": Array [
       Object {
         "app_id": "2",
-        "bindings": undefined,
         "call": Object {},
         "label": "a",
         "location": "/post_menu/locA",
       },
     ],
-    "call": Object {},
     "location": "/post_menu",
   },
   Object {
@@ -35,7 +31,6 @@ Array [
     "bindings": Array [
       Object {
         "app_id": "1",
-        "bindings": undefined,
         "call": Object {},
         "icon": "icon",
         "label": "b",
@@ -43,13 +38,11 @@ Array [
       },
       Object {
         "app_id": "1",
-        "bindings": undefined,
         "call": Object {},
         "label": "c",
         "location": "/channel_header/locC",
       },
     ],
-    "call": Object {},
     "location": "/channel_header",
   },
   Object {
@@ -57,14 +50,19 @@ Array [
     "bindings": Array [
       Object {
         "app_id": "2",
-        "bindings": undefined,
+        "call": Object {},
+        "icon": "icon",
+        "label": "locB",
+        "location": "/channel_header/locB",
+      },
+      Object {
+        "app_id": "2",
         "call": Object {},
         "icon": "icon",
         "label": "c",
         "location": "/channel_header/locC",
       },
     ],
-    "call": Object {},
     "location": "/channel_header",
   },
   Object {
@@ -72,13 +70,17 @@ Array [
     "bindings": Array [
       Object {
         "app_id": "3",
-        "bindings": undefined,
+        "call": Object {},
+        "label": "locB",
+        "location": "/channel_header/locB",
+      },
+      Object {
+        "app_id": "3",
         "call": Object {},
         "label": "c",
         "location": "/channel_header/locC",
       },
     ],
-    "call": Object {},
     "location": "/channel_header",
   },
   Object {
@@ -86,13 +88,11 @@ Array [
     "bindings": Array [
       Object {
         "app_id": "3",
-        "bindings": undefined,
         "call": Object {},
         "label": "c",
         "location": "/command/locC",
       },
     ],
-    "call": Object {},
     "location": "/command",
   },
 ]
@@ -105,20 +105,17 @@ Array [
     "bindings": Array [
       Object {
         "app_id": "1",
-        "bindings": undefined,
         "call": Object {},
         "label": "a",
         "location": "/post_menu/locA",
       },
       Object {
         "app_id": "1",
-        "bindings": undefined,
         "call": Object {},
         "label": "a",
         "location": "/post_menu/locB",
       },
     ],
-    "call": Object {},
     "location": "/post_menu",
   },
   Object {
@@ -126,14 +123,12 @@ Array [
     "bindings": Array [
       Object {
         "app_id": "1",
-        "bindings": undefined,
         "call": Object {},
         "icon": "icon",
         "label": "b",
         "location": "/channel_header/locB",
       },
     ],
-    "call": Object {},
     "location": "/channel_header",
   },
   Object {
@@ -144,23 +139,46 @@ Array [
         "bindings": Array [
           Object {
             "app_id": "3",
-            "bindings": undefined,
+            "call": Object {},
+            "label": "subC1",
+            "location": "/command/locC/subC1",
+          },
+          Object {
+            "app_id": "3",
             "call": Object {},
             "label": "c2",
             "location": "/command/locC/subC2",
           },
         ],
-        "call": Object {},
         "label": "c",
         "location": "/command/locC",
       },
+      Object {
+        "app_id": "3",
+        "bindings": Array [
+          Object {
+            "app_id": "3",
+            "call": Object {},
+            "label": "subC1",
+            "location": "/command/locD/subC1",
+          },
+        ],
+        "label": "d",
+        "location": "/command/locD",
+      },
     ],
-    "call": Object {},
     "location": "/command",
   },
   Object {
     "app_id": "1",
-    "bindings": Array [],
+    "bindings": Array [
+      Object {
+        "app_id": "1",
+        "call": Object {},
+        "label": "locC",
+        "location": "/command/locC",
+      },
+    ],
     "location": "/command",
   },
   Object {
@@ -171,14 +189,12 @@ Array [
         "bindings": Array [
           Object {
             "app_id": "2",
-            "bindings": undefined,
             "call": Object {},
             "label": "c1",
             "location": "/command/locC/subC1",
           },
           Object {
             "app_id": "2",
-            "bindings": undefined,
             "call": Object {},
             "label": "c2",
             "location": "/command/locC/subC2",
@@ -189,7 +205,6 @@ Array [
         "location": "/command/locC",
       },
     ],
-    "call": Object {},
     "location": "/command",
   },
 ]
@@ -202,13 +217,17 @@ Array [
     "bindings": Array [
       Object {
         "app_id": "1",
-        "bindings": undefined,
+        "call": Object {},
+        "label": "locA",
+        "location": "/post_menu/locA",
+      },
+      Object {
+        "app_id": "1",
         "call": Object {},
         "label": "a",
         "location": "/post_menu/locB",
       },
     ],
-    "call": Object {},
     "location": "/post_menu",
   },
   Object {
@@ -216,25 +235,29 @@ Array [
     "bindings": Array [
       Object {
         "app_id": "2",
-        "bindings": undefined,
         "call": Object {},
         "label": "a",
         "location": "/post_menu/locA",
       },
       Object {
         "app_id": "2",
-        "bindings": undefined,
         "call": Object {},
         "label": "b",
         "location": "/post_menu/locB",
       },
     ],
-    "call": Object {},
     "location": "/post_menu",
   },
   Object {
     "app_id": "3",
-    "bindings": Array [],
+    "bindings": Array [
+      Object {
+        "app_id": "3",
+        "call": Object {},
+        "label": "locA",
+        "location": "/post_menu/locA",
+      },
+    ],
     "location": "/post_menu",
   },
   Object {
@@ -242,14 +265,12 @@ Array [
     "bindings": Array [
       Object {
         "app_id": "1",
-        "bindings": undefined,
         "call": Object {},
         "icon": "icon",
         "label": "b",
         "location": "/channel_header/locB",
       },
     ],
-    "call": Object {},
     "location": "/channel_header",
   },
   Object {
@@ -257,13 +278,11 @@ Array [
     "bindings": Array [
       Object {
         "app_id": "3",
-        "bindings": undefined,
         "call": Object {},
         "label": "c",
         "location": "/command/locC",
       },
     ],
-    "call": Object {},
     "location": "/command",
   },
 ]
@@ -276,13 +295,11 @@ Array [
     "bindings": Array [
       Object {
         "app_id": "1",
-        "bindings": undefined,
         "call": Object {},
         "label": "a",
         "location": "/post_menu/locA",
       },
     ],
-    "call": Object {},
     "location": "/post_menu",
   },
   Object {
@@ -290,13 +307,11 @@ Array [
     "bindings": Array [
       Object {
         "app_id": "2",
-        "bindings": undefined,
         "call": Object {},
         "label": "a",
         "location": "/post_menu/locA",
       },
     ],
-    "call": Object {},
     "location": "/post_menu",
   },
   Object {
@@ -304,14 +319,12 @@ Array [
     "bindings": Array [
       Object {
         "app_id": "1",
-        "bindings": undefined,
         "call": Object {},
         "icon": "icon",
         "label": "b",
         "location": "/channel_header/locB",
       },
     ],
-    "call": Object {},
     "location": "/channel_header",
   },
   Object {
@@ -319,13 +332,11 @@ Array [
     "bindings": Array [
       Object {
         "app_id": "3",
-        "bindings": undefined,
         "call": Object {},
         "label": "c",
         "location": "/command/locC",
       },
     ],
-    "call": Object {},
     "location": "/command",
   },
 ]

app/mm-redux/reducers/entities/apps.ts

@@ -11,8 +11,11 @@ import {validateBindings} from '@utils/apps';
 export function bindings(state: AppBinding[] = [], action: GenericAction): AppBinding[] {
     switch (action.type) {
     case AppsTypes.RECEIVED_APP_BINDINGS: {
-        validateBindings(action.data);
-        return action.data || [];
+        const newBindings = validateBindings(action.data);
+        if (!newBindings.length && !state.length) {
+            return state;
+        }
+        return newBindings;
     }
     default:
         return state;

app/screens/apps_form/apps_form_component.tsx

@@ -18,6 +18,7 @@ import {AppCallRequest, AppField, AppForm, AppFormValue, AppFormValues, AppLooku
 import {DialogElement} from '@mm-redux/types/integrations';
 import {Theme} from '@mm-redux/types/theme';
 import {checkDialogElementForError, checkIfErrorsMatchElements} from '@mm-redux/utils/integration_utils';
+import {filterEmptyOptions} from '@utils/apps';
 import {getMarkdownBlockStyles, getMarkdownTextStyles} from '@utils/markdown';
 import {preventDoubleTap} from '@utils/tap';
 import {changeOpacity, makeStyleSheetFromTheme} from '@utils/theme';
@@ -245,8 +246,11 @@ export default class AppsFormComponent extends PureComponent<Props, State> {
 
         const callResp = res.data!;
         switch (callResp.type) {
-        case AppCallResponseTypes.OK:
-            return callResp.data?.items || [];
+        case AppCallResponseTypes.OK: {
+            let items = callResp.data?.items || [];
+            items = items.filter(filterEmptyOptions);
+            return items;
+        }
         case AppCallResponseTypes.FORM:
         case AppCallResponseTypes.NAVIGATE: {
             const errMsg = intl.formatMessage({

app/utils/apps.ts

@@ -1,135 +1,203 @@
 // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
 // See LICENSE.txt for license information.
-import {AppBindingLocations, AppCallResponseTypes} from '@mm-redux/constants/apps';
+import {AppBindingLocations, AppCallResponseTypes, AppFieldTypes} from '@mm-redux/constants/apps';
 import {getConfig} from '@mm-redux/selectors/entities/general';
-import {AppBinding, AppCall, AppCallRequest, AppCallValues, AppContext, AppExpand} from '@mm-redux/types/apps';
+import {AppBinding, AppCall, AppCallRequest, AppCallValues, AppContext, AppExpand, AppField, AppForm, AppSelectOption} from '@mm-redux/types/apps';
 import {Config} from '@mm-redux/types/config';
 import {GlobalState} from '@mm-redux/types/store';
 
-export function appsEnabled(state: GlobalState) { // eslint-disable-line @typescript-eslint/no-unused-vars
+export function appsEnabled(state: GlobalState): boolean { // eslint-disable-line @typescript-eslint/no-unused-vars
     const enabled = getConfig(state)?.['FeatureFlagAppsEnabled' as keyof Partial<Config>];
     return enabled === 'true';
 }
 
-export function copyAndFillBindings(binding?: AppBinding): AppBinding | undefined {
-    if (!binding) {
-        return undefined;
-    }
-
-    const copy = JSON.parse(JSON.stringify(binding));
-    fillAndTrimBindingsInformation(copy);
-    return copy;
+export function cleanBinding(binding: AppBinding, topLocation: string): AppBinding {
+    return cleanBindingRec(binding, topLocation, 0);
 }
 
-// fillAndTrimBindingsInformation does:
-// - Build the location (e.g. channel_header/binding)
-// - Inherit app calls
-// - Inherit app ids
-// - Trim invalid bindings (do not have an app call or app id at the leaf)
-export function fillAndTrimBindingsInformation(binding?: AppBinding) {
+function cleanBindingRec(binding: AppBinding, topLocation: string, depth: number): AppBinding {
     if (!binding) {
-        return;
+        return binding;
     }
 
-    binding.bindings?.forEach((b) => {
-        // Propagate id down if not defined
+    const toRemove: number[] = [];
+    const usedLabels: {[label: string]: boolean} = {};
+    binding.bindings?.forEach((b, i) => {
+        // Inheritance and defaults
+        if (!b.call && binding.call) {
+            b.call = binding.call;
+        }
+
+        if (b.form) {
+            cleanForm(b.form);
+        } else if (binding.form) {
+            b.form = binding.form;
+        }
+
         if (!b.app_id) {
             b.app_id = binding.app_id;
         }
 
-        // Compose location
-        b.location = binding.location + '/' + b.location;
-
-        // Propagate call down if not defined
-        if (!b.call) {
-            b.call = binding.call;
+        if (!b.label) {
+            b.label = b.location || '';
         }
 
-        fillAndTrimBindingsInformation(b);
+        b.location = binding.location + '/' + b.location;
+
+        // Validation
+        if (!b.label) {
+            toRemove.unshift(i);
+            return;
+        }
+
+        switch (topLocation) {
+        case AppBindingLocations.COMMAND: {
+            if (b.label.match(/ |\t/)) {
+                toRemove.unshift(i);
+                return;
+            }
+
+            if (usedLabels[b.label]) {
+                toRemove.unshift(i);
+                return;
+            }
+            break;
+        }
+        case AppBindingLocations.IN_POST: {
+            if (usedLabels[b.label]) {
+                toRemove.unshift(i);
+                return;
+            }
+            break;
+        }
+        }
+
+        if (b.bindings?.length) {
+            cleanBindingRec(b, topLocation, depth + 1);
+
+            // Remove invalid branches
+            if (!b.bindings?.length) {
+                toRemove.unshift(i);
+                return;
+            }
+        } else {
+            // Remove leaves without a call
+            if (!b.call && !b.form?.call) {
+                toRemove.unshift(i);
+                return;
+            }
+
+            // Remove leaves without app id
+            if (!b.app_id) {
+                toRemove.unshift(i);
+                return;
+            }
+        }
+
+        usedLabels[b.label] = true;
     });
 
-    // Trim branches without app_id
-    if (!binding.app_id) {
-        binding.bindings = binding.bindings?.filter((v) => v.app_id);
-    }
+    toRemove.forEach((i) => {
+        binding.bindings?.splice(i, 1);
+    });
 
-    // Trim branches without calls
-    if (!binding.call) {
-        binding.bindings = binding.bindings?.filter((v) => v.call);
-    }
-
-    // Pull up app_id if needed
-    if (binding.bindings?.length && !binding.app_id) {
-        binding.app_id = binding.bindings[0].app_id;
-    }
-
-    // Pull up call if needed
-    if (binding.bindings?.length && !binding.call) {
-        binding.call = binding.bindings[0].call;
-    }
+    return binding;
 }
 
-export function validateBindings(bindings?: AppBinding[]) {
-    filterInvalidChannelHeaderBindings(bindings);
-    filterInvalidCommands(bindings);
-    filterInvalidPostMenuBindings(bindings);
-    bindings?.forEach(fillAndTrimBindingsInformation);
+export function validateBindings(bindings: AppBinding[] = []): AppBinding[] {
+    const channelHeaderBindings = bindings?.filter((b) => b.location === AppBindingLocations.CHANNEL_HEADER_ICON);
+    const postMenuBindings = bindings?.filter((b) => b.location === AppBindingLocations.POST_MENU_ITEM);
+    const commandBindings = bindings?.filter((b) => b.location === AppBindingLocations.COMMAND);
+
+    channelHeaderBindings.forEach((b) => cleanBinding(b, AppBindingLocations.CHANNEL_HEADER_ICON));
+    postMenuBindings.forEach((b) => cleanBinding(b, AppBindingLocations.POST_MENU_ITEM));
+    commandBindings.forEach((b) => cleanBinding(b, AppBindingLocations.COMMAND));
+
+    const hasBindings = (b: AppBinding) => b.bindings?.length;
+    return postMenuBindings.filter(hasBindings).concat(channelHeaderBindings.filter(hasBindings), commandBindings.filter(hasBindings));
 }
 
-// filterInvalidCommands remove commands without a label
-function filterInvalidCommands(bindings?: AppBinding[]) {
-    if (!bindings) {
+export function cleanForm(form?: AppForm): void {
+    if (!form) {
         return;
     }
 
-    const isValidCommand = (b: AppBinding): boolean => {
-        return Boolean(b.label);
-    };
+    const toRemove: number[] = [];
+    const usedLabels: {[label: string]: boolean} = {};
+    form.fields?.forEach((field, i) => {
+        if (!field.name) {
+            toRemove.unshift(i);
+            return;
+        }
 
-    const validateCommand = (b: AppBinding) => {
-        b.bindings = b.bindings?.filter(isValidCommand);
-        b.bindings?.forEach(validateCommand);
-    };
+        if (field.name.match(/ |\t/)) {
+            toRemove.unshift(i);
+            return;
+        }
 
-    bindings?.filter((b) => b.location === AppBindingLocations.COMMAND).forEach(validateCommand);
+        let label = field.label;
+        if (!label) {
+            label = field.name;
+        }
+
+        if (label.match(/ |\t/)) {
+            toRemove.unshift(i);
+            return;
+        }
+
+        if (usedLabels[label]) {
+            toRemove.unshift(i);
+            return;
+        }
+
+        if (field.type === AppFieldTypes.STATIC_SELECT) {
+            cleanStaticSelect(field);
+            if (!field.options?.length) {
+                toRemove.unshift(i);
+                return;
+            }
+        }
+
+        usedLabels[label] = true;
+    });
+
+    toRemove.forEach((i) => {
+        form.fields.splice(i, 1);
+    });
 }
 
-// filterInvalidChannelHeaderBindings remove bindings
-// without a label.
-function filterInvalidChannelHeaderBindings(bindings?: AppBinding[]) {
-    if (!bindings) {
-        return;
-    }
+function cleanStaticSelect(field: AppField): void {
+    const toRemove: number[] = [];
+    const usedLabels: {[label: string]: boolean} = {};
+    const usedValues: {[label: string]: boolean} = {};
+    field.options?.forEach((option, i) => {
+        let label = option.label;
+        if (!label) {
+            label = option.value;
+        }
 
-    const isValidChannelHeaderBindings = (b: AppBinding): boolean => {
-        return Boolean(b.label);
-    };
+        if (!label) {
+            toRemove.unshift(i);
+            return;
+        }
 
-    const validateChannelHeaderBinding = (b: AppBinding) => {
-        b.bindings = b.bindings?.filter(isValidChannelHeaderBindings);
-        b.bindings?.forEach(validateChannelHeaderBinding);
-    };
+        if (usedLabels[label]) {
+            toRemove.unshift(i);
+            return;
+        }
 
-    bindings?.filter((b) => b.location === AppBindingLocations.CHANNEL_HEADER_ICON).forEach(validateChannelHeaderBinding);
-}
+        if (usedValues[option.value]) {
+            toRemove.unshift(i);
+            return;
+        }
 
-// filterInvalidPostMenuBindings remove bindings
-// without a label.
-function filterInvalidPostMenuBindings(bindings?: AppBinding[]) {
-    if (!bindings) {
-        return;
-    }
+        usedLabels[label] = true;
+        usedValues[option.value] = true;
+    });
 
-    const isValidPostMenuBinding = (b: AppBinding): boolean => {
-        return Boolean(b.label);
-    };
-
-    const validatePostMenuBinding = (b: AppBinding) => {
-        b.bindings = b.bindings?.filter(isValidPostMenuBinding);
-        b.bindings?.forEach(validatePostMenuBinding);
-    };
-
-    bindings?.filter((b) => b.location === AppBindingLocations.POST_MENU_ITEM).forEach(validatePostMenuBinding);
+    toRemove.forEach((i) => {
+        field.options?.splice(i, 1);
+    });
 }
 
 export function createCallContext(
@@ -179,3 +247,5 @@ export const makeCallErrorResponse = (errMessage: string) => {
         error: errMessage,
     };
 };
+
+export const filterEmptyOptions = (option: AppSelectOption): boolean => Boolean(option.value && !option.value.match(/^[ \t]+$/));