Skip to content

Commit

Permalink
Simplify widget edit submit logic by removing onSubmit prop from wi…
Browse files Browse the repository at this point in the history
…dget edit components. Instead we only call `applyAllWidgetChanges` from `WidgetEditApplyAllChangesContext.

The `WidgetEditApplyAllChangesProvider` now calls a `onSubmit` prop which takes care of updateing the widget.
  • Loading branch information
linuspahl committed Nov 26, 2024
1 parent fd167f1 commit e16be46
Show file tree
Hide file tree
Showing 16 changed files with 52 additions and 55 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ describe('AggregationWizard', () => {
<TestStoreProvider>
<FieldTypesContext.Provider value={fieldTypes}>
<AggregationWizard onChange={() => {}}
onSubmit={() => {}}
onCancel={() => {}}
config={widgetConfig}
editing
Expand Down Expand Up @@ -98,14 +97,6 @@ describe('AggregationWizard', () => {
await waitFor(() => expect(screen.queryByRole('menu')).not.toBeInTheDocument());
});

it('should call onSubmit', async () => {
const onSubmit = jest.fn();
renderSUT({ onSubmit });
userEvent.click(await screen.findByRole('button', { name: /update widget/i }));

await waitFor(() => expect(onSubmit).toHaveBeenCalledTimes(1));
});

it('should call onCancel', async () => {
const onCancel = jest.fn();
renderSUT({ onCancel });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ const validateForm = (formValues: WidgetConfigFormValues) => {
return elementValidationResults.reduce((prev, cur) => ({ ...prev, ...cur }), {});
};

const AggregationWizard = ({ onChange, config, children, onSubmit, onCancel }: EditWidgetComponentProps<AggregationWidgetConfig> & { children: React.ReactElement }) => {
const AggregationWizard = ({ onChange, config, children, onCancel }: EditWidgetComponentProps<AggregationWidgetConfig> & { children: React.ReactElement }) => {
const initialFormValues = _initialFormValues(config);

return (
Expand All @@ -114,7 +114,6 @@ const AggregationWizard = ({ onChange, config, children, onSubmit, onCancel }: E
<ElementsConfiguration aggregationElementsByKey={aggregationElementsByKey}
config={config}
onCreate={onCreateElement}
onSubmit={onSubmit}
onCancel={onCancel}
onConfigChange={onChange} />
</Section>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,19 +52,18 @@ type Props = {
values: WidgetConfigFormValues,
setValues: (formValues: WidgetConfigFormValues) => void,
) => void,
onSubmit: () => void,
onCancel: () => void,
}

const ElementsConfiguration = ({ aggregationElementsByKey, config, onConfigChange, onCreate, onSubmit, onCancel }: Props) => {
const ElementsConfiguration = ({ aggregationElementsByKey, config, onConfigChange, onCreate, onCancel }: Props) => {
const { values, setValues } = useFormikContext<WidgetConfigFormValues>();

return (
<Container>
<StickyBottomActions actions={(
<>
<ElementsConfigurationActions />
<SaveOrCancelButtons onCancel={onCancel} onSubmit={onSubmit} />
<SaveOrCancelButtons onCancel={onCancel} />
</>
)}>
<div>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,19 @@
/*
* Copyright (C) 2020 Graylog, Inc.
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the Server Side Public License, version 1,
* as published by MongoDB, Inc.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* Server Side Public License for more details.
*
* You should have received a copy of the Server Side Public License
* along with this program. If not, see
* <http://www.mongodb.com/licensing/server-side-public-license>.
*/
import * as React from 'react';
import { useContext } from 'react';
import Immutable from 'immutable';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ const SimpleAggregationWizard = (props: Partial<React.ComponentProps<typeof Aggr
fields={Immutable.List([])}
onCancel={() => {}}
onChange={() => {}}
onSubmit={() => {}}
{...props}>
<span>The visualization</span>
</AggregationWizard>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,6 @@ describe('AggregationWizard', () => {
editing
id="widget-id"
type="AGGREGATION"
onSubmit={() => {}}
onCancel={() => {}}
fields={Immutable.List([])}
onChange={() => {}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ describe('AggregationWizard', () => {
const renderSUT = (props = {}) => render(
<FieldTypesContext.Provider value={fieldTypes}>
<AggregationWizard onChange={() => {}}
onSubmit={() => {}}
onCancel={() => {}}
config={widgetConfig}
editing
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,6 @@ const renderSUT = (props = {}) => render((
<TestStoreProvider>
<FieldTypesContext.Provider value={fieldTypes}>
<AggregationWizard onChange={() => {}}
onSubmit={() => {}}
onCancel={() => {}}
config={widgetConfig}
editing
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ const useApplyAllWidgetChanges = (
widget: Widget,
applySearchControlsChanges: React.RefObject<(widget: Widget) => Widget>,
applyElementConfigurationChanges: React.RefObject<(widgetConfig: WidgetConfig) => WidgetConfig>,
onChange: (newWidget: Widget) => Promise<void>,
onSubmit: (newWidget: Widget, hasChanges: boolean) => Promise<void>,
) => {
const { setDisabled } = useContext(DisableSubmissionStateContext);
const setDisableWidgetEditSubmit = useCallback(
Expand Down Expand Up @@ -67,31 +67,27 @@ const useApplyAllWidgetChanges = (
}
}

if (hasChanges) {
setDisableWidgetEditSubmit(true);
setDisableWidgetEditSubmit(true);

return onChange(newWidget)
.catch((error) => {
UserNotification.error(`Applying widget changes failed with status: ${error}`);
return onSubmit(newWidget, hasChanges)
.catch((error) => {
UserNotification.error(`Applying widget changes failed with status: ${error}`);

return error;
}).finally(() => setDisableWidgetEditSubmit(false));
}

return Promise.resolve();
}, [widget, applySearchControlsChanges, applyElementConfigurationChanges, setDisableWidgetEditSubmit, onChange]);
return error;
}).finally(() => setDisableWidgetEditSubmit(false));
}, [widget, applySearchControlsChanges, applyElementConfigurationChanges, setDisableWidgetEditSubmit, onSubmit]);
};

type Props = {
widget: Widget,
children: React.ReactNode,
onChange: (newWidget: Widget) => Promise<void>,
onSubmit: (newWidget: Widget, hasChanges: boolean) => Promise<void>,
widget: Widget,
}

const WidgetEditApplyAllChangesProvider = ({ children, widget, onChange }: Props) => {
const WidgetEditApplyAllChangesProvider = ({ children, widget, onSubmit }: Props) => {
const { applyChangesRef: applySearchControlsChanges, bindApplyChanges: bindApplySearchControlsChanges } = useBindApplyChanges();
const { applyChangesRef: applyElementConfigurationChanges, bindApplyChanges: bindApplyElementConfigurationChanges } = useBindApplyChanges();
const applyAllWidgetChanges = useApplyAllWidgetChanges(widget, applySearchControlsChanges, applyElementConfigurationChanges, onChange);
const applyAllWidgetChanges = useApplyAllWidgetChanges(widget, applySearchControlsChanges, applyElementConfigurationChanges, onSubmit);

const contextValue = useMemo(() => ({
applyAllWidgetChanges,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ const _onSortDirectionChange = (direction: SortConfig['direction'], config, onCh
return onChange(newConfig);
};

const EditMessageList = ({ children, config, fields, onChange, onCancel, onSubmit }: EditWidgetComponentProps<MessagesWidgetConfig>) => {
const EditMessageList = ({ children, config, fields, onChange, onCancel }: EditWidgetComponentProps<MessagesWidgetConfig>) => {
const { sort } = config;
const [sortDirection] = (sort || []).map((s) => s.direction);
const onDecoratorsChange = (newDecorators) => onChange(config.toBuilder().decorators(newDecorators).build());
Expand All @@ -79,7 +79,7 @@ const EditMessageList = ({ children, config, fields, onChange, onCancel, onSubmi
return (
<FullHeightRow>
<FullHeightCol md={3}>
<StickyBottomActions actions={<SaveOrCancelButtons onCancel={onCancel} onSubmit={onSubmit} />}
<StickyBottomActions actions={<SaveOrCancelButtons onCancel={onCancel} />}
alignActionsAtBottom>
<DescriptionBox description="Fields">
<FieldsConfiguration onChange={(newFields) => _onFieldSelectionChanged(newFields, config, onChange)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ describe('EditWidgetFrame', () => {
const renderSUT = (props?: Partial<React.ComponentProps<typeof EditWidgetFrame>>) => render((
<TestStoreProvider>
<WidgetContext.Provider value={widget}>
<EditWidgetFrame onSubmit={() => {}} onCancel={() => {}} onChange={() => Promise.resolve()} {...props}>
<EditWidgetFrame onCancel={() => {}} onSubmit={() => Promise.resolve()} {...props}>
Hello World!
These are some buttons!
</EditWidgetFrame>
Expand Down Expand Up @@ -114,7 +114,7 @@ describe('EditWidgetFrame', () => {
});

it('calls onSubmit', async () => {
const onSubmit = jest.fn();
const onSubmit = jest.fn(() => Promise.resolve());
renderSUT({ onSubmit });

fireEvent.click(await screen.findByRole('button', { name: /update widget/i }));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,21 +51,20 @@ type Props = {
children: React.ReactNode,
displaySubmitActions?: boolean,
onCancel: () => void,
onSubmit: () => void,
showQueryControls?: boolean,
onChange: (newWidget: Widget) => Promise<void>,
onSubmit: (newWidget: Widget, hasChanges: boolean) => Promise<void>,
containerComponent?: React.ComponentType<React.PropsWithChildren>
};

const EditWidgetFrame = ({ children, onCancel, onSubmit, displaySubmitActions = true, showQueryControls = true, onChange, containerComponent: ContainerComponent = WidgetOverrideElements }: Props) => {
const EditWidgetFrame = ({ children, onCancel, onSubmit, displaySubmitActions = true, showQueryControls = true, containerComponent: ContainerComponent = WidgetOverrideElements }: Props) => {
const widget = useContext(WidgetContext);

if (!widget) {
return <Spinner text="Loading widget ..." />;
}

return (
<WidgetEditApplyAllChangesProvider widget={widget} onChange={onChange}>
<WidgetEditApplyAllChangesProvider widget={widget} onSubmit={onSubmit}>
<DisableSubmissionStateProvider>
<Container>
{(showQueryControls && !widget.returnsAllRecords) && (
Expand All @@ -82,7 +81,7 @@ const EditWidgetFrame = ({ children, onCancel, onSubmit, displaySubmitActions =
</Visualization>
{displaySubmitActions && (
<div>
<SaveOrCancelButtons onSubmit={onSubmit} onCancel={onCancel} />
<SaveOrCancelButtons onCancel={onCancel} />
</div>
)}
</Container>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,9 @@ export const UPDATE_WIDGET_BTN_TEXT = 'Update widget';

type Props = {
onCancel: () => void,
onSubmit: () => void,
};

const SaveOrCancelButtons = ({ onSubmit, onCancel }: Props) => {
const SaveOrCancelButtons = ({ onCancel }: Props) => {
const { applyAllWidgetChanges } = useContext(WidgetEditApplyAllChangesContext);
const [isSubmitting, setIsSubmitting] = useState(false);
const { disabled: disabledSubmit } = useContext(DisableSubmissionStateContext);
Expand All @@ -38,7 +37,6 @@ const SaveOrCancelButtons = ({ onSubmit, onCancel }: Props) => {

return applyAllWidgetChanges().then(() => {
setIsSubmitting(false);
onSubmit();
}).catch(() => {
setIsSubmitting(false);
});
Expand Down
16 changes: 10 additions & 6 deletions graylog2-web-interface/src/views/components/widgets/Widget.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -175,22 +175,26 @@ export const EditWrapper = ({
const EditComponent = useMemo(() => _editComponentForType(type), [type]);
const hasOwnSubmitButton = _hasOwnEditSubmitButton(type);
const dispatch = useAppDispatch();
const onChangeWidget = useCallback((newWidget: WidgetType) => (
dispatch(updateWidget(newWidget.id, newWidget)).then(() => {})
), [dispatch]);
const onSubmitEdit = useCallback((newWidget: WidgetType, hasChanges: boolean) => {
if (hasChanges) {
return dispatch(updateWidget(newWidget.id, newWidget)).then(() => onToggleEdit());
}

onToggleEdit();

return Promise.resolve();
}, [dispatch, onToggleEdit]);

return editing ? (
<EditWidgetFrame onSubmit={onToggleEdit}
<EditWidgetFrame onSubmit={onSubmitEdit}
onCancel={onCancelEdit}
onChange={onChangeWidget}
displaySubmitActions={!hasOwnSubmitButton}
showQueryControls={showQueryControls}>
<EditComponent config={config}
fields={fields}
editing={editing}
id={id}
type={type}
onSubmit={onToggleEdit}
onCancel={onCancelEdit}
onChange={onWidgetConfigChange}>
{children}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ const SubmitOnChange = () => {
return <></>;
};

const EventsWidgetEdit = ({ children, onCancel, config, onChange, onSubmit }: EditWidgetComponentProps<EventsWidgetConfig>) => {
const EventsWidgetEdit = ({ children, onCancel, config, onChange }: EditWidgetComponentProps<EventsWidgetConfig>) => {
const filterComponents = usePluginEntities('views.components.widgets.events.filterComponents');
const eventAttributes = useEventAttributes();

Expand Down Expand Up @@ -172,7 +172,7 @@ const EventsWidgetEdit = ({ children, onCancel, config, onChange, onSubmit }: Ed
<FullHeightRow>
<FullHeightCol md={4} lg={3}>
<Container>
<StickyBottomActions actions={<SaveOrCancelButtons onCancel={onCancel} onSubmit={onSubmit} />}
<StickyBottomActions actions={<SaveOrCancelButtons onCancel={onCancel} />}
alignActionsAtBottom>
<DescriptionBox description="Visualization">
<WidgetModeConfiguration name="mode" onChange={onChangeType} options={WIDGET_MODE_OPTIONS} />
Expand Down
1 change: 0 additions & 1 deletion graylog2-web-interface/src/views/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ export interface EditWidgetComponentProps<Config extends WidgetConfig = WidgetCo
type: string;
fields: Immutable.List<FieldTypeMapping>,
onChange: (newConfig: Config) => void,
onSubmit: () => void,
onCancel: () => void,
}

Expand Down

0 comments on commit e16be46

Please sign in to comment.