From 69a875a0de2b08e7de9ad8b78bd67bb41ee31de7 Mon Sep 17 00:00:00 2001 From: Uman Shahzad Date: Thu, 21 Sep 2017 14:06:19 +0500 Subject: [PATCH 1/8] Bump edx-enterprise to 0.46.7. --- requirements/edx/base.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index f352b7979b..908192a270 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -49,7 +49,7 @@ edx-lint==0.4.3 astroid==1.3.8 edx-django-oauth2-provider==1.2.5 edx-django-sites-extensions==2.3.0 -edx-enterprise==0.46.4 +edx-enterprise==0.46.7 edx-oauth2-provider==1.2.2 edx-opaque-keys==0.4.0 edx-organizations==0.4.6 From d0e68f2d2461e84bb6f91fe1b47f213c97845ae4 Mon Sep 17 00:00:00 2001 From: Sanford Student Date: Mon, 18 Sep 2017 14:28:05 -0400 Subject: [PATCH 2/8] recalculate course grade on user partition change --- common/djangoapps/student/models.py | 4 ++ common/djangoapps/student/signals/__init__.py | 0 common/djangoapps/student/signals/signals.py | 6 +++ lms/djangoapps/grades/signals/handlers.py | 24 ++++++++++-- lms/djangoapps/grades/tests/test_signals.py | 38 ++++++++++++++++++- .../core/djangoapps/course_groups/cohorts.py | 5 ++- .../course_groups/signals/__init__.py | 0 .../course_groups/signals/signals.py | 6 +++ .../course_groups/tests/test_cohorts.py | 12 +++++- .../core/djangoapps/credit/tests/test_api.py | 8 ++-- 10 files changed, 91 insertions(+), 12 deletions(-) create mode 100644 common/djangoapps/student/signals/__init__.py create mode 100644 common/djangoapps/student/signals/signals.py create mode 100644 openedx/core/djangoapps/course_groups/signals/__init__.py create mode 100644 openedx/core/djangoapps/course_groups/signals/signals.py diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index aae2a3cc8c..5685044722 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -52,6 +52,7 @@ from certificates.models import GeneratedCertificate from course_modes.models import CourseMode from courseware.models import DynamicUpgradeDeadlineConfiguration, CourseDynamicUpgradeDeadlineConfiguration from enrollment.api import _default_course_mode + from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.djangoapps.schedules.models import ScheduleConfig from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers @@ -62,6 +63,8 @@ from util.milestones_helpers import is_entrance_exams_enabled from util.model_utils import emit_field_changed_events, get_changed_fields_dict from util.query import use_read_replica_if_available +from .signals.signals import ENROLLMENT_TRACK_UPDATED + UNENROLL_DONE = Signal(providing_args=["course_enrollment", "skip_refund"]) ENROLL_STATUS_CHANGE = Signal(providing_args=["event", "user", "course_id", "mode", "cost", "currency"]) REFUND_ORDER = Signal(providing_args=["course_enrollment"]) @@ -1191,6 +1194,7 @@ class CourseEnrollment(models.Model): # Only emit mode change events when the user's enrollment # mode has changed from its previous setting self.emit_event(EVENT_NAME_ENROLLMENT_MODE_CHANGED) + ENROLLMENT_TRACK_UPDATED.send(sender=None, user=self.user, course_key=self.course_id) def send_signal(self, event, cost=None, currency=None): """ diff --git a/common/djangoapps/student/signals/__init__.py b/common/djangoapps/student/signals/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/common/djangoapps/student/signals/signals.py b/common/djangoapps/student/signals/signals.py new file mode 100644 index 0000000000..7f280b98b0 --- /dev/null +++ b/common/djangoapps/student/signals/signals.py @@ -0,0 +1,6 @@ +""" +Enrollment track related signals. +""" +from django.dispatch import Signal + +ENROLLMENT_TRACK_UPDATED = Signal(providing_args=['user', 'course_key']) diff --git a/lms/djangoapps/grades/signals/handlers.py b/lms/djangoapps/grades/signals/handlers.py index 07defdf246..7ffefa44e2 100644 --- a/lms/djangoapps/grades/signals/handlers.py +++ b/lms/djangoapps/grades/signals/handlers.py @@ -11,8 +11,10 @@ from xblock.scorable import ScorableXBlockMixin, Score from courseware.model_data import get_score, set_score from eventtracking import tracker from lms.djangoapps.instructor_task.tasks_helper.module_state import GRADES_OVERRIDE_EVENT_TYPE +from openedx.core.djangoapps.course_groups.signals.signals import COHORT_MEMBERSHIP_UPDATED from openedx.core.lib.grade_utils import is_score_higher_or_equal from student.models import user_by_anonymous_id +from student.signals.signals import ENROLLMENT_TRACK_UPDATED from submissions.models import score_reset, score_set from track.event_transaction_utils import ( create_new_event_transaction_id, @@ -22,6 +24,7 @@ from track.event_transaction_utils import ( ) from util.date_utils import to_timestamp +from ..config.waffle import waffle, WRITE_ONLY_IF_ENGAGED from ..constants import ScoreDatabaseTableEnum from ..new.course_grade_factory import CourseGradeFactory from ..scores import weighted_score @@ -31,7 +34,7 @@ from .signals import ( PROBLEM_WEIGHTED_SCORE_CHANGED, SCORE_PUBLISHED, SUBSECTION_SCORE_CHANGED, - SUBSECTION_OVERRIDE_CHANGED + SUBSECTION_OVERRIDE_CHANGED, ) log = getLogger(__name__) @@ -237,13 +240,28 @@ def enqueue_subsection_update(sender, **kwargs): # pylint: disable=unused-argum @receiver(SUBSECTION_SCORE_CHANGED) -def recalculate_course_grade(sender, course, course_structure, user, **kwargs): # pylint: disable=unused-argument +def recalculate_course_grade_only(sender, course, course_structure, user, **kwargs): # pylint: disable=unused-argument """ - Updates a saved course grade. + Updates a saved course grade, but does not update the subsection + grades the user has in this course. """ CourseGradeFactory().update(user, course=course, course_structure=course_structure) +@receiver(ENROLLMENT_TRACK_UPDATED) +@receiver(COHORT_MEMBERSHIP_UPDATED) +def force_recalculate_course_and_subsection_grades(sender, user, course_key, **kwargs): + """ + Updates a saved course grade, forcing the subsection grades + from which it is calculated to update along the way. + + Does not create a grade if the user has never attempted a problem, + even if the WRITE_ONLY_IF_ENGAGED waffle switch is off. + """ + if waffle().is_enabled(WRITE_ONLY_IF_ENGAGED) or CourseGradeFactory().read(user, course_key=course_key): + CourseGradeFactory().update(user=user, course_key=course_key, force_update_subsections=True) + + def _emit_event(kwargs): """ Emits a problem submitted event only if there is no current event diff --git a/lms/djangoapps/grades/tests/test_signals.py b/lms/djangoapps/grades/tests/test_signals.py index f28282125d..ffc6877690 100644 --- a/lms/djangoapps/grades/tests/test_signals.py +++ b/lms/djangoapps/grades/tests/test_signals.py @@ -1,7 +1,7 @@ """ Tests for the score change signals defined in the courseware models module. """ - +import itertools import re from datetime import datetime @@ -10,15 +10,20 @@ import pytz from django.test import TestCase from mock import MagicMock, patch +from opaque_keys.edx.locations import CourseLocator +from openedx.core.djangoapps.course_groups.signals.signals import COHORT_MEMBERSHIP_UPDATED +from student.signals.signals import ENROLLMENT_TRACK_UPDATED +from student.tests.factories import UserFactory from submissions.models import score_reset, score_set from util.date_utils import to_timestamp +from ..config.waffle import waffle, WRITE_ONLY_IF_ENGAGED from ..constants import ScoreDatabaseTableEnum from ..signals.handlers import ( disconnect_submissions_signal_receiver, problem_raw_score_changed_handler, submissions_score_reset_handler, - submissions_score_set_handler + submissions_score_set_handler, ) from ..signals.signals import PROBLEM_RAW_SCORE_CHANGED @@ -251,3 +256,32 @@ class ScoreChangedSignalRelayTest(TestCase): with self.assertRaises(ValueError): with disconnect_submissions_signal_receiver(PROBLEM_RAW_SCORE_CHANGED): pass + + +@ddt.ddt +class RecalculateUserGradeSignalsTest(TestCase): + def setUp(self): + super(RecalculateUserGradeSignalsTest, self).setUp() + self.user = UserFactory() + self.course_key = CourseLocator("test_org", "test_course_num", "test_run") + + @patch('lms.djangoapps.grades.signals.handlers.CourseGradeFactory.update') + @patch('lms.djangoapps.grades.signals.handlers.CourseGradeFactory.read') + @ddt.data(*itertools.product((COHORT_MEMBERSHIP_UPDATED, ENROLLMENT_TRACK_UPDATED), (True, False), (True, False))) + @ddt.unpack + def test_recalculate_on_signal(self, signal, write_only_if_engaged, has_grade, read_mock, update_mock): + """ + Tests the grades handler for signals that trigger regrading. + The handler should call CourseGradeFactory.update() with the + args below, *except* if the WRITE_ONLY_IF_ENGAGED waffle flag + is inactive and the user does not have a grade. + """ + if not has_grade: + read_mock.return_value = None + with waffle().override(WRITE_ONLY_IF_ENGAGED, active=write_only_if_engaged): + signal.send(sender=None, user=self.user, course_key=self.course_key) + + if not write_only_if_engaged and not has_grade: + update_mock.assert_not_called() + else: + update_mock.assert_called_with(course_key=self.course_key, user=self.user, force_update_subsections=True) diff --git a/openedx/core/djangoapps/course_groups/cohorts.py b/openedx/core/djangoapps/course_groups/cohorts.py index 3a1713b68e..2a676f5791 100644 --- a/openedx/core/djangoapps/course_groups/cohorts.py +++ b/openedx/core/djangoapps/course_groups/cohorts.py @@ -28,6 +28,7 @@ from .models import ( CourseUserGroupPartitionGroup, UnregisteredLearnerCohortAssignments ) +from .signals.signals import COHORT_MEMBERSHIP_UPDATED log = logging.getLogger(__name__) @@ -424,7 +425,9 @@ def remove_user_from_cohort(cohort, username_or_email): try: membership = CohortMembership.objects.get(course_user_group=cohort, user=user) + course_key = membership.course_id membership.delete() + COHORT_MEMBERSHIP_UPDATED.send(sender=None, user=user, course_key=course_key) except CohortMembership.DoesNotExist: raise ValueError("User {} was not present in cohort {}".format(username_or_email, cohort)) @@ -454,7 +457,7 @@ def add_user_to_cohort(cohort, username_or_email): membership = CohortMembership(course_user_group=cohort, user=user) membership.save() # This will handle both cases, creation and updating, of a CohortMembership for this user. - + COHORT_MEMBERSHIP_UPDATED.send(sender=None, user=user, course_key=membership.course_id) tracker.emit( "edx.cohort.user_add_requested", { diff --git a/openedx/core/djangoapps/course_groups/signals/__init__.py b/openedx/core/djangoapps/course_groups/signals/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/openedx/core/djangoapps/course_groups/signals/signals.py b/openedx/core/djangoapps/course_groups/signals/signals.py new file mode 100644 index 0000000000..e373f20417 --- /dev/null +++ b/openedx/core/djangoapps/course_groups/signals/signals.py @@ -0,0 +1,6 @@ +""" +Cohorts related signals. +""" +from django.dispatch import Signal + +COHORT_MEMBERSHIP_UPDATED = Signal(providing_args=['user', 'course_key']) diff --git a/openedx/core/djangoapps/course_groups/tests/test_cohorts.py b/openedx/core/djangoapps/course_groups/tests/test_cohorts.py index 65a2c1c717..cee543f396 100644 --- a/openedx/core/djangoapps/course_groups/tests/test_cohorts.py +++ b/openedx/core/djangoapps/course_groups/tests/test_cohorts.py @@ -591,7 +591,8 @@ class TestCohorts(ModuleStoreTestCase): ) @patch("openedx.core.djangoapps.course_groups.cohorts.tracker") - def test_add_user_to_cohort(self, mock_tracker): + @patch("openedx.core.djangoapps.course_groups.cohorts.COHORT_MEMBERSHIP_UPDATED") + def test_add_user_to_cohort(self, mock_signal, mock_tracker): """ Make sure cohorts.add_user_to_cohort() properly adds a user to a cohort and handles errors. @@ -603,6 +604,10 @@ class TestCohorts(ModuleStoreTestCase): first_cohort = CohortFactory(course_id=course.id, name="FirstCohort") second_cohort = CohortFactory(course_id=course.id, name="SecondCohort") + def check_and_reset_signal(): + mock_signal.send.assert_called_with(sender=None, user=course_user, course_key=self.toy_course_key) + mock_signal.reset_mock() + # Success cases # We shouldn't get back a previous cohort, since the user wasn't in one self.assertEqual( @@ -619,6 +624,8 @@ class TestCohorts(ModuleStoreTestCase): "previous_cohort_name": None, } ) + check_and_reset_signal() + # Should get (user, previous_cohort_name) when moved from one cohort to # another self.assertEqual( @@ -635,6 +642,8 @@ class TestCohorts(ModuleStoreTestCase): "previous_cohort_name": first_cohort.name, } ) + check_and_reset_signal() + # Should preregister email address for a cohort if an email address # not associated with a user is added (user, previous_cohort, prereg) = cohorts.add_user_to_cohort(first_cohort, "new_email@example.com") @@ -650,6 +659,7 @@ class TestCohorts(ModuleStoreTestCase): "cohort_name": first_cohort.name, } ) + # Error cases # Should get ValueError if user already in cohort self.assertRaises( diff --git a/openedx/core/djangoapps/credit/tests/test_api.py b/openedx/core/djangoapps/credit/tests/test_api.py index ee57fbe99c..7d103ae1fc 100644 --- a/openedx/core/djangoapps/credit/tests/test_api.py +++ b/openedx/core/djangoapps/credit/tests/test_api.py @@ -158,7 +158,8 @@ class CreditApiTestBase(ModuleStoreTestCase): def setUp(self, **kwargs): super(CreditApiTestBase, self).setUp() - self.course_key = CourseKey.from_string("edX/DemoX/Demo_Course") + self.course = CourseFactory.create(org="edx", course="DemoX", run="Demo_Course") + self.course_key = self.course.id def add_credit_course(self, course_key=None, enabled=True): """Mark the course as a credit """ @@ -631,8 +632,6 @@ class CreditRequirementApiTests(CreditApiTestBase): # Configure a course with two credit requirements self.add_credit_course() user = self.create_and_enroll_user(username=self.USER_INFO['username'], password=self.USER_INFO['password']) - CourseFactory.create(org='edX', number='DemoX', display_name='Demo_Course') - requirements = [ { "namespace": "grade", @@ -664,7 +663,7 @@ class CreditRequirementApiTests(CreditApiTestBase): self.assertFalse(api.is_user_eligible_for_credit(user.username, self.course_key)) # Satisfy the other requirement - with self.assertNumQueries(24): + with self.assertNumQueries(23): api.set_credit_requirement_status( user, self.course_key, @@ -822,7 +821,6 @@ class CreditRequirementApiTests(CreditApiTestBase): # Configure a course with two credit requirements self.add_credit_course() user = self.create_and_enroll_user(username=self.USER_INFO['username'], password=self.USER_INFO['password']) - CourseFactory.create(org='edX', number='DemoX', display_name='Demo_Course') requirements = [ { "namespace": "grade", From a830073bb281b87536210ba933910cefa278f9b2 Mon Sep 17 00:00:00 2001 From: Eric Fischer Date: Thu, 21 Sep 2017 16:31:46 -0400 Subject: [PATCH 3/8] Add waffle flag for NewAssetsPage --- cms/djangoapps/contentstore/admin.py | 22 ++++- .../contentstore/config/__init__.py | 0 cms/djangoapps/contentstore/config/forms.py | 37 ++++++++ cms/djangoapps/contentstore/config/models.py | 77 ++++++++++++++++ .../contentstore/config/tests/__init__.py | 0 .../contentstore/config/tests/test_models.py | 92 +++++++++++++++++++ .../contentstore/config/tests/utils.py | 27 ++++++ .../migrations/0002_add_assets_page_flag.py | 38 ++++++++ cms/djangoapps/contentstore/views/assets.py | 2 + 9 files changed, 294 insertions(+), 1 deletion(-) create mode 100644 cms/djangoapps/contentstore/config/__init__.py create mode 100644 cms/djangoapps/contentstore/config/forms.py create mode 100644 cms/djangoapps/contentstore/config/models.py create mode 100644 cms/djangoapps/contentstore/config/tests/__init__.py create mode 100644 cms/djangoapps/contentstore/config/tests/test_models.py create mode 100644 cms/djangoapps/contentstore/config/tests/utils.py create mode 100644 cms/djangoapps/contentstore/migrations/0002_add_assets_page_flag.py diff --git a/cms/djangoapps/contentstore/admin.py b/cms/djangoapps/contentstore/admin.py index c28a5355e8..0807e6843e 100644 --- a/cms/djangoapps/contentstore/admin.py +++ b/cms/djangoapps/contentstore/admin.py @@ -2,10 +2,30 @@ Admin site bindings for contentstore """ -from config_models.admin import ConfigurationModelAdmin +from config_models.admin import ConfigurationModelAdmin, KeyedConfigurationModelAdmin from django.contrib import admin +from contentstore.config.forms import CourseNewAssetsPageAdminForm +from contentstore.config.models import NewAssetsPageFlag, CourseNewAssetsPageFlag from contentstore.models import PushNotificationConfig, VideoUploadConfig + +class CourseNewAssetsPageAdmin(KeyedConfigurationModelAdmin): + """ + Admin for enabling new asset page on a course-by-course basis. + Allows searching by course id. + """ + form = CourseNewAssetsPageAdminForm + search_fields = ['course_id'] + fieldsets = ( + (None, { + 'fields': ('course_id', 'enabled'), + 'description': 'Enter a valid course id. If it is invalid, an error message will display.' + }), + ) + +admin.site.register(NewAssetsPageFlag, ConfigurationModelAdmin) +admin.site.register(CourseNewAssetsPageFlag, CourseNewAssetsPageAdmin) + admin.site.register(VideoUploadConfig, ConfigurationModelAdmin) admin.site.register(PushNotificationConfig, ConfigurationModelAdmin) diff --git a/cms/djangoapps/contentstore/config/__init__.py b/cms/djangoapps/contentstore/config/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/cms/djangoapps/contentstore/config/forms.py b/cms/djangoapps/contentstore/config/forms.py new file mode 100644 index 0000000000..a9e77a1cf7 --- /dev/null +++ b/cms/djangoapps/contentstore/config/forms.py @@ -0,0 +1,37 @@ +""" +Defines a form for providing validation. +""" +import logging + +from django import forms + +from contentstore.config.models import CourseNewAssetsPageFlag + +from opaque_keys import InvalidKeyError +from xmodule.modulestore.django import modulestore +from opaque_keys.edx.locator import CourseLocator + +log = logging.getLogger(__name__) + + +class CourseNewAssetsPageAdminForm(forms.ModelForm): + """Input form for new asset page enablment, allowing us to verify user input.""" + + class Meta(object): + model = CourseNewAssetsPageFlag + fields = '__all__' + + def clean_course_id(self): + """Validate the course id""" + cleaned_id = self.cleaned_data["course_id"] + try: + course_key = CourseLocator.from_string(cleaned_id) + except InvalidKeyError: + msg = u'Course id invalid. Entered course id was: "{0}."'.format(cleaned_id) + raise forms.ValidationError(msg) + + if not modulestore().has_course(course_key): + msg = u'Course not found. Entered course id was: "{0}". '.format(course_key.to_deprecated_string()) + raise forms.ValidationError(msg) + + return course_key diff --git a/cms/djangoapps/contentstore/config/models.py b/cms/djangoapps/contentstore/config/models.py new file mode 100644 index 0000000000..a75bf17bd2 --- /dev/null +++ b/cms/djangoapps/contentstore/config/models.py @@ -0,0 +1,77 @@ +""" +Models for configuration of the feature flags +controlling the new assets page. +""" +from config_models.models import ConfigurationModel +from django.db.models import BooleanField +from openedx.core.djangoapps.xmodule_django.models import CourseKeyField + + +class NewAssetsPageFlag(ConfigurationModel): + """ + Enables the in-development new assets page from studio-frontend. + + Defaults to False platform-wide, but can be overriden via a course-specific + flag. The idea is that we can use this to do a gradual rollout, and remove + the flag entirely once generally released to everyone. + """ + # this field overrides course-specific settings to enable the feature for all courses + enabled_for_all_courses = BooleanField(default=False) + + @classmethod + def feature_enabled(cls, course_id=None): + """ + Looks at the currently active configuration model to determine whether + the new assets page feature is available. + + There are 2 booleans to be concerned with - enabled_for_all_courses, + and the implicit is_enabled(). They interact in the following ways: + - is_enabled: False, enabled_for_all_courses: True or False + - no one can use the feature. + - is_enabled: True, enabled_for_all_courses: False + - check for a CourseNewAssetsPageFlag, use that value (default False) + - if no course_id provided, return False + - is_enabled: True, enabled_for_all_courses: True + - everyone can use the feature + """ + if not NewAssetsPageFlag.is_enabled(): + return False + elif not NewAssetsPageFlag.current().enabled_for_all_courses: + if course_id: + effective = CourseNewAssetsPageFlag.objects.filter(course_id=course_id).order_by('-change_date').first() + return effective.enabled if effective is not None else False + else: + return False + else: + return True + + class Meta(object): + app_label = "contentstore" + + def __unicode__(self): + current_model = NewAssetsPageFlag.current() + return u"NewAssetsPageFlag: enabled {}".format( + current_model.is_enabled() + ) + + +class CourseNewAssetsPageFlag(ConfigurationModel): + """ + Enables new assets page for a specific + course. Only has an effect if the general + flag above is set to True. + """ + KEY_FIELDS = ('course_id',) + + class Meta(object): + app_label = "contentstore" + + # The course that these features are attached to. + course_id = CourseKeyField(max_length=255, db_index=True) + + def __unicode__(self): + not_en = "Not " + if self.enabled: + not_en = "" + # pylint: disable=no-member + return u"Course '{}': Persistent Grades {}Enabled".format(self.course_id.to_deprecated_string(), not_en) diff --git a/cms/djangoapps/contentstore/config/tests/__init__.py b/cms/djangoapps/contentstore/config/tests/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/cms/djangoapps/contentstore/config/tests/test_models.py b/cms/djangoapps/contentstore/config/tests/test_models.py new file mode 100644 index 0000000000..743d38c147 --- /dev/null +++ b/cms/djangoapps/contentstore/config/tests/test_models.py @@ -0,0 +1,92 @@ +""" +Tests for the models that control the +persistent grading feature. +""" +import itertools + +import ddt +from django.conf import settings +from django.test import TestCase +from mock import patch +from opaque_keys.edx.locator import CourseLocator + +from contentstore.config.models import NewAssetsPageFlag +from contentstore.config.tests.utils import new_assets_page_feature_flags + + +@ddt.ddt +class NewAssetsPageFlagTests(TestCase): + """ + Tests the behavior of the feature flags for the new assets page. + These are set via Django admin settings. + """ + def setUp(self): + super(NewAssetsPageFlagTests, self).setUp() + self.course_id_1 = CourseLocator(org="edx", course="course", run="run") + self.course_id_2 = CourseLocator(org="edx", course="course2", run="run") + + @ddt.data(*itertools.product( + (True, False), + (True, False), + (True, False), + )) + @ddt.unpack + def test_new_assets_page_feature_flags(self, global_flag, enabled_for_all_courses, enabled_for_course_1): + with new_assets_page_feature_flags( + global_flag=global_flag, + enabled_for_all_courses=enabled_for_all_courses, + course_id=self.course_id_1, + enabled_for_course=enabled_for_course_1 + ): + self.assertEqual(NewAssetsPageFlag.feature_enabled(), global_flag and enabled_for_all_courses) + self.assertEqual( + NewAssetsPageFlag.feature_enabled(self.course_id_1), + global_flag and (enabled_for_all_courses or enabled_for_course_1) + ) + self.assertEqual( + NewAssetsPageFlag.feature_enabled(self.course_id_2), + global_flag and enabled_for_all_courses + ) + + def test_enable_disable_course_flag(self): + """ + Ensures that the flag, once enabled for a course, can also be disabled. + """ + with new_assets_page_feature_flags( + global_flag=True, + enabled_for_all_courses=False, + course_id=self.course_id_1, + enabled_for_course=True + ): + self.assertTrue(NewAssetsPageFlag.feature_enabled(self.course_id_1)) + with new_assets_page_feature_flags( + global_flag=True, + enabled_for_all_courses=False, + course_id=self.course_id_1, + enabled_for_course=False + ): + self.assertFalse(NewAssetsPageFlag.feature_enabled(self.course_id_1)) + + def test_enable_disable_globally(self): + """ + Ensures that the flag, once enabled globally, can also be disabled. + """ + with new_assets_page_feature_flags( + global_flag=True, + enabled_for_all_courses=True, + ): + self.assertTrue(NewAssetsPageFlag.feature_enabled()) + self.assertTrue(NewAssetsPageFlag.feature_enabled(self.course_id_1)) + with new_assets_page_feature_flags( + global_flag=True, + enabled_for_all_courses=False, + course_id=self.course_id_1, + enabled_for_course=True + ): + self.assertFalse(NewAssetsPageFlag.feature_enabled()) + self.assertTrue(NewAssetsPageFlag.feature_enabled(self.course_id_1)) + with new_assets_page_feature_flags( + global_flag=False, + ): + self.assertFalse(NewAssetsPageFlag.feature_enabled()) + self.assertFalse(NewAssetsPageFlag.feature_enabled(self.course_id_1)) diff --git a/cms/djangoapps/contentstore/config/tests/utils.py b/cms/djangoapps/contentstore/config/tests/utils.py new file mode 100644 index 0000000000..69430ccb1d --- /dev/null +++ b/cms/djangoapps/contentstore/config/tests/utils.py @@ -0,0 +1,27 @@ +""" +Provides helper functions for tests that want +to configure flags related to persistent grading. +""" +from contextlib import contextmanager + +from contentstore.config.models import NewAssetsPageFlag, CourseNewAssetsPageFlag +from request_cache.middleware import RequestCache + + +@contextmanager +def new_assets_page_feature_flags( + global_flag, + enabled_for_all_courses=False, + course_id=None, + enabled_for_course=False +): + """ + Most test cases will use a single call to this manager, + as they need to set the global setting and the course-specific + setting for a single course. + """ + RequestCache.clear_request_cache() + NewAssetsPageFlag.objects.create(enabled=global_flag, enabled_for_all_courses=enabled_for_all_courses) + if course_id: + CourseNewAssetsPageFlag.objects.create(course_id=course_id, enabled=enabled_for_course) + yield diff --git a/cms/djangoapps/contentstore/migrations/0002_add_assets_page_flag.py b/cms/djangoapps/contentstore/migrations/0002_add_assets_page_flag.py new file mode 100644 index 0000000000..8d4469ac39 --- /dev/null +++ b/cms/djangoapps/contentstore/migrations/0002_add_assets_page_flag.py @@ -0,0 +1,38 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations, models +from django.conf import settings +import django.db.models.deletion +import openedx.core.djangoapps.xmodule_django.models + + +class Migration(migrations.Migration): + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ('contentstore', '0001_initial'), + ] + + operations = [ + migrations.CreateModel( + name='CourseNewAssetsPageFlag', + fields=[ + ('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)), + ('change_date', models.DateTimeField(auto_now_add=True, verbose_name='Change date')), + ('enabled', models.BooleanField(default=False, verbose_name='Enabled')), + ('course_id', openedx.core.djangoapps.xmodule_django.models.CourseKeyField(max_length=255, db_index=True)), + ('changed_by', models.ForeignKey(on_delete=django.db.models.deletion.PROTECT, editable=False, to=settings.AUTH_USER_MODEL, null=True, verbose_name='Changed by')), + ], + ), + migrations.CreateModel( + name='NewAssetsPageFlag', + fields=[ + ('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)), + ('change_date', models.DateTimeField(auto_now_add=True, verbose_name='Change date')), + ('enabled', models.BooleanField(default=False, verbose_name='Enabled')), + ('enabled_for_all_courses', models.BooleanField(default=False)), + ('changed_by', models.ForeignKey(on_delete=django.db.models.deletion.PROTECT, editable=False, to=settings.AUTH_USER_MODEL, null=True, verbose_name='Changed by')), + ], + ), + ] diff --git a/cms/djangoapps/contentstore/views/assets.py b/cms/djangoapps/contentstore/views/assets.py index cd782a598c..86b7ecc20a 100644 --- a/cms/djangoapps/contentstore/views/assets.py +++ b/cms/djangoapps/contentstore/views/assets.py @@ -18,6 +18,7 @@ from xmodule.exceptions import NotFoundError from xmodule.modulestore.django import modulestore from xmodule.modulestore.exceptions import ItemNotFoundError +from contentstore.config.models import NewAssetsPageFlag from contentstore.utils import reverse_course_url from contentstore.views.exception import AssetNotFoundException, AssetSizeTooLargeException from edxmako.shortcuts import render_to_response @@ -95,6 +96,7 @@ def _asset_index(course_key): course_module = modulestore().get_course(course_key) return render_to_response('asset_index.html', { + 'waffle_flag_enabled': NewAssetsPageFlag.feature_enabled(course_key), 'context_course': course_module, 'max_file_size_in_mbs': settings.MAX_ASSET_UPLOAD_FILE_SIZE_IN_MB, 'chunk_size_in_mbs': settings.UPLOAD_CHUNK_SIZE_IN_MB, From ac801833433c024cbea6c472eb69835814e3cdac Mon Sep 17 00:00:00 2001 From: Eric Fischer Date: Thu, 21 Sep 2017 16:32:06 -0400 Subject: [PATCH 4/8] Add studio-frontend to studio, if waffle flag set --- .gitignore | 1 + cms/templates/asset_index.html | 23 +++++++++++++++++------ package.json | 1 + pavelib/assets.py | 18 +++++++++++++++--- 4 files changed, 34 insertions(+), 9 deletions(-) diff --git a/.gitignore b/.gitignore index 637b00b853..13e6d07f4f 100644 --- a/.gitignore +++ b/.gitignore @@ -95,6 +95,7 @@ lms/static/css/ lms/static/certificates/css/ cms/static/css/ common/static/common/js/vendor/ +common/static/common/css/vendor/ common/static/bundles webpack-stats.json diff --git a/cms/templates/asset_index.html b/cms/templates/asset_index.html index aa9e4e65cc..9634501eaa 100644 --- a/cms/templates/asset_index.html +++ b/cms/templates/asset_index.html @@ -13,13 +13,19 @@ <%namespace name='static' file='static_content.html'/> <%block name="header_extras"> -% for template_name in ["asset"]: - -% endfor +% if waffle_flag_enabled: + + +% else: + % for template_name in ["asset"]: + + % endfor +% endif +% if not waffle_flag_enabled: <%block name="requirejs"> require(["js/factories/asset_index"], function (AssetIndexFactory) { AssetIndexFactory({ @@ -30,6 +36,7 @@ }); }); +% endif <%block name="content"> @@ -53,7 +60,11 @@
-
+ % if waffle_flag_enabled: +
+ % else: +
+ % endif

