Skip to content

Conversation

@LiteSun
Copy link
Member

@LiteSun LiteSun commented Sep 17, 2020

Please answer these questions before submitting a pull request

  • Why submit this pull request?

  • Bug fix

  • New feature provided

  • Improve performance

  • Related issues


Bugfix

  • Description

  • How to fix?


New feature or improvement

  • Describe the details and related test reports.

@LiteSun LiteSun marked this pull request as ready for review September 21, 2020 01:03
Copy link
Member

@juzhiyuan juzhiyuan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me from those codes style, but could not make sure if it's working actually, we need some test cases to cover all PRs.

}
step3Data: RouteModule.Step3Data;
advancedMatchingRules: RouteModule.MatchingRule[];
upstreamHeaderList: RouteModule.UpstreamHeader[];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

upstreamHeaderList={rest.upstreamHeaderList}
form={form2}
disabled
onChange={() => {}}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

,

readonly
onChange={() => {}}
/>
<PluginOrchestration data={rest.step3Data.script.chart} readonly onChange={() => {}} />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

,

const MatchingRulesView: React.FC<Props> = ({ data, disabled, onChange }) => {
const { advancedMatchingRules } = data.step1Data;

const MatchingRulesView: React.FC<RouteModule.Step1PassProps> = ({
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pass?

});
return;
}
fetchRouteGroupItem(value.toString()).then((data) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's this value's typing?

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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any solution here?

Copy link
Member

@juzhiyuan juzhiyuan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch

@juzhiyuan juzhiyuan requested a review from moonming September 21, 2020 09:02
@juzhiyuan juzhiyuan merged commit 234e1ea into apache:master Sep 21, 2020
LiteSun added a commit to LiteSun/incubator-apisix-dashboard that referenced this pull request Sep 21, 2020
* feat: improve step1

* feat: improve step2

* feat: improve createStep4

* feat: improve transform

* fix: event loop

* feat: clean code

* fix: lost route_group info when enable redirect

* feat: UI improve
@juzhiyuan juzhiyuan linked an issue Sep 22, 2020 that may be closed by this pull request
juzhiyuan added a commit that referenced this pull request Sep 22, 2020
* feat: preview pluginchart (#456)

* feat: added pluginchart preview in route step4

* feat: change PluginChart to PluginOrchestration

* add tools to check ASF headers in code files. (#454)

* chore: remove unnecessary license file (#462)

* remove unnecessary license file

* remove unnecessary license file for `api7 dashboard`

* Increase the checking of .conf and .toml files. And delete the redundant configuration of the ASF-Release.cfg file. (#461)

* Increase the checking of .conf and .toml files.

* Skip the check of dag-to-lua-1.1/README.md file

* Put the "run Makefile" check before "get lua lib".

* Put the "run Makefile" check before "get lua lib".

* Put the "run Makefile" check before "get lua lib".

Co-authored-by: lin <lin@MBPro.local>

* ci: Add lint (#455)

Co-authored-by: Rapiz <code@rapiz.me>

* Feat: dashboard support route group (#433)

* feat: route group UI

* feat(route-ui): add routegroup when create route

* fix: add routegroup to route list

* fix: support add routegroup together with route

* fix: route path define error

* feat: trigger redeploy

* fix: update i18n key

Co-authored-by: 琚致远 <juzhiyuan@apache.org>

* feature:Independent license check operation file. (#465)

* feat: support ungroup in manager api (#460)

* feat: support ungroup in manager api

* fix: synchronize route group to route

* fix: update test case

* feat: add publish status to route in manager api (#450)

* feat: add publish status to route in manager api

* fix: update publish and offline route

* feat: add route publish test module

* fix: synchronize content for route publish/offline

* feat(route): add publish and offline to route (#451)

* feat(route): add publish and offline to route

* feat: trigger redeploy

* feat: trigger redeploy

* Update Create.tsx

* fix: update refer to the code review

* fix: init status default value

* fix: add a help msg to status Form.Item

Co-authored-by: 琚致远 <juzhiyuan@apache.org>

* feat: remove netlify (#472)

* fix: host should not be required (#477)

* feat: added new plugin dependency (#475)

* feat: added new plugin dependency

* feat: omit shadow var

* feat: omit shadow var

* fix: host should not be required (#479)

* fix: host should not be required

* feat: remove required rule

* feat: Route debug (#485)

* feat: new api get route and apisix url

* feat: online debug

* fix: update refer to the review, fix logical error as well

* fix: rename getRouteWithApisixUrl

* feat: update plugin (#482)

* feat: improve route (#483)

* feat: improve step1

* feat: improve step2

* feat: improve createStep4

* feat: improve transform

* fix: event loop

* feat: clean code

* fix: lost route_group info when enable redirect

* feat: UI improve

* fix: checkHostWithSSL with empty hosts

* Update Create.tsx

Co-authored-by: Firstsawyou <52862365+Firstsawyou@users.noreply.github.com>
Co-authored-by: nic-chen <33000667+nic-chen@users.noreply.github.com>
Co-authored-by: lin <lin@MBPro.local>
Co-authored-by: Rapiz <contact@rapiz.me>
Co-authored-by: Rapiz <code@rapiz.me>
Co-authored-by: liuxiran <belovedxixi@126.com>
Co-authored-by: 琚致远 <juzhiyuan@apache.org>
@LiteSun LiteSun deleted the feat-improve-route branch October 28, 2020 03:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Proposal: Improve the Route module

3 participants