Fix adding api routes to servers #1381
Conversation
greenape
added
bug
|
Test summary
Run details
View run in Cypress Dashboard ➡️ This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
Codecov Report
@@ Coverage Diff @@
## master #1381 +/- ##
==========================================
+ Coverage 94.98% 95.02% +0.03%
==========================================
Files 157 157
Lines 7551 7551
Branches 704 704
==========================================
+ Hits 7172 7175 +3
+ Misses 269 268 -1
+ Partials 110 108 -2
Continue to review full report at Codecov.
|
| .get("#api-exp") | ||
| .click(); | ||
| cy.get("#permissions").should("have.attr", "data-indeterminate", "true"); | ||
| cy.get("[data-permission-id='" + route_name + "_get_result']") |
There was a problem hiding this comment.
Worth creating a helper function for this check (with a meaningful name) to avoid the code repetition?
| .click({ | ||
| force: true | ||
| }); | ||
| cy.contains(route_name).should("not.exist"); |
There was a problem hiding this comment.
Just for my understanding: what does this check do, exactly? Why should the route_name element not exist any more?
There was a problem hiding this comment.
This is a convoluted way to click the dustbin icon next to it, which should delete it.
| @@ -206,7 +214,7 @@ class PermissionDetails extends React.Component { | |||
| isPermissionChecked, | |||
| permissionIndeterminate | |||
| } = this.state; | |||
| const { classes, onClick, permitted } = this.props; | |||
There was a problem hiding this comment.
Out of interest: was the presence of onClick a bug, or did you have to remove it because of any changes in this PR?
There was a problem hiding this comment.
Not a bug, just an unused var.
Closes #1373
I have:
Description
Fixes not being able to turn on a newly added api route for an existing server by correctly setting the permitted options on servers to all options, and adds a test to cover this case.