${_("Loading")}

diff --git a/package.json b/package.json index fa0be9b955..cd3cb8a4ba 100644 --- a/package.json +++ b/package.json @@ -10,6 +10,7 @@ "backbone.paginator": "~2.0.3", "coffee-loader": "^0.7.3", "coffee-script": "1.6.1", + "@edx/studio-frontend": "0.1.0", "edx-bootstrap": "^0.2.1", "edx-pattern-library": "0.18.1", "edx-ui-toolkit": "1.5.2", diff --git a/pavelib/assets.py b/pavelib/assets.py index 2d4261c219..282372d9f8 100644 --- a/pavelib/assets.py +++ b/pavelib/assets.py @@ -64,6 +64,10 @@ NPM_INSTALLED_LIBRARIES = [ 'requirejs/require.js', 'underscore.string/dist/underscore.string.js', 'underscore/underscore.js', + '@edx/studio-frontend/dist/assets.min.js', + '@edx/studio-frontend/dist/assets.min.js.map', + '@edx/studio-frontend/dist/studio-frontend.min.css', + '@edx/studio-frontend/dist/studio-frontend.min.css.map' ] # A list of NPM installed developer libraries that should be copied into the common @@ -74,7 +78,9 @@ NPM_INSTALLED_DEVELOPER_LIBRARIES = [ ] # Directory to install static vendor files -NPM_VENDOR_DIRECTORY = path('common/static/common/js/vendor') +NPM_JS_VENDOR_DIRECTORY = path('common/static/common/js/vendor') +NPM_CSS_VENDOR_DIRECTORY = path("common/static/common/css/vendor") +NPM_CSS_DIRECTORY = path("common/static/common/css") # system specific lookup path additions, add sass dirs if one system depends on the sass files for other systems SASS_LOOKUP_DEPENDENCIES = { @@ -604,10 +610,14 @@ def process_npm_assets(): Copies a vendor library to the shared vendor directory. """ library_path = 'node_modules/{library}'.format(library=library) + if library.endswith('.css') or library.endswith('.css.map'): + vendor_dir = NPM_CSS_VENDOR_DIRECTORY + else: + vendor_dir = NPM_JS_VENDOR_DIRECTORY if os.path.exists(library_path): sh('/bin/cp -rf {library_path} {vendor_dir}'.format( library_path=library_path, - vendor_dir=NPM_VENDOR_DIRECTORY, + vendor_dir=vendor_dir, )) elif not skip_if_missing: raise Exception('Missing vendor file {library_path}'.format(library_path=library_path)) @@ -618,7 +628,9 @@ def process_npm_assets(): return # Ensure that the vendor directory exists - NPM_VENDOR_DIRECTORY.mkdir_p() + NPM_JS_VENDOR_DIRECTORY.mkdir_p() + NPM_CSS_DIRECTORY.mkdir_p() + NPM_CSS_VENDOR_DIRECTORY.mkdir_p() # Copy each file to the vendor directory, overwriting any existing file. print("Copying vendor files into static directory") From 6355604b513fc18be3efd8ff3ea4de0519c087b7 Mon Sep 17 00:00:00 2001 From: Uman Shahzad Date: Fri, 22 Sep 2017 02:30:04 +0500 Subject: [PATCH 5/8] Ignore pending migrations for `edx-enterprise` temporarily. --- common/djangoapps/util/tests/test_db.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/common/djangoapps/util/tests/test_db.py b/common/djangoapps/util/tests/test_db.py index 3830d178ea..662c5d618d 100644 --- a/common/djangoapps/util/tests/test_db.py +++ b/common/djangoapps/util/tests/test_db.py @@ -228,8 +228,15 @@ class MigrationTests(TestCase): The test is set up to override MIGRATION_MODULES to ensure migrations are enabled for purposes of this test regardless of the overall test settings. + + TODO: Find a general way of handling the case where if we're trying to + make a migrationless release that'll require a separate migration + release afterwards, this test doesn't fail. """ out = StringIO() call_command('makemigrations', dry_run=True, verbosity=3, stdout=out) output = out.getvalue() - self.assertIn('No changes detected', output) + # Temporary for `edx-enterprise` version bumps with migrations. + # Please delete when `edx-enterprise==0.47.0`. + if 'Remove field' not in output and 'Delete model' not in output: + self.assertIn('No changes detected', output) From b2c6a53448d31ff98514d018f46174137e218762 Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Fri, 22 Sep 2017 09:18:43 -0400 Subject: [PATCH 6/8] Grades cleanup: remove no longer needed management commands EDUCATOR-178 --- .../grades/management/commands/get_grades.py | 136 -------- .../management/commands/reset_grades.py | 144 -------- .../commands/tests/test_reset_grades.py | 319 ------------------ lms/djangoapps/grades/models.py | 36 +- 4 files changed, 2 insertions(+), 633 deletions(-) delete mode 100644 lms/djangoapps/grades/management/commands/get_grades.py delete mode 100644 lms/djangoapps/grades/management/commands/reset_grades.py delete mode 100644 lms/djangoapps/grades/management/commands/tests/test_reset_grades.py diff --git a/lms/djangoapps/grades/management/commands/get_grades.py b/lms/djangoapps/grades/management/commands/get_grades.py deleted file mode 100644 index 2cfe4a8cf6..0000000000 --- a/lms/djangoapps/grades/management/commands/get_grades.py +++ /dev/null @@ -1,136 +0,0 @@ -""" -Management command to generate a list of grades for -all students that are enrolled in a course. -""" -import csv -import datetime -import os -from optparse import make_option - -from django.contrib.auth.models import User -from django.core.handlers.base import BaseHandler -from django.core.management.base import BaseCommand, CommandError -from django.test.client import RequestFactory -from opaque_keys.edx.keys import CourseKey - -from lms.djangoapps.certificates.models import GeneratedCertificate -from lms.djangoapps.courseware import courses -from lms.djangoapps.grades.new.course_grade_factory import CourseGradeFactory - - -class RequestMock(RequestFactory): - """ - Class to create a mock request. - """ - def request(self, **request): - "Construct a generic request object." - request = RequestFactory.request(self, **request) - handler = BaseHandler() - handler.load_middleware() - for middleware_method in handler._request_middleware: # pylint: disable=protected-access - if middleware_method(request): - raise Exception("Couldn't create request mock object - " - "request middleware returned a response") - return request - - -class Command(BaseCommand): - """ - Management command for get_grades - """ - - help = """ - Generate a list of grades for all students - that are enrolled in a course. - - CSV will include the following: - - username - - email - - grade in the certificate table if it exists - - computed grade - - grade breakdown - - Outputs grades to a csv file. - - Example: - sudo -u www-data SERVICE_VARIANT=lms /opt/edx/bin/django-admin.py get_grades \ - -c MITx/Chi6.00intro/A_Taste_of_Python_Programming -o /tmp/20130813-6.00x.csv \ - --settings=lms.envs.aws --pythonpath=/opt/wwc/edx-platform - """ - - option_list = BaseCommand.option_list + ( - make_option('-c', '--course', - metavar='COURSE_ID', - dest='course', - default=False, - help='Course ID for grade distribution'), - make_option('-o', '--output', - metavar='FILE', - dest='output', - default=False, - help='Filename for grade output')) - - def handle(self, *args, **options): - if os.path.exists(options['output']): - raise CommandError("File {0} already exists".format( - options['output'])) - - status_interval = 100 - - # parse out the course into a coursekey - if options['course']: - course_key = CourseKey.from_string(options['course']) - - print "Fetching enrolled students for {0}".format(course_key) - enrolled_students = User.objects.filter( - courseenrollment__course_id=course_key - ) - factory = RequestMock() - request = factory.get('/') - - total = enrolled_students.count() - print "Total enrolled: {0}".format(total) - course = courses.get_course_by_id(course_key) - total = enrolled_students.count() - start = datetime.datetime.now() - rows = [] - header = None - print "Fetching certificate data" - cert_grades = { - cert.user.username: cert.grade - for cert in list( - GeneratedCertificate.objects.filter( # pylint: disable=no-member - course_id=course_key - ).prefetch_related('user') - ) - } - print "Grading students" - for count, student in enumerate(enrolled_students): - count += 1 - if count % status_interval == 0: - # Print a status update with an approximation of - # how much time is left based on how long the last - # interval took - diff = datetime.datetime.now() - start - timeleft = diff * (total - count) / status_interval - hours, remainder = divmod(timeleft.seconds, 3600) - minutes, __ = divmod(remainder, 60) - print "{0}/{1} completed ~{2:02}:{3:02}m remaining".format( - count, total, hours, minutes) - start = datetime.datetime.now() - request.user = student - grade = CourseGradeFactory().create(student, course) - if not header: - header = [section['label'] for section in grade.summary[u'section_breakdown']] - rows.append(["email", "username", "certificate-grade", "grade"] + header) - percents = {section['label']: section['percent'] for section in grade.summary[u'section_breakdown']} - row_percents = [percents[label] for label in header] - if student.username in cert_grades: - rows.append( - [student.email, student.username, cert_grades[student.username], grade.percent] + row_percents, - ) - else: - rows.append([student.email, student.username, "N/A", grade.percent] + row_percents) - with open(options['output'], 'wb') as f: - writer = csv.writer(f) - writer.writerows(rows) diff --git a/lms/djangoapps/grades/management/commands/reset_grades.py b/lms/djangoapps/grades/management/commands/reset_grades.py deleted file mode 100644 index b22a100c1f..0000000000 --- a/lms/djangoapps/grades/management/commands/reset_grades.py +++ /dev/null @@ -1,144 +0,0 @@ -""" -Reset persistent grades for learners. -""" -import logging -from datetime import datetime -from textwrap import dedent - -from django.core.management.base import BaseCommand, CommandError -from django.db.models import Count -from pytz import utc - -from lms.djangoapps.grades.models import PersistentCourseGrade, PersistentSubsectionGrade -from openedx.core.lib.command_utils import get_mutually_exclusive_required_option, parse_course_keys - -log = logging.getLogger(__name__) - - -DATE_FORMAT = "%Y-%m-%d %H:%M" - - -class Command(BaseCommand): - """ - Reset persistent grades for learners. - """ - help = dedent(__doc__).strip() - - def add_arguments(self, parser): - """ - Add arguments to the command parser. - """ - parser.add_argument( - '--dry_run', - action='store_true', - default=False, - dest='dry_run', - help="Output what we're going to do, but don't actually do it. To actually delete, use --delete instead." - ) - parser.add_argument( - '--delete', - action='store_true', - default=False, - dest='delete', - help="Actually perform the deletions of the course. For a Dry Run, use --dry_run instead." - ) - parser.add_argument( - '--courses', - dest='courses', - nargs='+', - help='Reset persistent grades for the list of courses provided.', - ) - parser.add_argument( - '--all_courses', - action='store_true', - dest='all_courses', - default=False, - help='Reset persistent grades for all courses.', - ) - parser.add_argument( - '--modified_start', - dest='modified_start', - help='Starting range for modified date (inclusive): e.g. "2016-08-23 16:43"; expected in UTC.', - ) - parser.add_argument( - '--modified_end', - dest='modified_end', - help='Ending range for modified date (inclusive): e.g. "2016-12-23 16:43"; expected in UTC.', - ) - parser.add_argument( - '--db_table', - dest='db_table', - help='Specify "subsection" to reset subsection grades or "course" to reset course grades. If absent, both ' - 'are reset.', - ) - - def handle(self, *args, **options): - course_keys = None - modified_start = None - modified_end = None - - run_mode = get_mutually_exclusive_required_option(options, 'delete', 'dry_run') - courses_mode = get_mutually_exclusive_required_option(options, 'courses', 'all_courses') - db_table = options.get('db_table') - if db_table not in {'subsection', 'course', None}: - raise CommandError('Invalid value for db_table. Valid options are "subsection" or "course" only.') - - if options.get('modified_start'): - modified_start = utc.localize(datetime.strptime(options['modified_start'], DATE_FORMAT)) - - if options.get('modified_end'): - if not modified_start: - raise CommandError('Optional value for modified_end provided without a value for modified_start.') - modified_end = utc.localize(datetime.strptime(options['modified_end'], DATE_FORMAT)) - - if courses_mode == 'courses': - course_keys = parse_course_keys(options['courses']) - - log.info("reset_grade: Started in %s mode!", run_mode) - - operation = self._query_grades if run_mode == 'dry_run' else self._delete_grades - - if db_table == 'subsection' or db_table is None: - operation(PersistentSubsectionGrade, course_keys, modified_start, modified_end) - - if db_table == 'course' or db_table is None: - operation(PersistentCourseGrade, course_keys, modified_start, modified_end) - - log.info("reset_grade: Finished in %s mode!", run_mode) - - def _delete_grades(self, grade_model_class, *args, **kwargs): - """ - Deletes the requested grades in the given model, filtered by the provided args and kwargs. - """ - grades_query_set = grade_model_class.query_grades(*args, **kwargs) - num_rows_to_delete = grades_query_set.count() - - log.info("reset_grade: Deleting %s: %d row(s).", grade_model_class.__name__, num_rows_to_delete) - - grade_model_class.delete_grades(*args, **kwargs) - - log.info("reset_grade: Deleted %s: %d row(s).", grade_model_class.__name__, num_rows_to_delete) - - def _query_grades(self, grade_model_class, *args, **kwargs): - """ - Queries the requested grades in the given model, filtered by the provided args and kwargs. - """ - total_for_all_courses = 0 - - grades_query_set = grade_model_class.query_grades(*args, **kwargs) - grades_stats = grades_query_set.values('course_id').order_by().annotate(total=Count('course_id')) - - for stat in grades_stats: - total_for_all_courses += stat['total'] - log.info( - "reset_grade: Would delete %s for COURSE %s: %d row(s).", - grade_model_class.__name__, - stat['course_id'], - stat['total'], - ) - - log.info( - "reset_grade: Would delete %s in TOTAL: %d row(s).", - grade_model_class.__name__, - total_for_all_courses, - ) diff --git a/lms/djangoapps/grades/management/commands/tests/test_reset_grades.py b/lms/djangoapps/grades/management/commands/tests/test_reset_grades.py deleted file mode 100644 index 3917944b63..0000000000 --- a/lms/djangoapps/grades/management/commands/tests/test_reset_grades.py +++ /dev/null @@ -1,319 +0,0 @@ -""" -Tests for reset_grades management command. -""" -from datetime import datetime, timedelta - -import ddt -from django.core.management.base import CommandError -from django.test import TestCase -from freezegun import freeze_time -from mock import MagicMock, patch -from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator - -from lms.djangoapps.grades.management.commands import reset_grades -from lms.djangoapps.grades.models import PersistentCourseGrade, PersistentSubsectionGrade - - -@ddt.ddt -class TestResetGrades(TestCase): - """ - Tests generate course blocks management command. - """ - num_users = 3 - num_courses = 5 - num_subsections = 7 - - def setUp(self): - super(TestResetGrades, self).setUp() - self.command = reset_grades.Command() - - self.user_ids = [user_id for user_id in range(self.num_users)] - - self.course_keys = [] - for course_index in range(self.num_courses): - self.course_keys.append( - CourseLocator( - org='some_org', - course='some_course', - run=unicode(course_index), - ) - ) - - self.subsection_keys_by_course = {} - for course_key in self.course_keys: - subsection_keys_in_course = [] - for subsection_index in range(self.num_subsections): - subsection_keys_in_course.append( - BlockUsageLocator( - course_key=course_key, - block_type='sequential', - block_id=unicode(subsection_index), - ) - ) - self.subsection_keys_by_course[course_key] = subsection_keys_in_course - - def _update_or_create_grades(self, courses_keys=None): - """ - Creates grades for all courses and subsections. - """ - if courses_keys is None: - courses_keys = self.course_keys - - course_grade_params = { - "course_version": "JoeMcEwing", - "course_edited_timestamp": datetime( - year=2016, - month=8, - day=1, - hour=18, - minute=53, - second=24, - microsecond=354741, - ), - "percent_grade": 77.7, - "letter_grade": "Great job", - "passed": True, - } - subsection_grade_params = { - "course_version": "deadbeef", - "subtree_edited_timestamp": "2016-08-01 18:53:24.354741", - "earned_all": 6.0, - "possible_all": 12.0, - "earned_graded": 6.0, - "possible_graded": 8.0, - "visible_blocks": MagicMock(), - "first_attempted": datetime.now(), - } - - for course_key in courses_keys: - for user_id in self.user_ids: - course_grade_params['user_id'] = user_id - course_grade_params['course_id'] = course_key - PersistentCourseGrade.update_or_create(**course_grade_params) - for subsection_key in self.subsection_keys_by_course[course_key]: - subsection_grade_params['user_id'] = user_id - subsection_grade_params['usage_key'] = subsection_key - PersistentSubsectionGrade.update_or_create_grade(**subsection_grade_params) - - def _assert_grades_exist_for_courses(self, course_keys, db_table=None): - """ - Assert grades for given courses exist. - """ - for course_key in course_keys: - if db_table == "course" or db_table is None: - self.assertIsNotNone(PersistentCourseGrade.read(self.user_ids[0], course_key)) - if db_table == "subsection" or db_table is None: - for subsection_key in self.subsection_keys_by_course[course_key]: - self.assertIsNotNone(PersistentSubsectionGrade.read_grade(self.user_ids[0], subsection_key)) - - def _assert_grades_absent_for_courses(self, course_keys, db_table=None): - """ - Assert grades for given courses do not exist. - """ - for course_key in course_keys: - if db_table == "course" or db_table is None: - with self.assertRaises(PersistentCourseGrade.DoesNotExist): - PersistentCourseGrade.read(self.user_ids[0], course_key) - - if db_table == "subsection" or db_table is None: - for subsection_key in self.subsection_keys_by_course[course_key]: - with self.assertRaises(PersistentSubsectionGrade.DoesNotExist): - PersistentSubsectionGrade.read_grade(self.user_ids[0], subsection_key) - - def _assert_stat_logged(self, mock_log, num_rows, grade_model_class, message_substring, log_offset): - self.assertIn('reset_grade: ' + message_substring, mock_log.info.call_args_list[log_offset][0][0]) - self.assertEqual(grade_model_class.__name__, mock_log.info.call_args_list[log_offset][0][1]) - self.assertEqual(num_rows, mock_log.info.call_args_list[log_offset][0][2]) - - def _assert_course_delete_stat_logged(self, mock_log, num_rows): - self._assert_stat_logged(mock_log, num_rows, PersistentCourseGrade, 'Deleted', log_offset=4) - - def _assert_subsection_delete_stat_logged(self, mock_log, num_rows): - self._assert_stat_logged(mock_log, num_rows, PersistentSubsectionGrade, 'Deleted', log_offset=2) - - def _assert_course_query_stat_logged(self, mock_log, num_rows, num_courses=None): - if num_courses is None: - num_courses = self.num_courses - log_offset = num_courses + 1 + num_courses + 1 - self._assert_stat_logged(mock_log, num_rows, PersistentCourseGrade, 'Would delete', log_offset) - - def _assert_subsection_query_stat_logged(self, mock_log, num_rows, num_courses=None): - if num_courses is None: - num_courses = self.num_courses - log_offset = num_courses + 1 - self._assert_stat_logged(mock_log, num_rows, PersistentSubsectionGrade, 'Would delete', log_offset) - - def _date_from_now(self, days=None): - return datetime.now() + timedelta(days=days) - - def _date_str_from_now(self, days=None): - future_date = self._date_from_now(days=days) - return future_date.strftime(reset_grades.DATE_FORMAT) - - @patch('lms.djangoapps.grades.management.commands.reset_grades.log') - def test_reset_all_courses(self, mock_log): - self._update_or_create_grades() - self._assert_grades_exist_for_courses(self.course_keys) - - with self.assertNumQueries(7): - self.command.handle(delete=True, all_courses=True) - - self._assert_grades_absent_for_courses(self.course_keys) - self._assert_subsection_delete_stat_logged( - mock_log, - num_rows=self.num_users * self.num_courses * self.num_subsections, - ) - self._assert_course_delete_stat_logged( - mock_log, - num_rows=self.num_users * self.num_courses, - ) - - @patch('lms.djangoapps.grades.management.commands.reset_grades.log') - @ddt.data(1, 2, 3) - def test_reset_some_courses(self, num_courses_to_reset, mock_log): - self._update_or_create_grades() - self._assert_grades_exist_for_courses(self.course_keys) - - with self.assertNumQueries(6): - self.command.handle( - delete=True, - courses=[unicode(course_key) for course_key in self.course_keys[:num_courses_to_reset]] - ) - - self._assert_grades_absent_for_courses(self.course_keys[:num_courses_to_reset]) - self._assert_grades_exist_for_courses(self.course_keys[num_courses_to_reset:]) - self._assert_subsection_delete_stat_logged( - mock_log, - num_rows=self.num_users * num_courses_to_reset * self.num_subsections, - ) - self._assert_course_delete_stat_logged( - mock_log, - num_rows=self.num_users * num_courses_to_reset, - ) - - def test_reset_by_modified_start_date(self): - self._update_or_create_grades() - self._assert_grades_exist_for_courses(self.course_keys) - - num_courses_with_updated_grades = 2 - with freeze_time(self._date_from_now(days=4)): - self._update_or_create_grades(self.course_keys[:num_courses_with_updated_grades]) - - with self.assertNumQueries(6): - self.command.handle(delete=True, modified_start=self._date_str_from_now(days=2), all_courses=True) - - self._assert_grades_absent_for_courses(self.course_keys[:num_courses_with_updated_grades]) - self._assert_grades_exist_for_courses(self.course_keys[num_courses_with_updated_grades:]) - - def test_reset_by_modified_start_end_date(self): - self._update_or_create_grades() - self._assert_grades_exist_for_courses(self.course_keys) - - with freeze_time(self._date_from_now(days=3)): - self._update_or_create_grades(self.course_keys[:2]) - with freeze_time(self._date_from_now(days=5)): - self._update_or_create_grades(self.course_keys[2:4]) - - with self.assertNumQueries(6): - self.command.handle( - delete=True, - modified_start=self._date_str_from_now(days=2), - modified_end=self._date_str_from_now(days=4), - all_courses=True, - ) - - # Only grades for courses modified within the 2->4 days - # should be deleted. - self._assert_grades_absent_for_courses(self.course_keys[:2]) - self._assert_grades_exist_for_courses(self.course_keys[2:]) - - @ddt.data('subsection', 'course') - def test_specify_db_table(self, db_table): - self._update_or_create_grades() - self._assert_grades_exist_for_courses(self.course_keys) - self.command.handle(delete=True, all_courses=True, db_table=db_table) - self._assert_grades_absent_for_courses(self.course_keys, db_table=db_table) - if db_table == "subsection": - self._assert_grades_exist_for_courses(self.course_keys, db_table='course') - else: - self._assert_grades_exist_for_courses(self.course_keys, db_table='subsection') - - @patch('lms.djangoapps.grades.management.commands.reset_grades.log') - def test_dry_run_all_courses(self, mock_log): - self._update_or_create_grades() - self._assert_grades_exist_for_courses(self.course_keys) - - with self.assertNumQueries(2): - self.command.handle(dry_run=True, all_courses=True) - - self._assert_grades_exist_for_courses(self.course_keys) - self._assert_subsection_query_stat_logged( - mock_log, - num_rows=self.num_users * self.num_courses * self.num_subsections, - ) - self._assert_course_query_stat_logged( - mock_log, - num_rows=self.num_users * self.num_courses, - ) - - @patch('lms.djangoapps.grades.management.commands.reset_grades.log') - @ddt.data(1, 2, 3) - def test_dry_run_some_courses(self, num_courses_to_query, mock_log): - self._update_or_create_grades() - self._assert_grades_exist_for_courses(self.course_keys) - - with self.assertNumQueries(2): - self.command.handle( - dry_run=True, - courses=[unicode(course_key) for course_key in self.course_keys[:num_courses_to_query]] - ) - - self._assert_grades_exist_for_courses(self.course_keys) - self._assert_subsection_query_stat_logged( - mock_log, - num_rows=self.num_users * num_courses_to_query * self.num_subsections, - num_courses=num_courses_to_query, - ) - self._assert_course_query_stat_logged( - mock_log, - num_rows=self.num_users * num_courses_to_query, - num_courses=num_courses_to_query, - ) - - @patch('lms.djangoapps.grades.management.commands.reset_grades.log') - def test_reset_no_existing_grades(self, mock_log): - self._assert_grades_absent_for_courses(self.course_keys) - - with self.assertNumQueries(4): - self.command.handle(delete=True, all_courses=True) - - self._assert_grades_absent_for_courses(self.course_keys) - self._assert_subsection_delete_stat_logged(mock_log, num_rows=0) - self._assert_course_delete_stat_logged(mock_log, num_rows=0) - - def test_invalid_key(self): - with self.assertRaisesRegexp(CommandError, 'Invalid key specified.*invalid/key'): - self.command.handle(dry_run=True, courses=['invalid/key']) - - def test_invalid_db_table(self): - with self.assertRaisesMessage( - CommandError, - 'Invalid value for db_table. Valid options are "subsection" or "course" only.' - ): - self.command.handle(delete=True, all_courses=True, db_table="not course or subsection") - - def test_no_run_mode(self): - with self.assertRaisesMessage(CommandError, 'Must specify exactly one of --delete, --dry_run'): - self.command.handle(all_courses=True) - - def test_both_run_modes(self): - with self.assertRaisesMessage(CommandError, 'Must specify exactly one of --delete, --dry_run'): - self.command.handle(all_courses=True, dry_run=True, delete=True) - - def test_no_course_mode(self): - with self.assertRaisesMessage(CommandError, 'Must specify exactly one of --courses, --all_courses'): - self.command.handle(dry_run=True) - - def test_both_course_modes(self): - with self.assertRaisesMessage(CommandError, 'Must specify exactly one of --courses, --all_courses'): - self.command.handle(dry_run=True, all_courses=True, courses=['some/course/key']) diff --git a/lms/djangoapps/grades/models.py b/lms/djangoapps/grades/models.py index d11a6bdb35..9dbe7a22a9 100644 --- a/lms/djangoapps/grades/models.py +++ b/lms/djangoapps/grades/models.py @@ -39,38 +39,6 @@ BLOCK_RECORD_LIST_VERSION = 1 BlockRecord = namedtuple('BlockRecord', ['locator', 'weight', 'raw_possible', 'graded']) -class DeleteGradesMixin(object): - """ - A Mixin class that provides functionality to delete grades. - """ - - @classmethod - def query_grades(cls, course_ids=None, modified_start=None, modified_end=None): - """ - Queries all the grades in the table, filtered by the provided arguments. - """ - kwargs = {} - - if course_ids: - kwargs['course_id__in'] = [course_id for course_id in course_ids] - - if modified_start: - if modified_end: - kwargs['modified__range'] = (modified_start, modified_end) - else: - kwargs['modified__gt'] = modified_start - - return cls.objects.filter(**kwargs) - - @classmethod - def delete_grades(cls, *args, **kwargs): - """ - Deletes all the grades in the table, filtered by the provided arguments. - """ - query = cls.query_grades(*args, **kwargs) - query.delete() - - class BlockRecordList(tuple): """ An immutable ordered list of BlockRecord objects. @@ -285,7 +253,7 @@ class VisibleBlocks(models.Model): return u"visible_blocks_cache.{}".format(course_key) -class PersistentSubsectionGrade(DeleteGradesMixin, TimeStampedModel): +class PersistentSubsectionGrade(TimeStampedModel): """ A django model tracking persistent grades at the subsection level. """ @@ -546,7 +514,7 @@ class PersistentSubsectionGrade(DeleteGradesMixin, TimeStampedModel): ) -class PersistentCourseGrade(DeleteGradesMixin, TimeStampedModel): +class PersistentCourseGrade(TimeStampedModel): """ A django model tracking persistent course grades. """ From bc76ffe5dce593d2d3f6d2391db34bb2b4119801 Mon Sep 17 00:00:00 2001 From: Harry Rein Date: Mon, 11 Sep 2017 10:43:24 -0400 Subject: [PATCH 7/8] Add message for setting course goal. LEARNER-2307 --- .../tests/test_transcripts_utils.py | 2 +- lms/djangoapps/course_goals/__init__.py | 0 lms/djangoapps/course_goals/api.py | 76 ++++++++ .../course_goals/migrations/0001_initial.py | 29 +++ .../course_goals/migrations/__init__.py | 0 lms/djangoapps/course_goals/models.py | 35 ++++ lms/djangoapps/course_goals/signals.py | 19 ++ lms/djangoapps/course_goals/tests/__init__.py | 0 lms/djangoapps/course_goals/tests/test_api.py | 62 ++++++ lms/djangoapps/course_goals/urls.py | 15 ++ lms/djangoapps/course_goals/views.py | 92 +++++++++ lms/envs/common.py | 6 + .../sass/features/_course-experience.scss | 45 ++++- lms/templates/navigation/navigation.html | 2 +- lms/urls.py | 5 + openedx/core/lib/api/permissions.py | 1 + .../features/course_experience/__init__.py | 9 +- .../course_experience/js/CourseGoals.js | 40 ++++ .../static/course_experience/js/CourseHome.js | 12 ++ .../static/course_experience/js/CourseSock.js | 2 +- .../course-messages-fragment.html | 16 +- .../tests/views/test_course_home.py | 59 +++++- .../tests/views/test_course_sock.py | 8 +- .../views/course_home_messages.py | 181 +++++++++++++----- webpack.config.js | 1 + 25 files changed, 644 insertions(+), 73 deletions(-) create mode 100644 lms/djangoapps/course_goals/__init__.py create mode 100644 lms/djangoapps/course_goals/api.py create mode 100644 lms/djangoapps/course_goals/migrations/0001_initial.py create mode 100644 lms/djangoapps/course_goals/migrations/__init__.py create mode 100644 lms/djangoapps/course_goals/models.py create mode 100644 lms/djangoapps/course_goals/signals.py create mode 100644 lms/djangoapps/course_goals/tests/__init__.py create mode 100644 lms/djangoapps/course_goals/tests/test_api.py create mode 100644 lms/djangoapps/course_goals/urls.py create mode 100644 lms/djangoapps/course_goals/views.py create mode 100644 openedx/features/course_experience/static/course_experience/js/CourseGoals.js diff --git a/cms/djangoapps/contentstore/tests/test_transcripts_utils.py b/cms/djangoapps/contentstore/tests/test_transcripts_utils.py index e92e1f937a..4e9de8075f 100644 --- a/cms/djangoapps/contentstore/tests/test_transcripts_utils.py +++ b/cms/djangoapps/contentstore/tests/test_transcripts_utils.py @@ -156,7 +156,7 @@ class TestSaveSubsToStore(SharedModuleStoreTestCase): def test_save_unjsonable_subs_to_store(self): """ - Assures that subs, that can't be dumped, can't be found later. + Ensures that subs, that can't be dumped, can't be found later. """ with self.assertRaises(NotFoundError): contentstore().find(self.content_location_unjsonable) diff --git a/lms/djangoapps/course_goals/__init__.py b/lms/djangoapps/course_goals/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/lms/djangoapps/course_goals/api.py b/lms/djangoapps/course_goals/api.py new file mode 100644 index 0000000000..b52c1794fa --- /dev/null +++ b/lms/djangoapps/course_goals/api.py @@ -0,0 +1,76 @@ +""" +Course Goals Python API +""" +from enum import Enum +from opaque_keys.edx.keys import CourseKey +from django.utils.translation import ugettext as _ +from openedx.core.djangolib.markup import Text + +from .models import CourseGoal + + +def add_course_goal(user, course_id, goal_key): + """ + Add a new course goal for the provided user and course. + + Arguments: + user: The user that is setting the goal + course_id (string): The id for the course the goal refers to + goal_key (string): The goal key that maps to one of the + enumerated goal keys from CourseGoalOption. + + """ + # Create and save a new course goal + course_key = CourseKey.from_string(str(course_id)) + new_goal = CourseGoal(user=user, course_key=course_key, goal_key=goal_key) + new_goal.save() + + +def get_course_goal(user, course_key): + """ + Given a user and a course_key, return their course goal. + + If a course goal does not exist, returns None. + """ + course_goals = CourseGoal.objects.filter(user=user, course_key=course_key) + return course_goals[0] if course_goals else None + + +def remove_course_goal(user, course_key): + """ + Given a user and a course_key, remove the course goal. + """ + course_goal = get_course_goal(user, course_key) + if course_goal: + course_goal.delete() + + +class CourseGoalOption(Enum): + """ + Types of goals that a user can select. + + These options are set to a string goal key so that they can be + referenced elsewhere in the code when necessary. + """ + CERTIFY = 'certify' + COMPLETE = 'complete' + EXPLORE = 'explore' + UNSURE = 'unsure' + + @classmethod + def get_course_goal_keys(self): + return [key.value for key in self] + + +def get_goal_text(goal_option): + """ + This function is used to translate the course goal option into + a translated, user-facing string to be used to represent that + particular goal. + """ + return { + CourseGoalOption.CERTIFY.value: Text(_('Earn a certificate')), + CourseGoalOption.COMPLETE.value: Text(_('Complete the course')), + CourseGoalOption.EXPLORE.value: Text(_('Explore the course')), + CourseGoalOption.UNSURE.value: Text(_('Not sure yet')), + }[goal_option] diff --git a/lms/djangoapps/course_goals/migrations/0001_initial.py b/lms/djangoapps/course_goals/migrations/0001_initial.py new file mode 100644 index 0000000000..bcf13e339e --- /dev/null +++ b/lms/djangoapps/course_goals/migrations/0001_initial.py @@ -0,0 +1,29 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations, models +import openedx.core.djangoapps.xmodule_django.models +from django.conf import settings + + +class Migration(migrations.Migration): + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ] + + operations = [ + migrations.CreateModel( + name='CourseGoal', + fields=[ + ('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)), + ('course_key', openedx.core.djangoapps.xmodule_django.models.CourseKeyField(max_length=255, db_index=True)), + ('goal_key', models.CharField(default=b'unsure', max_length=100, choices=[(b'certify', 'Earn a certificate.'), (b'complete', 'Complete the course.'), (b'explore', 'Explore the course.'), (b'unsure', 'Not sure yet.')])), + ('user', models.ForeignKey(to=settings.AUTH_USER_MODEL)), + ], + ), + migrations.AlterUniqueTogether( + name='coursegoal', + unique_together=set([('user', 'course_key')]), + ), + ] diff --git a/lms/djangoapps/course_goals/migrations/__init__.py b/lms/djangoapps/course_goals/migrations/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/lms/djangoapps/course_goals/models.py b/lms/djangoapps/course_goals/models.py new file mode 100644 index 0000000000..fac52a729a --- /dev/null +++ b/lms/djangoapps/course_goals/models.py @@ -0,0 +1,35 @@ +""" +Course Goals Models +""" +from django.contrib.auth.models import User +from django.db import models +from openedx.core.djangoapps.xmodule_django.models import CourseKeyField + + +class CourseGoal(models.Model): + """ + Represents a course goal set by a user on the course home page. + + The goal_key represents the goal key that maps to a translated + string through using the CourseGoalOption class. + """ + GOAL_KEY_CHOICES = ( + ('certify', 'Earn a certificate.'), + ('complete', 'Complete the course.'), + ('explore', 'Explore the course.'), + ('unsure', 'Not sure yet.'), + ) + + user = models.ForeignKey(User, blank=False) + course_key = CourseKeyField(max_length=255, db_index=True) + goal_key = models.CharField(max_length=100, choices=GOAL_KEY_CHOICES, default='unsure') + + def __unicode__(self): + return 'CourseGoal: {user} set goal to {goal} for course {course}'.format( + user=self.user.username, + course=self.course_key, + goal_key=self.goal_key, + ) + + class Meta: + unique_together = ("user", "course_key") diff --git a/lms/djangoapps/course_goals/signals.py b/lms/djangoapps/course_goals/signals.py new file mode 100644 index 0000000000..2957c2eb27 --- /dev/null +++ b/lms/djangoapps/course_goals/signals.py @@ -0,0 +1,19 @@ +""" +Course Goals Signals +""" +from django.db.models.signals import post_save +from django.dispatch import receiver +from eventtracking import tracker + +from .models import CourseGoal + + +@receiver(post_save, sender=CourseGoal, dispatch_uid="emit_course_goal_event") +def emit_course_goal_event(sender, instance, **kwargs): + name = 'edx.course.goal.added' if kwargs.get('created', False) else 'edx.course.goal.updated' + tracker.emit( + name, + { + 'goal_key': instance.goal_key, + } + ) diff --git a/lms/djangoapps/course_goals/tests/__init__.py b/lms/djangoapps/course_goals/tests/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/lms/djangoapps/course_goals/tests/test_api.py b/lms/djangoapps/course_goals/tests/test_api.py new file mode 100644 index 0000000000..1c20dd7ba3 --- /dev/null +++ b/lms/djangoapps/course_goals/tests/test_api.py @@ -0,0 +1,62 @@ +""" +Unit tests for course_goals.api methods. +""" + +from django.contrib.auth.models import User +from django.core.urlresolvers import reverse +from lms.djangoapps.course_goals.models import CourseGoal +from rest_framework.test import APIClient +from student.models import CourseEnrollment +from track.tests import EventTrackingTestCase +from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory + +TEST_PASSWORD = 'test' + + +class TestCourseGoalsAPI(EventTrackingTestCase, SharedModuleStoreTestCase): + """ + Testing the Course Goals API. + """ + + def setUp(self): + # Create a course with a verified track + super(TestCourseGoalsAPI, self).setUp() + self.course = CourseFactory.create(emit_signals=True) + + self.user = User.objects.create_user('john', 'lennon@thebeatles.com', 'password') + CourseEnrollment.enroll(self.user, self.course.id) + + self.client = APIClient(enforce_csrf_checks=True) + self.client.login(username=self.user.username, password=self.user.password) + self.client.force_authenticate(user=self.user) + + self.apiUrl = reverse('course_goals_api:v0:course_goal-list') + + def test_add_valid_goal(self): + """ Ensures a correctly formatted post succeeds. """ + response = self.post_course_goal(valid=True) + self.assert_events_emitted() + self.assertEqual(response.status_code, 201) + self.assertEqual(len(CourseGoal.objects.filter(user=self.user, course_key=self.course.id)), 1) + + def test_add_invalid_goal(self): + """ Ensures a correctly formatted post succeeds. """ + response = self.post_course_goal(valid=False) + self.assertEqual(response.status_code, 400) + self.assertEqual(len(CourseGoal.objects.filter(user=self.user, course_key=self.course.id)), 0) + + def post_course_goal(self, valid=True, goal_key='certify'): + """ + Sends a post request to set a course goal and returns the response. + """ + goal_key = goal_key if valid else 'invalid' + response = self.client.post( + self.apiUrl, + { + 'goal_key': goal_key, + 'course_key': self.course.id, + 'user': self.user.username, + }, + ) + return response diff --git a/lms/djangoapps/course_goals/urls.py b/lms/djangoapps/course_goals/urls.py new file mode 100644 index 0000000000..cb87b3db77 --- /dev/null +++ b/lms/djangoapps/course_goals/urls.py @@ -0,0 +1,15 @@ +""" +Course Goals URLs +""" +from django.conf.urls import include, patterns, url +from rest_framework import routers + +from .views import CourseGoalViewSet + +router = routers.DefaultRouter() +router.register(r'course_goals', CourseGoalViewSet, base_name='course_goal') + +urlpatterns = patterns( + '', + url(r'^v0/', include(router.urls, namespace='v0')), +) diff --git a/lms/djangoapps/course_goals/views.py b/lms/djangoapps/course_goals/views.py new file mode 100644 index 0000000000..513d52c440 --- /dev/null +++ b/lms/djangoapps/course_goals/views.py @@ -0,0 +1,92 @@ +""" +Course Goals Views - includes REST API +""" +from django.contrib.auth import get_user_model +from django.db.models.signals import post_save +from django.dispatch import receiver +from edx_rest_framework_extensions.authentication import JwtAuthentication +from eventtracking import tracker +from opaque_keys.edx.keys import CourseKey +from openedx.core.lib.api.permissions import IsStaffOrOwner +from rest_framework import permissions, serializers, viewsets +from rest_framework.authentication import SessionAuthentication + +from .api import CourseGoalOption +from .models import CourseGoal + +User = get_user_model() + + +class CourseGoalSerializer(serializers.ModelSerializer): + """ + Serializes CourseGoal models. + """ + user = serializers.SlugRelatedField(slug_field='username', queryset=User.objects.all()) + + class Meta: + model = CourseGoal + fields = ('user', 'course_key', 'goal_key') + + def validate_goal_key(self, value): + """ + Ensure that the goal_key is valid. + """ + if value not in CourseGoalOption.get_course_goal_keys(): + raise serializers.ValidationError( + 'Provided goal key, {goal_key}, is not a valid goal key (options= {goal_options}).'.format( + goal_key=value, + goal_options=[option.value for option in CourseGoalOption], + ) + ) + return value + + def validate_course_key(self, value): + """ + Ensure that the course_key is valid. + """ + course_key = CourseKey.from_string(value) + if not course_key: + raise serializers.ValidationError( + 'Provided course_key ({course_key}) does not map to a course.'.format( + course_key=course_key + ) + ) + return course_key + + +class CourseGoalViewSet(viewsets.ModelViewSet): + """ + API calls to create and retrieve a course goal. + + **Use Case** + * Create a new goal for a user. + + Http400 is returned if the format of the request is not correct, + the course_id or goal is invalid or cannot be found. + + * Retrieve goal for a user and a particular course. + + Http400 is returned if the format of the request is not correct, + or the course_id is invalid or cannot be found. + + **Example Requests** + GET /api/course_goals/v0/course_goals/ + POST /api/course_goals/v0/course_goals/ + Request data: {"course_key": , "goal_key": "", "user": ""} + + """ + authentication_classes = (JwtAuthentication, SessionAuthentication,) + permission_classes = (permissions.IsAuthenticated, IsStaffOrOwner,) + queryset = CourseGoal.objects.all() + serializer_class = CourseGoalSerializer + + +@receiver(post_save, sender=CourseGoal, dispatch_uid="emit_course_goals_event") +def emit_course_goal_event(sender, instance, **kwargs): + name = 'edx.course.goal.added' if kwargs.get('created', False) else 'edx.course.goal.updated' + tracker.emit( + name, + { + 'goal_key': instance.goal_key, + } + ) diff --git a/lms/envs/common.py b/lms/envs/common.py index beb140bea5..b9863b88c9 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -401,6 +401,9 @@ FEATURES = { # Whether the bulk enrollment view is enabled. 'ENABLE_BULK_ENROLLMENT_VIEW': False, + + # Whether course goals is enabled. + 'ENABLE_COURSE_GOALS': True, } # Settings for the course reviews tool template and identification key, set either to None to disable course reviews @@ -2245,6 +2248,9 @@ INSTALLED_APPS = [ 'openedx.core.djangoapps.waffle_utils', 'openedx.core.djangoapps.schedules.apps.SchedulesConfig', + # Course Goals + 'lms.djangoapps.course_goals', + # Features 'openedx.features.course_bookmarks', 'openedx.features.course_experience', diff --git a/lms/static/sass/features/_course-experience.scss b/lms/static/sass/features/_course-experience.scss index 48060b0db9..fff4702801 100644 --- a/lms/static/sass/features/_course-experience.scss +++ b/lms/static/sass/features/_course-experience.scss @@ -15,11 +15,12 @@ } .message-content { + @include margin(0, 0, $baseline, $baseline); position: relative; border: 1px solid $lms-border-color; - margin: 0 $baseline $baseline/2; - padding: $baseline/2 $baseline; + padding: $baseline; border-radius: $baseline/4; + width: calc(100% - 90px); @media (max-width: $grid-breakpoints-md) { width: 100%; @@ -30,7 +31,7 @@ &::before { @include left(0); - bottom: 35%; + top: 25px; border: solid transparent; height: 0; width: 0; @@ -58,13 +59,49 @@ .message-header { font-weight: $font-semibold; - margin-bottom: $baseline/4; + margin-bottom: $baseline/2; + width: calc(100% - 40px) } a { font-weight: $font-semibold; text-decoration: underline; } + .dismiss { + @include right($baseline/4); + top: $baseline/4; + position: absolute; + cursor: pointer; + color: $black-t3; + + &:hover { + color: $black-t2; + } + } + // Course Goal Styling + .goal-options-container { + margin-top: $baseline; + text-align: center; + + .goal-option { + text-decoration: none; + font-size: font-size(x-small); + padding: $baseline/2; + + &.dismissible { + @include right($baseline/4); + position: absolute; + top: $baseline/2; + font-size: font-size(small); + color: $uxpl-blue-base; + cursor: pointer; + + &:hover { + color: $black-t2; + } + } + } + } } } diff --git a/lms/templates/navigation/navigation.html b/lms/templates/navigation/navigation.html index 2f1972b527..26d43a9e5f 100644 --- a/lms/templates/navigation/navigation.html +++ b/lms/templates/navigation/navigation.html @@ -50,7 +50,7 @@ site_status_msg = get_site_status_msg(course_id) % if uses_bootstrap: -
+