MICROBA-1011 | Remove duplicate allowlist entries and add constraint to model to prevent future occurrences

[MICROBA-1011]
- Add data migration to remove the duplicate certificate allowlist entries before adding the new constraint to the CertificateWhitelist model.
- Add `unique_together` constraint to the CertificateWhitelist model. A learner should only have a single allowlist entry per course-run.

lms/djangoapps/certificates/migrations/0021_remove_certificate_allowlist_duplicate_records.py

@@ -0,0 +1,36 @@
+# Generated by Django 2.2.19 on 2021-03-11 19:31
+
+from django.db import migrations
+from django.db.models import Count, Min
+
+
+class Migration(migrations.Migration):
+    """
+    Delete all the duplicate records in the `CERTIFICATES_CERTIFICATEWHITELIST` table.
+    """
+
+    def remove_duplicate_entries(apps, schema_editor):
+        CertificateWhitelist = apps.get_model('certificates', 'CertificateWhitelist')
+
+        allowlist_duplicates = (
+            CertificateWhitelist.objects.values("user_id", "course_id")
+            .annotate(first_entry_id=Min("id"), num_entries=Count("id"))
+            .filter(num_entries__gt=1)
+        )
+
+        # delete the duplicates, excluding the original allowlist entry for the user/course-run combo
+        for duplicate in allowlist_duplicates:
+            (
+                CertificateWhitelist.objects
+                .filter(user_id=duplicate["user_id"], course_id=duplicate["course_id"])
+                .exclude(id=duplicate["first_entry_id"])
+                .delete()
+            )
+
+    dependencies = [
+        ('certificates', '0020_remove_existing_mgmt_cmd_args'),
+    ]
+
+    operations = [
+        migrations.RunPython(remove_duplicate_entries, reverse_code=migrations.RunPython.noop)
+    ]

lms/djangoapps/certificates/migrations/0022_add_unique_constraints_to_certificatewhitelist_model.py

@@ -0,0 +1,19 @@
+# Generated by Django 2.2.19 on 2021-03-12 17:27
+
+from django.conf import settings
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        migrations.swappable_dependency(settings.AUTH_USER_MODEL),
+        ('certificates', '0021_remove_certificate_allowlist_duplicate_records'),
+    ]
+
+    operations = [
+        migrations.AlterUniqueTogether(
+            name='certificatewhitelist',
+            unique_together={('course_id', 'user')},
+        ),
+    ]

lms/djangoapps/certificates/models.py

@@ -97,6 +97,7 @@ class CertificateWhitelist(models.Model):
     """
     class Meta:
         app_label = "certificates"
+        unique_together = [['course_id', 'user']]
 
     objects = NoneToEmptyManager()
 

lms/djangoapps/certificates/tests/test_signals.py

@@ -83,7 +83,7 @@ class WhitelistGeneratedCertificatesTest(ModuleStoreTestCase):
             mode="verified",
         )
 
-    def test_cert_generation_on_whitelist_append_self_paced(self):
+    def test_cert_generation_on_allowlist_append_self_paced_auto_cert_generation_disabled(self):
         """
         Verify that signal is sent, received, and fires task
         based on 'AUTO_CERTIFICATE_GENERATION' flag
@@ -98,6 +98,16 @@ class WhitelistGeneratedCertificatesTest(ModuleStoreTestCase):
                     course_id=self.course.id
                 )
                 mock_generate_certificate_apply_async.assert_not_called()
+
+    def test_cert_generation_on_allowlist_append_self_paced_auto_cert_generation_enabled(self):
+        """
+        Verify that signal is sent, received, and fires task
+        based on 'AUTO_CERTIFICATE_GENERATION' flag
+        """
+        with mock.patch(
+            'lms.djangoapps.certificates.signals.generate_certificate.apply_async',
+            return_value=None
+        ) as mock_generate_certificate_apply_async:
             with override_waffle_switch(AUTO_CERTIFICATE_GENERATION_SWITCH, active=True):
                 CertificateWhitelistFactory(
                     user=self.user,
@@ -111,7 +121,7 @@ class WhitelistGeneratedCertificatesTest(ModuleStoreTestCase):
                     }
                 )
 
-    def test_cert_generation_on_whitelist_append_instructor_paced(self):
+    def test_cert_generation_on_allowlist_append_instructor_paced_cert_generation_disabled(self):
         """
         Verify that signal is sent, received, and fires task
         based on 'AUTO_CERTIFICATE_GENERATION' flag
@@ -126,6 +136,16 @@ class WhitelistGeneratedCertificatesTest(ModuleStoreTestCase):
                     course_id=self.ip_course.id
                 )
                 mock_generate_certificate_apply_async.assert_not_called()
+
+    def test_cert_generation_on_allowlist_append_instructor_paced_cert_generation_enabled(self):
+        """
+        Verify that signal is sent, received, and fires task
+        based on 'AUTO_CERTIFICATE_GENERATION' flag
+        """
+        with mock.patch(
+                'lms.djangoapps.certificates.signals.generate_certificate.apply_async',
+                return_value=None
+        ) as mock_generate_certificate_apply_async:
             with override_waffle_switch(AUTO_CERTIFICATE_GENERATION_SWITCH, active=True):
                 CertificateWhitelistFactory(
                     user=self.user,