Improve student activity registration system#2
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f6d96998a1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
This PR enhances the student activity registration system by adding participant management (duplicate prevention + unregister support), updating the frontend to display and remove participants, and introducing pytest-based API tests.
Changes:
- Backend: prevent duplicate signups and add an unregister endpoint.
- Frontend: render participant lists per activity and support unregister via a remove button.
- Testing: add pytest fixtures + API tests for success and failure cases.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/app.py |
Adds duplicate-signup validation and a DELETE endpoint to unregister participants; expands seeded activities. |
src/static/app.js |
Adds cache-busted activity fetching, renders participants, and implements unregister UI flow. |
src/static/styles.css |
Adds participant list / remove-button styling. |
tests/conftest.py |
Adds FastAPI TestClient fixture and autouse state reset for the in-memory activities store. |
tests/test_app.py |
Adds API tests for listing activities, signup, duplicate prevention, and unregister behavior. |
requirements.txt |
Adds pytest dependency. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/static/app.js
Outdated
| }); | ||
|
|
||
| activitiesList.addEventListener("click", async (event) => { | ||
| const removeButton = event.target.closest(".participant-remove-btn"); |
There was a problem hiding this comment.
event.target is not guaranteed to be an Element (it can be a Text node), so calling event.target.closest(...) can throw at runtime. Guard with an instanceof Element check (or use event.composedPath() / start from event.target.parentElement) before calling .closest.
| const removeButton = event.target.closest(".participant-remove-btn"); | |
| const target = event.target; | |
| if (!(target instanceof Element)) { | |
| return; | |
| } | |
| const removeButton = target.closest(".participant-remove-btn"); |
| # Validate student is not already signed up | ||
| if email in activity["participants"]: | ||
| raise HTTPException(status_code=400, detail="Student already signed up") | ||
|
|
||
| # Add student | ||
| activity["participants"].append(email) |
There was a problem hiding this comment.
The backend accepts email as an unconstrained str and stores it directly. This makes duplicates easy to bypass via case/whitespace differences (e.g., User@... vs user@...) and allows non-email / potentially unsafe strings to be persisted and later rendered by the frontend. Consider validating and normalizing the email (e.g., pydantic.EmailStr + strip() + lowercase) before checking membership / appending.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@copilot open a new pull request to apply changes based on the comments in this thread |
|
@mrobardsKO I've opened a new pull request, #3, to work on those changes. Once the pull request is ready, I'll request review from you. |
…alization in app.py Co-authored-by: mrobardsKO <113369208+mrobardsKO@users.noreply.github.com>
Harden registration system: XSS-safe DOM, email normalization, instanceof guard
This pull request adds new features for managing participants in school activities, improves the frontend to display and allow removal of participants, and introduces comprehensive backend and frontend validation. It also adds automated tests and test fixtures to ensure reliability and prevent regressions. The changes are grouped below by theme.
Backend features and validation:
unregister_from_activity) with validation for activity existence and participant status.Frontend improvements:
app.js) to fetch activities with cache-busting and display participants for each activity, including a button to remove participants.Testing and reliability: