Skip to content

Prevent dropping completed enrollments #529

Description

@DeFiVC

Bug Description

The dropEnrollment method in src/modules/courses/course.service.ts (lines 685-721) does not check whether the enrollment is completed before deleting it. A user could drop a course they've already completed, losing their enrollment record.

Location

src/modules/courses/course.service.ts lines 685-721

async dropEnrollment(userId: string, courseId: string): Promise<void> {
    await withLock(`enroll:${userId}:${courseId}`, async () => {
        const [deleted] = await db
            .delete(enrollments)
            .where(
                and(eq(enrollments.userId, userId), eq(enrollments.courseId, courseId)),
            )
            .returning();

        if (!deleted) {
            throw new NotFoundError("Enrollment");
        }
        // ... cache invalidation
    });
    // ...
}

The Problem

The method deletes the enrollment without checking if completedAt is set. This means:

  1. A user who completed a course can drop it
  2. Their completion record is lost
  3. If they have a credential for the course, the enrollment backing it disappears
  4. The completedAt field becomes meaningless if enrollments can be deleted after completion

Recommended Fix

async dropEnrollment(userId: string, courseId: string): Promise<void> {
    await withLock(`enroll:${userId}:${courseId}`, async () => {
        const [deleted] = await db
            .delete(enrollments)
            .where(
                and(
                    eq(enrollments.userId, userId),
                    eq(enrollments.courseId, courseId),
                    isNull(enrollments.completedAt),  // Only allow dropping active enrollments
                ),
            )
            .returning();

        if (!deleted) {
            throw new NotFoundError("Enrollment");
        }
        // ...
    });
}

Acceptance Criteria

  • Only allow dropping enrollments that are NOT completed
  • Return a clear error if the user tries to drop a completed course
  • Verify that completed courses cannot be dropped via the API

Severity

medium - Data integrity issue that could lose completion records.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions