feat: add page reload judgment #2370

oil-oil · 2022-03-09T06:32:42Z

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

Bugfix
New feature provided
Improve performance
Backport patches

What changes will this PR take into?

When the table needs to be reloaded after operation, judge whether there is no data on the current page. If there is no data, move the page number to the previous page.

Related issues

resolve: #2129

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 · 2022-03-09T07:10:54Z

Codecov Report

Merging #2370 (5b6671c) into master (561ed37) will increase coverage by 0.26%.
The diff coverage is 93.33%.

@@            Coverage Diff             @@
##           master    #2370      +/-   ##
==========================================
+ Coverage   68.52%   68.79%   +0.26%     
==========================================
  Files         131      131              
  Lines        3419     3432      +13     
  Branches      826      833       +7     
==========================================
+ Hits         2343     2361      +18     
+ Misses       1076     1071       -5

Flag	Coverage Δ
frontend-e2e-test	`68.79% <93.33%> (+0.26%)`	⬆️

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

Impacted Files	Coverage Δ
web/src/pages/Route/List.tsx	`83.63% <85.71%> (+0.60%)`	⬆️
web/src/hooks/usePagination.ts	`100.00% <100.00%> (ø)`
web/src/pages/Consumer/List.tsx	`91.66% <100.00%> (+3.09%)`	⬆️
web/src/pages/Proto/List.tsx	`100.00% <100.00%> (+3.84%)`	⬆️
web/src/pages/SSL/List.tsx	`85.00% <100.00%> (ø)`
web/src/pages/Service/List.tsx	`94.11% <100.00%> (ø)`
web/src/pages/Upstream/List.tsx	`94.28% <100.00%> (+2.85%)`	⬆️
web/src/pages/Consumer/Create.tsx	`82.69% <0.00%> (ø)`
web/src/pages/Consumer/components/Step1.tsx	`100.00% <0.00%> (ø)`
web/src/components/Plugin/PluginPage.tsx	`98.82% <0.00%> (+0.08%)`	⬆️
... and 6 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 561ed37...5b6671c. Read the comment docs.

Baoyuantop

need test case add

web/src/pages/Consumer/List.tsx

web/cypress/integration/consumer/batch-create-and-delete-consumer.spec.js

oil-oil · 2022-03-14T06:41:41Z

I will modify the simulation operation to send a request to ensure the stability of the test.

oil-oil · 2022-03-16T01:58:15Z

@Baoyuantop @guoqqqi I have updated the test case, and now I can pass the test successfully. Please review it for me again.🤓

Baoyuantop · 2022-03-16T03:10:09Z

The structure of the test needs to be more rationalized.

guoqqqi · 2022-03-16T08:28:50Z

web/cypress/integration/consumer/table-auto-jump-when-no-data.js.js

+  it('should delete last data and jump to first page', () => {
+    cy.visit('/');
+    cy.contains('Consumer').click();
+    cy.wait(500);


Do we really need to wait for this?

guoqqqi · 2022-03-16T08:32:42Z

web/cypress/integration/proto/table-auto-jump-when-no-data.js

+  const deleteProto = () => {
+    cy.wait(500);
+    cy.contains('Delete').click();
+    cy.contains('button', 'Confirm').click();
+    cy.get(selector.notification).should('contain', data.deleteProtoSuccess);
+    cy.get(selector.notificationCloseIcon).click();
+  };


We can use the custom command from the official documentation, By the way, we do not recommend using wait
https://docs.cypress.io/api/cypress-api/custom-commands
https://docs.cypress.io/api/commands/wait#Wait-for-an-arbitrary-period-of-milliseconds

guoqqqi · 2022-03-16T08:45:22Z

web/cypress/integration/proto/table-auto-jump-when-no-data.js

+    cy.wait(500);
+    cy.get(selector.page_item).click();
+    cy.wait(500);
+    deleteProto();


Suggested change

deleteProto();

deleteProto();

IMO, I think it looks strange the way it's written.

guoqqqi · 2022-03-16T08:49:39Z

web/src/hooks/usePagination.ts

+    }
+  };
+
+  return { paginationConfig, savePageList, checkPageList };


Since we have abstracted the pager into a hook, is it enough for us to write a test for the pagination

Baoyuantop · 2022-03-17T01:07:37Z

web/cypress/integration/consumer/table-auto-jump-when-no-data.spec.js

