Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
288 changes: 288 additions & 0 deletions api/CourseRegistration.API/Controllers/WaitlistController.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,288 @@
using Microsoft.AspNetCore.Mvc;
using CourseRegistration.Application.DTOs;
using CourseRegistration.Application.Interfaces;

namespace CourseRegistration.API.Controllers;

/// <summary>
/// Controller for waitlist management operations
/// </summary>
[ApiController]
[Route("api/[controller]")]
[Produces("application/json")]
public class WaitlistController : ControllerBase
{
private readonly IWaitlistService _waitlistService;
private readonly ILogger<WaitlistController> _logger;

/// <summary>
/// Initializes a new instance of the WaitlistController
/// </summary>
public WaitlistController(IWaitlistService waitlistService, ILogger<WaitlistController> logger)
{
_waitlistService = waitlistService ?? throw new ArgumentNullException(nameof(waitlistService));
_logger = logger ?? throw new ArgumentNullException(nameof(logger));
}

/// <summary>
/// Adds a student to a course waitlist
/// </summary>
/// <param name="createWaitlistEntryDto">Waitlist entry creation data</param>
/// <returns>Created waitlist entry</returns>
[HttpPost]
[ProducesResponseType(typeof(ApiResponseDto<WaitlistEntryDto>), StatusCodes.Status201Created)]
[ProducesResponseType(typeof(ApiResponseDto<object>), StatusCodes.Status400BadRequest)]
public async Task<ActionResult<ApiResponseDto<WaitlistEntryDto>>> JoinWaitlist([FromBody] CreateWaitlistEntryDto createWaitlistEntryDto)
{
_logger.LogInformation("Student {StudentId} attempting to join waitlist for course {CourseId}",
createWaitlistEntryDto.StudentId, createWaitlistEntryDto.CourseId);

try
{
var waitlistEntry = await _waitlistService.JoinWaitlistAsync(createWaitlistEntryDto);
return CreatedAtAction(
nameof(GetWaitlistEntry),
new { id = waitlistEntry.WaitlistEntryId },
new ApiResponseDto<WaitlistEntryDto>
{
Success = true,
Message = $"Successfully joined waitlist at position {waitlistEntry.Position}",
Data = waitlistEntry
});
}
catch (InvalidOperationException ex)
{
_logger.LogWarning("Failed to join waitlist: {Message}", ex.Message);
return BadRequest(new ApiResponseDto<object>
{
Success = false,
Message = ex.Message
});
}
}

/// <summary>
/// Gets a specific waitlist entry by ID
/// </summary>
/// <param name="id">Waitlist entry ID</param>
/// <returns>Waitlist entry details</returns>
[HttpGet("{id}")]
[ProducesResponseType(typeof(ApiResponseDto<WaitlistEntryDto>), StatusCodes.Status200OK)]
[ProducesResponseType(typeof(ApiResponseDto<object>), StatusCodes.Status404NotFound)]
public async Task<ActionResult<ApiResponseDto<WaitlistEntryDto>>> GetWaitlistEntry(Guid id)
{
_logger.LogInformation("Getting waitlist entry {Id}", id);

var waitlistEntry = await _waitlistService.GetWaitlistEntryAsync(id);

if (waitlistEntry == null)
{
return NotFound(new ApiResponseDto<object>
{
Success = false,
Message = "Waitlist entry not found"
});
}

return Ok(new ApiResponseDto<WaitlistEntryDto>
{
Success = true,
Data = waitlistEntry
});
}

/// <summary>
/// Gets active waitlist entries for a specific course
/// </summary>
/// <param name="courseId">Course ID</param>
/// <returns>List of waitlist entries for the course</returns>
[HttpGet("course/{courseId}")]
[ProducesResponseType(typeof(ApiResponseDto<IEnumerable<WaitlistEntryDto>>), StatusCodes.Status200OK)]
public async Task<ActionResult<ApiResponseDto<IEnumerable<WaitlistEntryDto>>>> GetCourseWaitlist(Guid courseId)
{
_logger.LogInformation("Getting waitlist for course {CourseId}", courseId);

var waitlistEntries = await _waitlistService.GetCourseWaitlistAsync(courseId);

return Ok(new ApiResponseDto<IEnumerable<WaitlistEntryDto>>
{
Success = true,
Data = waitlistEntries,
Message = $"Found {waitlistEntries.Count()} entries in waitlist"
});
}
Comment on lines +99 to +113
Copy link

Copilot AI Nov 27, 2025

Choose a reason for hiding this comment

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

Potential data exposure: The GetCourseWaitlist() endpoint returns all waitlist entries including student email addresses and personal information to any caller. This violates privacy principles—students on a waitlist shouldn't necessarily see other students' email addresses.

Consider:

