diff --git a/app/actions/apps.ts b/app/actions/apps.ts index 6e3db6182..8d1dee7dc 100644 --- a/app/actions/apps.ts +++ b/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(call: AppCallRequest, type: AppCallType, intl: any): ActionFunc { return async () => { @@ -30,6 +30,8 @@ export function doAppCall(call: AppCallRequest, type: AppCallType, return {error: makeCallErrorResponse(errMsg)}; } + cleanForm(res.form); + return {data: res}; } case AppCallResponseTypes.NAVIGATE: 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 eda4d1116..cac67bb4a 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 @@ -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 || '', 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 e8938b2b7..821d01ddd 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 @@ -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; diff --git a/app/components/post_list/post/body/content/embedded_bindings/embedded_binding.tsx b/app/components/post_list/post/body/content/embedded_bindings/embedded_binding.tsx index b79ca98e7..12e40a594 100644 --- a/app/components/post_list/post/body/content/embedded_bindings/embedded_binding.tsx +++ b/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 ( <> diff --git a/app/mm-redux/reducers/entities/__snapshots__/apps.test.js.snap b/app/mm-redux/reducers/entities/__snapshots__/apps.test.js.snap index 1929a615c..cb20632f1 100644 --- a/app/mm-redux/reducers/entities/__snapshots__/apps.test.js.snap +++ b/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", }, ] diff --git a/app/mm-redux/reducers/entities/apps.ts b/app/mm-redux/reducers/entities/apps.ts index 2a0360804..7b6398c31 100644 --- a/app/mm-redux/reducers/entities/apps.ts +++ b/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; diff --git a/app/screens/apps_form/apps_form_component.tsx b/app/screens/apps_form/apps_form_component.tsx index fc2a1a338..a505247d8 100644 --- a/app/screens/apps_form/apps_form_component.tsx +++ b/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 { 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({ diff --git a/app/utils/apps.test.ts b/app/utils/apps.test.ts index 57f7bd5bf..aef81a952 100644 --- a/app/utils/apps.test.ts +++ b/app/utils/apps.test.ts @@ -1,12 +1,13 @@ // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See LICENSE.txt for license information. -import {AppBinding, AppCall} from '@mm-redux/types/apps'; +import {AppBindingLocations, AppFieldTypes} from '@mm-redux/constants/apps'; +import {AppBinding, AppCall, AppField, AppForm, AppSelectOption} from '@mm-redux/types/apps'; -import {fillAndTrimBindingsInformation} from './apps'; +import {cleanForm, cleanBinding} from './apps'; describe('Apps Utils', () => { - describe('fillAndTrimBindingsInformation', () => { + describe('cleanBindings', () => { test('Apps IDs, and Calls propagate down, and locations get formed', () => { const inBinding: AppBinding = { app_id: 'id', @@ -42,6 +43,7 @@ describe('Apps Utils', () => { { location: 'loc1/loc2', app_id: 'id', + label: 'loc2', call: { path: 'url', } as AppCall, @@ -49,6 +51,7 @@ describe('Apps Utils', () => { { location: 'loc1/loc2/loc3', app_id: 'id', + label: 'loc3', call: { path: 'url', } as AppCall, @@ -56,6 +59,7 @@ describe('Apps Utils', () => { { location: 'loc1/loc2/loc4', app_id: 'id', + label: 'loc4', call: { path: 'url', } as AppCall, @@ -65,6 +69,7 @@ describe('Apps Utils', () => { { location: 'loc1/loc5', app_id: 'id', + label: 'loc5', call: { path: 'url', } as AppCall, @@ -72,7 +77,7 @@ describe('Apps Utils', () => { ], } as AppBinding; - fillAndTrimBindingsInformation(inBinding); + cleanBinding(inBinding, ''); expect(inBinding).toEqual(outBinding); }); @@ -115,6 +120,7 @@ describe('Apps Utils', () => { { location: 'loc1/loc2', app_id: 'id2', + label: 'loc2', call: { path: 'url', } as AppCall, @@ -122,6 +128,7 @@ describe('Apps Utils', () => { { location: 'loc1/loc2/loc3', app_id: 'id2', + label: 'loc3', call: { path: 'url', } as AppCall, @@ -129,6 +136,7 @@ describe('Apps Utils', () => { { location: 'loc1/loc2/loc4', app_id: 'id2', + label: 'loc4', call: { path: 'url', } as AppCall, @@ -138,6 +146,7 @@ describe('Apps Utils', () => { { location: 'loc1/loc5', app_id: 'id', + label: 'loc5', call: { path: 'url2', } as AppCall, @@ -145,155 +154,7 @@ describe('Apps Utils', () => { ], } as AppBinding; - fillAndTrimBindingsInformation(inBinding); - expect(inBinding).toEqual(outBinding); - }); - - test('Populate ids on the way up.', () => { - const inBinding: AppBinding = { - location: 'loc1', - call: { - path: 'url', - } as AppCall, - bindings: [ - { - location: 'loc2', - bindings: [ - { - app_id: 'id1', - location: 'loc3', - } as AppBinding, - { - app_id: 'id2', - location: 'loc4', - } as AppBinding, - ], - } as AppBinding, - { - app_id: 'id3', - location: 'loc5', - } as AppBinding, - ], - } as AppBinding; - - const outBinding: AppBinding = { - app_id: 'id1', - location: 'loc1', - call: { - path: 'url', - } as AppCall, - bindings: [ - { - location: 'loc1/loc2', - app_id: 'id1', - call: { - path: 'url', - } as AppCall, - bindings: [ - { - location: 'loc1/loc2/loc3', - app_id: 'id1', - call: { - path: 'url', - } as AppCall, - } as AppBinding, - { - location: 'loc1/loc2/loc4', - app_id: 'id2', - call: { - path: 'url', - } as AppCall, - } as AppBinding, - ], - } as AppBinding, - { - location: 'loc1/loc5', - app_id: 'id3', - call: { - path: 'url', - } as AppCall, - }, - ], - } as AppBinding; - - fillAndTrimBindingsInformation(inBinding); - expect(inBinding).toEqual(outBinding); - }); - - test('Populate calls on the way up.', () => { - const inBinding: AppBinding = { - location: 'loc1', - bindings: [ - { - location: 'loc2', - bindings: [ - { - app_id: 'id1', - location: 'loc3', - call: { - path: 'url1', - } as AppCall, - } as AppBinding, - { - app_id: 'id2', - location: 'loc4', - call: { - path: 'url2', - } as AppCall, - } as AppBinding, - ], - } as AppBinding, - { - app_id: 'id3', - location: 'loc5', - call: { - path: 'url3', - } as AppCall, - } as AppBinding, - ], - } as AppBinding; - - const outBinding: AppBinding = { - app_id: 'id1', - location: 'loc1', - call: { - path: 'url1', - } as AppCall, - bindings: [ - { - location: 'loc1/loc2', - app_id: 'id1', - call: { - path: 'url1', - } as AppCall, - bindings: [ - { - location: 'loc1/loc2/loc3', - app_id: 'id1', - call: { - path: 'url1', - } as AppCall, - } as AppBinding, - { - location: 'loc1/loc2/loc4', - app_id: 'id2', - call: { - path: 'url2', - } as AppCall, - } as AppBinding, - ], - } as AppBinding, - { - location: 'loc1/loc5', - app_id: 'id3', - call: { - path: 'url3', - } as AppCall, - }, - ], - } as AppBinding; - - fillAndTrimBindingsInformation(inBinding); + cleanBinding(inBinding, ''); expect(inBinding).toEqual(outBinding); }); @@ -323,7 +184,6 @@ describe('Apps Utils', () => { } as AppBinding; const outBinding: AppBinding = { - app_id: 'id', location: 'loc1', call: { path: 'url', @@ -331,7 +191,7 @@ describe('Apps Utils', () => { bindings: [ { location: 'loc1/loc2', - app_id: 'id', + label: 'loc2', call: { path: 'url', } as AppCall, @@ -339,6 +199,7 @@ describe('Apps Utils', () => { { location: 'loc1/loc2/loc4', app_id: 'id', + label: 'loc4', call: { path: 'url', } as AppCall, @@ -348,7 +209,7 @@ describe('Apps Utils', () => { ], } as AppBinding; - fillAndTrimBindingsInformation(inBinding); + cleanBinding(inBinding, ''); expect(inBinding).toEqual(outBinding); }); @@ -367,7 +228,7 @@ describe('Apps Utils', () => { location: 'loc4', call: { path: 'url', - } as AppCall, + }, } as AppBinding, ], } as AppBinding, @@ -380,30 +241,26 @@ describe('Apps Utils', () => { const outBinding: AppBinding = { app_id: 'id', location: 'loc1', - call: { - path: 'url', - } as AppCall, bindings: [ { location: 'loc1/loc2', app_id: 'id', - call: { - path: 'url', - } as AppCall, + label: 'loc2', bindings: [ { location: 'loc1/loc2/loc4', app_id: 'id', call: { path: 'url', - } as AppCall, + }, + label: 'loc4', } as AppBinding, ], } as AppBinding, ], } as AppBinding; - fillAndTrimBindingsInformation(inBinding); + cleanBinding(inBinding, ''); expect(inBinding).toEqual(outBinding); }); @@ -437,7 +294,995 @@ describe('Apps Utils', () => { bindings: [] as AppBinding[], } as AppBinding; - fillAndTrimBindingsInformation(inBinding); + cleanBinding(inBinding, ''); + expect(inBinding).toEqual(outBinding); + }); + test('Do not filter bindings with no call but with a form with a call', () => { + const inBinding = { + app_id: 'appID', + location: 'loc1', + bindings: [ + { + location: 'loc2', + form: { + call: { + path: 'url', + }, + }, + } as AppBinding, + ], + } as AppBinding; + + const outBinding = { + app_id: 'appID', + location: 'loc1', + bindings: [ + { + app_id: 'appID', + location: 'loc1/loc2', + label: 'loc2', + form: { + call: { + path: 'url', + }, + }, + }, + ], + } as AppBinding; + + cleanBinding(inBinding, ''); + expect(inBinding).toEqual(outBinding); + }); + }); + describe('cleanForm', () => { + test('no field filter on names', () => { + const inForm: AppForm = { + fields: [ + { + name: 'opt1', + type: AppFieldTypes.TEXT, + }, + { + name: 'opt2', + type: AppFieldTypes.TEXT, + }, + ], + }; + const outForm: AppForm = { + fields: [ + { + name: 'opt1', + type: AppFieldTypes.TEXT, + }, + { + name: 'opt2', + type: AppFieldTypes.TEXT, + }, + ], + }; + + cleanForm(inForm); + expect(inForm).toEqual(outForm); + }); + test('no field filter on labels', () => { + const inForm: AppForm = { + fields: [ + { + name: 'opt1', + type: AppFieldTypes.TEXT, + label: 'opt1', + }, + { + name: 'opt2', + type: AppFieldTypes.TEXT, + label: 'opt2', + }, + ], + }; + const outForm: AppForm = { + fields: [ + { + name: 'opt1', + type: AppFieldTypes.TEXT, + label: 'opt1', + }, + { + name: 'opt2', + type: AppFieldTypes.TEXT, + label: 'opt2', + }, + ], + }; + + cleanForm(inForm); + expect(inForm).toEqual(outForm); + }); + test('no field filter with no name', () => { + const inForm: AppForm = { + fields: [ + { + type: AppFieldTypes.TEXT, + label: 'opt1', + } as AppField, + { + name: 'opt2', + type: AppFieldTypes.TEXT, + label: 'opt2', + }, + ], + }; + const outForm: AppForm = { + fields: [ + { + name: 'opt2', + type: AppFieldTypes.TEXT, + label: 'opt2', + }, + ], + }; + + cleanForm(inForm); + expect(inForm).toEqual(outForm); + }); + test('field filter with same label inerred from name', () => { + const inForm: AppForm = { + fields: [ + { + name: 'same', + type: AppFieldTypes.TEXT, + }, + { + name: 'same', + type: AppFieldTypes.BOOL, + }, + ], + }; + const outForm: AppForm = { + fields: [ + { + name: 'same', + type: AppFieldTypes.TEXT, + }, + ], + }; + + cleanForm(inForm); + expect(inForm).toEqual(outForm); + }); + test('field filter with same labels', () => { + const inForm: AppForm = { + fields: [ + { + name: 'opt1', + type: AppFieldTypes.TEXT, + label: 'same', + }, + { + name: 'opt2', + type: AppFieldTypes.TEXT, + label: 'same', + }, + ], + }; + const outForm: AppForm = { + fields: [ + { + name: 'opt1', + type: AppFieldTypes.TEXT, + label: 'same', + }, + ], + }; + + cleanForm(inForm); + expect(inForm).toEqual(outForm); + }); + test('field filter with multiword name', () => { + const inForm: AppForm = { + fields: [ + { + name: 'opt1', + type: AppFieldTypes.TEXT, + label: 'opt1', + }, + { + name: 'opt 2', + type: AppFieldTypes.TEXT, + label: 'opt2', + }, + ], + }; + const outForm: AppForm = { + fields: [ + { + name: 'opt1', + type: AppFieldTypes.TEXT, + label: 'opt1', + }, + ], + }; + + cleanForm(inForm); + expect(inForm).toEqual(outForm); + }); + test('field filter with multiword label', () => { + const inForm: AppForm = { + fields: [ + { + name: 'opt1', + type: AppFieldTypes.TEXT, + label: 'opt1', + }, + { + name: 'opt2', + type: AppFieldTypes.TEXT, + label: 'opt 2', + }, + ], + }; + const outForm: AppForm = { + fields: [ + { + name: 'opt1', + type: AppFieldTypes.TEXT, + label: 'opt1', + }, + ], + }; + + cleanForm(inForm); + expect(inForm).toEqual(outForm); + }); + test('field filter more than one field', () => { + const inForm: AppForm = { + fields: [ + { + name: 'opt1', + type: AppFieldTypes.TEXT, + label: 'same', + }, + { + name: 'opt2', + type: AppFieldTypes.BOOL, + label: 'same', + }, + { + name: 'opt2', + type: AppFieldTypes.USER, + label: 'same', + }, + ], + }; + const outForm: AppForm = { + fields: [ + { + name: 'opt1', + type: AppFieldTypes.TEXT, + label: 'same', + }, + ], + }; + + cleanForm(inForm); + expect(inForm).toEqual(outForm); + }); + test('field filter static with no options', () => { + const inForm: AppForm = { + fields: [ + { + name: 'opt1', + type: AppFieldTypes.STATIC_SELECT, + }, + ], + }; + const outForm: AppForm = { + fields: [], + }; + + cleanForm(inForm); + expect(inForm).toEqual(outForm); + }); + test('field filter static options with no value', () => { + const inForm: AppForm = { + fields: [ + { + name: 'opt1', + type: AppFieldTypes.STATIC_SELECT, + options: [ + {value: 'opt1'} as AppSelectOption, + {} as AppSelectOption, + ], + }, + ], + }; + const outForm: AppForm = { + fields: [ + { + name: 'opt1', + type: AppFieldTypes.STATIC_SELECT, + options: [ + {value: 'opt1'} as AppSelectOption, + ], + }, + ], + }; + + cleanForm(inForm); + expect(inForm).toEqual(outForm); + }); + test('field filter static options with same label inferred from value', () => { + const inForm: AppForm = { + fields: [ + { + name: 'opt1', + type: AppFieldTypes.STATIC_SELECT, + options: [ + { + value: 'same', + icon_data: 'opt1', + } as AppSelectOption, + { + value: 'same', + icon_data: 'opt2', + } as AppSelectOption, + ], + }, + ], + }; + const outForm: AppForm = { + fields: [ + { + name: 'opt1', + type: AppFieldTypes.STATIC_SELECT, + options: [ + { + value: 'same', + icon_data: 'opt1', + } as AppSelectOption, + ], + }, + ], + }; + + cleanForm(inForm); + expect(inForm).toEqual(outForm); + }); + test('field filter static options with same label', () => { + const inForm: AppForm = { + fields: [ + { + name: 'opt1', + type: AppFieldTypes.STATIC_SELECT, + options: [ + { + value: 'opt1', + label: 'same', + }, + { + value: 'opt2', + label: 'same', + }, + ], + }, + ], + }; + const outForm: AppForm = { + fields: [ + { + name: 'opt1', + type: AppFieldTypes.STATIC_SELECT, + options: [ + { + value: 'opt1', + label: 'same', + }, + ], + }, + ], + }; + + cleanForm(inForm); + expect(inForm).toEqual(outForm); + }); + test('field filter static options with same value', () => { + const inForm: AppForm = { + fields: [ + { + name: 'opt1', + type: AppFieldTypes.STATIC_SELECT, + options: [ + { + label: 'opt1', + value: 'same', + }, + { + label: 'opt2', + value: 'same', + }, + ], + }, + ], + }; + const outForm: AppForm = { + fields: [ + { + name: 'opt1', + type: AppFieldTypes.STATIC_SELECT, + options: [ + { + label: 'opt1', + value: 'same', + }, + ], + }, + ], + }; + + cleanForm(inForm); + expect(inForm).toEqual(outForm); + }); + test('invalid static options don\'t consume namespace', () => { + const inForm: AppForm = { + fields: [ + { + name: 'opt1', + type: AppFieldTypes.STATIC_SELECT, + options: [ + { + label: 'same1', + value: 'same1', + }, + { + label: 'same1', + value: 'same2', + }, + { + label: 'same2', + value: 'same1', + }, + { + label: 'same2', + value: 'same2', + }, + ], + }, + ], + }; + const outForm: AppForm = { + fields: [ + { + name: 'opt1', + type: AppFieldTypes.STATIC_SELECT, + options: [ + { + label: 'same1', + value: 'same1', + }, + { + label: 'same2', + value: 'same2', + }, + ], + }, + ], + }; + + cleanForm(inForm); + expect(inForm).toEqual(outForm); + }); + test('field filter static with no valid options', () => { + const inForm: AppForm = { + fields: [ + { + name: 'opt1', + type: AppFieldTypes.STATIC_SELECT, + options: [ + {} as AppSelectOption, + ], + }, + ], + }; + const outForm: AppForm = { + fields: [], + }; + + cleanForm(inForm); + expect(inForm).toEqual(outForm); + }); + test('invalid static field does not consume namespace', () => { + const inForm: AppForm = { + fields: [ + { + name: 'field1', + type: AppFieldTypes.STATIC_SELECT, + options: [ + {} as AppSelectOption, + ], + }, + { + name: 'field1', + type: AppFieldTypes.TEXT, + }, + ], + }; + const outForm: AppForm = { + fields: [ + { + name: 'field1', + type: AppFieldTypes.TEXT, + }, + ], + }; + + cleanForm(inForm); + expect(inForm).toEqual(outForm); + }); + }); + describe('cleanCommands', () => { + test('happy path', () => { + const inBinding: AppBinding = { + location: '/command', + bindings: [ + { + app_id: 'app', + location: 'loc1', + label: 'loc1', + bindings: [ + { + app_id: 'app', + location: 'loc11', + label: 'loc11', + call: { + path: '/path', + }, + }, + { + app_id: 'app', + location: 'loc12', + label: 'loc12', + call: { + path: '/path', + }, + }, + ], + }, + { + app_id: 'app', + location: 'loc2', + label: 'loc2', + call: { + path: '/path', + }, + }, + ], + } as AppBinding; + const outBinding: AppBinding = { + location: '/command', + bindings: [ + { + app_id: 'app', + location: '/command/loc1', + label: 'loc1', + bindings: [ + { + app_id: 'app', + location: '/command/loc1/loc11', + label: 'loc11', + call: { + path: '/path', + }, + }, + { + app_id: 'app', + location: '/command/loc1/loc12', + label: 'loc12', + call: { + path: '/path', + }, + }, + ], + }, + { + app_id: 'app', + location: '/command/loc2', + label: 'loc2', + call: { + path: '/path', + }, + }, + ], + } as AppBinding; + + cleanBinding(inBinding, AppBindingLocations.COMMAND); + expect(inBinding).toEqual(outBinding); + }); + test('no label nor location', () => { + const inBinding: AppBinding = { + location: '/command', + bindings: [ + { + app_id: 'app', + location: 'loc1', + label: 'loc1', + bindings: [ + { + app_id: 'app', + location: 'loc11', + label: 'loc11', + call: { + path: '/path', + }, + }, + { + app_id: 'app', + call: { + path: '/path', + }, + } as AppBinding, + ], + }, + { + app_id: 'app', + location: 'loc2', + label: 'loc2', + call: { + path: '/path', + }, + }, + ], + } as AppBinding; + const outBinding: AppBinding = { + location: '/command', + bindings: [ + { + app_id: 'app', + location: '/command/loc1', + label: 'loc1', + bindings: [ + { + app_id: 'app', + location: '/command/loc1/loc11', + label: 'loc11', + call: { + path: '/path', + }, + }, + ], + }, + { + app_id: 'app', + location: '/command/loc2', + label: 'loc2', + call: { + path: '/path', + }, + }, + ], + } as AppBinding; + + cleanBinding(inBinding, AppBindingLocations.COMMAND); + expect(inBinding).toEqual(outBinding); + }); + test('no multiword infered from location', () => { + const inBinding: AppBinding = { + location: '/command', + bindings: [ + { + app_id: 'app', + location: 'loc1', + label: 'loc1', + bindings: [ + { + app_id: 'app', + location: 'loc11', + label: 'loc11', + call: { + path: '/path', + }, + }, + { + app_id: 'app', + location: 'loc1 2', + call: { + path: '/path', + }, + } as AppBinding, + ], + }, + { + app_id: 'app', + location: 'loc2', + label: 'loc2', + call: { + path: '/path', + }, + }, + ], + } as AppBinding; + const outBinding: AppBinding = { + location: '/command', + bindings: [ + { + app_id: 'app', + location: '/command/loc1', + label: 'loc1', + bindings: [ + { + app_id: 'app', + location: '/command/loc1/loc11', + label: 'loc11', + call: { + path: '/path', + }, + }, + ], + }, + { + app_id: 'app', + location: '/command/loc2', + label: 'loc2', + call: { + path: '/path', + }, + }, + ], + } as AppBinding; + + cleanBinding(inBinding, AppBindingLocations.COMMAND); + expect(inBinding).toEqual(outBinding); + }); + test('no multiword on label', () => { + const inBinding: AppBinding = { + location: '/command', + bindings: [ + { + app_id: 'app', + location: 'loc1', + label: 'loc1', + bindings: [ + { + app_id: 'app', + location: 'loc11', + label: 'loc11', + call: { + path: '/path', + }, + }, + { + app_id: 'app', + location: 'loc12', + label: 'loc1 2', + call: { + path: '/path', + }, + }, + ], + }, + { + app_id: 'app', + location: 'loc2', + label: 'loc2', + call: { + path: '/path', + }, + }, + ], + } as AppBinding; + const outBinding: AppBinding = { + location: '/command', + bindings: [ + { + app_id: 'app', + location: '/command/loc1', + label: 'loc1', + bindings: [ + { + app_id: 'app', + location: '/command/loc1/loc11', + label: 'loc11', + call: { + path: '/path', + }, + }, + ], + }, + { + app_id: 'app', + location: '/command/loc2', + label: 'loc2', + call: { + path: '/path', + }, + }, + ], + } as AppBinding; + + cleanBinding(inBinding, AppBindingLocations.COMMAND); + expect(inBinding).toEqual(outBinding); + }); + test('filter repeated label', () => { + const inBinding: AppBinding = { + location: '/command', + bindings: [ + { + app_id: 'app', + location: 'loc1', + label: 'loc1', + bindings: [ + { + app_id: 'app', + location: 'same', + description: 'loc11', + call: { + path: '/path', + }, + } as AppBinding, + { + app_id: 'app', + location: 'same', + description: 'loc12', + call: { + path: '/path', + }, + } as AppBinding, + ], + }, + { + app_id: 'app', + location: 'loc2', + label: 'loc2', + call: { + path: '/path', + }, + }, + ], + } as AppBinding; + const outBinding: AppBinding = { + location: '/command', + bindings: [ + { + app_id: 'app', + location: '/command/loc1', + label: 'loc1', + bindings: [ + { + app_id: 'app', + location: '/command/loc1/same', + label: 'same', + description: 'loc11', + call: { + path: '/path', + }, + } as AppBinding, + ], + }, + { + app_id: 'app', + location: '/command/loc2', + label: 'loc2', + call: { + path: '/path', + }, + }, + ], + } as AppBinding; + + cleanBinding(inBinding, AppBindingLocations.COMMAND); + expect(inBinding).toEqual(outBinding); + }); + test('filter with same label', () => { + const inBinding: AppBinding = { + location: '/command', + bindings: [ + { + app_id: 'app', + location: 'loc1', + label: 'loc1', + bindings: [ + { + app_id: 'app', + location: 'loc11', + label: 'same', + call: { + path: '/path', + }, + }, + { + app_id: 'app', + location: 'loc12', + label: 'same', + call: { + path: '/path', + }, + }, + ], + }, + { + app_id: 'app', + location: 'loc2', + label: 'loc2', + call: { + path: '/path', + }, + }, + ], + } as AppBinding; + const outBinding: AppBinding = { + location: '/command', + bindings: [ + { + app_id: 'app', + location: '/command/loc1', + label: 'loc1', + bindings: [ + { + app_id: 'app', + location: '/command/loc1/loc11', + label: 'same', + call: { + path: '/path', + }, + }, + ], + }, + { + app_id: 'app', + location: '/command/loc2', + label: 'loc2', + call: { + path: '/path', + }, + }, + ], + } as AppBinding; + + cleanBinding(inBinding, AppBindingLocations.COMMAND); + expect(inBinding).toEqual(outBinding); + }); + test('non-leaf command removed when it has no subcommands', () => { + const inBinding: AppBinding = { + location: '/command', + bindings: [ + { + app_id: 'app', + location: 'loc1', + label: 'loc1', + bindings: [ + { + app_id: 'app', + location: 'loc11', + label: 'loc 1 1', + call: { + path: '/path', + }, + }, + ], + }, + { + app_id: 'app', + location: 'loc2', + label: 'loc2', + call: { + path: '/path', + }, + }, + ], + } as AppBinding; + const outBinding: AppBinding = { + location: '/command', + bindings: [ + { + app_id: 'app', + location: '/command/loc2', + label: 'loc2', + call: { + path: '/path', + }, + }, + ], + } as AppBinding; + + cleanBinding(inBinding, AppBindingLocations.COMMAND); expect(inBinding).toEqual(outBinding); }); }); diff --git a/app/utils/apps.ts b/app/utils/apps.ts index 0e72d6c2b..c53e9832f 100644 --- a/app/utils/apps.ts +++ b/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]; 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]+$/));