From 697b5d3bf4a237d53987430c203f36751dde4744 Mon Sep 17 00:00:00 2001 From: bhuvanh66 Date: Sun, 25 Oct 2026 22:16:30 -0400 Subject: [PATCH] feat: add users and link them to hotels --- AGENTS.md | 1 + backend/foresight/cli.py | 59 ++++ .../e6235f613e08_users_and_user_hotels.py | 87 ++++++ backend/foresight/database/models/__init__.py | 3 + backend/foresight/database/models/user.py | 54 ++++ backend/foresight/repository/user.py | 68 +++++ backend/foresight/services/domain/user.py | 79 ++++++ backend/tests/test_user.py | 261 ++++++++++++++++++ .../0013-one-transaction-per-unit-of-work.md | 2 + .../0035-users-keyed-on-google-id.md | 30 ++ .../0036-operator-commands-in-cli.md | 25 ++ docs/decisions/README.md | 2 + docs/system-design.md | 25 +- justfile | 7 + 14 files changed, 698 insertions(+), 5 deletions(-) create mode 100644 backend/foresight/cli.py create mode 100644 backend/foresight/database/migrations/versions/e6235f613e08_users_and_user_hotels.py create mode 100644 backend/foresight/database/models/user.py create mode 100644 backend/foresight/repository/user.py create mode 100644 backend/foresight/services/domain/user.py create mode 100644 backend/tests/test_user.py create mode 100644 docs/decisions/0035-users-keyed-on-google-id.md create mode 100644 docs/decisions/0036-operator-commands-in-cli.md diff --git a/AGENTS.md b/AGENTS.md index a55d281..1a1a4a9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -45,6 +45,7 @@ just dev --neon # same, against Neon; `--neon` works on run/migrate*/test just migrate # apply migrations just migrate-create "msg" # new Alembic migration after model changes just migrate-down # roll back last migration +just link-user # link a user to a hotel (`--neon` works too) just lint # ruff checks just format # ruff format ``` diff --git a/backend/foresight/cli.py b/backend/foresight/cli.py new file mode 100644 index 0000000..c60bbb2 --- /dev/null +++ b/backend/foresight/cli.py @@ -0,0 +1,59 @@ +"""Operator commands, run through `just` (see the justfile). + +For jobs that can't wait for the app to offer them. Like `worker.run_job`, a +command opens its own session and commits, once, when it succeeds; everything it +does goes through `services/`. A `DomainError` is printed and exits 1, with +nothing committed. +""" + +from __future__ import annotations + +import argparse +import asyncio +import sys + +from foresight.database.session import async_session_factory, engine +from foresight.services.domain import user as user_service +from foresight.services.errors import DomainError + + +async def link_user(email: str, hotel_id: int) -> None: + async with async_session_factory() as session: + user = await user_service.get_user_by_email(session, email) + linked = await user_service.link_user_to_hotel( + session, user_id=user.id, hotel_id=hotel_id + ) + await session.commit() + if linked: + print(f"Linked {user.email} to hotel {hotel_id}") + else: + print(f"{user.email} is already linked to hotel {hotel_id}") + + +async def _run(args: argparse.Namespace) -> None: + try: + if args.command == "link-user": + await link_user(args.email, args.hotel_id) + finally: + await engine.dispose() + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(prog="foresight.cli") + commands = parser.add_subparsers(dest="command", required=True) + + link = commands.add_parser("link-user", help="Link a user to a hotel by email") + link.add_argument("email", help="The user's email, as Google reports it") + link.add_argument("hotel_id", type=int, help="The hotel's id") + + args = parser.parse_args(argv) + try: + asyncio.run(_run(args)) + except DomainError as exc: + print(f"error: {exc}", file=sys.stderr) + return 1 + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/backend/foresight/database/migrations/versions/e6235f613e08_users_and_user_hotels.py b/backend/foresight/database/migrations/versions/e6235f613e08_users_and_user_hotels.py new file mode 100644 index 0000000..a42361b --- /dev/null +++ b/backend/foresight/database/migrations/versions/e6235f613e08_users_and_user_hotels.py @@ -0,0 +1,87 @@ +"""users and user hotels + +The API has no notion of who is calling it. Adds users, identified by their +Google account id (unique; email can change and is not), and a link table so a +hotel can have several users and a user can belong to several hotels. A user +with no link is allowed: they have signed up but work for no hotel yet. Links +are removed with either side. + +Both tables are new, so existing data is untouched. A downgrade drops both +tables and every user and link in them. + +Revision ID: e6235f613e08 +Revises: 46b9cb210348 +Create Date: 2026-10-25 19:17:46.437463 + +""" + +from typing import Sequence, Union + +from alembic import op +import sqlalchemy as sa + +# revision identifiers, used by Alembic. +revision: str = "e6235f613e08" +down_revision: Union[str, None] = "46b9cb210348" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + op.create_table( + "users", + sa.Column("id", sa.Integer(), nullable=False), + sa.Column("google_id", sa.String(length=255), nullable=False), + sa.Column("email", sa.String(length=320), nullable=False), + sa.Column("name", sa.String(length=200), nullable=True), + sa.Column( + "created_at", + sa.DateTime(timezone=True), + server_default=sa.text("now()"), + nullable=False, + ), + sa.Column( + "last_signed_in_at", + sa.DateTime(timezone=True), + server_default=sa.text("now()"), + nullable=False, + ), + sa.PrimaryKeyConstraint("id", name=op.f("pk_users")), + sa.UniqueConstraint("google_id", name=op.f("uq_users_google_id")), + ) + op.create_table( + "user_hotels", + sa.Column("user_id", sa.Integer(), nullable=False), + sa.Column("hotel_id", sa.Integer(), nullable=False), + sa.Column( + "created_at", + sa.DateTime(timezone=True), + server_default=sa.text("now()"), + nullable=False, + ), + sa.ForeignKeyConstraint( + ["hotel_id"], + ["hotels.id"], + name=op.f("fk_user_hotels_hotel_id_hotels"), + ondelete="CASCADE", + ), + sa.ForeignKeyConstraint( + ["user_id"], + ["users.id"], + name=op.f("fk_user_hotels_user_id_users"), + ondelete="CASCADE", + ), + sa.PrimaryKeyConstraint("user_id", "hotel_id", name=op.f("pk_user_hotels")), + ) + op.create_index( + op.f("ix_user_hotels_hotel_id"), + "user_hotels", + ["hotel_id"], + unique=False, + ) + + +def downgrade() -> None: + op.drop_index(op.f("ix_user_hotels_hotel_id"), table_name="user_hotels") + op.drop_table("user_hotels") + op.drop_table("users") diff --git a/backend/foresight/database/models/__init__.py b/backend/foresight/database/models/__init__.py index 457250f..056284c 100644 --- a/backend/foresight/database/models/__init__.py +++ b/backend/foresight/database/models/__init__.py @@ -11,6 +11,7 @@ from foresight.database.models.signals.event import Event, EventObservation from foresight.database.models.signals.flight import FlightArrival from foresight.database.models.signals.weather import Weather +from foresight.database.models.user import User, UserHotel __all__ = [ "Base", @@ -24,6 +25,8 @@ "Job", "PricingModel", "RoomType", + "User", + "UserHotel", "Weather", "RecommendationSnapshot", "Widget", diff --git a/backend/foresight/database/models/user.py b/backend/foresight/database/models/user.py new file mode 100644 index 0000000..e1b4746 --- /dev/null +++ b/backend/foresight/database/models/user.py @@ -0,0 +1,54 @@ +"""A person who signs in with Google, and the hotels they work for.""" + +from datetime import datetime + +from sqlalchemy import DateTime, ForeignKey, String, func +from sqlalchemy.orm import Mapped, mapped_column + +from foresight.database.base import Base + + +class User(Base): + """One Google account. Created the first time someone signs in, so a user + may have no hotel yet. + """ + + __tablename__ = "users" + + id: Mapped[int] = mapped_column(primary_key=True) + # Google's account id (the `sub` claim). This is the identity: an email can + # change, the id can't. + google_id: Mapped[str] = mapped_column(String(255), unique=True) + # Not unique: an address can pass to another person before its previous + # owner signs in again and the row is updated. + email: Mapped[str] = mapped_column(String(320)) + name: Mapped[str | None] = mapped_column(String(200)) + + created_at: Mapped[datetime] = mapped_column( + DateTime(timezone=True), server_default=func.now() + ) + # A row only exists because they just signed in, so it starts at creation. + last_signed_in_at: Mapped[datetime] = mapped_column( + DateTime(timezone=True), server_default=func.now() + ) + + +class UserHotel(Base): + """A user works for a hotel. A hotel has several users and a user can belong + to several hotels. Deleting either side removes the link. + """ + + __tablename__ = "user_hotels" + + # The composite key rejects a second link for the same pair, and its leading + # column serves as the foreign key index for user_id. + user_id: Mapped[int] = mapped_column( + ForeignKey("users.id", ondelete="CASCADE"), primary_key=True + ) + hotel_id: Mapped[int] = mapped_column( + ForeignKey("hotels.id", ondelete="CASCADE"), primary_key=True, index=True + ) + + created_at: Mapped[datetime] = mapped_column( + DateTime(timezone=True), server_default=func.now() + ) diff --git a/backend/foresight/repository/user.py b/backend/foresight/repository/user.py new file mode 100644 index 0000000..9738d1b --- /dev/null +++ b/backend/foresight/repository/user.py @@ -0,0 +1,68 @@ +"""Data access for `users` and `user_hotels`. + +`create_user` returns None when the Google account already has a user. +`link_hotel` is idempotent: it reports whether it added a link or one was +already there. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +from sqlalchemy import func, select +from sqlalchemy.dialects.postgresql import insert + +from foresight.database.models.user import User, UserHotel + +if TYPE_CHECKING: + from collections.abc import Sequence + + from sqlalchemy.ext.asyncio import AsyncSession + + +async def create_user( + session: AsyncSession, *, google_id: str, email: str, name: str | None +) -> User | None: + """Insert a user; None if this Google account already has one.""" + return await session.scalar( + insert(User) + .values(google_id=google_id, email=email, name=name) + .on_conflict_do_nothing(constraint="uq_users_google_id") + .returning(User) + ) + + +async def get_user(session: AsyncSession, user_id: int) -> User | None: + return await session.get(User, user_id) + + +async def get_user_by_google_id(session: AsyncSession, google_id: str) -> User | None: + return await session.scalar(select(User).where(User.google_id == google_id)) + + +async def list_users_by_email(session: AsyncSession, email: str) -> Sequence[User]: + """Users with this email, ignoring case. Email isn't unique, so there can be + several.""" + stmt = select(User).where(func.lower(User.email) == email.lower()).order_by(User.id) + return (await session.scalars(stmt)).all() + + +async def link_hotel(session: AsyncSession, *, user_id: int, hotel_id: int) -> bool: + """Link a user to a hotel. True if the link is new, False if it already + existed. The caller checks both exist.""" + linked = await session.scalar( + insert(UserHotel) + .values(user_id=user_id, hotel_id=hotel_id) + .on_conflict_do_nothing(constraint="pk_user_hotels") + .returning(UserHotel.user_id) + ) + return linked is not None + + +async def list_hotel_ids(session: AsyncSession, user_id: int) -> Sequence[int]: + stmt = ( + select(UserHotel.hotel_id) + .where(UserHotel.user_id == user_id) + .order_by(UserHotel.hotel_id) + ) + return (await session.scalars(stmt)).all() diff --git a/backend/foresight/services/domain/user.py b/backend/foresight/services/domain/user.py new file mode 100644 index 0000000..1d59ee7 --- /dev/null +++ b/backend/foresight/services/domain/user.py @@ -0,0 +1,79 @@ +"""Users, and which hotels they work for.""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +from foresight.repository import user as user_repo +from foresight.services.domain.hotel import get_hotel +from foresight.services.errors import ConflictError, NotFoundError + +if TYPE_CHECKING: + from collections.abc import Sequence + + from sqlalchemy.ext.asyncio import AsyncSession + + from foresight.database.models.user import User + + +async def create_user( + session: AsyncSession, *, google_id: str, email: str, name: str | None +) -> User: + """Raises ConflictError if this Google account already has a user.""" + user = await user_repo.create_user( + session, google_id=google_id, email=email, name=name + ) + if user is None: + raise ConflictError(f"A user already exists for Google account {google_id!r}") + return user + + +async def get_user(session: AsyncSession, user_id: int) -> User: + """Raises NotFoundError for an unknown user.""" + user = await user_repo.get_user(session, user_id) + if user is None: + raise NotFoundError("User", user_id) + return user + + +async def get_user_by_google_id(session: AsyncSession, google_id: str) -> User: + """Raises NotFoundError if no user has this Google account id.""" + user = await user_repo.get_user_by_google_id(session, google_id) + if user is None: + raise NotFoundError("User", google_id) + return user + + +async def get_user_by_email(session: AsyncSession, email: str) -> User: + """The user with this email, ignoring case. + + Raises: + NotFoundError: No user has this email. + ConflictError: Several do. Emails change hands, so this never picks one: + it would give a hotel's access to the wrong person. + """ + users = await user_repo.list_users_by_email(session, email) + if not users: + raise NotFoundError("User", email) + if len(users) > 1: + raise ConflictError(f"More than one user has the email {email!r}") + return users[0] + + +async def link_user_to_hotel( + session: AsyncSession, *, user_id: int, hotel_id: int +) -> bool: + """Link a user to a hotel. True if the link is new, False if it already + existed; linking twice is not an error. + + Raises NotFoundError for an unknown user or hotel. + """ + await get_user(session, user_id) + await get_hotel(session, hotel_id) + return await user_repo.link_hotel(session, user_id=user_id, hotel_id=hotel_id) + + +async def list_hotel_ids(session: AsyncSession, user_id: int) -> Sequence[int]: + """The hotels a user is linked to. Raises NotFoundError for an unknown user.""" + await get_user(session, user_id) + return await user_repo.list_hotel_ids(session, user_id) diff --git a/backend/tests/test_user.py b/backend/tests/test_user.py new file mode 100644 index 0000000..c35010f --- /dev/null +++ b/backend/tests/test_user.py @@ -0,0 +1,261 @@ +"""Tests for users and their hotel links, at the repository and service layers. + +Need a migrated database (`just migrate`; CI migrates before it runs pytest). +The `session` fixture (conftest.py) runs each test inside a transaction that is +rolled back, so it leaves nothing behind. Names, emails and Google ids carry a +random suffix, so a test can't collide with rows already in the database it +runs against. +""" + +from __future__ import annotations + +import uuid +from typing import TYPE_CHECKING + +import pytest + +from foresight.repository import user as user_repo +from foresight.services.domain import city as city_service +from foresight.services.domain import hotel as hotel_service +from foresight.services.domain import user as user_service +from foresight.services.errors import ConflictError, NotFoundError + +if TYPE_CHECKING: + from sqlalchemy.ext.asyncio import AsyncSession + + from foresight.database.models.hotel import Hotel + +# Far above any id a sequence will reach. +MISSING_ID = 2_000_000_000 + + +# Builders + + +def unique(prefix: str) -> str: + return f"{prefix}-{uuid.uuid4().hex[:12]}" + + +def unique_email() -> str: + return f"{unique('person')}@example.com" + + +async def make_hotel(session: AsyncSession) -> Hotel: + """A hotel in a city of its own.""" + city = await city_service.create_city( + session, + name=unique("Testville"), + timezone="Europe/Dublin", + latitude=53.35, + longitude=-6.26, + ) + return await hotel_service.create_hotel( + session, + name=unique("Test Hotel"), + city_id=city.id, + total_rooms=10, + latitude=None, + longitude=None, + ) + + +# Repository + + +async def test_create_user_stores_the_google_identity(session: AsyncSession) -> None: + google_id, email = unique("google"), unique_email() + + user = await user_repo.create_user( + session, google_id=google_id, email=email, name="Ada Lovelace" + ) + + assert user is not None + assert (user.google_id, user.email, user.name) == (google_id, email, "Ada Lovelace") + assert user.created_at is not None + assert user.last_signed_in_at is not None + + +async def test_new_user_has_no_hotels(session: AsyncSession) -> None: + user = await user_repo.create_user( + session, google_id=unique("google"), email=unique_email(), name=None + ) + + assert user is not None + assert await user_repo.list_hotel_ids(session, user.id) == [] + + +async def test_create_user_returns_none_for_a_known_google_id( + session: AsyncSession, +) -> None: + google_id = unique("google") + await user_repo.create_user( + session, google_id=google_id, email=unique_email(), name=None + ) + + again = await user_repo.create_user( + session, google_id=google_id, email=unique_email(), name=None + ) + + assert again is None + + +async def test_get_user_by_google_id(session: AsyncSession) -> None: + google_id = unique("google") + created = await user_repo.create_user( + session, google_id=google_id, email=unique_email(), name=None + ) + + found = await user_repo.get_user_by_google_id(session, google_id) + + assert created is not None + assert found is not None + assert found.id == created.id + assert await user_repo.get_user_by_google_id(session, unique("google")) is None + + +async def test_users_are_found_by_email_ignoring_case(session: AsyncSession) -> None: + email = f"Ada.{unique('x')}@Example.com" + created = await user_repo.create_user( + session, google_id=unique("google"), email=email, name=None + ) + + assert created is not None + for query in (email.lower(), email.upper()): + found = await user_repo.list_users_by_email(session, query) + assert [u.id for u in found] == [created.id] + + +async def test_link_hotel_twice_keeps_one_link(session: AsyncSession) -> None: + user = await user_repo.create_user( + session, google_id=unique("google"), email=unique_email(), name=None + ) + hotel = await make_hotel(session) + assert user is not None + + first = await user_repo.link_hotel(session, user_id=user.id, hotel_id=hotel.id) + second = await user_repo.link_hotel(session, user_id=user.id, hotel_id=hotel.id) + + assert (first, second) == (True, False) + assert await user_repo.list_hotel_ids(session, user.id) == [hotel.id] + + +async def test_user_can_belong_to_several_hotels(session: AsyncSession) -> None: + user = await user_repo.create_user( + session, google_id=unique("google"), email=unique_email(), name=None + ) + hotels = [await make_hotel(session), await make_hotel(session)] + assert user is not None + + for hotel in hotels: + await user_repo.link_hotel(session, user_id=user.id, hotel_id=hotel.id) + + assert await user_repo.list_hotel_ids(session, user.id) == sorted( + h.id for h in hotels + ) + + +async def test_hotel_can_have_several_users(session: AsyncSession) -> None: + hotel = await make_hotel(session) + + for _ in range(2): + user = await user_repo.create_user( + session, google_id=unique("google"), email=unique_email(), name=None + ) + assert user is not None + assert await user_repo.link_hotel(session, user_id=user.id, hotel_id=hotel.id) + assert await user_repo.list_hotel_ids(session, user.id) == [hotel.id] + + +# Service + + +async def test_create_user_then_find_by_google_id(session: AsyncSession) -> None: + google_id = unique("google") + created = await user_service.create_user( + session, google_id=google_id, email=unique_email(), name="Ada Lovelace" + ) + + found = await user_service.get_user_by_google_id(session, google_id) + + assert found.id == created.id + assert found.name == "Ada Lovelace" + + +async def test_create_user_for_a_known_google_account_conflicts( + session: AsyncSession, +) -> None: + google_id = unique("google") + await user_service.create_user( + session, google_id=google_id, email=unique_email(), name=None + ) + + with pytest.raises(ConflictError): + await user_service.create_user( + session, google_id=google_id, email=unique_email(), name=None + ) + + +async def test_unknown_google_id_is_not_found(session: AsyncSession) -> None: + with pytest.raises(NotFoundError): + await user_service.get_user_by_google_id(session, unique("google")) + + +async def test_link_user_to_hotel_is_idempotent(session: AsyncSession) -> None: + user = await user_service.create_user( + session, google_id=unique("google"), email=unique_email(), name=None + ) + hotel = await make_hotel(session) + + first = await user_service.link_user_to_hotel( + session, user_id=user.id, hotel_id=hotel.id + ) + second = await user_service.link_user_to_hotel( + session, user_id=user.id, hotel_id=hotel.id + ) + + assert (first, second) == (True, False) + assert await user_service.list_hotel_ids(session, user.id) == [hotel.id] + + +async def test_link_needs_an_existing_user_and_hotel(session: AsyncSession) -> None: + user = await user_service.create_user( + session, google_id=unique("google"), email=unique_email(), name=None + ) + hotel = await make_hotel(session) + + with pytest.raises(NotFoundError): + await user_service.link_user_to_hotel( + session, user_id=MISSING_ID, hotel_id=hotel.id + ) + with pytest.raises(NotFoundError): + await user_service.link_user_to_hotel( + session, user_id=user.id, hotel_id=MISSING_ID + ) + assert await user_service.list_hotel_ids(session, user.id) == [] + + +async def test_get_user_by_email_ignores_case(session: AsyncSession) -> None: + email = f"Ada.{unique('x')}@Example.com" + created = await user_service.create_user( + session, google_id=unique("google"), email=email, name=None + ) + + found = await user_service.get_user_by_email(session, email.lower()) + + assert found.id == created.id + + +async def test_unknown_email_is_not_found(session: AsyncSession) -> None: + with pytest.raises(NotFoundError): + await user_service.get_user_by_email(session, unique_email()) + + +async def test_shared_email_is_a_conflict_not_a_guess(session: AsyncSession) -> None: + email = unique_email() + for _ in range(2): + await user_service.create_user( + session, google_id=unique("google"), email=email, name=None + ) + + with pytest.raises(ConflictError): + await user_service.get_user_by_email(session, email) diff --git a/docs/decisions/0013-one-transaction-per-unit-of-work.md b/docs/decisions/0013-one-transaction-per-unit-of-work.md index f76bb25..bfff4b3 100644 --- a/docs/decisions/0013-one-transaction-per-unit-of-work.md +++ b/docs/decisions/0013-one-transaction-per-unit-of-work.md @@ -20,3 +20,5 @@ hits a unique constraint). - Every request is all-or-nothing. - The session dependency runs with `scope="function"`, so the commit happens before the response is sent. A failed commit is a 500, never a false 200. +- See also [0036](0036-operator-commands-in-cli.md): an operator command is a + third place that commits. diff --git a/docs/decisions/0035-users-keyed-on-google-id.md b/docs/decisions/0035-users-keyed-on-google-id.md new file mode 100644 index 0000000..91f07a5 --- /dev/null +++ b/docs/decisions/0035-users-keyed-on-google-id.md @@ -0,0 +1,30 @@ +# 0035. Users are identified by their Google account id + +**Status:** Accepted · **Date:** 2026-10-25 + +## Context +The API has no idea who is calling it, so it can't limit what someone sees or +record who made a decision. Sign-in will be Google only. + +## Decision +- A user is one Google account, identified by Google's account id (the `sub` + claim), stored as `users.google_id` and unique. Never by email: an address + can change. +- `users.email` is not unique. An address can pass to another person before + its previous owner signs in again and the row is updated, and a unique + constraint would then fail that person's sign-in. Anything that finds a user + by email (`just link-user`) ignores case and refuses to choose when more than + one user matches, because picking one would grant a hotel's access to the + wrong person. +- `user_hotels` links users to hotels. Its primary key is (user_id, hotel_id), + so a pair can be linked only once, and it records when. Deleting a user or a + hotel removes its links. There are no roles yet. +- A user with no hotel is valid: they have signed up and aren't linked yet. +- There are no passwords. + +## Consequences +- Linking is idempotent: linking the same pair again changes nothing. +- A hotel can have several users and a user can belong to several hotels. + Adding roles later is a column on `user_hotels`. +- This is storage only. Signing in, and checking who is calling (which + replaces [0026](0026-no-authentication.md)), come later. diff --git a/docs/decisions/0036-operator-commands-in-cli.md b/docs/decisions/0036-operator-commands-in-cli.md new file mode 100644 index 0000000..2aba223 --- /dev/null +++ b/docs/decisions/0036-operator-commands-in-cli.md @@ -0,0 +1,25 @@ +# 0036. Operator commands live in `cli.py` and commit once + +**Status:** Accepted · **Date:** 2026-10-25 + +## Context +Linking a user to a hotel has no place in the app yet, so it is done from the +command line. [0013](0013-one-transaction-per-unit-of-work.md) says exactly two +places commit, the request and the job runner, and a command-line process is +neither. + +## Decision +- Operator commands live in `foresight/cli.py` and run as + `python -m foresight.cli `. Each has a `just` recipe that takes + `--neon` like the others (`just link-user `). +- A command is a third place that commits, alongside the two in 0013. It opens + a session from `async_session_factory` and commits once, when the whole + command has succeeded. A `DomainError` is printed, the command exits 1, and + nothing is committed. +- It reaches data only through services ([0012](0012-layered-architecture.md)). + `cli.py` holds no queries and no business rules. +- A command is for something the app can't do yet, and goes when the app can. + +## Consequences +- A command is all-or-nothing, like a request. +- A new command is a subparser and a function in `cli.py`, plus a recipe. diff --git a/docs/decisions/README.md b/docs/decisions/README.md index a99b424..16376cb 100644 --- a/docs/decisions/README.md +++ b/docs/decisions/README.md @@ -55,3 +55,5 @@ Number files sequentially (`0027-short-title.md`) and add them to the index. | [0032](0032-events-on-a-night-follow-the-model.md) | The events shown for a night follow the model's rule | API | | [0033](0033-data-freshness-rules.md) | Freshness comes from jobs; the service decides what's stale | API | | [0034](0034-snapshot-only-when-changed.md) | A snapshot is saved only when the model, rate or range changed | data model | +| [0035](0035-users-keyed-on-google-id.md) | Users are identified by their Google account id | data model | +| [0036](0036-operator-commands-in-cli.md) | Operator commands live in `cli.py` and commit once | architecture | diff --git a/docs/system-design.md b/docs/system-design.md index 9a66780..bb99c7e 100644 --- a/docs/system-design.md +++ b/docs/system-design.md @@ -83,6 +83,7 @@ backend/foresight/ main.py app, routers, error handlers, lifespan config.py settings (pydantic-settings) worker.py runs jobs; commits per chunk + cli.py operator commands (`just link-user`); commits once per command routes/ helpers.py SessionDep, schema bases, PageResponse, paginate errors.py domain error → status code @@ -91,7 +92,7 @@ backend/foresight/ services/ errors.py NotFound / Conflict / InvalidRequest / NotConfigured job.py job kinds, validation, handlers, lifecycle - domain/ city, hotel, weather, weather_coverage, event, flight, + domain/ city, hotel, user, weather, weather_coverage, event, flight, model_inputs, night, pricing_model, recommendation, freshness ingest/ weather, event, performance, window adapters/ open_meteo, predicthq, asknews, aerodatabox, opensky, cso, @@ -99,11 +100,11 @@ backend/foresight/ ml/ base, ebm, features, weighting repository/ upsert.py pagination.py - city.py hotel.py job.py pricing_model.py recommendation.py + city.py hotel.py job.py pricing_model.py recommendation.py user.py signals/ weather, event, flight, competitor_rate database/ base.py enums.py session.py - models/ city, hotel, job, pricing_model, recommendation + models/ city, hotel, job, pricing_model, recommendation, user signals/ weather, event, flight, competitor_rate migrations/ ``` @@ -117,12 +118,14 @@ pattern in this document. ### 3.1 Transactions -Only two places commit ([0013](decisions/0013-one-transaction-per-unit-of-work.md)): +Only three places commit ([0013](decisions/0013-one-transaction-per-unit-of-work.md), +[0036](decisions/0036-operator-commands-in-cli.md)): | Caller | Session from | Commits | |---|---|---| | HTTP request | `database/session.get_session` | once, after the handler; rolls back on any error | | job | `worker.run_job` | after every chunk, then once for the final status | +| operator command | `cli.py`, from `async_session_factory` | once, when the command succeeds; an error commits nothing | Repositories flush when they need a generated id. `update_hotel` and `update_city` run in a savepoint, so a rename that hits a unique constraint @@ -247,6 +250,8 @@ cities ─┬─< hotels ─┬─< room_types ─< daily_performance (the hot ├─< events ─< event_observations │ outside signals, ├─< flight_arrivals │ shared by every hotel └─< competitor_rates ┘ in the city + +users ─< user_hotels >─ hotels (who works for which hotel) ``` **Cities are the market** ([0017](decisions/0017-cities-are-the-market.md)). @@ -255,10 +260,20 @@ centre coordinates and both airport codes. Weather is read and events are searched at the centre; "today" for a job is today in the city's timezone. A hotel keeps its own coordinates for distance to venues. +**Users** ([0035](decisions/0035-users-keyed-on-google-id.md)) are Google +accounts, identified by Google's account id and never by email (`users.email` +is not unique). `user_hotels` links a user to a hotel, keyed on the pair so +linking twice changes nothing; a hotel can have several users, a user can +belong to several hotels, and a user with none is valid. Until the app can do +it, `just link-user ` links by email +([0036](decisions/0036-operator-commands-in-cli.md)). Nothing signs anyone in +yet. + **Foreign keys** always state `ON DELETE`: - `CASCADE` from a city to its signals, and from a hotel to its room types, - nights, pricing models, jobs and recommendation snapshots. + nights, pricing models, jobs and recommendation snapshots, and from a user + or a hotel to its `user_hotels` links. - `RESTRICT` from a hotel to its city: a city with hotels can't be deleted. - `NO ACTION` from a snapshot to its pricing model: a model with snapshots can't be deleted, but deleting the hotel removes both diff --git a/justfile b/justfile index 8a2a19d..f02049b 100644 --- a/justfile +++ b/justfile @@ -85,6 +85,13 @@ migrate-down *flags: (_flags flags) @echo "Database: {{ if flags == "--neon" { "neon" } else { "local" } }}" cd backend && {{ if flags == "--neon" { "" } else { _env_local } }} uv run alembic downgrade -1 +# --- Users --- + +# Link a user to a hotel by email (`just link-user a@b.com 1 --neon` for Neon) +link-user email hotel_id *flags: (_flags flags) + @echo "Database: {{ if flags == "--neon" { "neon" } else { "local" } }}" + cd backend && {{ if flags == "--neon" { "" } else { _env_local } }} uv run python -m foresight.cli link-user "{{email}}" {{hotel_id}} + # --- Testing --- # Run the test suite