Conversation
There was a problem hiding this comment.
Pull request overview
Adds initial “Game” functionality to the FastAPI backend, including endpoints to host a game (code + QR) and join a game by code, backed by new/updated SQLAlchemy models.
Changes:
- Added
POST /api/gamesto create a game with a unique 6-char code and base64 QR image. - Added
POST /api/games/join/{code}to join a game and prevent duplicate joins. - Updated DB models for the new
Gameschema and added aGameParticipantassociation table; wired routes into the app and addedcreate_all.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
| app/routes/game.py | New game create/join API plus code/QR generation logic. |
| app/main.py | Registers the new game router and creates tables on startup/import. |
| app/db/models.py | Refactors Game model fields and adds GameParticipant; fixes relationship() targets for GameUserBingo. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
app/db/models.py
Outdated
| @@ -31,6 +44,7 @@ class Bingo(Base): | |||
| col=Column(Integer, nullable=False) | |||
| bingo_char=Column(String, nullable=False) | |||
| image_link=Column(String, unique=True) | |||
| game_user_bingos = relationship("GameUserBingo", back_populates="bingo") | |||
|
|
|||
| class GameUserBingo(Base): | |||
| __tablename__ = "game_user_bingo" | |||
| @@ -39,6 +53,16 @@ class GameUserBingo(Base): | |||
| user_id = Column(ForeignKey("user.id"), primary_key=True) | |||
| bingo_id = Column(ForeignKey("bingo.id"), primary_key=True) | |||
|
|
|||
| game = relationship("game", back_populates="game_user_bingos") | |||
| user = relationship("user", back_populates="game_user_bingos") | |||
| bingo = relationship("bingo", back_populates="game_user_bingos") | |||
| game = relationship("Game", back_populates="game_user_bingos") | |||
| user = relationship("User", back_populates="game_user_bingos") | |||
| bingo = relationship("Bingo", back_populates="game_user_bingos") | |||
|
|
|||
|
|
|||
| class GameParticipant(Base): | |||
| __tablename__ = "game_participant" | |||
|
|
|||
| game_id = Column(ForeignKey("game.id"), primary_key=True) | |||
| user_id = Column(ForeignKey("user.id"), primary_key=True) | |||
|
|
|||
There was a problem hiding this comment.
host_id, winner_id, and the FK columns in GameUserBingo / GameParticipant are declared as Column(ForeignKey(...)) without an explicit column type (e.g., Integer). With sqlalchemy.Column, this is invalid and will raise at import/model-metadata time. Define these as Column(Integer, ForeignKey(...), ...) (or switch to SQLAlchemy 2.0 mapped_column(ForeignKey(...)) consistently).
| class Game(Base): | ||
| __tablename__="game" | ||
| __table_args__ = ( | ||
| CheckConstraint('start_time < end_time'), | ||
| CheckConstraint('host != winner') | ||
| ) | ||
| id=Column(Integer, primary_key=True) | ||
| host=Column(Integer, ForeignKey("user.id")) | ||
| description=Column(Text, nullable=False) | ||
| start_time=Column(DateTime, default=lambda: datetime.now(timezone.utc)) | ||
| end_time=Column(DateTime, nullable=False) | ||
| winner=Column(Integer, ForeignKey("user.id")) | ||
| __tablename__ = "game" | ||
|
|
||
| id = Column(Integer, primary_key=True) | ||
|
|
||
| host_id = Column(ForeignKey("user.id"), nullable=False) | ||
| winner_id = Column(ForeignKey("user.id"), nullable=True) | ||
|
|
||
| start_time = Column(DateTime, default=lambda: datetime.now(timezone.utc)) | ||
| end_time = Column(DateTime, nullable=False) | ||
|
|
||
| description = Column(Text, nullable=False) | ||
| location = Column(String(255), nullable=False) | ||
|
|
||
| code = Column(String(6), unique=True, nullable=False) | ||
|
|
||
| size = Column(Integer, nullable=False) | ||
|
|
||
| qr_img = Column(Text, nullable=True) | ||
|
|
There was a problem hiding this comment.
The previous Game model had integrity constraints (start_time < end_time and host != winner). These were removed when renaming columns to host_id/winner_id, which allows invalid games to be persisted (e.g., negative duration or host == winner). Consider reintroducing equivalent CheckConstraints using the new column names.
app/main.py
Outdated
| Base.metadata.create_all(bind=engine) | ||
|
|
||
| app = FastAPI(title="amMingo") | ||
|
|
There was a problem hiding this comment.
Base.metadata.create_all(bind=engine) runs at import time. This can have unwanted side effects (e.g., failing app import if DB is temporarily unavailable, multiple workers racing on startup) and makes schema changes harder to manage. Prefer running table creation in a FastAPI startup event (or adopting a migration tool) so startup behavior is explicit and controllable.
| Base.metadata.create_all(bind=engine) | |
| app = FastAPI(title="amMingo") | |
| app = FastAPI(title="amMingo") | |
| @app.on_event("startup") | |
| def on_startup() -> None: | |
| Base.metadata.create_all(bind=engine) |
| def create_unique_code(db: Session): | ||
| while True: | ||
| code = generate_game_code() | ||
| existing = db.query(Game).filter(Game.code == code).first() | ||
| if not existing: | ||
| return code | ||
|
|
||
|
|
||
| def generate_qr_base64(code: str): | ||
| join_url = f"http://127.0.0.1:8000/api/games/join/{code}" | ||
|
|
||
| qr = qrcode.make(join_url) | ||
|
|
||
| buffer = BytesIO() | ||
| qr.save(buffer, format="PNG") | ||
|
|
||
| return base64.b64encode(buffer.getvalue()).decode() | ||
|
|
||
|
|
||
|
|
||
|
|
||
| @router.post("/games") | ||
| def create_game( | ||
| data: CreateGameRequest, | ||
| db: Session = Depends(get_db) | ||
| ): | ||
|
|
||
|
|
||
| host_id = data.host_id | ||
|
|
||
| code = create_unique_code(db) | ||
| qr_img = generate_qr_base64(code) | ||
| start_time = datetime.now(timezone.utc) | ||
| end_time = start_time + timedelta(minutes=data.duration) | ||
|
|
||
| new_game = Game( | ||
| host_id=data.host_id, | ||
| description=data.description, | ||
| location=data.location, | ||
| start_time=start_time, | ||
| end_time=end_time, | ||
| size=data.size, | ||
| code=code, | ||
| qr_img=qr_img | ||
| ) | ||
|
|
||
| db.add(new_game) | ||
| db.commit() | ||
| db.refresh(new_game) |
There was a problem hiding this comment.
create_unique_code checks for an unused code before insert, but concurrent requests can still generate the same code and then hit the DB UNIQUE constraint on commit. Handle IntegrityError on db.commit() (or generate/insert in a retry loop) so collisions/races return a clean retry instead of a 500.
|
|
||
| code = Column(String(6), unique=True, nullable=False) | ||
|
|
||
| size = Column(Integer, nullable=False) |
There was a problem hiding this comment.
It's best if you make size an enum with 3, 4 and 5 as possible values
Or just make another check, either one works
| size: Literal[3, 4, 5] | ||
|
|
||
| class JoinGameRequest(BaseModel): | ||
| user_id: int No newline at end of file |
There was a problem hiding this comment.
Shouldn't joingame need game id as well ?
| class Game(Base): | ||
| __tablename__="game" | ||
| __table_args__ = ( | ||
| CheckConstraint('start_time < end_time'), | ||
| CheckConstraint('host != winner') | ||
| ) | ||
| id=Column(Integer, primary_key=True) | ||
| host=Column(Integer, ForeignKey("user.id")) | ||
| description=Column(Text, nullable=False) | ||
| start_time=Column(DateTime, default=lambda: datetime.now(timezone.utc)) | ||
| end_time=Column(DateTime, nullable=False) | ||
| winner=Column(Integer, ForeignKey("user.id")) | ||
| __tablename__ = "game" | ||
|
|
||
| id = Column(Integer, primary_key=True) | ||
|
|
||
| host_id = Column(ForeignKey("user.id"), nullable=False) | ||
| winner_id = Column(ForeignKey("user.id"), nullable=True) | ||
|
|
||
| start_time = Column(DateTime, default=lambda: datetime.now(timezone.utc)) | ||
| end_time = Column(DateTime, nullable=False) | ||
|
|
||
| description = Column(Text, nullable=False) | ||
| location = Column(String(255), nullable=False) | ||
|
|
||
| code = Column(String(6), unique=True, nullable=False) | ||
|
|
||
| size = Column(Integer, nullable=False) | ||
|
|
||
| qr_img = Column(Text, nullable=True) | ||
|
|
Changes Made: