`order` should be removed from unique constraint in `ProgramEnrollment` model

[aliraza-abbasi]

Steps to Reproduce

• Open the terminal from running the xpro app and run these commands
• python manage.py shell
• from django.contrib.auth import get_user_model
• from courses.models import Program, ProgramEnrollment
• from courses.utils import generate_program_certificate
• User = get_user_model()
• user = User.objects.get(id=<EXISTING_USER_ID>)
• program = Program.objects.get(id=<EXISTING_PROGRAM_ID>)
• ProgramEnrollment.objects.create(user=user, program=program) Run this command two times to create duplicate enrollments
• ProgramEnrollment.objects.create(user=user, program=program) This will create duplicate enrollments for a single user

Expected Behavior

• Enrollment should be unique for a single user in a specific program
• No matter if enrollment is with or without order

Actual Behavior

• It creates duplicate enrollments as much as if order is None or order is different in the same program

Related Issues

2568, https://github.com/mitodl/hq/issues/595, https://github.com/mitodl/hq/issues/833

Special Note

• This migration will require to delete duplicate enrollments from production as well

[pdpinch]

I believe this constraint was put in place to ensure that all enrollments were paid for. Can you explain again the reasoning behind removing the constraint?

[aliraza-abbasi]

I believe this constraint was put in place to ensure that all enrollments were paid for. Can you explain again the reasoning behind removing the constraint?

@pdpinch We recently faced certificate generation errors (mentioned above in Related Issues) due to duplicate program enrollments. I opened this ticket because:

1. None is not checked as unique so we can create as many program enrollments as we want to with order=None. Which I found on the production (ref) and tested locally as well
2. I believe a user can only be enrolled once in a program. if it's order is None it will be updated to order_id when he makes a payment. But I've found two enrollments of a user in the same program with two different orders which is also caused by order in a unique constraint

Currently, I've fixed the certificate's job error (with this PR) even if users have multiple enrollments in the same program. But I think this is not the proper solution. If we remove order from the unique constraint in the ProgramEnrollment model then the program enrollment will be unique on the basis of user and program

Please let me know your thoughts on this.

[pdpinch]

Looking over the duplicate enrollments, it seems that something like this happened:

1. A company made a bulk purchase of program enrollments for its employees.
2. One employee used a program enrollment code, and enrolled in 4 courses.
3. But the employee only completed one course and did not get the other certificates.
4. Later, the company made another bulk purchase of program enrollments.
5. The same employee used another program enrollment code, to enroll in the courses they did not complete.
6. The employee completed the remaining courses, qualifying them for the program certificate.

The employee in question had two program enrollments, with four course certificates.

If we remove the order constraint, what would happen in a case like this?

[rhysyngsun]

One option would be to upgrade ourselves to postgres 15.x, which supports a new unique constraint NULL NOT DISTINCT which means only one NULL would be permitted in the index. I see that docker-compose.yml uses postgres 11.6, production is on 10.21, and RC is on 14.6, so in general that really needs to be cleaned up, I'm not sure why the versions vary so much.

Another option would be to change the index to (user, program) and when a new order is placed we'd do a:

ProgramEnrollment.objects.update_or_create(user=user, program=program, defaults={"order": order"})

That would mean that any given ProgramEnrollment points to the most recent applicable Order, which probably makes sense.

I guess my only question is are there pieces of logic and/or reports that expect a 1-1 mapping of ProgramEnrollment to Order?

[aliraza-abbasi]

If we remove the order constraint, what would happen in a case like this?

@pdpinch You are right we need order in unique_constraints to fulfill the case you discussed. But I was thinking. What if we add program_run in the ProgramEnrollment model instead of program? If not then I think we can close this issue.

[pdpinch]

@aliraza-abbasi I think that's a reasonable suggestion, but we'd have to look to see where this is code that expects to find a program associated with a ProgramEnrollment (e.g. the dashboard). I don't know if there is a reasonable way to make the change you propose and see what tests break. We would also have to take care to test with production data.

[aliraza-abbasi]

After doing some R&D and analyzing the code, I discovered that replacing the program with program_run would require some major code modifications and significant data changes to the production data. The issue https://github.com/mitodl/mitxpro/issues/2568 we faced previously was fixed and currently, this issue does not have any effect on the data or workflow, so I placed it on hold. It can be addressed in the future if needed or please let me know if you would like me to close this issue. @pdpinch

[pdpinch]

Thanks, @aliraza-abbasi. I'm going to close this, but if we run into more issues, we can reopen it.