From 563b48c50cace71d694fa85a6a3c5b8b715e0731 Mon Sep 17 00:00:00 2001 From: Justin Hynes Date: Mon, 26 Apr 2021 07:41:04 -0400 Subject: [PATCH] [MICROBA-1139] * Change order of object Manager declarations in the GeneratedCertificate model. Declaration order matters and It appears that having the "base" `objects` declaration after the other Custom Managers creates an issue where I cannot access `objects` in a migration without this change. * As part of the manager migration there is a cleanup function that resets the `error_reason` field to an empty string for all certificates in the `downloadable` state that currently have an error listed in the record. --- .../0025_cleanup_certificate_errors.py | 30 +++++++++++++++++++ lms/djangoapps/certificates/models.py | 12 ++++---- 2 files changed, 36 insertions(+), 6 deletions(-) create mode 100644 lms/djangoapps/certificates/migrations/0025_cleanup_certificate_errors.py diff --git a/lms/djangoapps/certificates/migrations/0025_cleanup_certificate_errors.py b/lms/djangoapps/certificates/migrations/0025_cleanup_certificate_errors.py new file mode 100644 index 0000000000..791601a4c3 --- /dev/null +++ b/lms/djangoapps/certificates/migrations/0025_cleanup_certificate_errors.py @@ -0,0 +1,30 @@ +# Generated by Django 2.2.20 on 2021-04-27 18:12 + +from django.db import migrations + + +class Migration(migrations.Migration): + """ + Clean records in the `CERTIFICATES_GENERATEDCERTIFICATE` table that are in the downloadable state but also have + old errors still part of their certificate record. + + As part of this migration we are also altering the Managers for the GeneratedCertificate model. We need the ability + to access the `objects` attribute for the cleanup. + """ + def cleanup_certificate_records(apps, schema_editor): + GeneratedCertificate = apps.get_model('certificates', 'GeneratedCertificate') + + GeneratedCertificate.objects.filter(status='downloadable').exclude(error_reason='').update(error_reason='') + + dependencies = [ + ('certificates', '0024_delete_allowlistgenerationconfiguration'), + ] + + operations = [ + migrations.AlterModelManagers( + name='generatedcertificate', + managers=[ + ], + ), + migrations.RunPython(cleanup_certificate_records, reverse_code=migrations.RunPython.noop) + ] diff --git a/lms/djangoapps/certificates/models.py b/lms/djangoapps/certificates/models.py index 4d9674f711..3028a22449 100644 --- a/lms/djangoapps/certificates/models.py +++ b/lms/djangoapps/certificates/models.py @@ -202,7 +202,7 @@ class EligibleAvailableCertificateManager(EligibleCertificateManager): A manager for `GeneratedCertificate` models that automatically filters out ineligible certs and any linked to nonexistent courses. - Adds to the super class filtering ot also exclude certificates for + Adds to the super class filtering to also exclude certificates for courses that do not have a corresponding CourseOverview. """ @@ -233,6 +233,11 @@ class GeneratedCertificate(models.Model): # the course_modes app is loaded, resulting in a Django deprecation warning. from common.djangoapps.course_modes.models import CourseMode # pylint: disable=reimported + # Normal object manager, which should only be used when ineligible + # certificates (i.e. new audit certs) should be included in the + # results. Django requires us to explicitly declare this. + objects = models.Manager() + # Only returns eligible certificates. This should be used in # preference to the default `objects` manager in most cases. eligible_certificates = EligibleCertificateManager() @@ -241,11 +246,6 @@ class GeneratedCertificate(models.Model): # associated CourseOverview eligible_available_certificates = EligibleAvailableCertificateManager() - # Normal object manager, which should only be used when ineligible - # certificates (i.e. new audit certs) should be included in the - # results. Django requires us to explicitly declare this. - objects = models.Manager() - MODES = Choices( 'verified', 'honor',