feat: manage command to fix missing course runs (openedx#2148)

* feat: manage command to fix missing course runs
* populates foreign key connection to catalog.CourseRun if the course run key is present there.
* Adding some more type hints/annotations

FIXES: APER-2790

credentials/apps/catalog/api.py

@@ -1,3 +1,5 @@
+from typing import List
+
 from .data import OrganizationDetails, ProgramDetails
 from .models import CourseRun as _CourseRun, Program as _Program
 
@@ -73,7 +75,7 @@ def _convert_program_to_program_details(
     )
 
 
-def get_course_runs_by_course_run_keys(course_run_keys):
+def get_course_runs_by_course_run_keys(course_run_keys: List[str]) -> List[_CourseRun]:
     """
     Get course runs using given course run keys
 

credentials/apps/credentials/management/commands/populate_missing_courserun_info.py

@@ -0,0 +1,85 @@
+"""
+Django managment command to sync missing course_run_ids from the catalog
+course_run table.
+"""
+
+import logging
+from typing import TYPE_CHECKING
+
+from django.core.exceptions import MultipleObjectsReturned, ObjectDoesNotExist
+from django.core.management.base import BaseCommand
+
+from credentials.apps.catalog.api import get_course_runs_by_course_run_keys
+from credentials.apps.credentials.models import CourseCertificate
+
+
+if TYPE_CHECKING:
+    from credentials.apps.catalog.models import CourseRun
+
+
+logger = logging.getLogger(__name__)
+
+
+class Command(BaseCommand):
+    def add_arguments(self, parser):
+        parser.add_argument("--verbose", action="store_true", help="Log each update")
+        parser.add_argument("--dry_run", action="store_true", help="Don't actually change the data")
+
+    def handle(self, *args, **options):
+        """
+        Update the course_id from course certificates that are missing it
+        as long as the course run is already in the catalog.course_run table.
+        """
+        course_certificates_without_course_run_id = CourseCertificate.objects.filter(course_run_id=None)
+        count = course_certificates_without_course_run_id.count()
+        verbosity = options.get("verbose")  # type: bool
+        dry_run = options.get("dry_run")  # type: bool
+        count_ignored = 0  # type: int
+
+        logger.info(f"Start processing {count} CourseCertificates with no course_id")
+
+        # Because CourseCertificate.course_id isn't a ForeignKey, there's no
+        # completely graceful way to join the table with catalog.CourseRun.
+        # However, the list of these CourseCertificate object should be small,
+        # so the mild time waste of looping through them for occasional
+        # on-demand runs of this command is probably better than writing an
+        # objects.raw SQL query.
+        for course_cert in course_certificates_without_course_run_id:
+            course_run_key = course_cert.course_id
+            if not course_run_key:
+                logger.warning(f"Could not get course_id for CourseCertificate {course_cert.id}")
+            try:
+                course_run = get_course_runs_by_course_run_keys([course_run_key])[0]
+            except ObjectDoesNotExist:
+                # This is fine, and can happen with some frequency
+                count_ignored += 1
+                if verbosity:
+                    logger.info(f"No Catalog.CourseRun entry for {course_run_key}")
+            except MultipleObjectsReturned:
+                # This is not fine, and while it's not a bug this process needs
+                # to worry about, we definitely want to know.
+                logger.warning(f"Course run key in Catalog.CourseRun twice: {course_run_key}")
+            else:
+                self.update_course_certificate_with_course_run_id(course_cert, course_run, verbosity, dry_run)
+
+        logger.info(f"populate_missing_courserun_info finished! {count_ignored} CourseCertificate(s) safely ignored.")
+
+    def update_course_certificate_with_course_run_id(
+        self,
+        course_certificate: CourseCertificate,
+        course_run: "CourseRun",
+        verbosity: bool,
+        dry_run: bool,
+    ) -> None:
+        """Update course_run_id from catalog.CourseRun"""
+        if course_run.key:
+            course_certificate.course_run = course_run
+            dry_run_msg = "(dry run) " if dry_run else ""
+            if verbosity:
+                logger.info(
+                    f"{dry_run_msg}updating CourseCertificate {course_certificate.id} (course_id "
+                    f"{course_certificate.course_id}) with course_run_id {course_run.key}"
+                )
+            # use update_fields to just update this one piece of data
+            if not dry_run:
+                course_certificate.save(update_fields=["course_run"])

credentials/apps/credentials/management/commands/tests/test_populate_missing_courserun_info.py

@@ -0,0 +1,49 @@
+"""
+Tests for the populate_missing_courserun_info management command
+"""
+
+from django.core.exceptions import ObjectDoesNotExist
+from django.core.management import call_command
+from django.test import TestCase
+
+from credentials.apps.catalog.tests.factories import CourseRunFactory
+from credentials.apps.credentials.models import CourseCertificate
+from credentials.apps.credentials.tests.factories import CourseCertificateFactory
+
+
+class CourseCertificateCourseRunSyncTests(TestCase):
+    def setUp(self):
+        """
+        Create the error condition:
+
+        * A CourseRun
+        * A CourseCertificate that has the key of the CourseRun in course_id,
+          but doesn't have a FK to the CouseRun
+        """
+        super().setUp()
+        self.course_run = CourseRunFactory()
+        self.course_certificate = CourseCertificateFactory(course_id=self.course_run.key)
+
+    def test_update_ids(self):
+        """verify ids were updated"""
+        with self.assertRaises(ObjectDoesNotExist):
+            CourseCertificate.objects.get(course_run=self.course_run)
+
+        call_command("populate_missing_courserun_info", verbose=True)
+
+        course_cert = CourseCertificate.objects.get(course_run=self.course_run)
+        self.assertEqual(course_cert.course_id, course_cert.course_run.key)
+
+    def test_dry_run(self):
+        """verify course_ids were NOT updated"""
+        with self.assertRaises(ObjectDoesNotExist):
+            CourseCertificate.objects.get(course_run=self.course_run)
+
+        call_command(
+            "populate_missing_courserun_info",
+            verbose=True,
+            dry_run=True,
+        )
+
+        with self.assertRaises(ObjectDoesNotExist):
+            CourseCertificate.objects.get(course_run=self.course_run)

credentials/apps/credentials/models.py

@@ -198,6 +198,9 @@ class CourseCertificate(AbstractCertificate):
     .. no_pii: This model has no PII.
     """
 
+    # course_id, despite the name, is a course run key. It is a deprecated
+    # property and will be removed. If you need the value stored in course_id,
+    # get it from course_run.key.
     course_id = models.CharField(max_length=255, validators=[validate_course_key])
     course_run = models.OneToOneField(CourseRun, null=True, on_delete=models.PROTECT)
     certificate_available_date = models.DateTimeField(