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

[LLSC-16] Add basic user model #2

Merged
merged 15 commits into from
Oct 28, 2024
15 changes: 14 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -297,4 +297,17 @@ git push -f
- PRs can contain multiple commits, they do not need to be squashed together before merging as long as each commit is atomic. Our repo is configured to only allow squash commits to `main` so the entire PR will appear as 1 commit on `main`, but the individual commits are preserved when viewing the PR.

## Secrets
Secrets are stored in the Environment Variable [file](https://www.notion.so/uwblueprintexecs/Environment-Variables-11910f3fb1dc80e4bc67d35c3d65d073?pvs=4) within the LLSC notion.
Secrets are stored in the Environment Variable [file](https://www.notion.so/uwblueprintexecs/Environment-Variables-11910f3fb1dc80e4bc67d35c3d65d073?pvs=4) within the LLSC notion.

## Migrations (mirrors [backend README](./backend/README.md))

We use Alembic for database schema migrations. We mainly use migration scripts to keep track of the incremental and in theory revertible changes that have occurred on the database. But, we don't need to rely on this to build the datebase itself, as `Base.metadata.create_all(bind=engine)` achieves that based on the current models. To create a new migration, run the following command after adding or editing models in `backend/app/models.py`:
```bash
cd backend
pdm run alembic revision --autogenerate -m "<migration message>"
```

To apply the migration, run the following command:
```bash
pdm run alembic upgrade head
```
74 changes: 74 additions & 0 deletions backend/README.md
Original file line number Diff line number Diff line change
@@ -1 +1,75 @@
# llsc-backend

## Setup (mirrors [base README](../README.md#setup))
- Install pdm (this is a global installation, so location doesn't matter)
On macOS:
```bash
brew install pdm
```
Otherwise, feel free to follow install instructions [here](https://pdm-project.org/latest/#installation)

You will then need to go into each directory individually to install dependencies.

FastAPI backend
```bash
cd backend
pdm install
```

To run the backend server locally (recommended for development), run the following command:
```bash
cd backend
pdm run dev
```

To check if the database has been started up, type the following:
```bash
docker ps | grep llsc_db
```
This checks the list of docker containers and searchs for the container name `llsc_db`


## Formatting and Linting (mirrors [formatting in base README](../README.md#formatting-and-linting))

### Ruff

We use Ruff for code linting and formatting in the backend. To check for and automatically fix linting issues:

```bash
cd backend
pdm run ruff check --fix .
```

To format the code:
```bash
cd backend
pdm run ruff format .
```


## Adding a new model
When adding a new model, make sure to add it to `app/models/__init__.py` so that the migration script can pick it up when autogenerating the new migration.

In `app/models/__init__.py`, add the new model like so:
```python
from .Base import Base
...
from .<new_model_name> import <new_model_name>

__all__ = ["Base", ... , "<new_model_name>"]
```
Then run the steps found in the [Migrations](#migrations) section to create a new migration.


## Migrations

We use Alembic for database schema migrations. We mainly use migration scripts to keep track of the incremental and in theory revertible changes that have occurred on the database. To create a new migration, run the following command after adding or editing models in `backend/app/models.py`:
```bash
cd backend
pdm run alembic revision --autogenerate -m "<migration message>"
```

To apply the migration, run the following command:
```bash
pdm run alembic upgrade head
```
11 changes: 6 additions & 5 deletions backend/alembic.ini
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,8 @@ version_path_separator = os # Use os.pathsep. Default configuration used for ne
# are written from script.py.mako
# output_encoding = utf-8

sqlalchemy.url = driver://user:pass@localhost/dbname
# Updated in env.py using the POSTGRES_DATABASE_URL environment variable
# sqlalchemy.url =


[post_write_hooks]
Expand All @@ -76,10 +77,10 @@ sqlalchemy.url = driver://user:pass@localhost/dbname
# black.options = -l 79 REVISION_SCRIPT_FILENAME

# lint with attempts to fix using "ruff" - use the exec runner, execute a binary
# hooks = ruff
# ruff.type = exec
# ruff.executable = %(here)s/.venv/bin/ruff
# ruff.options = --fix REVISION_SCRIPT_FILENAME
hooks = ruff
ruff.type = exec
ruff.executable = %(here)s/.venv/bin/ruff
ruff.options = check --fix REVISION_SCRIPT_FILENAME

# Logging configuration
[loggers]
Expand Down
4 changes: 4 additions & 0 deletions backend/app/models/Base.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
from sqlalchemy.orm import registry

mapper_registry = registry()
Base = mapper_registry.generate_base()
9 changes: 9 additions & 0 deletions backend/app/models/Role.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
from sqlalchemy import Column, Integer, String

from .Base import Base


Copy link
Collaborator

@Mayank808 Mayank808 Oct 17, 2024

Choose a reason for hiding this comment

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

Can we add just a few comments above table definitions, giving a brief description of what the table is used for. Ie role tables will be like entitlements table housing all possible user roles on the system. Documentation on variables/relations if we think that's important could also be added but might be overkill.

class Role(Base):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we add in roles right now?

I added a migration with this in my ticket since we need the roles in the db to assign them to users. I can keep it there, or if you think it belongs in this PR that would work too.

    op.bulk_insert(
        sa.table('roles',
            sa.Column('id', sa.Integer(), nullable=False),
            sa.Column('name', sa.String(length=80), nullable=False),
        ),
        [
            {'id': 1, 'name': 'participant'},
            {'id': 2, 'name': 'volunteer'},
            {'id': 3, 'name': 'admin'},
        ]
    )
    

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think I'll move it to this PR, given that @emma-x1 can just work off this branch.

Copy link
Collaborator

Choose a reason for hiding this comment

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

In that case, can you please add in an auth_id field as well? It's needed for referencing the Firebase UID associated with a user in our db. I was keeping it in my ticket since that's where I initialized Firebase auth, but it's probably good to add it in here if @emma-x1 is working off this branch.

FIeld:

auth_id = Column(String, nullable=True) # Firebase Auth ID

Migration:

def upgrade() -> None:
    # ### commands auto generated by Alembic - please adjust! ###
    op.add_column('users', sa.Column('auth_id', sa.String(), nullable=True))
    # ### end Alembic commands ###


def downgrade() -> None:
    # ### commands auto generated by Alembic - please adjust! ###
    op.drop_column('users', 'auth_id')
    # ### end Alembic commands ###

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Should we make nullable=False when Firebase auth is implemented?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yep, since every user should have a Firebase UID. We can just add it in as nullable=False right now too, should be fine.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think with the new edge cases you mentioned, we should keep this. From what I understand, a participant can have different types (caregiver, patient) so we can maybe assign multiple roles to that user to handle those cases. It also seems safer since stuff does seem to change around a bit whenever we talk to the npo

__tablename__ = "roles"
id = Column(Integer, primary_key=True)
name = Column(String(80), nullable=False)
19 changes: 19 additions & 0 deletions backend/app/models/User.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import uuid

from sqlalchemy import Column, ForeignKey, Integer, String
from sqlalchemy.dialects.postgresql import UUID
from sqlalchemy.orm import relationship

from .Base import Base


class User(Base):
__tablename__ = "users"
id = Column(UUID(as_uuid=True), primary_key=True, default=uuid.uuid4)
first_name = Column(String(80), nullable=False)
last_name = Column(String(80), nullable=False)
email = Column(String(120), unique=True, nullable=False)
role_id = Column(Integer, ForeignKey("roles.id"), nullable=False)
auth_id = Column(String, nullable=False)

role = relationship("Role")
27 changes: 13 additions & 14 deletions backend/app/models/__init__.py
Original file line number Diff line number Diff line change
@@ -1,18 +1,17 @@
from sqlalchemy import SQLAlchemy
from alembic import command
from alembic.config import Config

db = SQLAlchemy()
# Make sure all models are here to reflect all current models
# when autogenerating new migration
from .Base import Base
Copy link
Collaborator

Choose a reason for hiding this comment

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

``

Suggested change
from .Base import Base

Copy link
Collaborator Author

@mslwang mslwang Oct 17, 2024

Choose a reason for hiding this comment

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

We need that for Base.metadata.create_all(bind=engine) and in backend/app/migrations/env.py

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Note: removing Base.metadata.create_all(bind=engine) so that we rely only on migrations to create tables.

from .Role import Role
from .User import User

# Used to avoid import errors for the models
__all__ = ["Base", "User", "Role"]

def init_app(app):
app.app_context().push()
db.init_app(app)

erase_db_and_sync = app.config["TESTING"]

if erase_db_and_sync:
# drop tables
db.reflect()
db.drop_all()

# recreate tables
db.create_all()
def run_migrations():
alembic_cfg = Config("alembic.ini")
# Emulates `alembic upgrade head` to migrate up to latest revision
command.upgrade(alembic_cfg, "head")
20 changes: 19 additions & 1 deletion backend/app/server.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,28 @@
import logging
from contextlib import asynccontextmanager
from typing import Union

from dotenv import load_dotenv
from fastapi import FastAPI

from . import models

load_dotenv()
app = FastAPI()

log = logging.getLogger("uvicorn")


@asynccontextmanager
async def lifespan(_: FastAPI):
log.info("Starting up...")
models.run_migrations()
yield
log.info("Shutting down...")


# Source: https://stackoverflow.com/questions/77170361/
# running-alembic-migrations-on-fastapi-startup
app = FastAPI(lifespan=lifespan)


@app.get("/")
Expand Down
13 changes: 11 additions & 2 deletions backend/migrations/env.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,14 @@
import os
from logging.config import fileConfig

from alembic import context
from dotenv import load_dotenv
from sqlalchemy import engine_from_config, pool

from app.models import Base

load_dotenv()

# this is the Alembic Config object, which provides
# access to the values within the .ini file in use.
config = context.config
Expand All @@ -16,12 +22,14 @@
# for 'autogenerate' support
# from myapp import mymodel
# target_metadata = mymodel.Base.metadata
target_metadata = None

target_metadata = Base.metadata

# other values from the config, defined by the needs of env.py,
# can be acquired:
# my_important_option = config.get_main_option("my_important_option")
# ... etc.
config.set_main_option("sqlalchemy.url", os.environ["POSTGRES_DATABASE_URL"])


def run_migrations_offline() -> None:
Expand Down Expand Up @@ -55,8 +63,9 @@ def run_migrations_online() -> None:
and associate a connection with the context.

"""
alembic_config = config.get_section(config.config_ini_section, {})
connectable = engine_from_config(
config.get_section(config.config_ini_section, {}),
alembic_config,
prefix="sqlalchemy.",
poolclass=pool.NullPool,
)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
"""create user table and roles

Revision ID: 4ba3479cb8df
Revises:
Create Date: 2024-10-03 00:41:13.800838

"""

from typing import Sequence, Union

import sqlalchemy as sa
from alembic import op

# revision identifiers, used by Alembic.
revision: str = "4ba3479cb8df"
down_revision: Union[str, None] = None
branch_labels: Union[str, Sequence[str], None] = None
depends_on: Union[str, Sequence[str], None] = None


def upgrade() -> None:
# ### commands auto generated by Alembic - please adjust! ###
op.create_table(
"roles",
sa.Column("id", sa.Integer(), nullable=False),
sa.Column("name", sa.String(length=80), nullable=False),
sa.PrimaryKeyConstraint("id"),
)
op.create_table(
"users",
sa.Column("id", sa.String(), nullable=False),
sa.Column("first_name", sa.String(length=80), nullable=False),
sa.Column("last_name", sa.String(length=80), nullable=False),
sa.Column("email", sa.String(length=120), nullable=False),
sa.Column("role_id", sa.Integer(), nullable=False),
sa.ForeignKeyConstraint(
["role_id"],
["roles.id"],
),
sa.PrimaryKeyConstraint("id"),
sa.UniqueConstraint("email"),
)
# ### end Alembic commands ###


def downgrade() -> None:
# ### commands auto generated by Alembic - please adjust! ###
op.drop_table("users")
op.drop_table("roles")
# ### end Alembic commands ###
37 changes: 37 additions & 0 deletions backend/migrations/versions/59bb2488a76b_insert_roles.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
"""insert roles

Revision ID: 59bb2488a76b
Revises: 4ba3479cb8df
Create Date: 2024-10-16 16:55:42.324525

"""

from typing import Sequence, Union

import sqlalchemy as sa
from alembic import op

# revision identifiers, used by Alembic.
revision: str = "59bb2488a76b"
down_revision: Union[str, None] = "4ba3479cb8df"
branch_labels: Union[str, Sequence[str], None] = None
depends_on: Union[str, Sequence[str], None] = None


def upgrade() -> None:
op.bulk_insert(
sa.table(
"roles",
sa.Column("id", sa.Integer(), nullable=False),
sa.Column("name", sa.String(length=80), nullable=False),
),
[
{"id": 1, "name": "participant"},
{"id": 2, "name": "volunteer"},
{"id": 3, "name": "admin"},
],
)


def downgrade() -> None:
op.execute("DELETE FROM roles WHERE id IN (1, 2, 3)")
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
"""Add auth_id to User model

Revision ID: 79de0b981dd8
Revises: 59bb2488a76b
Create Date: 2024-10-16 17:06:45.820859

"""

from typing import Sequence, Union

import sqlalchemy as sa
from alembic import op

# revision identifiers, used by Alembic.
revision: str = "79de0b981dd8"
down_revision: Union[str, None] = "59bb2488a76b"
branch_labels: Union[str, Sequence[str], None] = None
depends_on: Union[str, Sequence[str], None] = None


def upgrade() -> None:
# ### commands auto generated by Alembic - please adjust! ###
op.add_column("users", sa.Column("auth_id", sa.String(), nullable=False))
# ### end Alembic commands ###


def downgrade() -> None:
# ### commands auto generated by Alembic - please adjust! ###
op.drop_column("users", "auth_id")
# ### end Alembic commands ###
Loading