mirror of
https://github.com/Graylog2/graylog2-server.git
synced 2026-03-13 09:32:21 +08:00
Warn user before saving search in case of unconfirmed changes. (6.3) (#25212)
* Warn user before saving search in case of unconfirmed changes. * Adding tests. * Adding changelog snippet. * Extract mockFormDirtyState helper in SavedSearchForm tests Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Fix SearchActionsMenu tests by mocking useFormikContext SavedSearchForm now uses useFormikContext to detect dirty form state, which requires the Formik context to be available. Add the same mock pattern used in SavedSearchForm tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Improve wording. * Fixing unwanted merge artifact. --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
5
changelog/unreleased/issue-23967.toml
Normal file
5
changelog/unreleased/issue-23967.toml
Normal file
@@ -0,0 +1,5 @@
|
||||
type = "a"
|
||||
message = "Warning user when saving search if there are unconfirmed changes."
|
||||
|
||||
issues = ["23967"]
|
||||
pulls = ["25178"]
|
||||
@@ -26,16 +26,23 @@ type Props = {
|
||||
bsStyle?: ColorVariant;
|
||||
children: React.ReactNode;
|
||||
className?: string;
|
||||
compact?: boolean;
|
||||
onDismiss?: () => void;
|
||||
style?: CSSProperties;
|
||||
title?: React.ReactNode;
|
||||
noIcon?: boolean;
|
||||
};
|
||||
|
||||
const StyledAlert = styled(MantineAlert)<{ $bsStyle: ColorVariant }>(
|
||||
({ $bsStyle, theme }) => css`
|
||||
const StyledAlert = styled(MantineAlert)<{ $bsStyle: ColorVariant; $compact: boolean }>(
|
||||
({ $bsStyle, $compact, theme }) => css`
|
||||
margin: ${theme.spacings.md} 0;
|
||||
border: 1px solid ${theme.colors.variant.lighter[$bsStyle]};
|
||||
${
|
||||
$compact &&
|
||||
css`
|
||||
padding: 7px 10px;
|
||||
`
|
||||
}
|
||||
|
||||
.mantine-Alert-message {
|
||||
color: ${theme.colors.global.textDefault};
|
||||
@@ -67,6 +74,7 @@ const iconNameForType = (bsStyle: ColorVariant) => {
|
||||
|
||||
const Alert = ({
|
||||
children,
|
||||
compact = false,
|
||||
bsStyle = 'default',
|
||||
title = undefined,
|
||||
style = undefined,
|
||||
@@ -80,6 +88,7 @@ const Alert = ({
|
||||
return (
|
||||
<StyledAlert
|
||||
$bsStyle={bsStyle}
|
||||
$compact={compact}
|
||||
className={className}
|
||||
color={bsStyle}
|
||||
style={style}
|
||||
|
||||
@@ -21,7 +21,6 @@ import styled, { css } from 'styled-components';
|
||||
import { Icon } from 'components/common';
|
||||
import useIsDirty from 'views/hooks/useIsDirty';
|
||||
import { Button } from 'components/bootstrap';
|
||||
|
||||
const StyledIcon = styled(Icon)<{ $isDirty: boolean }>(
|
||||
({ theme, $isDirty }) => css`
|
||||
color: ${$isDirty ? theme.colors.variant.dark.warning : 'default'};
|
||||
|
||||
@@ -17,6 +17,7 @@
|
||||
import React from 'react';
|
||||
import { act, render, screen } from 'wrappedTestingLibrary';
|
||||
import userEvent from '@testing-library/user-event';
|
||||
import { useFormikContext } from 'formik';
|
||||
|
||||
import selectEvent from 'helpers/selectEvent';
|
||||
import { asMock } from 'helpers/mocking';
|
||||
@@ -26,6 +27,10 @@ import { EntityShareStore } from 'stores/permissions/EntityShareStore';
|
||||
|
||||
import OriginalSavedSearchForm from './SavedSearchForm';
|
||||
|
||||
jest.mock('formik', () => ({
|
||||
...jest.requireActual('formik'),
|
||||
useFormikContext: jest.fn(),
|
||||
}));
|
||||
jest.mock('views/hooks/useSaveViewFormControls');
|
||||
jest.mock('stores/permissions/EntityShareStore', () => ({
|
||||
__esModule: true,
|
||||
@@ -75,6 +80,11 @@ const SavedSearchForm = ({ ...props }: React.ComponentProps<typeof OriginalSaved
|
||||
);
|
||||
jest.setTimeout(10000);
|
||||
|
||||
const mockFormDirtyState = (dirty: boolean) =>
|
||||
asMock(useFormikContext)
|
||||
// @ts-expect-error context return type is not complete
|
||||
.mockReturnValue({ dirty });
|
||||
|
||||
describe('SavedSearchForm', () => {
|
||||
beforeEach(() => {
|
||||
asMock(EntityShareStore.getInitialState).mockReturnValue({ state: createEntityShareState });
|
||||
@@ -102,6 +112,7 @@ describe('SavedSearchForm', () => {
|
||||
|
||||
beforeEach(() => {
|
||||
asMock(useSaveViewFormControls).mockReturnValue([]);
|
||||
mockFormDirtyState(false);
|
||||
});
|
||||
|
||||
describe('render the SavedSearchForm', () => {
|
||||
@@ -198,6 +209,26 @@ describe('SavedSearchForm', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('unconfirmed changes warning', () => {
|
||||
it('should show warning when form has unconfirmed changes', async () => {
|
||||
mockFormDirtyState(true);
|
||||
|
||||
render(<SavedSearchForm {...props} />);
|
||||
|
||||
await screen.findByText(/unconfirmed changes to the search parameters/i);
|
||||
});
|
||||
|
||||
it('should not show warning when form has no unconfirmed changes', async () => {
|
||||
mockFormDirtyState(false);
|
||||
|
||||
render(<SavedSearchForm {...props} />);
|
||||
|
||||
await findByHeadline();
|
||||
|
||||
expect(screen.queryByText(/unconfirmed changes to the search parameters/i)).not.toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
||||
it('should render pluggable components', async () => {
|
||||
asMock(useSaveViewFormControls).mockReturnValue([
|
||||
{ component: () => <div>Pluggable component!</div>, id: 'example-plugin-component' },
|
||||
|
||||
@@ -17,6 +17,7 @@
|
||||
import * as React from 'react';
|
||||
import { useCallback, useState } from 'react';
|
||||
import styled from 'styled-components';
|
||||
import { useFormikContext } from 'formik';
|
||||
|
||||
import { ButtonToolbar, Button, ControlLabel, FormControl, FormGroup } from 'components/bootstrap';
|
||||
import Popover from 'components/common/Popover';
|
||||
@@ -24,6 +25,7 @@ import EntityCreateShareFormGroup from 'components/permissions/EntityCreateShare
|
||||
import useSaveViewFormControls from 'views/hooks/useSaveViewFormControls';
|
||||
import type { EntitySharePayload } from 'actions/permissions/EntityShareActions';
|
||||
import type { GRN } from 'logic/permissions/types';
|
||||
import Alert from 'components/bootstrap/Alert';
|
||||
|
||||
import styles from './SavedSearchForm.css';
|
||||
|
||||
@@ -38,7 +40,7 @@ type Props = React.PropsWithChildren<{
|
||||
selectedStreamGRN?: Array<GRN>;
|
||||
}>;
|
||||
|
||||
const stopEvent = (e) => {
|
||||
const stopEvent = (e: React.FormEvent<HTMLFormElement>) => {
|
||||
e.preventDefault();
|
||||
e.stopPropagation();
|
||||
};
|
||||
@@ -55,6 +57,8 @@ const SavedSearchForm = ({
|
||||
viewId = null,
|
||||
selectedStreamGRN = null,
|
||||
}: Props) => {
|
||||
const { dirty: formDirty } = useFormikContext();
|
||||
|
||||
const [title, setTitle] = useState(value);
|
||||
const [sharePayload, setSharePayload] = useState(null);
|
||||
const onChangeTitle = useCallback(
|
||||
@@ -75,6 +79,12 @@ const SavedSearchForm = ({
|
||||
<Popover.Target>{children}</Popover.Target>
|
||||
<StyledPopoverDropdown title="Name of search" id="saved-search-popover">
|
||||
<form onSubmit={stopEvent}>
|
||||
{formDirty && (
|
||||
<Alert compact noIcon bsStyle="warning">
|
||||
There are unconfirmed changes to the search parameters (time range, streams, or query). Saving now will
|
||||
discard them. If this is not intentional, execute the search to apply your changes before saving.{' '}
|
||||
</Alert>
|
||||
)}
|
||||
<FormGroup>
|
||||
<ControlLabel htmlFor="title">Title</ControlLabel>
|
||||
<FormControl type="text" value={title} id="title" placeholder="Enter title" onChange={onChangeTitle} />
|
||||
|
||||
@@ -19,6 +19,7 @@ import * as Immutable from 'immutable';
|
||||
import { render, screen, waitFor, waitForElementToBeRemoved } from 'wrappedTestingLibrary';
|
||||
import userEvent from '@testing-library/user-event';
|
||||
import { defaultUser } from 'defaultMockValues';
|
||||
import { useFormikContext } from 'formik';
|
||||
|
||||
import { asMock } from 'helpers/mocking';
|
||||
import { adminUser } from 'fixtures/users';
|
||||
@@ -47,6 +48,10 @@ import { EntityShareStore } from 'stores/permissions/EntityShareStore';
|
||||
|
||||
import SearchActionsMenu from './SearchActionsMenu';
|
||||
|
||||
jest.mock('formik', () => ({
|
||||
...jest.requireActual('formik'),
|
||||
useFormikContext: jest.fn(),
|
||||
}));
|
||||
jest.mock('views/hooks/useSaveViewFormControls');
|
||||
jest.mock('routing/useHistory');
|
||||
jest.mock('hooks/useCurrentUser');
|
||||
@@ -130,6 +135,8 @@ describe('SearchActionsMenu', () => {
|
||||
asMock(useIsDirty).mockReturnValue(false);
|
||||
asMock(useIsNew).mockReturnValue(false);
|
||||
asMock(EntityShareStore.getInitialState).mockReturnValue({ state: createEntityShareState });
|
||||
// @ts-expect-error context return type is not complete
|
||||
asMock(useFormikContext).mockReturnValue({ dirty: false });
|
||||
});
|
||||
|
||||
useViewsPlugin();
|
||||
|
||||
Reference in New Issue
Block a user