Rate testing continued#1670
Conversation
…to rate_testing
|
@aman04raza-stack Thank you for this PR. Issue #1648 indicates that @Johnsonjahnifah is also working on this. Please let me know if that is incorrect. The PR description states the CLA has been signed by all contributors. However, OED records do not have one for any of you. Thus, I would like you to:
Once this is done, this PR can be formally reviewed. Please let me know if you have any questions/thoughts. Having said that, I quickly looked at this PR, PR #1605 & issue #1648. From what I saw, there are comments in the original PR that are not yet addressed in this PR. OED would prefer if all comments are addressed so this PR covered everything wanted. My hope is that none of them are difficult to address. Please let me know any thoughts you have or if you need any help, |
huss
left a comment
There was a problem hiding this comment.
@aman04raza-stack & @Johnsonjahnifah Thank you for this contribution and the update. I have closed the old PR since this now has all the changes needed in comments on that PR. The only item I noticed was this PR seems fairly out of date with the development branch. I want to recheck with it being current so would you be able to do that (it looks like a clean merge without conflicts according to this PR on GitHub)? Then I'll do one last pass but I think this is ready to accept. Please let me know if you have any questions/thoughts.
Summary
This PR continues the work started in PR #1605 by @Zach-O-Bates and
collaborators, addressing issue #1648. The original commit history from
PR #1605 is preserved on this branch.
Changes made in this PR
app.use('api/login', login)(missing leading slash). This meant real requests to
/api/loginneverreached the login handler and returned a 404, even though the rate limiter
for that route was mounted correctly at
/api/login. Restored toapp.use('/api/login', login).node:testimport inrateTest.js.Fixes #1648
I have read the [contributing](https://openenergydashboard.org/developer/pr/) ideas
Limitations
Per the TODO left in rateTest.js, rate limit tests for other routes
(3D graphics, raw export) still need to be written. This PR only covers
the login rate limit test and the bug fix found while reviewing it.