Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Confirmation modal for topic & schema delete actions #384

Merged
merged 4 commits into from
Apr 21, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 20 additions & 7 deletions kafka-ui-react-app/src/components/Schemas/Details/Details.tsx
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
import React from 'react';
import { useHistory } from 'react-router';
import { SchemaSubject } from 'generated-sources';
import { ClusterName, SchemaName } from 'redux/interfaces';
import { clusterSchemasPath } from 'lib/paths';
import ClusterContext from 'components/contexts/ClusterContext';
import { useHistory } from 'react-router';
import Breadcrumb from '../../common/Breadcrumb/Breadcrumb';
import ConfirmationModal from 'components/common/ConfirmationModal/ConfirmationModal';
import PageLoader from 'components/common/PageLoader/PageLoader';
import Breadcrumb from 'components/common/Breadcrumb/Breadcrumb';
import SchemaVersion from './SchemaVersion';
import LatestVersionItem from './LatestVersionItem';
import PageLoader from '../../common/PageLoader/PageLoader';

export interface DetailsProps {
subject: SchemaName;
Expand All @@ -32,15 +33,20 @@ const Details: React.FC<DetailsProps> = ({
isFetched,
}) => {
const { isReadOnly } = React.useContext(ClusterContext);
const [
isDeleteSchemaConfirmationVisible,
setDeleteSchemaConfirmationVisible,
] = React.useState(false);

React.useEffect(() => {
fetchSchemaVersions(clusterName, subject);
}, [fetchSchemaVersions, clusterName]);

const history = useHistory();
const onDelete = async () => {
await deleteSchema(clusterName, subject);
const onDelete = React.useCallback(() => {
deleteSchema(clusterName, subject);
history.push(clusterSchemasPath(clusterName));
};
}, [deleteSchema, clusterName, subject]);

return (
<div className="section">
Expand Down Expand Up @@ -84,10 +90,17 @@ const Details: React.FC<DetailsProps> = ({
className="button is-danger is-small level-item"
type="button"
title="in development"
onClick={onDelete}
onClick={() => setDeleteSchemaConfirmationVisible(true)}
>
Remove
</button>
<ConfirmationModal
isOpen={isDeleteSchemaConfirmationVisible}
onCancel={() => setDeleteSchemaConfirmationVisible(false)}
onConfirm={onDelete}
>
Are you sure want to remove <b>{subject}</b> schema?
</ConfirmationModal>
</div>
)}
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@ import { schema, versions } from './fixtures';
const clusterName = 'testCluster';
const fetchSchemaVersionsMock = jest.fn();

jest.mock(
'components/common/ConfirmationModal/ConfirmationModal',
() => 'mock-ConfirmationModal'
);

describe('Details', () => {
describe('Container', () => {
const store = configureStore();
Expand Down Expand Up @@ -92,23 +97,34 @@ describe('Details', () => {
});

describe('when schema has versions', () => {
const wrapper = shallow(setupWrapper({ versions }));

it('renders table heading with SchemaVersion', () => {
const wrapper = shallow(setupWrapper({ versions }));
expect(wrapper.exists('LatestVersionItem')).toBeTruthy();
expect(wrapper.exists('button')).toBeTruthy();
expect(wrapper.exists('thead')).toBeTruthy();
expect(wrapper.find('SchemaVersion').length).toEqual(2);
});

it('calls deleteSchema on button click', () => {
it('calls deleteSchema after confirmation', () => {
const mockDelete = jest.fn();
const component = mount(
const wrapper = mount(
<StaticRouter>
{setupWrapper({ versions, deleteSchema: mockDelete })}
</StaticRouter>
);
component.find('button').at(1).simulate('click');
expect(
wrapper.find('mock-ConfirmationModal').prop('isOpen')
).toBeFalsy();

wrapper.find('button').at(1).simulate('click');
expect(
wrapper.find('mock-ConfirmationModal').prop('isOpen')
).toBeTruthy();

wrapper
.find('mock-ConfirmationModal')
.prop<() => void>('onConfirm')();

expect(mockDelete).toHaveBeenCalledTimes(1);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,17 @@ exports[`Details View Initial state matches snapshot 1`] = `
>
Remove
</button>
<mock-ConfirmationModal
isOpen={false}
onCancel={[Function]}
onConfirm={[Function]}
>
Are you sure want to remove
<b>
test
</b>
schema?
</mock-ConfirmationModal>
</div>
</div>
<LatestVersionItem
Expand Down Expand Up @@ -202,6 +213,17 @@ exports[`Details View when page with schema versions loaded when schema has vers
>
Remove
</button>
<mock-ConfirmationModal
isOpen={false}
onCancel={[Function]}
onConfirm={[Function]}
>
Are you sure want to remove
<b>
test
</b>
schema?
</mock-ConfirmationModal>
</div>
</div>
<LatestVersionItem
Expand Down Expand Up @@ -340,6 +362,17 @@ exports[`Details View when page with schema versions loaded when versions are em
>
Remove
</button>
<mock-ConfirmationModal
isOpen={false}
onCancel={[Function]}
onConfirm={[Function]}
>
Are you sure want to remove
<b>
test
</b>
schema?
</mock-ConfirmationModal>
</div>
</div>
<LatestVersionItem
Expand Down
41 changes: 29 additions & 12 deletions kafka-ui-react-app/src/components/Topics/List/ListItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
} from 'redux/interfaces';
import DropdownItem from 'components/common/Dropdown/DropdownItem';
import Dropdown from 'components/common/Dropdown/Dropdown';
import ConfirmationModal from 'components/common/ConfirmationModal/ConfirmationModal';

export interface ListItemProps {
topic: TopicWithDetailedInfo;
Expand All @@ -20,6 +21,11 @@ const ListItem: React.FC<ListItemProps> = ({
deleteTopic,
clusterName,
}) => {
const [
isDeleteTopicConfirmationVisible,
setDeleteTopicConfirmationVisible,
] = React.useState(false);

const outOfSyncReplicas = React.useMemo(() => {
if (partitions === undefined || partitions.length === 0) {
return 0;
Expand Down Expand Up @@ -54,19 +60,30 @@ const ListItem: React.FC<ListItemProps> = ({
{internal ? 'Internal' : 'External'}
</div>
</td>
<td className="has-text-right">
<Dropdown
label={
<span className="icon">
<i className="fas fa-cog" />
</span>
}
right
<td>
<div className="has-text-right">
<Dropdown
label={
<span className="icon">
<i className="fas fa-cog" />
</span>
}
right
>
<DropdownItem
onClick={() => setDeleteTopicConfirmationVisible(true)}
>
<span className="has-text-danger">Remove Topic</span>
</DropdownItem>
</Dropdown>
</div>
<ConfirmationModal
isOpen={isDeleteTopicConfirmationVisible}
onCancel={() => setDeleteTopicConfirmationVisible(false)}
onConfirm={deleteTopicHandler}
>
<DropdownItem onClick={deleteTopicHandler}>
<span className="has-text-danger">Remove Topic</span>
</DropdownItem>
</Dropdown>
Are you sure want to remove <b>{name}</b> topic?
</ConfirmationModal>
</td>
</tr>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@ import ListItem, { ListItemProps } from '../ListItem';
const mockDelete = jest.fn();
const clusterName = 'local';

jest.mock(
'components/common/ConfirmationModal/ConfirmationModal',
() => 'mock-ConfirmationModal'
);

describe('ListItem', () => {
const setupComponent = (props: Partial<ListItemProps> = {}) => (
<ListItem
Expand All @@ -22,11 +27,25 @@ describe('ListItem', () => {

it('triggers the deleteTopic when clicked on the delete button', () => {
const wrapper = shallow(setupComponent());
wrapper.find('DropdownItem').simulate('click');
expect(wrapper.find('mock-ConfirmationModal').prop('isOpen')).toBeFalsy();
wrapper.find('DropdownItem').last().simulate('click');
const modal = wrapper.find('mock-ConfirmationModal');
expect(modal.prop('isOpen')).toBeTruthy();
modal.simulate('confirm');
expect(mockDelete).toBeCalledTimes(1);
expect(mockDelete).toBeCalledWith(clusterName, internalTopicPayload.name);
});

it('closes ConfirmationModal when clicked on the cancel button', () => {
const wrapper = shallow(setupComponent());
expect(wrapper.find('mock-ConfirmationModal').prop('isOpen')).toBeFalsy();
wrapper.find('DropdownItem').last().simulate('click');
expect(wrapper.find('mock-ConfirmationModal').prop('isOpen')).toBeTruthy();
wrapper.find('mock-ConfirmationModal').simulate('cancel');
expect(mockDelete).toBeCalledTimes(0);
expect(wrapper.find('mock-ConfirmationModal').prop('isOpen')).toBeFalsy();
});

it('renders correct tags for internal topic', () => {
const wrapper = mount(
<StaticRouter>
Expand All @@ -50,4 +69,20 @@ describe('ListItem', () => {

expect(wrapper.find('.tag.is-primary').text()).toEqual('External');
});

it('renders correct out of sync replicas number', () => {
const wrapper = mount(
<StaticRouter>
<table>
<tbody>
{setupComponent({
topic: { ...externalTopicPayload, partitions: undefined },
})}
</tbody>
</table>
</StaticRouter>
);

expect(wrapper.find('td').at(2).text()).toEqual('0');
});
});
Original file line number Diff line number Diff line change
@@ -1,25 +1,37 @@
import React from 'react';
import { ClusterName, TopicName } from 'redux/interfaces';
import { Topic, TopicDetails } from 'generated-sources';
import { NavLink, Switch, Route, Link } from 'react-router-dom';
import { NavLink, Switch, Route, Link, useHistory } from 'react-router-dom';
import {
clusterTopicSettingsPath,
clusterTopicPath,
clusterTopicMessagesPath,
clusterTopicsTopicEditPath,
clusterTopicsPath,
clusterTopicEditPath,
} from 'lib/paths';
import ClusterContext from 'components/contexts/ClusterContext';
import ConfirmationModal from 'components/common/ConfirmationModal/ConfirmationModal';
import OverviewContainer from './Overview/OverviewContainer';
import MessagesContainer from './Messages/MessagesContainer';
import SettingsContainer from './Settings/SettingsContainer';

interface Props extends Topic, TopicDetails {
clusterName: ClusterName;
topicName: TopicName;
deleteTopic: (clusterName: ClusterName, topicName: TopicName) => void;
}

const Details: React.FC<Props> = ({ clusterName, topicName }) => {
const Details: React.FC<Props> = ({ clusterName, topicName, deleteTopic }) => {
const history = useHistory();
const { isReadOnly } = React.useContext(ClusterContext);
const [
isDeleteTopicConfirmationVisible,
setDeleteTopicConfirmationVisible,
] = React.useState(false);
const deleteTopicHandler = React.useCallback(() => {
deleteTopic(clusterName, topicName);
history.push(clusterTopicsPath(clusterName));
}, [clusterName, topicName]);

return (
<div className="box">
Expand Down Expand Up @@ -51,14 +63,34 @@ const Details: React.FC<Props> = ({ clusterName, topicName }) => {
</NavLink>
</div>
<div className="navbar-end">
{!isReadOnly && (
<Link
to={clusterTopicsTopicEditPath(clusterName, topicName)}
className="button"
>
Edit settings
</Link>
)}
<div className="buttons">
{!isReadOnly && (
<>
<button
className="button is-danger"
type="button"
onClick={() => setDeleteTopicConfirmationVisible(true)}
>
Delete Topic
</button>

<Link
to={clusterTopicEditPath(clusterName, topicName)}
className="button"
>
Edit settings
</Link>

<ConfirmationModal
isOpen={isDeleteTopicConfirmationVisible}
onCancel={() => setDeleteTopicConfirmationVisible(false)}
onConfirm={deleteTopicHandler}
>
Are you sure want to remove <b>{topicName}</b> topic?
</ConfirmationModal>
</>
)}
</div>
</div>
</nav>
<br />
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { connect } from 'react-redux';
import { ClusterName, RootState, TopicName } from 'redux/interfaces';
import { withRouter, RouteComponentProps } from 'react-router-dom';
import { deleteTopic } from 'redux/actions';
import Details from './Details';

interface RouteProps {
Expand All @@ -22,4 +23,10 @@ const mapStateToProps = (
topicName,
});

export default withRouter(connect(mapStateToProps)(Details));
const mapDispatchToProps = {
deleteTopic,
};

export default withRouter(
connect(mapStateToProps, mapDispatchToProps)(Details)
);
Loading