refactor: Remove use of CourseOverview model directly in the Certificates app
[MICROBA-1289] * Remove use of the CourseOverview model directly in the Certificates app * Introduce a few new Python API functions in the `course_overview` and `catalog` django apps to replace functionality in the Certificates app.
This commit is contained in:
@@ -44,7 +44,6 @@ from lms.djangoapps.certificates.utils import (
|
||||
has_html_certificates_enabled as _has_html_certificates_enabled
|
||||
)
|
||||
from openedx.core.djangoapps.content.course_overviews.api import get_course_overview_or_none
|
||||
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
|
||||
|
||||
log = logging.getLogger("edx.certificate")
|
||||
User = get_user_model()
|
||||
@@ -243,7 +242,7 @@ def certificate_downloadable_status(student, course_key):
|
||||
'uuid': None,
|
||||
}
|
||||
|
||||
course_overview = CourseOverview.get_from_id(course_key)
|
||||
course_overview = get_course_overview_or_none(course_key)
|
||||
if (
|
||||
not certificates_viewable_for_course(course_overview) and
|
||||
CertificateStatuses.is_passing_status(current_status['status']) and
|
||||
|
||||
@@ -396,7 +396,7 @@ class CertificatesListRestApiTest(AuthAndScopesTestMixin, SharedModuleStoreTestC
|
||||
expected_download_url = reverse('certificates:render_cert_by_uuid', kwargs=kwargs)
|
||||
self.assert_success_response_for_student(response, download_url=expected_download_url)
|
||||
|
||||
@patch('lms.djangoapps.certificates.apis.v0.views.get_course_run_details')
|
||||
@patch('openedx.core.djangoapps.content.course_overviews.api.get_course_run_details')
|
||||
def test_certificate_without_course(self, mock_get_course_run_details):
|
||||
"""
|
||||
Verify that certificates are returned for deleted XML courses.
|
||||
|
||||
@@ -21,8 +21,11 @@ from lms.djangoapps.certificates.api import (
|
||||
get_certificates_for_user
|
||||
)
|
||||
from lms.djangoapps.certificates.apis.v0.permissions import IsOwnerOrPublicCertificates
|
||||
from openedx.core.djangoapps.catalog.utils import get_course_run_details
|
||||
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
|
||||
from openedx.core.djangoapps.content.course_overviews.api import (
|
||||
get_course_overview_or_none,
|
||||
get_course_overviews_from_ids,
|
||||
get_pseudo_course_overview
|
||||
)
|
||||
from openedx.core.djangoapps.user_api.accounts.api import visible_fields
|
||||
from openedx.core.lib.api.authentication import BearerAuthenticationAllowInactiveUser
|
||||
|
||||
@@ -97,7 +100,7 @@ class CertificatesDetailView(APIView):
|
||||
|
||||
def get(self, request, username, course_id):
|
||||
"""
|
||||
Gets a certificate information.
|
||||
Retrieves certificate information for a user in a specified course run.
|
||||
|
||||
Args:
|
||||
request (Request): Django request object.
|
||||
@@ -123,9 +126,10 @@ class CertificatesDetailView(APIView):
|
||||
data={'error_code': 'no_certificate_for_user'}
|
||||
)
|
||||
|
||||
course_overview = CourseOverview.get_from_id(course_id)
|
||||
course_overview = get_course_overview_or_none(course_id)
|
||||
# return 404 if it's not a PDF certificates and there is no active certificate configuration.
|
||||
if not user_cert['is_pdf_certificate'] and not course_overview.has_any_active_web_certificate:
|
||||
if not user_cert['is_pdf_certificate'] and (not course_overview or
|
||||
not course_overview.has_any_active_web_certificate):
|
||||
return Response(
|
||||
status=404,
|
||||
data={'error_code': 'no_certificate_configuration_for_course'}
|
||||
@@ -174,7 +178,7 @@ class CertificatesListView(APIView):
|
||||
)
|
||||
])
|
||||
def get(self, request, username):
|
||||
"""Get a paginated list of bookmarks for a user.
|
||||
"""Get a paginated list of certificates for a user.
|
||||
|
||||
**Use Case**
|
||||
|
||||
@@ -270,32 +274,22 @@ class CertificatesListView(APIView):
|
||||
passing_certificates[course_key] = course_certificate
|
||||
|
||||
viewable_certificates = []
|
||||
for course_key, course_overview in CourseOverview.get_from_ids(
|
||||
list(passing_certificates.keys())
|
||||
).items():
|
||||
course_ids = list(passing_certificates.keys())
|
||||
course_overviews = get_course_overviews_from_ids(course_ids)
|
||||
for course_key, course_overview in course_overviews.items():
|
||||
if not course_overview:
|
||||
# For deleted XML courses in which learners have a valid certificate.
|
||||
# i.e. MITx/7.00x/2013_Spring
|
||||
course_overview = self._get_pseudo_course_overview(course_key)
|
||||
course_overview = get_pseudo_course_overview(course_key)
|
||||
if certificates_viewable_for_course(course_overview):
|
||||
course_certificate = passing_certificates[course_key]
|
||||
# add certificate into viewable certificate list only if it's a PDF certificate
|
||||
# or there is an active certificate configuration.
|
||||
if course_certificate['is_pdf_certificate'] or course_overview.has_any_active_web_certificate:
|
||||
if course_certificate['is_pdf_certificate'] or (course_overview and
|
||||
course_overview.has_any_active_web_certificate):
|
||||
course_certificate['course_display_name'] = course_overview.display_name_with_default
|
||||
course_certificate['course_organization'] = course_overview.display_org_with_default
|
||||
viewable_certificates.append(course_certificate)
|
||||
|
||||
viewable_certificates.sort(key=lambda certificate: certificate['created'])
|
||||
return viewable_certificates
|
||||
|
||||
def _get_pseudo_course_overview(self, course_key):
|
||||
"""
|
||||
Returns a pseudo course overview object for deleted courses.
|
||||
"""
|
||||
course_run = get_course_run_details(course_key, ['title'])
|
||||
return CourseOverview(
|
||||
display_name=course_run.get('title'),
|
||||
display_org_with_default=course_key.org,
|
||||
certificates_show_before_end=True
|
||||
)
|
||||
|
||||
@@ -30,7 +30,7 @@ from lms.djangoapps.badges.events.course_complete import course_badge_check
|
||||
from lms.djangoapps.badges.events.course_meta import completion_check, course_group_check
|
||||
from lms.djangoapps.certificates.data import CertificateStatuses
|
||||
from lms.djangoapps.instructor_task.models import InstructorTask
|
||||
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
|
||||
from openedx.core.djangoapps.content.course_overviews.api import get_course_overview_or_none
|
||||
from openedx.core.djangoapps.signals.signals import COURSE_CERT_AWARDED, COURSE_CERT_CHANGED, COURSE_CERT_REVOKED
|
||||
from openedx.core.djangoapps.xmodule_django.models import NoneToEmptyManager
|
||||
|
||||
@@ -657,7 +657,12 @@ def certificate_info_for_user(user, course_id, grade, user_is_allowlisted, user_
|
||||
certificate_type = 'N/A'
|
||||
status = certificate_status(user_certificate)
|
||||
certificate_generated = status['status'] == CertificateStatuses.downloadable
|
||||
can_have_certificate = CourseOverview.get_from_id(course_id).may_certify()
|
||||
|
||||
can_have_certificate = False
|
||||
course_overview = get_course_overview_or_none(course_id)
|
||||
if course_overview:
|
||||
can_have_certificate = course_overview.may_certify()
|
||||
|
||||
enrollment_mode, __ = CourseEnrollment.enrollment_mode_for_user(user, course_id)
|
||||
mode_is_verified = enrollment_mode in CourseMode.VERIFIED_MODES
|
||||
user_is_verified = grade is not None and mode_is_verified
|
||||
|
||||
@@ -25,7 +25,6 @@ from lms.djangoapps.certificates.api import generate_certificate_task, get_certi
|
||||
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_or_none
|
||||
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
|
||||
|
||||
log = logging.getLogger(__name__)
|
||||
|
||||
@@ -113,16 +112,16 @@ def search_certificates(request):
|
||||
except InvalidKeyError:
|
||||
return HttpResponseBadRequest(_("Course id '{course_id}' is not valid").format(course_id=course_id))
|
||||
else:
|
||||
try:
|
||||
if CourseOverview.get_from_id(course_key):
|
||||
certificates = [certificate for certificate in certificates
|
||||
if certificate['course_key'] == course_id]
|
||||
if not certificates:
|
||||
return JsonResponse([{'username': user.username, 'course_key': course_id, 'regenerate': False}])
|
||||
except CourseOverview.DoesNotExist:
|
||||
course_overview = get_course_overview_or_none(course_key)
|
||||
if not course_overview:
|
||||
msg = _("The course does not exist against the given key '{course_key}'").format(course_key=course_key)
|
||||
return HttpResponseBadRequest(msg)
|
||||
|
||||
certificates = [certificate for certificate in certificates
|
||||
if certificate['course_key'] == course_id]
|
||||
if not certificates:
|
||||
return JsonResponse([{'username': user.username, 'course_key': course_id, 'regenerate': False}])
|
||||
|
||||
return JsonResponse(certificates)
|
||||
|
||||
|
||||
@@ -245,26 +244,24 @@ def generate_certificate_for_user(request):
|
||||
if response is not None:
|
||||
return response
|
||||
|
||||
try:
|
||||
# Check that the course exists
|
||||
CourseOverview.get_from_id(params["course_key"])
|
||||
except CourseOverview.DoesNotExist:
|
||||
course_overview = get_course_overview_or_none(params["course_key"])
|
||||
if not course_overview:
|
||||
msg = _("The course {course_key} does not exist").format(course_key=params["course_key"])
|
||||
return HttpResponseBadRequest(msg)
|
||||
else:
|
||||
# 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"]
|
||||
)
|
||||
return HttpResponseBadRequest(msg)
|
||||
|
||||
# Attempt to generate certificate
|
||||
generate_certificates_for_students(
|
||||
request,
|
||||
params["course_key"],
|
||||
student_set="specific_student",
|
||||
specific_student_id=params["user"].id
|
||||
# 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"]
|
||||
)
|
||||
return HttpResponse(200)
|
||||
return HttpResponseBadRequest(msg)
|
||||
|
||||
# Attempt to generate certificate
|
||||
generate_certificates_for_students(
|
||||
request,
|
||||
params["course_key"],
|
||||
student_set="specific_student",
|
||||
specific_student_id=params["user"].id
|
||||
)
|
||||
return HttpResponse(200)
|
||||
|
||||
@@ -44,7 +44,7 @@ from lms.djangoapps.certificates.models import (
|
||||
)
|
||||
from lms.djangoapps.certificates.permissions import PREVIEW_CERTIFICATES
|
||||
from lms.djangoapps.certificates.utils import emit_certificate_event, get_certificate_url
|
||||
from openedx.core.djangoapps.catalog.utils import get_course_run_details
|
||||
from openedx.core.djangoapps.catalog.api import get_course_run_details
|
||||
from openedx.core.djangoapps.certificates.api import display_date_for_certificate
|
||||
from openedx.core.djangoapps.lang_pref.api import get_closest_released_language
|
||||
from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
|
||||
|
||||
@@ -5,6 +5,7 @@ Python APIs exposed by the catalog app to other in-process apps.
|
||||
from .utils import get_programs_by_type_slug as _get_programs_by_type_slug
|
||||
from .utils import get_programs as _get_programs
|
||||
from .utils import course_run_keys_for_program as _course_run_keys_for_program
|
||||
from .utils import get_course_run_details as _get_course_run_details
|
||||
|
||||
|
||||
def get_programs_by_type(site, program_type_slug):
|
||||
@@ -47,3 +48,16 @@ def get_course_run_key_for_program_from_cache(program):
|
||||
(set): A set of Course Run Keys.
|
||||
"""
|
||||
return _course_run_keys_for_program(parent_program=program)
|
||||
|
||||
|
||||
def get_course_run_details(course_key, fields_list):
|
||||
"""
|
||||
Retrieves course run details for a given course run key.
|
||||
|
||||
Params:
|
||||
course_key (CourseKey): The identifier for the course.
|
||||
fields_list (List, string): A list of fields (as strings) of values we want returned.
|
||||
Returns:
|
||||
dict containing response from the Discovery API that includes the fields specified in `fields_list`
|
||||
"""
|
||||
return _get_course_run_details(course_key, fields_list)
|
||||
|
||||
32
openedx/core/djangoapps/catalog/tests/test_api.py
Normal file
32
openedx/core/djangoapps/catalog/tests/test_api.py
Normal file
@@ -0,0 +1,32 @@
|
||||
"""
|
||||
Tests for the Catalog apps `api.py` functions.
|
||||
"""
|
||||
from mock import patch
|
||||
|
||||
from django.test import TestCase
|
||||
|
||||
from openedx.core.djangoapps.catalog.api import get_course_run_details
|
||||
from openedx.core.djangoapps.catalog.tests.factories import CourseRunFactory
|
||||
from openedx.core.djangolib.testing.utils import skip_unless_lms
|
||||
|
||||
|
||||
@skip_unless_lms
|
||||
class TestCatalogApi(TestCase):
|
||||
"""
|
||||
Tests for the Catalog apps `api.py` functions.
|
||||
"""
|
||||
|
||||
@patch("openedx.core.djangoapps.catalog.api._get_course_run_details")
|
||||
def test_get_course_run_details(self, mock_get_course_run_details):
|
||||
"""
|
||||
Test for Python API `get_course_run_details` function.
|
||||
"""
|
||||
course_run = CourseRunFactory()
|
||||
|
||||
mock_get_course_run_details.return_value = {
|
||||
'title': course_run['title'],
|
||||
}
|
||||
|
||||
results = get_course_run_details(course_run['key'], ['title'])
|
||||
|
||||
assert results['title'] == course_run['title']
|
||||
@@ -3,6 +3,7 @@ CourseOverview api
|
||||
"""
|
||||
import logging
|
||||
|
||||
from openedx.core.djangoapps.catalog.api import get_course_run_details
|
||||
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
|
||||
from openedx.core.djangoapps.content.course_overviews.serializers import (
|
||||
CourseOverviewBaseSerializer,
|
||||
@@ -24,9 +25,41 @@ def get_course_overview_or_none(course_id):
|
||||
return None
|
||||
|
||||
|
||||
def get_pseudo_course_overview(course_id):
|
||||
"""
|
||||
Returns a pseudo course overview object for a deleted course.
|
||||
|
||||
Params:
|
||||
course_id (CourseKey): The identifier for the course.
|
||||
Returns:
|
||||
(Temporary) CourseOverview object representing for a deleted course.
|
||||
"""
|
||||
fields = ['title']
|
||||
course_run = get_course_run_details(course_id, fields)
|
||||
|
||||
return CourseOverview(
|
||||
display_name=course_run.get('title'),
|
||||
display_org_with_default=course_id.org,
|
||||
certificates_show_before_end=True
|
||||
)
|
||||
|
||||
|
||||
def get_course_overviews_from_ids(course_ids):
|
||||
"""
|
||||
Return course overviews for the specified course ids.
|
||||
|
||||
Params:
|
||||
course_ids (iterable[CourseKey])
|
||||
|
||||
Returns:
|
||||
dict[CourseKey, CourseOverview|None]
|
||||
"""
|
||||
return CourseOverview.get_from_ids(course_ids)
|
||||
|
||||
|
||||
def get_course_overviews(course_ids):
|
||||
"""
|
||||
Return course_overview data for a given list of opaque_key course_ids.
|
||||
Return (serialized) course_overview data for a given list of opaque_key course_ids.
|
||||
"""
|
||||
overviews = CourseOverview.objects.filter(id__in=course_ids)
|
||||
return CourseOverviewBaseSerializer(overviews, many=True).data
|
||||
|
||||
@@ -1,16 +1,20 @@
|
||||
"""
|
||||
course_overview api tests
|
||||
"""
|
||||
from mock import patch
|
||||
|
||||
from opaque_keys.edx.keys import CourseKey
|
||||
|
||||
from openedx.core.djangoapps.catalog.tests.factories import CourseRunFactory
|
||||
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
|
||||
from openedx.core.djangoapps.content.course_overviews.api import (
|
||||
get_course_overview_or_none,
|
||||
get_course_overviews
|
||||
get_course_overviews,
|
||||
get_course_overviews_from_ids,
|
||||
get_pseudo_course_overview,
|
||||
)
|
||||
from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory
|
||||
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
|
||||
from ..models import CourseOverview
|
||||
|
||||
|
||||
class TestCourseOverviewsApi(ModuleStoreTestCase):
|
||||
@@ -39,6 +43,19 @@ class TestCourseOverviewsApi(ModuleStoreTestCase):
|
||||
retrieved_course_overview = get_course_overview_or_none(course_run_key)
|
||||
assert retrieved_course_overview is None
|
||||
|
||||
def test_get_course_overview_from_ids(self):
|
||||
"""
|
||||
Test for `get_course_overviews_from_ids` function.
|
||||
"""
|
||||
course_ids = []
|
||||
course_overview_data = CourseOverview.objects.all()
|
||||
for course_overview in course_overview_data:
|
||||
course_ids.append(course_overview.id)
|
||||
|
||||
results = get_course_overviews_from_ids(course_ids)
|
||||
|
||||
assert len(results) == 3
|
||||
|
||||
def test_get_course_overviews(self):
|
||||
"""
|
||||
get_course_overviews should return the expected CourseOverview data
|
||||
@@ -61,3 +78,20 @@ class TestCourseOverviewsApi(ModuleStoreTestCase):
|
||||
]
|
||||
for field in fields:
|
||||
assert field in data[0]
|
||||
|
||||
@patch("openedx.core.djangoapps.content.course_overviews.api.get_course_run_details")
|
||||
def test_get_pseudo_course_overview(self, mock_get_course_run_details):
|
||||
"""
|
||||
Test for the `get_pseudo_course_overview` function that creates a temporary course overview for courses that
|
||||
have been deleted.
|
||||
"""
|
||||
course_run = CourseRunFactory()
|
||||
mock_get_course_run_details.return_value = {
|
||||
'title': course_run['title'],
|
||||
}
|
||||
course_key = CourseKey.from_string(course_run['key'])
|
||||
|
||||
result = get_pseudo_course_overview(course_key)
|
||||
assert result.display_name == course_run['title']
|
||||
assert result.display_org_with_default == course_key.org
|
||||
assert result.certificates_show_before_end
|
||||
|
||||
Reference in New Issue
Block a user