  1. Adding authorization to restrict this endpoint to admins only
  2. Creating a separate public endpoint that returns only aggregated data (e.g., total count, student's own position)
  3. Removing sensitive fields like email from the response for non-admin users
  4. Having students query their own position via GetStudentWaitlists() instead

Copilot uses AI. Check for mistakes.

/// <summary>
/// Gets active waitlist entries for a specific student
/// </summary>
/// <param name="studentId">Student ID</param>
/// <returns>List of waitlist entries for the student</returns>
[HttpGet("student/{studentId}")]
[ProducesResponseType(typeof(ApiResponseDto<IEnumerable<WaitlistEntryDto>>), StatusCodes.Status200OK)]
public async Task<ActionResult<ApiResponseDto<IEnumerable<WaitlistEntryDto>>>> GetStudentWaitlists(Guid studentId)
{
_logger.LogInformation("Getting waitlists for student {StudentId}", studentId);

var waitlistEntries = await _waitlistService.GetStudentWaitlistsAsync(studentId);

return Ok(new ApiResponseDto<IEnumerable<WaitlistEntryDto>>
{
Success = true,
Data = waitlistEntries,
Message = $"Student is on {waitlistEntries.Count()} waitlist(s)"
});
}

/// <summary>
/// Removes a student from a waitlist
/// </summary>
/// <param name="id">Waitlist entry ID</param>
/// <returns>Success status</returns>
[HttpDelete("{id}")]
[ProducesResponseType(typeof(ApiResponseDto<object>), StatusCodes.Status200OK)]
[ProducesResponseType(typeof(ApiResponseDto<object>), StatusCodes.Status404NotFound)]
public async Task<ActionResult<ApiResponseDto<object>>> LeaveWaitlist(Guid id)
{
_logger.LogInformation("Attempting to remove waitlist entry {Id}", id);

var result = await _waitlistService.LeaveWaitlistAsync(id);

if (!result)
{
return NotFound(new ApiResponseDto<object>
{
Success = false,
Message = "Waitlist entry not found or already inactive"
});
}

return Ok(new ApiResponseDto<object>
{
Success = true,
Message = "Successfully left the waitlist"
});
}

/// <summary>
/// Updates a waitlist entry (admin function)
/// </summary>
/// <param name="id">Waitlist entry ID</param>
/// <param name="updateDto">Update data</param>
/// <returns>Updated waitlist entry</returns>
[HttpPatch("{id}")]
[ProducesResponseType(typeof(ApiResponseDto<WaitlistEntryDto>), StatusCodes.Status200OK)]
[ProducesResponseType(typeof(ApiResponseDto<object>), StatusCodes.Status404NotFound)]
[ProducesResponseType(typeof(ApiResponseDto<object>), StatusCodes.Status400BadRequest)]
public async Task<ActionResult<ApiResponseDto<WaitlistEntryDto>>> UpdateWaitlistEntry(
Guid id,
[FromBody] UpdateWaitlistEntryDto updateDto)
{
_logger.LogInformation("Updating waitlist entry {Id}", id);

try
{
var updatedEntry = await _waitlistService.UpdateWaitlistEntryAsync(id, updateDto);

if (updatedEntry == null)
{
return NotFound(new ApiResponseDto<object>
{
Success = false,
Message = "Waitlist entry not found"
});
}

return Ok(new ApiResponseDto<WaitlistEntryDto>
{
Success = true,
Message = "Waitlist entry updated successfully",
Data = updatedEntry
});
}
catch (InvalidOperationException ex)
{
_logger.LogWarning("Failed to update waitlist entry: {Message}", ex.Message);
return BadRequest(new ApiResponseDto<object>
{
Success = false,
Message = ex.Message
});
}
}

/// <summary>
/// Clears the entire waitlist for a course (admin function)
/// </summary>
/// <param name="courseId">Course ID</param>
/// <returns>Success status</returns>
[HttpDelete("course/{courseId}/clear")]
[ProducesResponseType(typeof(ApiResponseDto<object>), StatusCodes.Status200OK)]
public async Task<ActionResult<ApiResponseDto<object>>> ClearWaitlist(Guid courseId)
{
_logger.LogInformation("Clearing waitlist for course {CourseId}", courseId);

await _waitlistService.ClearWaitlistAsync(courseId);

return Ok(new ApiResponseDto<object>
{
Success = true,
Message = "Waitlist cleared successfully"
});
}

/// <summary>
/// Notifies the next student on the waitlist (admin/system function)
/// </summary>
/// <param name="courseId">Course ID</param>
/// <returns>Success status</returns>
[HttpPost("course/{courseId}/notify-next")]
[ProducesResponseType(typeof(ApiResponseDto<object>), StatusCodes.Status200OK)]
public async Task<ActionResult<ApiResponseDto<object>>> NotifyNextStudent(Guid courseId)
{
_logger.LogInformation("Notifying next student on waitlist for course {CourseId}", courseId);

await _waitlistService.NotifyNextStudentAsync(courseId);

return Ok(new ApiResponseDto<object>
{
Success = true,
Message = "Next student notified successfully"
});
}

/// <summary>
/// Reorders waitlist entries for a course (admin function)
/// </summary>
/// <param name="courseId">Course ID</param>
/// <param name="newPositions">Dictionary of waitlist entry IDs and their new positions</param>
/// <returns>Success status</returns>
[HttpPut("course/{courseId}/reorder")]
[ProducesResponseType(typeof(ApiResponseDto<object>), StatusCodes.Status200OK)]
[ProducesResponseType(typeof(ApiResponseDto<object>), StatusCodes.Status400BadRequest)]
public async Task<ActionResult<ApiResponseDto<object>>> ReorderWaitlist(
Guid courseId,
[FromBody] Dictionary<Guid, int> newPositions)
Comment on lines +259 to +264
Copy link

Copilot AI Nov 27, 2025

Choose a reason for hiding this comment

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

Missing input validation: The controller accepts the newPositions dictionary without validating its content before passing to the service. Malicious input could include:

  • Empty dictionary
  • Invalid GUIDs
  • Negative or zero positions
  • Extremely large position numbers

While the service validates count and sequential positions (lines 256-278 in WaitlistService.cs), add controller-level validation:

if (newPositions == null || !newPositions.Any())
{
    return BadRequest(new ApiResponseDto<object>
    {
        Success = false,
        Message = "Position data is required"
    });
}

Copilot uses AI. Check for mistakes.
{
_logger.LogInformation("Reordering waitlist for course {CourseId}", courseId);

try
{
await _waitlistService.ReorderWaitlistAsync(courseId, newPositions);

return Ok(new ApiResponseDto<object>
{
Success = true,
Message = "Waitlist reordered successfully"
});
}
catch (InvalidOperationException ex)
{
_logger.LogWarning("Failed to reorder waitlist: {Message}", ex.Message);
return BadRequest(new ApiResponseDto<object>
{
Success = false,
Message = ex.Message
});
}
}
Comment on lines +1 to +287
Copy link

Copilot AI Nov 27, 2025

Choose a reason for hiding this comment

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

Missing authorization: All waitlist endpoints lack authorization checks. Critical operations that should be restricted:

Admin-only operations (no authorization implemented):

  • ClearWaitlist() - line 220: Any user can clear entire waitlists
  • ReorderWaitlist() - line 262: Any user can manipulate positions
  • NotifyNextStudent() - line 240: Could be abused to spam notifications
  • UpdateWaitlistEntry() - line 176: Should only allow admins to change positions, or students to update their own preferences

Student-owned operations (no ownership verification):

  • LeaveWaitlist() - line 144: A student could remove another student's entry
  • UpdateWaitlistEntry() - line 176: Should verify the student owns the entry

Add role-based authorization attributes like [Authorize(Roles = "Admin")] and implement ownership checks in the service layer.

Copilot uses AI. Check for mistakes.
}
3 changes: 3 additions & 0 deletions api/CourseRegistration.API/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,14 @@
builder.Services.AddScoped<IStudentRepository, StudentRepository>();
builder.Services.AddScoped<ICourseRepository, CourseRepository>();
builder.Services.AddScoped<IRegistrationRepository, RegistrationRepository>();
builder.Services.AddScoped<IWaitlistRepository, WaitlistRepository>();
builder.Services.AddScoped<IUnitOfWork, UnitOfWork>();

// Register services
builder.Services.AddScoped<IStudentService, StudentService>();
builder.Services.AddScoped<ICourseService, CourseService>();
builder.Services.AddScoped<INotificationService, NotificationService>();
builder.Services.AddScoped<IWaitlistService, WaitlistService>();
builder.Services.AddScoped<IRegistrationService, RegistrationService>();
Comment on lines 66 to 71
Copy link

Copilot AI Nov 27, 2025

Choose a reason for hiding this comment

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

[nitpick] Service registration order creates potential initialization issue: WaitlistService is registered (line 70) after NotificationService (line 69), but RegistrationService (line 71) depends on IWaitlistService. While dependency injection resolves this at runtime, WaitlistService depends on INotificationService.

The correct order should register dependencies before dependents:

// Register services (dependencies first)
builder.Services.AddScoped<IStudentService, StudentService>();
builder.Services.AddScoped<ICourseService, CourseService>();
builder.Services.AddScoped<INotificationService, NotificationService>();
builder.Services.AddScoped<IWaitlistService, WaitlistService>();  // depends on INotificationService
builder.Services.AddScoped<IRegistrationService, RegistrationService>();  // depends on IWaitlistService

While this works in the current implementation (DI resolves at request time), following a logical dependency order improves code readability and prevents issues if eager initialization is added later.

Suggested change
// Register services
builder.Services.AddScoped<IStudentService, StudentService>();
builder.Services.AddScoped<ICourseService, CourseService>();
builder.Services.AddScoped<INotificationService, NotificationService>();
builder.Services.AddScoped<IWaitlistService, WaitlistService>();
builder.Services.AddScoped<IRegistrationService, RegistrationService>();
// Register services (dependencies first)
builder.Services.AddScoped<IStudentService, StudentService>();
builder.Services.AddScoped<ICourseService, CourseService>();
builder.Services.AddScoped<INotificationService, NotificationService>();
builder.Services.AddScoped<IWaitlistService, WaitlistService>(); // depends on INotificationService
builder.Services.AddScoped<IRegistrationService, RegistrationService>(); // depends on IWaitlistService

Copilot uses AI. Check for mistakes.

// Add CORS policy for development
Expand Down
25 changes: 25 additions & 0 deletions api/CourseRegistration.Application/DTOs/CourseDtos.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,11 @@ public class CreateCourseDto
/// Course schedule
/// </summary>
public string Schedule { get; set; } = string.Empty;

/// <summary>
/// Maximum number of students allowed
/// </summary>
public int MaxEnrollment { get; set; } = 30;
Comment on lines +38 to +41
Copy link

Copilot AI Nov 27, 2025

Choose a reason for hiding this comment

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

Missing validation: The MaxEnrollment field accepts any integer value without validation. This allows invalid values such as:

