Merge pull request #5876 from edx/cdodge/catalog-visibility

Add new course_module field which describes what the catalog visibility ...

common/lib/xmodule/xmodule/course_module.py

@@ -24,6 +24,9 @@ _ = lambda text: text
 
 DEFAULT_START_DATE = datetime(2030, 1, 1, tzinfo=UTC())
 
+CATALOG_VISIBILITY_CATALOG_AND_ABOUT = "both"
+CATALOG_VISIBILITY_ABOUT = "about"
+CATALOG_VISIBILITY_NONE = "none"
 
 class StringOrDate(Date):
     def from_json(self, value):
@@ -575,6 +578,17 @@ class CourseFields(object):
         deprecated=True
     )
 
+    catalog_visibility = String(
+        display_name=_("Course Visibility In Catalog"),
+        help=_("Defines the access permissions for showing the course in the course catalog. This can be set to one of three values: 'both' (show in catalog and allow access to about page), 'about' (only allow access to about page), 'none' (do not show in catalog and do not allow access to an about page)."),
+        default=CATALOG_VISIBILITY_CATALOG_AND_ABOUT,
+        scope=Scope.settings,
+        values=[
+            {"display_name": _("Both"), "value": CATALOG_VISIBILITY_CATALOG_AND_ABOUT},
+            {"display_name": _("About"), "value": CATALOG_VISIBILITY_ABOUT},
+            {"display_name": _("None"), "value": CATALOG_VISIBILITY_NONE}]
+    )
+
 class CourseDescriptor(CourseFields, SequenceDescriptor):
     module_class = SequenceModule
 

lms/djangoapps/courseware/access.py

@@ -8,7 +8,9 @@ import pytz
 from django.conf import settings
 from django.contrib.auth.models import AnonymousUser
 
-from xmodule.course_module import CourseDescriptor
+from xmodule.course_module import (
+    CourseDescriptor, CATALOG_VISIBILITY_CATALOG_AND_ABOUT,
+    CATALOG_VISIBILITY_ABOUT)
 from xmodule.error_module import ErrorDescriptor
 from xmodule.x_module import XModule
 
@@ -110,6 +112,8 @@ def _has_access_course_desc(user, action, course):
                   ACCESS_REQUIRE_STAFF_FOR_COURSE,
     'see_exists' -- can see that the course exists.
     'staff' -- staff access to course.
+    'see_in_catalog' -- user is able to see the course listed in the course catalog.
+    'see_about_page' -- user is able to see the course about page.
     """
     def can_load():
         """
@@ -204,6 +208,29 @@ def _has_access_course_desc(user, action, course):
 
         return can_enroll() or can_load()
 
+    def can_see_in_catalog():
+        """
+        Implements the "can see course in catalog" logic if a course should be visible in the main course catalog
+        In this case we use the catalog_visibility property on the course descriptor
+        but also allow course staff to see this.
+        """
+        return (
+            course.catalog_visibility == CATALOG_VISIBILITY_CATALOG_AND_ABOUT or
+            _has_staff_access_to_descriptor(user, course, course.id)
+        )
+
+    def can_see_about_page():
+        """
+        Implements the "can see course about page" logic if a course about page should be visible
+        In this case we use the catalog_visibility property on the course descriptor
+        but also allow course staff to see this.
+        """
+        return (
+            course.catalog_visibility == CATALOG_VISIBILITY_CATALOG_AND_ABOUT or
+            course.catalog_visibility == CATALOG_VISIBILITY_ABOUT or
+            _has_staff_access_to_descriptor(user, course, course.id)
+        )
+
     checkers = {
         'load': can_load,
         'load_forum': can_load_forum,
@@ -211,6 +238,8 @@ def _has_access_course_desc(user, action, course):
         'see_exists': see_exists,
         'staff': lambda: _has_staff_access_to_descriptor(user, course, course.id),
         'instructor': lambda: _has_instructor_access_to_descriptor(user, course, course.id),
+        'see_in_catalog': can_see_in_catalog,
+        'see_about_page': can_see_about_page,
     }
 
     return _dispatch(checkers, action, user, course)

