From a5e5591262b18e0c796ed2df888d974b48078e7b Mon Sep 17 00:00:00 2001 From: "Dave St.Germain" Date: Tue, 23 Apr 2019 14:14:32 -0400 Subject: [PATCH 1/3] Automatically add master's to the group access of verified content --- common/djangoapps/course_modes/helpers.py | 39 +++++++++++++++++++ common/djangoapps/course_modes/signals.py | 18 +++++++++ .../course_modes/tests/test_signals.py | 29 +++++++++++++- 3 files changed, 85 insertions(+), 1 deletion(-) diff --git a/common/djangoapps/course_modes/helpers.py b/common/djangoapps/course_modes/helpers.py index a7a04e85e9..1444d1a9ff 100644 --- a/common/djangoapps/course_modes/helpers.py +++ b/common/djangoapps/course_modes/helpers.py @@ -1,14 +1,25 @@ """ Helper methods for CourseModes. """ +from __future__ import absolute_import, unicode_literals +import logging + +from django.conf import settings from django.utils.translation import ugettext_lazy as _ from course_modes.models import CourseMode from student.helpers import VERIFY_STATUS_APPROVED, VERIFY_STATUS_NEED_TO_VERIFY, VERIFY_STATUS_SUBMITTED +from xmodule.partitions.partitions import ENROLLMENT_TRACK_PARTITION_ID + DISPLAY_VERIFIED = "verified" DISPLAY_HONOR = "honor" DISPLAY_AUDIT = "audit" DISPLAY_PROFESSIONAL = "professional" +MASTERS_ID = settings.COURSE_ENROLLMENT_MODES.get('masters', {}).get('id', None) +VERIFIED_ID = settings.COURSE_ENROLLMENT_MODES['verified']['id'] + +log = logging.getLogger(__name__) + def enrollment_mode_display(mode, verification_status, course_id): """ Select appropriate display strings and CSS classes. @@ -80,3 +91,31 @@ def _enrollment_mode_display(enrollment_mode, verification_status, course_id): display_mode = enrollment_mode return display_mode + + +def update_masters_access(item): + """ + Update the XBlock's group access to allow the master's group, + in addition to the verified content group. + """ + group_access = item.group_access + enrollment_groups = group_access.get(ENROLLMENT_TRACK_PARTITION_ID, None) + if enrollment_groups is not None: + if VERIFIED_ID in enrollment_groups and MASTERS_ID not in enrollment_groups: + enrollment_groups.append(MASTERS_ID) + item.group_access = group_access + return True + + +def update_masters_access_course(store, course_id, user_id): + """ + Update all blocks in the verified content group to include the master's content group + """ + + with store.bulk_operations(course_id): + items = store.get_items(course_id, settings={'group_access': {'$exists': True}}, include_orphans=False) + for item in items: + if update_masters_access(item): + log.info("Publishing %s with Master's group access", item.location) + store.update_item(item, user_id) + store.publish(item.location, user_id) diff --git a/common/djangoapps/course_modes/signals.py b/common/djangoapps/course_modes/signals.py index ed8c550211..06f959990a 100644 --- a/common/djangoapps/course_modes/signals.py +++ b/common/djangoapps/course_modes/signals.py @@ -1,11 +1,17 @@ """ Signal handler for setting default course mode expiration dates """ +from __future__ import absolute_import, unicode_literals + +from crum import get_current_user + from django.core.exceptions import ObjectDoesNotExist +from django.db.models.signals import post_save from django.dispatch.dispatcher import receiver from xmodule.modulestore.django import SignalHandler, modulestore +from .helpers import update_masters_access_course from .models import CourseMode, CourseModeExpirationConfig @@ -35,3 +41,15 @@ def _listen_for_course_publish(sender, course_key, **kwargs): # pylint: disable def _should_update_date(verified_mode): """ Returns whether or not the verified mode should be updated. """ return not(verified_mode is None or verified_mode.expiration_datetime_is_explicit) + + +@receiver(post_save, sender=CourseMode) +def update_access_for_masters_mode(sender, instance=None, **kwargs): # pylint: disable=unused-argument + """ + Adds master's access to verified content when the master's mode is created + """ + if instance.mode_slug != CourseMode.MASTERS: + return + user = get_current_user() + user_id = user.id if user else None + update_masters_access_course(modulestore(), instance.course_id, user_id) diff --git a/common/djangoapps/course_modes/tests/test_signals.py b/common/djangoapps/course_modes/tests/test_signals.py index 853622808b..4c7dc66809 100644 --- a/common/djangoapps/course_modes/tests/test_signals.py +++ b/common/djangoapps/course_modes/tests/test_signals.py @@ -1,6 +1,7 @@ """ Unit tests for the course_mode signals """ +from __future__ import absolute_import, unicode_literals from datetime import datetime, timedelta @@ -10,8 +11,9 @@ from pytz import UTC from course_modes.models import CourseMode from course_modes.signals import _listen_for_course_publish +from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase -from xmodule.modulestore.tests.factories import CourseFactory +from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory @ddt.ddt @@ -87,3 +89,28 @@ class CourseModeSignalTest(ModuleStoreTestCase): course_mode.refresh_from_db() self.assertEqual(course_mode.expiration_datetime, self.end - timedelta(days=verification_window)) + + def test_masters_mode(self): + # create an xblock with verified group access + verified_section = ItemFactory.create( + category="sequential", + metadata={'group_access': {50: [2]}} + ) + # and a section with no restriction + section2 = ItemFactory.create( + category="sequential", + ) + section3 = ItemFactory.create( + category='sequential', + metadata={'group_access': {50: [1]}} + ) + with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred): + # create the master's mode. signal will add masters to the verified section + self.create_mode('masters', 'masters') + verified_section_ret = self.store.get_item(verified_section.location) + section2_ret = self.store.get_item(section2.location) + section3_ret = self.store.get_item(section3.location) + # group 2 is verified. 7 is masters + assert verified_section_ret.group_access[50] == [2, 7] + assert section2_ret.group_access == {} + assert section2_ret.group_access == {50: [1]} From 27437bc03f707f4923c9c6c3040129ba2c2add67 Mon Sep 17 00:00:00 2001 From: "Dave St.Germain" Date: Tue, 23 Apr 2019 14:39:24 -0400 Subject: [PATCH 2/3] Ran python-modernize --- common/djangoapps/course_modes/admin.py | 6 ++- .../course_modes/api/serializers.py | 2 + common/djangoapps/course_modes/api/urls.py | 3 +- common/djangoapps/course_modes/api/v1/urls.py | 1 + .../djangoapps/course_modes/api/v1/views.py | 1 + common/djangoapps/course_modes/apps.py | 4 +- common/djangoapps/course_modes/helpers.py | 15 ++++-- .../course_modes/migrations/0001_initial.py | 3 +- ...rsemode_expiration_datetime_is_explicit.py | 2 +- .../migrations/0003_auto_20151113_1443.py | 2 +- .../migrations/0004_auto_20151113_1457.py | 2 +- .../migrations/0005_auto_20151217_0958.py | 2 +- .../migrations/0006_auto_20160208_1407.py | 2 +- .../migrations/0007_coursemode_bulk_sku.py | 2 +- .../0008_course_key_field_to_foreign_key.py | 2 +- .../0009_suggested_prices_to_charfield.py | 5 +- ..._archived_suggested_prices_to_charfield.py | 5 +- ...1_change_regex_for_comma_separated_ints.py | 5 +- common/djangoapps/course_modes/models.py | 11 ++-- common/djangoapps/course_modes/signals.py | 1 - .../course_modes/tests/factories.py | 2 + .../course_modes/tests/test_admin.py | 9 ++-- .../course_modes/tests/test_models.py | 19 +++---- .../course_modes/tests/test_signals.py | 2 +- .../course_modes/tests/test_views.py | 53 ++++++++++--------- common/djangoapps/course_modes/urls.py | 3 ++ common/djangoapps/course_modes/views.py | 19 ++++--- 27 files changed, 110 insertions(+), 73 deletions(-) diff --git a/common/djangoapps/course_modes/admin.py b/common/djangoapps/course_modes/admin.py index 38e6cda823..1b5cdd728d 100644 --- a/common/djangoapps/course_modes/admin.py +++ b/common/djangoapps/course_modes/admin.py @@ -1,3 +1,6 @@ +"""Django admin for course_modes""" +from __future__ import absolute_import, unicode_literals + import six from django import forms from django.conf import settings @@ -22,7 +25,8 @@ from lms.djangoapps.verify_student import models as verification_models from openedx.core.lib.courses import clean_course_id from util.date_utils import get_time_display -COURSE_MODE_SLUG_CHOICES = [(key, enrollment_mode['display_name']) for key, enrollment_mode in six.iteritems(settings.COURSE_ENROLLMENT_MODES)] +COURSE_MODE_SLUG_CHOICES = [(key, enrollment_mode['display_name']) + for key, enrollment_mode in six.iteritems(settings.COURSE_ENROLLMENT_MODES)] class CourseModeForm(forms.ModelForm): diff --git a/common/djangoapps/course_modes/api/serializers.py b/common/djangoapps/course_modes/api/serializers.py index ef3dfe5df7..2df0493b79 100644 --- a/common/djangoapps/course_modes/api/serializers.py +++ b/common/djangoapps/course_modes/api/serializers.py @@ -1,6 +1,8 @@ """ Course modes API serializers. """ +from __future__ import absolute_import + from rest_framework import serializers from course_modes.models import CourseMode diff --git a/common/djangoapps/course_modes/api/urls.py b/common/djangoapps/course_modes/api/urls.py index 3f1d1d4286..ce149243b2 100644 --- a/common/djangoapps/course_modes/api/urls.py +++ b/common/djangoapps/course_modes/api/urls.py @@ -1,8 +1,9 @@ """ URL definitions for the course_modes API. """ -from django.conf.urls import include, url +from __future__ import absolute_import +from django.conf.urls import include, url app_name = 'common.djangoapps.course_modes.api' diff --git a/common/djangoapps/course_modes/api/v1/urls.py b/common/djangoapps/course_modes/api/v1/urls.py index 8456b1f449..c660c788f2 100644 --- a/common/djangoapps/course_modes/api/v1/urls.py +++ b/common/djangoapps/course_modes/api/v1/urls.py @@ -1,6 +1,7 @@ """ URL definitions for the course_modes v1 API. """ +from __future__ import absolute_import from django.conf import settings from django.conf.urls import url diff --git a/common/djangoapps/course_modes/api/v1/views.py b/common/djangoapps/course_modes/api/v1/views.py index 5806b156db..5c61496f92 100644 --- a/common/djangoapps/course_modes/api/v1/views.py +++ b/common/djangoapps/course_modes/api/v1/views.py @@ -2,6 +2,7 @@ Defines the "ReSTful" API for course modes. """ +from __future__ import absolute_import import logging from django.shortcuts import get_object_or_404 diff --git a/common/djangoapps/course_modes/apps.py b/common/djangoapps/course_modes/apps.py index d0b1b2c32c..12f2869139 100644 --- a/common/djangoapps/course_modes/apps.py +++ b/common/djangoapps/course_modes/apps.py @@ -1,3 +1,5 @@ +"""Django App config for course_modes""" +from __future__ import absolute_import from django.apps import AppConfig @@ -7,4 +9,4 @@ class CourseModesConfig(AppConfig): verbose_name = "Course Modes" def ready(self): - import course_modes.signals # pylint: disable=unused-import + import course_modes.signals # pylint: disable=unused-variable diff --git a/common/djangoapps/course_modes/helpers.py b/common/djangoapps/course_modes/helpers.py index 1444d1a9ff..c1a664fa89 100644 --- a/common/djangoapps/course_modes/helpers.py +++ b/common/djangoapps/course_modes/helpers.py @@ -1,15 +1,17 @@ """ Helper methods for CourseModes. """ from __future__ import absolute_import, unicode_literals + import logging +import six from django.conf import settings from django.utils.translation import ugettext_lazy as _ from course_modes.models import CourseMode from student.helpers import VERIFY_STATUS_APPROVED, VERIFY_STATUS_NEED_TO_VERIFY, VERIFY_STATUS_SUBMITTED +from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule.partitions.partitions import ENROLLMENT_TRACK_PARTITION_ID - DISPLAY_VERIFIED = "verified" DISPLAY_HONOR = "honor" DISPLAY_AUDIT = "audit" @@ -59,10 +61,10 @@ def enrollment_mode_display(mode, verification_status, course_id): enrollment_value = _("Professional Ed") return { - 'enrollment_title': unicode(enrollment_title), - 'enrollment_value': unicode(enrollment_value), + 'enrollment_title': six.text_type(enrollment_title), + 'enrollment_value': six.text_type(enrollment_value), 'show_image': show_image, - 'image_alt': unicode(image_alt), + 'image_alt': six.text_type(image_alt), 'display_mode': _enrollment_mode_display(mode, verification_status, course_id) } @@ -113,7 +115,10 @@ def update_masters_access_course(store, course_id, user_id): """ with store.bulk_operations(course_id): - items = store.get_items(course_id, settings={'group_access': {'$exists': True}}, include_orphans=False) + try: + items = store.get_items(course_id, settings={'group_access': {'$exists': True}}, include_orphans=False) + except ItemNotFoundError: + return for item in items: if update_masters_access(item): log.info("Publishing %s with Master's group access", item.location) diff --git a/common/djangoapps/course_modes/migrations/0001_initial.py b/common/djangoapps/course_modes/migrations/0001_initial.py index a440411f6c..11692dcad0 100644 --- a/common/djangoapps/course_modes/migrations/0001_initial.py +++ b/common/djangoapps/course_modes/migrations/0001_initial.py @@ -1,8 +1,7 @@ # -*- coding: utf-8 -*- -from __future__ import unicode_literals +from __future__ import absolute_import, unicode_literals from django.db import migrations, models - from opaque_keys.edx.django.models import CourseKeyField diff --git a/common/djangoapps/course_modes/migrations/0002_coursemode_expiration_datetime_is_explicit.py b/common/djangoapps/course_modes/migrations/0002_coursemode_expiration_datetime_is_explicit.py index ea6a601342..ad010d9a3b 100644 --- a/common/djangoapps/course_modes/migrations/0002_coursemode_expiration_datetime_is_explicit.py +++ b/common/djangoapps/course_modes/migrations/0002_coursemode_expiration_datetime_is_explicit.py @@ -1,5 +1,5 @@ # -*- coding: utf-8 -*- -from __future__ import unicode_literals +from __future__ import absolute_import, unicode_literals from django.db import migrations, models diff --git a/common/djangoapps/course_modes/migrations/0003_auto_20151113_1443.py b/common/djangoapps/course_modes/migrations/0003_auto_20151113_1443.py index 04d20662aa..bca415f6d8 100644 --- a/common/djangoapps/course_modes/migrations/0003_auto_20151113_1443.py +++ b/common/djangoapps/course_modes/migrations/0003_auto_20151113_1443.py @@ -1,5 +1,5 @@ # -*- coding: utf-8 -*- -from __future__ import unicode_literals +from __future__ import absolute_import, unicode_literals from django.db import migrations, models diff --git a/common/djangoapps/course_modes/migrations/0004_auto_20151113_1457.py b/common/djangoapps/course_modes/migrations/0004_auto_20151113_1457.py index ec11036d4d..de21813631 100644 --- a/common/djangoapps/course_modes/migrations/0004_auto_20151113_1457.py +++ b/common/djangoapps/course_modes/migrations/0004_auto_20151113_1457.py @@ -1,5 +1,5 @@ # -*- coding: utf-8 -*- -from __future__ import unicode_literals +from __future__ import absolute_import, unicode_literals from datetime import timedelta diff --git a/common/djangoapps/course_modes/migrations/0005_auto_20151217_0958.py b/common/djangoapps/course_modes/migrations/0005_auto_20151217_0958.py index 1e6e3f7e82..574575749b 100644 --- a/common/djangoapps/course_modes/migrations/0005_auto_20151217_0958.py +++ b/common/djangoapps/course_modes/migrations/0005_auto_20151217_0958.py @@ -1,5 +1,5 @@ # -*- coding: utf-8 -*- -from __future__ import unicode_literals +from __future__ import absolute_import, unicode_literals from django.db import migrations, models diff --git a/common/djangoapps/course_modes/migrations/0006_auto_20160208_1407.py b/common/djangoapps/course_modes/migrations/0006_auto_20160208_1407.py index 338a886745..7355c03817 100644 --- a/common/djangoapps/course_modes/migrations/0006_auto_20160208_1407.py +++ b/common/djangoapps/course_modes/migrations/0006_auto_20160208_1407.py @@ -1,5 +1,5 @@ # -*- coding: utf-8 -*- -from __future__ import unicode_literals +from __future__ import absolute_import, unicode_literals from django.db import migrations, models diff --git a/common/djangoapps/course_modes/migrations/0007_coursemode_bulk_sku.py b/common/djangoapps/course_modes/migrations/0007_coursemode_bulk_sku.py index bdbbac49f2..8a81d980cd 100644 --- a/common/djangoapps/course_modes/migrations/0007_coursemode_bulk_sku.py +++ b/common/djangoapps/course_modes/migrations/0007_coursemode_bulk_sku.py @@ -1,5 +1,5 @@ # -*- coding: utf-8 -*- -from __future__ import unicode_literals +from __future__ import absolute_import, unicode_literals from django.db import migrations, models diff --git a/common/djangoapps/course_modes/migrations/0008_course_key_field_to_foreign_key.py b/common/djangoapps/course_modes/migrations/0008_course_key_field_to_foreign_key.py index 75cb8ed0d7..2ed4bfaf74 100644 --- a/common/djangoapps/course_modes/migrations/0008_course_key_field_to_foreign_key.py +++ b/common/djangoapps/course_modes/migrations/0008_course_key_field_to_foreign_key.py @@ -1,5 +1,5 @@ # -*- coding: utf-8 -*- -from __future__ import unicode_literals +from __future__ import absolute_import, unicode_literals from django.db import migrations, models from opaque_keys.edx.django.models import CourseKeyField diff --git a/common/djangoapps/course_modes/migrations/0009_suggested_prices_to_charfield.py b/common/djangoapps/course_modes/migrations/0009_suggested_prices_to_charfield.py index 60ed720cfd..c6ba8b0b43 100644 --- a/common/djangoapps/course_modes/migrations/0009_suggested_prices_to_charfield.py +++ b/common/djangoapps/course_modes/migrations/0009_suggested_prices_to_charfield.py @@ -1,9 +1,10 @@ # -*- coding: utf-8 -*- -from __future__ import unicode_literals +from __future__ import absolute_import, unicode_literals -from django.db import migrations, models import re + import django.core.validators +from django.db import migrations, models class Migration(migrations.Migration): diff --git a/common/djangoapps/course_modes/migrations/0010_archived_suggested_prices_to_charfield.py b/common/djangoapps/course_modes/migrations/0010_archived_suggested_prices_to_charfield.py index b307d06472..56254117ed 100644 --- a/common/djangoapps/course_modes/migrations/0010_archived_suggested_prices_to_charfield.py +++ b/common/djangoapps/course_modes/migrations/0010_archived_suggested_prices_to_charfield.py @@ -1,9 +1,10 @@ # -*- coding: utf-8 -*- -from __future__ import unicode_literals +from __future__ import absolute_import, unicode_literals -from django.db import migrations, models import re + import django.core.validators +from django.db import migrations, models class Migration(migrations.Migration): diff --git a/common/djangoapps/course_modes/migrations/0011_change_regex_for_comma_separated_ints.py b/common/djangoapps/course_modes/migrations/0011_change_regex_for_comma_separated_ints.py index 73f1046eee..a69f54997d 100644 --- a/common/djangoapps/course_modes/migrations/0011_change_regex_for_comma_separated_ints.py +++ b/common/djangoapps/course_modes/migrations/0011_change_regex_for_comma_separated_ints.py @@ -1,10 +1,11 @@ # -*- coding: utf-8 -*- # Generated by Django 1.11.8 on 2018-01-30 17:38 -from __future__ import unicode_literals +from __future__ import absolute_import, unicode_literals + +import re import django.core.validators from django.db import migrations, models -import re class Migration(migrations.Migration): diff --git a/common/djangoapps/course_modes/models.py b/common/djangoapps/course_modes/models.py index 11a97b24ea..143d67b5a1 100644 --- a/common/djangoapps/course_modes/models.py +++ b/common/djangoapps/course_modes/models.py @@ -1,9 +1,12 @@ """ Add and create new modes for running courses on this particular LMS """ +from __future__ import absolute_import + from collections import defaultdict, namedtuple from datetime import timedelta +import six from config_models.models import ConfigurationModel from django.conf import settings from django.core.exceptions import ValidationError @@ -14,8 +17,8 @@ from django.dispatch import receiver from django.utils.timezone import now from django.utils.translation import ugettext_lazy as _ from edx_django_utils.cache import RequestCache -from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.django.models import CourseKeyField +from opaque_keys.edx.keys import CourseKey from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.lib.cache_utils import request_cached @@ -59,7 +62,7 @@ class CourseMode(models.Model): @course_id.setter def course_id(self, value): - if isinstance(value, basestring): + if isinstance(value, six.string_types): self._course_id = CourseKey.from_string(value) else: self._course_id = value @@ -296,7 +299,7 @@ class CourseMode(models.Model): mode for mode in modes if mode.expiration_datetime is None or mode.expiration_datetime >= now_dt ] - for course_id, modes in all_modes.iteritems() + for course_id, modes in six.iteritems(all_modes) } return (all_modes, unexpired_modes) @@ -910,4 +913,4 @@ class CourseModeExpirationConfig(ConfigurationModel): def __unicode__(self): """ Returns the unicode date of the verification window. """ - return unicode(self.verification_window) + return six.text_type(self.verification_window) diff --git a/common/djangoapps/course_modes/signals.py b/common/djangoapps/course_modes/signals.py index 06f959990a..23b217eb3d 100644 --- a/common/djangoapps/course_modes/signals.py +++ b/common/djangoapps/course_modes/signals.py @@ -4,7 +4,6 @@ Signal handler for setting default course mode expiration dates from __future__ import absolute_import, unicode_literals from crum import get_current_user - from django.core.exceptions import ObjectDoesNotExist from django.db.models.signals import post_save from django.dispatch.dispatcher import receiver diff --git a/common/djangoapps/course_modes/tests/factories.py b/common/djangoapps/course_modes/tests/factories.py index 7983440862..cbffbe05a1 100644 --- a/common/djangoapps/course_modes/tests/factories.py +++ b/common/djangoapps/course_modes/tests/factories.py @@ -1,6 +1,8 @@ """ Factories for course mode models. """ +from __future__ import absolute_import + import random from factory import lazy_attribute diff --git a/common/djangoapps/course_modes/tests/test_admin.py b/common/djangoapps/course_modes/tests/test_admin.py index 0827c5b258..18dbd08a3b 100644 --- a/common/djangoapps/course_modes/tests/test_admin.py +++ b/common/djangoapps/course_modes/tests/test_admin.py @@ -1,10 +1,13 @@ """ Tests for the course modes Django admin interface. """ +from __future__ import absolute_import, unicode_literals + import unittest from datetime import datetime, timedelta import ddt +import six from django.conf import settings from django.urls import reverse from pytz import UTC, timezone @@ -12,12 +15,12 @@ from pytz import UTC, timezone from course_modes.admin import CourseModeForm from course_modes.models import CourseMode from course_modes.tests.factories import CourseModeFactory -from openedx.core.djangoapps.content.course_overviews.models import CourseOverview # Technically, we shouldn't be importing verify_student, since it's # defined in the LMS and course_modes is in common. However, the benefits # of putting all this configuration in one place outweigh the downsides. # Once the course admin tool is deployed, we can remove this dependency. from lms.djangoapps.verify_student.models import VerificationDeadline +from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from student.tests.factories import UserFactory from util.date_utils import get_time_display from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase @@ -44,7 +47,7 @@ class AdminCourseModePageTest(ModuleStoreTestCase): CourseOverview.load_from_module_store(course.id) data = { - 'course': unicode(course.id), + 'course': six.text_type(course.id), 'mode_slug': 'verified', 'mode_display_name': 'verified', 'min_price': 10, @@ -199,7 +202,7 @@ class AdminCourseModeFormTest(ModuleStoreTestCase): mode_slug=mode, ) return CourseModeForm({ - "course": unicode(self.course.id), + "course": six.text_type(self.course.id), "mode_slug": mode, "mode_display_name": mode, "_expiration_datetime": upgrade_deadline, diff --git a/common/djangoapps/course_modes/tests/test_models.py b/common/djangoapps/course_modes/tests/test_models.py index 6055be65e8..de4b12ea6f 100644 --- a/common/djangoapps/course_modes/tests/test_models.py +++ b/common/djangoapps/course_modes/tests/test_models.py @@ -4,6 +4,7 @@ when you run "manage.py test". Replace this with more appropriate tests for your application. """ +from __future__ import absolute_import, unicode_literals import itertools from datetime import timedelta @@ -14,14 +15,13 @@ from django.test import TestCase, override_settings from django.utils.timezone import now from mock import patch from opaque_keys.edx.locator import CourseLocator +from six.moves import zip from course_modes.helpers import enrollment_mode_display -from course_modes.models import CourseMode, Mode, invalidate_course_mode_cache, get_cosmetic_display_price +from course_modes.models import CourseMode, Mode, get_cosmetic_display_price, invalidate_course_mode_cache from course_modes.tests.factories import CourseModeFactory +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory -from xmodule.modulestore.tests.django_utils import ( - ModuleStoreTestCase, -) @ddt.ddt @@ -41,6 +41,7 @@ class CourseModeModelTest(TestCase): CourseMode.objects.all().delete() def tearDown(self): + super(CourseModeModelTest, self).tearDown() invalidate_course_mode_cache(sender=None) def create_mode( @@ -398,11 +399,11 @@ class CourseModeModelTest(TestCase): # Check the selectable modes, which should exclude credit selectable_modes = CourseMode.modes_for_course_dict(self.course_key) - self.assertItemsEqual(selectable_modes.keys(), expected_selectable_modes) + self.assertItemsEqual(list(selectable_modes.keys()), expected_selectable_modes) # When we get all unexpired modes, we should see credit as well all_modes = CourseMode.modes_for_course_dict(self.course_key, only_selectable=False) - self.assertItemsEqual(all_modes.keys(), available_modes) + self.assertItemsEqual(list(all_modes.keys()), available_modes) def _enrollment_display_modes_dicts(self, dict_type): """ @@ -421,11 +422,11 @@ class CourseModeModelTest(TestCase): 'professional'] } if dict_type in ['verify_need_to_verify', 'verify_submitted']: - return dict(zip(dict_keys, display_values.get('verify_need_to_verify'))) + return dict(list(zip(dict_keys, display_values.get('verify_need_to_verify')))) elif dict_type is None or dict_type == 'dummy': - return dict(zip(dict_keys, display_values.get('verify_none'))) + return dict(list(zip(dict_keys, display_values.get('verify_none')))) else: - return dict(zip(dict_keys, display_values.get(dict_type))) + return dict(list(zip(dict_keys, display_values.get(dict_type)))) def test_expiration_datetime_explicitly_set(self): """ Verify that setting the expiration_date property sets the explicit flag. """ diff --git a/common/djangoapps/course_modes/tests/test_signals.py b/common/djangoapps/course_modes/tests/test_signals.py index 4c7dc66809..951e99fb5f 100644 --- a/common/djangoapps/course_modes/tests/test_signals.py +++ b/common/djangoapps/course_modes/tests/test_signals.py @@ -113,4 +113,4 @@ class CourseModeSignalTest(ModuleStoreTestCase): # group 2 is verified. 7 is masters assert verified_section_ret.group_access[50] == [2, 7] assert section2_ret.group_access == {} - assert section2_ret.group_access == {50: [1]} + assert section3_ret.group_access == {50: [1]} diff --git a/common/djangoapps/course_modes/tests/test_views.py b/common/djangoapps/course_modes/tests/test_views.py index fed9623f8f..d836b4ea80 100644 --- a/common/djangoapps/course_modes/tests/test_views.py +++ b/common/djangoapps/course_modes/tests/test_views.py @@ -2,6 +2,8 @@ Tests for course_modes views. """ +from __future__ import absolute_import + import decimal import unittest from datetime import datetime, timedelta @@ -10,6 +12,7 @@ import ddt import freezegun import httpretty import pytz +import six from django.conf import settings from django.urls import reverse from mock import patch @@ -84,7 +87,7 @@ class CourseModeViewTest(CatalogIntegrationMixin, UrlResetMixin, ModuleStoreTest ) # Configure whether we're upgrading or not - url = reverse('course_modes_choose', args=[unicode(course.id)]) + url = reverse('course_modes_choose', args=[six.text_type(course.id)]) response = self.client.get(url) # Check whether we were correctly redirected @@ -111,11 +114,11 @@ class CourseModeViewTest(CatalogIntegrationMixin, UrlResetMixin, ModuleStoreTest ) # Configure whether we're upgrading or not - url = reverse('course_modes_choose', args=[unicode(self.course.id)]) + url = reverse('course_modes_choose', args=[six.text_type(self.course.id)]) response = self.client.get(url) # Check whether we were correctly redirected purchase_workflow = "?purchase_workflow=single" - start_flow_url = reverse('verify_student_start_flow', args=[unicode(self.course.id)]) + purchase_workflow + start_flow_url = reverse('verify_student_start_flow', args=[six.text_type(self.course.id)]) + purchase_workflow self.assertRedirects(response, start_flow_url) def test_no_id_redirect_otto(self): @@ -132,7 +135,7 @@ class CourseModeViewTest(CatalogIntegrationMixin, UrlResetMixin, ModuleStoreTest user=self.user ) # Configure whether we're upgrading or not - url = reverse('course_modes_choose', args=[unicode(prof_course.id)]) + url = reverse('course_modes_choose', args=[six.text_type(prof_course.id)]) response = self.client.get(url) self.assertRedirects(response, 'http://testserver/test_basket/add/?sku=TEST', fetch_redirect_response=False) ecomm_test_utils.update_commerce_config(enabled=False) @@ -166,7 +169,7 @@ class CourseModeViewTest(CatalogIntegrationMixin, UrlResetMixin, ModuleStoreTest # Verify that the prices render correctly response = self.client.get( - reverse('course_modes_choose', args=[unicode(self.course.id)]), + reverse('course_modes_choose', args=[six.text_type(self.course.id)]), follow=False, ) @@ -187,7 +190,7 @@ class CourseModeViewTest(CatalogIntegrationMixin, UrlResetMixin, ModuleStoreTest # Check whether credit upsell is shown on the page # This should *only* be shown when a credit mode is available - url = reverse('course_modes_choose', args=[unicode(self.course.id)]) + url = reverse('course_modes_choose', args=[six.text_type(self.course.id)]) response = self.client.get(url) if show_upsell: @@ -201,13 +204,13 @@ class CourseModeViewTest(CatalogIntegrationMixin, UrlResetMixin, ModuleStoreTest CourseModeFactory.create(mode_slug=mode, course_id=self.course.id, min_price=1) # Go to the "choose your track" page - choose_track_url = reverse('course_modes_choose', args=[unicode(self.course.id)]) + choose_track_url = reverse('course_modes_choose', args=[six.text_type(self.course.id)]) response = self.client.get(choose_track_url) # Since the only available track is professional ed, expect that # we're redirected immediately to the start of the payment flow. purchase_workflow = "?purchase_workflow=single" - start_flow_url = reverse('verify_student_start_flow', args=[unicode(self.course.id)]) + purchase_workflow + start_flow_url = reverse('verify_student_start_flow', args=[six.text_type(self.course.id)]) + purchase_workflow self.assertRedirects(response, start_flow_url) # Now enroll in the course @@ -215,7 +218,7 @@ class CourseModeViewTest(CatalogIntegrationMixin, UrlResetMixin, ModuleStoreTest user=self.user, is_active=True, mode=mode, - course_id=unicode(self.course.id), + course_id=six.text_type(self.course.id), ) # Expect that this time we're redirected to the dashboard (since we're already registered) @@ -244,7 +247,7 @@ class CourseModeViewTest(CatalogIntegrationMixin, UrlResetMixin, ModuleStoreTest CourseModeFactory.create(mode_slug=mode, course_id=self.course.id, min_price=min_price) # Choose the mode (POST request) - choose_track_url = reverse('course_modes_choose', args=[unicode(self.course.id)]) + choose_track_url = reverse('course_modes_choose', args=[six.text_type(self.course.id)]) response = self.client.post(choose_track_url, self.POST_PARAMS_FOR_COURSE_MODE[course_mode]) # Verify the redirect @@ -253,7 +256,7 @@ class CourseModeViewTest(CatalogIntegrationMixin, UrlResetMixin, ModuleStoreTest elif expected_redirect == 'start-flow': redirect_url = reverse( 'verify_student_start_flow', - kwargs={'course_id': unicode(self.course.id)} + kwargs={'course_id': six.text_type(self.course.id)} ) else: self.fail("Must provide a valid redirect URL name") @@ -273,7 +276,7 @@ class CourseModeViewTest(CatalogIntegrationMixin, UrlResetMixin, ModuleStoreTest self.assertIsNone(is_active) # Choose the audit mode (POST request) - choose_track_url = reverse('course_modes_choose', args=[unicode(self.course.id)]) + choose_track_url = reverse('course_modes_choose', args=[six.text_type(self.course.id)]) self.client.post(choose_track_url, self.POST_PARAMS_FOR_COURSE_MODE[audit_mode]) # Assert learner is enrolled in Audit track post-POST @@ -301,14 +304,14 @@ class CourseModeViewTest(CatalogIntegrationMixin, UrlResetMixin, ModuleStoreTest CourseModeFactory.create(mode_slug='verified', course_id=self.course.id, min_price=1) # Choose the mode (POST request) - choose_track_url = reverse('course_modes_choose', args=[unicode(self.course.id)]) + choose_track_url = reverse('course_modes_choose', args=[six.text_type(self.course.id)]) self.client.post(choose_track_url, self.POST_PARAMS_FOR_COURSE_MODE['verified']) # Expect that the contribution amount is stored in the user's session self.assertIn('donation_for_course', self.client.session) - self.assertIn(unicode(self.course.id), self.client.session['donation_for_course']) + self.assertIn(six.text_type(self.course.id), self.client.session['donation_for_course']) - actual_amount = self.client.session['donation_for_course'][unicode(self.course.id)] + actual_amount = self.client.session['donation_for_course'][six.text_type(self.course.id)] expected_amount = decimal.Decimal(self.POST_PARAMS_FOR_COURSE_MODE['verified']['contribution']) self.assertEqual(actual_amount, expected_amount) @@ -321,12 +324,12 @@ class CourseModeViewTest(CatalogIntegrationMixin, UrlResetMixin, ModuleStoreTest # automatic enrollment params = { 'enrollment_action': 'enroll', - 'course_id': unicode(self.course.id) + 'course_id': six.text_type(self.course.id) } self.client.post(reverse('change_enrollment'), params) # Explicitly select the honor mode (POST request) - choose_track_url = reverse('course_modes_choose', args=[unicode(self.course.id)]) + choose_track_url = reverse('course_modes_choose', args=[six.text_type(self.course.id)]) self.client.post(choose_track_url, self.POST_PARAMS_FOR_COURSE_MODE[CourseMode.DEFAULT_MODE_SLUG]) # Verify that the user's enrollment remains unchanged @@ -340,7 +343,7 @@ class CourseModeViewTest(CatalogIntegrationMixin, UrlResetMixin, ModuleStoreTest CourseModeFactory.create(mode_slug=mode, course_id=self.course.id) # Choose an unsupported mode (POST request) - choose_track_url = reverse('course_modes_choose', args=[unicode(self.course.id)]) + choose_track_url = reverse('course_modes_choose', args=[six.text_type(self.course.id)]) response = self.client.post(choose_track_url, self.POST_PARAMS_FOR_COURSE_MODE['unsupported']) self.assertEqual(400, response.status_code) @@ -348,7 +351,7 @@ class CourseModeViewTest(CatalogIntegrationMixin, UrlResetMixin, ModuleStoreTest @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') def test_default_mode_creation(self): # Hit the mode creation endpoint with no querystring params, to create an honor mode - url = reverse('create_mode', args=[unicode(self.course.id)]) + url = reverse('create_mode', args=[six.text_type(self.course.id)]) response = self.client.get(url) self.assertEquals(response.status_code, 200) @@ -372,7 +375,7 @@ class CourseModeViewTest(CatalogIntegrationMixin, UrlResetMixin, ModuleStoreTest parameters['suggested_prices'] = suggested_prices parameters['currency'] = currency - url = reverse('create_mode', args=[unicode(self.course.id)]) + url = reverse('create_mode', args=[six.text_type(self.course.id)]) response = self.client.get(url, parameters) self.assertEquals(response.status_code, 200) @@ -397,7 +400,7 @@ class CourseModeViewTest(CatalogIntegrationMixin, UrlResetMixin, ModuleStoreTest @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') def test_multiple_mode_creation(self): # Create an honor mode - base_url = reverse('create_mode', args=[unicode(self.course.id)]) + base_url = reverse('create_mode', args=[six.text_type(self.course.id)]) self.client.get(base_url) # Excluding the currency parameter implicitly tests the mode creation endpoint's ability to @@ -409,7 +412,7 @@ class CourseModeViewTest(CatalogIntegrationMixin, UrlResetMixin, ModuleStoreTest parameters['suggested_prices'] = '10,20' # Create a verified mode - url = reverse('create_mode', args=[unicode(self.course.id)]) + url = reverse('create_mode', args=[six.text_type(self.course.id)]) self.client.get(url, parameters) honor_mode = Mode(u'honor', u'Honor Code Certificate', 0, '', 'usd', None, None, None, None) @@ -428,7 +431,7 @@ class CourseModeViewTest(CatalogIntegrationMixin, UrlResetMixin, ModuleStoreTest CourseModeFactory.create(mode_slug=mode, course_id=self.course.id) # Load the track selection page - url = reverse('course_modes_choose', args=[unicode(self.course.id)]) + url = reverse('course_modes_choose', args=[six.text_type(self.course.id)]) response = self.client.get(url) # Verify that the header navigation links are hidden for the edx.org version @@ -445,7 +448,7 @@ class CourseModeViewTest(CatalogIntegrationMixin, UrlResetMixin, ModuleStoreTest self.course.enrollment_end = datetime(2015, 1, 1) modulestore().update_item(self.course, self.user.id) - url = reverse('course_modes_choose', args=[unicode(self.course.id)]) + url = reverse('course_modes_choose', args=[six.text_type(self.course.id)]) response = self.client.get(url) # URL-encoded version of 1/1/15, 12:00 AM redirect_url = reverse('dashboard') + '?course_closed=1%2F1%2F15%2C+12%3A00+AM' @@ -472,7 +475,7 @@ class TrackSelectionEmbargoTest(UrlResetMixin, ModuleStoreTestCase): self.client.login(username=self.user.username, password="edx") # Construct the URL for the track selection page - self.url = reverse('course_modes_choose', args=[unicode(self.course.id)]) + self.url = reverse('course_modes_choose', args=[six.text_type(self.course.id)]) @patch.dict(settings.FEATURES, {'EMBARGO': True}) def test_embargo_restrict(self): diff --git a/common/djangoapps/course_modes/urls.py b/common/djangoapps/course_modes/urls.py index 5253c164b2..96b8d1e4fe 100644 --- a/common/djangoapps/course_modes/urls.py +++ b/common/djangoapps/course_modes/urls.py @@ -1,3 +1,6 @@ +"""URLs for course_mode API""" +from __future__ import absolute_import, unicode_literals + from django.conf import settings from django.conf.urls import url diff --git a/common/djangoapps/course_modes/views.py b/common/djangoapps/course_modes/views.py index 536c60bc0d..91ccacfbdf 100644 --- a/common/djangoapps/course_modes/views.py +++ b/common/djangoapps/course_modes/views.py @@ -1,10 +1,15 @@ """ Views for the course_mode module """ +from __future__ import absolute_import, unicode_literals import decimal import json -import urllib + +import six +import six.moves.urllib.error +import six.moves.urllib.parse +import six.moves.urllib.request import waffle from babel.dates import format_datetime @@ -96,7 +101,7 @@ class ChooseModeView(View): has_enrolled_professional = (CourseMode.is_professional_slug(enrollment_mode) and is_active) if CourseMode.has_professional_mode(modes) and not has_enrolled_professional: purchase_workflow = request.GET.get("purchase_workflow", "single") - verify_url = reverse('verify_student_start_flow', kwargs={'course_id': unicode(course_key)}) + verify_url = reverse('verify_student_start_flow', kwargs={'course_id': six.text_type(course_key)}) redirect_url = "{url}?purchase_workflow={workflow}".format(url=verify_url, workflow=purchase_workflow) if ecommerce_service.is_enabled(request.user): professional_mode = modes.get(CourseMode.NO_ID_PROFESSIONAL_MODE) or modes.get(CourseMode.PROFESSIONAL) @@ -121,12 +126,12 @@ class ChooseModeView(View): return redirect(reverse('dashboard')) donation_for_course = request.session.get("donation_for_course", {}) - chosen_price = donation_for_course.get(unicode(course_key), None) + chosen_price = donation_for_course.get(six.text_type(course_key), None) if CourseEnrollment.is_enrollment_closed(request.user, course): locale = to_locale(get_language()) enrollment_end_date = format_datetime(course.enrollment_end, 'short', locale=locale) - params = urllib.urlencode({'course_closed': enrollment_end_date}) + params = six.moves.urllib.parse.urlencode({'course_closed': enrollment_end_date}) return redirect('{0}?{1}'.format(reverse('dashboard'), params)) # When a credit mode is available, students will be given the option @@ -278,13 +283,13 @@ class ChooseModeView(View): return self.get(request, course_id, error=error_msg) donation_for_course = request.session.get("donation_for_course", {}) - donation_for_course[unicode(course_key)] = amount_value + donation_for_course[six.text_type(course_key)] = amount_value request.session["donation_for_course"] = donation_for_course return redirect( reverse( 'verify_student_start_flow', - kwargs={'course_id': unicode(course_key)} + kwargs={'course_id': six.text_type(course_key)} ) ) @@ -342,7 +347,7 @@ def create_mode(request, course_id): } # Try pulling querystring parameters out of the request - for parameter, default in PARAMETERS.iteritems(): + for parameter, default in six.iteritems(PARAMETERS): PARAMETERS[parameter] = request.GET.get(parameter, default) # Attempt to create the new mode for the given course From 0e590c0f7e6475317c56c47bddfea5f6c4907eb4 Mon Sep 17 00:00:00 2001 From: "Dave St.Germain" Date: Wed, 24 Apr 2019 09:36:13 -0400 Subject: [PATCH 3/3] Moved these functions out of helpers because they're not needed elsewhere --- common/djangoapps/course_modes/helpers.py | 41 ------------------- common/djangoapps/course_modes/signals.py | 37 +++++++++++++++-- .../course_modes/tests/test_signals.py | 15 ++++--- common/djangoapps/course_modes/views.py | 1 - 4 files changed, 43 insertions(+), 51 deletions(-) diff --git a/common/djangoapps/course_modes/helpers.py b/common/djangoapps/course_modes/helpers.py index c1a664fa89..9d90cefcc1 100644 --- a/common/djangoapps/course_modes/helpers.py +++ b/common/djangoapps/course_modes/helpers.py @@ -1,27 +1,17 @@ """ Helper methods for CourseModes. """ from __future__ import absolute_import, unicode_literals -import logging - import six -from django.conf import settings from django.utils.translation import ugettext_lazy as _ from course_modes.models import CourseMode from student.helpers import VERIFY_STATUS_APPROVED, VERIFY_STATUS_NEED_TO_VERIFY, VERIFY_STATUS_SUBMITTED -from xmodule.modulestore.exceptions import ItemNotFoundError -from xmodule.partitions.partitions import ENROLLMENT_TRACK_PARTITION_ID DISPLAY_VERIFIED = "verified" DISPLAY_HONOR = "honor" DISPLAY_AUDIT = "audit" DISPLAY_PROFESSIONAL = "professional" -MASTERS_ID = settings.COURSE_ENROLLMENT_MODES.get('masters', {}).get('id', None) -VERIFIED_ID = settings.COURSE_ENROLLMENT_MODES['verified']['id'] - -log = logging.getLogger(__name__) - def enrollment_mode_display(mode, verification_status, course_id): """ Select appropriate display strings and CSS classes. @@ -93,34 +83,3 @@ def _enrollment_mode_display(enrollment_mode, verification_status, course_id): display_mode = enrollment_mode return display_mode - - -def update_masters_access(item): - """ - Update the XBlock's group access to allow the master's group, - in addition to the verified content group. - """ - group_access = item.group_access - enrollment_groups = group_access.get(ENROLLMENT_TRACK_PARTITION_ID, None) - if enrollment_groups is not None: - if VERIFIED_ID in enrollment_groups and MASTERS_ID not in enrollment_groups: - enrollment_groups.append(MASTERS_ID) - item.group_access = group_access - return True - - -def update_masters_access_course(store, course_id, user_id): - """ - Update all blocks in the verified content group to include the master's content group - """ - - with store.bulk_operations(course_id): - try: - items = store.get_items(course_id, settings={'group_access': {'$exists': True}}, include_orphans=False) - except ItemNotFoundError: - return - for item in items: - if update_masters_access(item): - log.info("Publishing %s with Master's group access", item.location) - store.update_item(item, user_id) - store.publish(item.location, user_id) diff --git a/common/djangoapps/course_modes/signals.py b/common/djangoapps/course_modes/signals.py index 23b217eb3d..f225dc021f 100644 --- a/common/djangoapps/course_modes/signals.py +++ b/common/djangoapps/course_modes/signals.py @@ -3,16 +3,22 @@ Signal handler for setting default course mode expiration dates """ from __future__ import absolute_import, unicode_literals +import logging + from crum import get_current_user +from django.conf import settings from django.core.exceptions import ObjectDoesNotExist from django.db.models.signals import post_save from django.dispatch.dispatcher import receiver from xmodule.modulestore.django import SignalHandler, modulestore +from xmodule.modulestore.exceptions import ItemNotFoundError +from xmodule.partitions.partitions import ENROLLMENT_TRACK_PARTITION_ID -from .helpers import update_masters_access_course from .models import CourseMode, CourseModeExpirationConfig +log = logging.getLogger(__name__) + @receiver(SignalHandler.course_published) def _listen_for_course_publish(sender, course_key, **kwargs): # pylint: disable=unused-argument @@ -43,12 +49,35 @@ def _should_update_date(verified_mode): @receiver(post_save, sender=CourseMode) -def update_access_for_masters_mode(sender, instance=None, **kwargs): # pylint: disable=unused-argument +def update_masters_access_course(sender, instance, **kwargs): # pylint: disable=unused-argument """ - Adds master's access to verified content when the master's mode is created + Update all blocks in the verified content group to include the master's content group """ if instance.mode_slug != CourseMode.MASTERS: return + masters_id = getattr(settings, 'COURSE_ENROLLMENT_MODES', {}).get('masters', {}).get('id', None) + verified_id = getattr(settings, 'COURSE_ENROLLMENT_MODES', {}).get('verified', {}).get('id', None) + if not (masters_id and verified_id): + log.error("Missing settings.COURSE_ENROLLMENT_MODES -> verified:%s masters:%s", verified, masters) + return + + course_id = instance.course_id user = get_current_user() user_id = user.id if user else None - update_masters_access_course(modulestore(), instance.course_id, user_id) + store = modulestore() + + with store.bulk_operations(course_id): + try: + items = store.get_items(course_id, settings={'group_access': {'$exists': True}}, include_orphans=False) + except ItemNotFoundError: + return + for item in items: + group_access = item.group_access + enrollment_groups = group_access.get(ENROLLMENT_TRACK_PARTITION_ID, None) + if enrollment_groups is not None: + if verified_id in enrollment_groups and masters_id not in enrollment_groups: + enrollment_groups.append(masters_id) + item.group_access = group_access + log.info("Publishing %s with Master's group access", item.location) + store.update_item(item, user_id) + store.publish(item.location, user_id) diff --git a/common/djangoapps/course_modes/tests/test_signals.py b/common/djangoapps/course_modes/tests/test_signals.py index 951e99fb5f..b3675e4f3c 100644 --- a/common/djangoapps/course_modes/tests/test_signals.py +++ b/common/djangoapps/course_modes/tests/test_signals.py @@ -11,9 +11,11 @@ from pytz import UTC from course_modes.models import CourseMode from course_modes.signals import _listen_for_course_publish +from django.conf import settings from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory +from xmodule.partitions.partitions import ENROLLMENT_TRACK_PARTITION_ID @ddt.ddt @@ -92,9 +94,12 @@ class CourseModeSignalTest(ModuleStoreTestCase): def test_masters_mode(self): # create an xblock with verified group access + AUDIT_ID = settings.COURSE_ENROLLMENT_MODES['audit']['id'] + VERIFIED_ID = settings.COURSE_ENROLLMENT_MODES['verified']['id'] + MASTERS_ID = settings.COURSE_ENROLLMENT_MODES['masters']['id'] verified_section = ItemFactory.create( category="sequential", - metadata={'group_access': {50: [2]}} + metadata={'group_access': {ENROLLMENT_TRACK_PARTITION_ID: [VERIFIED_ID]}} ) # and a section with no restriction section2 = ItemFactory.create( @@ -102,7 +107,7 @@ class CourseModeSignalTest(ModuleStoreTestCase): ) section3 = ItemFactory.create( category='sequential', - metadata={'group_access': {50: [1]}} + metadata={'group_access': {ENROLLMENT_TRACK_PARTITION_ID: [AUDIT_ID]}} ) with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred): # create the master's mode. signal will add masters to the verified section @@ -110,7 +115,7 @@ class CourseModeSignalTest(ModuleStoreTestCase): verified_section_ret = self.store.get_item(verified_section.location) section2_ret = self.store.get_item(section2.location) section3_ret = self.store.get_item(section3.location) - # group 2 is verified. 7 is masters - assert verified_section_ret.group_access[50] == [2, 7] + # the verified section will now also be visible to master's + assert verified_section_ret.group_access[ENROLLMENT_TRACK_PARTITION_ID] == [VERIFIED_ID, MASTERS_ID] assert section2_ret.group_access == {} - assert section3_ret.group_access == {50: [1]} + assert section3_ret.group_access == {ENROLLMENT_TRACK_PARTITION_ID: [AUDIT_ID]} diff --git a/common/djangoapps/course_modes/views.py b/common/djangoapps/course_modes/views.py index 91ccacfbdf..9eb116bf27 100644 --- a/common/djangoapps/course_modes/views.py +++ b/common/djangoapps/course_modes/views.py @@ -10,7 +10,6 @@ import six import six.moves.urllib.error import six.moves.urllib.parse import six.moves.urllib.request - import waffle from babel.dates import format_datetime from django.contrib.auth.decorators import login_required