Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: remove broken management command (and celery task) logic (#34972)

Browse Source 
[APER-3385]

This PR fixes an existing management command that now has incorrect logic. We have recently done a lot of work to
improve certificate-related date business logic to fix data inconsistencies between systems. Instead of maintaining
separate and duplicated logic for sending date data to Credentials, instead we can use an existing (and tested)
Celery task that will determine and send the correct date to the Credentials IDA.

Additionally, the original version of this management command skipped self-paced courses completely. This is no
longer the case as we _know_ that there are self-paced courses that have been associated with a certificate available
date because of bugs in the product. This management command will serve as a means to do a mass data fixup for data stored by the Credentials IDA.
This commit is contained in:
Justin Hynes
2024-06-12 09:44:11 -04:00
committed by GitHub
parent 66fa388dea
commit 1e653d74e7
3 changed files with 115 additions and 53 deletions
Download Patch File Download Diff File Expand all files Collapse all files

15
openedx/core/djangoapps/credentials/management/commands/update_credentials_available_date.py
View File

@@ -1,10 +1,6 @@
"""
A manangement command to populate the `certificate_available_date` field of the CourseCertificateConfiguration model in
the Credentials IDA.
This command is designed to be ran once to initially backpopulate data. Otherwise, anytime an existing course run
adjusts its certificate available date or certificates display behavior settings, updates will automatically be queued
and transmit to the Credentials IDA.
A manangement command to populate or correct the `certificate_available_date` data of the
CourseCertificateConfiguration model instances stored by the Credentials IDA.
"""
from django.core.management.base import BaseCommand
@@ -13,8 +9,11 @@ from openedx.core.djangoapps.credentials.tasks.v1.tasks import backfill_date_for
class Command(BaseCommand):
"""
A management command reponsible for populating the `certificate_available_date` field of
CourseCertificateConfiguration instances in the Credentials IDA.
Enqueue the `backfill_date_for_all_course_runs` Celery task, which will enqueue additional subtasks responsible for
sending certificate availability updates to the Credentials IDA.
Example usage:
$ ./manage.py lms update_credentials_available_date
"""
def handle(self, *args, **options):
backfill_date_for_all_course_runs.delay()

52
openedx/core/djangoapps/credentials/tasks/v1/tasks.py
View File

@@ -30,6 +30,7 @@ from openedx.core.djangoapps.programs.signals import (
handle_course_cert_changed,
handle_course_cert_revoked,
)
from openedx.core.djangoapps.programs.tasks import update_certificate_available_date_on_course_update
from openedx.core.djangoapps.site_configuration.models import SiteConfiguration
User = get_user_model()
@@ -429,51 +430,18 @@ def is_course_run_in_a_program(course_run_key):
@set_code_owner_attribute
def backfill_date_for_all_course_runs():
"""
This task will update the course certificate configuration's certificate_available_date in credentials for all
course runs. This is different from the "visable_date" attribute. This date will either be the available date that
is set in studio for a given course, or it will be None. This will exclude any course runs that do not have a
certificate_available_date or are self paced.
This task enqueues an `update_certificate_available_date_on_course_update` subtask for each course overview in the
system in order to determine and update the certificate date stored by the Credentials IDA.
"""
course_run_list = CourseOverview.objects.exclude(self_paced=True).exclude(certificate_available_date=None)
for index, course_run in enumerate(course_run_list):
course_overviews = CourseOverview.objects.all()
for index, course_overview in enumerate(course_overviews):
logger.info(
f"updating certificate_available_date for course {course_run.id} "
f"with date {course_run.certificate_available_date}"
"Enqueueing an `update_certificate_available_date_on_course_update` task for course run "
f"`{course_overview.id}`, self_paced={course_overview.self_paced}, end={course_overview.end}, "
f"available_date={course_overview.certificate_available_date}, and "
f"display_behavior={course_overview.certificates_display_behavior}"
)
course_key = str(course_run.id)
course_modes = CourseMode.objects.filter(course_id=course_key)
# There should only ever be one certificate relevant mode per course run
modes = [
mode.slug for mode in course_modes
if mode.slug in CourseMode.CERTIFICATE_RELEVANT_MODES or CourseMode.is_eligible_for_certificate(mode.slug)
]
if len(modes) != 1:
logger.exception(
f'Either course {course_key} has no certificate mode or multiple modes. Task failed.'
)
# if there is only one relevant mode, post to credentials
else:
try:
credentials_client = get_credentials_api_client(
User.objects.get(username=settings.CREDENTIALS_SERVICE_USERNAME),
)
api_url = urljoin(f"{get_credentials_api_base_url()}/", "course_certificates/")
response = credentials_client.post(
api_url,
json={
"course_id": course_key,
"certificate_type": modes[0],
"certificate_available_date": course_run.certificate_available_date.strftime(
'%Y-%m-%dT%H:%M:%SZ'
),
"is_active": True,
}
)
response.raise_for_status()
update_certificate_available_date_on_course_update.delay(str(course_overview.id))
logger.info(f"certificate_available_date updated for course {course_key}")
except Exception: # lint-amnesty, pylint: disable=W0703
error_msg = f"Failed to send certificate_available_date for course {course_key}."
logger.exception(error_msg)
if index % 10 == 0:
time.sleep(3)

101
openedx/core/djangoapps/credentials/tests/test_tasks.py
View File

@@ -2,8 +2,9 @@
Test credentials tasks
"""
import logging
from unittest import mock
from datetime import datetime, timezone
from datetime import datetime, timezone, timedelta
import ddt
import pytest
@@ -13,6 +14,7 @@ from django.db import connection, reset_queries
from django.test import TestCase, override_settings
from freezegun import freeze_time
from opaque_keys.edx.keys import CourseKey
from testfixtures import LogCapture
from common.djangoapps.student.tests.factories import UserFactory
from lms.djangoapps.certificates.api import get_recently_modified_certificates
@@ -23,14 +25,16 @@ from lms.djangoapps.grades.models_api import get_recently_modified_grades
from lms.djangoapps.grades.tests.utils import mock_passing_grade
from lms.djangoapps.certificates.tests.factories import CertificateDateOverrideFactory, GeneratedCertificateFactory
from openedx.core.djangoapps.catalog.tests.factories import CourseFactory, CourseRunFactory, ProgramFactory
from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory
from openedx.core.djangoapps.credentials.helpers import is_learner_records_enabled
from openedx.core.djangoapps.credentials.tasks.v1 import tasks
from openedx.core.djangoapps.site_configuration.tests.factories import SiteConfigurationFactory, SiteFactory
from openedx.core.djangolib.testing.utils import skip_unless_lms
from openedx.core.djangoapps.credentials.tasks.v1 import tasks
from xmodule.data import CertificatesDisplayBehaviors
User = get_user_model()
LOGGER_NAME = "openedx.core.djangoapps.credentials.tasks.v1.tasks"
TASKS_MODULE = 'openedx.core.djangoapps.credentials.tasks.v1.tasks'
@@ -757,3 +761,94 @@ class TestSendNotifications(TestCase):
else:
mock_revoked.assert_not_called()
mock_awarded.assert_called_once_with(**expected_signal_args)
@skip_unless_lms
class TestBackfillDateForAllCourseRuns(TestCase):
"""
Unit Tests for the `backfill_date_for_all_course_runs` Celery task.
"""
def setUp(self):
super().setUp()
self.co_instructor_paced_cdb_early_no_info_key = "course-v1:OpenEdX+InstructorPacedEarly+Run1"
self.co_instructor_paced_cbd_end_key = "course-v1:OpenEdX+InstructorPacedEnd+Run1"
self.co_instructor_paced_cdb_end_with_date_key = "course-v1:OpenEdX+InstructorPacedCAD+Run1"
self.co_self_paced_key = "course-v1:OpenEdX+SelfPaced+Run1"
self.course_run_keys = [
self.co_instructor_paced_cdb_early_no_info_key,
self.co_instructor_paced_cbd_end_key,
self.co_instructor_paced_cdb_end_with_date_key,
self.co_self_paced_key,
]
self.co_instructor_paced_cdb_early_no_info = CourseOverviewFactory(
certificate_available_date=None,
certificates_display_behavior=CertificatesDisplayBehaviors.EARLY_NO_INFO,
id=CourseKey.from_string(self.co_instructor_paced_cdb_early_no_info_key),
self_paced=False,
)
self.co_instructor_paced_cbd_end = CourseOverviewFactory(
certificate_available_date=None,
certificates_display_behavior=CertificatesDisplayBehaviors.END,
id=CourseKey.from_string(self.co_instructor_paced_cbd_end_key),
self_paced=False,
)
self.co_instructor_paced_cdb_end_with_date = CourseOverviewFactory(
certificate_available_date=(datetime.now(timezone.utc) + timedelta(days=30)),
certificates_display_behavior=CertificatesDisplayBehaviors.END_WITH_DATE,
id=CourseKey.from_string(self.co_instructor_paced_cdb_end_with_date_key),
self_paced=False,
)
self.co_self_paced = CourseOverviewFactory(
id=CourseKey.from_string(self.co_self_paced_key),
self_paced=True,
)
@mock.patch(
'openedx.core.djangoapps.credentials.tasks.v1.tasks.update_certificate_available_date_on_course_update.delay'
)
def test_backfill_dates(self, mock_update):
"""
Tests that the `backfill_date_for_all_course_runs` task enqueues the expected number of
`update_certificate_available_date_on_course_update` subtasks. We also capture and verify the contents of the
logs to ensure debugging capabilities by humans.
"""
expected_messages = [
# instructor-paced, cdb=early_no_info msg
"Enqueueing an `update_certificate_available_date_on_course_update` task for course run "
f"`{self.co_instructor_paced_cdb_early_no_info.id}`, "
f"self_paced={self.co_instructor_paced_cdb_early_no_info.self_paced}, "
f"end={self.co_instructor_paced_cdb_early_no_info.end}, "
f"available_date={self.co_instructor_paced_cdb_early_no_info.certificate_available_date}, and "
f"display_behavior={self.co_instructor_paced_cdb_early_no_info.certificates_display_behavior.value}",
# instructor-paced, cdb=end msg
"Enqueueing an `update_certificate_available_date_on_course_update` task for course run "
f"`{self.co_instructor_paced_cbd_end.id}`, "
f"self_paced={self.co_instructor_paced_cbd_end.self_paced}, "
f"end={self.co_instructor_paced_cbd_end.end}, "
f"available_date={self.co_instructor_paced_cbd_end.certificate_available_date}, and "
f"display_behavior={self.co_instructor_paced_cbd_end.certificates_display_behavior.value}",
# instructor-paced, cdb=end_with_date msg
"Enqueueing an `update_certificate_available_date_on_course_update` task for course run "
f"`{self.co_instructor_paced_cdb_end_with_date.id}`, "
f"self_paced={self.co_instructor_paced_cdb_end_with_date.self_paced}, "
f"end={self.co_instructor_paced_cdb_end_with_date.end}, "
f"available_date={self.co_instructor_paced_cdb_end_with_date.certificate_available_date}, and "
f"display_behavior={self.co_instructor_paced_cdb_end_with_date.certificates_display_behavior.value}",
# self-paced course run msg
"Enqueueing an `update_certificate_available_date_on_course_update` task for course run "
f"`{self.co_self_paced.id}`, self_paced={self.co_self_paced.self_paced}, end={self.co_self_paced.end}, "
f"available_date={self.co_self_paced.certificate_available_date}, and "
f"display_behavior={self.co_self_paced.certificates_display_behavior}",
]
with LogCapture(LOGGER_NAME, level=logging.INFO) as log:
tasks.backfill_date_for_all_course_runs.delay()
# verify the content of captured log messages
for message in expected_messages:
log.check_present((LOGGER_NAME, 'INFO', message))
# verify that our mocked function has the expected calls
assert mock_update.call_count == len(expected_messages)
for mock_call in mock_update.call_args_list:
assert mock_call.args[0] in self.course_run_keys