Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ParseInt for validateCode #29

Closed
wants to merge 1 commit into from
Closed

ParseInt for validateCode #29

wants to merge 1 commit into from

Conversation

evantobin
Copy link

Parse validateCode as an integer. We could make the UI take in an integer, but then the user is presented with a number selector which isn't ideal. Easiest to keep it as a string in the UI and just parse it as an int on the backend.

♻️ Current situation

The Homebridge UI puts the verification code in the config as a string which causes an authentication error on august's servers. Changing it to an int works, but then you have to manually update the config.

💡 Proposed solution

Add parseInt before the code is sent to the server. This way both integer and string verification codes will work right with august's servers.

⚙️ Release Notes

Plugin now works correctly with string-based verification code configs.

Testing

N/A

Parse validateCode as an integer. We could make the UI take in an integer, but then the user is presented with a number selector which isn't ideal. Easiest to keep it as a string in the UI and just parse it as an int on the backend.
@donavanbecker donavanbecker changed the base branch from latest to beta-1.0.6 December 8, 2022 05:00
@donavanbecker
Copy link
Owner

@evantobin, I have re-based this to the beta-1.0.6 branch. Can you resolve the conflicts? or close this PR and create a new one based on the beta-1.0.6 branch?

@evantobin
Copy link
Author

I'll create a new one on the beta branch. Github by default only forked the latest branch.

@evantobin evantobin closed this Dec 8, 2022
@donavanbecker
Copy link
Owner

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants