From 0e590c0f7e6475317c56c47bddfea5f6c4907eb4 Mon Sep 17 00:00:00 2001
From: "Dave St.Germain" <dstgermain@edx.org>
Date: Wed, 24 Apr 2019 09:36:13 -0400
Subject: [PATCH] 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 c1a664fa899..9d90cefcc17 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 23b217eb3d7..f225dc021fe 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 951e99fb5f0..b3675e4f3c4 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 91ccacfbdf6..9eb116bf27e 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
-- 
GitLab