Revert "feat: Reimagine certificate_availability_date and certificates_display_behavior"

This commit is contained in:
Matt Tuchfarber
2021-07-07 16:53:05 -04:00
committed by GitHub
parent b4df37d48e
commit 63cb6a97ff
34 changed files with 572 additions and 933 deletions

View File

@@ -11,7 +11,6 @@ from lms.djangoapps.certificates import api as certs_api
from lms.djangoapps.certificates.data import CertificateStatuses
from openedx.core.djangoapps.certificates.config import waffle
from common.djangoapps.student.models import CourseEnrollment
from xmodule.data import CertificatesDisplayBehaviors
log = logging.getLogger(__name__)
@@ -57,10 +56,7 @@ def can_show_certificate_available_date_field(course):
def _course_uses_available_date(course):
return (
can_show_certificate_available_date_field(course)
and course.certificates_display_behavior == CertificatesDisplayBehaviors.END_WITH_DATE
)
return can_show_certificate_available_date_field(course) and course.certificate_available_date
def available_date_for_certificate(course, certificate, certificate_available_date=None):

View File

@@ -11,11 +11,10 @@ from edx_toggles.toggles import LegacyWaffleSwitch
from edx_toggles.toggles.testutils import override_waffle_switch
from common.djangoapps.course_modes.models import CourseMode
from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory
from openedx.core.djangoapps.certificates import api
from openedx.core.djangoapps.certificates.config import waffle as certs_waffle
from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory
from xmodule.data import CertificatesDisplayBehaviors
from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory
# TODO: Copied from lms.djangoapps.certificates.models,
@@ -168,7 +167,6 @@ class CertificatesApiTestCase(TestCase):
# With an available date set in the past, both return the available date (if configured)
self.course.certificate_available_date = datetime(2017, 2, 1, tzinfo=pytz.UTC)
self.course.certificates_display_behavior = CertificatesDisplayBehaviors.END_WITH_DATE
maybe_avail = self.course.certificate_available_date if uses_avail_date else self.certificate.modified_date
assert maybe_avail == api.available_date_for_certificate(self.course, self.certificate)
assert maybe_avail == api.display_date_for_certificate(self.course, self.certificate)

View File

@@ -1,24 +0,0 @@
# Generated by Django 2.2.24 on 2021-06-15 17:06
from django.db import migrations, models
import xmodule.data
class Migration(migrations.Migration):
dependencies = [
('course_overviews', '0025_auto_20210702_1602'),
]
operations = [
migrations.AlterField(
model_name='courseoverview',
name='certificates_display_behavior',
field=models.TextField(choices=[('end', 'END'), ('end_with_date', 'END_WITH_DATE'), ('early_no_info', 'EARLY_NO_INFO')], default=xmodule.data.CertificatesDisplayBehaviors('end'), null=True),
),
migrations.AlterField(
model_name='historicalcourseoverview',
name='certificates_display_behavior',
field=models.TextField(choices=[('end', 'END'), ('end_with_date', 'END_WITH_DATE'), ('early_no_info', 'EARLY_NO_INFO')], default=xmodule.data.CertificatesDisplayBehaviors('end'), null=True),
),
]

View File

