Skip to content

Commit

Permalink
[ML] Transforms: Update state management for edit form to use constat…
Browse files Browse the repository at this point in the history 
…e lib. (#149670)

The transform wizard and the flyout to edit a transform use different
state management. The wizard has the very first forms we did using React
Hooks, it uses basic hooks (`useState`, `useEffect`) and passes around
values and callbacks via props. The form to edit a transform is less
complex (less fields, no wizard steps) and was used as a bit of a
playground to experiment with an alternative state management approach.
It uses `useReducer` to manage state and actions of the form in a
centralized place. This was done before the introduction of `useContext`
so passing around values and actions still includes using
props/callbacks. `useContext` can simplify state management, but the way
it's implemented has drawbacks for dynamic content because it triggers
rerenders everywhere it's consumed, not just where certain attributes
are requested. The `constate` library
(https://github.com/diegohaz/constate) can improve this by creating
multiple contexts and corresponding custom hooks. This PR makes use of
the `constate` library and refactors components to trigger less
rerenders. As documented further down in this description, some
benchmarking was done to demonstrate the improvements. There are similar
libraries available, but `constate` is already a package that's part of
Kibana and used in other team's code so I chose to go with it.
  • Loading branch information
@walterra
walterra authored Mar 7, 2023
1 parent 9a67404 commit 813a5a5
Show file tree
Hide file tree
Showing 15 changed files with 728 additions and 569 deletions.
2 changes: 1 addition & 1 deletion ...lugins/transform/public/app/sections/transform_management/components/action_edit/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@
* 2.0.
*/

export { useEditAction } from './use_edit_action';
export { useEditAction, type EditAction } from './use_edit_action';
1 change: 1 addition & 0 deletions ...sform/public/app/sections/transform_management/components/action_edit/use_edit_action.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { useSearchItems } from '../../../../hooks/use_search_items';
import { useAppDependencies, useToastNotifications } from '../../../../app_dependencies';
import { TransformConfigUnion } from '../../../../../../common/types/transform';

export type EditAction = ReturnType<typeof useEditAction>;
export const useEditAction = (forceDisable: boolean, transformNodes: number) => {
const { canCreateTransform } = useContext(AuthorizationContext).capabilities;

Expand Down
40 changes: 40 additions & 0 deletions ...ransform_management/components/edit_transform_flyout/edit_transform_api_error_callout.tsx
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import React, { type FC } from 'react';

import { EuiCallOut, EuiSpacer } from '@elastic/eui';

import { i18n } from '@kbn/i18n';

import { useEditTransformFlyout } from './use_edit_transform_flyout';

export const EditTransformApiErrorCallout: FC = () => {
const apiErrorMessage = useEditTransformFlyout('apiErrorMessage');

return (
<>
{apiErrorMessage !== undefined && (
<>
<EuiSpacer size="m" />
<EuiCallOut
title={i18n.translate(
'xpack.transform.transformList.editTransformGenericErrorMessage',
{
defaultMessage: 'An error occurred calling the API endpoint to update transforms.',
}
)}
color="danger"
iconType="alert"
>
<p>{apiErrorMessage}</p>
</EuiCallOut>
</>
)}
</>
);
};
189 changes: 61 additions & 128 deletions .../sections/transform_management/components/edit_transform_flyout/edit_transform_flyout.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,154 +5,87 @@
* 2.0.
*/

import React, { useState, FC } from 'react';
import React, { type FC } from 'react';

import { i18n } from '@kbn/i18n';

import {
EuiButton,
EuiButtonEmpty,
EuiCallOut,
EuiFlexGroup,
EuiFlexItem,
EuiFlyout,
EuiFlyoutBody,
EuiFlyoutFooter,
EuiFlyoutHeader,
EuiSpacer,
EuiTitle,
} from '@elastic/eui';

import { isPostTransformsUpdateResponseSchema } from '../../../../../../common/api_schemas/type_guards';
import { TransformConfigUnion } from '../../../../../../common/types/transform';

import { getErrorMessage } from '../../../../../../common/utils/errors';
import { isManagedTransform } from '../../../../common/managed_transforms_utils';

import { refreshTransformList$, REFRESH_TRANSFORM_LIST_STATE } from '../../../../common';
import { useToastNotifications } from '../../../../app_dependencies';
import { useApi } from '../../../../hooks/use_api';
import { ManagedTransformsWarningCallout } from '../managed_transforms_callout/managed_transforms_callout';
import type { EditAction } from '../action_edit';

import { EditTransformApiErrorCallout } from './edit_transform_api_error_callout';
import { EditTransformFlyoutCallout } from './edit_transform_flyout_callout';
import { EditTransformFlyoutForm } from './edit_transform_flyout_form';
import {
applyFormStateToTransformConfig,
useEditTransformFlyout,
} from './use_edit_transform_flyout';
import { ManagedTransformsWarningCallout } from '../managed_transforms_callout/managed_transforms_callout';
import { isManagedTransform } from '../../../../common/managed_transforms_utils';
import { EditTransformFlyoutProvider } from './use_edit_transform_flyout';
import { EditTransformUpdateButton } from './edit_transform_update_button';

interface EditTransformFlyoutProps {
closeFlyout: () => void;
config: TransformConfigUnion;
dataViewId?: string;
}

export const EditTransformFlyout: FC<EditTransformFlyoutProps> = ({
export const EditTransformFlyout: FC<EditAction> = ({
closeFlyout,
config,
dataViewId,
}) => {
const api = useApi();
const toastNotifications = useToastNotifications();

const [state, dispatch] = useEditTransformFlyout(config);
const [errorMessage, setErrorMessage] = useState<string | undefined>(undefined);

async function submitFormHandler() {
setErrorMessage(undefined);
const requestConfig = applyFormStateToTransformConfig(config, state);
const transformId = config.id;

const resp = await api.updateTransform(transformId, requestConfig);

if (!isPostTransformsUpdateResponseSchema(resp)) {
setErrorMessage(getErrorMessage(resp));
return;
}

toastNotifications.addSuccess(
i18n.translate('xpack.transform.transformList.editTransformSuccessMessage', {
defaultMessage: 'Transform {transformId} updated.',
values: { transformId },
})
);
closeFlyout();
refreshTransformList$.next(REFRESH_TRANSFORM_LIST_STATE.REFRESH);
}

const isUpdateButtonDisabled = !state.isFormValid || !state.isFormTouched;

return (
<EuiFlyout
onClose={closeFlyout}
hideCloseButton
aria-labelledby="transformEditFlyoutTitle"
data-test-subj="transformEditFlyout"
>
<EuiFlyoutHeader hasBorder>
<EuiTitle size="m">
<h2 id="transformEditFlyoutTitle">
{i18n.translate('xpack.transform.transformList.editFlyoutTitle', {
defaultMessage: 'Edit {transformId}',
values: {
transformId: config.id,
},
})}
</h2>
</EuiTitle>
</EuiFlyoutHeader>
{isManagedTransform({ config }) ? (
<ManagedTransformsWarningCallout
count={1}
action={i18n.translate('xpack.transform.transformList.editManagedTransformsDescription', {
defaultMessage: 'editing',
})}
/>
) : null}
<EuiFlyoutBody banner={<EditTransformFlyoutCallout />}>
<EditTransformFlyoutForm editTransformFlyout={[state, dispatch]} dataViewId={dataViewId} />
{errorMessage !== undefined && (
<>
<EuiSpacer size="m" />
<EuiCallOut
title={i18n.translate(
'xpack.transform.transformList.editTransformGenericErrorMessage',
{
defaultMessage:
'An error occurred calling the API endpoint to update transforms.',
}
)}
color="danger"
iconType="alert"
>
<p>{errorMessage}</p>
</EuiCallOut>
</>
)}
</EuiFlyoutBody>
<EuiFlyoutFooter>
<EuiFlexGroup justifyContent="spaceBetween">
<EuiFlexItem grow={false}>
<EuiButtonEmpty iconType="cross" onClick={closeFlyout} flush="left">
{i18n.translate('xpack.transform.transformList.editFlyoutCancelButtonText', {
defaultMessage: 'Cancel',
})}
</EuiButtonEmpty>
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiButton
data-test-subj="transformEditFlyoutUpdateButton"
onClick={submitFormHandler}
fill
isDisabled={isUpdateButtonDisabled}
>
{i18n.translate('xpack.transform.transformList.editFlyoutUpdateButtonText', {
defaultMessage: 'Update',
isFlyoutVisible,
}) =>
config && isFlyoutVisible ? (
<EditTransformFlyoutProvider config={config} dataViewId={dataViewId}>
<EuiFlyout
onClose={closeFlyout}
hideCloseButton
aria-labelledby="transformEditFlyoutTitle"
data-test-subj="transformEditFlyout"
>
<EuiFlyoutHeader hasBorder>
<EuiTitle size="m">
<h2 id="transformEditFlyoutTitle">
{i18n.translate('xpack.transform.transformList.editFlyoutTitle', {
defaultMessage: 'Edit {transformId}',
values: {
transformId: config.id,
},
})}
</EuiButton>
</EuiFlexItem>
</EuiFlexGroup>
</EuiFlyoutFooter>
</EuiFlyout>
);
};
</h2>
</EuiTitle>
</EuiFlyoutHeader>
{isManagedTransform({ config }) ? (
<ManagedTransformsWarningCallout
count={1}
action={i18n.translate(
'xpack.transform.transformList.editManagedTransformsDescription',
{
defaultMessage: 'editing',
}
)}
/>
) : null}
<EuiFlyoutBody banner={<EditTransformFlyoutCallout />}>
<EditTransformFlyoutForm />
<EditTransformApiErrorCallout />
</EuiFlyoutBody>
<EuiFlyoutFooter>
<EuiFlexGroup justifyContent="spaceBetween">
<EuiFlexItem grow={false}>
<EuiButtonEmpty iconType="cross" onClick={closeFlyout} flush="left">
{i18n.translate('xpack.transform.transformList.editFlyoutCancelButtonText', {
defaultMessage: 'Cancel',
})}
</EuiButtonEmpty>
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EditTransformUpdateButton closeFlyout={closeFlyout} />
</EuiFlexItem>
</EuiFlexGroup>
</EuiFlyoutFooter>
</EuiFlyout>
</EditTransformFlyoutProvider>
) : null;
Loading

0 comments on commit 813a5a5

Please sign in to comment.