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.

Sign up for GitHub

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

Jump to bottom

Add config flow to iCloud #28968

Merged
merged 15 commits into from Dec 9, 2019
Merged

Add config flow to iCloud #28968

merged 15 commits into from Dec 9, 2019

Conversation

Quentame
Copy link
Member

@Quentame Quentame commented Nov 22, 2019 •
edited

Breaking Change:

The iCloud component leaves the device_tracker platform to become an integration.

From:

device_tracker:
  - platform: icloud
    username: __email__
    password: __password__

To:

icloud:
  - username: __email__
    password: __password__

Here are all the services :
SERVICE_ICLOUD_PLAY_SOUND = "play_sound"
SERVICE_ICLOUD_DISPLAY_MESSAGE = "display_message"
SERVICE_ICLOUD_LOST_DEVICE = "lost_device"
SERVICE_ICLOUD_UPDATE = "update"

Some services are renamed :
'icloud_lost_device' --> 'lost_device'
'icloud_update' --> 'update'

'icloud_reset' is removed

The iCloud cookie folder moved from [HA_dir]/icloud to [HA_dir]/.storage/icloud.

Description:

Dramatically improve the iCloud integration addition UX with config flow, translated steps (will do),
and list selection while choosing the trusted device (instead of a number).

It also adds documentation to services, and will fix issues.

Following the draft PR #24053 and crowded PR #26590

PR doing :

  • add config flow + tests
  • fix existing services
  • add play_sound & display_message services
  • document services
  • can use devices with the same name
  • prepare to add sensor platform (battery for devices)

Related issue (if applicable):
fixes #13312
fixes #20195
fixes #28826

Pull request with documentation for home-assistant.io : home-assistant/home-assistant.io#11259

Example entry for configuration.yaml (if applicable):

icloud:
  - username: __email__
    password: __password__

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • The manifest file has all fields filled out correctly. Update and include derived files by running python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt by running python3 -m script.gen_requirements_all.
  • Untested files have been added to .coveragerc.

@project-bot project-bot bot added this to Needs review in Dev Nov 22, 2019
This was referenced Nov 22, 2019
Merged
Closed
@balloob balloob mentioned this pull request Nov 26, 2019
Merged
4 tasks
@MartinHjelmare MartinHjelmare moved this from Needs review to Review in progress in Dev Nov 26, 2019
@Quentame Quentame mentioned this pull request Nov 27, 2019
Merged
8 tasks
@Quentame Quentame force-pushed the icloud/config-flow branch 4 times, most recently from 74c3fdd to 8df4d76 Compare December 2, 2019 21:45
@Quentame
Copy link
Member Author

Quentame commented Dec 2, 2019

I've updated the description to follow the added breaking changes : the location of the iCloud cookie folder + the deletion of the reset service

@MartinHjelmare
Copy link
Member

Do the tests need updating to not do I/O?

@Quentame
Copy link
Member Author

Quentame commented Dec 3, 2019

@MartinHjelmare : Yep, I'm working on it right now. Hope to make it work for 0.103.0 😄

@Quentame
Copy link
Member Author

Quentame commented Dec 3, 2019

If you wanna help, I have this for now :

Capture d’écran 2019-12-02 à 23 17 53

Capture d’écran 2019-12-02 à 23 17 03

@MartinHjelmare
Copy link
Member

Why not patch the service class as we talked about above?

@Quentame
Copy link
Member Author

Quentame commented Dec 3, 2019

Ok, just working in my computer, pushing

@Quentame
Copy link
Member Author

Quentame commented Dec 3, 2019

@MartinHjelmare : What do you think of e2d040eb06514eb8ac369f549495b906b508e326 ?

@Quentame Quentame force-pushed the icloud/config-flow branch 2 times, most recently from 33ac49d to fc02053 Compare December 6, 2019 12:15
@Quentame
Copy link
Member Author

Quentame commented Dec 9, 2019

What is the state of the PR now ?

MartinHjelmare
MartinHjelmare reviewed Dec 9, 2019
View reviewed changes
tests/components/icloud/test_config_flow.py Outdated Show resolved Hide resolved
tests/components/icloud/test_config_flow.py Outdated Show resolved Hide resolved
tests/components/icloud/test_config_flow.py Outdated Show resolved Hide resolved
@MartinHjelmare
Copy link
Member

There seems to be one line in the config flow that isn't covered by tests:
https://codecov.io/gh/home-assistant/home-assistant/compare/27530be46fd7550edba2b1725ea6b790b74328cc...fc020539714d60c5a57c64c9ba0a2f6d33da6acd/diff#D4-213

@Quentame
iCloud: setup ConfigFlow and prepare for more platforms
f32850c 
- add config flow + tests
- fix existing services
- add play_sound & display_message services
- document services
- can use devices with the same name
- prepare to add sensor platform
@Quentame
Review : not copy account conf
0fa1941
@Quentame
Review: Safer test patch
cb0334a
@Quentame
Review: remove reset_account
73f819d
@Quentame
Review: Use executor_job while IO
d3c5622
@Quentame
Review: Use executor_job while IO 2
5826318
@Quentame
Review: use hass.helpers.storage.Store()
d0711e2
@Quentame
Review: no IO in tests
1be9cad
@Quentame
Remove reset from services.yaml
0666a9d
@Quentame
Review: remove authenticate.return_value = Mock()
af22f11
@Quentame
Review: do not initialize the api with the mocked service
0859212
@Quentame
isort
438e340
@Quentame
Review: @MartinHjelmare Test config flow with all steps
f4d0370
@Quentame
Review: Fix failed tests names
0ecf822
@Quentame
Codevov: Add one missing test
33079eb
@Quentame
Copy link
Member Author

Quentame commented Dec 9, 2019

Got the 100% code coverage 😉

MartinHjelmare
MartinHjelmare approved these changes Dec 9, 2019
View reviewed changes
Copy link
Member

@MartinHjelmare MartinHjelmare left a comment

Choose a reason for hiding this comment

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

Looks good!

Dev automation moved this from Review in progress to Reviewer approved Dec 9, 2019
@MartinHjelmare MartinHjelmare merged commit c804f8f into home-assistant:dev Dec 9, 2019
Dev automation moved this from Reviewer approved to Done Dec 9, 2019
@Quentame Quentame deleted the icloud/config-flow branch December 9, 2019 16:33
@frenck frenck mentioned this pull request Dec 9, 2019
Merged
4 tasks
@Quentame Quentame mentioned this pull request Dec 10, 2019
Merged
8 tasks
@lock lock bot locked and limited conversation to collaborators Dec 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@balloob balloob balloob left review comments

@MartinHjelmare MartinHjelmare MartinHjelmare approved these changes

Assignees
No one assigned
Labels
breaking-change cla-signed has-tests integration: icloud
Projects
Dev
  
Done
Milestone
No milestone
4 participants
@Quentame @MartinHjelmare @balloob @homeassistant