Simon-Initiative / oli-torus

Next Generation OLI Authoring and Delivery Platform
https://proton.oli.cmu.edu
MIT License
83 stars 35 forks source link

[BUG FIX] [MER-3390] Only include enrolled students in section enrollment count #4974

Closed eliknebel closed 1 month ago

eliknebel commented 1 month ago

https://eliterate.atlassian.net/browse/MER-3390

https://github.com/user-attachments/assets/0522b298-aa39-43f4-989b-4c76d8d8673f

This PR fixes 2 separate issue regarding enrollment counts outlined in the ticket.

  1. First, the browse sections query was modified to only count enrollments with status of enrolled (excluding suspended) which fixes an issue where the count was including "unenrolled" students
  2. Second, when adding a duplicate enrollment, most of the operations were taking into account the possibility that a duplicate, or already enrolled, student may have been entered again, and upserting user and enrollment records in that case, However, there was a gap here were duplicate enrollments_context_roles records were being created, which was affecting the count being returned by the enrollments count query. The fix here was to remove these duplicate records and create a constraint on the table that ensures no duplicates and also adding the proper conflict handler into the application code so that when a conflict is detected, no record is created.

NOTE This PR contains a required migration that removes duplicate enrollments_context_roles. This represents a somewhat risky change and should be thoroughly tested after the migration is deployed to ensure that only the intended duplicates are being deleted. This means that all enrollments should still exist as expected and the enrollment counts should reflect the correct amount after the migration is executed/deployed.