+    deleteConsumerSuccess: 'Delete Consumer Successfully',
+  };
+
+  beforeEach(() => {


beforeEach or before?

Baoyuantop

LGTM

Baoyuantop · 2022-03-18T01:56:44Z

cc @guoqqqi

guoqqqi · 2022-03-18T02:14:57Z

web/cypress/integration/consumer/table-auto-jump-when-no-data.spec.js

+    cy.visit('/');
+    cy.contains('Consumer').click();
+    cy.get(selector.page_item).click();
+    cy.wait(1000);


Why wait for 1 s?

@oil-oil ping

guoqqqi · 2022-03-18T02:16:19Z

web/cypress/integration/consumer/table-auto-jump-when-no-data.spec.js

+    cy.contains('Consumer').click();
+    cy.get(selector.page_item).click();
+    cy.wait(1000);
+    cy.contains('Delete').click({ force: true });


the force: true will not ensure that the click on delete will be successful, so why use force here?

guoqqqi · 2022-03-18T02:17:54Z

web/cypress/integration/proto/table-auto-jump-when-no-data.spec.js

+  beforeEach(() => {
+    cy.login();
+    Array.from({ length: 11 }).forEach(async (value, key) => {
+      const payload = {
+        content: 'test',
+        desc: '',
+        id: `protoId${key}`,
+      };
+      cy.requestWithToken({ method: 'POST', payload, url: '/apisix/admin/proto' });
+    });
+  });


Note that using an array loop in beforeEach will cause the loop to be run once for each test case run

guoqqqi · 2022-03-18T02:18:35Z

web/cypress/integration/proto/table-auto-jump-when-no-data.spec.js

+    cy.visit('/');
+    cy.contains('Protocol Buffers').click();
+    cy.get(selector.page_item).click();
+    cy.wait(1000);


web/cypress/integration/upstream/table-auto-jump-when-no-data.spec.js

SkyeYoung · 2022-03-29T03:13:01Z

web/src/hooks/usePagination.ts

@@ -30,5 +33,14 @@ export default function usePagination() {
    history.replace(`${location.pathname}?page=${page}&pageSize=${pageSize}`);
  };

-  return { paginationConfig, savePageList };
+  const checkPageList = (ref: MutableRefObject<ActionType | undefined>) => {
+    const { current, pageSize, total } = ref.current!.pageInfo as PageInfo;


Suggested change

const { current, pageSize, total } = ref.current!.pageInfo as PageInfo;

const { current, pageSize, total } = ref.current?.pageInfo as PageInfo;

!. will just remove undefined and null from the current's type. Maybe ?. is better?

Baoyuantop

cc @SkyeYoung @guoqqqi

SkyeYoung · 2022-04-12T07:58:21Z

There is no preview here, the code looks ok.

LiteSun · 2022-04-26T01:00:12Z

web/cypress/integration/consumer/table-auto-jump-when-no-data.spec.js

+  };
+
+  before(() => {
+    cy.login();


Suggested change

cy.login();

cy.login().then( () => { cy.requestWithToken({ method: 'PUT', payload, url: '/apisix/admin/consumers' });

});

LiteSun · 2022-04-26T01:00:49Z

web/cypress/integration/proto/table-auto-jump-when-no-data.spec.js

+  };
+
+  before(() => {
+    cy.login();


oil-oil · 2022-04-28T00:54:51Z

@LiteSun OK, I'll update it

Baoyuantop · 2022-05-05T03:33:30Z

cc @LiteSun

* upstream: (28 commits) docs: update some tips in develop.md (apache#2451) fix: create upstream error when pass host is node and nodes without port (apache#2421) fix: correct data type of filed Active.checks.active.https_verify_certificate (apache#2422) feat: basic support Apache APISIX 2.13.0 (apache#2428) feat: add page reload judgment (apache#2370) docs: Update RPM install package link (apache#2439) docs: Remove hyperlinks from documents (apache#2431) chore(deps): bump actions/upload-artifact from 2 to 3 (apache#2423) fix: consumer without plugins causes page crashes (apache#2437) chore(deps): bump axios from 0.21.1 to 0.21.4 in /web (apache#2420) feat: Modify plugin preview page (apache#2359) chore(deps): bump moment from 2.29.1 to 2.29.2 in /web (apache#2418) chore: use json schema instead hard code (apache#2399) refactor: migrate route tests to e2enew (apache#2411) chore(deps): bump actions/setup-python from 2.3.2 to 3.1.1 (apache#2414) chore: update the year of copyright and fix typo (apache#2417) docs: update online playground url (apache#2416) chore: update README for online demo (apache#2404) refactor: migrate id compatible tests to e2enew (apache#2400) docs: fix invalid link (apache#2366) ... # Conflicts: # api/internal/route.go # web/src/components/Footer/index.tsx

feat: add page reload judgment

ddb64f0

Baoyuantop reviewed Mar 10, 2022

View reviewed changes

web/src/pages/Consumer/List.tsx Outdated Show resolved Hide resolved

feat: add test case

629ca33

Baoyuantop reviewed Mar 14, 2022

View reviewed changes

web/cypress/integration/consumer/batch-create-and-delete-consumer.spec.js Outdated Show resolved Hide resolved

oil-oil added 5 commits March 15, 2022 11:39

feat: add requestWithToken command

c9f1684

fix: modify manual action to send request

b148e46

fix: udpate test case

6cfad8e

fix: add test case wait time

8278d2e

fix: update test case

2bfb532

oil-oil added 2 commits March 16, 2022 14:49

fix: modify the test case code structure

d6274a5

fix: update proto test case

24fd393

guoqqqi reviewed Mar 16, 2022

View reviewed changes

fix: update test case

8f616c3

Baoyuantop reviewed Mar 17, 2022

View reviewed changes

Baoyuantop approved these changes Mar 18, 2022

View reviewed changes

guoqqqi reviewed Mar 18, 2022

View reviewed changes

fix: remove the force attribute of the click event

a797c9b

guoqqqi reviewed Mar 22, 2022

View reviewed changes

web/cypress/integration/upstream/table-auto-jump-when-no-data.spec.js Outdated Show resolved Hide resolved

fix: delete extra characters

ca5b007

SkyeYoung reviewed Mar 29, 2022

View reviewed changes

fix: update hook type

b23afeb

oil-oil requested a review from guoqqqi April 2, 2022 10:31

oil-oil requested a review from Baoyuantop April 12, 2022 07:04

Baoyuantop reviewed Apr 12, 2022

View reviewed changes

guoqqqi approved these changes Apr 13, 2022

View reviewed changes

Baoyuantop requested a review from LiteSun April 24, 2022 05:06

LiteSun reviewed Apr 26, 2022

View reviewed changes

fix: move requestWithToken position

5b6671c

oil-oil requested a review from LiteSun April 30, 2022 06:39

SkyeYoung approved these changes Apr 30, 2022

View reviewed changes

LiteSun approved these changes May 6, 2022

View reviewed changes

Baoyuantop merged commit 30fa852 into apache:master May 6, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add page reload judgment #2370

feat: add page reload judgment #2370

oil-oil commented Mar 9, 2022

codecov-commenter commented Mar 9, 2022 •

edited

Loading

Baoyuantop left a comment

oil-oil commented Mar 14, 2022

oil-oil commented Mar 16, 2022

Baoyuantop commented Mar 16, 2022

guoqqqi Mar 16, 2022

guoqqqi Mar 16, 2022

guoqqqi Mar 16, 2022

guoqqqi Mar 16, 2022

Baoyuantop Mar 17, 2022

Baoyuantop left a comment

Baoyuantop commented Mar 18, 2022

guoqqqi Mar 18, 2022

SkyeYoung Apr 12, 2022

guoqqqi Mar 18, 2022

guoqqqi Mar 18, 2022

guoqqqi Mar 18, 2022

SkyeYoung Mar 29, 2022

Baoyuantop left a comment

SkyeYoung commented Apr 12, 2022

LiteSun Apr 26, 2022

LiteSun Apr 26, 2022

oil-oil commented Apr 28, 2022

Baoyuantop commented May 5, 2022

	const { current, pageSize, total } = ref.current!.pageInfo as PageInfo;
	const { current, pageSize, total } = ref.current?.pageInfo as PageInfo;

	cy.login();
	cy.login().then( () => { cy.requestWithToken({ method: 'PUT', payload, url: '/apisix/admin/consumers' });
	});

feat: add page reload judgment #2370

feat: add page reload judgment #2370

Conversation

oil-oil commented Mar 9, 2022

codecov-commenter commented Mar 9, 2022 • edited Loading

Codecov Report

Baoyuantop left a comment

Choose a reason for hiding this comment

oil-oil commented Mar 14, 2022

oil-oil commented Mar 16, 2022

Baoyuantop commented Mar 16, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Baoyuantop left a comment

Choose a reason for hiding this comment

Baoyuantop commented Mar 18, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Baoyuantop left a comment

Choose a reason for hiding this comment

SkyeYoung commented Apr 12, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oil-oil commented Apr 28, 2022

Baoyuantop commented May 5, 2022

codecov-commenter commented Mar 9, 2022 •

edited

Loading