@@ -32,11 +32,11 @@ from openedx.core.lib.cache_utils import request_cached, RequestCache
from common.djangoapps.static_replace.models import AssetBaseUrlConfig
from xmodule import block_metadata_utils, course_metadata_utils
from xmodule.course_module import DEFAULT_START_DATE, CourseBlock
from xmodule.data import CertificatesDisplayBehaviors
from xmodule.error_module import ErrorBlock
from xmodule.modulestore.django import modulestore
from xmodule.tabs import CourseTab
log = logging.getLogger(__name__)
@@ -56,10 +56,6 @@ class CourseOverview(TimeStampedModel):
course catalog (courses to enroll in)
course about (meta data about the course)
When you bump the VERSION you will invalidate all existing course overviews. This
will cause a slew of modulestore reads as each course needs to be re-cached into
the course overview.
.. no_pii:
"""
@@ -67,7 +63,7 @@ class CourseOverview(TimeStampedModel):
app_label = 'course_overviews'
# IMPORTANT: Bump this whenever you modify this model and/or add a migration.
VERSION = 14
VERSION = 13
# Cache entry versioning.
version = IntegerField()
@@ -100,11 +96,7 @@ class CourseOverview(TimeStampedModel):
end_of_course_survey_url = TextField(null=True)
# Certification data
certificates_display_behavior = TextField(
null=True,
choices=[(choice.value, choice.name) for choice in CertificatesDisplayBehaviors],
default=CertificatesDisplayBehaviors.END
)
certificates_display_behavior = TextField(null=True)
certificates_show_before_end = BooleanField(default=False)
cert_html_view_enabled = BooleanField(default=False)
has_any_active_web_certificate = BooleanField(default=False)
@@ -219,15 +211,13 @@ class CourseOverview(TimeStampedModel):
course_overview.course_image_url = course_image_url(course)
course_overview.social_sharing_url = course.social_sharing_url
updated_display_behavior, updated_available_date = cls.validate_certificate_settings(course)
course_overview.certificates_display_behavior = updated_display_behavior
course_overview.certificate_available_date = updated_available_date
course_overview.certificates_display_behavior = course.certificates_display_behavior
course_overview.certificates_show_before_end = course.certificates_show_before_end
course_overview.cert_html_view_enabled = course.cert_html_view_enabled
course_overview.has_any_active_web_certificate = (get_active_web_certificate(course) is not None)
course_overview.cert_name_short = course.cert_name_short
course_overview.cert_name_long = course.cert_name_long
course_overview.certificate_available_date = course.certificate_available_date
course_overview.lowest_passing_grade = lowest_passing_grade
course_overview.end_of_course_survey_url = course.end_of_course_survey_url
@@ -906,35 +896,6 @@ class CourseOverview(TimeStampedModel):
"""
return self._original_course.edxnotes_visibility
@staticmethod
def validate_certificate_settings(course):
"""
Take a course and returns validated certificate display settings
Arguments:
course (CourseBlock): any course descriptor object
Returns:
tuple[str, str]: updated certificates_display_behavior, updated certificate_available_date
None
"""
# Backwards compatibility for existing courses that set availability date, didn't set behavior,
# and expect availability date to be used
certificates_display_behavior = course.certificates_display_behavior
certificate_available_date = course.certificate_available_date
if certificates_display_behavior == "" and certificate_available_date:
certificates_display_behavior = CertificatesDisplayBehaviors.END_WITH_DATE
if not CertificatesDisplayBehaviors.includes_value(certificates_display_behavior):
certificates_display_behavior = CertificatesDisplayBehaviors.END
# Null the date if it's not going to be used
if certificates_display_behavior != CertificatesDisplayBehaviors.END_WITH_DATE:
certificate_available_date = None
return (certificates_display_behavior, certificate_available_date)
def __str__(self):
"""Represent ourselves with the course key."""
return str(self.id)

View File

@@ -2,21 +2,16 @@
import datetime
from unittest.mock import patch
from collections import namedtuple
import pytest
import ddt
from xmodule.data import CertificatesDisplayBehaviors
from xmodule.modulestore import ModuleStoreEnum
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory, check_mongo_calls
from ..models import CourseOverview
# represents a change of a course overview field. Used to avoid confusing indicies
Change = namedtuple("Change", ["field_name", "initial_value", "changed_value"])
@ddt.ddt
class CourseOverviewSignalsTestCase(ModuleStoreTestCase):
@@ -76,12 +71,8 @@ class CourseOverviewSignalsTestCase(ModuleStoreTestCase):
self.store.delete_course(course.id, ModuleStoreEnum.UserID.test)
CourseOverview.get_from_id(course.id)
def assert_changed_signal_sent(self, changes, mock_signal): # lint-amnesty, pylint: disable=missing-function-docstring
course = CourseFactory.create(
emit_signals=True,
**{change.field_name: change.initial_value for change in changes}
)
def assert_changed_signal_sent(self, field_name, initial_value, changed_value, mock_signal): # lint-amnesty, pylint: disable=missing-function-docstring
course = CourseFactory.create(emit_signals=True, **{field_name: initial_value})
# changing display name doesn't fire the signal
course.display_name = course.display_name + 'changed'
@@ -89,27 +80,18 @@ class CourseOverviewSignalsTestCase(ModuleStoreTestCase):
assert not mock_signal.called
# changing the given field fires the signal
for change in changes:
setattr(course, change.field_name, change.changed_value)
setattr(course, field_name, changed_value)
self.store.update_item(course, ModuleStoreEnum.UserID.test)
assert mock_signal.called
@patch('openedx.core.djangoapps.content.course_overviews.signals.COURSE_START_DATE_CHANGED.send')
def test_start_changed(self, mock_signal):
self.assert_changed_signal_sent([Change('start', self.TODAY, self.NEXT_WEEK)], mock_signal)
self.assert_changed_signal_sent('start', self.TODAY, self.NEXT_WEEK, mock_signal)
@patch('openedx.core.djangoapps.content.course_overviews.signals.COURSE_PACING_CHANGED.send')
def test_pacing_changed(self, mock_signal):
self.assert_changed_signal_sent([Change('self_paced', True, False)], mock_signal)
self.assert_changed_signal_sent('self_paced', True, False, mock_signal)
@patch('openedx.core.djangoapps.content.course_overviews.signals.COURSE_CERT_DATE_CHANGE.send_robust')
def test_cert_date_changed(self, mock_signal):
changes = [
Change("certificate_available_date", self.TODAY, self.NEXT_WEEK),
Change(
"certificates_display_behavior",
CertificatesDisplayBehaviors.END,
CertificatesDisplayBehaviors.END_WITH_DATE
)
]
self.assert_changed_signal_sent(changes, mock_signal)
self.assert_changed_signal_sent('certificate_available_date', self.TODAY, self.NEXT_WEEK, mock_signal)

View File

@@ -2,7 +2,7 @@
Tests for courseware API
"""
import unittest
from datetime import datetime, timedelta
from datetime import datetime
from urllib.parse import urlencode
from typing import Optional
@@ -39,7 +39,6 @@ from common.djangoapps.student.roles import CourseInstructorRole
from common.djangoapps.student.tests.factories import CourseEnrollmentCelebrationFactory, UserFactory
from openedx.core.djangoapps.agreements.api import create_integrity_signature
from openedx.core.djangoapps.agreements.toggles import ENABLE_INTEGRITY_SIGNATURE
from xmodule.data import CertificatesDisplayBehaviors
from xmodule.modulestore.django import modulestore
from xmodule.modulestore.tests.django_utils import TEST_DATA_SPLIT_MODULESTORE, SharedModuleStoreTestCase
from xmodule.modulestore.tests.factories import ItemFactory, ToyCourseFactory
@@ -47,8 +46,6 @@ from xmodule.modulestore.tests.factories import ItemFactory, ToyCourseFactory
User = get_user_model()
_NEXT_WEEK = datetime.now() + timedelta(days=7)
@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms')
class BaseCoursewareTests(SharedModuleStoreTestCase):
@@ -67,8 +64,6 @@ class BaseCoursewareTests(SharedModuleStoreTestCase):
enrollment_end=datetime(2028, 1, 1, 1, 1, 1),
emit_signals=True,
modulestore=cls.store,
certificate_available_date=_NEXT_WEEK,
certificates_display_behavior=CertificatesDisplayBehaviors.END_WITH_DATE
)
cls.chapter = ItemFactory(parent=cls.course, category='chapter')
cls.sequence = ItemFactory(parent=cls.chapter, category='sequential', display_name='sequence')
@@ -152,7 +147,6 @@ class CourseApiTestViews(BaseCoursewareTests, MasqueradeMixin):
CourseEnrollment.enroll(self.user, self.course.id, enrollment_mode)
response = self.client.get(self.url)
assert response.status_code == 200
if enrollment_mode:
enrollment = response.data['enrollment']

View File

@@ -48,8 +48,6 @@ class CourseDetails:
self.end_date = None # 'end'
self.enrollment_start = None
self.enrollment_end = None
self.certificate_available_date = None
self.certificates_display_behavior = None
self.syllabus = None # a pdf file asset
self.title = ""
self.subtitle = ""
@@ -110,8 +108,7 @@ class CourseDetails:
course_details = cls(course_key.org, course_key.course, course_key.run)
course_details.start_date = course_descriptor.start
course_details.end_date = course_descriptor.end
course_details.certificate_available_date = course_descriptor.certificate_available_date
course_details.certificates_display_behavior = course_descriptor.certificates_display_behavior
course_details.certificate_available_date = course_descriptor.certificate_available_date # lint-amnesty, pylint: disable=attribute-defined-outside-init
course_details.enrollment_start = course_descriptor.enrollment_start
course_details.enrollment_end = course_descriptor.enrollment_end
course_details.pre_requisite_courses = course_descriptor.pre_requisite_courses
@@ -247,13 +244,6 @@ class CourseDetails:
dirty = True
descriptor.certificate_available_date = converted
if (
'certificates_display_behavior' in jsondict
and jsondict['certificates_display_behavior'] != descriptor.certificates_display_behavior
):
descriptor.certificates_display_behavior = jsondict['certificates_display_behavior']
dirty = True
if 'course_image_name' in jsondict and jsondict['course_image_name'] != descriptor.course_image:
descriptor.course_image = jsondict['course_image_name']
dirty = True

View File

@@ -30,7 +30,6 @@ from openedx.core.djangoapps.oauth_dispatch.tests.factories import ApplicationFa
from openedx.core.djangoapps.programs import tasks
from openedx.core.djangoapps.site_configuration.tests.factories import SiteConfigurationFactory, SiteFactory
from openedx.core.djangolib.testing.utils import skip_unless_lms
from xmodule.data import CertificatesDisplayBehaviors
log = logging.getLogger(__name__)
@@ -521,7 +520,6 @@ class AwardCourseCertificatesTestCase(CredentialsApiConfigMixin, TestCase):
self.course = CourseOverviewFactory.create(
self_paced=True, # Any option to allow the certificate to be viewable for the course
certificate_available_date=self.available_date,
certificates_display_behavior=CertificatesDisplayBehaviors.END_WITH_DATE
)
self.student = UserFactory.create(username='test-student')
# Instantiate the Certificate first so that the config doesn't execute issuance

View File

@@ -51,7 +51,6 @@ from openedx.core.djangoapps.site_configuration.tests.factories import SiteFacto
from openedx.core.djangolib.testing.utils import skip_unless_lms
from common.djangoapps.student.tests.factories import AnonymousUserFactory, CourseEnrollmentFactory, UserFactory
from common.djangoapps.util.date_utils import strftime_localized
from xmodule.data import CertificatesDisplayBehaviors
from xmodule.modulestore.django import modulestore
from xmodule.modulestore.tests.django_utils import (
ModuleStoreTestCase, SharedModuleStoreTestCase, TEST_DATA_SPLIT_MODULESTORE
@@ -500,7 +499,7 @@ class TestProgramProgressMeter(ModuleStoreTestCase):
end=two_days_ago,
self_paced=False,
certificate_available_date=tomorrow,
certificates_display_behavior=CertificatesDisplayBehaviors.END_WITH_DATE
certificates_display_behavior='end'
)
third_course_run_key = str(course3.id)
@@ -608,7 +607,6 @@ class TestProgramProgressMeter(ModuleStoreTestCase):
# 3 certs, all available, program cert in the past/now
course3_overview = CourseOverview.get_from_id(course3.id)
course3_overview.certificate_available_date = yesterday
course3_overview.certificates_display_behavior = CertificatesDisplayBehaviors.END_WITH_DATE
course3_overview.save()
meter = ProgramProgressMeter(self.site, self.user)
self._assert_progress(
@@ -624,7 +622,7 @@ class TestProgramProgressMeter(ModuleStoreTestCase):
def test_old_course_runs(self, mock_get_programs):
"""
Test that old course runs may exist for a program which do not exist in LMS.
In that case, continue considering the course run to have been failed by the learner
In that case, continue considering the course run to've been failed by the learner
"""
course_run = CourseRunFactory.create()
course = CourseFactory.create(course_runs=[course_run])

View File

@@ -21,7 +21,6 @@ from openedx.core.djangoapps.content.course_overviews.models import CourseOvervi
from openedx.core.djangoapps.site_configuration.tests.mixins import SiteMixin
from openedx.features.learner_profile.toggles import REDIRECT_TO_PROFILE_MICROFRONTEND
from openedx.features.learner_profile.views.learner_profile import learner_profile_context
from xmodule.data import CertificatesDisplayBehaviors
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory
@@ -223,8 +222,7 @@ class LearnerProfileViewTest(SiteMixin, UrlResetMixin, ModuleStoreTestCase):
"""
# add new course with certificate_available_date is future date.
course = CourseFactory.create(
certificate_available_date=datetime.datetime.now() + datetime.timedelta(days=5),
certificates_display_behavior=CertificatesDisplayBehaviors.END_WITH_DATE
certificate_available_date=datetime.datetime.now() + datetime.timedelta(days=5)
)
cert = self._create_certificate(course_key=course.id)