From f64c7332f84d058989b525705debaaccc67d2f9f Mon Sep 17 00:00:00 2001 From: Justin Hynes Date: Tue, 4 May 2021 13:03:48 -0400 Subject: [PATCH] refactor: Remove use of modulestore in certificates app [MICROBA-1178] * Remove use of modulestore in `../certificates/generation.py`. * Remove use of modulestore in `../certificates/views/support.py`. * Remove use of modulestore in `../certificates/views/xqueue.py`. --- lms/djangoapps/certificates/generation.py | 25 ++++++--------- lms/djangoapps/certificates/views/support.py | 32 +++++++++----------- lms/djangoapps/certificates/views/xqueue.py | 6 ++-- 3 files changed, 26 insertions(+), 37 deletions(-) diff --git a/lms/djangoapps/certificates/generation.py b/lms/djangoapps/certificates/generation.py index 7c19313178..93a3710466 100644 --- a/lms/djangoapps/certificates/generation.py +++ b/lms/djangoapps/certificates/generation.py @@ -15,10 +15,10 @@ from uuid import uuid4 from common.djangoapps.student.models import CourseEnrollment, UserProfile from lms.djangoapps.certificates.models import CertificateStatuses, GeneratedCertificate from lms.djangoapps.certificates.queue import XQueueCertInterface -from lms.djangoapps.certificates.utils import emit_certificate_event, has_html_certificates_enabled +from lms.djangoapps.certificates.utils import emit_certificate_event, has_html_certificates_enabled_from_course_overview from lms.djangoapps.grades.api import CourseGradeFactory -from lms.djangoapps.instructor.access import list_with_level -from xmodule.modulestore.django import modulestore +from lms.djangoapps.instructor.access import list_with_level_from_course_key +from openedx.core.djangoapps.content.course_overviews.api import get_course_overview log = logging.getLogger(__name__) @@ -63,8 +63,7 @@ def _generate_certificate(user, course_key): profile = UserProfile.objects.get(user=user) profile_name = profile.name - course = modulestore().get_course(course_key, depth=0) - course_grade = CourseGradeFactory().read(user, course) + course_grade = CourseGradeFactory().read(user, course_key=course_key) enrollment_mode, __ = CourseEnrollment.enrollment_mode_for_user(user, course_key) # Retain the `verify_uuid` from an existing certificate if possible, this will make it possible for the learner to @@ -126,22 +125,17 @@ def generate_user_certificates(student, course_key, course=None, insecure=False, forced_grade - a string indicating to replace grade parameter. if present grading will be skipped. """ - - if not course: - course = modulestore().get_course(course_key, depth=0) - - beta_testers_queryset = list_with_level(course, 'beta') - + beta_testers_queryset = list_with_level_from_course_key(course_key, 'beta') if beta_testers_queryset.filter(username=student.username): - message = 'Cancelling course certificate generation for user [{}] against course [{}], user is a Beta Tester.' - log.info(message.format(student.username, course_key)) + log.info(f"Canceling Certificate Generation task for user {student.id} : {course_key}. User is a Beta Tester.") return xqueue = XQueueCertInterface() if insecure: xqueue.use_https = False - generate_pdf = not has_html_certificates_enabled(course) + course_overview = get_course_overview(course_key) + generate_pdf = not has_html_certificates_enabled_from_course_overview(course_overview) cert = xqueue.add_cert( student, @@ -151,8 +145,7 @@ def generate_user_certificates(student, course_key, course=None, insecure=False, forced_grade=forced_grade ) - message = 'Queued Certificate Generation task for {user} : {course}' - log.info(message.format(user=student.id, course=course_key)) + log.info(f"Queued Certificate Generation task for {student.id} : {course_key}") # If cert_status is not present in certificate valid_statuses (for example unverified) then # add_cert returns None and raises AttributeError while accessing cert attributes. diff --git a/lms/djangoapps/certificates/views/support.py b/lms/djangoapps/certificates/views/support.py index 543ebd36fc..82166ee5ba 100644 --- a/lms/djangoapps/certificates/views/support.py +++ b/lms/djangoapps/certificates/views/support.py @@ -24,8 +24,8 @@ from common.djangoapps.util.json_request import JsonResponse from lms.djangoapps.certificates.api import get_certificates_for_user, regenerate_user_certificates from lms.djangoapps.certificates.permissions import GENERATE_ALL_CERTIFICATES, VIEW_ALL_CERTIFICATES from lms.djangoapps.instructor_task.api import generate_certificates_for_students +from openedx.core.djangoapps.content.course_overviews.api import get_course_overview from openedx.core.djangoapps.content.course_overviews.models import CourseOverview -from xmodule.modulestore.django import modulestore log = logging.getLogger(__name__) @@ -184,37 +184,35 @@ def regenerate_certificate_for_user(request): if response is not None: return response - # Check that the course exists - course = modulestore().get_course(params["course_key"]) - if course is None: - msg = _("The course {course_key} does not exist").format(course_key=params["course_key"]) + user = params["user"] + course_key = params["course_key"] + + try: + get_course_overview(course_key) + except CourseOverview.DoesNotExist: + msg = _("The course {course_key} does not exist").format(course_key=course_key) return HttpResponseBadRequest(msg) # Check that the user is enrolled in the course - if not CourseEnrollment.is_enrolled(params["user"], params["course_key"]): - msg = _("User {username} is not enrolled in the course {course_key}").format( - username=params["user"].username, - course_key=params["course_key"] + if not CourseEnrollment.is_enrolled(user, course_key): + msg = _("User {user_id} is not enrolled in the course {course_key}").format( + user_id=user.id, + course_key=course_key ) return HttpResponseBadRequest(msg) # Attempt to regenerate certificates try: - regenerate_user_certificates(params["user"], params["course_key"], course=course) + regenerate_user_certificates(user, course_key) except: # pylint: disable=bare-except # We are pessimistic about the kinds of errors that might get thrown by the # certificates API. This may be overkill, but we're logging everything so we can # track down unexpected errors. - log.exception( - "Could not regenerate certificates for user %s in course %s", - params["user"].id, - params["course_key"] - ) + log.exception(f"Could not regenerate certificate for user {user.id} in course {course_key}") return HttpResponseServerError(_("An unexpected error occurred while regenerating certificates.")) log.info( - "Started regenerating certificates for user %s in course %s from the support page.", - params["user"].id, params["course_key"] + f"Started regenerating certificates for user {user.id} in course {course_key} from the support page." ) return HttpResponse(200) diff --git a/lms/djangoapps/certificates/views/xqueue.py b/lms/djangoapps/certificates/views/xqueue.py index 56b49be24a..ff99759ab1 100644 --- a/lms/djangoapps/certificates/views/xqueue.py +++ b/lms/djangoapps/certificates/views/xqueue.py @@ -26,7 +26,6 @@ from lms.djangoapps.certificates.models import ( GeneratedCertificate, certificate_status_for_student ) -from xmodule.modulestore.django import modulestore log = logging.getLogger(__name__) User = get_user_model() @@ -48,9 +47,8 @@ def request_certificate(request): username = request.user.username student = User.objects.get(username=username) course_key = CourseKey.from_string(request.POST.get('course_id')) - course = modulestore().get_course(course_key, depth=2) - status = certificate_status_for_student(student, course_key)['status'] + if can_generate_certificate_task(student, course_key): log.info(f'{course_key} is using V2 course certificates. Attempt will be made to generate a V2 ' f'certificate for user {student.id}.') @@ -58,7 +56,7 @@ def request_certificate(request): elif status in [CertificateStatuses.unavailable, CertificateStatuses.notpassing, CertificateStatuses.error]: log_msg = 'Grading and certification requested for user %s in course %s via /request_certificate call' log.info(log_msg, username, course_key) - status = generate_user_certificates(student, course_key, course=course) + status = generate_user_certificates(student, course_key) return HttpResponse(json.dumps({'add_status': status}), content_type='application/json') # pylint: disable=http-response-with-content-type-json, http-response-with-json-dumps return HttpResponse(json.dumps({'add_status': 'ERRORANONYMOUSUSER'}), content_type='application/json') # pylint: disable=http-response-with-content-type-json, http-response-with-json-dumps