Merge pull request #28237 from edx/tuchfarber/move_may_certify_2

Move cert display decisions to certificates app
This commit is contained in:
Matt Tuchfarber
2021-07-22 10:20:13 -04:00
committed by GitHub
18 changed files with 234 additions and 184 deletions

View File

@@ -39,10 +39,10 @@ from lms.djangoapps.certificates.api import (
certificates_viewable_for_course,
cert_generation_enabled,
get_certificate_url,
has_html_certificates_enabled
has_html_certificates_enabled,
certificate_status_for_student,
)
from lms.djangoapps.certificates.data import CertificateStatuses
from lms.djangoapps.certificates.models import certificate_status_for_student
from lms.djangoapps.grades.api import CourseGradeFactory
from lms.djangoapps.verify_student.models import VerificationDeadline
from lms.djangoapps.verify_student.services import IDVerificationService

View File

@@ -131,7 +131,7 @@ class TestStudentDashboardUnenrollments(SharedModuleStoreTestCase):
def test_cant_unenroll_status(self):
""" Assert that the dashboard loads when cert_status does not allow for unenrollment"""
with patch(
'lms.djangoapps.certificates.models.certificate_status_for_student',
'lms.djangoapps.certificates.api.certificate_status_for_student',
return_value={'status': 'downloadable'},
):
response = self.client.get(reverse('dashboard'))

View File

@@ -134,40 +134,6 @@ def course_start_date_is_default(start, advertised_start):
return advertised_start is None and start == DEFAULT_START_DATE
def may_certify_for_course(
certificates_display_behavior,
certificates_show_before_end,
has_ended,
certificate_available_date,
self_paced
):
"""
Returns whether it is acceptable to show the student a certificate download
link for a course, based on provided attributes of the course.
Arguments:
certificates_display_behavior (str): string describing the course's
certificate display behavior.
See CourseFields.certificates_display_behavior.help for more detail.
certificates_show_before_end (bool): whether user can download the
course's certificates before the course has ended.
has_ended (bool): Whether the course has ended.
certificate_available_date (datetime): the date the certificate is available on for the course.
self_paced (bool): Whether the course is self-paced.
"""
show_early = (
certificates_display_behavior in ('early_with_info', 'early_no_info')
or certificates_show_before_end
)
past_available_date = (
certificate_available_date
and certificate_available_date < datetime.now(utc)
)
ended_without_available_date = (certificate_available_date is None) and has_ended
return any((self_paced, show_early, past_available_date, ended_without_available_date))
def sorting_score(start, advertised_start, announcement):
"""
Returns a tuple that can be used to sort the courses according

View File

@@ -1197,18 +1197,6 @@ class CourseBlock(
"""
return course_metadata_utils.has_course_ended(self.end)
def may_certify(self):
"""
Return whether it is acceptable to show the student a certificate download link.
"""
return course_metadata_utils.may_certify_for_course(
self.certificates_display_behavior,
self.certificates_show_before_end,
self.has_ended(),
self.certificate_available_date,
self.self_paced
)
def has_started(self):
return course_metadata_utils.has_course_started(self.start)

View File

@@ -20,7 +20,6 @@ from xmodule.course_metadata_utils import (
course_start_date_is_default,
has_course_ended,
has_course_started,
may_certify_for_course,
number_for_course_location
)
from xmodule.modulestore.tests.utils import (
@@ -162,18 +161,6 @@ class CourseMetadataUtilsTestCase(TestCase):
TestScenario((DEFAULT_START_DATE, advertised_start_parsable), False),
TestScenario((DEFAULT_START_DATE, None), True),
]),
FunctionTest(may_certify_for_course, [
TestScenario(('early_with_info', True, True, test_datetime, False), True),
TestScenario(('early_no_info', False, False, test_datetime, False), True),
TestScenario(('end', True, False, test_datetime, False), True),
TestScenario(('end', False, True, test_datetime, False), True),
TestScenario(('end', False, False, _NEXT_WEEK, False), False),
TestScenario(('end', False, False, _LAST_WEEK, False), True),
TestScenario(('end', False, False, None, False), False),
TestScenario(('early_with_info', False, False, None, False), True),
TestScenario(('end', False, False, _NEXT_WEEK, False), False),
TestScenario(('end', False, False, _NEXT_WEEK, True), True),
]),
]
for function_test in function_tests:

View File

@@ -138,15 +138,6 @@ class HasEndedMayCertifyTestCase(unittest.TestCase):
assert not self.future_show_certs_no_info.has_ended()
assert not self.future_noshow_certs.has_ended()
def test_may_certify(self):
"""Check that may_certify correctly tells us when a course may wrap."""
assert self.past_show_certs.may_certify()
assert self.past_noshow_certs.may_certify()
assert self.past_show_certs_no_info.may_certify()
assert self.future_show_certs.may_certify()
assert self.future_show_certs_no_info.may_certify()
assert not self.future_noshow_certs.may_certify()
class CourseSummaryHasEnded(unittest.TestCase):
""" Test for has_ended method when end date is missing timezone information. """

View File

@@ -19,6 +19,7 @@ from eventtracking import tracker
from opaque_keys.edx.django.models import CourseKeyField
from organizations.api import get_course_organization_id
from common.djangoapps.course_modes.models import CourseMode
from common.djangoapps.student.api import is_user_enrolled_in_course
from common.djangoapps.student.models import CourseEnrollment
from lms.djangoapps.branding import api as branding_api
@@ -36,12 +37,14 @@ from lms.djangoapps.certificates.models import (
CertificateTemplateAsset,
ExampleCertificateSet,
GeneratedCertificate,
certificate_status_for_student
)
from lms.djangoapps.certificates.queue import XQueueCertInterface
from lms.djangoapps.certificates.utils import (
get_certificate_url as _get_certificate_url,
has_html_certificates_enabled as _has_html_certificates_enabled
has_html_certificates_enabled as _has_html_certificates_enabled,
should_certificate_be_visible as _should_certificate_be_visible,
certificate_status as _certificate_status,
certificate_status_for_student as _certificate_status_for_student,
)
from openedx.core.djangoapps.content.course_overviews.api import get_course_overview_or_none
@@ -228,7 +231,7 @@ def certificate_downloadable_status(student, course_key):
Returns:
Dict containing student passed status also download url, uuid for cert if available
"""
current_status = certificate_status_for_student(student, course_key)
current_status = _certificate_status_for_student(student, course_key)
# If the certificate status is an error user should view that status is "generating".
# On the back-end, need to monitor those errors and re-submit the task.
@@ -251,7 +254,13 @@ def certificate_downloadable_status(student, course_key):
response_data['earned_but_not_available'] = True
response_data['certificate_available_date'] = course_overview.certificate_available_date
may_view_certificate = course_overview.may_certify()
may_view_certificate = _should_certificate_be_visible(
course_overview.certificates_display_behavior,
course_overview.certificates_show_before_end,
course_overview.has_ended(),
course_overview.certificate_available_date,
course_overview.self_paced
)
if current_status['status'] == CertificateStatuses.downloadable and may_view_certificate:
response_data['is_downloadable'] = True
response_data['download_url'] = current_status['download_url'] or get_certificate_url(
@@ -745,3 +754,69 @@ def get_enrolled_allowlisted_not_passing_users(course_key):
generatedcertificate__course_id=course_key,
generatedcertificate__status__in=CertificateStatuses.PASSED_STATUSES
)
def should_certificate_be_visible(
certificates_display_behavior,
certificates_show_before_end,
has_ended,
certificate_available_date,
self_paced
):
"""
Returns whether it is acceptable to show the student a certificate download
link for a course, based on provided attributes of the course.
Arguments:
certificates_display_behavior (str): string describing the course's
certificate display behavior.
See CourseFields.certificates_display_behavior.help for more detail.
certificates_show_before_end (bool): whether user can download the
course's certificates before the course has ended.
has_ended (bool): Whether the course has ended.
certificate_available_date (datetime): the date the certificate is available on for the course.
self_paced (bool): Whether the course is self-paced.
"""
return _should_certificate_be_visible(
certificates_display_behavior,
certificates_show_before_end,
has_ended,
certificate_available_date,
self_paced
)
def certificate_info_for_user(user, course_id, grade, user_is_allowlisted, user_certificate):
"""
Returns the certificate info for a user for grade report.
"""
certificate_is_delivered = 'N'
certificate_type = 'N/A'
status = _certificate_status(user_certificate)
certificate_generated = status['status'] == CertificateStatuses.downloadable
course_overview = get_course_overview_or_none(course_id)
if not course_overview:
return None
can_have_certificate = _should_certificate_be_visible(
course_overview.certificates_display_behavior,
course_overview.certificates_show_before_end,
course_overview.has_ended(),
course_overview.certificate_available_date,
course_overview.self_paced
)
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
eligible_for_certificate = 'Y' if (user_is_allowlisted or user_is_verified or certificate_generated) \
else 'N'
if certificate_generated and can_have_certificate:
certificate_is_delivered = 'Y'
certificate_type = status['mode']
return [eligible_for_certificate, certificate_is_delivered, certificate_type]
def certificate_status_for_student(student, course_id):
"""This returns a dictionary with a key for status, and other information."""
return _certificate_status_for_student(student, course_id)

View File

@@ -24,13 +24,11 @@ from model_utils.models import TimeStampedModel
from opaque_keys.edx.django.models import CourseKeyField
from simple_history.models import HistoricalRecords
from common.djangoapps.course_modes.models import CourseMode
from common.djangoapps.util.milestones_helpers import fulfill_course_milestone, is_prerequisite_courses_enabled
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.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
@@ -596,87 +594,6 @@ def handle_course_cert_awarded(sender, user, course_key, **kwargs): # pylint: d
fulfill_course_milestone(course_key, user)
def certificate_status_for_student(student, course_id):
"""
This returns a dictionary with a key for status, and other information.
See certificate_status for more information.
"""
try:
generated_certificate = GeneratedCertificate.objects.get(user=student, course_id=course_id)
except GeneratedCertificate.DoesNotExist:
generated_certificate = None
return certificate_status(generated_certificate)
def certificate_status(generated_certificate):
"""
This returns a dictionary with a key for status, and other information.
If the status is "downloadable", the dictionary also contains
"download_url".
If the student has been graded, the dictionary also contains their
grade for the course with the key "grade".
"""
# Import here instead of top of file since this module gets imported before
# the course_modes app is loaded, resulting in a Django deprecation warning.
from common.djangoapps.course_modes.models import CourseMode # pylint: disable=redefined-outer-name, reimported
if generated_certificate:
cert_status = {
'status': generated_certificate.status,
'mode': generated_certificate.mode,
'uuid': generated_certificate.verify_uuid,
}
if generated_certificate.grade:
cert_status['grade'] = generated_certificate.grade
if generated_certificate.mode == 'audit':
course_mode_slugs = [mode.slug for mode in CourseMode.modes_for_course(generated_certificate.course_id)]
# Short term fix to make sure old audit users with certs still see their certs
# only do this if there if no honor mode
if 'honor' not in course_mode_slugs:
cert_status['status'] = CertificateStatuses.auditing
return cert_status
if generated_certificate.status == CertificateStatuses.downloadable:
cert_status['download_url'] = generated_certificate.download_url
return cert_status
else:
return {'status': CertificateStatuses.unavailable, 'mode': GeneratedCertificate.MODES.honor, 'uuid': None}
def certificate_info_for_user(user, course_id, grade, user_is_allowlisted, user_certificate):
"""
Returns the certificate info for a user for grade report.
"""
from common.djangoapps.student.models import CourseEnrollment
certificate_is_delivered = 'N'
certificate_type = 'N/A'
status = certificate_status(user_certificate)
certificate_generated = status['status'] == CertificateStatuses.downloadable
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
eligible_for_certificate = 'Y' if (user_is_allowlisted or user_is_verified or certificate_generated) \
else 'N'
if certificate_generated and can_have_certificate:
certificate_is_delivered = 'Y'
certificate_type = status['mode']
return [eligible_for_certificate, certificate_is_delivered, certificate_type]
class ExampleCertificateSet(TimeStampedModel):
"""
A set of example certificates.