lms/djangoapps/courseware/courses.py

@@ -16,6 +16,7 @@ from xmodule.modulestore.exceptions import ItemNotFoundError
 from static_replace import replace_static_urls
 from xmodule.modulestore import ModuleStoreEnum
 from xmodule.x_module import STUDENT_VIEW
+from microsite_configuration import microsite
 
 from courseware.access import has_access
 from courseware.model_data import FieldDataCache
@@ -256,7 +257,7 @@ def get_course_info_section_module(request, course, section_key):
         log_if_not_found=False,
         wrap_xmodule_display=False,
         static_asset_path=course.static_asset_path
-    )    
+    )
 
 def get_course_info_section(request, course, section_key):
     """
@@ -345,7 +346,13 @@ def get_courses(user, domain=None):
     Returns a list of courses available, sorted by course.number
     '''
     courses = branding.get_visible_courses()
-    courses = [c for c in courses if has_access(user, 'see_exists', c)]
+
+    permission_name = microsite.get_value(
+        'COURSE_CATALOG_VISIBILITY_PERMISSION',
+        settings.COURSE_CATALOG_VISIBILITY_PERMISSION
+    )
+
+    courses = [c for c in courses if has_access(user, permission_name, c)]
 
     courses = sorted(courses, key=lambda course: course.number)
 

lms/djangoapps/courseware/tests/test_access.py

@@ -11,8 +11,12 @@ from student.tests.factories import AnonymousUserFactory, CourseEnrollmentAllowe
 import pytz
 from opaque_keys.edx.locations import SlashSeparatedCourseKey
 
-# pylint: disable=C0111
+from xmodule.course_module import (
+    CATALOG_VISIBILITY_CATALOG_AND_ABOUT, CATALOG_VISIBILITY_ABOUT,
+    CATALOG_VISIBILITY_NONE)
 
+# pylint: disable=C0111
+# pylint: disable=W0212
 
 class AccessTestCase(TestCase):
     """
@@ -202,6 +206,43 @@ class AccessTestCase(TestCase):
         """Ensure has_access handles a user being passed as null"""
         access.has_access(None, 'staff', 'global', None)
 
+    def test__catalog_visibility(self):
+        """
+        Tests the catalog visibility tri-states
+        """
+        user = UserFactory.create()
+        course_id = SlashSeparatedCourseKey('edX', 'test', '2012_Fall')
+        staff = StaffFactory.create(course_key=course_id)
+
+        course = Mock(
+            id=course_id,
+            catalog_visibility=CATALOG_VISIBILITY_CATALOG_AND_ABOUT
+        )
+        self.assertTrue(access._has_access_course_desc(user, 'see_in_catalog', course))
+        self.assertTrue(access._has_access_course_desc(user, 'see_about_page', course))
+        self.assertTrue(access._has_access_course_desc(staff, 'see_in_catalog', course))
+        self.assertTrue(access._has_access_course_desc(staff, 'see_about_page', course))
+
+        # Now set visibility to just about page
+        course = Mock(
+            id=SlashSeparatedCourseKey('edX', 'test', '2012_Fall'),
+            catalog_visibility=CATALOG_VISIBILITY_ABOUT
+        )
+        self.assertFalse(access._has_access_course_desc(user, 'see_in_catalog', course))
+        self.assertTrue(access._has_access_course_desc(user, 'see_about_page', course))
+        self.assertTrue(access._has_access_course_desc(staff, 'see_in_catalog', course))
+        self.assertTrue(access._has_access_course_desc(staff, 'see_about_page', course))
+
+        # Now set visibility to none, which means neither in catalog nor about pages
+        course = Mock(
+            id=SlashSeparatedCourseKey('edX', 'test', '2012_Fall'),
+            catalog_visibility=CATALOG_VISIBILITY_NONE
+        )
+        self.assertFalse(access._has_access_course_desc(user, 'see_in_catalog', course))
+        self.assertFalse(access._has_access_course_desc(user, 'see_about_page', course))
+        self.assertTrue(access._has_access_course_desc(staff, 'see_in_catalog', course))
+        self.assertTrue(access._has_access_course_desc(staff, 'see_about_page', course))
+
 
 class UserRoleTestCase(TestCase):
     """

lms/djangoapps/courseware/tests/test_microsites.py

@@ -11,6 +11,8 @@ from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from helpers import LoginEnrollmentTestCase
 from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE
 
+from xmodule.course_module import (
+    CATALOG_VISIBILITY_CATALOG_AND_ABOUT, CATALOG_VISIBILITY_NONE)
 
 @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
 class TestMicrosites(ModuleStoreTestCase, LoginEnrollmentTestCase):
@@ -42,6 +44,20 @@ class TestMicrosites(ModuleStoreTestCase, LoginEnrollmentTestCase):
 
         self.course_outside_microsite = CourseFactory.create(display_name='Robot_Course_Outside_Microsite', org='FooX')
 
+        # have a course which explicitly sets visibility in catalog to False
+        self.course_hidden_visibility = CourseFactory.create(
+            display_name='Hidden_course',
+            org='TestMicrositeX',
+            catalog_visibility=CATALOG_VISIBILITY_NONE,
+        )
+
+        # have a course which explicitly sets visibility in catalog and about to true
+        self.course_with_visibility = CourseFactory.create(
+            display_name='visible_course',
+            org='TestMicrositeX',
+            catalog_visibility=CATALOG_VISIBILITY_CATALOG_AND_ABOUT,
+        )
+
     def setup_users(self):
         # Create student accounts and activate them.
         for i in range(len(self.STUDENT_INFO)):
@@ -71,9 +87,15 @@ class TestMicrosites(ModuleStoreTestCase, LoginEnrollmentTestCase):
         # assert that test course display name is visible
         self.assertContains(resp, 'Robot_Super_Course')
 
+        # assert that test course with 'visible_in_catalog' to True is showing up
+        self.assertContains(resp, 'visible_course')
+
         # assert that test course that is outside microsite is not visible
         self.assertNotContains(resp, 'Robot_Course_Outside_Microsite')
 
+        # assert that a course that has visible_in_catalog=False is not visible
+        self.assertNotContains(resp, 'Hidden_course')
+
         # assert that footer template has been properly overriden on homepage
         self.assertContains(resp, 'This is a Test Microsite footer')
 
@@ -153,3 +175,17 @@ class TestMicrosites(ModuleStoreTestCase, LoginEnrollmentTestCase):
         resp = self.client.get(reverse('dashboard'))
         self.assertNotContains(resp, 'Robot_Super_Course')
         self.assertContains(resp, 'Robot_Course_Outside_Microsite')
+
+    @override_settings(SITE_NAME=settings.MICROSITE_TEST_HOSTNAME)
+    def test_visible_about_page_settings(self):
+        """
+        Make sure the Microsite is honoring the visible_about_page permissions that is
+        set in configuration
+        """
+        url = reverse('about_course', args=[self.course_with_visibility.id.to_deprecated_string()])
+        resp = self.client.get(url, HTTP_HOST=settings.MICROSITE_TEST_HOSTNAME)
+        self.assertEqual(resp.status_code, 200)
+
+        url = reverse('about_course', args=[self.course_hidden_visibility.id.to_deprecated_string()])
+        resp = self.client.get(url, HTTP_HOST=settings.MICROSITE_TEST_HOSTNAME)
+        self.assertEqual(resp.status_code, 404)

lms/djangoapps/courseware/views.py

@@ -701,7 +701,12 @@ def course_about(request, course_id):
     """
 
     course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id)
-    course = get_course_with_access(request.user, 'see_exists', course_key)
+
+    permission_name = microsite.get_value(
+        'COURSE_ABOUT_VISIBILITY_PERMISSION',
+        settings.COURSE_ABOUT_VISIBILITY_PERMISSION
+    )
+    course = get_course_with_access(request.user, permission_name, course_key)
 
     if microsite.get_value(
         'ENABLE_MKTG_SITE',
@@ -710,6 +715,7 @@ def course_about(request, course_id):
         return redirect(reverse('info', args=[course.id.to_deprecated_string()]))
 
     registered = registered_for_course(course, request.user)
+
     staff_access = has_access(request.user, 'staff', course)
     studio_url = get_studio_url(course, 'settings/details')
 
@@ -782,8 +788,12 @@ def mktg_course_about(request, course_id):
     course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id)
 
     try:
