Improve Remote failure error messaging#24
Conversation
| guard let password = notification["otp"] as? String else { | ||
| return .failure(NotificationValidationError.missingOTP) | ||
| } | ||
|
|
There was a problem hiding this comment.
The time the notification was delivered from Nightscout will be presented in a user message for expired OTPs.
8c563c9 to
60a47b1
Compare
| @@ -89,30 +89,32 @@ public class OTPManager { | |||
| } | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
The validation of the OTP was refactored in this class to allow for better error messaging.
| guard password.count == passwordDigitCount else { | ||
| throw OTPValidationError.invalidFormat(otp: password) | ||
| } | ||
|
|
There was a problem hiding this comment.
This will distinguish between a bad password and an expired. That should be helpful for user troubleshooting.
| @@ -155,28 +157,62 @@ public class OTPManager { | |||
| return Token(name: secretKeyName, issuer: issuerName, generator: generator) | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
Refactored this method getLastPasswordsAscending for reuse.
| @@ -13,99 +13,131 @@ import XCTest | |||
| class OTPManagerTestCase: XCTestCase { | |||
There was a problem hiding this comment.
Some test updates and also I refactored the test data modeling to be more flexible as these tests grow.
60a47b1 to
219faf1
Compare
| } | ||
|
|
||
| } | ||
|
|
There was a problem hiding this comment.
Several of these methods in this class, and tests, use these structures to wrap up OTPs and their associated valid periods.
|
@ps2 I've since updated the "OTP" verbiage to use "Password" instead. Just let me know if there is something else preferred. |
See associated Loop PR for high-level info LoopKit/Loop#1867
These changes will update the NigthscoutService to return more detailed descriptions for display.