View File

@@ -23,8 +23,8 @@ from lms.djangoapps.certificates.models import (
CertificateAllowlist,
ExampleCertificate,
GeneratedCertificate,
certificate_status_for_student
)
from lms.djangoapps.certificates.utils import certificate_status_for_student
from lms.djangoapps.grades.api import CourseGradeFactory
from lms.djangoapps.verify_student.services import IDVerificationService
from openedx.core.djangoapps.content.course_overviews.api import get_course_overview_or_none

View File

@@ -52,14 +52,14 @@ from lms.djangoapps.certificates.api import (
is_certificate_invalidated,
is_on_allowlist,
remove_allowlist_entry,
set_cert_generation_enabled
set_cert_generation_enabled,
certificate_status_for_student,
)
from lms.djangoapps.certificates.models import (
CertificateGenerationConfiguration,
CertificateStatuses,
ExampleCertificate,
GeneratedCertificate,
certificate_status_for_student
)
from lms.djangoapps.certificates.queue import XQueueAddToQueueError, XQueueCertInterface
from lms.djangoapps.certificates.tests.factories import (

View File

@@ -1,14 +1,23 @@
"""
Tests for Certificates app utility functions
"""
from datetime import datetime, timedelta
from unittest.mock import patch
import ddt
from django.test import TestCase
from pytz import utc
from lms.djangoapps.certificates.utils import has_html_certificates_enabled
from lms.djangoapps.certificates.utils import has_html_certificates_enabled, should_certificate_be_visible
from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory
_TODAY = datetime.now(utc)
_LAST_MONTH = _TODAY - timedelta(days=30)
_LAST_WEEK = _TODAY - timedelta(days=7)
_NEXT_WEEK = _TODAY + timedelta(days=7)
@ddt.ddt
class CertificateUtilityTests(TestCase):
"""
Tests for course certificate utility functions
@@ -43,3 +52,34 @@ class CertificateUtilityTests(TestCase):
self.course_overview.save()
assert not has_html_certificates_enabled(self.course_overview)
@ddt.data(
('early_with_info', True, True, _LAST_MONTH, False, True),
('early_no_info', False, False, _LAST_MONTH, False, True),
('end', True, False, _LAST_MONTH, False, True),
('end', False, True, _LAST_MONTH, False, True),
('end', False, False, _NEXT_WEEK, False, False),
('end', False, False, _LAST_WEEK, False, True),
('end', False, False, None, False, False),
('early_with_info', False, False, None, False, True),
('end', False, False, _NEXT_WEEK, False, False),
('end', False, False, _NEXT_WEEK, True, True),
)
@ddt.unpack
def test_should_certificate_be_visible(
self,
certificates_display_behavior,
certificates_show_before_end,
has_ended,
certificate_available_date,
self_paced,
expected_value
):
"""Test whether the certificate should be visible to user given multiple usecases"""
assert should_certificate_be_visible(
certificates_display_behavior,
certificates_show_before_end,
has_ended,
certificate_available_date,
self_paced
) == expected_value

View File

@@ -15,11 +15,10 @@ from common.djangoapps.student.models import CourseEnrollment
from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory
from common.djangoapps.util.milestones_helpers import milestones_achieved_by_user, set_prerequisite_courses
from lms.djangoapps.badges.tests.factories import CourseCompleteImageConfigurationFactory
from lms.djangoapps.certificates.api import certificate_info_for_user, certificate_status_for_student
from lms.djangoapps.certificates.models import (
CertificateStatuses,
GeneratedCertificate,
certificate_info_for_user,
certificate_status_for_student
)
from lms.djangoapps.certificates.tests.factories import GeneratedCertificateFactory
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase

View File

@@ -1,14 +1,16 @@
"""
Certificates utilities
"""
from datetime import datetime
import logging
from django.conf import settings
from django.urls import reverse
from eventtracking import tracker
from opaque_keys.edx.keys import CourseKey
from pytz import utc
from lms.djangoapps.certificates.data import CertificateStatuses
from lms.djangoapps.certificates.models import GeneratedCertificate
from openedx.core.djangoapps.content.course_overviews.api import get_course_overview_or_none
@@ -126,3 +128,87 @@ def _safe_course_key(course_key):
if not isinstance(course_key, CourseKey):
return CourseKey.from_string(course_key)
return course_key
def should_certificate_be_visible(
certificates_display_behavior,
certificates_show_before_end,
has_ended,
certificate_available_date,
self_paced
):
"""
Returns whether it is acceptable to show the student a certificate download
link for a course, based on provided attributes of the course.
Arguments:
certificates_display_behavior (str): string describing the course's
certificate display behavior.
See CourseFields.certificates_display_behavior.help for more detail.
certificates_show_before_end (bool): whether user can download the
course's certificates before the course has ended.
has_ended (bool): Whether the course has ended.
certificate_available_date (datetime): the date the certificate is available on for the course.
self_paced (bool): Whether the course is self-paced.
"""
show_early = (
certificates_display_behavior in ('early_with_info', 'early_no_info')
or certificates_show_before_end
)
past_available_date = (
certificate_available_date
and certificate_available_date < datetime.now(utc)
)
ended_without_available_date = (certificate_available_date is None) and has_ended
return any((self_paced, show_early, past_available_date, ended_without_available_date))
def certificate_status(generated_certificate):
"""
This returns a dictionary with a key for status, and other information.
If the status is "downloadable", the dictionary also contains
"download_url".
If the student has been graded, the dictionary also contains their
grade for the course with the key "grade".
"""
# Import here instead of top of file since this module gets imported before
# the course_modes app is loaded, resulting in a Django deprecation warning.
from common.djangoapps.course_modes.models import CourseMode # pylint: disable=redefined-outer-name, reimported
if generated_certificate:
cert_status = {
'status': generated_certificate.status,
'mode': generated_certificate.mode,
'uuid': generated_certificate.verify_uuid,
}
if generated_certificate.grade:
cert_status['grade'] = generated_certificate.grade
if generated_certificate.mode == 'audit':
course_mode_slugs = [mode.slug for mode in CourseMode.modes_for_course(generated_certificate.course_id)]
# Short term fix to make sure old audit users with certs still see their certs
# only do this if there if no honor mode
if 'honor' not in course_mode_slugs:
cert_status['status'] = CertificateStatuses.auditing
return cert_status
if generated_certificate.status == CertificateStatuses.downloadable:
cert_status['download_url'] = generated_certificate.download_url
return cert_status
else:
return {'status': CertificateStatuses.unavailable, 'mode': GeneratedCertificate.MODES.honor, 'uuid': None}
def certificate_status_for_student(student, course_id):
"""
This returns a dictionary with a key for status, and other information.
See certificate_status for more information.
"""
try:
generated_certificate = GeneratedCertificate.objects.get(user=student, course_id=course_id)
except GeneratedCertificate.DoesNotExist:
generated_certificate = None
return certificate_status(generated_certificate)

View File

@@ -19,8 +19,8 @@ from lms.djangoapps.certificates.api import generate_certificate_task
from lms.djangoapps.certificates.models import (
ExampleCertificate,
GeneratedCertificate,
certificate_status_for_student
)
from lms.djangoapps.certificates.utils import certificate_status_for_student
log = logging.getLogger(__name__)
User = get_user_model()

View File

@@ -20,7 +20,7 @@ from common.djangoapps.course_modes.models import CourseMode
from common.djangoapps.student.models import CourseEnrollment
from common.djangoapps.student.roles import BulkRoleCache
from lms.djangoapps.certificates import api as certs_api
from lms.djangoapps.certificates.models import GeneratedCertificate, certificate_info_for_user
from lms.djangoapps.certificates.models import GeneratedCertificate
from lms.djangoapps.course_blocks.api import get_course_blocks
from lms.djangoapps.courseware.user_state_client import DjangoXBlockUserStateClient
from lms.djangoapps.grades.api import CourseGradeFactory
@@ -677,7 +677,7 @@ class CourseGradeReport:
Returns the course certification information for the given user.
"""
is_allowlisted = user.id in bulk_certs.allowlisted_user_ids
certificate_info = certificate_info_for_user(
certificate_info = certs_api.certificate_info_for_user(
user,
context.course_id,
course_grade.letter_grade,

View File

@@ -585,19 +585,6 @@ class CourseOverview(TimeStampedModel):
else:
return None
def may_certify(self):
"""
Returns whether it is acceptable to show the student a certificate
download link.
"""
return course_metadata_utils.may_certify_for_course(
self.certificates_display_behavior,
self.certificates_show_before_end,
self.has_ended(),
self.certificate_available_date,
self.self_paced
)
@property
def pre_requisite_courses(self):
"""

View File

@@ -141,7 +141,6 @@ class CourseOverviewTestCase(CatalogIntegrationMixin, ModuleStoreTestCase, Cache
('clean_id', ('#',)),
('has_ended', ()),
('has_started', ()),
('may_certify', ()),
]
for method_name, method_args in methods_to_test:
course_value = getattr(course, method_name)(*method_args)

View File

@@ -444,9 +444,18 @@ class ProgramProgressMeter:
}
try:
may_certify = CourseOverview.get_from_id(course_key).may_certify()
course_overview = CourseOverview.get_from_id(course_key)
except CourseOverview.DoesNotExist:
may_certify = True
else:
may_certify = certificate_api.should_certificate_be_visible(
course_overview.certificates_display_behavior,
course_overview.certificates_show_before_end,
course_overview.has_ended(),
course_overview.certificate_available_date,
course_overview.self_paced
)
if (
CertificateStatuses.is_passing_status(certificate['status'])
and may_certify
@@ -583,7 +592,13 @@ class ProgramDataExtender:
run_mode['upgrade_url'] = None
def _attach_course_run_may_certify(self, run_mode):
run_mode['may_certify'] = self.course_overview.may_certify()
run_mode['may_certify'] = certificate_api.should_certificate_be_visible(
self.course_overview.certificates_display_behavior,
self.course_overview.certificates_show_before_end,
self.course_overview.has_ended(),
self.course_overview.certificate_available_date,
self.course_overview.self_paced
)
def _attach_course_run_is_mobile_only(self, run_mode):
run_mode['is_mobile_only'] = self.mobile_only