You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2020/09/17 14:31:06 UTC

[GitHub] [apisix-dashboard] LiteSun opened a new pull request #483: feat: improve route

LiteSun opened a new pull request #483:
URL: https://github.com/apache/apisix-dashboard/pull/483


   Please answer these questions before submitting a pull request
   
   - Why submit this pull request?
   - [ ] Bug fix
   - [ ] New feature provided
   - [x] Improve performance
   
   - Related issues
   
   ___
   ### Bugfix
   - Description
   
   - How to fix?
   
   ___
   ### New feature or improvement
   - Describe the details and related test reports.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-dashboard] juzhiyuan commented on a change in pull request #483: feat: improve route

Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on a change in pull request #483:
URL: https://github.com/apache/apisix-dashboard/pull/483#discussion_r491840463



##########
File path: src/pages/Route/Create.tsx
##########
@@ -68,80 +60,63 @@ const Page: React.FC<Props> = (props) => {
     formatMessage({ id: 'route.constants.preview' }),
   ];
 
-  const [step1Data, setStep1Data] = useState(DEFAULT_STEP_1_DATA);
-  const [step2Data, setStep2Data] = useState(DEFAULT_STEP_2_DATA);
+  const [advancedMatchingRules, setAdvancedMatchingRules] = useState<RouteModule.MatchingRule[]>(
+    [],
+  );
+  const [upstreamHeaderList, setUpstreamHeaderList] = useState<RouteModule.UpstreamHeader[]>([]);
   const [step3Data, setStep3Data] = useState(DEFAULT_STEP_3_DATA);
-
   const [redirect, setRedirect] = useState(false);
 
   const [form1] = Form.useForm();
   const [form2] = Form.useForm();
 
   const [step, setStep] = useState(1);
   const [stepHeader, setStepHeader] = useState(STEP_HEADER_4);
-
   const [chart, setChart] = useState(INIT_CHART);
 
-  const routeData = {
-    step1Data,
-    step2Data,
-    step3Data,
-  };
   const setupRoute = (rid: number) =>
     fetchItem(rid).then((data) => {
-      form1.setFieldsValue(data.step1Data);
-      setStep1Data(data.step1Data as RouteModule.Step1Data);
-
-      form2.setFieldsValue(data.step2Data);
-      setStep2Data(data.step2Data);
-
+      form1.setFieldsValue(data.form1Data);
+      setAdvancedMatchingRules(data.advancedMatchingRules);
+      form2.setFieldsValue(data.form2Data);
+      setUpstreamHeaderList(data.upstreamHeaderList);
       setStep3Data(data.step3Data);
     });
 
-  useEffect(() => {
-    if (props.route.path.indexOf('edit') !== -1) {
-      setupRoute(props.match.params.rid);
-    }
-  }, []);
-
-  useEffect(() => {
-    const { redirectOption } = step1Data;
-
-    if (redirectOption === 'customRedirect') {
-      setRedirect(true);
-      setStepHeader(STEP_HEADER_2);
-      return;
-    }
-    setRedirect(false);
-    setStepHeader(STEP_HEADER_4);
-  }, [step1Data]);
-
   const onReset = () => {
-    setStep1Data(DEFAULT_STEP_1_DATA);
-    setStep2Data(DEFAULT_STEP_2_DATA);
+    setAdvancedMatchingRules([]);
+    setUpstreamHeaderList([]);
     setStep3Data(DEFAULT_STEP_3_DATA);
-
     form1.setFieldsValue(DEFAULT_STEP_1_DATA);
     form2.setFieldsValue(DEFAULT_STEP_2_DATA);
     setStep(1);
   };
 
+  useEffect(() => {
+    if (props.route.path.indexOf('edit') !== -1) {
+      setupRoute(props.match.params.rid);
+    } else {
+      onReset();

Review comment:
       setupXX would be better than reset




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-dashboard] juzhiyuan merged pull request #483: feat: improve route

Posted by GitBox <gi...@apache.org>.
juzhiyuan merged pull request #483:
URL: https://github.com/apache/apisix-dashboard/pull/483


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-dashboard] LiteSun commented on a change in pull request #483: feat: improve route

Posted by GitBox <gi...@apache.org>.
LiteSun commented on a change in pull request #483:
URL: https://github.com/apache/apisix-dashboard/pull/483#discussion_r491781970



##########
File path: src/pages/Route/transform.ts
##########
@@ -76,34 +80,33 @@ export const transformStepData = ({
       return [key, operator, value];
     }),
     upstream: {
-      type: step2Data.type,
+      type: form2Data.type,
       ...chashData,
       nodes,
-      timeout: step2Data.timeout,
+      timeout: form2Data.timeout,
     },
     upstream_header,
   };
 
