Feat/waitlisting/integration by gabrielhan23 · Pull Request #533 · csmberkeley/csm_web

gabrielhan23 · 2026-02-25T13:01:32Z

No description provided.

…d drop), and auto add to section when student is dropped

… sections db on admin site

…ntend

* update sections * added a count waitlist method * displaying active feature for waitlisted database in admin site * showing only active students when we click on particular section from sections db on admin site * add waitlistCapacity to MetaEditModal and MentorSectionInfo --------- Co-authored-by: Eileen Hwang <eileenhwang@berkeley.edu> Co-authored-by: rajoshi <rajoshibasu@gmail.com>

…m_web into feat/waitlisting/frontend

Co-authored-by: maxmwang <orangemax888@gmail.com>

…t race conditions

cypress · 2026-02-25T13:08:03Z

csm_web Run #445

Run Properties: Failed #445 • 2beb83759c: Feat/waitlisting/integration

Project	`csm_web`
Branch Review	`feat/waitlisting/integration`
Run status	`Failed #445`
Run duration	`04m 28s`
Commit	`2beb83759c: Feat/waitlisting/integration`
Committer	`gabrielhan23`
View all properties for this run ↗︎

Test results
Failures	`30`
Flaky	`0`
Pending	`0`
Skipped	`0`
Passing	`54`
View all changes introduced in this branch ↗︎

Tests for review

section/coordinator-section.cy.ts • 6 failed tests • Tests on Firefox

View Output

Test		Artifacts
modifying students > valid operations > should do nothing if no emails are given		`Screenshots`
modifying students > invalid operations > should retry when adding mentor for another section		`Screenshots`
modifying students > invalid operations > should retry when adding student in another section		`Screenshots`
modifying students > invalid operations > should retry when adding banned student		`Screenshots`
modifying students > invalid operations > should retry when section is at capacity		`Screenshots`
modifying students > invalid operations > should retry with multiple kinds of errors		`Screenshots`

course/student-course.cy.ts • 6 failed tests • Tests on Firefox

View Output

Test		Artifacts
... > should see enroll button for all sections		`Screenshots`
... > should see disabled enroll button and enrollment time for all sections		`Screenshots`
... > should not be able to enroll in any section		`Screenshots`
... > should see enroll button for any section		`Screenshots`
... > should see disabled enroll button and priority enrollment text for all sections		`Screenshots`
... > should not be able to enroll in any section		`Screenshots`

course/restricted-courses.cy.ts • 1 failed test • Tests on Firefox

View Output

Test		Artifacts
whitelisted courses > should be unable to enroll in restricted course before enrollment date		`Screenshots`

section/mentor-section.cy.ts • 1 failed test • Tests on Firefox

View Output

Test		Artifacts
section details accessibility > should be able to navigate to and view section details		`Screenshots`

course/coordinator-course.cy.ts • 1 failed test • Tests on Firefox

View Output

Test		Artifacts
coordinator course view > should display all section information when course is open		`Screenshots`

The first 5 failed specs are shown, see all 10 specs in Cypress Cloud.

Copilot

Pull request overview

This pull request implements a comprehensive waitlist feature for course sections, allowing students to join waitlists when sections are full and be automatically enrolled when spots open up. The feature includes backend models, API endpoints, frontend UI components, and extensive test coverage.

Changes:

Adds WaitlistedStudent model with position-based queuing and automatic enrollment from waitlist when spots open
Implements waitlist API endpoints for adding, dropping, viewing waitlisted students and checking position
Updates frontend to display waitlist information, allow students to join waitlists, and show waitlist positions
Adds comprehensive test suite with 20+ test cases covering various waitlist scenarios

Reviewed changes

Copilot reviewed 27 out of 28 changed files in this pull request and generated 21 comments.

Show a summary per file

