Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: limit-* plugin UI form support reject-message field #2097

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

iamayushdas
Copy link
Contributor

Please answer these questions before submitting a pull request, or your PR will get closed.

Why submit this pull request?

  • Bugfix
  • New feature provided
  • Improve performance
  • Backport patches

What changes will this PR take into?

Please update this section with detailed description.

Related issues

fix/resolve #2094

Checklist:

  • Did you explain what problem does this PR solve? Or what new features have been added?
  • Have you added corresponding test cases?
  • Have you modified the corresponding document?
  • Is this PR backward compatible? If it is not backward compatible, please discuss on the mailing list first

@netlify
Copy link

netlify bot commented Aug 26, 2021

✔️ Deploy Preview for apisix-dashboard ready!

🔨 Explore the source changes: ed85d12

🔍 Inspect the deploy log: https://app.netlify.com/sites/apisix-dashboard/deploys/6138381f615ca8000808de69

😎 Browse the preview: https://deploy-preview-2097--apisix-dashboard.netlify.app/

@iamayushdas
Copy link
Contributor Author

cc @liuxiran @juzhiyuan

@codecov-commenter
Copy link

codecov-commenter commented Aug 26, 2021

Codecov Report

Merging #2097 (ed85d12) into master (98fe38a) will increase coverage by 4.73%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2097      +/-   ##
==========================================
+ Coverage   66.12%   70.86%   +4.73%     
==========================================
  Files         186       60     -126     
  Lines        7077     3783    -3294     
  Branches      802        0     -802     
==========================================
- Hits         4680     2681    -1999     
+ Misses       2111      816    -1295     
  Partials      286      286              
Flag Coverage Δ
backend-e2e-test 47.23% <ø> (+0.02%) ⬆️
backend-e2e-test-ginkgo 48.79% <ø> (?)
backend-unit-test 52.70% <ø> (-0.04%) ⬇️
frontend-e2e-test ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
web/src/pages/PluginTemplate/components/Step1.tsx
...eam/components/passive-check/Healthy/Successes.tsx
web/src/pages/Route/service.ts
web/src/libs/iconfont.js
web/src/components/ActionBar/ActionBar.tsx
...nents/Upstream/components/active-check/Timeout.tsx
...pages/Route/components/CreateStep4/CreateStep4.tsx
web/src/components/Upstream/service.ts
...components/PluginFlow/components/Toolbar/index.tsx
web/src/components/Upstream/components/Scheme.tsx
... and 129 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 98fe38a...ed85d12. Read the comment docs.

@liuxiran
Copy link
Contributor

Hi @iamayushdas , would you please add related test cases for this changes? something like we can file in reject-message field and submit the form ok. Thanks a lot

@iamayushdas
Copy link
Contributor Author

Sure

@liuxiran
Copy link
Contributor

liuxiran commented Sep 5, 2021

any updates? and please fix the conflict file when you have time @iamayushdas thanks

@iamayushdas
Copy link
Contributor Author

any updates? and please fix the conflict file when you have time @iamayushdas thanks

Its already been fixed by someone.

@iamayushdas
Copy link
Contributor Author

any updates? and please fix the conflict file when you have time @iamayushdas thanks

Its already been fixed by someone. reject_msg has been merged by someone

@liuxiran
Copy link
Contributor

liuxiran commented Sep 6, 2021

Hi @iamayushdas , since we have updated schema file uniformly, see: #2117

so you will see there are some conflicts in schema.json file.

and for the reject-message, it is still not added to the UI form, you can go on your pr, thanks a lot

@liuxiran
Copy link
Contributor

liuxiran commented Sep 8, 2021

Hi @iamayushdas , you can go on your work when you have time, thinks

@bzp2010
Copy link
Contributor

bzp2010 commented Sep 20, 2021

Hi, @iamayushdas.

Although the information about reject message has been added in schema.json, the frontend end can't be configured through UI forms, and the input boxes in JSON and YAML can't be automatically filled and prompted for types.

image

form

image

input

This still needs some modifications. Do you want to continue this PR?

@juzhiyuan
Copy link
Member

@iamayushdas Hi, do you have time to finish this PR?

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.

Feature request: limit-* plugin UI form support reject-message field
5 participants