From 00f7ef3510638eefced17f6854d1f7710c1c3562 Mon Sep 17 00:00:00 2001 From: Gidi Meir Morris Date: Tue, 24 Nov 2020 12:23:43 +0000 Subject: [PATCH 1/4] fixed buggy default message behaviour --- .../email/email_params.tsx | 17 ++++++------ .../server_log/server_log_params.tsx | 26 +++++++++---------- .../slack/slack_params.tsx | 19 +++++++------- 3 files changed, 32 insertions(+), 30 deletions(-) diff --git a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/email/email_params.tsx b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/email/email_params.tsx index eacdf20747fdc..1030ce34d2569 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/email/email_params.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/email/email_params.tsx @@ -11,7 +11,6 @@ import { ActionParamsProps } from '../../../../types'; import { EmailActionParams } from '../types'; import { TextFieldWithMessageVariables } from '../../text_field_with_message_variables'; import { TextAreaWithMessageVariables } from '../../text_area_with_message_variables'; -import { resolvedActionGroupMessage } from '../../../constants'; export const EmailParamsFields = ({ actionParams, @@ -28,17 +27,19 @@ export const EmailParamsFields = ({ const [addCC, setAddCC] = useState(false); const [addBCC, setAddBCC] = useState(false); + const [[isUsingDefault, defaultMessageUsed], setDefaultMessageUsage] = useState< + [boolean, string | undefined] + >([false, defaultMessage]); useEffect(() => { - if (defaultMessage === resolvedActionGroupMessage) { - editAction('message', defaultMessage, index); - } else if ( - (!message || message === resolvedActionGroupMessage) && - defaultMessage && - defaultMessage.length > 0 + if ( + !actionParams?.message || + (isUsingDefault && + actionParams?.message === defaultMessageUsed && + defaultMessageUsed !== defaultMessage) ) { + setDefaultMessageUsage([true, defaultMessage]); editAction('message', defaultMessage, index); } - // eslint-disable-next-line react-hooks/exhaustive-deps }, [defaultMessage]); diff --git a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/server_log/server_log_params.tsx b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/server_log/server_log_params.tsx index e8c427371c4a5..ea734439ef41a 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/server_log/server_log_params.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/server_log/server_log_params.tsx @@ -3,17 +3,16 @@ * or more contributor license agreements. Licensed under the Elastic License; * you may not use this file except in compliance with the Elastic License. */ -import React, { Fragment, useEffect } from 'react'; +import React, { Fragment, useEffect, useState } from 'react'; import { i18n } from '@kbn/i18n'; import { EuiSelect, EuiFormRow } from '@elastic/eui'; import { ActionParamsProps } from '../../../../types'; import { ServerLogActionParams } from '.././types'; import { TextAreaWithMessageVariables } from '../../text_area_with_message_variables'; -import { resolvedActionGroupMessage } from '../../../constants'; -export const ServerLogParamsFields: React.FunctionComponent< - ActionParamsProps -> = ({ actionParams, editAction, index, errors, messageVariables, defaultMessage }) => { +export const ServerLogParamsFields: React.FunctionComponent> = ({ actionParams, editAction, index, errors, messageVariables, defaultMessage }) => { const { message, level } = actionParams; const levelOptions = [ { value: 'trace', text: 'Trace' }, @@ -23,7 +22,6 @@ export const ServerLogParamsFields: React.FunctionComponent< { value: 'error', text: 'Error' }, { value: 'fatal', text: 'Fatal' }, ]; - useEffect(() => { if (!actionParams?.level) { editAction('level', 'info', index); @@ -31,17 +29,19 @@ export const ServerLogParamsFields: React.FunctionComponent< // eslint-disable-next-line react-hooks/exhaustive-deps }, []); + const [[isUsingDefault, defaultMessageUsed], setDefaultMessageUsage] = useState< + [boolean, string | undefined] + >([false, defaultMessage]); useEffect(() => { - if (defaultMessage === resolvedActionGroupMessage) { - editAction('message', defaultMessage, index); - } else if ( - (!message || message === resolvedActionGroupMessage) && - defaultMessage && - defaultMessage.length > 0 + if ( + !actionParams?.message || + (isUsingDefault && + actionParams?.message === defaultMessageUsed && + defaultMessageUsed !== defaultMessage) ) { + setDefaultMessageUsage([true, defaultMessage]); editAction('message', defaultMessage, index); } - // eslint-disable-next-line react-hooks/exhaustive-deps }, [defaultMessage]); diff --git a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/slack/slack_params.tsx b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/slack/slack_params.tsx index d1498567218d3..40818c327af60 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/slack/slack_params.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/slack/slack_params.tsx @@ -3,12 +3,11 @@ * or more contributor license agreements. Licensed under the Elastic License; * you may not use this file except in compliance with the Elastic License. */ -import React, { useEffect } from 'react'; +import React, { useEffect, useState } from 'react'; import { i18n } from '@kbn/i18n'; import { ActionParamsProps } from '../../../../types'; import { SlackActionParams } from '../types'; import { TextAreaWithMessageVariables } from '../../text_area_with_message_variables'; -import { resolvedActionGroupMessage } from '../../../constants'; const SlackParamsFields: React.FunctionComponent> = ({ actionParams, @@ -19,17 +18,19 @@ const SlackParamsFields: React.FunctionComponent { const { message } = actionParams; + const [[isUsingDefault, defaultMessageUsed], setDefaultMessageUsage] = useState< + [boolean, string | undefined] + >([false, defaultMessage]); useEffect(() => { - if (defaultMessage === resolvedActionGroupMessage) { - editAction('message', defaultMessage, index); - } else if ( - (!message || message === resolvedActionGroupMessage) && - defaultMessage && - defaultMessage.length > 0 + if ( + !actionParams?.message || + (isUsingDefault && + actionParams?.message === defaultMessageUsed && + defaultMessageUsed !== defaultMessage) ) { + setDefaultMessageUsage([true, defaultMessage]); editAction('message', defaultMessage, index); } - // eslint-disable-next-line react-hooks/exhaustive-deps }, [defaultMessage]); From f9c7560cd96df8a2fcac209f423da2173114daf0 Mon Sep 17 00:00:00 2001 From: Gidi Meir Morris Date: Tue, 24 Nov 2020 15:26:31 +0000 Subject: [PATCH 2/4] added missing test --- .../email/email_params.test.tsx | 93 +++++++++++++++++ .../server_log/server_log_params.test.tsx | 99 +++++++++++++++++-- .../server_log/server_log_params.tsx | 8 +- .../triggers_actions_ui/public/types.ts | 2 +- 4 files changed, 189 insertions(+), 13 deletions(-) diff --git a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/email/email_params.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/email/email_params.test.tsx index 3cd54b58bf29a..1020b784f3c58 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/email/email_params.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/email/email_params.test.tsx @@ -33,4 +33,97 @@ describe('EmailParamsFields renders', () => { expect(wrapper.find('[data-test-subj="subjectInput"]').length > 0).toBeTruthy(); expect(wrapper.find('[data-test-subj="messageTextArea"]').length > 0).toBeTruthy(); }); + + test('message param field is rendered with default value if not set', () => { + const actionParams = { + cc: [], + bcc: [], + to: ['test@test.com'], + subject: 'test', + }; + + const editAction = jest.fn(); + const wrapper = mountWithIntl( + + ); + + expect(editAction).toHaveBeenCalledWith('message', 'Some default message', 0); + }); + + test('when the default message changes, so is the underlying message if it was set by the previous default', () => { + const actionParams = { + cc: [], + bcc: [], + to: ['test@test.com'], + subject: 'test', + }; + + const editAction = jest.fn(); + const wrapper = mountWithIntl( + + ); + + expect(editAction).toHaveBeenCalledWith('message', 'Some default message', 0); + + wrapper.setProps({ + defaultMessage: 'Some different default message', + }); + + expect(editAction).toHaveBeenCalledWith('message', 'Some different default message', 0); + }); + + test('when the default message changes, it doesnt change the underlying message if it wasnt set by a previous default', () => { + const actionParams = { + cc: [], + bcc: [], + to: ['test@test.com'], + subject: 'test', + }; + + const editAction = jest.fn(); + const wrapper = mountWithIntl( + + ); + + expect(editAction).toHaveBeenCalledWith('message', 'Some default message', 0); + + // simulate value being updated + const valueToSimulate = 'some new value'; + wrapper + .find('[data-test-subj="messageTextArea"]') + .first() + .simulate('change', { target: { value: valueToSimulate } }); + expect(editAction).toHaveBeenCalledWith('message', valueToSimulate, 0); + wrapper.setProps({ + actionParams: { + ...actionParams, + message: valueToSimulate, + }, + }); + + // simulate default changing + wrapper.setProps({ + defaultMessage: 'Some different default message', + }); + + expect(editAction).not.toHaveBeenCalledWith('message', 'Some different default message', 0); + }); }); diff --git a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/server_log/server_log_params.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/server_log/server_log_params.test.tsx index 0552a126ca66f..86dd304bf6b12 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/server_log/server_log_params.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/server_log/server_log_params.test.tsx @@ -9,8 +9,8 @@ import { ServerLogLevelOptions } from '.././types'; import ServerLogParamsFields from './server_log_params'; describe('ServerLogParamsFields renders', () => { - const editAction = jest.fn(); test('all params fields is rendered', () => { + const editAction = jest.fn(); const actionParams = { level: ServerLogLevelOptions.TRACE, message: 'test', @@ -35,20 +35,103 @@ describe('ServerLogParamsFields renders', () => { test('level param field is rendered with default value if not selected', () => { const actionParams = { message: 'test message', - level: ServerLogLevelOptions.INFO, }; + const editAction = jest.fn(); + + mountWithIntl( + + ); + + expect(editAction).toHaveBeenCalledWith('level', 'info', 0); + }); + + test('message param field is rendered with default value if not set', () => { + const actionParams = { + level: ServerLogLevelOptions.TRACE, + }; + + const editAction = jest.fn(); + + mountWithIntl( + + ); + + expect(editAction).toHaveBeenCalledWith('message', 'Some default message', 0); + }); + + test('when the default message changes, so is the underlying message if it was set by the previous default', () => { + const actionParams = { + level: ServerLogLevelOptions.TRACE, + }; + + const editAction = jest.fn(); const wrapper = mountWithIntl( {}} + editAction={editAction} index={0} /> ); - expect(wrapper.find('[data-test-subj="loggingLevelSelect"]').length > 0).toBeTruthy(); - expect( - wrapper.find('[data-test-subj="loggingLevelSelect"]').first().prop('value') - ).toStrictEqual('info'); - expect(wrapper.find('[data-test-subj="messageTextArea"]').length > 0).toBeTruthy(); + + expect(editAction).toHaveBeenCalledWith('message', 'Some default message', 0); + + wrapper.setProps({ + defaultMessage: 'Some different default message', + }); + + expect(editAction).toHaveBeenCalledWith('message', 'Some different default message', 0); + }); + + test('when the default message changes, it doesnt change the underlying message if it wasnt set by a previous default', () => { + const actionParams = { + level: ServerLogLevelOptions.TRACE, + }; + + const editAction = jest.fn(); + const wrapper = mountWithIntl( + + ); + + expect(editAction).toHaveBeenCalledWith('message', 'Some default message', 0); + + // simulate value being updated + const valueToSimulate = 'some new value'; + wrapper + .find('[data-test-subj="messageTextArea"]') + .first() + .simulate('change', { target: { value: valueToSimulate } }); + expect(editAction).toHaveBeenCalledWith('message', valueToSimulate, 0); + wrapper.setProps({ + actionParams: { + ...actionParams, + message: valueToSimulate, + }, + }); + + // simulate default changing + wrapper.setProps({ + defaultMessage: 'Some different default message', + }); + + expect(editAction).not.toHaveBeenCalledWith('message', 'Some different default message', 0); }); }); diff --git a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/server_log/server_log_params.tsx b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/server_log/server_log_params.tsx index ea734439ef41a..ce426c72b64b4 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/server_log/server_log_params.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/server_log/server_log_params.tsx @@ -10,9 +10,9 @@ import { ActionParamsProps } from '../../../../types'; import { ServerLogActionParams } from '.././types'; import { TextAreaWithMessageVariables } from '../../text_area_with_message_variables'; -export const ServerLogParamsFields: React.FunctionComponent> = ({ actionParams, editAction, index, errors, messageVariables, defaultMessage }) => { +export const ServerLogParamsFields: React.FunctionComponent< + ActionParamsProps +> = ({ actionParams, editAction, index, errors, messageVariables, defaultMessage }) => { const { message, level } = actionParams; const levelOptions = [ { value: 'trace', text: 'Trace' }, @@ -23,7 +23,7 @@ export const ServerLogParamsFields: React.FunctionComponent { - if (!actionParams?.level) { + if (!actionParams.level) { editAction('level', 'info', index); } // eslint-disable-next-line react-hooks/exhaustive-deps diff --git a/x-pack/plugins/triggers_actions_ui/public/types.ts b/x-pack/plugins/triggers_actions_ui/public/types.ts index be8b7b9757e9e..a4eac1ab1da21 100644 --- a/x-pack/plugins/triggers_actions_ui/public/types.ts +++ b/x-pack/plugins/triggers_actions_ui/public/types.ts @@ -48,7 +48,7 @@ export interface ActionConnectorFieldsProps { } export interface ActionParamsProps { - actionParams: TParams; + actionParams: Partial; index: number; editAction: (key: string, value: AlertActionParam, index: number) => void; errors: IErrorObject; From b1eb47b114a525774438335cdc8d2aa3e2511893 Mon Sep 17 00:00:00 2001 From: Gidi Meir Morris Date: Thu, 26 Nov 2020 09:51:31 +0000 Subject: [PATCH 3/4] fixed typing --- .../components/builtin_action_types/email/email_params.test.tsx | 2 +- .../components/builtin_action_types/jira/jira_params.tsx | 2 +- .../builtin_action_types/jira/use_get_fields_by_issue_type.tsx | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/email/email_params.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/email/email_params.test.tsx index 1020b784f3c58..6ead65d958bd5 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/email/email_params.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/email/email_params.test.tsx @@ -43,7 +43,7 @@ describe('EmailParamsFields renders', () => { }; const editAction = jest.fn(); - const wrapper = mountWithIntl( + mountWithIntl( - {hasParent && ( + {hasParent && parent && ( <> diff --git a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/jira/use_get_fields_by_issue_type.tsx b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/jira/use_get_fields_by_issue_type.tsx index 8685ee1e615b0..6129f42923e45 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/jira/use_get_fields_by_issue_type.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/jira/use_get_fields_by_issue_type.tsx @@ -23,7 +23,7 @@ interface Props { ToastsApi, 'get$' | 'add' | 'remove' | 'addSuccess' | 'addWarning' | 'addDanger' | 'addError' >; - issueType: string; + issueType: string | undefined; actionConnector?: ActionConnector; } From c967f84b42b16a2e2ffb707180f449d7c00a0d1c Mon Sep 17 00:00:00 2001 From: Gidi Meir Morris Date: Thu, 26 Nov 2020 12:22:17 +0000 Subject: [PATCH 4/4] support unspecified parent field on jira --- .../components/builtin_action_types/jira/jira_params.tsx | 2 +- .../components/builtin_action_types/jira/search_issues.tsx | 2 +- .../builtin_action_types/jira/use_get_single_issue.tsx | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/jira/jira_params.tsx b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/jira/jira_params.tsx index d2e97f8f52207..385872ed67bc7 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/jira/jira_params.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/jira/jira_params.tsx @@ -197,7 +197,7 @@ const JiraParamsFields: React.FunctionComponent - {hasParent && parent && ( + {hasParent && ( <> diff --git a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/jira/search_issues.tsx b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/jira/search_issues.tsx index fff606982677a..8a98a9eb7bb75 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/jira/search_issues.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/jira/search_issues.tsx @@ -14,7 +14,7 @@ import { useGetSingleIssue } from './use_get_single_issue'; import * as i18n from './translations'; interface Props { - selectedValue: string | null; + selectedValue?: string | null; http: HttpSetup; toastNotifications: Pick< ToastsApi, diff --git a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/jira/use_get_single_issue.tsx b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/jira/use_get_single_issue.tsx index c0d2eae14bead..85fc94733e8a8 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/jira/use_get_single_issue.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/jira/use_get_single_issue.tsx @@ -22,7 +22,7 @@ interface Props { ToastsApi, 'get$' | 'add' | 'remove' | 'addSuccess' | 'addWarning' | 'addDanger' | 'addError' >; - id: string | null; + id?: string | null; actionConnector?: ActionConnector; }