Small login bugs #1788
Small login bugs #1788
Conversation
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1788 +/- ##
==========================================
+ Coverage 79.56% 79.58% +0.01%
==========================================
Files 517 517
Lines 40737 40737
==========================================
+ Hits 32414 32421 +7
+ Misses 8323 8316 -7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| @@ -64,7 +64,7 @@ | |||
| <v-row> | |||
| <v-btn | |||
| type="submit" | |||
| @click.prevent="verifyPassword" | |||
| @click.prevent="() => verifyPassword()" | |||
There was a problem hiding this comment.
How does this work since verifyPassword takes 2 parameters?
There was a problem hiding this comment.
All js function arguments default to undefined if you don't pass them in. So for when it's called from here, the ||= operator on line 163 will set token, and the ! operator on line 181 will negate undefined to set this.showAlert to true.
This line could be written as @click.prevent="() => verifyPassword(password, false)" (or even verifyPassword(null, false)) but it's unnecessary to be explicit
| // Valid relative redirect URL | ||
| window.location = decodeURI(redirect) | ||
| } else { | ||
| window.location = '/' | ||
| } | ||
| }, | ||
| verifyPassword: function () { | ||
| verifyPassword: function (token, noAlert) { |
There was a problem hiding this comment.
Is this ever called with noAlert set to false?
There was a problem hiding this comment.
It's called with undefined as discussed above
| this.showAlert = false | ||
| Api.post('/openc3-api/auth/verify', { | ||
| data: { | ||
| token: this.password, | ||
| token, |
There was a problem hiding this comment.
data: { token }, is equivalent to data: { token: token }, but reads cleaner imo
Couple minor issues that have been bugging me:
?redirect=query param,redirect.startsWithwould error (redirect is undefined) and trigger the catch block that erroneously alerted them that they entered an incorrect passwordThe TODO I left in AppNav is a combination of those two issues. It's not a regression, it's just something that I'm not sure how to fix at the moment. I think AppNav's redirect logic needs to be somewhere else