address review feedback

elastic · Apr 27, 2020 · aea38ed · aea38ed
1 parent 09c06cd
commit aea38ed
Show file tree

Hide file tree

Showing 7 changed files with 35 additions and 46 deletions.
diff --git a/...ck/plugins/ingest_pipelines/public/application/components/pipeline_form/pipeline_form.tsx b/...ck/plugins/ingest_pipelines/public/application/components/pipeline_form/pipeline_form.tsx
@@ -13,7 +13,6 @@ import { Pipeline } from '../../../../common/types';
 
 import { PipelineRequestFlyout } from '../';
 import { PipelineTestFlyout } from './pipeline_test_flyout';
-import { useTestConfigContext } from './test_config_context';
 import { PipelineFormFields } from './pipeline_form_fields';
 import { PipelineFormError } from './pipeline_form_error';
 import { pipelineFormSchema } from './schema';
@@ -44,23 +43,15 @@ export const PipelineForm: React.FunctionComponent<PipelineFormProps> = ({
   const [isRequestVisible, setIsRequestVisible] = useState<boolean>(false);
 
   const [isTestingPipeline, setIsTestingPipeline] = useState<boolean>(false);
-  const [shouldTestImmediately, setShouldTestImmediately] = useState<boolean>(false);
 
   const handleSave: FormConfig['onSubmit'] = (formData, isValid) => {
     if (isValid) {
       onSave(formData as Pipeline);
     }
   };
 
-  const { testConfig } = useTestConfigContext();
-  const { documents: cachedDocuments } = testConfig;
-
   const handleTestPipelineClick = () => {
     setIsTestingPipeline(true);
-
-    if (cachedDocuments) {
-      setShouldTestImmediately(true);
-    }
   };
 
   const { form } = useForm({
@@ -164,7 +155,6 @@ export const PipelineForm: React.FunctionComponent<PipelineFormProps> = ({
         {/* Test pipeline flyout */}
         {isTestingPipeline ? (
           <PipelineTestFlyout
-            shouldTestImmediately={shouldTestImmediately}
             closeFlyout={() => {
               setIsTestingPipeline(prevIsTestingPipeline => !prevIsTestingPipeline);
             }}

diff --git a/...s/ingest_pipelines/public/application/components/pipeline_form/pipeline_form_provider.tsx b/...s/ingest_pipelines/public/application/components/pipeline_form/pipeline_form_provider.tsx
@@ -6,15 +6,13 @@
 
 import React from 'react';
 
-import { PipelineForm, PipelineFormProps } from './pipeline_form';
-import { TestConfigContextProvider, useTestConfig } from './test_config_context';
+import { PipelineForm as PipelineFormUI, PipelineFormProps } from './pipeline_form';
+import { TestConfigContextProvider } from './test_config_context';
 
 export const PipelineFormProvider: React.FunctionComponent<PipelineFormProps> = passThroughProps => {
-  const testConfigContextValue = useTestConfig();
-
   return (
-    <TestConfigContextProvider value={testConfigContextValue}>
-      <PipelineForm {...passThroughProps} />
+    <TestConfigContextProvider>
+      <PipelineFormUI {...passThroughProps} />
     </TestConfigContextProvider>
   );
 };
diff --git a/...public/application/components/pipeline_form/pipeline_test_flyout/pipeline_test_flyout.tsx b/...public/application/components/pipeline_form/pipeline_test_flyout/pipeline_test_flyout.tsx
@@ -26,14 +26,12 @@ export interface PipelineTestFlyoutProps {
   closeFlyout: () => void;
   pipeline: Pipeline;
   isPipelineValid: boolean;
-  shouldTestImmediately: boolean;
 }
 
 export const PipelineTestFlyout: React.FunctionComponent<PipelineTestFlyoutProps> = ({
   closeFlyout,
   pipeline,
   isPipelineValid,
-  shouldTestImmediately,
 }) => {
   const { services } = useKibana();
 
@@ -43,7 +41,7 @@ export const PipelineTestFlyout: React.FunctionComponent<PipelineTestFlyoutProps
   const initialSelectedTab = cachedDocuments ? 'output' : 'documents';
   const [selectedTab, setSelectedTab] = useState<Tab>(initialSelectedTab);
 
-  const [hasNotExecuted, setHasNotExecuted] = useState<boolean>(true);
+  const [shouldExecuteImmediately, setShouldExecuteImmediately] = useState<boolean>(false);
   const [isExecuting, setIsExecuting] = useState<boolean>(false);
   const [executeError, setExecuteError] = useState<any>(null);
   const [executeOutput, setExecuteOutput] = useState<any>(undefined);
@@ -73,28 +71,39 @@ export const PipelineTestFlyout: React.FunctionComponent<PipelineTestFlyoutProps
       services.notifications.toasts.addSuccess(
         i18n.translate('xpack.ingestPipelines.testPipelineFlyout.successNotificationText', {
           defaultMessage: 'Pipeline executed',
-        })
+        }),
+        {
+          toastLifeTimeMs: 1000,
+        }
       );
 
       setSelectedTab('output');
     },
     [pipeline, services.api, services.notifications.toasts]
   );
 
+  useEffect(() => {
+    if (cachedDocuments) {
+      setShouldExecuteImmediately(true);
+    }
+    // We only want to know on initial mount if there are cached documents
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, []);
+
   useEffect(() => {
     // If the user has already tested the pipeline once,
     // use the cached test config and automatically execute the pipeline
-    if (hasNotExecuted && shouldTestImmediately && Object.entries(pipeline).length > 0) {
+    if (shouldExecuteImmediately && Object.entries(pipeline).length > 0) {
+      setShouldExecuteImmediately(false);
       handleExecute(cachedDocuments!, cachedVerbose);
-      setHasNotExecuted(false);
     }
   }, [
     pipeline,
     handleExecute,
     cachedDocuments,
     cachedVerbose,
-    hasNotExecuted,
-    shouldTestImmediately,
+    isExecuting,
+    shouldExecuteImmediately,
   ]);
 
   let tabContent;

diff --git a/...plication/components/pipeline_form/pipeline_test_flyout/pipeline_test_flyout_provider.tsx b/...plication/components/pipeline_form/pipeline_test_flyout/pipeline_test_flyout_provider.tsx
@@ -12,10 +12,7 @@ import { PipelineTestFlyout, PipelineTestFlyoutProps } from './pipeline_test_fly
 
 type Props = Omit<PipelineTestFlyoutProps, 'pipeline' | 'isPipelineValid'>;
 
-export const PipelineTestFlyoutProvider: React.FunctionComponent<Props> = ({
-  closeFlyout,
-  shouldTestImmediately,
-}) => {
+export const PipelineTestFlyoutProvider: React.FunctionComponent<Props> = ({ closeFlyout }) => {
   const form = useFormContext();
   const [formData, setFormData] = useState<Pipeline>({} as Pipeline);
   const [isFormDataValid, setIsFormDataValid] = useState<boolean>(false);
@@ -37,7 +34,6 @@ export const PipelineTestFlyoutProvider: React.FunctionComponent<Props> = ({
       pipeline={formData}
       closeFlyout={closeFlyout}
       isPipelineValid={isFormDataValid}
-      shouldTestImmediately={shouldTestImmediately}
     />
   );
 };
diff --git a/...ines/public/application/components/pipeline_form/pipeline_test_flyout/tabs/tab_output.tsx b/...ines/public/application/components/pipeline_form/pipeline_test_flyout/tabs/tab_output.tsx
@@ -41,7 +41,7 @@ export const OutputTab: React.FunctionComponent<Props> = ({
     handleExecute(cachedDocuments!, isVerboseEnabled);
   };
 
-  let content;
+  let content: React.ReactNode | undefined;
 
   if (isExecuting) {
     content = <EuiLoadingSpinner size="m" />;

diff --git a/...gins/ingest_pipelines/public/application/components/pipeline_form/test_config_context.tsx b/...gins/ingest_pipelines/public/application/components/pipeline_form/test_config_context.tsx
@@ -35,17 +35,7 @@ export const useTestConfigContext = () => {
   return ctx;
 };
 
-export const TestConfigContextProvider = ({
-  children,
-  value,
-}: {
-  value: TestConfigContext;
-  children: React.ReactNode;
-}) => {
-  return <TestConfigContext.Provider value={value}> {children} </TestConfigContext.Provider>;
-};
-
-export const useTestConfig = () => {
+export const TestConfigContextProvider = ({ children }: { children: React.ReactNode }) => {
   const [testConfig, setTestConfig] = useState<TestConfig>({
     verbose: false,
   });
@@ -54,8 +44,14 @@ export const useTestConfig = () => {
     setTestConfig(currentTestConfig);
   }, []);
 
-  return {
-    testConfig,
-    setCurrentTestConfig,
-  };
+  return (
+    <TestConfigContext.Provider
+      value={{
+        testConfig,
+        setCurrentTestConfig,
+      }}
+    >
+      {children}
+    </TestConfigContext.Provider>
+  );
 };
diff --git a/...ck/plugins/ingest_pipelines/public/application/sections/pipelines_edit/pipelines_edit.tsx b/...ck/plugins/ingest_pipelines/public/application/sections/pipelines_edit/pipelines_edit.tsx
@@ -65,7 +65,7 @@ export const PipelinesEdit: React.FunctionComponent<RouteComponentProps<MatchPar
     services.breadcrumbs.setBreadcrumbs('edit');
   }, [services.breadcrumbs]);
 
-  let content;
+  let content: React.ReactNode;
 
   if (isLoading) {
     content = (