Merge pull request #27607 from edx/jhynes/microba-1178_modulestore-cont
refactor: remove use of modulestore in certificates app
This commit is contained in:
@@ -27,7 +27,7 @@ from opaque_keys.edx.keys import CourseKey
|
||||
|
||||
from lms.djangoapps.certificates.api import generate_user_certificates
|
||||
from lms.djangoapps.certificates.models import CertificateStatuses, GeneratedCertificate
|
||||
from xmodule.modulestore.django import modulestore
|
||||
from openedx.core.djangoapps.content.course_overviews.api import get_course_overview
|
||||
|
||||
LOGGER = logging.getLogger(__name__)
|
||||
|
||||
@@ -92,10 +92,9 @@ class Command(BaseCommand):
|
||||
course_cache = {}
|
||||
resubmit_count = 0
|
||||
for user, course_key in resubmit_list:
|
||||
course = self._load_course_with_cache(course_key, course_cache)
|
||||
|
||||
if course is not None:
|
||||
generate_user_certificates(user, course_key, course=course)
|
||||
course_overview = self._load_course_overview_with_cache(course_key, course_cache)
|
||||
if course_overview:
|
||||
generate_user_certificates(user, course_key)
|
||||
resubmit_count += 1
|
||||
LOGGER.info(
|
||||
(
|
||||
@@ -113,11 +112,11 @@ class Command(BaseCommand):
|
||||
|
||||
LOGGER.info("Finished resubmitting %s certificate tasks", resubmit_count)
|
||||
|
||||
def _load_course_with_cache(self, course_key, course_cache):
|
||||
"""Retrieve the course, then cache it to avoid Mongo queries. """
|
||||
course = (
|
||||
def _load_course_overview_with_cache(self, course_key, course_cache):
|
||||
"""Retrieve the course-overview for the given course-key and store it."""
|
||||
course_overview = (
|
||||
course_cache[course_key] if course_key in course_cache
|
||||
else modulestore().get_course(course_key, depth=0)
|
||||
else get_course_overview(course_key)
|
||||
)
|
||||
course_cache[course_key] = course
|
||||
return course
|
||||
course_cache[course_key] = course_overview
|
||||
return course_overview
|
||||
|
||||
@@ -17,8 +17,9 @@ from lms.djangoapps.badges.tests.factories import BadgeAssertionFactory, CourseC
|
||||
from lms.djangoapps.certificates.models import CertificateStatuses, GeneratedCertificate
|
||||
from lms.djangoapps.certificates.tests.factories import GeneratedCertificateFactory
|
||||
from lms.djangoapps.grades.tests.utils import mock_passing_grade
|
||||
from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory
|
||||
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
|
||||
from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory, check_mongo_calls
|
||||
from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
|
||||
|
||||
|
||||
class CertificateManagementTest(ModuleStoreTestCase):
|
||||
@@ -73,8 +74,7 @@ class ResubmitErrorCertificatesTest(CertificateManagementTest):
|
||||
self._create_cert(self.courses[0].id, self.user, CertificateStatuses.error, mode)
|
||||
|
||||
# Re-submit all certificates with status 'error'
|
||||
with check_mongo_calls(1):
|
||||
call_command(self.command)
|
||||
call_command(self.command)
|
||||
|
||||
# Expect that the certificate was re-submitted
|
||||
self._assert_cert_status(self.courses[0].id, self.user, CertificateStatuses.notpassing)
|
||||
@@ -129,25 +129,24 @@ class ResubmitErrorCertificatesTest(CertificateManagementTest):
|
||||
self._create_cert(self.courses[0].id, UserFactory.create(), CertificateStatuses.error)
|
||||
self._create_cert(self.courses[0].id, UserFactory.create(), CertificateStatuses.error)
|
||||
|
||||
# Verify that we make only one Mongo query
|
||||
# because the course is cached.
|
||||
with check_mongo_calls(1):
|
||||
course_overview = CourseOverviewFactory.create(
|
||||
id=self.courses[0].id
|
||||
)
|
||||
|
||||
with patch(
|
||||
'lms.djangoapps.certificates.management.commands.resubmit_error_certificates.get_course_overview'
|
||||
) as mock_get_course_overview:
|
||||
mock_get_course_overview.return_value = course_overview
|
||||
|
||||
call_command(self.command)
|
||||
|
||||
mock_get_course_overview.assert_called_once()
|
||||
|
||||
def test_invalid_course_key(self):
|
||||
invalid_key = "invalid/"
|
||||
with self.assertRaisesRegex(CommandError, invalid_key):
|
||||
call_command(self.command, course_key_list=[invalid_key])
|
||||
|
||||
def test_course_does_not_exist(self):
|
||||
phantom_course = CourseLocator(org='phantom', course='phantom', run='phantom')
|
||||
self._create_cert(phantom_course, self.user, CertificateStatuses.error)
|
||||
call_command(self.command)
|
||||
|
||||
# Expect that the certificate was NOT resubmitted
|
||||
# since the course doesn't actually exist.
|
||||
self._assert_cert_status(phantom_course, self.user, CertificateStatuses.error)
|
||||
|
||||
|
||||
@ddt.ddt
|
||||
class RegenerateCertificatesTest(CertificateManagementTest):
|
||||
|
||||
@@ -14,7 +14,6 @@ from pytz import UTC
|
||||
|
||||
from lms.djangoapps.certificates.api import generate_user_certificates
|
||||
from lms.djangoapps.certificates.models import CertificateStatuses, certificate_status_for_student
|
||||
from xmodule.modulestore.django import modulestore
|
||||
|
||||
LOGGER = logging.getLogger(__name__)
|
||||
User = get_user_model()
|
||||
@@ -91,9 +90,6 @@ class Command(BaseCommand):
|
||||
ended_courses = [course]
|
||||
|
||||
for course_key in ended_courses:
|
||||
# prefetch all chapters/sequentials by saying depth=2
|
||||
course = modulestore().get_course(course_key, depth=2)
|
||||
|
||||
enrolled_students = User.objects.filter(
|
||||
courseenrollment__course_id=course_key
|
||||
)
|
||||
@@ -133,7 +129,6 @@ class Command(BaseCommand):
|
||||
generate_user_certificates(
|
||||
student,
|
||||
course_key,
|
||||
course=course,
|
||||
insecure=options['insecure']
|
||||
)
|
||||
|
||||
|
||||
@@ -27,7 +27,7 @@ from lms.djangoapps.certificates.models import (
|
||||
)
|
||||
from lms.djangoapps.grades.api import CourseGradeFactory
|
||||
from lms.djangoapps.verify_student.services import IDVerificationService
|
||||
from xmodule.modulestore.django import modulestore
|
||||
from openedx.core.djangoapps.content.course_overviews.api import get_course_overview
|
||||
|
||||
LOGGER = logging.getLogger(__name__)
|
||||
|
||||
@@ -253,12 +253,6 @@ class XQueueCertInterface:
|
||||
)
|
||||
return None
|
||||
|
||||
# The caller can optionally pass a course in to avoid
|
||||
# re-fetching it from Mongo. If they have not provided one,
|
||||
# get it from the modulestore.
|
||||
if course is None:
|
||||
course = modulestore().get_course(course_id, depth=0)
|
||||
|
||||
profile = UserProfile.objects.get(user=student)
|
||||
profile_name = profile.name
|
||||
|
||||
@@ -267,7 +261,7 @@ class XQueueCertInterface:
|
||||
self.request.session = {}
|
||||
|
||||
is_whitelisted = self.whitelist.filter(user=student, course_id=course_id, whitelist=True).exists()
|
||||
course_grade = CourseGradeFactory().read(student, course)
|
||||
course_grade = CourseGradeFactory().read(student, course_key=course_id)
|
||||
enrollment_mode, __ = CourseEnrollment.enrollment_mode_for_user(student, course_id)
|
||||
mode_is_verified = enrollment_mode in GeneratedCertificate.VERIFIED_CERTS_MODES
|
||||
user_is_verified = IDVerificationService.user_is_verified(student)
|
||||
@@ -400,14 +394,15 @@ class XQueueCertInterface:
|
||||
return cert
|
||||
|
||||
# Finally, generate the certificate and send it off.
|
||||
return self._generate_cert(cert, course, student, grade_contents, template_pdf, generate_pdf)
|
||||
return self._generate_cert(cert, student, grade_contents, template_pdf, generate_pdf)
|
||||
|
||||
def _generate_cert(self, cert, course, student, grade_contents, template_pdf, generate_pdf):
|
||||
def _generate_cert(self, cert, student, grade_contents, template_pdf, generate_pdf):
|
||||
"""
|
||||
Generate a certificate for the student. If `generate_pdf` is True,
|
||||
sends a request to XQueue.
|
||||
"""
|
||||
course_id = str(course.id)
|
||||
course_id = str(cert.course_id)
|
||||
course_overview = get_course_overview(course_id)
|
||||
|
||||
key = make_hashkey(random.random())
|
||||
cert.key = key
|
||||
@@ -415,7 +410,7 @@ class XQueueCertInterface:
|
||||
'action': 'create',
|
||||
'username': student.username,
|
||||
'course_id': course_id,
|
||||
'course_name': course.display_name or course_id,
|
||||
'course_name': course_overview.display_name or course_id,
|
||||
'name': cert.name,
|
||||
'grade': grade_contents,
|
||||
'template_pdf': template_pdf,
|
||||
|
||||
Reference in New Issue
Block a user