From fb07d1ae2b3c2ae9454ddc67590574ce5a5ae0f2 Mon Sep 17 00:00:00 2001 From: Oleg Shuralev Date: Tue, 20 Apr 2021 22:38:12 +0300 Subject: [PATCH 1/4] Add schema deletion confirmation modal --- .../components/Schemas/Details/Details.tsx | 27 +++++++++++---- .../Schemas/Details/__test__/Details.spec.tsx | 26 ++++++++++++--- .../__snapshots__/Details.spec.tsx.snap | 33 +++++++++++++++++++ 3 files changed, 74 insertions(+), 12 deletions(-) diff --git a/kafka-ui-react-app/src/components/Schemas/Details/Details.tsx b/kafka-ui-react-app/src/components/Schemas/Details/Details.tsx index c5562a070d8..c4225e6f1c6 100644 --- a/kafka-ui-react-app/src/components/Schemas/Details/Details.tsx +++ b/kafka-ui-react-app/src/components/Schemas/Details/Details.tsx @@ -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; @@ -32,15 +33,20 @@ const Details: React.FC = ({ 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 (
@@ -84,10 +90,17 @@ const Details: React.FC = ({ className="button is-danger is-small level-item" type="button" title="in development" - onClick={onDelete} + onClick={() => setDeleteSchemaConfirmationVisible(true)} > Remove + setDeleteSchemaConfirmationVisible(false)} + onConfirm={onDelete} + > + Are you sure want to remove {subject} schema? +
)} diff --git a/kafka-ui-react-app/src/components/Schemas/Details/__test__/Details.spec.tsx b/kafka-ui-react-app/src/components/Schemas/Details/__test__/Details.spec.tsx index 9413cd9de61..ef4f220a6c4 100644 --- a/kafka-ui-react-app/src/components/Schemas/Details/__test__/Details.spec.tsx +++ b/kafka-ui-react-app/src/components/Schemas/Details/__test__/Details.spec.tsx @@ -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(); @@ -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( {setupWrapper({ versions, deleteSchema: mockDelete })} ); - 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); }); diff --git a/kafka-ui-react-app/src/components/Schemas/Details/__test__/__snapshots__/Details.spec.tsx.snap b/kafka-ui-react-app/src/components/Schemas/Details/__test__/__snapshots__/Details.spec.tsx.snap index a7e02e6df24..ef119b69215 100644 --- a/kafka-ui-react-app/src/components/Schemas/Details/__test__/__snapshots__/Details.spec.tsx.snap +++ b/kafka-ui-react-app/src/components/Schemas/Details/__test__/__snapshots__/Details.spec.tsx.snap @@ -67,6 +67,17 @@ exports[`Details View Initial state matches snapshot 1`] = ` > Remove + + Are you sure want to remove + + test + + schema? + Remove + + Are you sure want to remove + + test + + schema? + Remove + + Are you sure want to remove + + test + + schema? + Date: Tue, 20 Apr 2021 23:14:56 +0300 Subject: [PATCH 2/4] Topic deletion confirmation --- .../src/components/Topics/List/ListItem.tsx | 41 +++++++++----- .../Topics/List/__tests__/ListItem.spec.tsx | 37 ++++++++++++- .../Topics/Topic/Details/Details.tsx | 54 +++++++++++++++---- .../Topics/Topic/Details/DetailsContainer.ts | 9 +++- .../Details/Settings/SettingsContainer.ts | 3 +- .../src/lib/__tests__/paths.spec.ts | 4 +- kafka-ui-react-app/src/lib/paths.ts | 2 +- 7 files changed, 120 insertions(+), 30 deletions(-) diff --git a/kafka-ui-react-app/src/components/Topics/List/ListItem.tsx b/kafka-ui-react-app/src/components/Topics/List/ListItem.tsx index 82097ddb94a..77c1e985faf 100644 --- a/kafka-ui-react-app/src/components/Topics/List/ListItem.tsx +++ b/kafka-ui-react-app/src/components/Topics/List/ListItem.tsx @@ -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; @@ -20,6 +21,11 @@ const ListItem: React.FC = ({ deleteTopic, clusterName, }) => { + const [ + isDeleteTopicConfirmationVisible, + setDeleteTopicConfirmationVisible, + ] = React.useState(false); + const outOfSyncReplicas = React.useMemo(() => { if (partitions === undefined || partitions.length === 0) { return 0; @@ -54,19 +60,30 @@ const ListItem: React.FC = ({ {internal ? 'Internal' : 'External'} - - - - - } - right + +
+ + + + } + right + > + setDeleteTopicConfirmationVisible(true)} + > + Remove Topic + + +
+ setDeleteTopicConfirmationVisible(false)} + onConfirm={deleteTopicHandler} > - - Remove Topic - - + Are you sure want to remove {name} topic? + ); diff --git a/kafka-ui-react-app/src/components/Topics/List/__tests__/ListItem.spec.tsx b/kafka-ui-react-app/src/components/Topics/List/__tests__/ListItem.spec.tsx index 26f16cdf77d..81e5335ef64 100644 --- a/kafka-ui-react-app/src/components/Topics/List/__tests__/ListItem.spec.tsx +++ b/kafka-ui-react-app/src/components/Topics/List/__tests__/ListItem.spec.tsx @@ -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 = {}) => ( { 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( @@ -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( + + + + {setupComponent({ + topic: { ...externalTopicPayload, partitions: undefined }, + })} + +
+ + ); + + expect(wrapper.find('td').at(2).text()).toEqual('0'); + }); }); diff --git a/kafka-ui-react-app/src/components/Topics/Topic/Details/Details.tsx b/kafka-ui-react-app/src/components/Topics/Topic/Details/Details.tsx index 099c255ba42..6ee66383a23 100644 --- a/kafka-ui-react-app/src/components/Topics/Topic/Details/Details.tsx +++ b/kafka-ui-react-app/src/components/Topics/Topic/Details/Details.tsx @@ -1,14 +1,16 @@ 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'; @@ -16,10 +18,20 @@ import SettingsContainer from './Settings/SettingsContainer'; interface Props extends Topic, TopicDetails { clusterName: ClusterName; topicName: TopicName; + deleteTopic: (clusterName: ClusterName, topicName: TopicName) => void; } -const Details: React.FC = ({ clusterName, topicName }) => { +const Details: React.FC = ({ 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 (
@@ -51,14 +63,34 @@ const Details: React.FC = ({ clusterName, topicName }) => {
- {!isReadOnly && ( - - Edit settings - - )} +
+ {!isReadOnly && ( + <> + + + + Edit settings + + + setDeleteTopicConfirmationVisible(false)} + onConfirm={deleteTopicHandler} + > + Are you sure want to remove {topicName} topic? + + + )} +

diff --git a/kafka-ui-react-app/src/components/Topics/Topic/Details/DetailsContainer.ts b/kafka-ui-react-app/src/components/Topics/Topic/Details/DetailsContainer.ts index f62a34f9879..9a9af206831 100644 --- a/kafka-ui-react-app/src/components/Topics/Topic/Details/DetailsContainer.ts +++ b/kafka-ui-react-app/src/components/Topics/Topic/Details/DetailsContainer.ts @@ -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 { @@ -22,4 +23,10 @@ const mapStateToProps = ( topicName, }); -export default withRouter(connect(mapStateToProps)(Details)); +const mapDispatchToProps = { + deleteTopic, +}; + +export default withRouter( + connect(mapStateToProps, mapDispatchToProps)(Details) +); diff --git a/kafka-ui-react-app/src/components/Topics/Topic/Details/Settings/SettingsContainer.ts b/kafka-ui-react-app/src/components/Topics/Topic/Details/Settings/SettingsContainer.ts index af2c9a244be..4e4278aead8 100644 --- a/kafka-ui-react-app/src/components/Topics/Topic/Details/Settings/SettingsContainer.ts +++ b/kafka-ui-react-app/src/components/Topics/Topic/Details/Settings/SettingsContainer.ts @@ -30,8 +30,7 @@ const mapStateToProps = ( }); const mapDispatchToProps = { - fetchTopicConfig: (clusterName: ClusterName, topicName: TopicName) => - fetchTopicConfig(clusterName, topicName), + fetchTopicConfig, }; export default withRouter( diff --git a/kafka-ui-react-app/src/lib/__tests__/paths.spec.ts b/kafka-ui-react-app/src/lib/__tests__/paths.spec.ts index f1118960c70..18d2e26c839 100644 --- a/kafka-ui-react-app/src/lib/__tests__/paths.spec.ts +++ b/kafka-ui-react-app/src/lib/__tests__/paths.spec.ts @@ -61,8 +61,8 @@ describe('Paths', () => { '/ui/clusters/local/topics/topic123/messages' ); }); - it('clusterTopicsTopicEditPath', () => { - expect(paths.clusterTopicsTopicEditPath('local', 'topic123')).toEqual( + it('clusterTopicEditPath', () => { + expect(paths.clusterTopicEditPath('local', 'topic123')).toEqual( '/ui/clusters/local/topics/topic123/edit' ); }); diff --git a/kafka-ui-react-app/src/lib/paths.ts b/kafka-ui-react-app/src/lib/paths.ts index e05e880612c..b0d6ae7229b 100644 --- a/kafka-ui-react-app/src/lib/paths.ts +++ b/kafka-ui-react-app/src/lib/paths.ts @@ -41,7 +41,7 @@ export const clusterTopicMessagesPath = ( clusterName: ClusterName, topicName: TopicName ) => `${clusterTopicsPath(clusterName)}/${topicName}/messages`; -export const clusterTopicsTopicEditPath = ( +export const clusterTopicEditPath = ( clusterName: ClusterName, topicName: TopicName ) => `${clusterTopicsPath(clusterName)}/${topicName}/edit`; From 068d8c5b4e3a7d9718d7bf5dde56ceb7acade94c Mon Sep 17 00:00:00 2001 From: Oleg Shuralev Date: Wed, 21 Apr 2021 13:35:14 +0300 Subject: [PATCH 3/4] Specs --- .../Schemas/Details/__test__/Details.spec.tsx | 29 +++++++++++++++++-- .../lib/{__tests__ => __test__}/paths.spec.ts | 1 - 2 files changed, 26 insertions(+), 4 deletions(-) rename kafka-ui-react-app/src/lib/{__tests__ => __test__}/paths.spec.ts (99%) diff --git a/kafka-ui-react-app/src/components/Schemas/Details/__test__/Details.spec.tsx b/kafka-ui-react-app/src/components/Schemas/Details/__test__/Details.spec.tsx index ef4f220a6c4..ab98c48193c 100644 --- a/kafka-ui-react-app/src/components/Schemas/Details/__test__/Details.spec.tsx +++ b/kafka-ui-react-app/src/components/Schemas/Details/__test__/Details.spec.tsx @@ -121,13 +121,36 @@ describe('Details', () => { wrapper.find('mock-ConfirmationModal').prop('isOpen') ).toBeTruthy(); - wrapper - .find('mock-ConfirmationModal') - .prop<() => void>('onConfirm')(); + // @ts-expect-error lack of typing of enzyme#invoke + wrapper.find('mock-ConfirmationModal').invoke('onConfirm')(); expect(mockDelete).toHaveBeenCalledTimes(1); }); + it('calls deleteSchema after confirmation', () => { + const mockDelete = jest.fn(); + const wrapper = mount( + + {setupWrapper({ versions, deleteSchema: mockDelete })} + + ); + expect( + wrapper.find('mock-ConfirmationModal').prop('isOpen') + ).toBeFalsy(); + + wrapper.find('button').at(1).simulate('click'); + expect( + wrapper.find('mock-ConfirmationModal').prop('isOpen') + ).toBeTruthy(); + + // @ts-expect-error lack of typing of enzyme#invoke + wrapper.find('mock-ConfirmationModal').invoke('onCancel')(); + + expect( + wrapper.find('mock-ConfirmationModal').prop('isOpen') + ).toBeFalsy(); + }); + it('matches snapshot', () => { expect(shallow(setupWrapper({ versions }))).toMatchSnapshot(); }); diff --git a/kafka-ui-react-app/src/lib/__tests__/paths.spec.ts b/kafka-ui-react-app/src/lib/__test__/paths.spec.ts similarity index 99% rename from kafka-ui-react-app/src/lib/__tests__/paths.spec.ts rename to kafka-ui-react-app/src/lib/__test__/paths.spec.ts index 18d2e26c839..ef4b0f3e4fe 100644 --- a/kafka-ui-react-app/src/lib/__tests__/paths.spec.ts +++ b/kafka-ui-react-app/src/lib/__test__/paths.spec.ts @@ -66,7 +66,6 @@ describe('Paths', () => { '/ui/clusters/local/topics/topic123/edit' ); }); - it('clusterConnectorsPath', () => { expect(paths.clusterConnectorsPath('local')).toEqual( '/ui/clusters/local/connectors' From 5196311b9b50a807f8570d939d1e6a6ccdb786b3 Mon Sep 17 00:00:00 2001 From: Oleg Shuralev Date: Wed, 21 Apr 2021 13:53:58 +0300 Subject: [PATCH 4/4] Specs --- .../Schemas/Details/__test__/Details.spec.tsx | 78 ++++++++----------- 1 file changed, 34 insertions(+), 44 deletions(-) diff --git a/kafka-ui-react-app/src/components/Schemas/Details/__test__/Details.spec.tsx b/kafka-ui-react-app/src/components/Schemas/Details/__test__/Details.spec.tsx index ab98c48193c..7816369f63c 100644 --- a/kafka-ui-react-app/src/components/Schemas/Details/__test__/Details.spec.tsx +++ b/kafka-ui-react-app/src/components/Schemas/Details/__test__/Details.spec.tsx @@ -1,6 +1,6 @@ import React from 'react'; import { Provider } from 'react-redux'; -import { shallow, mount } from 'enzyme'; +import { shallow, mount, ReactWrapper } from 'enzyme'; import configureStore from 'redux/store/configureStore'; import { StaticRouter } from 'react-router'; import ClusterContext from 'components/contexts/ClusterContext'; @@ -105,54 +105,44 @@ describe('Details', () => { expect(wrapper.find('SchemaVersion').length).toEqual(2); }); - it('calls deleteSchema after confirmation', () => { - const mockDelete = jest.fn(); - const wrapper = mount( - - {setupWrapper({ versions, deleteSchema: mockDelete })} - - ); - expect( - wrapper.find('mock-ConfirmationModal').prop('isOpen') - ).toBeFalsy(); - - wrapper.find('button').at(1).simulate('click'); - expect( - wrapper.find('mock-ConfirmationModal').prop('isOpen') - ).toBeTruthy(); - - // @ts-expect-error lack of typing of enzyme#invoke - wrapper.find('mock-ConfirmationModal').invoke('onConfirm')(); - - expect(mockDelete).toHaveBeenCalledTimes(1); + it('matches snapshot', () => { + expect(shallow(setupWrapper({ versions }))).toMatchSnapshot(); }); - it('calls deleteSchema after confirmation', () => { + describe('confirmation', () => { + let wrapper: ReactWrapper; + let confirmationModal: ReactWrapper; const mockDelete = jest.fn(); - const wrapper = mount( - - {setupWrapper({ versions, deleteSchema: mockDelete })} - - ); - expect( - wrapper.find('mock-ConfirmationModal').prop('isOpen') - ).toBeFalsy(); - wrapper.find('button').at(1).simulate('click'); - expect( - wrapper.find('mock-ConfirmationModal').prop('isOpen') - ).toBeTruthy(); - - // @ts-expect-error lack of typing of enzyme#invoke - wrapper.find('mock-ConfirmationModal').invoke('onCancel')(); - - expect( - wrapper.find('mock-ConfirmationModal').prop('isOpen') - ).toBeFalsy(); - }); + const findConfirmationModal = () => + wrapper.find('mock-ConfirmationModal'); - it('matches snapshot', () => { - expect(shallow(setupWrapper({ versions }))).toMatchSnapshot(); + beforeEach(() => { + wrapper = mount( + + {setupWrapper({ versions, deleteSchema: mockDelete })} + + ); + confirmationModal = findConfirmationModal(); + }); + + it('calls deleteSchema after confirmation', () => { + expect(confirmationModal.prop('isOpen')).toBeFalsy(); + wrapper.find('button').at(1).simulate('click'); + expect(findConfirmationModal().prop('isOpen')).toBeTruthy(); + // @ts-expect-error lack of typing of enzyme#invoke + confirmationModal.invoke('onConfirm')(); + expect(mockDelete).toHaveBeenCalledTimes(1); + }); + + it('calls deleteSchema after confirmation', () => { + expect(confirmationModal.prop('isOpen')).toBeFalsy(); + wrapper.find('button').at(1).simulate('click'); + expect(findConfirmationModal().prop('isOpen')).toBeTruthy(); + // @ts-expect-error lack of typing of enzyme#invoke + wrapper.find('mock-ConfirmationModal').invoke('onCancel')(); + expect(findConfirmationModal().prop('isOpen')).toBeFalsy(); + }); }); });