655 Add User Roles and Basic User Data to Db

Joshua-Douglas commented 1 month ago

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

Video of new Sign In User Initials

Visuals after changes are applied

https://github.com/hackforla/HomeUniteUs/assets/22138019/9bdf0326-269e-432b-be1f-335f64605367 ![image](Paste_Your_Image_Link_Here_After_Attaching_Files)

paulespinosa commented 1 month ago

(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://github.com/kdeldycke/awesome-falsehood

paulespinosa commented 1 month ago

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

Joshua-Douglas commented 1 month ago

(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:

Last names should not be required. I updated the database model to only require first names.
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 commented 1 month ago

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 commented 1 month ago

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

paulespinosa commented 1 month ago

@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?

hackforla / HomeUniteUs