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

Tweak provisioning interaction with OS User capability #11352

Merged
merged 2 commits into from
Oct 11, 2023

Conversation

rtibbles
Copy link
Member

@rtibbles rtibbles commented Oct 4, 2023

Summary

  • Updates device provisioning serializer to allow associating a created superuser with the OSUser of the user provisioning the device
  • Updates the setup wizard to always create a superuser and send the information to the backend, except when the get os user capability is active, and the on my own setup flow is being used

References

No extant issue, noticed in the course of fixing #133

Reviewer guidance

  • Test using the Android App that should now have the required OS User integration built in
  • Use the group learning workflow
  • Repeat for each of these permutations:
    • create a new facility
    • import a facility
    • create a new user on a remote facility
    • import a user on a remote facility
  • Finish setup wizard, confirm that the username you are now logged in as matches what you just created
  • Logout
  • Exit the app and reopen.
  • Confirm that you are again logged in as the created user in the facility (and that it hasn't created a new user)

Testing checklist

  • 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

  • PR has the correct target branch and milestone
  • 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

@github-actions github-actions bot added DEV: backend Python, databases, networking, filesystem... APP: Setup Wizard Re: Setup Wizard (facility import, superuser creation, settings, etc.) SIZE: small labels Oct 4, 2023
Copy link
Member

@nucleogenesis nucleogenesis left a comment

Choose a reason for hiding this comment

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

The updates in serializers are great and straightforward.

I have one question/suggestion regarding the changes to the wizardMachine

Copy link
Member

@nucleogenesis nucleogenesis left a comment

Choose a reason for hiding this comment

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

Code LGTM! Will leave final approval to @pcenov after manual testing

@pcenov
Copy link
Member

pcenov commented Oct 5, 2023

Hi @rtibbles, I confirm that the points listed above are working as described. I just have the following note:

After closing and relaunching the app I am always signed in as the super admin user regardless of the fact that there are other users on the device. This is especially confusing if I have been signed in as any of the other users prior to closing the app. Shouldn't this rule for automatically signing in the user be applied only if that is the only user on the device?

Admin.is.always.signed.in.mp4

@rtibbles
Copy link
Member Author

@pcenov I have opened a follow up issue for what you are describing here.

Copy link
Member

@nucleogenesis nucleogenesis left a comment

Choose a reason for hiding this comment

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

Good to go!

@rtibbles rtibbles merged commit 28bfafa into learningequality:release-v0.16.x Oct 11, 2023
@rtibbles rtibbles deleted the lod_osuser branch October 11, 2023 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
APP: Setup Wizard Re: Setup Wizard (facility import, superuser creation, settings, etc.) DEV: backend Python, databases, networking, filesystem... SIZE: small
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants