Browse Source

Confirmation modal for topic & schema delete actions (#384)

Oleg Shur 4 years ago
parent
commit
ca4b3f12f9

+ 20 - 7
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<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">
@@ -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>

+ 43 - 14
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';
@@ -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,29 +97,53 @@ 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', () => {
-          const mockDelete = jest.fn();
-          const component = mount(
-            <StaticRouter>
-              {setupWrapper({ versions, deleteSchema: mockDelete })}
-            </StaticRouter>
-          );
-          component.find('button').at(1).simulate('click');
-          expect(mockDelete).toHaveBeenCalledTimes(1);
-        });
-
         it('matches snapshot', () => {
           expect(shallow(setupWrapper({ versions }))).toMatchSnapshot();
         });
+
+        describe('confirmation', () => {
+          let wrapper: ReactWrapper;
+          let confirmationModal: ReactWrapper;
+          const mockDelete = jest.fn();
+
+          const findConfirmationModal = () =>
+            wrapper.find('mock-ConfirmationModal');
+
+          beforeEach(() => {
+            wrapper = mount(
+              <StaticRouter>
+                {setupWrapper({ versions, deleteSchema: mockDelete })}
+              </StaticRouter>
+            );
+            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();
+          });
+        });
       });
 
       describe('when the readonly flag is set', () => {

+ 33 - 0
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
         </button>
+        <mock-ConfirmationModal
+          isOpen={false}
+          onCancel={[Function]}
+          onConfirm={[Function]}
+        >
+          Are you sure want to remove 
+          <b>
+            test
+          </b>
+           schema?
+        </mock-ConfirmationModal>
       </div>
     </div>
     <LatestVersionItem
@@ -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
@@ -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

+ 29 - 12
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<ListItemProps> = ({
   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<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>
   );

+ 36 - 1
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<ListItemProps> = {}) => (
     <ListItem
@@ -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>
@@ -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');
+  });
 });

+ 43 - 11
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<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">
@@ -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 />

+ 8 - 1
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)
+);

+ 1 - 2
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(

+ 2 - 3
kafka-ui-react-app/src/lib/__tests__/paths.spec.ts → kafka-ui-react-app/src/lib/__test__/paths.spec.ts

@@ -61,12 +61,11 @@ 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'
     );
   });
-
   it('clusterConnectorsPath', () => {
     expect(paths.clusterConnectorsPath('local')).toEqual(
       '/ui/clusters/local/connectors'

+ 1 - 1
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`;