kolibri icon indicating copy to clipboard operation
kolibri copied to clipboard

SetupWizard - Partial provisioning, On My Own Flow

Open nucleogenesis opened this issue 3 years ago • 1 comments

Summary

  • Prepares SetupWizard flow so that it can be partially provisioned (ie, a Facility can have been created/imported without the DeviceSettings.is_provisioned flag being set to True) -- this covers the edge case of a user not completing the process of importing users one by one and returning to it without ever saying "Okay I'm done onboarding"
  • The FacilityImportResource / FacilityImportViewSet were both set to use the new is_provisioned flag when given in a request - it defaults to False honestly not sure what default to use, as is evidenced by it defaulting to True in device settings
  • The same two were also changed so that createsuperuser will create a facility with a given facility_name.

Outstanding issues (follow-up things)

The following is what, at a high level, remains for the Setup Wizard. I will create a new set of issues and add them to the backlog following up on this when approved and before merging.

  • Only the "On My Own" flow is setup to work properly here, other flows need work still (high effort)
  • The "Step X of Y" logic is not yet implemented (medium effort)
  • Wiring up bits around the user's device name and such needs to be implemented when creating facilities and devices (medium effort)
  • The Interstitial needs to be designed (low effort)

Reviewer guidance

  • Can you go from start to finish through the On My Own flow?
  • Thoughts on what the sane default for "is_provisioned" should be end to end?
  • Any concerns with creating the facility inside the "createsuperuser" method?
  • Does your language selection during Onboarding get reflected properly? (refresh on the page that says "-- INTERSTITIAL --")

Testing checklist

  • [x] Contributor has fully tested the PR manually
  • [ ] If there are any front-end changes, before/after screenshots are included
  • [ ] Critical user journeys are covered by Gherkin stories
  • [ ] Critical and brittle code paths are covered by unit tests

PR process

  • [x] PR has the correct target branch and milestone
  • [x] PR has 'needs review' or 'work-in-progress' label
  • [ ] If PR is ready for review, a reviewer has been added. (Don't use 'Assignees')
  • [ ] If this is an important user-facing change, PR or related issue has a 'changelog' label
  • [ ] If this includes an internal dependency change, a link to the diff is provided

Reviewer checklist

  • Automated test coverage is satisfactory
  • PR is fully functional
  • PR has been tested for accessibility regressions
  • External dependency files were updated if necessary (yarn and pip)
  • Documentation is updated
  • Contributor is in AUTHORS.md

nucleogenesis avatar Sep 20 '22 20:09 nucleogenesis

@jredrejo I'll address that in a follow-up issue if that is alright -- if you're okay with that we can merge this and I'll draw up some new issues today for the follow-up on the finishing up the setup wizard altogether

nucleogenesis avatar Oct 13 '22 15:10 nucleogenesis

@jredrejo I'll address that in a follow-up issue if that is alright -- if you're okay with that we can merge this and I'll draw up some new issues today for the follow-up on the finishing up the setup wizard altogether

The thing is that without that setting, there are features that can't be tested. It's fine if people testing them know that it will require some shell work to do it to test the "Change facility" flow, as it is only meant to be working in devices having that setting.

jredrejo avatar Oct 13 '22 15:10 jredrejo

From the QA team point of view, we can only answer these 2 questions in the Review guidance:

  1. Can you go from start to finish through the On My Own flow?

    Not sure if I could call it from-start-to-finish with the -- INTERSTITIAL -- hickup in between, but if it's going to be addressed soon OK... 🤔

    The bigger concern is that, once we sign in again with the user created in this On My Own flow, what we see is still the full admin user with access to Coach, Facility and the rest of Device page options. We chatted with @marcellamaki to confirm that the way we understood the Figma design for this flow aligns: this onboarding should end up with the user with the same type of permissions as on the current Learn-only devices, irregardless they are installing on the app or doing the onboarding in the browser.

    on-my-own2

    Is this also something that will be addressed in the follow-up PRs, or are we misunderstanding something?

  2. Does your language selection during Onboarding get reflected properly?

    It seems OK, some strings are missing translations at this point, but otherwise the experience is correct.

radinamatic avatar Oct 18 '22 18:10 radinamatic

Apologies for late reply @nucleogenesis

There seems to be some progress, but the main concern exposed above still stands: user created with this flow has full superadmin device permissions.

  1. No -- INTERSTITIAL -- just the spinner which does not get to lead anywhere (I waited for 5 minutes and stopped the recording)

    on-my-own-second-take

  2. After manually going to the sign in page I can still see the normal superadmin UI

https://user-images.githubusercontent.com/1457929/197795862-fd0c96a0-40e7-49db-8251-51ab7a556b02.mp4

I can approve this PR if working of it will make your next task easier, but this is not the state we were expecting the on-my-own flow to be, nor we can use it for testing other features like Merge accounts and similar.

radinamatic avatar Oct 25 '22 14:10 radinamatic

@radinamatic let's merge this one and I'll follow up on all of the above in a separate PR

nucleogenesis avatar Oct 26 '22 17:10 nucleogenesis