File	Description
csm_web/scheduler/models.py	Adds WaitlistedStudent model, waitlist capacity fields, and user permission methods for waitlisting
csm_web/scheduler/views/waitlistedStudent.py	New file with API endpoints for waitlist operations (view, add, drop, position)
csm_web/scheduler/views/section.py	Refactors enrollment logic into helper functions, adds waitlist promotion on drop
csm_web/scheduler/views/student.py	Updates student drop to trigger waitlist promotion, improves logging format
csm_web/scheduler/serializers.py	Adds WaitlistedStudent serializer and waitlist fields to section serializer
csm_web/scheduler/urls.py	Registers waitlist API endpoints
csm_web/scheduler/admin.py	Adds admin interface for WaitlistedStudent model
csm_web/scheduler/factories.py	Adds factory for generating test waitlist data
csm_web/scheduler/tests/models/test_waitlisted_student.py	Comprehensive test suite with 880 lines covering waitlist functionality
csm_web/scheduler/migrations/0035_*.py	Database migration adding waitlist tables and fields
csm_web/frontend/src/components/section/*	UI components for displaying and managing waitlists
csm_web/frontend/src/utils/queries/sections.tsx	React Query hooks for waitlist API operations
csm_web/frontend/src/utils/types.tsx	TypeScript type definitions for waitlist role and section fields
csm_web/frontend/src/components/course/SectionCard.tsx	Updates enrollment UI to show waitlist capacity and handle waitlist joins
.babelrc.json	Removes lodash plugin from Babel configuration
csm_web/pytest.ini	Adds --reuse-db and --no-cacheprovider options

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

Copilot · 2026-02-25T13:08:35Z

csm_web/scheduler/views/student.py

        student.save()
        logger.info(
-            f"<Drop> User {log_str(request.user)} dropped Section {log_str(student.section)} for Student user {log_str(student.user)}"
+            "<Drop> User %s dropped Section %sfor Student user %s",


Missing space in log message. Should be "Section %s for Student user %s" (add space after %s).

Suggested change

"<Drop> User %s dropped Section %sfor Student user %s",

"<Drop> User %s dropped Section %s for Student user %s",

Copilot · 2026-02-25T13:08:36Z

csm_web/scheduler/models.py

    enrollment_end = models.DateTimeField()
    permitted_absences = models.PositiveSmallIntegerField()
-    # time limit for wotd submission;
+    # time limit fdocor wotd submission;


Typo in comment. "fdocor" should be "for".

Suggested change

# time limit fdocor wotd submission;

# time limit for wotd submission;

Copilot · 2026-02-25T13:08:36Z

csm_web/scheduler/views/waitlistedStudent.py

+@api_view(["GET"])
+def view(request, pk=None):
+    """
+    Endpoint: /api/waitlist/<pk>
+    pk = section id
+
+    GET: View all students on the waitlist for a section
+    """
+    section = get_object_or_error(Section.objects, pk=pk)
+    if section.mentor is None:
+        raise NotFound("This section has no mentor assigned.")
+    is_mentor = request.user == section.mentor.user
+    is_coord = bool(
+        section.mentor.course.coordinator_set.filter(user=request.user).count()
+    )
+    if not is_mentor and not is_coord:
+        raise PermissionDenied("You do not have permission to view this waitlist")
+
+    waitlist_queryset = WaitlistedStudent.objects.filter(active=True, section=section)
+    return Response(WaitlistedStudentSerializer(waitlist_queryset, many=True).data)


Missing authentication check. The view, count_waitlist, and position endpoints don't verify that the user is authenticated. While Django REST Framework may handle this at a higher level, it's best practice to be explicit about authentication requirements, especially for sensitive endpoints that return student waitlist information. Consider adding @permission_classes([IsAuthenticated]) or similar decorator to ensure only authenticated users can access these endpoints.

Copilot · 2026-02-25T13:08:36Z

csm_web/scheduler/models.py

+    @property
    def current_student_count(self):
        """Query the number of students currently enrolled in this section."""
        return self.students.filter(active=True).count()


The @property decorator is used here instead of @cached_property. The original code used @functional.cached_property for current_student_count. Since this property now makes a database query every time it's accessed, this could lead to N+1 query problems if the property is accessed multiple times. Consider using @cached_property from functools (Python 3.8+) or Django's equivalent to cache the result within a single request lifecycle.

Copilot · 2026-02-25T13:08:37Z

csm_web/scheduler/views/waitlistedStudent.py

+        section = get_object_or_error(Section.objects, pk=pk)
+        section = Section.objects.select_for_update().get(pk=section.pk)


The section queried at line 51 is then re-queried with select_for_update at line 52, which is redundant. The first query with get_object_or_error is unnecessary since you immediately re-fetch it. Consider using Section.objects.select_for_update().get(pk=pk) directly within the transaction and handling the DoesNotExist exception appropriately (or using get_object_or_error with a queryset that already has select_for_update applied).

Suggested change

section = get_object_or_error(Section.objects, pk=pk)

section = Section.objects.select_for_update().get(pk=section.pk)

section = get_object_or_error(Section.objects.select_for_update(), pk=pk)

Copilot · 2026-02-25T13:08:41Z

csm_web/scheduler/views/section.py

+        for waitlisted_student in waitlisted_students:
+            try:
+                cascade_section_id = swap_into_section(
+                    waitlisted_student.section, waitlisted_student.user
+                )
+            except PermissionDenied:
+                continue
+
+            logger.info(
+                "<Enrollment:Success> User %s removed from all Waitlists for Course %s",
+                log_str(waitlisted_student.user),
+                log_str(waitlisted_student.course),
+            )
+            response = Response(status=status.HTTP_201_CREATED)
+            break


The waitlisted student is not being deactivated after successful enrollment from waitlist. The function documentation at line 198 states "Waitlist student is deactivated" but the code doesn't set waitlisted_student.active = False after successfully adding the student via swap_into_section. The student removal from waitlists happens inside the add_student function, but that only handles the enrolled user's waitlists, not the specific waitlisted_student record being processed here. This needs to be explicitly deactivated.

Copilot · 2026-02-25T13:08:41Z

csm_web/scheduler/views/waitlistedStudent.py

+    return Response(status=status.HTTP_204_NO_CONTENT)
+
+
+def log_enroll_result(success, user, section, reason=None):


The helper function name log_enroll_result is misleading in the context of waitlisting. The function is logging waitlist enrollment, not section enrollment. Consider renaming to log_waitlist_result for clarity.

Copilot · 2026-02-25T13:08:41Z

.babelrc.json

 {
  "presets": ["@babel/preset-react", "@babel/preset-typescript"],
-  "plugins": ["transform-class-properties", "lodash", "@babel/plugin-transform-runtime", "dynamic-import-node"]
+  "plugins": ["transform-class-properties", "@babel/plugin-transform-runtime", "dynamic-import-node"]


The lodash plugin was removed from the Babel configuration. If lodash is used anywhere in the frontend code, this could lead to larger bundle sizes as tree-shaking won't be as effective. Verify that lodash is not used in the codebase, or if it is, ensure that imports are done correctly (e.g., import map from 'lodash/map' instead of import { map } from 'lodash') to enable tree-shaking.

Suggested change

"plugins": ["transform-class-properties", "@babel/plugin-transform-runtime", "dynamic-import-node"]

"plugins": ["transform-class-properties", "@babel/plugin-transform-runtime", "lodash", "dynamic-import-node"]

Copilot · 2026-02-25T13:08:42Z

csm_web/pytest.ini

 [pytest]
 DJANGO_SETTINGS_MODULE = csm_web.settings
 python_files = tests.py test_*.py *_tests.py
+addopts = --reuse-db -p no:cacheprovider


The pytest.ini configuration adds --reuse-db which can cause issues in CI/CD environments or when running tests in parallel. This flag reuses the test database between runs, which can lead to test pollution if not handled carefully. Consider making this an optional flag rather than a default option, especially if tests are run in CI where a clean database state is typically expected.

Suggested change

addopts = --reuse-db -p no:cacheprovider

addopts = -p no:cacheprovider

Copilot · 2026-02-25T13:08:42Z

csm_web/scheduler/views/section.py

+
+    for waitlist in waitlist_set:
+        waitlist.active = False
+        # waitlist.delete()


The commented-out code for deleting waitlisted students (lines 134, 227) should either be removed or have a comment explaining why it's kept. Commented-out code can be confusing for future maintainers and clutters the codebase. If there's a reason to keep the delete approach as a reference, add a comment explaining the decision to use active=False instead of deletion.

Suggested change

# waitlist.delete()

gabrielhan23 and others added 30 commits September 29, 2025 21:29

waitlisting backend code: waitlist student model, apis (view, add, an…

d140eb4

…d drop), and auto add to section when student is dropped

reverted docker compose

fe784c6

added a count waitlist method

af5d7ed

displaying active feature for waitlisted database in admin site

4affe39

showing only active students when we click on particular section from…

260cfdc

… sections db on admin site

update sections

481626c

added a count waitlist method

1dc9477

displaying active feature for waitlisted database in admin site

6a0b708

showing only active students when we click on particular section from…

bbaca01

… sections db on admin site

add max_waitlist_capacity to section partial_update

97cf4a9

Merge branch 'feat/waitlisting/integration' into feat/waitlisting/fro…

86ddc72

…ntend

add max_waitlist_capacity to section partial_update

61fb87c

Merge branch 'feat/waitlisting/integration' into feat/waitlisting/fro…

fc24af8

…ntend

add waitlistCapacity to MetaEditModal and MentorSectionInfo

9a20d84

add numWaitlistedStudnets to section serializer (#528)

14ce137

add numWaitlistedStudnets to section serializer (#528)

2362aae

allow students to join waitlist for sections

bb3707a

add view waitlist frontend and fix backend endpoint add

489118c

Merge branch 'feat/waitlisting/frontend' of github.com:csmberkeley/cs…

3ba529f

…m_web into feat/waitlisting/frontend

basic view, can add to waitlist, fixed card bugs

ff874a1

added waitlist section view

bbe7a3e

fix resources

b1f3139

add migrations

46cbe03

fix migration

6e2c981

add numWaitlistedStudnets to section serializer (#531)

3bd1e5c

Co-authored-by: maxmwang <orangemax888@gmail.com>

merge

a9ec880

fix waitlisting tests, allow cascade waitlist, add from coord, preven…

55691ff

…t race conditions

fix tests

2be41d9

fix backend for section swap

5fc7cd0

gabrielhan23 added 3 commits February 25, 2026 04:41

update frontend to show warning modals

058a158

add tests

914a282

merge with main

2beb837

gabrielhan23 requested review from Copilot and heidi-chann February 25, 2026 13:02

Copilot started reviewing on behalf of gabrielhan23 February 25, 2026 13:02 View session

Copilot AI reviewed Feb 25, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat/waitlisting/integration#533

Feat/waitlisting/integration#533
gabrielhan23 wants to merge 33 commits intomasterfrom
feat/waitlisting/integration

gabrielhan23 commented Feb 25, 2026

Uh oh!

cypress bot commented Feb 25, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 25, 2026

Uh oh!

Copilot AI Feb 25, 2026

Uh oh!

Copilot AI Feb 25, 2026

Uh oh!

Copilot AI Feb 25, 2026

Uh oh!

Copilot AI Feb 25, 2026

Uh oh!

Copilot AI Feb 25, 2026

Uh oh!

Copilot AI Feb 25, 2026

Uh oh!

Copilot AI Feb 25, 2026

Uh oh!

Copilot AI Feb 25, 2026

Uh oh!

Copilot AI Feb 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

	"<Drop> User %s dropped Section %sfor Student user %s",
	"<Drop> User %s dropped Section %s for Student user %s",

	# time limit fdocor wotd submission;
	# time limit for wotd submission;

		section = get_object_or_error(Section.objects, pk=pk)
		section = Section.objects.select_for_update().get(pk=section.pk)

		return Response(status=status.HTTP_204_NO_CONTENT)


		def log_enroll_result(success, user, section, reason=None):

	"plugins": ["transform-class-properties", "@babel/plugin-transform-runtime", "dynamic-import-node"]
	"plugins": ["transform-class-properties", "@babel/plugin-transform-runtime", "lodash", "dynamic-import-node"]

	addopts = --reuse-db -p no:cacheprovider
	addopts = -p no:cacheprovider

Conversation

gabrielhan23 commented Feb 25, 2026

Uh oh!

cypress bot commented Feb 25, 2026

csm_web Run #445

Tests for review

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants