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

Allow providing master_token during integration setup #723

Merged
merged 13 commits into from
May 31, 2023
Merged

Allow providing master_token during integration setup #723

merged 13 commits into from
May 31, 2023

Conversation

Brephlas
Copy link
Contributor

Small changed so that the Integration now asks for master_token in the set-up process and is therefore able to access the Google API again
Tested with my own environment/account and this was working fine.

Integration Set-up process:
ha-google-home

Test timer on my office's google home device:
ha-google-home-timers

Might be a hacky solution but I tried to keep the usual set up process with an optional input field. Couldn't test this as I was getting the bad authentication error as well.

Brephlas added 3 commits May 29, 2023 10:37
Integration now asks for master_token in the set-up process and is therefore able to access the Google API again
The initial version did not allow the usual set up process without a master_token

Also mention this entire issue in the README
@github-actions github-actions bot added the translation Translations label May 29, 2023
Copy link
Owner

@leikoilja leikoilja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thanks for finding the time for fixing it :)
A small comment on how to make it possibly better. I would love to test it as well before we merge (will try to do it later today/tomorrow), since i have both username/password and master token working on my setup

README.md Outdated Show resolved Hide resolved
Add reference to glocaltokens

Co-authored-by: Ilja Leiko <[email protected]>
Brephlas added 2 commits May 29, 2023 16:59
Make all input fields optional and check for configuration method based on the provided fields
Copy link
Owner

@leikoilja leikoilja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome stuff! 💪

README.md Outdated Show resolved Hide resolved
custom_components/google_home/config_flow.py Outdated Show resolved Hide resolved
custom_components/google_home/config_flow.py Outdated Show resolved Hide resolved
custom_components/google_home/translations/en.json Outdated Show resolved Hide resolved
custom_components/google_home/config_flow.py Outdated Show resolved Hide resolved
@leikoilja leikoilja changed the title Fix/master_token Allow providing master_token during integration setup May 30, 2023
Brephlas and others added 6 commits May 30, 2023 17:41
Co-authored-by: Ilja Leiko <[email protected]>
simplify getting user inputs

Co-authored-by: Ilja Leiko <[email protected]>
Add translations to all supported languages. However, these are mostly done using DeepL
Add missing translation for ca
Add a test which tries to get an access_token during the setup process
@Brephlas
Copy link
Contributor Author

Updated the PR with a validity check for the master token as well as translations (using DeepL + Google Translator)✌️

Copy link
Owner

@leikoilja leikoilja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Boom, amazing job @Brephlas 🚀
Thanks for contributing and fixing such a crucial part!

Let's merge it in and ask peps to help test it out using the master branch build. Once it's verified to work - we release a new version 🔥

@leikoilja leikoilja added feature New feature or request minor Minor changes. Used for release bump labels May 31, 2023
@leikoilja
Copy link
Owner

@Brephlas, since pre-commit hooks were failing on your last commit, i have pushed a minor refactoring to fix linting and typing issues. Please see the commit i just pushed if you agree with the changes :) I would also highly appreciate if you could pull these changes to your setup and verify that it still all works as expected before we merge :)
Thank you

@Brephlas
Copy link
Contributor Author

Thanks for refactoring. Just did the tests on my setup and everything works as expected :)

@leikoilja leikoilja merged commit 6cc7994 into leikoilja:master May 31, 2023
@leikoilja
Copy link
Owner

Closes #339

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request minor Minor changes. Used for release bump translation Translations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants