From 59fefa10ebab73597e363bbf79816e7e0e511ab2 Mon Sep 17 00:00:00 2001 From: oliviaruizknott Date: Fri, 10 Sep 2021 08:11:50 -0600 Subject: [PATCH] feat: Change override date to datetime MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When first building the Certificate Date Override feature, I set up the CertificateDateOverride model to store the override dates as Dates instead of DateTimes. Turns out this is not how edX typically handles dates, and it’s causing some minor headaches around needing to convert values. Also, using just Dates causes timezone issues. MICROBA-1488 --- ...032_change_certificatedateoverride_date.py | 23 +++++++++++++++++++ lms/djangoapps/certificates/models.py | 4 ++-- .../certificates/tests/factories.py | 2 +- openedx/core/djangoapps/programs/tasks.py | 11 ++++----- .../djangoapps/programs/tests/test_tasks.py | 2 +- 5 files changed, 32 insertions(+), 10 deletions(-) create mode 100644 lms/djangoapps/certificates/migrations/0032_change_certificatedateoverride_date.py diff --git a/lms/djangoapps/certificates/migrations/0032_change_certificatedateoverride_date.py b/lms/djangoapps/certificates/migrations/0032_change_certificatedateoverride_date.py new file mode 100644 index 0000000000..47556dff5a --- /dev/null +++ b/lms/djangoapps/certificates/migrations/0032_change_certificatedateoverride_date.py @@ -0,0 +1,23 @@ +# Generated by Django 2.2.24 on 2021-09-09 22:10 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('certificates', '0031_certificatedateoverride_historicalcertificatedateoverride'), + ] + + operations = [ + migrations.AlterField( + model_name='certificatedateoverride', + name='date', + field=models.DateTimeField(help_text="The date to display on the certificate. Set 'Time' to midnight (00:00:00)."), + ), + migrations.AlterField( + model_name='historicalcertificatedateoverride', + name='date', + field=models.DateTimeField(help_text="The date to display on the certificate. Set 'Time' to midnight (00:00:00)."), + ), + ] diff --git a/lms/djangoapps/certificates/models.py b/lms/djangoapps/certificates/models.py index 67d1158fcf..c9f9bf6eca 100644 --- a/lms/djangoapps/certificates/models.py +++ b/lms/djangoapps/certificates/models.py @@ -1214,8 +1214,8 @@ class CertificateDateOverride(TimeStampedModel): related_name='date_override', help_text="The id of the Generated Certificate to override", ) - date = models.DateField( - help_text="The date to display on the certificate", + date = models.DateTimeField( + help_text="The date to display on the certificate. Set 'Time' to midnight (00:00:00).", ) reason = models.TextField( help_text="The reason why you are overriding the certificate date (Update this when you add OR edit the date.)", diff --git a/lms/djangoapps/certificates/tests/factories.py b/lms/djangoapps/certificates/tests/factories.py index 10e6d747db..15d5386aa4 100644 --- a/lms/djangoapps/certificates/tests/factories.py +++ b/lms/djangoapps/certificates/tests/factories.py @@ -115,5 +115,5 @@ class CertificateDateOverrideFactory(DjangoModelFactory): class Meta: model = CertificateDateOverride - date = datetime.datetime(2021, 5, 11) + date = datetime.datetime(2021, 5, 11, 0, 0, tzinfo=datetime.timezone.utc) reason = "Learner really wanted this on their birthday" diff --git a/openedx/core/djangoapps/programs/tasks.py b/openedx/core/djangoapps/programs/tasks.py index 869d73f3dc..40c727c5bf 100644 --- a/openedx/core/djangoapps/programs/tasks.py +++ b/openedx/core/djangoapps/programs/tasks.py @@ -33,8 +33,7 @@ MAX_RETRIES = 11 PROGRAM_CERTIFICATE = 'program' COURSE_CERTIFICATE = 'course-run' -VISIBLE_DATE_FORMAT = '%Y-%m-%dT%H:%M:%SZ' -DATE_OVERRIDE_FORMAT = '%Y-%m-%d' +DATE_FORMAT = '%Y-%m-%dT%H:%M:%SZ' def get_completed_programs(site, student): @@ -120,7 +119,7 @@ def award_program_certificate(client, user, program_uuid, visible_date): 'attributes': [ { 'name': 'visible_date', - 'value': visible_date.strftime(VISIBLE_DATE_FORMAT) + 'value': visible_date.strftime(DATE_FORMAT) } ] }) @@ -310,11 +309,11 @@ def post_course_certificate(client, username, certificate, visible_date, date_ov 'mode': certificate.mode, 'type': COURSE_CERTIFICATE, }, - 'date_override': {'date': date_override.strftime(DATE_OVERRIDE_FORMAT)} if date_override else None, + 'date_override': {'date': date_override.strftime(DATE_FORMAT)} if date_override else None, 'attributes': [ { 'name': 'visible_date', - 'value': visible_date.strftime(VISIBLE_DATE_FORMAT) + 'value': visible_date.strftime(DATE_FORMAT) } ] }) @@ -450,7 +449,7 @@ def award_course_certificate(self, username, course_run_key, certificate_availab # Date is being passed via JSON and is encoded in the EMCA date time string format. The rest of the code # expects a datetime. if certificate_available_date: - certificate_available_date = datetime.strptime(certificate_available_date, VISIBLE_DATE_FORMAT) + certificate_available_date = datetime.strptime(certificate_available_date, DATE_FORMAT) # Even in the cases where this task is called with a certificate_available_date, we still need to retrieve # the course overview because it's required to determine if we should use the certificate_available_date or diff --git a/openedx/core/djangoapps/programs/tests/test_tasks.py b/openedx/core/djangoapps/programs/tests/test_tasks.py index 5a6f9869ce..7e18b66699 100644 --- a/openedx/core/djangoapps/programs/tests/test_tasks.py +++ b/openedx/core/djangoapps/programs/tests/test_tasks.py @@ -587,7 +587,7 @@ class AwardCourseCertificatesTestCase(CredentialsApiConfigMixin, TestCase): assert call_args[1] == self.student.username assert call_args[2] == self.certificate assert call_args[3] == self.certificate.modified_date - assert call_args[4] == self.certificate.date_override.date.date() + assert call_args[4] == self.certificate.date_override.date def test_award_course_cert_not_called_if_disabled(self, mock_post_course_certificate): """