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/21 02:24:42 UTC
[GitHub] [apisix-dashboard] juzhiyuan commented on a change in pull request #483: feat: improve route
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