-  if (step2Data.upstreamPath) {
+  if (form2Data.upstreamPath) {
     data.upstream_path = {
-      to: step2Data.upstreamPath,
+      to: form2Data.upstreamPath,
     };
-    if (step2Data.mappingStrategy) {
+    if (form2Data.mappingStrategy) {
       data.upstream_path = {
         ...data.upstream_path,
-        from: step2Data.mappingStrategy,
+        from: form2Data.mappingStrategy,
         type: 'regx',
       };
     }
   }
 
-  if (step3Data.plugins.prometheus) {
-    // eslint-disable-next-line no-param-reassign
-    step3Data.plugins.prometheus = {};
-  }
-
   // 未启用 redirect
   if (!redirect.uri) {
+    if (step3Data.plugins.prometheus) {
+      // eslint-disable-next-line no-param-reassign
+      step3Data.plugins.prometheus = {};

Review comment:
       ok

##########
File path: src/pages/Route/Create.tsx
##########
@@ -68,80 +60,63 @@ const Page: React.FC<Props> = (props) => {
     formatMessage({ id: 'route.constants.preview' }),
   ];
 
-  const [step1Data, setStep1Data] = useState(DEFAULT_STEP_1_DATA);
-  const [step2Data, setStep2Data] = useState(DEFAULT_STEP_2_DATA);
+  const [advancedMatchingRules, setAdvancedMatchingRules] = useState<RouteModule.MatchingRule[]>(
+    [],
+  );
+  const [upstreamHeaderList, setUpstreamHeaderList] = useState<RouteModule.UpstreamHeader[]>([]);
   const [step3Data, setStep3Data] = useState(DEFAULT_STEP_3_DATA);
-
   const [redirect, setRedirect] = useState(false);
 
   const [form1] = Form.useForm();
   const [form2] = Form.useForm();
 
   const [step, setStep] = useState(1);
   const [stepHeader, setStepHeader] = useState(STEP_HEADER_4);
-
   const [chart, setChart] = useState(INIT_CHART);
 
-  const routeData = {
-    step1Data,
-    step2Data,
-    step3Data,
-  };
   const setupRoute = (rid: number) =>
     fetchItem(rid).then((data) => {
-      form1.setFieldsValue(data.step1Data);
-      setStep1Data(data.step1Data as RouteModule.Step1Data);
-
-      form2.setFieldsValue(data.step2Data);
-      setStep2Data(data.step2Data);
-
+      form1.setFieldsValue(data.form1Data);
+      setAdvancedMatchingRules(data.advancedMatchingRules);
+      form2.setFieldsValue(data.form2Data);
+      setUpstreamHeaderList(data.upstreamHeaderList);
       setStep3Data(data.step3Data);
     });
 
-  useEffect(() => {
-    if (props.route.path.indexOf('edit') !== -1) {
-      setupRoute(props.match.params.rid);
-    }
-  }, []);
-
-  useEffect(() => {
-    const { redirectOption } = step1Data;
-
-    if (redirectOption === 'customRedirect') {
-      setRedirect(true);
-      setStepHeader(STEP_HEADER_2);
-      return;
-    }
-    setRedirect(false);
-    setStepHeader(STEP_HEADER_4);
-  }, [step1Data]);
-
   const onReset = () => {
-    setStep1Data(DEFAULT_STEP_1_DATA);
-    setStep2Data(DEFAULT_STEP_2_DATA);
+    setAdvancedMatchingRules([]);
+    setUpstreamHeaderList([]);
     setStep3Data(DEFAULT_STEP_3_DATA);
-
     form1.setFieldsValue(DEFAULT_STEP_1_DATA);
     form2.setFieldsValue(DEFAULT_STEP_2_DATA);
     setStep(1);
   };
 
+  useEffect(() => {
+    if (props.route.path.indexOf('edit') !== -1) {
+      setupRoute(props.match.params.rid);
+    } else {
+      onReset();

Review comment:
       > Why we should reset all fields when creating a route? the initial data should be empty, right?
   `onReset()` is here to initialize the page data.

##########
File path: src/pages/Route/Create.tsx
##########
@@ -68,80 +60,63 @@ const Page: React.FC<Props> = (props) => {
     formatMessage({ id: 'route.constants.preview' }),
   ];
 
-  const [step1Data, setStep1Data] = useState(DEFAULT_STEP_1_DATA);
-  const [step2Data, setStep2Data] = useState(DEFAULT_STEP_2_DATA);
+  const [advancedMatchingRules, setAdvancedMatchingRules] = useState<RouteModule.MatchingRule[]>(
+    [],
+  );
+  const [upstreamHeaderList, setUpstreamHeaderList] = useState<RouteModule.UpstreamHeader[]>([]);
   const [step3Data, setStep3Data] = useState(DEFAULT_STEP_3_DATA);
-
   const [redirect, setRedirect] = useState(false);
 
   const [form1] = Form.useForm();
   const [form2] = Form.useForm();
 
   const [step, setStep] = useState(1);
   const [stepHeader, setStepHeader] = useState(STEP_HEADER_4);
-
   const [chart, setChart] = useState(INIT_CHART);
 
-  const routeData = {
-    step1Data,
-    step2Data,
-    step3Data,
-  };
   const setupRoute = (rid: number) =>
     fetchItem(rid).then((data) => {
-      form1.setFieldsValue(data.step1Data);
-      setStep1Data(data.step1Data as RouteModule.Step1Data);
-
-      form2.setFieldsValue(data.step2Data);
-      setStep2Data(data.step2Data);
-
+      form1.setFieldsValue(data.form1Data);
+      setAdvancedMatchingRules(data.advancedMatchingRules);
+      form2.setFieldsValue(data.form2Data);
+      setUpstreamHeaderList(data.upstreamHeaderList);
       setStep3Data(data.step3Data);
     });
 
-  useEffect(() => {
-    if (props.route.path.indexOf('edit') !== -1) {
-      setupRoute(props.match.params.rid);
-    }
-  }, []);
-
-  useEffect(() => {
-    const { redirectOption } = step1Data;
-
-    if (redirectOption === 'customRedirect') {
-      setRedirect(true);
-      setStepHeader(STEP_HEADER_2);
-      return;
-    }
-    setRedirect(false);
-    setStepHeader(STEP_HEADER_4);
-  }, [step1Data]);
-
   const onReset = () => {
-    setStep1Data(DEFAULT_STEP_1_DATA);
-    setStep2Data(DEFAULT_STEP_2_DATA);
+    setAdvancedMatchingRules([]);
+    setUpstreamHeaderList([]);
     setStep3Data(DEFAULT_STEP_3_DATA);
-
     form1.setFieldsValue(DEFAULT_STEP_1_DATA);
     form2.setFieldsValue(DEFAULT_STEP_2_DATA);
     setStep(1);
   };
 
+  useEffect(() => {
+    if (props.route.path.indexOf('edit') !== -1) {
+      setupRoute(props.match.params.rid);
+    } else {
+      onReset();

Review comment:
       > Why we should reset all fields when creating a route? the initial data should be empty, right?
   
   `onReset()` is here to initialize the page data.

##########
File path: src/pages/Route/transform.ts
##########
@@ -18,49 +18,53 @@
 import { omit, pick } from 'lodash';
 
 export const transformStepData = ({
-  data: { step1Data, step2Data, step3Data },
-}: Pick<RouteModule.Data, 'data'>) => {
+  form1Data,
+  form2Data,
+  advancedMatchingRules,
+  upstreamHeaderList,
+  step3Data,
+}: RouteModule.RequestData) => {
   const nodes = {};
-  step2Data.upstreamHostList.forEach((node) => {
+  (form2Data.upstreamHostList || []).forEach((node) => {
     nodes[`${node.host}:${node.port}`] = node.weight;
   });
 
   const upstream_header = {};
-  (step2Data.upstreamHeaderList || []).forEach((header) => {
+  (upstreamHeaderList || []).forEach((header) => {

Review comment:
       Yep




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-dashboard] LiteSun commented on a change in pull request #483: feat: improve route

Posted by GitBox <gi...@apache.org>.
LiteSun commented on a change in pull request #483:
URL: https://github.com/apache/apisix-dashboard/pull/483#discussion_r491781970



##########
File path: src/pages/Route/transform.ts
##########
@@ -76,34 +80,33 @@ export const transformStepData = ({
       return [key, operator, value];
     }),
     upstream: {
-      type: step2Data.type,
+      type: form2Data.type,
       ...chashData,
       nodes,
-      timeout: step2Data.timeout,
+      timeout: form2Data.timeout,
     },
     upstream_header,
   };
 
-  if (step2Data.upstreamPath) {
+  if (form2Data.upstreamPath) {
     data.upstream_path = {
-      to: step2Data.upstreamPath,
+      to: form2Data.upstreamPath,
     };
-    if (step2Data.mappingStrategy) {
+    if (form2Data.mappingStrategy) {
       data.upstream_path = {
         ...data.upstream_path,
-        from: step2Data.mappingStrategy,
+        from: form2Data.mappingStrategy,
         type: 'regx',
       };
     }
   }
 
-  if (step3Data.plugins.prometheus) {
-    // eslint-disable-next-line no-param-reassign
-    step3Data.plugins.prometheus = {};
-  }
-
   // 未启用 redirect
   if (!redirect.uri) {
+    if (step3Data.plugins.prometheus) {
+      // eslint-disable-next-line no-param-reassign
+      step3Data.plugins.prometheus = {};

Review comment:
       ok




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-dashboard] LiteSun commented on a change in pull request #483: feat: improve route

Posted by GitBox <gi...@apache.org>.
LiteSun commented on a change in pull request #483:
URL: https://github.com/apache/apisix-dashboard/pull/483#discussion_r491783103



##########
File path: src/pages/Route/Create.tsx
##########
@@ -68,80 +60,63 @@ const Page: React.FC<Props> = (props) => {
     formatMessage({ id: 'route.constants.preview' }),
   ];
 
-  const [step1Data, setStep1Data] = useState(DEFAULT_STEP_1_DATA);
-  const [step2Data, setStep2Data] = useState(DEFAULT_STEP_2_DATA);
+  const [advancedMatchingRules, setAdvancedMatchingRules] = useState<RouteModule.MatchingRule[]>(
+    [],
+  );
+  const [upstreamHeaderList, setUpstreamHeaderList] = useState<RouteModule.UpstreamHeader[]>([]);
   const [step3Data, setStep3Data] = useState(DEFAULT_STEP_3_DATA);
-
   const [redirect, setRedirect] = useState(false);
 
   const [form1] = Form.useForm();
   const [form2] = Form.useForm();
 
   const [step, setStep] = useState(1);
   const [stepHeader, setStepHeader] = useState(STEP_HEADER_4);
-
   const [chart, setChart] = useState(INIT_CHART);
 
-  const routeData = {
-    step1Data,
-    step2Data,
-    step3Data,
-  };
   const setupRoute = (rid: number) =>
     fetchItem(rid).then((data) => {
-      form1.setFieldsValue(data.step1Data);
-      setStep1Data(data.step1Data as RouteModule.Step1Data);
-
-      form2.setFieldsValue(data.step2Data);
-      setStep2Data(data.step2Data);
-
+      form1.setFieldsValue(data.form1Data);
+      setAdvancedMatchingRules(data.advancedMatchingRules);
+      form2.setFieldsValue(data.form2Data);
+      setUpstreamHeaderList(data.upstreamHeaderList);
       setStep3Data(data.step3Data);
     });
 
-  useEffect(() => {
-    if (props.route.path.indexOf('edit') !== -1) {
-      setupRoute(props.match.params.rid);
-    }
-  }, []);
-
-  useEffect(() => {
-    const { redirectOption } = step1Data;
-
-    if (redirectOption === 'customRedirect') {
-      setRedirect(true);
-      setStepHeader(STEP_HEADER_2);
-      return;
-    }
-    setRedirect(false);
-    setStepHeader(STEP_HEADER_4);
-  }, [step1Data]);
-
   const onReset = () => {
-    setStep1Data(DEFAULT_STEP_1_DATA);
-    setStep2Data(DEFAULT_STEP_2_DATA);
+    setAdvancedMatchingRules([]);
+    setUpstreamHeaderList([]);
     setStep3Data(DEFAULT_STEP_3_DATA);
-
     form1.setFieldsValue(DEFAULT_STEP_1_DATA);
     form2.setFieldsValue(DEFAULT_STEP_2_DATA);
     setStep(1);
   };
 
+  useEffect(() => {
+    if (props.route.path.indexOf('edit') !== -1) {
+      setupRoute(props.match.params.rid);
+    } else {
+      onReset();

Review comment:
       > Why we should reset all fields when creating a route? the initial data should be empty, right?
   
   `onReset()` is here to initialize the page data.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-dashboard] LiteSun commented on a change in pull request #483: feat: improve route

Posted by GitBox <gi...@apache.org>.
LiteSun commented on a change in pull request #483:
URL: https://github.com/apache/apisix-dashboard/pull/483#discussion_r491783280



##########
File path: src/pages/Route/transform.ts
##########
@@ -18,49 +18,53 @@
 import { omit, pick } from 'lodash';
 
 export const transformStepData = ({
-  data: { step1Data, step2Data, step3Data },
-}: Pick<RouteModule.Data, 'data'>) => {
+  form1Data,
+  form2Data,
+  advancedMatchingRules,
+  upstreamHeaderList,
+  step3Data,
+}: RouteModule.RequestData) => {
   const nodes = {};
-  step2Data.upstreamHostList.forEach((node) => {
+  (form2Data.upstreamHostList || []).forEach((node) => {
     nodes[`${node.host}:${node.port}`] = node.weight;
   });
 
   const upstream_header = {};
-  (step2Data.upstreamHeaderList || []).forEach((header) => {
+  (upstreamHeaderList || []).forEach((header) => {

Review comment:
       Yep




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-dashboard] juzhiyuan commented on a change in pull request #483: feat: improve route

Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on a change in pull request #483:
URL: https://github.com/apache/apisix-dashboard/pull/483#discussion_r491763007



##########
File path: src/pages/Route/Create.tsx
##########
@@ -68,80 +60,63 @@ const Page: React.FC<Props> = (props) => {
     formatMessage({ id: 'route.constants.preview' }),
   ];
 
-  const [step1Data, setStep1Data] = useState(DEFAULT_STEP_1_DATA);
-  const [step2Data, setStep2Data] = useState(DEFAULT_STEP_2_DATA);
+  const [advancedMatchingRules, setAdvancedMatchingRules] = useState<RouteModule.MatchingRule[]>(
+    [],
+  );
+  const [upstreamHeaderList, setUpstreamHeaderList] = useState<RouteModule.UpstreamHeader[]>([]);
   const [step3Data, setStep3Data] = useState(DEFAULT_STEP_3_DATA);
-
   const [redirect, setRedirect] = useState(false);
 
   const [form1] = Form.useForm();
   const [form2] = Form.useForm();
 
   const [step, setStep] = useState(1);
   const [stepHeader, setStepHeader] = useState(STEP_HEADER_4);
-
   const [chart, setChart] = useState(INIT_CHART);
 
-  const routeData = {
-    step1Data,
-    step2Data,
-    step3Data,
-  };
   const setupRoute = (rid: number) =>
     fetchItem(rid).then((data) => {
-      form1.setFieldsValue(data.step1Data);
-      setStep1Data(data.step1Data as RouteModule.Step1Data);
-
-      form2.setFieldsValue(data.step2Data);
-      setStep2Data(data.step2Data);
-
+      form1.setFieldsValue(data.form1Data);
+      setAdvancedMatchingRules(data.advancedMatchingRules);
+      form2.setFieldsValue(data.form2Data);
+      setUpstreamHeaderList(data.upstreamHeaderList);
       setStep3Data(data.step3Data);
     });
 
-  useEffect(() => {
-    if (props.route.path.indexOf('edit') !== -1) {
-      setupRoute(props.match.params.rid);
-    }
-  }, []);
-
-  useEffect(() => {
-    const { redirectOption } = step1Data;
-
-    if (redirectOption === 'customRedirect') {
-      setRedirect(true);
-      setStepHeader(STEP_HEADER_2);
-      return;
-    }
-    setRedirect(false);
-    setStepHeader(STEP_HEADER_4);
-  }, [step1Data]);
-
   const onReset = () => {
-    setStep1Data(DEFAULT_STEP_1_DATA);
-    setStep2Data(DEFAULT_STEP_2_DATA);
+    setAdvancedMatchingRules([]);
+    setUpstreamHeaderList([]);
     setStep3Data(DEFAULT_STEP_3_DATA);
-
     form1.setFieldsValue(DEFAULT_STEP_1_DATA);
     form2.setFieldsValue(DEFAULT_STEP_2_DATA);
     setStep(1);
   };
 
+  useEffect(() => {
+    if (props.route.path.indexOf('edit') !== -1) {
+      setupRoute(props.match.params.rid);
+    } else {
+      onReset();

Review comment:
       Why we should reset all fields when creating a route? the initial data should be empty, right?

##########
File path: src/pages/Route/components/CreateStep4/CreateStep4.tsx
##########
@@ -44,17 +47,18 @@ const CreateStep4: React.FC<Props> = ({ form1, form2, redirect, ...rest }) => {
       {!redirect && (
         <>
           <h2 style={style}>{formatMessage({ id: 'route.create.define.api.backend.server' })}</h2>
-          <Step2 {...rest} form={form2} disabled />
+          <Step2
+            upstreamHeaderList={rest.upstreamHeaderList}
+            form={form2}
+            disabled
+            onChange={() => {}}
+          />
           <h2 style={style}>{formatMessage({ id: 'route.create.plugin.configuration' })}</h2>
           {Boolean(Object.keys(plugins).length !== 0) && (
-            <PluginPage initialData={rest.data.step3Data.plugins} readonly />
+            <PluginPage initialData={rest.step3Data.plugins} readonly />
           )}
           {Boolean(Object.keys(script).length !== 0) && (
-            <PluginOrchestration
-              data={rest.data.step3Data.script.chart}
-              readonly
-              onChange={() => {}}
-            />
+            <PluginOrchestration data={rest.step3Data.script.chart} readonly onChange={() => {}} />

Review comment:
       ,

##########
File path: src/pages/Route/Create.tsx
##########
@@ -68,80 +60,63 @@ const Page: React.FC<Props> = (props) => {
     formatMessage({ id: 'route.constants.preview' }),
   ];
 
-  const [step1Data, setStep1Data] = useState(DEFAULT_STEP_1_DATA);
-  const [step2Data, setStep2Data] = useState(DEFAULT_STEP_2_DATA);
+  const [advancedMatchingRules, setAdvancedMatchingRules] = useState<RouteModule.MatchingRule[]>(
+    [],
+  );
+  const [upstreamHeaderList, setUpstreamHeaderList] = useState<RouteModule.UpstreamHeader[]>([]);
   const [step3Data, setStep3Data] = useState(DEFAULT_STEP_3_DATA);
-
   const [redirect, setRedirect] = useState(false);
 
   const [form1] = Form.useForm();
   const [form2] = Form.useForm();
 
   const [step, setStep] = useState(1);
   const [stepHeader, setStepHeader] = useState(STEP_HEADER_4);
-
   const [chart, setChart] = useState(INIT_CHART);
 
-  const routeData = {
-    step1Data,
-    step2Data,
-    step3Data,
-  };
   const setupRoute = (rid: number) =>
     fetchItem(rid).then((data) => {
-      form1.setFieldsValue(data.step1Data);
-      setStep1Data(data.step1Data as RouteModule.Step1Data);
-
-      form2.setFieldsValue(data.step2Data);
-      setStep2Data(data.step2Data);
-
+      form1.setFieldsValue(data.form1Data);
+      setAdvancedMatchingRules(data.advancedMatchingRules);
+      form2.setFieldsValue(data.form2Data);
+      setUpstreamHeaderList(data.upstreamHeaderList);
       setStep3Data(data.step3Data);
     });
 
-  useEffect(() => {
-    if (props.route.path.indexOf('edit') !== -1) {
-      setupRoute(props.match.params.rid);
-    }
-  }, []);
-
-  useEffect(() => {
-    const { redirectOption } = step1Data;
-
-    if (redirectOption === 'customRedirect') {
-      setRedirect(true);
-      setStepHeader(STEP_HEADER_2);
-      return;
-    }
-    setRedirect(false);
-    setStepHeader(STEP_HEADER_4);
-  }, [step1Data]);
-
   const onReset = () => {
-    setStep1Data(DEFAULT_STEP_1_DATA);
-    setStep2Data(DEFAULT_STEP_2_DATA);
+    setAdvancedMatchingRules([]);
+    setUpstreamHeaderList([]);
     setStep3Data(DEFAULT_STEP_3_DATA);
-
     form1.setFieldsValue(DEFAULT_STEP_1_DATA);
     form2.setFieldsValue(DEFAULT_STEP_2_DATA);
     setStep(1);
   };
 
+  useEffect(() => {
+    if (props.route.path.indexOf('edit') !== -1) {
+      setupRoute(props.match.params.rid);
+    } else {
+      onReset();
+    }
+  }, []);
+
   const renderStep = () => {
     if (step === 1) {
       return (
         <Step1
-          data={routeData}
           form={form1}
-          onChange={(params: RouteModule.Step1Data) => {
-            if (params.route_group_id) {
-              fetchRouteGroupItem(params.route_group_id).then((data) => {
-                form1.setFieldsValue({
-                  ...form1.getFieldsValue(),
-                  ...data,
-                });
-              });
+          advancedMatchingRules={advancedMatchingRules}
+          onChange={({ action, data }) => {
+            if (action === 'redirectOptionChange' && data === 'customRedirect') {

Review comment:
       We may maintain a logic flow about Route.

##########
File path: src/pages/Route/components/CreateStep4/CreateStep4.tsx
##########
@@ -23,19 +23,22 @@ import PluginOrchestration from '@api7-dashboard/pluginchart';
 import Step1 from '../Step1';
 import Step2 from '../Step2';
 
-interface Props extends RouteModule.Data {
+type Props = {
   form1: FormInstance;
   form2: FormInstance;
   redirect?: boolean;
-}
+  step3Data: RouteModule.Step3Data;
+  advancedMatchingRules: RouteModule.MatchingRule[];
+  upstreamHeaderList: RouteModule.UpstreamHeader[];

Review comment:
       we'd better have a unified style on List variables, e.g advancedMatchingRuleList or upstramHeaders, I would prefer the former one.

##########
File path: src/pages/Route/components/Step1/MatchingRulesView.tsx
##########
@@ -19,11 +19,11 @@ import { Button, Table, Modal, Form, Select, Input, Space } from 'antd';
 import { useIntl } from 'umi';
 import { PanelSection } from '@api7-dashboard/ui';
 
-interface Props extends RouteModule.Data {}
-
-const MatchingRulesView: React.FC<Props> = ({ data, disabled, onChange }) => {
-  const { advancedMatchingRules } = data.step1Data;
-
+const MatchingRulesView: React.FC<RouteModule.Step1PassProps> = ({

Review comment:
       Pass?

##########
File path: src/pages/Route/components/Step1/MetaView.tsx
##########
@@ -63,9 +60,20 @@ const MetaView: React.FC<Props> = ({ data, disabled, onChange, isEdit }) => {
       <Form.Item label={formatMessage({ id: 'route.meta.api.group.name' })} name="route_group_id">
         <Select
           onChange={(value) => {
-            if (step1Data.route_group_id) {
-              onChange({ route_group_id: value });
+            if (!value) {
+              form.setFieldsValue({
+                ...form.getFieldsValue(),
+                route_group_name: '',
+              });
+              return;
             }
+            fetchRouteGroupItem(value.toString()).then((data) => {

Review comment:
       What's this value's typing?

##########
File path: src/pages/Route/components/CreateStep4/CreateStep4.tsx
##########
@@ -44,17 +47,18 @@ const CreateStep4: React.FC<Props> = ({ form1, form2, redirect, ...rest }) => {
       {!redirect && (
         <>
           <h2 style={style}>{formatMessage({ id: 'route.create.define.api.backend.server' })}</h2>
-          <Step2 {...rest} form={form2} disabled />
+          <Step2
+            upstreamHeaderList={rest.upstreamHeaderList}
+            form={form2}
+            disabled
+            onChange={() => {}}

Review comment:
       ,

##########
File path: src/pages/Route/transform.ts
##########
@@ -76,34 +80,33 @@ export const transformStepData = ({
       return [key, operator, value];
     }),
     upstream: {
-      type: step2Data.type,
+      type: form2Data.type,
       ...chashData,
       nodes,
-      timeout: step2Data.timeout,
+      timeout: form2Data.timeout,
     },
     upstream_header,
   };
 
-  if (step2Data.upstreamPath) {
+  if (form2Data.upstreamPath) {
     data.upstream_path = {
-      to: step2Data.upstreamPath,
+      to: form2Data.upstreamPath,
     };
-    if (step2Data.mappingStrategy) {
+    if (form2Data.mappingStrategy) {
       data.upstream_path = {
         ...data.upstream_path,
-        from: step2Data.mappingStrategy,
+        from: form2Data.mappingStrategy,
         type: 'regx',
       };
     }
   }
 
-  if (step3Data.plugins.prometheus) {
-    // eslint-disable-next-line no-param-reassign
-    step3Data.plugins.prometheus = {};
-  }
-
   // 未启用 redirect
   if (!redirect.uri) {
+    if (step3Data.plugins.prometheus) {
+      // eslint-disable-next-line no-param-reassign
+      step3Data.plugins.prometheus = {};

Review comment:
       This logic is no longer needed because of the new PluginModule

##########
File path: src/pages/Route/components/Step2/RequestRewriteView.tsx
##########
@@ -29,15 +29,23 @@ import {
   HASH_ON_LIST,
 } from '@/pages/Route/constants';
 import styles from '../../Create.less';
-import { fetchUpstreamList } from '../../service';
+import { fetchUpstreamList, fetchUpstreamItem } from '../../service';
 
-interface Props extends RouteModule.Data {
-  form: FormInstance;
-}
-const RequestRewriteView: React.FC<Props> = ({ data, form, disabled, onChange }) => {
-  const { step2Data } = data;
+const RequestRewriteView: React.FC<RouteModule.Step2PassProps> = ({ form, disabled }) => {
   const [upstearms, setUpstreams] = useState<{ id: string; name: string }[]>();
-  const upstreamDisabled = disabled || !!step2Data.upstream_id;
+  const [upstreamId, setUpstreamId] = useState(form.getFieldValue('upstream_id'));
+  // TODO: need to check

Review comment:
       any solution here?

##########
File path: src/pages/Route/transform.ts
##########
@@ -18,49 +18,53 @@
 import { omit, pick } from 'lodash';
 
 export const transformStepData = ({
-  data: { step1Data, step2Data, step3Data },
-}: Pick<RouteModule.Data, 'data'>) => {
+  form1Data,
+  form2Data,
+  advancedMatchingRules,
+  upstreamHeaderList,
+  step3Data,
+}: RouteModule.RequestData) => {
   const nodes = {};
-  step2Data.upstreamHostList.forEach((node) => {
+  (form2Data.upstreamHostList || []).forEach((node) => {
     nodes[`${node.host}:${node.port}`] = node.weight;
   });
 
   const upstream_header = {};
-  (step2Data.upstreamHeaderList || []).forEach((header) => {
+  (upstreamHeaderList || []).forEach((header) => {

Review comment:
       could we set a default value for this list?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-dashboard] LiteSun commented on a change in pull request #483: feat: improve route

Posted by GitBox <gi...@apache.org>.
LiteSun commented on a change in pull request #483:
URL: https://github.com/apache/apisix-dashboard/pull/483#discussion_r491783103



##########
File path: src/pages/Route/Create.tsx
##########
@@ -68,80 +60,63 @@ const Page: React.FC<Props> = (props) => {
     formatMessage({ id: 'route.constants.preview' }),
   ];
 
-  const [step1Data, setStep1Data] = useState(DEFAULT_STEP_1_DATA);
-  const [step2Data, setStep2Data] = useState(DEFAULT_STEP_2_DATA);
+  const [advancedMatchingRules, setAdvancedMatchingRules] = useState<RouteModule.MatchingRule[]>(
+    [],
+  );
+  const [upstreamHeaderList, setUpstreamHeaderList] = useState<RouteModule.UpstreamHeader[]>([]);
   const [step3Data, setStep3Data] = useState(DEFAULT_STEP_3_DATA);
-
   const [redirect, setRedirect] = useState(false);
 
   const [form1] = Form.useForm();
   const [form2] = Form.useForm();
 
   const [step, setStep] = useState(1);
   const [stepHeader, setStepHeader] = useState(STEP_HEADER_4);
-
   const [chart, setChart] = useState(INIT_CHART);
 
-  const routeData = {
-    step1Data,
-    step2Data,
-    step3Data,
-  };
   const setupRoute = (rid: number) =>
     fetchItem(rid).then((data) => {
-      form1.setFieldsValue(data.step1Data);
-      setStep1Data(data.step1Data as RouteModule.Step1Data);
-
-      form2.setFieldsValue(data.step2Data);
-      setStep2Data(data.step2Data);
-
+      form1.setFieldsValue(data.form1Data);
+      setAdvancedMatchingRules(data.advancedMatchingRules);
+      form2.setFieldsValue(data.form2Data);
+      setUpstreamHeaderList(data.upstreamHeaderList);
       setStep3Data(data.step3Data);
     });
 
-  useEffect(() => {
-    if (props.route.path.indexOf('edit') !== -1) {
-      setupRoute(props.match.params.rid);
-    }
-  }, []);
-
-  useEffect(() => {
-    const { redirectOption } = step1Data;
-
-    if (redirectOption === 'customRedirect') {
-      setRedirect(true);
-      setStepHeader(STEP_HEADER_2);
-      return;
-    }
-    setRedirect(false);
-    setStepHeader(STEP_HEADER_4);
-  }, [step1Data]);
-
   const onReset = () => {
-    setStep1Data(DEFAULT_STEP_1_DATA);
-    setStep2Data(DEFAULT_STEP_2_DATA);
+    setAdvancedMatchingRules([]);
+    setUpstreamHeaderList([]);
     setStep3Data(DEFAULT_STEP_3_DATA);
-
     form1.setFieldsValue(DEFAULT_STEP_1_DATA);
     form2.setFieldsValue(DEFAULT_STEP_2_DATA);
     setStep(1);
   };
 
+  useEffect(() => {
+    if (props.route.path.indexOf('edit') !== -1) {
+      setupRoute(props.match.params.rid);
+    } else {
+      onReset();

Review comment:
       > Why we should reset all fields when creating a route? the initial data should be empty, right?
   `onReset()` is here to initialize the page data.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-dashboard] juzhiyuan commented on a change in pull request #483: feat: improve route

Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on a change in pull request #483:
URL: https://github.com/apache/apisix-dashboard/pull/483#discussion_r491763007



##########
File path: src/pages/Route/Create.tsx
##########
@@ -68,80 +60,63 @@ const Page: React.FC<Props> = (props) => {
     formatMessage({ id: 'route.constants.preview' }),
   ];
 
-  const [step1Data, setStep1Data] = useState(DEFAULT_STEP_1_DATA);
-  const [step2Data, setStep2Data] = useState(DEFAULT_STEP_2_DATA);
+  const [advancedMatchingRules, setAdvancedMatchingRules] = useState<RouteModule.MatchingRule[]>(
+    [],
+  );
+  const [upstreamHeaderList, setUpstreamHeaderList] = useState<RouteModule.UpstreamHeader[]>([]);
   const [step3Data, setStep3Data] = useState(DEFAULT_STEP_3_DATA);
-
   const [redirect, setRedirect] = useState(false);
 
   const [form1] = Form.useForm();
   const [form2] = Form.useForm();
 
   const [step, setStep] = useState(1);
   const [stepHeader, setStepHeader] = useState(STEP_HEADER_4);
-
   const [chart, setChart] = useState(INIT_CHART);
 
-  const routeData = {
-    step1Data,
-    step2Data,
-    step3Data,
-  };
   const setupRoute = (rid: number) =>
     fetchItem(rid).then((data) => {
-      form1.setFieldsValue(data.step1Data);
-      setStep1Data(data.step1Data as RouteModule.Step1Data);
-
-      form2.setFieldsValue(data.step2Data);
-      setStep2Data(data.step2Data);
-
+      form1.setFieldsValue(data.form1Data);
+      setAdvancedMatchingRules(data.advancedMatchingRules);
+      form2.setFieldsValue(data.form2Data);
+      setUpstreamHeaderList(data.upstreamHeaderList);
       setStep3Data(data.step3Data);
     });
 
-  useEffect(() => {
-    if (props.route.path.indexOf('edit') !== -1) {
-      setupRoute(props.match.params.rid);
-    }
-  }, []);
-
-  useEffect(() => {
-    const { redirectOption } = step1Data;
-
-    if (redirectOption === 'customRedirect') {
-      setRedirect(true);
-      setStepHeader(STEP_HEADER_2);
-      return;
-    }
-    setRedirect(false);
-    setStepHeader(STEP_HEADER_4);
-  }, [step1Data]);
-
   const onReset = () => {
-    setStep1Data(DEFAULT_STEP_1_DATA);
-    setStep2Data(DEFAULT_STEP_2_DATA);
+    setAdvancedMatchingRules([]);
+    setUpstreamHeaderList([]);
     setStep3Data(DEFAULT_STEP_3_DATA);
-
     form1.setFieldsValue(DEFAULT_STEP_1_DATA);
     form2.setFieldsValue(DEFAULT_STEP_2_DATA);
     setStep(1);
   };
 
+  useEffect(() => {
+    if (props.route.path.indexOf('edit') !== -1) {
+      setupRoute(props.match.params.rid);
+    } else {
+      onReset();

Review comment:
       Why we should reset all fields when creating a route? the initial data should be empty, right?

##########
File path: src/pages/Route/components/CreateStep4/CreateStep4.tsx
##########
@@ -44,17 +47,18 @@ const CreateStep4: React.FC<Props> = ({ form1, form2, redirect, ...rest }) => {
       {!redirect && (
         <>
           <h2 style={style}>{formatMessage({ id: 'route.create.define.api.backend.server' })}</h2>
-          <Step2 {...rest} form={form2} disabled />
+          <Step2
+            upstreamHeaderList={rest.upstreamHeaderList}
+            form={form2}
+            disabled
+            onChange={() => {}}
+          />
           <h2 style={style}>{formatMessage({ id: 'route.create.plugin.configuration' })}</h2>
           {Boolean(Object.keys(plugins).length !== 0) && (
-            <PluginPage initialData={rest.data.step3Data.plugins} readonly />
+            <PluginPage initialData={rest.step3Data.plugins} readonly />
           )}
           {Boolean(Object.keys(script).length !== 0) && (
-            <PluginOrchestration
-              data={rest.data.step3Data.script.chart}
-              readonly
-              onChange={() => {}}
-            />
+            <PluginOrchestration data={rest.step3Data.script.chart} readonly onChange={() => {}} />

Review comment:
       ,

##########
File path: src/pages/Route/Create.tsx
##########
@@ -68,80 +60,63 @@ const Page: React.FC<Props> = (props) => {
     formatMessage({ id: 'route.constants.preview' }),
   ];
 
-  const [step1Data, setStep1Data] = useState(DEFAULT_STEP_1_DATA);
-  const [step2Data, setStep2Data] = useState(DEFAULT_STEP_2_DATA);
+  const [advancedMatchingRules, setAdvancedMatchingRules] = useState<RouteModule.MatchingRule[]>(
+    [],
+  );
+  const [upstreamHeaderList, setUpstreamHeaderList] = useState<RouteModule.UpstreamHeader[]>([]);
   const [step3Data, setStep3Data] = useState(DEFAULT_STEP_3_DATA);
-
   const [redirect, setRedirect] = useState(false);
 
   const [form1] = Form.useForm();
   const [form2] = Form.useForm();
 
   const [step, setStep] = useState(1);
   const [stepHeader, setStepHeader] = useState(STEP_HEADER_4);
-
   const [chart, setChart] = useState(INIT_CHART);
 
-  const routeData = {
-    step1Data,
-    step2Data,
-    step3Data,
-  };
   const setupRoute = (rid: number) =>
     fetchItem(rid).then((data) => {
-      form1.setFieldsValue(data.step1Data);
-      setStep1Data(data.step1Data as RouteModule.Step1Data);
-
-      form2.setFieldsValue(data.step2Data);
-      setStep2Data(data.step2Data);
-
+      form1.setFieldsValue(data.form1Data);
+      setAdvancedMatchingRules(data.advancedMatchingRules);
+      form2.setFieldsValue(data.form2Data);
+      setUpstreamHeaderList(data.upstreamHeaderList);
       setStep3Data(data.step3Data);
     });
 
-  useEffect(() => {
-    if (props.route.path.indexOf('edit') !== -1) {
-      setupRoute(props.match.params.rid);
-    }
-  }, []);
-
-  useEffect(() => {
-    const { redirectOption } = step1Data;
-
-    if (redirectOption === 'customRedirect') {
-      setRedirect(true);
-      setStepHeader(STEP_HEADER_2);
-      return;
-    }
-    setRedirect(false);
-    setStepHeader(STEP_HEADER_4);
-  }, [step1Data]);
-
   const onReset = () => {
-    setStep1Data(DEFAULT_STEP_1_DATA);
-    setStep2Data(DEFAULT_STEP_2_DATA);
+    setAdvancedMatchingRules([]);
+    setUpstreamHeaderList([]);
     setStep3Data(DEFAULT_STEP_3_DATA);
-
     form1.setFieldsValue(DEFAULT_STEP_1_DATA);
     form2.setFieldsValue(DEFAULT_STEP_2_DATA);
     setStep(1);
   };
 
+  useEffect(() => {
+    if (props.route.path.indexOf('edit') !== -1) {
+      setupRoute(props.match.params.rid);
+    } else {
+      onReset();
+    }
+  }, []);
+
   const renderStep = () => {
     if (step === 1) {
       return (
         <Step1
-          data={routeData}
           form={form1}
-          onChange={(params: RouteModule.Step1Data) => {
-            if (params.route_group_id) {
-              fetchRouteGroupItem(params.route_group_id).then((data) => {
-                form1.setFieldsValue({
-                  ...form1.getFieldsValue(),
-                  ...data,
-                });
-              });
+          advancedMatchingRules={advancedMatchingRules}
+          onChange={({ action, data }) => {
+            if (action === 'redirectOptionChange' && data === 'customRedirect') {

Review comment:
       We may maintain a logic flow about Route.

##########
File path: src/pages/Route/components/CreateStep4/CreateStep4.tsx
##########
@@ -23,19 +23,22 @@ import PluginOrchestration from '@api7-dashboard/pluginchart';
 import Step1 from '../Step1';
 import Step2 from '../Step2';
 
-interface Props extends RouteModule.Data {
+type Props = {
   form1: FormInstance;
   form2: FormInstance;
   redirect?: boolean;
-}
+  step3Data: RouteModule.Step3Data;
+  advancedMatchingRules: RouteModule.MatchingRule[];
+  upstreamHeaderList: RouteModule.UpstreamHeader[];

Review comment:
       we'd better have a unified style on List variables, e.g advancedMatchingRuleList or upstramHeaders, I would prefer the former one.

##########
File path: src/pages/Route/components/Step1/MatchingRulesView.tsx
##########
@@ -19,11 +19,11 @@ import { Button, Table, Modal, Form, Select, Input, Space } from 'antd';
 import { useIntl } from 'umi';
 import { PanelSection } from '@api7-dashboard/ui';
 
-interface Props extends RouteModule.Data {}
-
-const MatchingRulesView: React.FC<Props> = ({ data, disabled, onChange }) => {
-  const { advancedMatchingRules } = data.step1Data;
-
+const MatchingRulesView: React.FC<RouteModule.Step1PassProps> = ({

Review comment:
       Pass?

##########
File path: src/pages/Route/components/Step1/MetaView.tsx
##########
@@ -63,9 +60,20 @@ const MetaView: React.FC<Props> = ({ data, disabled, onChange, isEdit }) => {
       <Form.Item label={formatMessage({ id: 'route.meta.api.group.name' })} name="route_group_id">
         <Select
           onChange={(value) => {
-            if (step1Data.route_group_id) {
-              onChange({ route_group_id: value });
+            if (!value) {
+              form.setFieldsValue({
+                ...form.getFieldsValue(),
+                route_group_name: '',
+              });
+              return;
             }
+            fetchRouteGroupItem(value.toString()).then((data) => {

Review comment:
       What's this value's typing?

##########
File path: src/pages/Route/components/CreateStep4/CreateStep4.tsx
##########
@@ -44,17 +47,18 @@ const CreateStep4: React.FC<Props> = ({ form1, form2, redirect, ...rest }) => {
       {!redirect && (
         <>
           <h2 style={style}>{formatMessage({ id: 'route.create.define.api.backend.server' })}</h2>
-          <Step2 {...rest} form={form2} disabled />
+          <Step2
+            upstreamHeaderList={rest.upstreamHeaderList}
+            form={form2}
+            disabled
+            onChange={() => {}}

Review comment:
       ,

##########
File path: src/pages/Route/transform.ts
##########
@@ -76,34 +80,33 @@ export const transformStepData = ({
       return [key, operator, value];
     }),
     upstream: {
-      type: step2Data.type,
+      type: form2Data.type,
       ...chashData,
       nodes,
-      timeout: step2Data.timeout,
+      timeout: form2Data.timeout,
     },
     upstream_header,
   };
 
-  if (step2Data.upstreamPath) {
+  if (form2Data.upstreamPath) {
     data.upstream_path = {
-      to: step2Data.upstreamPath,
+      to: form2Data.upstreamPath,
     };
-    if (step2Data.mappingStrategy) {
+    if (form2Data.mappingStrategy) {
       data.upstream_path = {
         ...data.upstream_path,
-        from: step2Data.mappingStrategy,
+        from: form2Data.mappingStrategy,
         type: 'regx',
       };
     }
   }
 
-  if (step3Data.plugins.prometheus) {
-    // eslint-disable-next-line no-param-reassign
-    step3Data.plugins.prometheus = {};
-  }
-
   // 未启用 redirect
   if (!redirect.uri) {
+    if (step3Data.plugins.prometheus) {
+      // eslint-disable-next-line no-param-reassign
+      step3Data.plugins.prometheus = {};

Review comment:
       This logic is no longer needed because of the new PluginModule

##########
File path: src/pages/Route/components/Step2/RequestRewriteView.tsx
##########
@@ -29,15 +29,23 @@ import {
   HASH_ON_LIST,
 } from '@/pages/Route/constants';
 import styles from '../../Create.less';
-import { fetchUpstreamList } from '../../service';
+import { fetchUpstreamList, fetchUpstreamItem } from '../../service';
 
-interface Props extends RouteModule.Data {
-  form: FormInstance;
-}
-const RequestRewriteView: React.FC<Props> = ({ data, form, disabled, onChange }) => {
-  const { step2Data } = data;
+const RequestRewriteView: React.FC<RouteModule.Step2PassProps> = ({ form, disabled }) => {
   const [upstearms, setUpstreams] = useState<{ id: string; name: string }[]>();
-  const upstreamDisabled = disabled || !!step2Data.upstream_id;
+  const [upstreamId, setUpstreamId] = useState(form.getFieldValue('upstream_id'));
+  // TODO: need to check

Review comment:
       any solution here?

##########
File path: src/pages/Route/transform.ts
##########
@@ -18,49 +18,53 @@
 import { omit, pick } from 'lodash';
 
 export const transformStepData = ({
-  data: { step1Data, step2Data, step3Data },
-}: Pick<RouteModule.Data, 'data'>) => {
+  form1Data,
+  form2Data,
+  advancedMatchingRules,
+  upstreamHeaderList,
+  step3Data,
+}: RouteModule.RequestData) => {
   const nodes = {};
-  step2Data.upstreamHostList.forEach((node) => {
+  (form2Data.upstreamHostList || []).forEach((node) => {
     nodes[`${node.host}:${node.port}`] = node.weight;
   });
 
   const upstream_header = {};
-  (step2Data.upstreamHeaderList || []).forEach((header) => {
+  (upstreamHeaderList || []).forEach((header) => {

Review comment:
       could we set a default value for this list?

##########
File path: src/pages/Route/Create.tsx
##########
@@ -68,80 +60,63 @@ const Page: React.FC<Props> = (props) => {
     formatMessage({ id: 'route.constants.preview' }),
   ];
 
-  const [step1Data, setStep1Data] = useState(DEFAULT_STEP_1_DATA);
-  const [step2Data, setStep2Data] = useState(DEFAULT_STEP_2_DATA);
+  const [advancedMatchingRules, setAdvancedMatchingRules] = useState<RouteModule.MatchingRule[]>(
+    [],
+  );
+  const [upstreamHeaderList, setUpstreamHeaderList] = useState<RouteModule.UpstreamHeader[]>([]);
   const [step3Data, setStep3Data] = useState(DEFAULT_STEP_3_DATA);
-
   const [redirect, setRedirect] = useState(false);
 
   const [form1] = Form.useForm();
   const [form2] = Form.useForm();
 
   const [step, setStep] = useState(1);
   const [stepHeader, setStepHeader] = useState(STEP_HEADER_4);
-
   const [chart, setChart] = useState(INIT_CHART);
 
-  const routeData = {
-    step1Data,
-    step2Data,
-    step3Data,
-  };
   const setupRoute = (rid: number) =>
     fetchItem(rid).then((data) => {
-      form1.setFieldsValue(data.step1Data);
-      setStep1Data(data.step1Data as RouteModule.Step1Data);
-
-      form2.setFieldsValue(data.step2Data);
-      setStep2Data(data.step2Data);
-
+      form1.setFieldsValue(data.form1Data);
+      setAdvancedMatchingRules(data.advancedMatchingRules);
+      form2.setFieldsValue(data.form2Data);
+      setUpstreamHeaderList(data.upstreamHeaderList);
       setStep3Data(data.step3Data);
     });
 
-  useEffect(() => {
-    if (props.route.path.indexOf('edit') !== -1) {
-      setupRoute(props.match.params.rid);
-    }
-  }, []);
-
-  useEffect(() => {
-    const { redirectOption } = step1Data;
-
-    if (redirectOption === 'customRedirect') {
-      setRedirect(true);
-      setStepHeader(STEP_HEADER_2);
-      return;
-    }
-    setRedirect(false);
-    setStepHeader(STEP_HEADER_4);
-  }, [step1Data]);
-
   const onReset = () => {
-    setStep1Data(DEFAULT_STEP_1_DATA);
-    setStep2Data(DEFAULT_STEP_2_DATA);
+    setAdvancedMatchingRules([]);
+    setUpstreamHeaderList([]);
     setStep3Data(DEFAULT_STEP_3_DATA);
-
     form1.setFieldsValue(DEFAULT_STEP_1_DATA);
     form2.setFieldsValue(DEFAULT_STEP_2_DATA);
     setStep(1);
   };
 
+  useEffect(() => {
+    if (props.route.path.indexOf('edit') !== -1) {
+      setupRoute(props.match.params.rid);
+    } else {
+      onReset();

Review comment:
       setupXX would be better than reset




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-dashboard] juzhiyuan merged pull request #483: feat: improve route

Posted by GitBox <gi...@apache.org>.
juzhiyuan merged pull request #483:
URL: https://github.com/apache/apisix-dashboard/pull/483


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org