Browse Source

[Draft] Refactored topic creation form state management (#802)

* Refactor topic creation

* Remove unused thunk

* Remove excess interface

* Add New page snapshot test

* Refactor new component tests

* Remove excess function
Azat Belgibayev 3 years ago
parent
commit
76af95ed78

+ 33 - 28
kafka-ui-react-app/src/components/Topics/New/New.tsx

@@ -1,40 +1,46 @@
 import React from 'react';
-import { ClusterName, TopicName, TopicFormData } from 'redux/interfaces';
+import { ClusterName, TopicFormData, FailurePayload } from 'redux/interfaces';
 import { useForm, FormProvider } from 'react-hook-form';
 import Breadcrumb from 'components/common/Breadcrumb/Breadcrumb';
-import { clusterTopicsPath } from 'lib/paths';
+import { clusterTopicPath, clusterTopicsPath } from 'lib/paths';
 import TopicForm from 'components/Topics/shared/Form/TopicForm';
+import {
+  formatTopicCreation,
+  topicsApiClient,
+  createTopicAction,
+} from 'redux/actions';
+import { useDispatch } from 'react-redux';
+import { getResponse } from 'lib/errorHandling';
+import { useHistory, useParams } from 'react-router';
 
-interface Props {
+interface RouterParams {
   clusterName: ClusterName;
-  isTopicCreated: boolean;
-  createTopic: (clusterName: ClusterName, form: TopicFormData) => void;
-  redirectToTopicPath: (clusterName: ClusterName, topicName: TopicName) => void;
-  resetUploadedState: () => void;
 }
 
-const New: React.FC<Props> = ({
-  clusterName,
-  isTopicCreated,
-  createTopic,
-  redirectToTopicPath,
-}) => {
+const New: React.FC = () => {
   const methods = useForm<TopicFormData>();
-  const [isSubmitting, setIsSubmitting] = React.useState<boolean>(false);
-
-  React.useEffect(() => {
-    if (isSubmitting && isTopicCreated) {
-      const { name } = methods.getValues();
-      redirectToTopicPath(clusterName, name);
-    }
-  }, [isSubmitting, isTopicCreated, redirectToTopicPath, clusterName, methods]);
+  const { clusterName } = useParams<RouterParams>();
+  const history = useHistory();
+  const dispatch = useDispatch();
 
   const onSubmit = async (data: TopicFormData) => {
-    // TODO: need to fix loader. After success loading the first time, we won't wait for creation any more, because state is
-    // loaded, and we will try to get entity immediately after pressing the button, and we will receive null
-    // going to object page on the second creation. Setting of isSubmitting after createTopic is a workaround, need to tweak loader logic
-    createTopic(clusterName, data);
-    setIsSubmitting(true); // Keep this action after createTopic to prevent redirect before create.
+    try {
+      await topicsApiClient.createTopic({
+        clusterName,
+        topicCreation: formatTopicCreation(data),
+      });
+
+      history.push(clusterTopicPath(clusterName, data.name));
+    } catch (error) {
+      const response = await getResponse(error);
+      const alert: FailurePayload = {
+        subject: ['schema', data.name].join('-'),
+        title: `Schema ${data.name}`,
+        response,
+      };
+
+      dispatch(createTopicAction.failure({ alert }));
+    }
   };
 
   return (
@@ -52,10 +58,9 @@ const New: React.FC<Props> = ({
       </div>
 
       <div className="box">
-        {/* eslint-disable react/jsx-props-no-spreading */}
         <FormProvider {...methods}>
           <TopicForm
-            isSubmitting={isSubmitting}
+            isSubmitting={methods.formState.isSubmitting}
             onSubmit={methods.handleSubmit(onSubmit)}
           />
         </FormProvider>

+ 0 - 48
kafka-ui-react-app/src/components/Topics/New/NewContainer.ts

@@ -1,48 +0,0 @@
-import { connect } from 'react-redux';
-import {
-  RootState,
-  ClusterName,
-  TopicName,
-  Action,
-  TopicFormData,
-} from 'redux/interfaces';
-import { withRouter, RouteComponentProps } from 'react-router-dom';
-import { createTopic, createTopicAction } from 'redux/actions';
-import { getTopicCreated } from 'redux/reducers/topics/selectors';
-import { clusterTopicPath } from 'lib/paths';
-import { ThunkDispatch } from 'redux-thunk';
-
-import New from './New';
-
-interface RouteProps {
-  clusterName: ClusterName;
-}
-
-type OwnProps = RouteComponentProps<RouteProps>;
-
-const mapStateToProps = (
-  state: RootState,
-  {
-    match: {
-      params: { clusterName },
-    },
-  }: OwnProps
-) => ({
-  clusterName,
-  isTopicCreated: getTopicCreated(state),
-});
-
-const mapDispatchToProps = (
-  dispatch: ThunkDispatch<RootState, undefined, Action>,
-  { history }: OwnProps
-) => ({
-  createTopic: (clusterName: ClusterName, form: TopicFormData) => {
-    dispatch(createTopic(clusterName, form));
-  },
-  redirectToTopicPath: (clusterName: ClusterName, topicName: TopicName) => {
-    history.push(clusterTopicPath(clusterName, topicName));
-  },
-  resetUploadedState: () => dispatch(createTopicAction.failure({})),
-});
-
-export default withRouter(connect(mapStateToProps, mapDispatchToProps)(New));

+ 69 - 0
kafka-ui-react-app/src/components/Topics/New/__tests__/New.spec.tsx

@@ -0,0 +1,69 @@
+import React from 'react';
+import New from 'components/Topics/New/New';
+import { Router } from 'react-router';
+import configureStore from 'redux-mock-store';
+import { RootState } from 'redux/interfaces';
+import { Provider } from 'react-redux';
+import { fireEvent, render, screen, waitFor } from '@testing-library/react';
+import { createMemoryHistory } from 'history';
+import fetchMock from 'fetch-mock-jest';
+import { clusterTopicNewPath, clusterTopicPath } from 'lib/paths';
+
+const mockStore = configureStore();
+
+describe('New', () => {
+  const clusterName = 'local';
+  const topicName = 'test-topic';
+
+  const initialState: Partial<RootState> = {};
+  const storeMock = mockStore(initialState);
+  const historyMock = createMemoryHistory();
+
+  beforeEach(() => {
+    fetchMock.restore();
+  });
+
+  const setupComponent = (history = historyMock, store = storeMock) => (
+    <Router history={history}>
+      <Provider store={store}>
+        <New />
+      </Provider>
+    </Router>
+  );
+
+  it('validates form', async () => {
+    const mockedHistory = createMemoryHistory();
+    jest.spyOn(mockedHistory, 'push');
+
+    render(setupComponent(mockedHistory));
+
+    await waitFor(async () => {
+      fireEvent.click(await screen.findByText('Send'));
+      const errorText = await screen.findByText('Topic Name is required.');
+      expect(mockedHistory.push).toBeCalledTimes(0);
+      expect(errorText).toBeTruthy();
+    });
+  });
+
+  it('submits valid form', async () => {
+    const mockedHistory = createMemoryHistory({
+      initialEntries: [clusterTopicNewPath(clusterName)],
+    });
+    jest.spyOn(mockedHistory, 'push');
+
+    render(setupComponent());
+
+    const input = await screen.findByPlaceholderText('Topic Name');
+    fireEvent.change(input, { target: { value: topicName } });
+    expect(input).toHaveValue(topicName);
+
+    waitFor(async () => {
+      fireEvent.click(await screen.findByText('Send'));
+
+      expect(mockedHistory.location.pathname).toBe(
+        clusterTopicPath(clusterName, topicName)
+      );
+      expect(mockedHistory.push).toBeCalledTimes(1);
+    });
+  });
+});

+ 2 - 6
kafka-ui-react-app/src/components/Topics/Topics.tsx

@@ -8,7 +8,7 @@ import {
 
 import ListContainer from './List/ListContainer';
 import TopicContainer from './Topic/TopicContainer';
-import NewContainer from './New/NewContainer';
+import New from './New/New';
 
 const Topics: React.FC = () => (
   <Switch>
@@ -17,11 +17,7 @@ const Topics: React.FC = () => (
       path={clusterTopicsPath(':clusterName')}
       component={ListContainer}
     />
-    <Route
-      exact
-      path={clusterTopicNewPath(':clusterName')}
-      component={NewContainer}
-    />
+    <Route exact path={clusterTopicNewPath(':clusterName')} component={New} />
     <Route
       path={clusterTopicPath(':clusterName', ':topicName')}
       component={TopicContainer}

+ 1 - 1
kafka-ui-react-app/src/components/Topics/shared/Form/TopicForm.tsx

@@ -156,7 +156,7 @@ const TopicForm: React.FC<Props> = ({
 
         <CustomParamsContainer isSubmitting={isSubmitting} config={config} />
 
-        <input type="submit" className="button is-primary" />
+        <input type="submit" className="button is-primary" value="Send" />
       </fieldset>
     </form>
   );

+ 1 - 35
kafka-ui-react-app/src/redux/actions/thunks/topics.ts

@@ -162,7 +162,7 @@ const topicReducer = (
   };
 };
 
-const formatTopicCreation = (form: TopicFormData): TopicCreation => {
+export const formatTopicCreation = (form: TopicFormData): TopicCreation => {
   const {
     name,
     partitions,
@@ -212,40 +212,6 @@ const formatTopicUpdate = (form: TopicFormDataRaw): TopicUpdate => {
   };
 };
 
-export const createTopic =
-  (clusterName: ClusterName, form: TopicFormData): PromiseThunkResult =>
-  async (dispatch, getState) => {
-    dispatch(actions.createTopicAction.request());
-    try {
-      const topic: Topic = await topicsApiClient.createTopic({
-        clusterName,
-        topicCreation: formatTopicCreation(form),
-      });
-
-      const state = getState().topics;
-      const newState = {
-        ...state,
-        byName: {
-          ...state.byName,
-          [topic.name]: {
-            ...topic,
-          },
-        },
-        allNames: [...state.allNames, topic.name],
-      };
-
-      dispatch(actions.createTopicAction.success(newState));
-    } catch (error) {
-      const response = await getResponse(error);
-      const alert: FailurePayload = {
-        subject: ['schema', form.name].join('-'),
-        title: `Schema ${form.name}`,
-        response,
-      };
-      dispatch(actions.createTopicAction.failure({ alert }));
-    }
-  };
-
 export const updateTopic =
   (
     clusterName: ClusterName,

+ 1 - 1
kafka-ui-react-app/src/redux/reducers/topics/selectors.ts

@@ -156,7 +156,7 @@ export const getTopicsOrderBy = createSelector(
 
 export const getIsTopicInternal = createSelector(
   getTopicByName,
-  ({ internal }) => !!internal
+  (topic) => !!topic?.internal
 );
 
 export const getTopicConsumerGroups = createSelector(