  • Negative numbers (e.g., -5)
  • Zero
  • Unrealistically large numbers

Add validation in the DTO:

[Range(1, 1000, ErrorMessage = "Max enrollment must be between 1 and 1000")]
public int MaxEnrollment { get; set; } = 30;

Also consider adding validation in the domain entity or service layer to enforce business rules.

Copilot uses AI. Check for mistakes.
}

/// <summary>
Expand Down Expand Up @@ -70,6 +75,11 @@ public class UpdateCourseDto
/// Course schedule
/// </summary>
public string Schedule { get; set; } = string.Empty;

/// <summary>
/// Maximum number of students allowed
/// </summary>
public int MaxEnrollment { get; set; } = 30;
Comment on lines +79 to +82
Copy link

Copilot AI Nov 27, 2025

Choose a reason for hiding this comment

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

Missing validation: The MaxEnrollment field in UpdateCourseDto also lacks validation. Additionally, when updating MaxEnrollment on an existing course, there's no check to prevent setting it below the current enrollment count, which would create an inconsistent state where a course is "over capacity."

Add:

  1. Range validation attribute as with CreateCourseDto
  2. Business logic in CourseService.UpdateCourseAsync() to validate:
    if (updateDto.MaxEnrollment < course.CurrentEnrollment)
        throw new InvalidOperationException(
            $"Cannot set max enrollment below current enrollment ({course.CurrentEnrollment})");

Copilot uses AI. Check for mistakes.
}

/// <summary>
Expand Down Expand Up @@ -117,6 +127,21 @@ public class CourseDto
/// </summary>
public int CurrentEnrollment { get; set; }

/// <summary>
/// Maximum enrollment capacity
/// </summary>
public int MaxEnrollment { get; set; }

/// <summary>
/// Indicates if the course is full
/// </summary>
public bool IsFull { get; set; }

/// <summary>
/// Number of students on the waitlist
/// </summary>
public int WaitlistCount { get; set; }

/// <summary>
/// Indicates if the course is active
/// </summary>
Expand Down
Loading