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 2021/05/25 12:29:41 UTC

[GitHub] [apisix-dashboard] iamayushdas opened a new pull request #1918: feat: added route path search

iamayushdas opened a new pull request #1918:
URL: https://github.com/apache/apisix-dashboard/pull/1918


   Please answer these questions before submitting a pull request, **or your PR will get closed**.
   
   **Why submit this pull request?**
   
   - [ ] Bugfix
   - [x] New feature provided
   - [x] Improve performance
   - [ ] Backport patches
   
   **What changes will this PR take into?**
   added route `path` search option in route page
   
   **Related issues**
   
   fix/resolve #1916 
   
   **Checklist:**
   
   - [x] 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?
   - [x] Is this PR backward compatible? If it is not backward compatible, please discuss on the mailing list first
   


-- 
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

[GitHub] [apisix-dashboard] LiteSun commented on a change in pull request #1918: feat: added route path search

Posted by GitBox <gi...@apache.org>.
LiteSun commented on a change in pull request #1918:
URL: https://github.com/apache/apisix-dashboard/pull/1918#discussion_r642739879



##########
File path: web/cypress/integration/route/search-route.spec.js
##########
@@ -17,18 +17,20 @@
 /* eslint-disable no-undef */
 
 context('Create and Search Route', () => {
-  const timeout = 500;
+  const timeout = 1000;

Review comment:
       Just for notes: the difference in timeout may be due to different computer performance. Also, cypress does not officially recommend using `cy.wait()`,  which can make the test unstable.




-- 
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

[GitHub] [apisix-dashboard] iamayushdas commented on a change in pull request #1918: feat: added route path search

Posted by GitBox <gi...@apache.org>.
iamayushdas commented on a change in pull request #1918:
URL: https://github.com/apache/apisix-dashboard/pull/1918#discussion_r640235079



##########
File path: web/src/pages/Route/List.tsx
##########
@@ -343,6 +343,7 @@ const Page: React.FC = () => {
     },
     {
       title: formatMessage({ id: 'page.route.path' }),
+      dataIndex: 'uri',

Review comment:
       I have done it according to the params , is it okay to do so? Also i have texted you the message regarding this on slack
   
   ![image](https://user-images.githubusercontent.com/40708551/119755944-745a3800-bec0-11eb-97d7-ea429c4f598b.jpeg)




-- 
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

[GitHub] [apisix-dashboard] juzhiyuan commented on a change in pull request #1918: feat: added route path search

Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on a change in pull request #1918:
URL: https://github.com/apache/apisix-dashboard/pull/1918#discussion_r640216614



##########
File path: web/src/pages/Route/List.tsx
##########
@@ -343,6 +343,7 @@ const Page: React.FC = () => {
     },
     {
       title: formatMessage({ id: 'page.route.path' }),
+      dataIndex: 'uri',

Review comment:
       Actually, we use `uris` in the frontend 🤔 




-- 
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

[GitHub] [apisix-dashboard] netlify[bot] edited a comment on pull request #1918: feat: added route path search

Posted by GitBox <gi...@apache.org>.
netlify[bot] edited a comment on pull request #1918:
URL: https://github.com/apache/apisix-dashboard/pull/1918#issuecomment-847827497


   Deploy Preview for *apisix-dashboard* ready!
   
   Built with commit 0fd9616de17fc1a9b0e6109639440a3ad641bb1f
   
   https://deploy-preview-1918--apisix-dashboard.netlify.app


-- 
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

[GitHub] [apisix-dashboard] codecov-commenter commented on pull request #1918: feat: added route path search

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #1918:
URL: https://github.com/apache/apisix-dashboard/pull/1918#issuecomment-851764336


   # [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/1918?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#1918](https://codecov.io/gh/apache/apisix-dashboard/pull/1918?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (39baf1c) into [master](https://codecov.io/gh/apache/apisix-dashboard/commit/91ad26be6dcda1813419b51bc9d0203c86a1d5d0?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (91ad26b) will **decrease** coverage by `2.93%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/apisix-dashboard/pull/1918/graphs/tree.svg?width=650&height=150&src=pr&token=Q1HERXN96P&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/apisix-dashboard/pull/1918?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #1918      +/-   ##
   ==========================================
   - Coverage   67.00%   64.07%   -2.94%     
   ==========================================
     Files         169      122      -47     
     Lines        6314     3112    -3202     
     Branches      745      748       +3     
   ==========================================
   - Hits         4231     1994    -2237     
   + Misses       1832     1118     -714     
   + Partials      251        0     -251     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | backend-e2e-test | `?` | |
   | backend-e2e-test-ginkgo | `?` | |
   | backend-unit-test | `?` | |
   | frontend-e2e-test | `64.07% <ø> (+0.09%)` | :arrow_up: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/1918?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [web/src/pages/Route/List.tsx](https://codecov.io/gh/apache/apisix-dashboard/pull/1918/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-d2ViL3NyYy9wYWdlcy9Sb3V0ZS9MaXN0LnRzeA==) | `84.52% <ø> (ø)` | |
   | [web/src/pages/Route/transform.ts](https://codecov.io/gh/apache/apisix-dashboard/pull/1918/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-d2ViL3NyYy9wYWdlcy9Sb3V0ZS90cmFuc2Zvcm0udHM=) | `72.35% <0.00%> (-0.11%)` | :arrow_down: |
   | [...b/src/components/Plugin/UI/referer-restriction.tsx](https://codecov.io/gh/apache/apisix-dashboard/pull/1918/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-d2ViL3NyYy9jb21wb25lbnRzL1BsdWdpbi9VSS9yZWZlcmVyLXJlc3RyaWN0aW9uLnRzeA==) | `66.66% <0.00%> (ø)` | |
   | [...pages/Route/components/Step1/RequestConfigView.tsx](https://codecov.io/gh/apache/apisix-dashboard/pull/1918/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-d2ViL3NyYy9wYWdlcy9Sb3V0ZS9jb21wb25lbnRzL1N0ZXAxL1JlcXVlc3RDb25maWdWaWV3LnRzeA==) | `81.48% <0.00%> (ø)` | |
   | [api/internal/core/store/store\_mock.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1918/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvc3RvcmVfbW9jay5nbw==) | | |
   | [api/internal/handler/schema/schema.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1918/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvc2NoZW1hL3NjaGVtYS5nbw==) | | |
   | [api/internal/filter/authentication.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1918/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YXBpL2ludGVybmFsL2ZpbHRlci9hdXRoZW50aWNhdGlvbi5nbw==) | | |
   | [api/internal/handler/global\_rule/global\_rule.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1918/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvZ2xvYmFsX3J1bGUvZ2xvYmFsX3J1bGUuZ28=) | | |
   | [api/internal/log/zap.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1918/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YXBpL2ludGVybmFsL2xvZy96YXAuZ28=) | | |
   | [api/internal/log/log.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1918/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YXBpL2ludGVybmFsL2xvZy9sb2cuZ28=) | | |
   | ... and [42 more](https://codecov.io/gh/apache/apisix-dashboard/pull/1918/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/1918?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/1918?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [7551b78...39baf1c](https://codecov.io/gh/apache/apisix-dashboard/pull/1918?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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

[GitHub] [apisix-dashboard] iamayushdas commented on a change in pull request #1918: feat: added route path search

Posted by GitBox <gi...@apache.org>.
iamayushdas commented on a change in pull request #1918:
URL: https://github.com/apache/apisix-dashboard/pull/1918#discussion_r642584682



##########
File path: web/cypress/integration/route/search-route.spec.js
##########
@@ -126,9 +135,35 @@ context('Create and Search Route', () => {
     cy.contains(data.test2).should('not.exist');
   });
 
+  it('should search the route with path', function () {
+    cy.visit('/');
+    cy.contains('Route').click();
+    // full match
+    cy.get(selector.pathSearch).type(data.uris1);
+    cy.contains('Search').click();
+    cy.contains(data.uris1).should('contain', data.uris1);
+    cy.contains(data.uris0).should('not.exist');
+    cy.contains(data.uris2).should('not.exist');
+    // partial match
+    cy.reload();

Review comment:
       should i change name search part too?
   




-- 
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

[GitHub] [apisix-dashboard] netlify[bot] commented on pull request #1918: feat: added route path search

Posted by GitBox <gi...@apache.org>.
netlify[bot] commented on pull request #1918:
URL: https://github.com/apache/apisix-dashboard/pull/1918#issuecomment-847827497


   Deploy Preview for *apisix-dashboard* processing.
   
   Building with commit 0fd9616de17fc1a9b0e6109639440a3ad641bb1f
   
   https://app.netlify.com/sites/apisix-dashboard/deploys/60acedaaed3c500007fb4247


-- 
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

[GitHub] [apisix-dashboard] iamayushdas commented on pull request #1918: feat: added route path search

Posted by GitBox <gi...@apache.org>.
iamayushdas commented on pull request #1918:
URL: https://github.com/apache/apisix-dashboard/pull/1918#issuecomment-851797277


   @nic-chen @LiteSun need your review


-- 
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

[GitHub] [apisix-dashboard] netlify[bot] edited a comment on pull request #1918: feat: added route path search

Posted by GitBox <gi...@apache.org>.
netlify[bot] edited a comment on pull request #1918:
URL: https://github.com/apache/apisix-dashboard/pull/1918#issuecomment-847827497


   Deploy Preview for *apisix-dashboard* ready!
   
   Built with commit 51e01b90f9e43f3d8e4526adefe22b585b3ebc5e
   
   https://deploy-preview-1918--apisix-dashboard.netlify.app


-- 
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

[GitHub] [apisix-dashboard] iamayushdas commented on a change in pull request #1918: feat: added route path search

Posted by GitBox <gi...@apache.org>.
iamayushdas commented on a change in pull request #1918:
URL: https://github.com/apache/apisix-dashboard/pull/1918#discussion_r640235079



##########
File path: web/src/pages/Route/List.tsx
##########
@@ -343,6 +343,7 @@ const Page: React.FC = () => {
     },
     {
       title: formatMessage({ id: 'page.route.path' }),
+      dataIndex: 'uri',

Review comment:
       I have done it according to the params , is it okay to do so? Also i have texted you the message regarding this on slack




-- 
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

[GitHub] [apisix-dashboard] iamayushdas commented on pull request #1918: feat: added route path search

Posted by GitBox <gi...@apache.org>.
iamayushdas commented on pull request #1918:
URL: https://github.com/apache/apisix-dashboard/pull/1918#issuecomment-851949155


   @starsz can i have your review here?


-- 
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

[GitHub] [apisix-dashboard] liuxiran commented on a change in pull request #1918: feat: added route path search

Posted by GitBox <gi...@apache.org>.
liuxiran commented on a change in pull request #1918:
URL: https://github.com/apache/apisix-dashboard/pull/1918#discussion_r642589562



##########
File path: web/cypress/integration/route/search-route.spec.js
##########
@@ -17,18 +17,20 @@
 /* eslint-disable no-undef */
 
 context('Create and Search Route', () => {
-  const timeout = 500;
+  const timeout = 1000;

Review comment:
       BTW @iamayushdas , why increase the `timeout` here?




-- 
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

[GitHub] [apisix-dashboard] iamayushdas commented on a change in pull request #1918: feat: added route path search

Posted by GitBox <gi...@apache.org>.
iamayushdas commented on a change in pull request #1918:
URL: https://github.com/apache/apisix-dashboard/pull/1918#discussion_r642740274



##########
File path: web/cypress/integration/route/search-route.spec.js
##########
@@ -17,18 +17,20 @@
 /* eslint-disable no-undef */
 
 context('Create and Search Route', () => {
-  const timeout = 500;
+  const timeout = 1000;

Review comment:
       cool @LiteSun 




-- 
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

[GitHub] [apisix-dashboard] starsz commented on a change in pull request #1918: feat: added route path search

Posted by GitBox <gi...@apache.org>.
starsz commented on a change in pull request #1918:
URL: https://github.com/apache/apisix-dashboard/pull/1918#discussion_r641946755



##########
File path: web/src/pages/Route/List.tsx
##########
@@ -343,6 +343,7 @@ const Page: React.FC = () => {
     },
     {
       title: formatMessage({ id: 'page.route.path' }),
+      dataIndex: 'uri',

Review comment:
       Hi @juzhiyuan.I check the backend code. We don't need to cover uri to [uri] as a query parameter.
   
   The backend will check the URI in both `route.uri` and `route.uris`.




-- 
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

[GitHub] [apisix-dashboard] iamayushdas commented on a change in pull request #1918: feat: added route path search

Posted by GitBox <gi...@apache.org>.
iamayushdas commented on a change in pull request #1918:
URL: https://github.com/apache/apisix-dashboard/pull/1918#discussion_r640334955



##########
File path: web/src/pages/Route/List.tsx
##########
@@ -343,6 +343,7 @@ const Page: React.FC = () => {
     },
     {
       title: formatMessage({ id: 'page.route.path' }),
+      dataIndex: 'uri',

Review comment:
       Sorry, i didn't got you




-- 
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

[GitHub] [apisix-dashboard] iamayushdas commented on a change in pull request #1918: feat: added route path search

Posted by GitBox <gi...@apache.org>.
iamayushdas commented on a change in pull request #1918:
URL: https://github.com/apache/apisix-dashboard/pull/1918#discussion_r640235079



##########
File path: web/src/pages/Route/List.tsx
##########
@@ -343,6 +343,7 @@ const Page: React.FC = () => {
     },
     {
       title: formatMessage({ id: 'page.route.path' }),
+      dataIndex: 'uri',

Review comment:
       I have done it according to the params , is it okay to do so?




-- 
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

[GitHub] [apisix-dashboard] netlify[bot] edited a comment on pull request #1918: feat: added route path search

Posted by GitBox <gi...@apache.org>.
netlify[bot] edited a comment on pull request #1918:
URL: https://github.com/apache/apisix-dashboard/pull/1918#issuecomment-847827497


   Deploy Preview for *apisix-dashboard* processing.
   
   Building with commit 51e01b90f9e43f3d8e4526adefe22b585b3ebc5e
   
   https://app.netlify.com/sites/apisix-dashboard/deploys/60ad168714ae990007849462


-- 
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

[GitHub] [apisix-dashboard] juzhiyuan merged pull request #1918: feat: added route path search

Posted by GitBox <gi...@apache.org>.
juzhiyuan merged pull request #1918:
URL: https://github.com/apache/apisix-dashboard/pull/1918


   


-- 
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

[GitHub] [apisix-dashboard] netlify[bot] edited a comment on pull request #1918: feat: added route path search

Posted by GitBox <gi...@apache.org>.
netlify[bot] edited a comment on pull request #1918:
URL: https://github.com/apache/apisix-dashboard/pull/1918#issuecomment-847827497


   :construction_worker: Deploy Preview for *apisix-dashboard* processing.
   
   
   :hammer: Explore the source changes: 39baf1cd34db1b3e68fb5e9d33e97c7be36cb285
   
   :mag: Inspect the deploy log: [https://app.netlify.com/sites/apisix-dashboard/deploys/60b59a10217ef800083058b3](https://app.netlify.com/sites/apisix-dashboard/deploys/60b59a10217ef800083058b3?utm_source=github&utm_campaign=bot_dl)
   


-- 
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

[GitHub] [apisix-dashboard] liuxiran commented on a change in pull request #1918: feat: added route path search

Posted by GitBox <gi...@apache.org>.
liuxiran commented on a change in pull request #1918:
URL: https://github.com/apache/apisix-dashboard/pull/1918#discussion_r642603653



##########
File path: web/cypress/integration/route/search-route.spec.js
##########
@@ -17,18 +17,20 @@
 /* eslint-disable no-undef */
 
 context('Create and Search Route', () => {
-  const timeout = 500;
+  const timeout = 1000;

Review comment:
       > because in 500ms, all the components were not loading, so i increased i gave it sweet 1 second, and it was giving a error
   
   may be we can  analyze the cause of the error in your env , it seems that `timeout=500` works well in my local several times, and the code associated with timeout seems not modified in this pr




-- 
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

[GitHub] [apisix-dashboard] iamayushdas commented on a change in pull request #1918: feat: added route path search

Posted by GitBox <gi...@apache.org>.
iamayushdas commented on a change in pull request #1918:
URL: https://github.com/apache/apisix-dashboard/pull/1918#discussion_r642590569



##########
File path: web/cypress/integration/route/search-route.spec.js
##########
@@ -17,18 +17,20 @@
 /* eslint-disable no-undef */
 
 context('Create and Search Route', () => {
-  const timeout = 500;
+  const timeout = 1000;

Review comment:
       because in 500ms, all the components were not loading, so i increased i gave it sweet 1 second




-- 
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

[GitHub] [apisix-dashboard] iamayushdas commented on a change in pull request #1918: feat: added route path search

Posted by GitBox <gi...@apache.org>.
iamayushdas commented on a change in pull request #1918:
URL: https://github.com/apache/apisix-dashboard/pull/1918#discussion_r642591651



##########
File path: web/cypress/integration/route/search-route.spec.js
##########
@@ -17,18 +17,20 @@
 /* eslint-disable no-undef */
 
 context('Create and Search Route', () => {
-  const timeout = 500;
+  const timeout = 1000;

Review comment:
       is there any thing else ?
   




-- 
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

[GitHub] [apisix-dashboard] juzhiyuan commented on a change in pull request #1918: feat: added route path search

Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on a change in pull request #1918:
URL: https://github.com/apache/apisix-dashboard/pull/1918#discussion_r640284149



##########
File path: web/src/pages/Route/List.tsx
##########
@@ -343,6 +343,7 @@ const Page: React.FC = () => {
     },
     {
       title: formatMessage({ id: 'page.route.path' }),
+      dataIndex: 'uri',

Review comment:
       You need to convert uri to [uri] and uris




-- 
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

[GitHub] [apisix-dashboard] liuxiran commented on a change in pull request #1918: feat: added route path search

Posted by GitBox <gi...@apache.org>.
liuxiran commented on a change in pull request #1918:
URL: https://github.com/apache/apisix-dashboard/pull/1918#discussion_r642598730



##########
File path: web/cypress/integration/route/search-route.spec.js
##########
@@ -126,9 +135,35 @@ context('Create and Search Route', () => {
     cy.contains(data.test2).should('not.exist');
   });
 
+  it('should search the route with path', function () {
+    cy.visit('/');
+    cy.contains('Route').click();
+    // full match
+    cy.get(selector.pathSearch).type(data.uris1);
+    cy.contains('Search').click();
+    cy.contains(data.uris1).should('contain', data.uris1);
+    cy.contains(data.uris0).should('not.exist');
+    cy.contains(data.uris2).should('not.exist');
+    // partial match
+    cy.reload();

Review comment:
       > okay, Do you want me to give another PR?
   
   I just created an issue: https://github.com/apache/apisix-dashboard/issues/1925, pr is welcome ^^




-- 
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

[GitHub] [apisix-dashboard] iamayushdas commented on a change in pull request #1918: feat: added route path search

Posted by GitBox <gi...@apache.org>.
iamayushdas commented on a change in pull request #1918:
URL: https://github.com/apache/apisix-dashboard/pull/1918#discussion_r642579753



##########
File path: web/cypress/integration/route/search-route.spec.js
##########
@@ -126,9 +135,35 @@ context('Create and Search Route', () => {
     cy.contains(data.test2).should('not.exist');
   });
 
+  it('should search the route with path', function () {
+    cy.visit('/');
+    cy.contains('Route').click();
+    // full match
+    cy.get(selector.pathSearch).type(data.uris1);
+    cy.contains('Search').click();
+    cy.contains(data.uris1).should('contain', data.uris1);
+    cy.contains(data.uris0).should('not.exist');
+    cy.contains(data.uris2).should('not.exist');
+    // partial match
+    cy.reload();

Review comment:
       I was following the legacy of above name search test, You can see that in name search also we have used `cy.reload()`
   just to reload the screen to refresh
   ![Screenshot from 2021-05-31 21-27-42](https://user-images.githubusercontent.com/40708551/120218428-58a3b880-c257-11eb-8473-3a7730d2057a.png)
   




-- 
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

[GitHub] [apisix-dashboard] liuxiran commented on a change in pull request #1918: feat: added route path search

Posted by GitBox <gi...@apache.org>.
liuxiran commented on a change in pull request #1918:
URL: https://github.com/apache/apisix-dashboard/pull/1918#discussion_r642588962



##########
File path: web/cypress/integration/route/search-route.spec.js
##########
@@ -126,9 +135,35 @@ context('Create and Search Route', () => {
     cy.contains(data.test2).should('not.exist');
   });
 
+  it('should search the route with path', function () {
+    cy.visit('/');
+    cy.contains('Route').click();
+    // full match
+    cy.get(selector.pathSearch).type(data.uris1);
+    cy.contains('Search').click();
+    cy.contains(data.uris1).should('contain', data.uris1);
+    cy.contains(data.uris0).should('not.exist');
+    cy.contains(data.uris2).should('not.exist');
+    // partial match
+    cy.reload();

Review comment:
       got it @iamayushdas , we can optimize this point in a new pr




-- 
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

[GitHub] [apisix-dashboard] iamayushdas commented on a change in pull request #1918: feat: added route path search

Posted by GitBox <gi...@apache.org>.
iamayushdas commented on a change in pull request #1918:
URL: https://github.com/apache/apisix-dashboard/pull/1918#discussion_r642590569



##########
File path: web/cypress/integration/route/search-route.spec.js
##########
@@ -17,18 +17,20 @@
 /* eslint-disable no-undef */
 
 context('Create and Search Route', () => {
-  const timeout = 500;
+  const timeout = 1000;

Review comment:
       because in 500ms, all the components were not loading, so i increased i gave it sweet 1 second, and it was giving a error
   




-- 
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

[GitHub] [apisix-dashboard] liuxiran commented on a change in pull request #1918: feat: added route path search

Posted by GitBox <gi...@apache.org>.
liuxiran commented on a change in pull request #1918:
URL: https://github.com/apache/apisix-dashboard/pull/1918#discussion_r642172470



##########
File path: web/cypress/integration/route/search-route.spec.js
##########
@@ -126,9 +135,35 @@ context('Create and Search Route', () => {
     cy.contains(data.test2).should('not.exist');
   });
 
+  it('should search the route with path', function () {
+    cy.visit('/');
+    cy.contains('Route').click();
+    // full match
+    cy.get(selector.pathSearch).type(data.uris1);
+    cy.contains('Search').click();
+    cy.contains(data.uris1).should('contain', data.uris1);
+    cy.contains(data.uris0).should('not.exist');
+    cy.contains(data.uris2).should('not.exist');
+    // partial match
+    cy.reload();

Review comment:
       Hi @iamayushdas , may I ask why we need `cy.reload` here? It seems that we don't need to reload the page, If we only want to change the path for search param, shall we use e.g. `selector.clear().type('new path')`?




-- 
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

[GitHub] [apisix-dashboard] starsz commented on pull request #1918: feat: added route path search

Posted by GitBox <gi...@apache.org>.
starsz commented on pull request #1918:
URL: https://github.com/apache/apisix-dashboard/pull/1918#issuecomment-852041085


   > @starsz can i have your review here?
   
   Sorry. I am not familiar with the FE.


-- 
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

[GitHub] [apisix-dashboard] iamayushdas commented on a change in pull request #1918: feat: added route path search

Posted by GitBox <gi...@apache.org>.
iamayushdas commented on a change in pull request #1918:
URL: https://github.com/apache/apisix-dashboard/pull/1918#discussion_r642589697



##########
File path: web/cypress/integration/route/search-route.spec.js
##########
@@ -126,9 +135,35 @@ context('Create and Search Route', () => {
     cy.contains(data.test2).should('not.exist');
   });
 
+  it('should search the route with path', function () {
+    cy.visit('/');
+    cy.contains('Route').click();
+    // full match
+    cy.get(selector.pathSearch).type(data.uris1);
+    cy.contains('Search').click();
+    cy.contains(data.uris1).should('contain', data.uris1);
+    cy.contains(data.uris0).should('not.exist');
+    cy.contains(data.uris2).should('not.exist');
+    // partial match
+    cy.reload();

Review comment:
       okay, Do you want me to give another PR?




-- 
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

[GitHub] [apisix-dashboard] iamayushdas commented on pull request #1918: feat: added route path search

Posted by GitBox <gi...@apache.org>.
iamayushdas commented on pull request #1918:
URL: https://github.com/apache/apisix-dashboard/pull/1918#issuecomment-850754509


   @nic-chen @liuxiran can i have a review on this


-- 
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

[GitHub] [apisix-dashboard] iamayushdas commented on pull request #1918: feat: added route path search

Posted by GitBox <gi...@apache.org>.
iamayushdas commented on pull request #1918:
URL: https://github.com/apache/apisix-dashboard/pull/1918#issuecomment-847847993


   writing the fe e2e test,
   


-- 
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

[GitHub] [apisix-dashboard] netlify[bot] edited a comment on pull request #1918: feat: added route path search

Posted by GitBox <gi...@apache.org>.
netlify[bot] edited a comment on pull request #1918:
URL: https://github.com/apache/apisix-dashboard/pull/1918#issuecomment-847827497


   :heavy_check_mark: Deploy Preview for *apisix-dashboard* ready!
   
   
   :hammer: Explore the source changes: 39baf1cd34db1b3e68fb5e9d33e97c7be36cb285
   
   :mag: Inspect the deploy log: [https://app.netlify.com/sites/apisix-dashboard/deploys/60b59a10217ef800083058b3](https://app.netlify.com/sites/apisix-dashboard/deploys/60b59a10217ef800083058b3?utm_source=github&utm_campaign=bot_dl)
   
   :sunglasses: Browse the preview: [https://deploy-preview-1918--apisix-dashboard.netlify.app/](https://deploy-preview-1918--apisix-dashboard.netlify.app/?utm_source=github&utm_campaign=bot_dp)
   


-- 
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