-        course = get_course_with_access(request.user, 'see_exists', course_key)
-    except (ValueError, Http404) as e:
+        permission_name = microsite.get_value(
+            'COURSE_ABOUT_VISIBILITY_PERMISSION',
+            settings.COURSE_ABOUT_VISIBILITY_PERMISSION
+        )
+        course = get_course_with_access(request.user, permission_name, course_key)
+    except (ValueError, Http404):
         # if a course does not exist yet, display a coming
         # soon button
         return render_to_response(

lms/envs/common.py

@@ -1851,3 +1851,11 @@ INVOICE_PAYMENT_INSTRUCTIONS = "This is where you can\nput directions on how peo
 COUNTRIES_OVERRIDE = {
     "TW": _("Taiwan"),
 }
+
+# which access.py permission name to check in order to determine if a course is visible in
+# the course catalog. We default this to the legacy permission 'see_exists'.
+COURSE_CATALOG_VISIBILITY_PERMISSION = 'see_exists'
+
+# which access.py permission name to check in order to determine if a course about page is
+# visible. We default this to the legacy permission 'see_exists'.
+COURSE_ABOUT_VISIBILITY_PERMISSION = 'see_exists'

lms/envs/test.py

@@ -350,6 +350,8 @@ MICROSITE_CONFIGURATION = {
         "course_index_overlay_logo_file": "test_microsite/images/header-logo.png",
         "homepage_overlay_html": "<h1>This is a Test Microsite Overlay HTML</h1>",
         "ALWAYS_REDIRECT_HOMEPAGE_TO_DASHBOARD_FOR_AUTHENTICATED_USER": False,
+        "COURSE_CATALOG_VISIBILITY_PERMISSION": "see_in_catalog",
+        "COURSE_ABOUT_VISIBILITY_PERMISSION": "see_about_page",
     },
     "default": {
         "university": "default_university",

lms/templates/courseware/course_about.html

@@ -151,6 +151,17 @@
           <span class="add-to-cart">
             ${_('This course is in your <a href="{cart_link}">cart</a>.').format(cart_link=cart_link)}
           </span>
+        % elif is_course_full:
+          <span class="register disabled">
+            ${_("Course is full")}
+          </span>
+        % elif invitation_only and not can_enroll:
+          <span class="register disabled">${_("Enrollment in this course is by invitation only")}</span>
+        ## Shib courses need the enrollment button to be displayed even when can_enroll is False,
+        ## because AnonymousUsers cause can_enroll for shib courses to be False, but we need them to be able to click
+        ## so that they can register and become a real user that can enroll.
+        % elif not is_shib_course and not can_enroll:
+          <span class="register disabled">${_("Enrollment is Closed")}</span>
         %elif settings.FEATURES.get('ENABLE_PAID_COURSE_REGISTRATION') and registration_price:
           <%
           if user.is_authenticated():
@@ -166,17 +177,6 @@
                       cost=registration_price)}
           </a>
           <div id="register_error"></div>
-        % elif is_course_full:
-          <span class="register disabled">
-            ${_("Course is full")}
-          </span>
-        % elif invitation_only and not can_enroll:
-          <span class="register disabled">${_("Enrollment in this course is by invitation only")}</span>
-        ## Shib courses need the enrollment button to be displayed even when can_enroll is False,
-        ## because AnonymousUsers cause can_enroll for shib courses to be False, but we need them to be able to click
-        ## so that they can register and become a real user that can enroll.
-        % elif not is_shib_course and not can_enroll:
-          <span class="register disabled">${_("Enrollment is Closed")}</span>
         %else:
           <a href="#" class="register">
             ${_("Register for {course.display_number_with_default}").format(course=course) | h}