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: Adding batch deletion function for routing pages #2502

Merged
merged 87 commits into from
Aug 9, 2022

Conversation

FangSen9000
Copy link
Contributor

@FangSen9000 FangSen9000 commented Jul 8, 2022

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.

批量删除.webm
You can watch this video for a preview.

On the routing page, the user cannot delete in the state of select all or batch delete. I added related operations.
image

When selecting multiple or single items, a footer bar will pop up for batch deletion of table data. The deselect button will appear in the title of the table.

It also has full and anti - full pages
image

Related issues

#2565
#2562

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

@codecov-commenter
Copy link

codecov-commenter commented Jul 8, 2022

Codecov Report

Merging #2502 (4edbd58) into master (adcd2e3) will decrease coverage by 8.91%.
The diff coverage is 50.00%.

❗ Current head 4edbd58 differs from pull request most recent head 1a1a902. Consider uploading reports for the commit 1a1a902 to get more accurate results

@@            Coverage Diff             @@
##           master    #2502      +/-   ##
==========================================
- Coverage   75.92%   67.00%   -8.92%     
==========================================
  Files         136      133       -3     
  Lines        3576     3537      -39     
  Branches      864      850      -14     
==========================================
- Hits         2715     2370     -345     
- Misses        861     1167     +306     
Flag Coverage Δ
frontend-e2e-test 67.00% <50.00%> (-8.92%) ⬇️

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

Impacted Files Coverage Δ
web/src/pages/Route/List.tsx 58.49% <42.85%> (-15.86%) ⬇️
web/src/pages/Route/service.ts 78.78% <100.00%> (-3.04%) ⬇️
web/src/components/LabelsfDrawer/LabelsDrawer.tsx 4.16% <0.00%> (-79.17%) ⬇️
...b/src/pages/Route/components/DataLoader/Import.tsx 4.91% <0.00%> (-78.69%) ⬇️
...es/Route/components/DataLoader/loader/OpenAPI3.tsx 33.33% <0.00%> (-66.67%) ⬇️
...pages/Route/components/Step1/MatchingRulesView.tsx 25.00% <0.00%> (-54.17%) ⬇️
web/src/components/Plugin/UI/limit-count.tsx 44.18% <0.00%> (-44.19%) ⬇️
web/src/pages/Route/transform.ts 52.91% <0.00%> (-27.52%) ⬇️
.../src/pages/Route/components/Step1/ProxyRewrite.tsx 70.68% <0.00%> (-24.14%) ⬇️
...b/src/components/Plugin/UI/referer-restriction.tsx 69.69% <0.00%> (-21.22%) ⬇️
... and 17 more

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@SkyeYoung
Copy link
Member

Pls try to follow the lint CI to fix lint problems

@SkyeYoung
Copy link
Member

@FangSen9000 Any update here?

@FangSen9000
Copy link
Contributor Author

@FangSen9000 Any update here?
I've had a day off, and I'll try to update it tonight

web/src/pages/Route/List.tsx Outdated Show resolved Hide resolved
web/src/pages/Route/List.tsx Outdated Show resolved Hide resolved
@FangSen9000
Copy link
Contributor Author

@Baoyuantop @guoqqqi Do you have any questions or comments on this test case? If not, you can approve it, and then let the back-end testers debug their use cases

@guoqqqi
Copy link
Member

guoqqqi commented Aug 7, 2022

image
A good tip is that usually the content of these reviews should not be marked as resolved by you, but by the person who reviewed it for you 😄

@FangSen9000
Copy link
Contributor Author

FangSen9000 commented Aug 7, 2022 via email

@FangSen9000
Copy link
Contributor Author

image A good tip is that usually the content of these reviews should not be marked as resolved by you, but by the person who reviewed it for you 😄

I made some mistakes after I adopted the code. Do you have any good suggestions. Is there any way to solve it? I'm debugging your update on the local machine

@guoqqqi
Copy link
Member

guoqqqi commented Aug 7, 2022

image A good tip is that usually the content of these reviews should not be marked as resolved by you, but by the person who reviewed it for you 😄

I made some mistakes after I adopted the code. Do you have any good suggestions. Is there any way to solve it? I'm debugging your update on the local machine

you need update the i18n file for component.global.delete.routes.success

@FangSen9000
Copy link
Contributor Author

FangSen9000 commented Aug 7, 2022

@guoqqqi I got it on the local machine, should pass him in as a variable. There was a slight problem with the syntax, which has now been resolved. You're missing a '$'. Thanks for your suggestion.
${formatMessage({ id: 'component.global.delete.routes.success' })}

@FangSen9000
Copy link
Contributor Author

you need update the i18n file for component.global.delete.routes.success
@guoqqqi You are right. This tag is a non-existent tag. I will add this tag tomorrow.
component.global.delete.routes.success
You created it...

Copy link
Member

@guoqqqi guoqqqi left a comment

Choose a reason for hiding this comment

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

LGTM

@FangSen9000
Copy link
Contributor Author

@guoqqqi I have completed the changes you suggested. Check that everything is normal
@Baoyuantop @LiteSun Are there any good questions and suggestions.

cy.contains(data.test0).should('not.exist');
cy.contains(data.test1).should('not.exist');
cy.contains(data.test2).should('not.exist');
});
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need to test these search functions? We only need to judge that the data does not exist under https://github.com/apache/apisix-dashboard/pull/2502/files#diff-6918beb43c48e71616c503a8dffc6d9cc572f37a69d1e27f792b71fda2e5416aR117.

@FangSen9000
Copy link
Contributor Author

FangSen9000 commented Aug 8, 2022 via email

@bzp2010 bzp2010 merged commit 0273865 into apache:master Aug 9, 2022
hongbinhsu pushed a commit to fitphp/apix-dashboard that referenced this pull request Sep 10, 2022
* upstream/master: (23 commits)
  feat: Add config struct of OpenID-Connect Login (apache#2597)
  feat: set serverUrlMap with env, update cypress, update stylelint (apache#2583)
  chore: fix function name typo (apache#2599)
  fix: page refresh causes deletion exception (apache#2593)
  feat: support show all enable plugin list tab (apache#2585)
  fix: drawer components delete plugin not working (apache#2573)
  feat: add batch delete function for route (apache#2502)
  test: reduce fe ci time (apache#2557)
  doc(csp): add correct csp rule (apache#2548)
  doc: add a notice about the compatibility of Ingress and Dashboard (apache#2552)
  fix: add judgement for last_report_time (apache#2551)
  fix: cli test invalid etcd (apache#2544)
  feat: fix actions version to root version (apache#2521)
  fix: duplicate ID (apache#2501)
  fix: block arbitrary file index (apache#2497)
  docs: update deploy-with-docker.md (apache#2472)
  feat: translating Turkish for new features (apache#2487)
  docs: add new import and export docs to sidebar (apache#2485)
  docs: add data loader and new OpenAPI 3 loader (apache#2484)
  feat: support data loader in frontend (apache#2480)
  ...

# Conflicts:
#	api/internal/route.go
#	web/config/defaultSettings.ts
#	web/yarn.lock
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.

6 participants