mitodl / micromasters

Portal for learners and course teams to access MITx Micromasters® programs
https://mm.mit.edu
BSD 3-Clause "New" or "Revised" License
28 stars 16 forks source link

erroneous program letters created on 12/19/2019 #5252

Closed pdpinch closed 1 year ago

pdpinch commented 1 year ago

Steps to Reproduce

approximately 1024 program letters were created on 12/19/2019, for a program that had not yet issued and certificates, and did not have a program letter template until February 2022.

Expected Behavior

Actual Behavior

Related issues:

arslanashraf7 commented 1 year ago

Update:

I looked around our code state from back on 12/19/2019 and I think I figured out what might have caused this issue.

From the possible places where the Program letter could have been created here are the details:

(Not probable causes)

  1. Creation from certificate signal did not happen based on the certificate's data in this case because these users don't have any certificates created so this signal was not hit.
  2. Creation from Final Grades also didn't happen in this case, while checking Final grades of the erroneous letters I could see a grade with 0 but even those were created in late 2020 so this also skips the probable cause list.

(Probable cause)

So in my analysis, this was probably caused by a 3rd case which is generate_program_letter management command.

Why do I think this caused it?

  1. All the letters were generated almost with the same timestamp with a difference of a few seconds.
  2. The grade records were not created for these users until 2020, even these created ones have 0 grades.
  3. No Certificates are present for these letters because this is non-fa program.

For the generation of program letters, we had this specific logic of checking the required vs passed courses. The problem with this was that we didn't exclusively check for elective courses to be passed to match the total number and also in a case where the program has no associated course this logic would always create letters.

Conclusion

So, I think we probably ran generate letters command on 12/19/2019, and back then this program didn't have any associated courses because the earliest course runs I could see for this program were started in 2020. So the required courses were 0 and passed courses were also 0 but the logic of that time created the letters anyway since what it does is courses_passed < program_course_count.

arslanashraf7 commented 1 year ago

it appears there were 3 additional erroneous letters created after this date

Regarding this :

Two of these were created in 2021 because of a bug that we fixed and released on Sep 8, 2021. There are more details on https://github.com/mitodl/micromasters/issues/5021 for reference. Looks like we missed doing the cleanup for these two letters after this fix.

Regarding, The last one was created in 2022. This could be a manual creation or some change in the requirements of the associated Electives Set at that time. I could not confirm if the elective set was changed around that time because the Elective Set model is not TimeStamped.

pdpinch commented 1 year ago

Thank you for this analysis. It makes sense to me and I agree that it was probably due to the management command being run at a time when we had many people enrolled in the FIN program, but no courses yet.

Can you confirm that the three non-FIN letters have been made inactive as well?

arslanashraf7 commented 1 year ago

Can you confirm that the three non-FIN letters have been made inactive as well?

Well, I just checked and the two letters created in 2021 are marked inactive.

But, Something different with the letter that was created in 2022. After the migration, it has been marked active instead of inactive. This happened because program_completed now returns Ture for this user and letter. This is strangely different from the last time we ran completed_program against all letters because it was returning False for this letter.

I went on checking what could have been changed:

FYI, @pdpinch

arslanashraf7 commented 1 year ago

I think we should close this. What are your thoughts @pdpinch ?