Page MenuHomePhabricator

Initial user registration workflow
ClosedPublic

Authored by chris on Aug 12 2019, 5:40 PM.

Details

Summary

This is a first stab at getting a workflow set up to configure the initial administrative user account on a fresh install. Currently, the whole "register an account" functionality is missing and it'll just fatal, but I'm planning to build that out pretty soon here ๐Ÿ˜ฌ

Test Plan

Ran locally and saw the application crash and burn when it realized I didn't have an account. Then added the appropriate redirect and saw things get redirected to the approriate /auth/register route.

Diff Detail

Repository
rSIM Simplex
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

chris requested review of this revision.Aug 12 2019, 5:40 PM
chris planned changes to this revision.
chris created this revision.
brooke added inline comments.Aug 14 2019, 8:08 PM
simplex/simplex.py
18โ€“19

So this works for you! I need to try one more time myself.

19

Curious why not just import register_bp? Do we need other things from register.py?

chris updated this revision to Diff 79.Aug 18 2019, 2:26 PM

Super basic check to see if at least 1 admin account has been registered yet

chris retitled this revision from WIP Registration workflow to Initial user registration workflow.Aug 18 2019, 2:28 PM
chris edited the summary of this revision. (Show Details)
chris edited the test plan for this revision. (Show Details)
chris updated this revision to Diff 80.Aug 18 2019, 2:45 PM
chris edited the summary of this revision. (Show Details)

Redirect to the appropriate registration route

chris edited the test plan for this revision. (Show Details)Aug 18 2019, 2:46 PM
chris added a comment.Aug 18 2019, 2:55 PM

This implementation is also wrong atm. Properly, I'll need to use some flask wrapper function for this since __init__ by definition can't actually return anything (the implication being that, short of raising an exception, we can't interrupt the normal course of action and say, "Return page Y instead of page X"). I'll fix this in a subsequent diff.

chris updated this revision to Diff 83.Aug 18 2019, 4:39 PM

working redirect

chris added a comment.Aug 18 2019, 4:48 PM

My rough plan following this diff is basically:

  • Switch up how we're doing page rendering since that's kind of murky current in terms of how we distinguish between 'build a sanitized HTML element' and 'render a specific page'. That can be cleaned up pretty easily and it's probably best to do that now while the codebase is still small
  • Build out the UI for the initial user registration flow. I think this will be the same deal as for a generic user registration, just with a 'this user is an admin' flag toggled
  • Build out some infrastructure around password management and credential checking. This'll be a "fun" learning adventure for me.
  • Build out the ORM piece so we can actually register a user and store their info to the database
  • Profit???
brooke added inline comments.Aug 18 2019, 7:01 PM
simplex/__init__.py
1

where did you use APP_ROOT?

simplex/application/config/setup_check.py
46

So for the first pass, since self.issues is empty, we will jump to line 47. If there is any issues, we will start appending issues?

81

priority is always 1? what is priority for?

124

Questions about this class.

  1. where did you use self.checks?
  2. what does nux stand for?
  3. not going to do anything under check_configuration_set?
139

Does this return a list of dictionary?

simplex/infrastructure/storage/management/storage_management_api.py
20

why do you have empty line on 21 and 29? What makes you decide whether the name should be upper or lower case?

68

do we want to set echo or echo_pool? pool_timeout?
how do you decide the number for max_overflow?

chris added inline comments.Aug 18 2019, 7:17 PM
simplex/__init__.py
1

Uhhh... Somewhere way downstream, I forget. You can use the "Pattern Search" option if you open up the repo in Diffusion to find where all it's used if you want. Not relevant to this revision, though

simplex/application/config/setup_check.py
46

Almost. Basically, this is checking if all of the required variables to be able to establish a database connection have been set in the config files. If any are missing, it'll be impossible to check if the database is reachable because we don't have all the necessary info, so there's no point in even running this check until those other issues get resolved.

81

Haha I'm not really sure. I mean, right now it isn't used. But eventually I'd like to have some mechanism to determine "is this issue a stop-the-world, everything's on fire" type of deal, or just a "hey, maybe get around to this when you can" sort of thing.

124
  1. I don't currently, but check out the databaseSetupCheck class above for how it will be used

NUX is New User Experience. Basically "do we need training wheels mode" or not.

  1. I will eventually, just not right now
139

Yup!

simplex/infrastructure/storage/management/storage_management_api.py
20

Empty lines are just to break things up a bit, no real rationale otherwise as long as it's PEP8-compliant.

SHOUTY are constants; normal cased are ones that can be overwritten

68

No idea! I don't even know if I have pooling set up properly. It's on my to-do list to figure out.

brooke accepted this revision.Sep 1 2019, 1:55 PM
This revision is now accepted and ready to land.Sep 1 2019, 1:55 PM
This revision was automatically updated to reflect the committed changes.