655 Add User Roles and Basic User Data to Db

[Joshua-Douglas]

Closes #655.

What changes did you make?

Added user roles (Guest, Host, Coordinator, Admin) and updated the user model to include first, middle, and last name. PR #661 made an update that passes the first and last name to the backend, so I was also able to update the frontend app to begin receiving and storing the user names as well (video below).

The user name initials are now displayed on the user avatar menu after logging in, and the user's name is accessible from all authenticated components.

Making this change required refactoring our alembic migration scripts, the API test project, and the API itself. The following detailed changes were required:

• Add a database version check during API startup. If the database is not at the head revision then an error will be thrown to notify the developer that an alembic migration is required. The logic relies on a static version number that should be incremented.
• Integrate the alembic database migration process into our test client setup fixture. Our backend tests will run against a properly versioned database, to ensure the test database has all pre-populated values (such as the roles table).
• Upgrade marshmallow-sqlalchemy to the latest release. I needed a bug fix from the latest release.
• Update signUpHost and signUpCoordinator endpoints to 1) assign a role to their created users and 2) require the user first and last name.
• Update the user and signIn endpoints to return the user first and last name as part of the user data.
• Update data model diagram within api/openapi_server/models/README.md, and include instructions on how to update the database model. Turns out there are a lot of steps!
• Add User model (de)serialization json schema, complete with a test suite. We could add some name validation here (e.g. don't allow spaces in names) and filter the json requests through the schema, but it didn't seem worth while.
• Remove the Host model object. This is now replaced by the User model with a Host role.
• Remove the create_host endpoint. This is now completely replaced by signUpHost
• Modify the base alembic migration script, 3ceec084158f_.py, to work with unversioned local databases. This will allow other devs to run alembic upgrade head even if they've been working off of an API-generated database.
• Add migration script to introduce user roles and user names. All existing users will be assigned the guest role and will be given the names "unknown".
• Update run-tests.yml to include a workflow_dispatch trigger to allow manual triggering.
• Add cypress:run:mock and cypress:run:nomock configurations to allow more easily running the full e2e test suites.

Rationale behind the changes?

Before this PR there was no mechanism for determining user type and there was no mechanism for storing basic user information. We can now differentiate between guests, hosts, and coordinators. This will allow us to begin creating different views for each user type, and to begin restricting portions of the API that require elevated access.

This PR does not implement any data model relationships between user types, and it does not implement role-specific user data.

Testing done for these changes

• Added a suite of backend alembic migration tests that will test all current and future migration scripts. These tests cycle through the upgrades and downgrade scripts and check for common database migration errors. These tests are assisted by the new dev dependency, pytest-alembic.
• Modified the e2e sign up tests to include a 'no mocking' mode. I ran the signup tests against the real API to confirm our signup still works.
• Tested each of the modified endpoints
• Tested the new schema objects, and their validation

What did you learn or can share that is new?(optional)

I learned how to write and test alembic database migrations. The steps are documented in api/openapi_server/models/README.md.

New Data Model

classDiagram
class alembic_version{
   *VARCHAR<32> version_num NOT NULL
}
class housing_program{
   *INTEGER id NOT NULL
   VARCHAR program_name NOT NULL
   INTEGER service_provider NOT NULL
}
class housing_program_service_provider{
   *INTEGER id NOT NULL
   VARCHAR provider_name NOT NULL
}
class role{
   *INTEGER id NOT NULL
   VARCHAR name NOT NULL
}
class user{
   *INTEGER id NOT NULL
   VARCHAR email NOT NULL
   VARCHAR<255> firstName NOT NULL
   VARCHAR<255> lastName NOT NULL
   VARCHAR<255> middleName
   INTEGER role_id NOT NULL
}
housing_program_service_provider "1" -- "0..n" housing_program
role "1" -- "0..n" user

Loading

Video of new Sign In User Initials

Visuals after changes are applied

UserNameDemo.mp4

[erikguntner]

Everything looks great! Thanks for the detailed steps for updating the data model. Approving. Only question is regarding the workflow_dispatch property in the test workflow.

[paulespinosa]

This means there can be duplicate role names. The id column isn't strictly necessary. The name column can be the primary key.

[Joshua-Douglas]

Correct, the id isn't strictly necessary, but it does provide us the flexibility to rename the roles at any time. I think being able to easily rename our roles is worth storing 4-10 extra integers in the db.

To your point about duplicate names, I added a safeguard against this in 6ad02dd by adding the unique=True constraint to the role.name field.

[paulespinosa]

Using on update cascade can also be considered easy when defining a table. What other reasons are there to use a surrogate key?

[Joshua-Douglas]

Using surrogate keys like an id field offers several advantages beyond the ease of updates provided by ON UPDATE CASCADE. Surrogate keys are unique and stable, providing an abstraction layer that decouples database structure from business logic, thus reducing the need for changes due to business modifications. They enhance performance with quicker indexing and simplify relational management by avoiding the complexities and potential performance issues of cascading updates. Additionally, surrogate keys maintain data integrity by minimizing risks associated with data duplication and erroneous updates, making them ideal for robust, scalable database architectures.

[paulespinosa]

Is that a chatgpt response? 😄 Do these reasons apply to this data model?

[paulespinosa]

• (e.g. don't allow spaces in names)

Some people have spaces in their names. Here's some resources to understand more about names:

• https://designsystem.digital.gov/patterns/create-a-user-profile/name/
• https://shinesolutions.com/2018/01/08/falsehoods-programmers-believe-about-names-with-examples/

https://github.com/kdeldycke/awesome-falsehood

[paulespinosa]

Looking good. Is there a PM created issue this can relate back to?

[Joshua-Douglas]

• (e.g. don't allow spaces in names)

Some people have spaces in their names. Here's some resources to understand more about names:

• https://designsystem.digital.gov/patterns/create-a-user-profile/name/
• https://shinesolutions.com/2018/01/08/falsehoods-programmers-believe-about-names-with-examples/

https://github.com/kdeldycke/awesome-falsehood

That's a great illustration of why I did not validate the names. Great sources, thanks!

After reading through them I realized two things and corrected both in 6ad02dd:

1. Last names should not be required. I updated the database model to only require first names.
2. First names should be at least one character in length. The data model now validates that the name is at least one non-space character.

[Joshua-Douglas]

Hey @erikguntner and @paulespinosa,

Thank you so much for taking the time to do a detailed review. I've updated the PR in response to your comments.

Since I had to modify the migration script, it is probably best for you to downgrade and re-upgrade if your local db is on the current head.

Here are all the changes from the review:

• Make the user last name optional, since some cultures only use one name. I modified both the frontend and backend to accommodate this change.
• Add validation to the user first name, to require at least 1 non-space character in length.
• Auto-increment the database head version by reading from the alembic configuration file. Before this change the version had to be manually incremented, but this change removes a manual step from the db upgrade process.

I also noticed that #602 broke our e2e tests, so I submitted a fix for this in 74f861e.

[Joshua-Douglas]

@paulespinosa, I saw your question but I don't see any requested changes. Does this PR require more work?

[paulespinosa]

@paulespinosa, I saw your question but I don't see any requested changes. Does this PR require more work?

Code seems fine. @sanya301, @rpradheap this feature adds fields to the sign up page (sorry for the confusion) the user initials as shown in the video above. Look good to you?