Skip to content

Add onboarding support#15492

Merged
balloob merged 4 commits intodevfrom
onboarding
Jul 17, 2018
Merged

Add onboarding support#15492
balloob merged 4 commits intodevfrom
onboarding

Conversation

@balloob
Copy link
Copy Markdown
Member

@balloob balloob commented Jul 16, 2018

Description:

Add an onboarding component.

Will only be activated if the new user system is active. Works together with home-assistant/frontend#1452

Frontend: home-assistant/frontend#1452

Related issue (if applicable): fixes #15198

Example entry for configuration.yaml (if applicable):

frontend:

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass

If the code does not interact with devices:

  • Tests have been added to verify that the new code works.

@callback
def _async_is_done(self):
"""Return if this step is done."""
print(self.step, self._data)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Remember to remove this before merging.

if self._async_is_done():
return self.json_message('User step already done', 403)

provider = _async_get_hass_provider(hass)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe in the future we can allow the first user coming from other provider by adding setup_shema to the provider.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We could but I don't know if I would want to . If people want to configure their own auth providers, surely they will skip onboarding?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Then we need to check if owner already created in async_is_onboarded()

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It's not a use case I am currently worried about.

Copy link
Copy Markdown
Contributor

@awarecan awarecan Jul 17, 2018

Choose a reason for hiding this comment

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

I am thinking the migration problem for current 'new auth' user, they have the need to skip onboarding user step.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

That's a good point. If an owner exists I'll mark the user step as done.

})
resp2 = client.post('/api/onboarding/users', json={
'name': 'Test 1',
'username': '1-user',
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe add a scenario to use different username to make sure the error not caused by duplicate username

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good catch. That was my initial intention but somehow forgot to change 1 -> 2. Fixed.

if not hass.auth.active:
return True

return hass.data.get(DOMAIN, True)
Copy link
Copy Markdown
Contributor

@awarecan awarecan Jul 17, 2018

Choose a reason for hiding this comment

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

Should check if owner already created

if hass.auth.has_owner():
    return True

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We do those checks inside the async_setup. We're not checking explicitly for owner, but instead check if the user step is done. There can be many steps in the future.

@balloob balloob merged commit b0a3207 into dev Jul 17, 2018
@ghost ghost removed the in progress label Jul 17, 2018
@balloob balloob deleted the onboarding branch July 17, 2018 08:49
balloob added a commit that referenced this pull request Jul 17, 2018
* Add onboarding support

* Lint

* Address comments

* Mark user step as done if owner user already created
@balloob balloob mentioned this pull request Jul 20, 2018
@home-assistant home-assistant locked and limited conversation to collaborators Dec 10, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add onboarding flow to frontend to create initial user

4 participants