Skip to content

Improve student activity registration system#2

Merged
terioki merged 4 commits intomainfrom
accelerate-with-copilot
Mar 24, 2026
Merged

Improve student activity registration system#2
terioki merged 4 commits intomainfrom
accelerate-with-copilot

Conversation

@terioki
Copy link
Copy Markdown
Owner

@terioki terioki commented Mar 24, 2026

This pull request introduces several major improvements to the activities management app, focusing on enhanced participant management, user interface updates, backend API changes, and the addition of comprehensive automated tests. The most important changes include the ability to unregister participants from activities, improved frontend display and interactivity for managing participants, and robust backend and test coverage to ensure reliability.

Backend functionality enhancements:

  • Added a new DELETE endpoint /activities/{activity_name}/participants in app.py to allow unregistering a student from an activity, including validation for activity and participant existence.
  • Updated the signup logic to prevent duplicate signups by checking if a student is already registered for an activity before adding them.
  • Expanded the set of available activities in the in-memory activities data structure, introducing new clubs and teams.

Frontend and UI improvements:

  • Enhanced the activity cards in app.js to display the list of participants for each activity, including a delete button next to each participant for quick removal.
  • Implemented event handling for unregistering participants via the UI, with appropriate feedback messages and error handling.
  • Refined the visual design in styles.css for activity cards and participant lists, adding a modern look and clear separation for participant management. [1] [2]

Testing and reliability:

  • Added a comprehensive test suite in tests/test_app.py covering all API endpoints, including signup, unregister, error cases, and activity retrieval, with automatic state reset between tests for isolation.

@terioki terioki requested a review from Copilot March 24, 2026 11:17
@terioki terioki merged commit 7ae3d32 into main Mar 24, 2026
8 checks passed
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR enhances the student activities app by adding participant unregistration support end-to-end (API + UI) and introducing an automated test suite to cover the activity management endpoints.

Changes:

  • Added a DELETE API endpoint to unregister a participant and prevented duplicate signups in the signup endpoint.
  • Updated the frontend to display participants per activity and allow unregistering via a delete button, with improved message handling.
  • Added pytest-based API tests with automatic reset of the in-memory activities state, plus minor dependency updates.

Reviewed changes

Copilot reviewed 4 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/app.py Expands seeded activities, prevents duplicate signups, adds DELETE unregister endpoint.
src/static/app.js Renders participants per activity, adds unregister UI flow, refactors user messaging.
src/static/styles.css Introduces styling for participant list and delete controls; refreshes activity card styling.
tests/test_app.py Adds API test suite for root, activities listing, signup, and unregister flows with isolation fixture.
requirements.txt Adds pytest dependency and fixes watchfiles line formatting.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/static/app.js
Comment on lines 6 to +13

function showMessage(text, type) {
messageDiv.textContent = text;
messageDiv.className = type;
messageDiv.classList.remove("hidden");

setTimeout(() => {
messageDiv.classList.add("hidden");
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

showMessage() overwrites messageDiv.className and starts a new setTimeout on every call without clearing any prior timeout. This can cause the message to be hidden unexpectedly if multiple actions happen within 5 seconds, and it also makes it hard to preserve any base styling classes on the message element. Consider keeping a single timeout handle (clear it before setting a new one) and setting classes in a way that preserves any baseline class (e.g., add/remove success/error rather than replacing the entire class list).

Suggested change
function showMessage(text, type) {
messageDiv.textContent = text;
messageDiv.className = type;
messageDiv.classList.remove("hidden");
setTimeout(() => {
messageDiv.classList.add("hidden");
let messageTimeoutId = null;
function showMessage(text, type) {
messageDiv.textContent = text;
// Preserve any baseline classes on messageDiv, only toggle status classes.
messageDiv.classList.remove("success", "error");
if (type) {
messageDiv.classList.add(type);
}
messageDiv.classList.remove("hidden");
// Ensure only one hide timeout is active at a time.
if (messageTimeoutId !== null) {
clearTimeout(messageTimeoutId);
}
messageTimeoutId = setTimeout(() => {
messageDiv.classList.add("hidden");
messageTimeoutId = null;

Copilot uses AI. Check for mistakes.
Comment thread src/static/app.js
Comment on lines +33 to 65
const participantsMarkup = details.participants.length
? details.participants
.map(
(participant) => `
<li class="participant-item">
<span class="participant-email">${participant}</span>
<button
class="participant-delete-button"
type="button"
data-activity="${encodeURIComponent(name)}"
data-email="${encodeURIComponent(participant)}"
aria-label="Unregister ${participant} from ${name}"
>
<svg viewBox="0 0 24 24" aria-hidden="true" focusable="false">
<path d="M9 3h6l1 2h4v2H4V5h4l1-2zm1 6h2v8h-2V9zm4 0h2v8h-2V9zM7 9h2v8H7V9zm-1 11h12l1-12H5l1 12z" />
</svg>
</button>
</li>
`
)
.join("")
: '<li class="participant-empty">No students registered 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">
<p class="participants-title">Participants</p>
<ul class="participants-list">${participantsMarkup}</ul>
</div>
`;
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Participant emails (and activity names) are interpolated directly into innerHTML (including into an attribute via aria-label). Since emails come from user input and the backend doesn’t enforce strict email validation/escaping, this enables HTML injection/XSS in the activity list. Build the participants list with DOM APIs (createElement, textContent, setAttribute) or escape the values before injecting them into HTML strings.

Copilot uses AI. Check for mistakes.
Comment thread tests/test_app.py
Comment on lines +21 to +25
# Arrange: Restore original activities state
activities.clear()
activities.update(copy.deepcopy(ORIGINAL_ACTIVITIES))
yield
# Cleanup: Restore after test completes
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reset_activities fixture’s comments/docstring say there is cleanup after the test, but there is no post-yield restoration logic. Either remove the cleanup comment and the unnecessary yield, or actually restore the state after yield (so failures mid-test don’t leak state into the rest of the session).

Suggested change
# Arrange: Restore original activities state
activities.clear()
activities.update(copy.deepcopy(ORIGINAL_ACTIVITIES))
yield
# Cleanup: Restore after test completes
# Reset activities to the original state before each test
activities.clear()
activities.update(copy.deepcopy(ORIGINAL_ACTIVITIES))

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants