diff --git a/.env.example b/.env.example index 9336ade..b2ab0b9 100644 --- a/.env.example +++ b/.env.example @@ -14,6 +14,11 @@ DB_NAME= DATABASE_URL= # 3. Neither set, in development: server/app.db (SQLite). +# Apply database migrations when the server starts. Set on staging and production +# only, never on PR previews (they share the staging database). Locally, run +# `flask --app main db upgrade` yourself instead. +RUN_MIGRATIONS= + # Canvas developer key for this app. Locally, use the UC Berkeley sandbox key: # https://ucberkeleysandbox.instructure.com/ CANVAS_SERVER_URL=https://ucberkeleysandbox.instructure.com/ diff --git a/Dockerfile b/Dockerfile index 86f8855..3901735 100644 --- a/Dockerfile +++ b/Dockerfile @@ -62,7 +62,8 @@ EXPOSE 8080 HEALTHCHECK --interval=30s --timeout=5s --start-period=20s --retries=3 \ CMD python -c 'import os, urllib.request; urllib.request.urlopen("http://127.0.0.1:" + os.environ.get("PORT", "8080") + "/health")' +# With RUN_MIGRATIONS=true (set on staging and production, never on PR previews, +# which share the staging database), apply database migrations before starting. # `exec` keeps gunicorn as PID 1 so it gets SIGTERM directly on redeploy. -# One worker process with threads, as in seating: Cloud Run's default instance -# is 1 vCPU, and a single worker also keeps db.create_all() at startup from racing. -CMD ["sh", "-c", "exec gunicorn -b 0.0.0.0:${PORT:-8080} main:app --workers ${WEB_CONCURRENCY:-1} --threads ${GUNICORN_THREADS:-8} --timeout ${GUNICORN_TIMEOUT:-120} --access-logfile -"] +# One worker process with threads, as in seating: Cloud Run's default instance is 1 vCPU. +CMD ["sh", "-c", "if [ \"$RUN_MIGRATIONS\" = true ]; then flask --app main db upgrade || exit 1; fi; exec gunicorn -b 0.0.0.0:${PORT:-8080} main:app --workers ${WEB_CONCURRENCY:-1} --threads ${GUNICORN_THREADS:-8} --timeout ${GUNICORN_TIMEOUT:-120} --access-logfile -"] diff --git a/README.md b/README.md index 894c2f8..243ed08 100644 --- a/README.md +++ b/README.md @@ -13,8 +13,9 @@ Requires Python 3.14 and Node 24 (`mise install` picks both up from `mise.toml`) pip install -r server/requirements.txt ``` 3. Install the frontend dependencies with `yarn install`. -4. Run both with `yarn dev`, or separately: the backend with `python3 main.py` from `server/` (port 8000) and the frontend with `yarn start` (the Vite dev server on port 3000, which forwards API calls, the login routes and `/offerings` to port 8000). -5. Run the backend tests from the repository root with `python -m unittest discover -s server -p "test_*.py"`. +4. Create or update the local database with `flask --app main db upgrade` from `server/`. +5. Run both with `yarn dev`, or separately: the backend with `python3 main.py` from `server/` (port 8000) and the frontend with `yarn start` (the Vite dev server on port 3000, which forwards API calls, the login routes and `/offerings` to port 8000). +6. Run the backend tests from the repository root with `python -m unittest discover -s server -p "test_*.py"`. ## Authentication @@ -50,6 +51,17 @@ python3 import_locally.py --course 157 --type enrollment --file test_csvs/lab_en 4. Open `localhost:3000/offerings/157/`; you should see the example sections. +## Database Migrations + +The schema is managed with Flask-Migrate (Alembic); migrations live in `server/migrations/versions`. To change the schema, edit `server/models.py`, then from `server/`: + +``` +flask --app main db migrate -m "describe the change" # generates a migration; review and edit it +flask --app main db upgrade # applies it to your local database +``` + +Commit the migration with the model change. Deployments with `RUN_MIGRATIONS=true` (staging and production) run `flask db upgrade` when they start; PR previews never do, because they share the staging database. + ## Import Sections from GCP 1. Download the sections database to obtain a `.sql` file. Relevant Links [1](https://cloud.google.com/sql/docs/mysql/import-export/import-export-sql) and [2](https://cloud.google.com/storage/docs/downloading-objects). 2. Likely the export uses MySQL syntax, so update it to SQLite. diff --git a/server/import_sheet.py b/server/import_sheet.py index c60b965..68587e5 100644 --- a/server/import_sheet.py +++ b/server/import_sheet.py @@ -8,7 +8,7 @@ from offering import get_course from google_sheets import read_spreadsheet from dataclasses import asdict, dataclass, field, fields -from models import Failure, Section, User, db, user_section +from models import Failure, Section, db, find_or_add_person, offering_member, user_section # Sample Spreadsheet Link: https://docs.google.com/spreadsheets/d/1WL7gXiBxPe6aUFBKfEOVS6iY27ITy9mARupLoJX-ouE/edit?usp=sharing pst = ZoneInfo("US/Pacific") @@ -95,16 +95,10 @@ def import_sections(data: Iterator): raise Failure(f"Unknown boolean value: {can_self_enroll}") can_self_enroll = can_self_enroll == "true" - staff = User.query.filter_by( - email=email, - course=get_course(), - ).one_or_none() or User( - email=email, - name=name, - is_staff=True, - is_admin=False, - course=get_course(), - ) + staff = offering_member(email=email) + if staff is None: + staff = find_or_add_person(email, name) + staff.set_role(get_course(), is_staff=True) section_type = row[header.type_index] location = row[header.location_index] @@ -156,21 +150,14 @@ def import_enrollment(data: Iterator): start_time = parse_time_string(row[header.day_index], row[header.start_index]) section_type = row[header.type_index] - student = User.query.filter_by( - email=student_email, - course=get_course() - ).one_or_none() or User( - email=student_email, - name=student_name, - is_staff=False, - is_admin=False, - course=get_course() - ) + student = offering_member(email=student_email) + if student is None: + student = find_or_add_person(student_email, student_name) + student.set_role(get_course(), is_staff=False) - staff = User.query.filter_by( - email=staff_email, - course=get_course(), - ).one() + staff = offering_member(email=staff_email) + if staff is None: + raise Failure(f"Unable to import enrollment data for {student_email}! {staff_email} isn't on staff.") # Assumes that each section can be uniquely identified by the below parameters section = Section.query.filter_by( diff --git a/server/login.py b/server/login.py index e1aa535..906ce40 100644 --- a/server/login.py +++ b/server/login.py @@ -8,8 +8,8 @@ from markupsafe import escape import canvas_service -from offering import current_offering, get_course, in_offering -from models import Account, User, db +from offering import current_offering, in_offering +from models import User, db AFTER_LOGIN_KEY = "after_login" @@ -22,8 +22,8 @@ def _safe_next(target): def complete_login(token: dict): - """Create or update the signed-in Account from a Canvas OAuth token response.""" - canvas_user_id = token["user"]["id"] + """Create or update the signed-in User from a Canvas OAuth token response.""" + canvas_user_id = str(token["user"]["id"]) access_token = token["access_token"] profile = canvas_service.get_profile(canvas_user_id, access_token) email = profile.get("primary_email") or profile.get("login_id") @@ -31,50 +31,50 @@ def complete_login(token: dict): courses = canvas_service.get_user_courses(canvas_user_id, access_token) override_id = current_app.config.get("ADMIN_OVERRIDE_CANVAS_COURSE_ID") - account = Account.query.filter_by(canvas_id=str(canvas_user_id)).one_or_none() - if account is None: - account = Account(canvas_id=str(canvas_user_id)) - db.session.add(account) - account.email = email - account.name = name - account.canvas_courses = courses - account.is_global_admin = override_id is not None and str(override_id) in courses + # People added by staff or imported from a spreadsheet have a User (found + # by email) before they ever log in. + user = ( + User.query.filter_by(canvas_id=canvas_user_id).one_or_none() + or User.query.filter_by(email=email).one_or_none() + ) + if user is None: + user = User(email=email) + db.session.add(user) + user.canvas_id = canvas_user_id + user.email = email + user.name = name + user.canvas_courses = courses + user.is_global_admin = override_id is not None and str(override_id) in courses db.session.commit() - login_user(account, remember=True) + login_user(user, remember=True) session.permanent = True # Keep only a short-lived access token, and only for staff, who need it to # look up students; reauthorize when it expires. session.pop("canvas_access_token", None) session.pop("canvas_token_expires_at", None) - if account.is_global_admin or any(c["is_staff"] for c in courses.values()): + if user.is_global_admin or any(c["is_staff"] for c in courses.values()): session["canvas_access_token"] = access_token session["canvas_token_expires_at"] = time.time() + token.get("expires_in", 3600) -def course_user(account: Account): - """The account's User row in the current course, created on first visit. +def sync_offering_role(user: User) -> bool: + """Bring the user's role in the current offering in line with Canvas. - Staff and admin flags follow the account's Canvas roles from its last - sign-in. Returns None if the account isn't enrolled in the course. + Returns False if the user isn't enrolled in the offering's Canvas course. """ - roles = account.roles_in(current_offering().canvas_id) + if user.is_global_admin: + return True + offering = current_offering() + roles = user.canvas_roles_in(offering.canvas_id) if roles is None: - return None + return False is_staff, is_admin = roles - course = get_course() - user = User.query.filter_by(email=account.email, course=course).one_or_none() - if user is None: - user = User(email=account.email, name=account.name, is_staff=is_staff, - is_admin=is_admin, course=course) - db.session.add(user) - elif (user.name, user.is_staff, user.is_admin) == (account.name, is_staff, is_admin): - return user - user.name = account.name - user.is_staff = is_staff - user.is_admin = is_admin - db.session.commit() - return user + if (offering.key in user.staff_offerings, offering.key in user.admin_offerings, + user.is_member_of(offering.key)) != (is_staff, is_admin, True): + user.set_role(offering.key, is_staff=is_staff, is_admin=is_admin) + db.session.commit() + return True def create_login_client(app: flask.Flask): @@ -98,12 +98,9 @@ def create_login_client(app: flask.Flask): @login_manager.user_loader def load_user(session_id): - account_id = Account.parse_session_id(session_id) - account = db.session.get(Account, account_id) if account_id else None - if account is None or not in_offering(): - return account - user = course_user(account) - if user is None: + user_id = User.parse_session_id(session_id) + user = db.session.get(User, user_id) if user_id else None + if user is not None and in_offering() and not sync_offering_role(user): abort(403, "You aren't enrolled in this course on bCourses. If you just " "enrolled, sign out and sign in again.") return user diff --git a/server/main.py b/server/main.py index 670155e..97ce78c 100644 --- a/server/main.py +++ b/server/main.py @@ -1,6 +1,7 @@ import os from flask import Flask +from flask_migrate import Migrate from jinja2 import ChoiceLoader, FileSystemLoader from werkzeug.middleware.proxy_fix import ProxyFix @@ -24,6 +25,8 @@ load_config(app) db.init_app(app) +# Schema changes are Alembic migrations in server/migrations; `flask db upgrade` applies them. +Migrate(app, db, directory=os.path.join(os.path.dirname(os.path.abspath(__file__)), "migrations")) create_state_client(app) create_login_client(app) create_offering_pages(app) @@ -34,9 +37,5 @@ def health(): return "ok" -with app.app_context(): - db.create_all() - - if __name__ == "__main__": app.run(host="127.0.0.1", port=8000, debug=True) diff --git a/server/migrations/README b/server/migrations/README new file mode 100644 index 0000000..0e04844 --- /dev/null +++ b/server/migrations/README @@ -0,0 +1 @@ +Single-database configuration for Flask. diff --git a/server/migrations/alembic.ini b/server/migrations/alembic.ini new file mode 100644 index 0000000..ec9d45c --- /dev/null +++ b/server/migrations/alembic.ini @@ -0,0 +1,50 @@ +# A generic, single database configuration. + +[alembic] +# template used to generate migration files +# file_template = %%(rev)s_%%(slug)s + +# set to 'true' to run the environment during +# the 'revision' command, regardless of autogenerate +# revision_environment = false + + +# Logging configuration +[loggers] +keys = root,sqlalchemy,alembic,flask_migrate + +[handlers] +keys = console + +[formatters] +keys = generic + +[logger_root] +level = WARN +handlers = console +qualname = + +[logger_sqlalchemy] +level = WARN +handlers = +qualname = sqlalchemy.engine + +[logger_alembic] +level = INFO +handlers = +qualname = alembic + +[logger_flask_migrate] +level = INFO +handlers = +qualname = flask_migrate + +[handler_console] +class = StreamHandler +args = (sys.stderr,) +level = NOTSET +formatter = generic + +[formatter_generic] +format = %(levelname)-5.5s [%(name)s] %(message)s +datefmt = %H:%M:%S diff --git a/server/migrations/env.py b/server/migrations/env.py new file mode 100644 index 0000000..559948e --- /dev/null +++ b/server/migrations/env.py @@ -0,0 +1,113 @@ +import logging +from logging.config import fileConfig + +from flask import current_app + +from alembic import context +from sqlalchemy import text + +# this is the Alembic Config object, which provides +# access to the values within the .ini file in use. +config = context.config + +# Interpret the config file for Python logging. +# This line sets up loggers basically. +fileConfig(config.config_file_name) +logger = logging.getLogger('alembic.env') + + +def get_engine(): + return current_app.extensions['migrate'].db.engine + + +def get_engine_url(): + try: + return get_engine().url.render_as_string(hide_password=False).replace( + '%', '%%') + except AttributeError: + return str(get_engine().url).replace('%', '%%') + + +# add your model's MetaData object here +# for 'autogenerate' support +# from myapp import mymodel +# target_metadata = mymodel.Base.metadata +config.set_main_option('sqlalchemy.url', get_engine_url()) +target_db = current_app.extensions['migrate'].db + +# 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. + + +def get_metadata(): + if hasattr(target_db, 'metadatas'): + return target_db.metadatas[None] + return target_db.metadata + + +def run_migrations_offline(): + """Run migrations in 'offline' mode. + + This configures the context with just a URL + and not an Engine, though an Engine is acceptable + here as well. By skipping the Engine creation + we don't even need a DBAPI to be available. + + Calls to context.execute() here emit the given string to the + script output. + + """ + url = config.get_main_option("sqlalchemy.url") + context.configure( + url=url, target_metadata=get_metadata(), literal_binds=True + ) + + with context.begin_transaction(): + context.run_migrations() + + +def run_migrations_online(): + """Run migrations in 'online' mode. + + In this scenario we need to create an Engine + and associate a connection with the context. + + """ + + # this callback is used to prevent an auto-migration from being generated + # when there are no changes to the schema + # reference: http://alembic.zzzcomputing.com/en/latest/cookbook.html + def process_revision_directives(context, revision, directives): + if getattr(config.cmd_opts, 'autogenerate', False): + script = directives[0] + if script.upgrade_ops.is_empty(): + directives[:] = [] + logger.info('No changes in schema detected.') + + conf_args = current_app.extensions['migrate'].configure_args + if conf_args.get("process_revision_directives") is None: + conf_args["process_revision_directives"] = process_revision_directives + + connectable = get_engine() + + with connectable.connect() as connection: + context.configure( + connection=connection, + target_metadata=get_metadata(), + **conf_args + ) + + with context.begin_transaction(): + if connection.dialect.name == "postgresql": + # Several Cloud Run instances can start at once; the first takes + # this lock and migrates, and the rest wait, then find nothing to do. + connection.execute(text("SELECT pg_advisory_xact_lock(724501)")) + context.run_migrations() + + +if context.is_offline_mode(): + run_migrations_offline() +else: + run_migrations_online() diff --git a/server/migrations/script.py.mako b/server/migrations/script.py.mako new file mode 100644 index 0000000..2c01563 --- /dev/null +++ b/server/migrations/script.py.mako @@ -0,0 +1,24 @@ +"""${message} + +Revision ID: ${up_revision} +Revises: ${down_revision | comma,n} +Create Date: ${create_date} + +""" +from alembic import op +import sqlalchemy as sa +${imports if imports else ""} + +# revision identifiers, used by Alembic. +revision = ${repr(up_revision)} +down_revision = ${repr(down_revision)} +branch_labels = ${repr(branch_labels)} +depends_on = ${repr(depends_on)} + + +def upgrade(): + ${upgrades if upgrades else "pass"} + + +def downgrade(): + ${downgrades if downgrades else "pass"} diff --git a/server/migrations/versions/0001_baseline.py b/server/migrations/versions/0001_baseline.py new file mode 100644 index 0000000..588ffe4 --- /dev/null +++ b/server/migrations/versions/0001_baseline.py @@ -0,0 +1,141 @@ +"""Baseline: the schema as of PR #14, before migrations existed. + +Revision ID: 0001_baseline +Revises: +Create Date: 2026-10-09 12:09:13.121880 + +Databases created before this (staging, whose data came from the monorepo) +already have these tables, so upgrading them only records this revision. +""" +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = '0001_baseline' +down_revision = None +branch_labels = None +depends_on = None + + +def upgrade(): + if sa.inspect(op.get_bind()).has_table("offerings"): + return + op.create_table('account', + sa.Column('id', sa.Integer(), nullable=False), + sa.Column('canvas_id', sa.String(length=255), nullable=False), + sa.Column('email', sa.String(length=255), nullable=False), + sa.Column('name', sa.String(length=255), nullable=False), + sa.Column('is_global_admin', sa.Boolean(), nullable=False), + sa.Column('canvas_courses', sa.JSON(), nullable=False), + sa.PrimaryKeyConstraint('id') + ) + with op.batch_alter_table('account', schema=None) as batch_op: + batch_op.create_index(batch_op.f('ix_account_canvas_id'), ['canvas_id'], unique=True) + batch_op.create_index(batch_op.f('ix_account_email'), ['email'], unique=False) + + op.create_table('course_config', + sa.Column('id', sa.Integer(), nullable=False), + sa.Column('course', sa.String(length=255), nullable=True), + sa.Column('can_students_join_lab', sa.Boolean(), nullable=True), + sa.Column('can_students_change_lab', sa.Boolean(), nullable=True), + sa.Column('can_tutors_change_lab', sa.Boolean(), nullable=True), + sa.Column('can_tutors_reassign_lab', sa.Boolean(), nullable=True), + sa.Column('can_students_join_disc', sa.Boolean(), nullable=True), + sa.Column('can_students_change_disc', sa.Boolean(), nullable=True), + sa.Column('can_tutors_change_disc', sa.Boolean(), nullable=True), + sa.Column('can_tutors_reassign_disc', sa.Boolean(), nullable=True), + sa.Column('can_students_join_tutoring', sa.Boolean(), nullable=True), + sa.Column('can_students_change_tutoring', sa.Boolean(), nullable=True), + sa.Column('can_tutors_change_tutoring', sa.Boolean(), nullable=True), + sa.Column('can_tutors_reassign_tutoring', sa.Boolean(), nullable=True), + sa.Column('message', sa.String(length=1024), nullable=True), + sa.PrimaryKeyConstraint('id') + ) + with op.batch_alter_table('course_config', schema=None) as batch_op: + batch_op.create_index(batch_op.f('ix_course_config_course'), ['course'], unique=False) + + op.create_table('offerings', + sa.Column('id', sa.Integer(), nullable=False), + sa.Column('canvas_id', sa.Integer(), nullable=False), + sa.Column('key', sa.String(length=255), nullable=False), + sa.Column('name', sa.String(length=255), nullable=False), + sa.Column('code', sa.String(length=255), nullable=True), + sa.Column('start_at', sa.String(length=255), nullable=True), + sa.PrimaryKeyConstraint('id'), + sa.UniqueConstraint('key') + ) + with op.batch_alter_table('offerings', schema=None) as batch_op: + batch_op.create_index(batch_op.f('ix_offerings_canvas_id'), ['canvas_id'], unique=True) + + op.create_table('user', + sa.Column('id', sa.Integer(), nullable=False), + sa.Column('course', sa.String(length=255), nullable=True), + sa.Column('email', sa.String(length=255), nullable=True), + sa.Column('name', sa.String(length=255), nullable=True), + sa.Column('is_staff', sa.Boolean(), nullable=True), + sa.Column('is_admin', sa.Boolean(), nullable=True), + sa.PrimaryKeyConstraint('id') + ) + with op.batch_alter_table('user', schema=None) as batch_op: + batch_op.create_index(batch_op.f('ix_user_course'), ['course'], unique=False) + batch_op.create_index(batch_op.f('ix_user_email'), ['email'], unique=False) + + op.create_table('section', + sa.Column('id', sa.Integer(), nullable=False), + sa.Column('course', sa.String(length=255), nullable=True), + sa.Column('description', sa.String(length=255), nullable=True), + sa.Column('capacity', sa.Integer(), nullable=True), + sa.Column('can_self_enroll', sa.Boolean(), nullable=True), + sa.Column('enrollment_code', sa.String(length=255), nullable=True), + sa.Column('staff_id', sa.Integer(), nullable=True), + sa.Column('tag_string', sa.String(length=255), nullable=False), + sa.Column('name', sa.String(length=255), nullable=True), + sa.Column('start_time', sa.Integer(), nullable=True), + sa.Column('end_time', sa.Integer(), nullable=True), + sa.Column('location', sa.String(length=255), nullable=False), + sa.Column('call_link', sa.String(length=255), nullable=True), + sa.ForeignKeyConstraint(['staff_id'], ['user.id'], ), + sa.PrimaryKeyConstraint('id') + ) + with op.batch_alter_table('section', schema=None) as batch_op: + batch_op.create_index(batch_op.f('ix_section_course'), ['course'], unique=False) + + op.create_table('session', + sa.Column('id', sa.Integer(), nullable=False), + sa.Column('course', sa.String(length=255), nullable=True), + sa.Column('start_time', sa.Integer(), nullable=True), + sa.Column('section_id', sa.Integer(), nullable=True), + sa.ForeignKeyConstraint(['section_id'], ['section.id'], ), + sa.PrimaryKeyConstraint('id') + ) + with op.batch_alter_table('session', schema=None) as batch_op: + batch_op.create_index(batch_op.f('ix_session_course'), ['course'], unique=False) + batch_op.create_index(batch_op.f('ix_session_section_id'), ['section_id'], unique=False) + + op.create_table('user_section', + sa.Column('user_id', sa.Integer(), nullable=False), + sa.Column('section_id', sa.Integer(), nullable=False), + sa.ForeignKeyConstraint(['section_id'], ['section.id'], ), + sa.ForeignKeyConstraint(['user_id'], ['user.id'], ), + sa.PrimaryKeyConstraint('user_id', 'section_id') + ) + op.create_table('attendance', + sa.Column('id', sa.Integer(), nullable=False), + sa.Column('course', sa.String(length=255), nullable=True), + sa.Column('status', sa.Enum('present', 'excused', 'absent', name='attendance_status'), nullable=True), + sa.Column('session_id', sa.Integer(), nullable=True), + sa.Column('student_id', sa.Integer(), nullable=True), + sa.ForeignKeyConstraint(['session_id'], ['session.id'], ), + sa.ForeignKeyConstraint(['student_id'], ['user.id'], ), + sa.PrimaryKeyConstraint('id') + ) + with op.batch_alter_table('attendance', schema=None) as batch_op: + batch_op.create_index(batch_op.f('ix_attendance_course'), ['course'], unique=False) + batch_op.create_index(batch_op.f('ix_attendance_session_id'), ['session_id'], unique=False) + batch_op.create_index(batch_op.f('ix_attendance_student_id'), ['student_id'], unique=False) + + +def downgrade(): + # Dropping the baseline would drop every table, including pre-existing data. + raise NotImplementedError("The baseline migration can't be downgraded.") diff --git a/server/migrations/versions/0002_single_user.py b/server/migrations/versions/0002_single_user.py new file mode 100644 index 0000000..0a4d5c0 --- /dev/null +++ b/server/migrations/versions/0002_single_user.py @@ -0,0 +1,159 @@ +"""One User per person across all offerings, like seating. + +Revision ID: 0002_single_user +Revises: 0001_baseline +Create Date: 2026-10-09 + +Before: a `user` row per person per offering (with that offering's is_staff and +is_admin), plus an `account` row per person who has logged in. +After: one `users` row per person (matched by email), with their roles in each +offering as sets of offering keys. Enrollments, attendance and section staff +are re-pointed at the merged rows. + +Also drops tables nothing reads: `account`, the old `user`, `course` (left over +from renaming offerings), and `slot`, `survey` and `user_section_junction` (from +the monorepo), along with `session.slot_id`, the monorepo's link to `slot`. +""" +from alembic import op +import sqlalchemy as sa + + +revision = "0002_single_user" +down_revision = "0001_baseline" +branch_labels = None +depends_on = None + +# Tables with a foreign key to the old `user` table, and the column holding it. +USER_REFERENCES = [("attendance", "student_id"), ("section", "staff_id"), ("user_section", "user_id")] +# Tables from the monorepo (and the pre-rename `course`) that nothing reads. +LEFTOVER_TABLES = ("course", "slot", "survey") +# Gives SQLite's unnamed foreign keys a name, so batch mode can drop them. +FK_NAMING = {"fk": "fk_%(table_name)s_%(column_0_name)s_%(referred_table_name)s"} + + +def _people(bind): + """One dict per person, merged by email, from the old user and account rows.""" + meta = sa.MetaData() + old_user = sa.Table("user", meta, autoload_with=bind) + people = {} + for row in bind.execute(sa.select(old_user).order_by(old_user.c.id)): + person = people.setdefault(row.email.lower(), { + "email": row.email, "name": row.name or row.email, "canvas_id": None, + "is_global_admin": False, "canvas_courses": {}, + "staff": set(), "admin": set(), "student": set(), "old_ids": [], + }) + person["old_ids"].append(row.id) + if row.name: + person["name"] = row.name # rows are in id order, so the newest name wins + if row.is_staff: + person["staff"].add(row.course) + else: + person["student"].add(row.course) + if row.is_admin: + person["admin"].add(row.course) + + if sa.inspect(bind).has_table("account"): + account = sa.Table("account", meta, autoload_with=bind) + for row in bind.execute(sa.select(account)): + person = people.setdefault(row.email.lower(), { + "email": row.email, "name": row.name, "staff": set(), "admin": set(), + "student": set(), "old_ids": [], + }) + # Logged-in people get their Canvas name and details. + person.update(name=row.name, canvas_id=row.canvas_id, + is_global_admin=bool(row.is_global_admin), + canvas_courses=row.canvas_courses or {}) + return people + + +def _drop_foreign_keys_to(table, referred): + fks = [fk for fk in sa.inspect(op.get_bind()).get_foreign_keys(table) + if fk["referred_table"] == referred] + if not fks: + return + with op.batch_alter_table(table, naming_convention=FK_NAMING) as batch: + for fk in fks: + name = fk["name"] or f"fk_{table}_{fk['constrained_columns'][0]}_{referred}" + batch.drop_constraint(name, type_="foreignkey") + + +def upgrade(): + bind = op.get_bind() + + users = op.create_table( + "users", + sa.Column("id", sa.Integer(), nullable=False), + sa.Column("email", sa.String(length=255), nullable=False), + sa.Column("name", sa.String(length=255), nullable=False), + sa.Column("canvas_id", sa.String(length=255), nullable=True), + sa.Column("is_global_admin", sa.Boolean(), nullable=False, server_default=sa.false()), + sa.Column("canvas_courses", sa.JSON(), nullable=False), + sa.Column("staff_offerings", sa.Text(), nullable=False, server_default=""), + sa.Column("admin_offerings", sa.Text(), nullable=False, server_default=""), + sa.Column("student_offerings", sa.Text(), nullable=False, server_default=""), + sa.PrimaryKeyConstraint("id"), + ) + op.create_index("ix_users_email", "users", ["email"], unique=True) + op.create_index("ix_users_canvas_id", "users", ["canvas_id"], unique=True) + + people = _people(bind) + for person in people.values(): + person["id"] = bind.execute( + sa.insert(users).returning(users.c.id), + { + "email": person["email"], "name": person["name"], + "canvas_id": person.get("canvas_id"), + "is_global_admin": person.get("is_global_admin", False), + "canvas_courses": person.get("canvas_courses") or {}, + "staff_offerings": ",".join(sorted(person["staff"])), + "admin_offerings": ",".join(sorted(person["admin"])), + "student_offerings": ",".join(sorted(person["student"])), + }, + ).scalar_one() + + # Map every old per-offering row to its person, then re-point references. + id_map = op.create_table( + "user_id_map", + sa.Column("old_id", sa.Integer(), primary_key=True), + sa.Column("new_id", sa.Integer(), nullable=False), + ) + mappings = [{"old_id": old_id, "new_id": person["id"]} + for person in people.values() for old_id in person["old_ids"]] + if mappings: + op.bulk_insert(id_map, mappings) + + if sa.inspect(bind).has_table("user_section_junction"): + op.drop_table("user_section_junction") + for table, column in USER_REFERENCES: + _drop_foreign_keys_to(table, "user") + # Two steps, through negative ids: new ids overlap old ones, and + # user_section's primary key is checked row by row during an UPDATE. + bind.execute(sa.text( + f"UPDATE {table} SET {column} = " + f"-(SELECT new_id FROM user_id_map WHERE old_id = {table}.{column}) " + f"WHERE {column} IS NOT NULL" + )) + bind.execute(sa.text(f"UPDATE {table} SET {column} = -{column} WHERE {column} < 0")) + with op.batch_alter_table(table) as batch: + batch.create_foreign_key(f"fk_{table}_{column}_users", "users", [column], ["id"]) + op.drop_table("user_id_map") + + # Databases from the monorepo still link sessions to the old slot table. + inspector = sa.inspect(bind) + for table in inspector.get_table_names(): + if any(fk["referred_table"] in LEFTOVER_TABLES for fk in inspector.get_foreign_keys(table)): + for referred in LEFTOVER_TABLES: + _drop_foreign_keys_to(table, referred) + if "slot_id" in {c["name"] for c in sa.inspect(bind).get_columns("session")}: + with op.batch_alter_table("session") as batch: + batch.drop_column("slot_id") + + inspector = sa.inspect(bind) + for table in ("account", "user") + LEFTOVER_TABLES: + if inspector.has_table(table): + op.drop_table(table) + + +def downgrade(): + # Per-offering rows can't be recovered once merged. + raise NotImplementedError("Merging users into one row per person can't be undone.") diff --git a/server/models.py b/server/models.py index a19f00e..52fc908 100644 --- a/server/models.py +++ b/server/models.py @@ -3,8 +3,10 @@ from typing import List from urllib.parse import quote +from flask import g from flask_login import UserMixin, current_user from flask_sqlalchemy import SQLAlchemy +from sqlalchemy import literal, types from sqlalchemy.orm import DeclarativeBase, joinedload @@ -15,9 +17,32 @@ class Base(DeclarativeBase): db = SQLAlchemy(model_class=Base) + +class StringSet(types.TypeDecorator): + """A set of strings stored as comma-separated text, as in seating.""" + + impl = types.Text + cache_ok = True + + def process_bind_param(self, value, dialect): + return ",".join(sorted(set(value or ()))) + + def process_result_value(self, value, dialect): + return set(value.split(",")) if value else set() + + +def set_contains(column, item: str): + """SQL condition: the StringSet ``column`` contains ``item``.""" + return (literal(",") + db.func.coalesce(column, "") + literal(",")).like(f"%,{item},%") + + +def current_course_key(): + """The current offering's key, or None outside an offering's pages.""" + return g.offering.key if "offering" in g else None + # Association Table for User - Section Pairs because each user can have multiple sections user_section = db.Table('user_section', - db.Column('user_id', db.Integer, db.ForeignKey('user.id'), primary_key=True), + db.Column('user_id', db.Integer, db.ForeignKey('users.id'), primary_key=True), db.Column('section_id', db.Integer, db.ForeignKey('section.id'), primary_key=True) ) @@ -29,7 +54,7 @@ class Section(db.Model): capacity: int = db.Column(db.Integer) can_self_enroll: bool = db.Column(db.Boolean) enrollment_code: str = db.Column(db.String(255), nullable=True) - staff_id: int = db.Column(db.Integer, db.ForeignKey("user.id"), nullable=True) + staff_id: int = db.Column(db.Integer, db.ForeignKey("users.id"), nullable=True) staff: "User" = db.relationship( "User", backref=db.backref("sections_taught", lazy="joined"), @@ -161,7 +186,7 @@ class Attendance(db.Model): lazy="joined", innerjoin=True, ) - student_id: int = db.Column(db.Integer, db.ForeignKey("user.id"), index=True) + student_id: int = db.Column(db.Integer, db.ForeignKey("users.id"), index=True) student: "User" = db.relationship( lambda: User, backref=db.backref("attendances"), lazy="joined", innerjoin=True ) @@ -182,22 +207,92 @@ def full_json(self): class User(db.Model, UserMixin): - # just here to make PyCharm stop complaining - def __init__(self, email: str, name: str, is_staff: bool, course: str, is_admin: bool): - # noinspection PyArgumentList - super().__init__(email=email, name=name, is_staff=is_staff, course=course, is_admin=is_admin) + """One person, across all offerings, like seating's User. + + Roles are per offering, in sets of offering keys (the value of each row's + `course` column) rather than seating's Canvas IDs, because offerings moved + from the monorepo aren't all linked to Canvas. Inside an offering's pages, + `is_staff` and `is_admin` answer for that offering. + """ + + __tablename__ = "users" id: int = db.Column(db.Integer, primary_key=True) - course: str = db.Column(db.String(255), index=True) - email: str = db.Column(db.String(255), index=True) - name: str = db.Column(db.String(255)) - is_staff: bool = db.Column(db.Boolean) - is_admin: bool = db.Column(db.Boolean) + email: str = db.Column(db.String(255), unique=True, index=True, nullable=False) + name: str = db.Column(db.String(255), nullable=False) + # Set when the person first logs in; people added by staff or imported from + # a spreadsheet may not have one. + canvas_id: str = db.Column(db.String(255), unique=True, index=True, nullable=True) + # Staff and admin in every offering (member of ADMIN_OVERRIDE_CANVAS_COURSE_ID). + is_global_admin: bool = db.Column(db.Boolean, nullable=False, default=False) + # Active Canvas courses at last login, keyed by str(canvas course id): + # {"name", "code", "start_at", "is_staff", "is_admin", "is_student"}. + canvas_courses: dict = db.Column(db.JSON, nullable=False, default=dict) + staff_offerings: set = db.Column(StringSet, nullable=False, default=set) + admin_offerings: set = db.Column(StringSet, nullable=False, default=set) + student_offerings: set = db.Column(StringSet, nullable=False, default=set) sections: List["Section"] = db.relationship( 'Section', secondary=user_section, back_populates='students', lazy='joined' ) # attendances: List[Attendance] is a backref defined on Attendance + def __init__(self, **kwargs): + # Column defaults only apply on insert; set_role needs real sets before then. + for field in ("staff_offerings", "admin_offerings", "student_offerings"): + kwargs.setdefault(field, set()) + kwargs.setdefault("canvas_courses", {}) + kwargs.setdefault("is_global_admin", False) + super().__init__(**kwargs) + + # Session ids are prefixed so cookies from before this table existed, which + # hold the id of a different table's row, can't be mistaken for a user id. + SESSION_ID_PREFIX = "user-" + + def get_id(self): + return f"{self.SESSION_ID_PREFIX}{self.id}" + + @classmethod + def parse_session_id(cls, session_id: str): + prefix, _, user_id = session_id.partition(cls.SESSION_ID_PREFIX) + return int(user_id) if not prefix and user_id.isdigit() else None + + @property + def is_staff(self) -> bool: + key = current_course_key() + return key is not None and (self.is_global_admin or key in self.staff_offerings) + + @property + def is_admin(self) -> bool: + key = current_course_key() + return key is not None and (self.is_global_admin or key in self.admin_offerings) + + def is_member_of(self, key: str) -> bool: + return key in self.staff_offerings or key in self.student_offerings + + def set_role(self, key: str, *, is_staff: bool, is_admin: bool = False): + """Make this person staff (optionally admin) or a student in an offering.""" + if is_staff: + self.staff_offerings = self.staff_offerings | {key} + self.student_offerings = self.student_offerings - {key} + else: + self.student_offerings = self.student_offerings | {key} + self.staff_offerings = self.staff_offerings - {key} + if is_admin: + self.admin_offerings = self.admin_offerings | {key} + else: + self.admin_offerings = self.admin_offerings - {key} + + def sections_in(self, key: str) -> List["Section"]: + return [section for section in self.sections if section.course == key] + + def canvas_roles_in(self, canvas_course_id: int): + """``(is_staff, is_admin)`` in a Canvas course from the last login, or + None if not enrolled there.""" + info = (self.canvas_courses or {}).get(str(canvas_course_id)) + if info is None or not (info["is_staff"] or info["is_student"]): + return None + return info["is_staff"], info["is_admin"] + @property def identity_json(self): return { @@ -230,7 +325,7 @@ def json(self): @property def full_json(self): attendances = ( - Attendance.query.filter_by(student_id=self.id) + Attendance.query.filter_by(student_id=self.id, course=current_course_key()) .options( joinedload(Attendance.session, innerjoin=True) .joinedload(Session.section) @@ -252,7 +347,7 @@ def full_json(self): @property def simple_json(self): attendances = ( - Attendance.query.filter_by(student_id=self.id) + Attendance.query.filter_by(student_id=self.id, course=current_course_key()) .options( joinedload(Attendance.session, innerjoin=True) .joinedload(Session.section) @@ -286,6 +381,22 @@ def simple_json(self): } +def offering_member(**filters): + """The person matching ``filters`` (e.g. email=...) if they're in the + current offering, otherwise None.""" + user = User.query.filter_by(**filters).one_or_none() + return user if user is not None and user.is_member_of(current_course_key()) else None + + +def find_or_add_person(email: str, name: str) -> User: + """The person with this email, created (not yet in any offering) if new.""" + user = User.query.filter_by(email=email).one_or_none() + if user is None: + user = User(email=email, name=name) + db.session.add(user) + return user + + class CourseConfig(db.Model): id: int = db.Column(db.Integer, primary_key=True) course: str = db.Column(db.String(255), index=True) @@ -356,44 +467,5 @@ def __repr__(self): return f"" -class Account(db.Model, UserMixin): - """Someone signed in with Canvas, across all of their courses. - - Flask-Login keeps the account's id in the session. Inside a course, the - login manager resolves it to that course's User row (see login.py). - """ - - id: int = db.Column(db.Integer, primary_key=True) - canvas_id: str = db.Column(db.String(255), unique=True, index=True, nullable=False) - email: str = db.Column(db.String(255), index=True, nullable=False) - name: str = db.Column(db.String(255), nullable=False) - # Staff and admin in every course (member of ADMIN_OVERRIDE_CANVAS_COURSE_ID). - is_global_admin: bool = db.Column(db.Boolean, nullable=False, default=False) - # Active Canvas courses at last sign-in, keyed by str(canvas course id): - # {"name", "code", "start_at", "is_staff", "is_admin", "is_student"}. - canvas_courses: dict = db.Column(db.JSON, nullable=False, default=dict) - - # Session ids are prefixed so cookies from before accounts existed, which - # hold a per-course User id, can't be mistaken for an account id. - SESSION_ID_PREFIX = "account-" - - def get_id(self): - return f"{self.SESSION_ID_PREFIX}{self.id}" - - @classmethod - def parse_session_id(cls, session_id: str): - prefix, _, account_id = session_id.partition(cls.SESSION_ID_PREFIX) - return int(account_id) if not prefix and account_id.isdigit() else None - - def roles_in(self, canvas_course_id: int): - """``(is_staff, is_admin)`` in the course, or None if not enrolled.""" - if self.is_global_admin: - return True, True - info = self.canvas_courses.get(str(canvas_course_id)) - if info is None or not (info["is_staff"] or info["is_student"]): - return None - return info["is_staff"], info["is_admin"] - - class Failure(Exception): pass diff --git a/server/offerings.py b/server/offerings.py index 4f9044f..b8bb6bb 100644 --- a/server/offerings.py +++ b/server/offerings.py @@ -8,24 +8,24 @@ from models import Offering, db -def _categorized_offerings(account): - """Set-up offerings the account is staff in, a student in, or otherwise +def _categorized_offerings(user): + """Set-up offerings the user is staff in, a student in, or otherwise enrolled in, like seating's offerings page.""" - canvas_ids = [int(i) for i in account.canvas_courses] + canvas_ids = [int(i) for i in user.canvas_courses] existing = Offering.query.filter(Offering.canvas_id.in_(canvas_ids)).all() staff, students, other = [], [], [] for offering in existing: - info = account.canvas_courses[str(offering.canvas_id)] + info = user.canvas_courses[str(offering.canvas_id)] # Offerings linked from the monorepo start without Canvas details. if offering.code is None: offering.name, offering.code, offering.start_at = info["name"], info["code"], info["start_at"] - if info["is_staff"] or account.is_global_admin: + if info["is_staff"] or user.is_global_admin: staff.append(offering) elif info["is_student"]: students.append(offering) else: other.append(offering) - if account.is_global_admin: + if user.is_global_admin: staff += Offering.query.filter(Offering.canvas_id.notin_(canvas_ids)).all() db.session.commit() diff --git a/server/requirements.txt b/server/requirements.txt index 87e386d..d6da3ad 100644 --- a/server/requirements.txt +++ b/server/requirements.txt @@ -3,6 +3,8 @@ canvasapi==3.6.0 cloud-sql-python-connector[pg8000]==1.22.0 Flask==3.1.3 Flask_Login==0.6.3 +Flask-Migrate==4.1.0 +alembic==1.20.0 Flask_SQLAlchemy==3.1.1 google_api_python_client==2.200.0 google-auth==2.60.0 diff --git a/server/seed.py b/server/seed.py index f2a28b8..4c97702 100644 --- a/server/seed.py +++ b/server/seed.py @@ -4,8 +4,10 @@ from zoneinfo import ZoneInfo +from flask_migrate import upgrade + from main import app -from models import Offering, Section, User, db, user_section +from models import Offering, Section, db, find_or_add_person def seed(canvas_course_id: int): @@ -13,7 +15,7 @@ def seed(canvas_course_id: int): if app.config["APP_ENV"] != "development": return with app.app_context(): - db.create_all() + upgrade() if Offering.query.filter_by(canvas_id=canvas_course_id).one_or_none() is None: db.session.add(Offering(canvas_id=canvas_course_id, key=str(canvas_course_id), name="Demo 101")) course = str(canvas_course_id) @@ -57,13 +59,8 @@ def seed(canvas_course_id: int): for i in range(section_count * 2): discussion = choice(discussions) lab = choice(labs) - user = User( - course=course, - email=f"gobears{i}@berkeley.edu", - name=f"Oski {i}th of his name", - is_staff=False, - is_admin=False, - ) + user = find_or_add_person(f"gobears{i}@berkeley.edu", f"Oski {i}th of his name") + user.set_role(course, is_staff=False) user.sections = [discussion, lab] db.session.add(user) users.append(user) diff --git a/server/state.py b/server/state.py index 9c3833e..5c1e0da 100644 --- a/server/state.py +++ b/server/state.py @@ -31,6 +31,9 @@ Session, User, db, + find_or_add_person, + offering_member, + set_contains, user_section, ) @@ -95,11 +98,13 @@ def is_valid_api_secret(secret) -> bool: def add_student_helper(student: User, target_section: Section): - if len(set([s.name for s in student.sections])) != len(student.sections): + # Only the target's offering matters; the person may be in other offerings too. + sections = student.sections_in(target_section.course) + if len(set([s.name for s in sections])) != len(sections): raise Failure("Student has multiple sections of the same type") # If user is already in a section of the same type, remove it and add target section - for s in student.sections: + for s in sections: if s.name == target_section.name: student.sections.remove(s) break @@ -144,7 +149,9 @@ def sudo_wrapped(): if not is_valid_api_secret(secret): abort(401) - user = User.query.filter_by(course=get_course(), email=email).one() + user = offering_member(email=email) + if user is None: + abort(404) # Act as this user for this request only, without touching the session. g._login_user = user try: @@ -208,8 +215,8 @@ def refresh_state(): if current_user.is_authenticated: course = get_course() - enrolled_sections = list(current_user.sections) - taught_sections = list(current_user.sections_taught) + enrolled_sections = current_user.sections_in(course) + taught_sections = [s for s in current_user.sections_taught if s.course == course] current_section_ids = {section.id for section in enrolled_sections} counts_started_at = time.perf_counter() @@ -316,23 +323,23 @@ def join_section(target_section_id: str, enrollment_code: str = ""): # TODO: Really shouldn't be hard coded, but has to be with the code right now config = get_config() if target_section.name == "Discussion" and not config.can_students_join_disc: - if any(current_user.sections): + if any(current_user.sections_in(get_course())): raise Failure("Students cannot change their enrolled discussion!") else: raise Failure("Students cannot add themselves themselves to discussions!") if target_section.name == "Lab" and not config.can_students_join_lab: - if any(current_user.sections): + if any(current_user.sections_in(get_course())): raise Failure("Students cannot change their enrolled lab!") else: raise Failure("Students cannot add themselves themselves to labs!") if target_section.name == "Tutoring" and not config.can_students_join_tutoring: - if any(current_user.sections): + if any(current_user.sections_in(get_course())): raise Failure("Students cannot change their enrolled tutorial!") else: raise Failure("Students cannot add themselves themselves to tutorials!") # If student is already enrolled in a section of the same type/name (Lab/Disc/etc) - if any(map(lambda section: section.name == target_section.name, current_user.sections)): + if any(map(lambda section: section.name == target_section.name, current_user.sections_in(get_course()))): if target_section.name == "Discussion" and not config.can_students_change_disc: raise Failure("Students cannot change their enrolled discussion!") if target_section.name == "Lab" and not config.can_students_change_lab: @@ -390,10 +397,11 @@ def leave_all_sections(): "Discussion": config.can_students_change_disc, "Tutoring": config.can_students_change_tutoring, } - if not all(can_change.get(s.name, True) for s in current_user.sections): + course_sections = current_user.sections_in(get_course()) + if not all(can_change.get(s.name, True) for s in course_sections): raise Failure("Students cannot remove themselves from sections!") - current_user.sections = [] + current_user.sections = [s for s in current_user.sections if s not in course_sections] db.session.commit() return refresh_state() @@ -520,9 +528,7 @@ def set_attendance(session_id: str, students: str, status: Optional[str]): session = Session.query.filter_by(id=session_id, course=get_course()).one() status = AttendanceStatus[status] for email in parse_emails(students): - student = User.query.filter_by( - email=email, course=get_course() - ).one_or_none() + student = offering_member(email=email) if student is None: raise Failure(f"Student {email} is not enrolled") Attendance.query.filter_by(session_id=session_id, student=student).delete() @@ -551,7 +557,9 @@ def update_config(**kwargs): @staff_required def remove_student(student: str, section_id: str): section_id = int(section_id) - student = User.query.filter_by(email=student, course=get_course()).one() + student = offering_member(email=student) + if student is None: + raise Failure("That student is not enrolled in this course") section = Section.query.filter_by(id=section_id, course=get_course()).one() student.sections.remove(section) db.session.commit() @@ -561,9 +569,10 @@ def remove_student(student: str, section_id: str): @admin_required def remove_students(students: str): for s in parse_emails(students): - student = User.query.filter_by(email=s, course=get_course()).one_or_none() + student = offering_member(email=s) if student: - student.sections = [] + # Only this offering's sections; the person may be in others. + student.sections = [sec for sec in student.sections if sec.course != get_course()] db.session.commit() return refresh_state() @@ -571,9 +580,12 @@ def remove_students(students: str): @admin_required def remove_students_from_tutoring(students: str): for s in parse_emails(students): - student = User.query.filter_by(email=s, course=get_course()).one_or_none() + student = offering_member(email=s) if student: - student.sections = [sec for sec in student.sections if sec.name != "Tutoring"] + student.sections = [ + sec for sec in student.sections + if not (sec.course == get_course() and sec.name == "Tutoring") + ] db.session.commit() return refresh_state() @@ -583,9 +595,10 @@ def add_student(email: str, section_id: str): #this function is never called!! use add_students instead section_id = int(section_id) section = Section.query.filter_by(id=section_id, course=get_course()).one() - student = User.query.filter_by(email=email, course=get_course()).one_or_none() + student = offering_member(email=email) if student is None: - student = User(email=email, name=email, is_staff=False, is_admin=False, course=get_course()) + student = find_or_add_person(email, email) + student.set_role(get_course(), is_staff=False) # for decoupling add_student_helper(student, section) @@ -600,9 +613,7 @@ def add_students(emails: str, section_id: str): section = Section.query.filter_by(id=section_id, course=get_course()).one() access_token = session.get("canvas_access_token") for email in parse_emails(emails): - student = User.query.filter_by( - email=email, course=get_course() - ).one_or_none() + student = offering_member(email=email) if student is not None and student.is_staff: raise Failure("Attempted to add staff: {student.name}") if student is None: @@ -629,9 +640,8 @@ def add_students(emails: str, section_id: str): if (not canvasname): raise Failure("Could not find email that belongs to this class") - student = User( - email=email, name=canvasname, is_staff=False, is_admin=False, course=get_course() - ) + student = find_or_add_person(email, canvasname) + student.set_role(get_course(), is_staff=False) add_student_helper(student, section) db.session.commit() return fetch_section(section_id=section_id) @@ -663,13 +673,14 @@ def export_helper(): stringify = dumps attendances = dict() emails = set() + course = get_course() for user in ( - User.query.filter_by(is_staff=False, course=get_course()) + User.query.filter(set_contains(User.student_offerings, course)) .options(joinedload(User.attendances).joinedload(Attendance.session)) .all() ): emails.add(user.email) - for attendance in user.attendances: + for attendance in [a for a in user.attendances if a.course == course]: try: section_name = attendance.session.section.name except AttributeError: @@ -751,7 +762,7 @@ def export_rosters(): @staff_required def fetch_user(user_id: str): user_id = int(user_id) - user = User.query.filter_by(id=user_id, course=get_course()).one_or_none() + user = offering_member(id=user_id) if user is None: raise Failure(f"No user found with id {user_id}") return user.simple_json @@ -759,7 +770,7 @@ def fetch_user(user_id: str): @api @staff_required def get_userid(email: str): - user = User.query.filter_by(email=email, course=get_course()).one_or_none() + user = offering_member(email=email) if user is None: raise Failure(f"No user found with email {email}") return user.id @@ -785,10 +796,12 @@ def reset_sections(): Attendance.query.filter_by(course=course).delete() Session.query.filter_by(course=course).delete() - for user in User.query.filter_by(course=course).all(): - user.sections.clear() - - User.query.filter_by(course=course).delete() + # Remove the offering's sections and everyone's assignments to them, but + # keep the people and their roles in the offering. + course_section_ids = [s.id for s in Section.query.filter_by(course=course)] + db.session.execute( + user_section.delete().where(user_section.c.section_id.in_(course_section_ids)) + ) Section.query.filter_by(course=course).delete() db.session.commit() return refresh_state() @@ -797,16 +810,17 @@ def reset_sections(): @admin_required def fetch_to_drop(): students = "" + course = get_course() for student in ( - User.query.filter_by(is_staff=False, course=get_course()) - .filter(User.sections.any()) + User.query.filter(set_contains(User.student_offerings, course)) + .filter(User.sections.any(Section.course == course)) .all() ): tutoring_section = None absences = [] excused = [] if student: - for section in student.sections: + for section in student.sections_in(course): if section.name == 'Tutoring': tutoring_section = section if tutoring_section: @@ -839,22 +853,16 @@ def fetch_to_drop(): @api @admin_required def get_student_section_ids(email: str): - student = User.query.filter_by( - email= email, - course=get_course() - ).one_or_none() + student = offering_member(email=email) if (student is None): return {email: []} - section_ids = [section.id for section in student.sections] + section_ids = [section.id for section in student.sections_in(get_course())] return {email: section_ids} @api @admin_required def get_student_discussion_attendance(email:str): - student = User.query.filter_by( - email = email, - course=get_course() - ).one_or_none() + student = offering_member(email=email) discussion_present_days = [] if student: attendances = (Attendance.query.join(Attendance.session).join(Session.section) @@ -874,10 +882,7 @@ def get_student_discussion_attendance(email:str): @api @admin_required def get_student_lab_attendance(email:str): - student = User.query.filter_by( - email = email, - course=get_course() - ).one_or_none() + student = offering_member(email=email) lab_present_days = [] if student: attendances = (Attendance.query.join(Attendance.session).join(Session.section) @@ -896,10 +901,7 @@ def get_student_lab_attendance(email:str): @api @admin_required def get_student_tutoring_attendance(email:str): - student = User.query.filter_by( - email = email, - course=get_course() - ).one_or_none() + student = offering_member(email=email) tutoring_present_days = [] if student: attendances = (Attendance.query.join(Attendance.session).join(Session.section) diff --git a/server/test_canvas_login.py b/server/test_canvas_login.py index a8aa9ca..4616678 100644 --- a/server/test_canvas_login.py +++ b/server/test_canvas_login.py @@ -8,10 +8,10 @@ from offerings import create_offering_pages from login import complete_login, create_login_client -from models import Account, Offering, Section, User, db +from models import Attendance, AttendanceStatus, Offering, Section, Session, User, db from state import create_state_client -# Course 101 was moved from the monorepo, so its rows use the old key "cs61a". +# Offering 101 was moved from the monorepo, so its rows use the old key "cs61a". CS61A, DATA8, UNSET_UP = 101, 202, 303 @@ -20,6 +20,11 @@ def canvas_course(code, *, staff=False, admin=False, student=False): "is_staff": staff, "is_admin": admin, "is_student": student} +def lab(course, location="Room"): + return Section(course=course, name="Lab", location=location, description="", + capacity=30, can_self_enroll=True, start_time=0, end_time=3600) + + class CanvasLoginTests(unittest.TestCase): def setUp(self): self.app = Flask(__name__) @@ -41,39 +46,36 @@ def setUp(self): # Linked from the monorepo: no Canvas details until a member signs in. Offering(canvas_id=CS61A, key="cs61a", name="CS 61A"), Offering(canvas_id=DATA8, key=str(DATA8), name="DATA 8 - LEC 001", code="DATA 8", - start_at="2026-08-19T07:00:00Z"), + start_at="2026-08-19T07:00:00Z"), ]) - self.staff_account = Account(canvas_id="1", email="staff@test", name="Staff", canvas_courses={ + self.staff = User(email="staff@test", name="Staff", canvas_id="1", canvas_courses={ str(CS61A): canvas_course("CS 61A", staff=True), str(UNSET_UP): canvas_course("CS 70", staff=True, admin=True), }) - self.known_account = Account(canvas_id="2", email="known@test", name="Known", canvas_courses={ + self.staff.set_role("cs61a", is_staff=True) + self.known = User(email="known@test", name="Known", canvas_id="2", canvas_courses={ str(CS61A): canvas_course("CS 61A", student=True), str(UNSET_UP): canvas_course("CS 70", student=True), }) - staff = User(email="staff@test", name="Staff", is_staff=True, is_admin=False, course="cs61a") - known = User(email="known@test", name="Known", is_staff=False, is_admin=False, course="cs61a") - section = Section(course="cs61a", name="Lab", location="Room", description="", - capacity=30, can_self_enroll=True, start_time=0, end_time=3600) - data8_section = Section(course=str(DATA8), name="Lab", location="Elsewhere", - description="", capacity=30, can_self_enroll=True, - start_time=0, end_time=3600) - db.session.add_all([self.staff_account, self.known_account, staff, known, section, data8_section]) + self.known.set_role("cs61a", is_staff=False) + section = lab("cs61a") + data8_section = lab(str(DATA8), location="Elsewhere") + db.session.add_all([self.staff, self.known, section, data8_section]) db.session.commit() - self.staff_id = staff.id - self.known_id = known.id self.section_id = section.id + self.data8_section_id = data8_section.id + self.staff_id, self.known_id = self.staff.id, self.known.id self.client = self.app.test_client() - self.sign_in(self.staff_account) + self.sign_in(self.staff) def tearDown(self): db.session.remove() db.drop_all() self.context.pop() - def sign_in(self, account): + def sign_in(self, user): with self.client.session_transaction() as browser: - browser["_user_id"] = account.get_id() + browser["_user_id"] = user.get_id() def get(self, path): self.clear_request_globals() @@ -104,6 +106,15 @@ def authorize(self, expires_in=3600): browser["canvas_access_token"] = "access" browser["canvas_token_expires_at"] = time.time() + expires_in + def enroll_known_in_data8(self): + """Make "known" a student in DATA 8 too, enrolled in its lab.""" + known = db.session.get(User, self.known_id) + known.canvas_courses = {**known.canvas_courses, str(DATA8): canvas_course("DATA 8", student=True)} + known.set_role(str(DATA8), is_staff=False) + known.sections.append(db.session.get(Section, self.data8_section_id)) + db.session.commit() + return known + def test_existing_students_need_no_canvas_token(self): with patch("state.canvas_service.get_student_from_email") as lookup: self.assertTrue(self.add_students("known@test")["success"]) @@ -129,7 +140,7 @@ def test_fresh_token_is_passed_to_canvas(self): lookup.assert_called_once_with("new@test", "access") db.session.remove() new = User.query.filter_by(email="new@test").one() - self.assertEqual(new.course, "cs61a") + self.assertEqual(new.student_offerings, {"cs61a"}) self.assertEqual([s.id for s in new.sections], [self.section_id]) def test_revoked_token_requests_reauthentication(self): @@ -155,14 +166,30 @@ def test_login_retains_only_staff_access_token_and_logout_clears_it(self): self.assertEqual(session.get("canvas_access_token"), "access" if is_staff else None) self.assertNotIn("refresh_token", session) self.assertTrue(session.permanent) - self.assertEqual(session["_user_id"], self.staff_account.get_id()) - self.assertEqual(db.session.get(Account, self.staff_account.id).canvas_courses, courses) + self.assertEqual(session["_user_id"], self.staff.get_id()) + self.assertEqual(db.session.get(User, self.staff_id).canvas_courses, courses) self.authorize() self.get("/logout/") with self.client.session_transaction() as browser: self.assertNotIn("canvas_access_token", browser) self.assertNotIn("canvas_token_expires_at", browser) + def test_first_login_claims_the_imported_person_with_that_email(self): + imported = User(email="tutor@test", name="Imported Name") + imported.set_role("cs61a", is_staff=True) + db.session.add(imported) + db.session.commit() + with self.app.test_request_context(), \ + patch("login.canvas_service.get_profile", + return_value={"primary_email": "tutor@test", "short_name": "Canvas Name"}), \ + patch("login.canvas_service.get_user_courses", + return_value={str(CS61A): canvas_course("CS 61A", staff=True)}): + complete_login({"user": {"id": 77}, "access_token": "access"}) + self.assertEqual(User.query.filter_by(email="tutor@test").count(), 1) + tutor = db.session.get(User, imported.id) + self.assertEqual((tutor.canvas_id, tutor.name), ("77", "Canvas Name")) + self.assertEqual(tutor.staff_offerings, {"cs61a"}) + def test_admin_override_course_makes_global_admin(self): self.app.config["ADMIN_OVERRIDE_CANVAS_COURSE_ID"] = 999 with self.app.test_request_context(), \ @@ -171,7 +198,7 @@ def test_admin_override_course_makes_global_admin(self): return_value={"999": canvas_course("Override", student=True)}): complete_login({"user": {"id": 9}, "access_token": "access"}) self.assertIsNotNone(session.get("canvas_access_token")) - boss = Account.query.filter_by(canvas_id="9").one() + boss = User.query.filter_by(canvas_id="9").one() self.assertTrue(boss.is_global_admin) self.sign_in(boss) user = self.api("refresh_state", DATA8).get_json()["data"]["currentUser"] @@ -181,9 +208,10 @@ def test_admin_override_course_makes_global_admin(self): def test_student_refresh_uses_summaries_and_keeps_enrolled_roster(self): known = db.session.get(User, self.known_id) section = db.session.get(Section, self.section_id) - peer = User(email="peer@test", name="Peer", is_staff=False, is_admin=False, course="cs61a") - unrelated = User(email="unrelated@test", name="Unrelated", is_staff=False, - is_admin=False, course="cs61a") + peer = User(email="peer@test", name="Peer") + unrelated = User(email="unrelated@test", name="Unrelated") + for person in (peer, unrelated): + person.set_role("cs61a", is_staff=False) other_section = Section(course="cs61a", name="Discussion", location="Other Room", description="", capacity=25, can_self_enroll=True, start_time=7200, end_time=10800) @@ -192,7 +220,7 @@ def test_student_refresh_uses_summaries_and_keeps_enrolled_roster(self): db.session.add_all([peer, unrelated, other_section]) db.session.commit() - self.sign_in(self.known_account) + self.sign_in(self.known) response = self.api("refresh_state") self.assertEqual(response.status_code, 200) @@ -223,41 +251,119 @@ def test_courses_are_isolated(self): state = self.api("refresh_state").get_json()["data"] self.assertEqual([s["location"] for s in state["sections"]], ["Room"]) # Sections from another course can't be reached through this one. - data8_section = Section.query.filter_by(course=str(DATA8)).one() with self.assertRaises(NoResultFound): # a 500 outside tests - self.api("claim_section", section_id=str(data8_section.id)) - self.assertIsNone(db.session.get(Section, data8_section.id).staff_id) + self.api("claim_section", section_id=str(self.data8_section_id)) + self.assertIsNone(db.session.get(Section, self.data8_section_id).staff_id) # Not enrolled in DATA 8, so its pages and APIs are forbidden. self.assertEqual(self.api("refresh_state", DATA8).status_code, 403) self.assertEqual(self.get(f"/offerings/{DATA8}/").status_code, 403) + def test_one_person_in_two_offerings_sees_each_separately(self): + known = self.enroll_known_in_data8() + known.sections.append(db.session.get(Section, self.section_id)) + db.session.commit() + self.sign_in(known) + for canvas_id, location in ((CS61A, "Room"), (DATA8, "Elsewhere")): + state = self.api("refresh_state", canvas_id).get_json()["data"] + self.assertEqual([s["location"] for s in state["enrolledSections"]], [location]) + + def test_joining_a_lab_keeps_the_lab_in_another_offering(self): + known = self.enroll_known_in_data8() + self.sign_in(known) + self.api("refresh_state") # the app loads state (and the course config) first + result = self.api("join_section", target_section_id=str(self.section_id)).get_json() + self.assertTrue(result["success"], result) + db.session.remove() + sections = {s.id for s in db.session.get(User, self.known_id).sections} + self.assertEqual(sections, {self.section_id, self.data8_section_id}) + + def test_leaving_and_removal_only_affect_this_offering(self): + known = self.enroll_known_in_data8() + known.sections.append(db.session.get(Section, self.section_id)) + db.session.commit() + self.sign_in(known) + self.api("refresh_state") # the app loads state (and the course config) first + self.assertTrue(self.api("leave_all_sections").get_json()["success"]) + db.session.remove() + self.assertEqual([s.id for s in db.session.get(User, self.known_id).sections], + [self.data8_section_id]) + + # Admins removing a student also leave their other offerings alone. + known = db.session.get(User, self.known_id) + known.sections.append(db.session.get(Section, self.section_id)) + staff = db.session.get(User, self.staff_id) + staff.canvas_courses = {str(CS61A): canvas_course("CS 61A", staff=True, admin=True)} + db.session.commit() + self.sign_in(staff) + self.assertTrue(self.api("remove_students", students="known@test").get_json()["success"]) + db.session.remove() + self.assertEqual([s.id for s in db.session.get(User, self.known_id).sections], + [self.data8_section_id]) + + def test_attendance_history_is_per_offering(self): + known = self.enroll_known_in_data8() + data8_session = Session(course=str(DATA8), start_time=0, section_id=self.data8_section_id) + db.session.add(data8_session) + db.session.add(Attendance(course=str(DATA8), status=AttendanceStatus.present, + session=data8_session, student=known)) + db.session.commit() + self.sign_in(known) + for canvas_id, expected in ((CS61A, 0), (DATA8, 1)): + user = self.api("refresh_state", canvas_id).get_json()["data"]["currentUser"] + self.assertEqual(len(user["attendanceHistory"]), expected) + + def test_reset_removes_sections_and_assignments_but_keeps_people(self): + known = self.enroll_known_in_data8() + section = db.session.get(Section, self.section_id) + known.sections.append(section) + section.staff = db.session.get(User, self.staff_id) + session = Session(course="cs61a", start_time=0, section=section) + db.session.add(Attendance(course="cs61a", status=AttendanceStatus.present, + session=session, student=known)) + staff = db.session.get(User, self.staff_id) + staff.canvas_courses = {str(CS61A): canvas_course("CS 61A", staff=True, admin=True)} + db.session.commit() + + self.assertTrue(self.api("reset_sections").get_json()["success"]) + db.session.remove() + self.assertIsNone(db.session.get(Section, self.section_id)) + self.assertEqual(Session.query.filter_by(course="cs61a").count(), 0) + self.assertEqual(Attendance.query.filter_by(course="cs61a").count(), 0) + known = db.session.get(User, self.known_id) + # Still in the course, and untouched in DATA 8. + self.assertEqual(known.student_offerings, {"cs61a", str(DATA8)}) + self.assertEqual([s.id for s in known.sections], [self.data8_section_id]) + self.assertIn("cs61a", db.session.get(User, self.staff_id).staff_offerings) + def test_course_that_is_not_set_up_is_not_found(self): self.assertEqual(self.api("refresh_state", UNSET_UP).status_code, 404) - def test_first_visit_creates_course_user_and_roles_follow_canvas(self): - self.staff_account.canvas_courses = { - **self.staff_account.canvas_courses, + def test_roles_follow_canvas(self): + staff = db.session.get(User, self.staff_id) + staff.canvas_courses = { + **staff.canvas_courses, str(DATA8): canvas_course("DATA 8", staff=True, admin=True), } db.session.commit() user = self.api("refresh_state", DATA8).get_json()["data"]["currentUser"] self.assertTrue(user["isAdmin"]) - self.assertEqual(User.query.filter_by(email="staff@test", course=str(DATA8)).count(), 1) + self.assertIn(str(DATA8), db.session.get(User, self.staff_id).admin_offerings) - # Dropped from staff in Canvas: the next sign-in updates the account, - # and the course user follows. - self.staff_account.canvas_courses = {str(CS61A): canvas_course("CS 61A", student=True)} + # Dropped from staff in Canvas: the next login updates canvas_courses, + # and the offering role follows. + staff = db.session.get(User, self.staff_id) + staff.canvas_courses = {str(CS61A): canvas_course("CS 61A", student=True)} db.session.commit() user = self.api("refresh_state").get_json()["data"]["currentUser"] self.assertEqual(user["id"], self.staff_id) self.assertFalse(user["isStaff"]) def test_staff_import_offerings_and_students_cannot(self): - self.sign_in(self.known_account) + self.sign_in(self.known) self.post("/offerings/new", data={"offerings": [str(UNSET_UP)]}) self.assertIsNone(Offering.query.filter_by(canvas_id=UNSET_UP).one_or_none()) - self.sign_in(self.staff_account) + self.sign_in(self.staff) page = self.get("/offerings/new").get_data(as_text=True) self.assertIn(f'value="{UNSET_UP}"', page) self.assertIn("2026-08 | CS 70 | CS 70 - LEC 001", page) @@ -265,8 +371,8 @@ def test_staff_import_offerings_and_students_cannot(self): # Only courses the user is staff in can be imported. response = self.post("/offerings/new", data={"offerings": [str(UNSET_UP), str(DATA8), "999"]}) self.assertTrue(response.headers["Location"].endswith("/offerings")) - course = Offering.query.filter_by(canvas_id=UNSET_UP).one() - self.assertEqual((course.key, course.code, course.start_month), (str(UNSET_UP), "CS 70", "2026-08")) + offering = Offering.query.filter_by(canvas_id=UNSET_UP).one() + self.assertEqual((offering.key, offering.code, offering.start_month), (str(UNSET_UP), "CS 70", "2026-08")) self.assertIsNone(Offering.query.filter_by(canvas_id=999).one_or_none()) self.assertTrue(self.api("refresh_state", UNSET_UP).get_json()["success"]) @@ -276,12 +382,12 @@ def test_offerings_page_matches_seating(self): self.assertIn("Staff Course Offerings", page) self.assertIn(f'href="/offerings/{CS61A}/"', page) self.assertNotIn(f"/offerings/{DATA8}/", page) - # The linked course picked up its Canvas details. + # The linked offering picked up its Canvas details. self.assertIn("CS 61A - LEC 001", page) self.assertEqual(Offering.query.filter_by(canvas_id=CS61A).one().code, "CS 61A") self.assertEqual(self.api("refresh_state").get_json()["data"]["course"], "CS 61A") - self.sign_in(self.known_account) + self.sign_in(self.known) page = self.get("/offerings").get_data(as_text=True) student_part = page[page.index("Student Course Offerings"):page.index("Other Course Offerings")] self.assertIn(f'href="/offerings/{CS61A}/"', student_part) @@ -293,12 +399,14 @@ def test_logged_out_pages_go_to_login_and_apis_return_401(self): self.assertIn("/login/", self.get("/offerings").headers["Location"]) self.assertEqual(self.api("join_section", target_section_id=str(self.section_id)).status_code, 401) - def test_session_from_before_accounts_is_ignored(self): - # Sessions from the single-course app hold a per-course User id. - with self.client.session_transaction() as browser: - browser["_user_id"] = str(self.staff_account.id) - state = self.api("refresh_state").get_json()["data"] - self.assertIsNone(state["currentUser"]) + def test_sessions_from_before_this_change_are_ignored(self): + # Older cookies hold a per-course row id or an "account-" id. + for old_id in (str(self.staff_id), f"account-{self.staff_id}"): + with self.subTest(old_id=old_id): + with self.client.session_transaction() as browser: + browser["_user_id"] = old_id + state = self.api("refresh_state").get_json()["data"] + self.assertIsNone(state["currentUser"]) if __name__ == "__main__": diff --git a/server/test_migrations.py b/server/test_migrations.py new file mode 100644 index 0000000..2fa603a --- /dev/null +++ b/server/test_migrations.py @@ -0,0 +1,103 @@ +import os +import tempfile +import unittest + +import sqlalchemy as sa +from flask import Flask +from flask_migrate import Migrate, upgrade + +from models import db + +MIGRATIONS = os.path.join(os.path.dirname(os.path.abspath(__file__)), "migrations") + + +class SingleUserMigrationTests(unittest.TestCase): + """0002_single_user merges per-offering user rows into one user per person.""" + + def setUp(self): + handle, self.path = tempfile.mkstemp(suffix=".db") + os.close(handle) + self.app = Flask(__name__) + self.app.config["SQLALCHEMY_DATABASE_URI"] = f"sqlite:///{self.path}" + db.init_app(self.app) + Migrate(self.app, db, directory=MIGRATIONS) + self.context = self.app.app_context() + self.context.push() + + def tearDown(self): + db.session.remove() + db.engine.dispose() + self.context.pop() + os.remove(self.path) + + def sql(self, statement, **params): + with db.engine.begin() as conn: + return conn.execute(sa.text(statement), params).all() if statement.lstrip().upper().startswith("SELECT") \ + else conn.execute(sa.text(statement), params) + + def test_merges_people_and_repoints_references(self): + upgrade(revision="0001_baseline") + self.sql("""INSERT INTO "user" (id, course, email, name, is_staff, is_admin) VALUES + (10, 'cs61a', 'ada@test', 'Ada', 0, 0), + (11, 'data8', 'ada@test', 'Ada L.', 1, 1), + (12, 'cs61a', 'tutor@test', 'Tutor', 1, 0), + (13, 'eecs16b', 'bob@test', 'Bob', NULL, NULL)""") + self.sql("""INSERT INTO account (id, canvas_id, email, name, is_global_admin, canvas_courses) + VALUES (1, '555', 'ada@test', 'Ada Lovelace', 0, '{"101": {"is_staff": false}}'), + (2, '556', 'new@test', 'Only Logged In', 1, '{}')""") + self.sql("""INSERT INTO section (id, course, staff_id, tag_string, location, name) VALUES + (1, 'cs61a', 12, '', 'Soda', 'Lab'), (2, 'data8', 11, '', 'Evans', 'Lab')""") + self.sql("INSERT INTO user_section (user_id, section_id) VALUES (10, 1), (13, 1)") + self.sql("INSERT INTO session (id, course, start_time, section_id) VALUES (1, 'cs61a', 0, 1)") + self.sql("""INSERT INTO attendance (id, course, status, session_id, student_id) + VALUES (1, 'cs61a', 'present', 1, 10)""") + self.sql("INSERT INTO offerings (id, canvas_id, key, name) VALUES (1, 101, 'cs61a', 'CS 61A')") + self.sql("CREATE TABLE course (id INTEGER PRIMARY KEY)") + self.sql("CREATE TABLE slot (id INTEGER PRIMARY KEY)") + # Monorepo databases link sessions to slots. + self.sql("ALTER TABLE session ADD COLUMN slot_id INTEGER REFERENCES slot(id)") + self.sql("CREATE TABLE survey (id INTEGER PRIMARY KEY)") + self.sql("CREATE TABLE user_section_junction (user_id INTEGER REFERENCES \"user\"(id), section_id INTEGER)") + + upgrade() + + self.assertNotIn("slot_id", {c["name"] for c in sa.inspect(db.engine).get_columns("session")}) + tables = set(sa.inspect(db.engine).get_table_names()) + self.assertIn("users", tables) + for gone in ("user", "account", "course", "slot", "survey", "user_section_junction", "user_id_map"): + self.assertNotIn(gone, tables) + + people = {row.email: row for row in self.sql("SELECT * FROM users")} + self.assertEqual(set(people), {"ada@test", "tutor@test", "bob@test", "new@test"}) + ada = people["ada@test"] + # Logged-in people keep their Canvas name and details. + self.assertEqual((ada.name, ada.canvas_id), ("Ada Lovelace", "555")) + self.assertEqual( + (ada.student_offerings, ada.staff_offerings, ada.admin_offerings), ("cs61a", "data8", "data8")) + self.assertEqual(people["bob@test"].student_offerings, "eecs16b") # unknown role counts as student + self.assertTrue(people["new@test"].is_global_admin) + + # Every reference now points at the merged person. + ids = {email: row.id for email, row in people.items()} + self.assertEqual(self.sql("SELECT staff_id FROM section ORDER BY id"), + [(ids["tutor@test"],), (ids["ada@test"],)]) + self.assertEqual(sorted(self.sql("SELECT user_id FROM user_section")), + sorted([(ids["ada@test"],), (ids["bob@test"],)])) + self.assertEqual(self.sql("SELECT student_id FROM attendance"), [(ids["ada@test"],)]) + foreign_keys = { + (table, fk["referred_table"]) + for table in ("attendance", "section", "user_section") + for fk in sa.inspect(db.engine).get_foreign_keys(table) + } + self.assertIn(("attendance", "users"), foreign_keys) + self.assertIn(("section", "users"), foreign_keys) + self.assertIn(("user_section", "users"), foreign_keys) + self.assertNotIn("user", {referred for _, referred in foreign_keys}) + + def test_fresh_database(self): + upgrade() + self.assertEqual(self.sql("SELECT count(*) FROM users"), [(0,)]) + + +if __name__ == "__main__": + unittest.main() diff --git a/src/ResetSectionsModal.tsx b/src/ResetSectionsModal.tsx index 4216e9c..379d84f 100644 --- a/src/ResetSectionsModal.tsx +++ b/src/ResetSectionsModal.tsx @@ -14,8 +14,9 @@ export default function ResetSectionsModal({ show, onReset, onClose }: Props) { Reset Sections Tool - WARNING: This action will delete all attendance history, sessions, - sections, and users associated with your course. This is + WARNING: This action will delete all of your course's sections, their + sessions and attendance history, and every student and staff assignment + to them. Students and staff stay in the course. This is irreversible and should only be done in order to prepare sections tool for a new semester. Be extremely cautious when using this button. You have been warned.