-
Notifications
You must be signed in to change notification settings - Fork 0
Improve student activity registration system #2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
3645f34
1fa7503
1ccdfcc
a637a20
e1d27c0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| fastapi | ||
| uvicorn | ||
| httpx | ||
| watchfiles | ||
| watchfiles | ||
| pytest |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -20,11 +20,26 @@ document.addEventListener("DOMContentLoaded", () => { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const spotsLeft = details.max_participants - details.participants.length; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const participantsList = details.participants.length | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ? details.participants.map(email => | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| `<li> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <span class="participant-email">${email}</span> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <button class="remove-btn" data-activity="${name}" data-email="${email}" title="Remove participant">✕</button> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <button class="remove-btn" data-activity="${name}" data-email="${email}" title="Remove participant">✕</button> | |
| <button class="remove-btn" data-activity="${name}" data-email="${email}" title="Remove participant" aria-label="Remove participant ${email}">✕</button> |
Copilot
AI
Mar 31, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The UI is built via innerHTML with interpolated name, details.description, and especially email (user-controlled via signup). This allows HTML/attribute injection (XSS) if an email contains markup/quotes. Prefer creating DOM nodes and setting textContent, or sanitize/escape values before inserting into HTML.
| const participantsList = details.participants.length | |
| ? details.participants.map(email => | |
| `<li> | |
| <span class="participant-email">${email}</span> | |
| <button class="remove-btn" data-activity="${name}" data-email="${email}" title="Remove participant">✕</button> | |
| </li>` | |
| ).join("") | |
| : "<li class='no-participants'>No participants yet</li>"; | |
| activityCard.innerHTML = ` | |
| <h4>${name}</h4> | |
| <p>${details.description}</p> | |
| <p><strong>Schedule:</strong> ${details.schedule}</p> | |
| <p><strong>Availability:</strong> ${spotsLeft} spots left</p> | |
| <div class="participants-section"> | |
| <strong>Participants:</strong> | |
| <ul class="participants-list"> | |
| ${participantsList} | |
| </ul> | |
| </div> | |
| `; | |
| // Build activity card content safely using DOM APIs to avoid XSS | |
| const titleEl = document.createElement("h4"); | |
| titleEl.textContent = name; | |
| const descEl = document.createElement("p"); | |
| descEl.textContent = details.description; | |
| const scheduleEl = document.createElement("p"); | |
| const scheduleStrong = document.createElement("strong"); | |
| scheduleStrong.textContent = "Schedule:"; | |
| scheduleEl.appendChild(scheduleStrong); | |
| scheduleEl.appendChild(document.createTextNode(" " + details.schedule)); | |
| const availabilityEl = document.createElement("p"); | |
| const availabilityStrong = document.createElement("strong"); | |
| availabilityStrong.textContent = "Availability:"; | |
| availabilityEl.appendChild(availabilityStrong); | |
| availabilityEl.appendChild(document.createTextNode(" " + spotsLeft + " spots left")); | |
| const participantsSection = document.createElement("div"); | |
| participantsSection.className = "participants-section"; | |
| const participantsLabel = document.createElement("strong"); | |
| participantsLabel.textContent = "Participants:"; | |
| participantsSection.appendChild(participantsLabel); | |
| const participantsUl = document.createElement("ul"); | |
| participantsUl.className = "participants-list"; | |
| if (details.participants.length) { | |
| details.participants.forEach(email => { | |
| const li = document.createElement("li"); | |
| const emailSpan = document.createElement("span"); | |
| emailSpan.className = "participant-email"; | |
| emailSpan.textContent = email; | |
| const removeBtn = document.createElement("button"); | |
| removeBtn.className = "remove-btn"; | |
| removeBtn.dataset.activity = name; | |
| removeBtn.dataset.email = email; | |
| removeBtn.title = "Remove participant"; | |
| removeBtn.textContent = "✕"; | |
| li.appendChild(emailSpan); | |
| li.appendChild(removeBtn); | |
| participantsUl.appendChild(li); | |
| }); | |
| } else { | |
| const li = document.createElement("li"); | |
| li.className = "no-participants"; | |
| li.textContent = "No participants yet"; | |
| participantsUl.appendChild(li); | |
| } | |
| participantsSection.appendChild(participantsUl); | |
| activityCard.appendChild(titleEl); | |
| activityCard.appendChild(descEl); | |
| activityCard.appendChild(scheduleEl); | |
| activityCard.appendChild(availabilityEl); | |
| activityCard.appendChild(participantsSection); |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -74,6 +74,62 @@ section h3 { | |||||||||||||||||||
| margin-bottom: 8px; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| .participants-section { | ||||||||||||||||||||
| margin-top: 10px; | ||||||||||||||||||||
| padding-top: 8px; | ||||||||||||||||||||
| border-top: 1px dashed #ddd; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| .participants-list { | ||||||||||||||||||||
| list-style: none; | ||||||||||||||||||||
| padding: 0; | ||||||||||||||||||||
| margin: 6px 0 0 0; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| .participants-list li { | ||||||||||||||||||||
| padding: 4px 8px; | ||||||||||||||||||||
| position: relative; | ||||||||||||||||||||
| font-size: 14px; | ||||||||||||||||||||
| color: #555; | ||||||||||||||||||||
| display: flex; | ||||||||||||||||||||
| align-items: center; | ||||||||||||||||||||
| justify-content: space-between; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| .participants-list li::before { | ||||||||||||||||||||
| content: none; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| .participant-email { | ||||||||||||||||||||
| flex: 1; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| .remove-btn { | ||||||||||||||||||||
| background: none; | ||||||||||||||||||||
| border: none; | ||||||||||||||||||||
| color: #c62828; | ||||||||||||||||||||
| cursor: pointer; | ||||||||||||||||||||
| font-size: 14px; | ||||||||||||||||||||
| padding: 2px 6px; | ||||||||||||||||||||
| border-radius: 3px; | ||||||||||||||||||||
| transition: background-color 0.2s; | ||||||||||||||||||||
| line-height: 1; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| .remove-btn:hover { | ||||||||||||||||||||
| background-color: #ffebee; | ||||||||||||||||||||
| color: #b71c1c; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
||||||||||||||||||||
| .remove-btn:focus-visible, | |
| .remove-btn:focus { | |
| background-color: #ffebee; | |
| color: #b71c1c; | |
| outline: 2px solid #b71c1c; | |
| outline-offset: 2px; | |
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,129 @@ | ||
| """Tests for the Mergington High School API endpoints.""" | ||
|
|
||
| import copy | ||
| import pytest | ||
| from fastapi.testclient import TestClient | ||
| from src.app import app, activities | ||
|
|
||
|
|
||
| @pytest.fixture(autouse=True) | ||
| def reset_activities(): | ||
| """Reset the activities dict to its original state before each test.""" | ||
| original = copy.deepcopy(activities) | ||
| yield | ||
| activities.clear() | ||
| activities.update(original) | ||
|
|
||
|
|
||
| client = TestClient(app) | ||
|
|
||
|
|
||
| def test_get_activities(): | ||
| # Arrange | ||
| expected_keys = {"description", "schedule", "max_participants", "participants"} | ||
|
|
||
| # Act | ||
| response = client.get("/activities") | ||
|
|
||
| # Assert | ||
| assert response.status_code == 200 | ||
| data = response.json() | ||
| assert len(data) == 9 | ||
| for name, details in data.items(): | ||
| assert expected_keys.issubset(details.keys()), f"{name} missing keys" | ||
|
|
||
|
|
||
| def test_signup_success(): | ||
| # Arrange | ||
| activity_name = "Chess Club" | ||
| email = "newstudent@mergington.edu" | ||
|
|
||
| # Act | ||
| response = client.post( | ||
| f"/activities/{activity_name}/signup", | ||
| params={"email": email}, | ||
| ) | ||
|
|
||
| # Assert | ||
| assert response.status_code == 200 | ||
| assert email in activities[activity_name]["participants"] | ||
|
|
||
|
|
||
| def test_signup_duplicate(): | ||
| # Arrange | ||
| activity_name = "Chess Club" | ||
| email = "michael@mergington.edu" # already in participants | ||
|
|
||
| # Act | ||
| response = client.post( | ||
| f"/activities/{activity_name}/signup", | ||
| params={"email": email}, | ||
| ) | ||
|
|
||
| # Assert | ||
| assert response.status_code == 400 | ||
| assert "already signed up" in response.json()["detail"].lower() | ||
|
|
||
|
|
||
| def test_signup_nonexistent_activity(): | ||
| # Arrange | ||
| activity_name = "Nonexistent Activity" | ||
| email = "someone@mergington.edu" | ||
|
|
||
| # Act | ||
| response = client.post( | ||
| f"/activities/{activity_name}/signup", | ||
| params={"email": email}, | ||
| ) | ||
|
|
||
| # Assert | ||
| assert response.status_code == 404 | ||
| assert "not found" in response.json()["detail"].lower() | ||
|
|
||
|
|
||
| def test_unregister_success(): | ||
| # Arrange | ||
| activity_name = "Chess Club" | ||
| email = "michael@mergington.edu" # existing participant | ||
|
|
||
| # Act | ||
| response = client.delete( | ||
| f"/activities/{activity_name}/signup", | ||
| params={"email": email}, | ||
| ) | ||
|
|
||
| # Assert | ||
| assert response.status_code == 200 | ||
| assert email not in activities[activity_name]["participants"] | ||
|
|
||
|
|
||
| def test_unregister_not_found(): | ||
| # Arrange | ||
| activity_name = "Chess Club" | ||
| email = "nonexistent@mergington.edu" | ||
|
|
||
| # Act | ||
| response = client.delete( | ||
| f"/activities/{activity_name}/signup", | ||
| params={"email": email}, | ||
| ) | ||
|
|
||
| # Assert | ||
| assert response.status_code == 404 | ||
| assert "not found" in response.json()["detail"].lower() | ||
|
|
||
|
|
||
| def test_unregister_nonexistent_activity(): | ||
| # Arrange | ||
| activity_name = "Nonexistent Activity" | ||
| email = "someone@mergington.edu" | ||
|
|
||
| # Act | ||
| response = client.delete( | ||
| f"/activities/{activity_name}/signup", | ||
| params={"email": email}, | ||
| ) | ||
|
|
||
| # Assert | ||
| assert response.status_code == 404 | ||
| assert "not found" in response.json()["detail"].lower() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
signup_for_activityappends the student without enforcingmax_participants, so activities can be overbooked (and the frontend will show negative spots left). Add a capacity check before appending and return an appropriate 4xx error when the activity is full.