From 9ddb1cc0749843270a6787ea8a4ad15222d48742 Mon Sep 17 00:00:00 2001 From: Pooja Kulkarni Date: Thu, 7 Feb 2019 12:33:59 +0530 Subject: [PATCH] Implement public cohort This PR is based on #19284 and is part of the series of work related to the proposal #18134. This PR avoids the assignment of anonymous/unenrolled users to any cohort when course is public. Anonymous or unenrolled users will only see content that does not have a content group assigned. The "View Course" link to the course outline is shown on the course about page for a course marked public/public outline. It also makes course handouts available for public courses (not for public_outline). This PR also hides the different warnings and messages asking the user to sign-in and enroll in the course, when the course is marked public. It modifies the default public_view text to include the component display_name when unenrolled access is not available. --- .../xmodule/tests/test_xblock_wrappers.py | 38 +++++++++++++++++-- common/lib/xmodule/xmodule/x_module.py | 18 ++++++++- .../transformers/library_content.py | 2 +- lms/djangoapps/course_blocks/utils.py | 3 ++ lms/djangoapps/courseware/courses.py | 11 ++++++ lms/djangoapps/courseware/models.py | 12 ++++++ lms/djangoapps/courseware/tests/test_about.py | 33 +++++++++++++++- lms/djangoapps/courseware/tests/test_views.py | 4 ++ lms/djangoapps/courseware/views/index.py | 30 ++++++++------- lms/djangoapps/courseware/views/views.py | 25 ++++++++---- lms/templates/courseware/course_about.html | 6 +++ .../course_groups/tests/test_cohorts.py | 4 ++ .../tests/views/test_course_home.py | 13 ++++--- .../course_experience/views/course_home.py | 2 + .../views/course_home_messages.py | 4 +- 15 files changed, 170 insertions(+), 35 deletions(-) diff --git a/common/lib/xmodule/xmodule/tests/test_xblock_wrappers.py b/common/lib/xmodule/xmodule/tests/test_xblock_wrappers.py index 4e7112e171..d82913e16a 100644 --- a/common/lib/xmodule/xmodule/tests/test_xblock_wrappers.py +++ b/common/lib/xmodule/xmodule/tests/test_xblock_wrappers.py @@ -1,3 +1,4 @@ +# -*- coding: utf-8 -*- """ Tests for the wrapping layer that provides the XBlock API using XModule/Descriptor functionality @@ -27,7 +28,7 @@ from xblock.core import XBlock from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator -from xmodule.x_module import ModuleSystem, XModule, XModuleDescriptor, DescriptorSystem, STUDENT_VIEW, STUDIO_VIEW +from xmodule.x_module import ModuleSystem, XModule, XModuleDescriptor, DescriptorSystem, STUDENT_VIEW, STUDIO_VIEW, PUBLIC_VIEW from xmodule.annotatable_module import AnnotatableDescriptor from xmodule.capa_module import CapaDescriptor from xmodule.course_module import CourseDescriptor @@ -63,8 +64,8 @@ LEAF_XMODULES = { CONTAINER_XMODULES = { ConditionalDescriptor: [{}], CourseDescriptor: [{}], - RandomizeDescriptor: [{}], - SequenceDescriptor: [{}], + RandomizeDescriptor: [{'display_name': 'Test String Display'}], + SequenceDescriptor: [{'display_name': u'Test Unicode हिंदी Display'}], VerticalBlock: [{}], WrapperBlock: [{}], } @@ -433,3 +434,34 @@ class TestXmlExport(XBlockWrapperTestMixin, TestCase): self.assertEquals(list(xmodule_api_fs.walk()), list(xblock_api_fs.walk())) self.assertEquals(etree.tostring(xmodule_node), etree.tostring(xblock_node)) + + +class TestPublicView(XBlockWrapperTestMixin, TestCase): + """ + This tests that default public_view shows the correct message. + """ + shard = 1 + + def skip_if_invalid(self, descriptor_cls): + pure_xblock_class = issubclass(descriptor_cls, XBlock) and not issubclass(descriptor_cls, XModuleDescriptor) + if pure_xblock_class: + public_view = descriptor_cls.public_view + else: + public_view = descriptor_cls.module_class.public_view + if public_view != XModule.public_view: + raise SkipTest(descriptor_cls.__name__ + " implements public_view") + + def check_property(self, descriptor): + """ + Assert that public_view contains correct message. + """ + if descriptor.display_name: + self.assertIn( + descriptor.display_name, + descriptor.render(PUBLIC_VIEW).content + ) + else: + self.assertIn( + "This content is only accessible", + descriptor.render(PUBLIC_VIEW).content + ) diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index 299a3e3d8b..521d2fd445 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -72,7 +72,10 @@ STUDIO_VIEW = 'studio_view' # Views that present a "preview" view of an xblock (as opposed to an editing view). PREVIEW_VIEWS = [STUDENT_VIEW, PUBLIC_VIEW, AUTHOR_VIEW] -DEFAULT_PUBLIC_VIEW_MESSAGE = u'Please enroll to view this content.' +DEFAULT_PUBLIC_VIEW_MESSAGE = ( + u'This content is only accessible to enrolled learners. ' + u'Sign in or register, and enroll in this course to view it.' +) # Make '_' a no-op so we can scrape strings. Using lambda instead of # `django.utils.translation.ugettext_noop` because Django cannot be imported in this file @@ -766,7 +769,18 @@ class XModuleMixin(XModuleFields, XBlock): u'' u'
{}
' ) - return Fragment(alert_html.format(DEFAULT_PUBLIC_VIEW_MESSAGE)) + + if self.display_name: + display_text = _( + u'{display_name} is only accessible to enrolled learners. ' + 'Sign in or register, and enroll in this course to view it.' + ).format( + display_name=self.display_name + ) + else: + display_text = _(DEFAULT_PUBLIC_VIEW_MESSAGE) + + return Fragment(alert_html.format(display_text)) class ProxyAttribute(object): diff --git a/lms/djangoapps/course_blocks/transformers/library_content.py b/lms/djangoapps/course_blocks/transformers/library_content.py index eb893a459d..b92b58dbeb 100644 --- a/lms/djangoapps/course_blocks/transformers/library_content.py +++ b/lms/djangoapps/course_blocks/transformers/library_content.py @@ -98,7 +98,7 @@ class ContentLibraryTransformer(FilteringTransformerMixin, BlockStructureTransfo # Save back any changes if any(block_keys[changed] for changed in ('invalid', 'overlimit', 'added')): state_dict['selected'] = list(selected) - StudentModule.objects.update_or_create( + StudentModule.save_state( # pylint: disable=no-value-for-parameter student=usage_info.user, course_id=usage_info.course_key, module_state_key=block_key, diff --git a/lms/djangoapps/course_blocks/utils.py b/lms/djangoapps/course_blocks/utils.py index b2db403d0b..b36a691477 100644 --- a/lms/djangoapps/course_blocks/utils.py +++ b/lms/djangoapps/course_blocks/utils.py @@ -18,6 +18,9 @@ def get_student_module_as_dict(user, course_key, block_key): Returns: StudentModule as a (possibly empty) dict. """ + if not user.is_authenticated(): + return {} + try: student_module = StudentModule.objects.get( student=user, diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index 8a3266a12a..cc6ee05f2e 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -38,6 +38,7 @@ from opaque_keys.edx.keys import UsageKey from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from openedx.core.lib.api.view_utils import LazySequence +from openedx.features.course_experience import COURSE_ENABLE_UNENROLLED_ACCESS_FLAG from path import Path as path from six import text_type from static_replace import replace_static_urls @@ -643,3 +644,13 @@ def get_course_chapter_ids(course_key): log.exception('Failed to retrieve course from modulestore.') return [] return [unicode(chapter_key) for chapter_key in chapter_keys if chapter_key.block_type == 'chapter'] + + +def allow_public_access(course, visibilities): + """ + This checks if the unenrolled access waffle flag for the course is set + and the course visibility matches any of the input visibilities. + """ + unenrolled_access_flag = COURSE_ENABLE_UNENROLLED_ACCESS_FLAG.is_enabled(course.id) + allow_access = unenrolled_access_flag and course.course_visibility in visibilities + return allow_access diff --git a/lms/djangoapps/courseware/models.py b/lms/djangoapps/courseware/models.py index abeff61837..d54fcb96df 100644 --- a/lms/djangoapps/courseware/models.py +++ b/lms/djangoapps/courseware/models.py @@ -161,6 +161,18 @@ class StudentModule(models.Model): module_states = module_states.filter(student_id=student_id) return module_states + @classmethod + def save_state(cls, student, course_id, module_state_key, defaults): + if not student.is_authenticated(): + return + else: + cls.objects.update_or_create( + student=student, + course_id=course_id, + module_state_key=module_state_key, + defaults=defaults, + ) + class BaseStudentModuleHistory(models.Model): """Abstract class containing most fields used by any class diff --git a/lms/djangoapps/courseware/tests/test_about.py b/lms/djangoapps/courseware/tests/test_about.py index 2e1892d95d..5108de1379 100644 --- a/lms/djangoapps/courseware/tests/test_about.py +++ b/lms/djangoapps/courseware/tests/test_about.py @@ -3,6 +3,7 @@ Test the about xblock """ import datetime import ddt +import mock import pytz from ccx_keys.locator import CCXLocator from django.conf import settings @@ -16,14 +17,22 @@ from waffle.testutils import override_switch from course_modes.models import CourseMode from lms.djangoapps.ccx.tests.factories import CcxFactory from openedx.core.lib.tests import attr +from openedx.core.djangoapps.waffle_utils.testutils import override_waffle_flag from openedx.features.course_experience.waffle import WAFFLE_NAMESPACE as COURSE_EXPERIENCE_WAFFLE_NAMESPACE from openedx.features.course_experience.waffle import ENABLE_COURSE_ABOUT_SIDEBAR_HTML +from openedx.features.course_experience import COURSE_ENABLE_UNENROLLED_ACCESS_FLAG from shoppingcart.models import Order, PaidCourseRegistration from student.models import CourseEnrollment from student.tests.factories import AdminFactory, CourseEnrollmentAllowedFactory, UserFactory from track.tests import EventTrackingTestCase from util.milestones_helpers import get_prerequisite_courses_display, set_prerequisite_courses -from xmodule.course_module import CATALOG_VISIBILITY_ABOUT, CATALOG_VISIBILITY_NONE +from xmodule.course_module import ( + CATALOG_VISIBILITY_ABOUT, + CATALOG_VISIBILITY_NONE, + COURSE_VISIBILITY_PRIVATE, + COURSE_VISIBILITY_PUBLIC_OUTLINE, + COURSE_VISIBILITY_PUBLIC +) from xmodule.modulestore.tests.django_utils import ( TEST_DATA_MIXED_MODULESTORE, TEST_DATA_SPLIT_MODULESTORE, @@ -41,6 +50,7 @@ REG_STR = "
{sign_in_label}').format( - sign_in_label=_('sign in'), - url='{}?{}'.format(reverse('signin_user'), qs), - ), - register_link=HTML(u'{register_label}').format( - register_label=_('register'), - url='{}?{}'.format(reverse('register_user'), qs), - ), + allow_anonymous = allow_public_access(self.course, [COURSE_VISIBILITY_PUBLIC]) + + if not allow_anonymous: + PageLevelMessages.register_warning_message( + request, + Text(_("You are not signed in. To see additional course content, {sign_in_link} or " + "{register_link}, and enroll in this course.")).format( + sign_in_link=HTML('{sign_in_label}').format( + sign_in_label=_('sign in'), + url='{}?{}'.format(reverse('signin_user'), qs), + ), + register_link=HTML('{register_label}').format( + register_label=_('register'), + url='{}?{}'.format(reverse('register_user'), qs), + ), + ) ) - ) return render_to_response('courseware/courseware.html', self._create_courseware_context(request)) diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py index 3d651b9c37..8b660bc061 100644 --- a/lms/djangoapps/courseware/views/views.py +++ b/lms/djangoapps/courseware/views/views.py @@ -65,6 +65,7 @@ from lms.djangoapps.ccx.custom_exception import CCXLocatorValidationException from lms.djangoapps.certificates import api as certs_api from lms.djangoapps.certificates.models import CertificateStatuses from lms.djangoapps.commerce.utils import EcommerceService +from lms.djangoapps.courseware.courses import allow_public_access from lms.djangoapps.courseware.exceptions import CourseAccessRedirect, Redirect from lms.djangoapps.experiments.utils import get_experiment_user_metadata_context from lms.djangoapps.grades.course_grade_factory import CourseGradeFactory @@ -87,7 +88,11 @@ from openedx.core.djangoapps.site_configuration import helpers as configuration_ from openedx.core.djangoapps.util.user_messages import PageLevelMessages from openedx.core.djangolib.markup import HTML, Text from openedx.features.course_duration_limits.access import generate_course_expired_fragment -from openedx.features.course_experience import UNIFIED_COURSE_TAB_FLAG, course_home_url_name +from openedx.features.course_experience import ( + UNIFIED_COURSE_TAB_FLAG, + COURSE_ENABLE_UNENROLLED_ACCESS_FLAG, + course_home_url_name, +) from openedx.features.course_experience.course_tools import CourseToolsPluginManager from openedx.features.course_experience.views.course_dates import CourseDatesFragmentView from openedx.features.course_experience.waffle import ENABLE_COURSE_ABOUT_SIDEBAR_HTML @@ -101,6 +106,7 @@ from util.cache import cache, cache_if_anonymous from util.db import outer_atomic from util.milestones_helpers import get_prerequisite_courses_display from util.views import _record_feedback_in_zendesk, ensure_valid_course_key, ensure_valid_usage_key +from xmodule.course_module import COURSE_VISIBILITY_PUBLIC, COURSE_VISIBILITY_PUBLIC_OUTLINE from web_fragments.fragment import Fragment from xmodule.modulestore.django import modulestore from xmodule.modulestore.exceptions import ItemNotFoundError, NoPathToItem @@ -459,7 +465,7 @@ class StaticCourseTabView(EdxFragmentView): raise Http404 # Show warnings if the user has limited access - CourseTabView.register_user_access_warning_messages(request, course_key) + CourseTabView.register_user_access_warning_messages(request, course) return super(StaticCourseTabView, self).get(request, course=course, tab=tab, **kwargs) @@ -504,7 +510,7 @@ class CourseTabView(EdxFragmentView): # Show warnings if the user has limited access # Must come after masquerading on creation of page context - self.register_user_access_warning_messages(request, course_key) + self.register_user_access_warning_messages(request, course) set_custom_metrics_for_course_key(course_key) return super(CourseTabView, self).get(request, course=course, page_context=page_context, **kwargs) @@ -522,11 +528,13 @@ class CourseTabView(EdxFragmentView): return url_to_enroll @staticmethod - def register_user_access_warning_messages(request, course_key): + def register_user_access_warning_messages(request, course): """ Register messages to be shown to the user if they have limited access. """ - if request.user.is_anonymous: + allow_anonymous = allow_public_access(course, [COURSE_VISIBILITY_PUBLIC]) + + if request.user.is_anonymous and not allow_anonymous: PageLevelMessages.register_warning_message( request, Text(_(u"To see course content, {sign_in_link} or {register_link}.")).format( @@ -541,9 +549,9 @@ class CourseTabView(EdxFragmentView): ) ) else: - if not CourseEnrollment.is_enrolled(request.user, course_key): + if not CourseEnrollment.is_enrolled(request.user, course.id) and not allow_anonymous: # Only show enroll button if course is open for enrollment. - if course_open_for_self_enrollment(course_key): + if course_open_for_self_enrollment(course.id): enroll_message = _(u'You must be enrolled in the course to see course content. \ {enroll_link_start}Enroll now{enroll_link_end}.') PageLevelMessages.register_warning_message( @@ -840,6 +848,8 @@ def course_about(request, course_id): sidebar_html_enabled = course_experience_waffle().is_enabled(ENABLE_COURSE_ABOUT_SIDEBAR_HTML) + allow_anonymous = allow_public_access(course, [COURSE_VISIBILITY_PUBLIC, COURSE_VISIBILITY_PUBLIC_OUTLINE]) + # This local import is due to the circularity of lms and openedx references. # This may be resolved by using stevedore to allow web fragments to be used # as plugins, and to avoid the direct import. @@ -878,6 +888,7 @@ def course_about(request, course_id): 'course_image_urls': overview.image_urls, 'reviews_fragment_view': reviews_fragment_view, 'sidebar_html_enabled': sidebar_html_enabled, + 'allow_anonymous': allow_anonymous, } return render_to_response('courseware/course_about.html', context) diff --git a/lms/templates/courseware/course_about.html b/lms/templates/courseware/course_about.html index 6fb28f0084..e33fd0f034 100644 --- a/lms/templates/courseware/course_about.html +++ b/lms/templates/courseware/course_about.html @@ -168,6 +168,12 @@ from six import string_types .format(course_name=course.display_number_with_default, price=course_price)}
+ %elif allow_anonymous: + %if show_courseware_link: + + ${_("View Course")} + + %endif %else: <% if ecommerce_checkout: diff --git a/openedx/core/djangoapps/course_groups/tests/test_cohorts.py b/openedx/core/djangoapps/course_groups/tests/test_cohorts.py index 785a1c6e7e..d9de491e36 100644 --- a/openedx/core/djangoapps/course_groups/tests/test_cohorts.py +++ b/openedx/core/djangoapps/course_groups/tests/test_cohorts.py @@ -391,6 +391,10 @@ class TestCohorts(ModuleStoreTestCase): Anonymous user is not assigned to any cohort group. """ course = modulestore().get_course(self.toy_course_key) + + # verify cohorts is None when course is not cohorted + self.assertIsNone(cohorts.get_cohort(AnonymousUser(), course.id)) + config_course_cohorts( course, is_cohorted=True, diff --git a/openedx/features/course_experience/tests/views/test_course_home.py b/openedx/features/course_experience/tests/views/test_course_home.py index 9a32e5976c..aba381d892 100644 --- a/openedx/features/course_experience/tests/views/test_course_home.py +++ b/openedx/features/course_experience/tests/views/test_course_home.py @@ -313,12 +313,13 @@ class TestCourseHomePageAccess(CourseHomePageTestCase): self.assertContains(response, TEST_CHAPTER_NAME, count=(1 if expected_course_outline else 0)) # Verify that the expected message is shown to the user - self.assertContains( - response, 'To see course content', count=(1 if is_anonymous else 0) - ) - self.assertContains(response, '
'), + close_enroll_link=HTML('') ), title=Text(_('Welcome to {course_display_name}')).format( course_display_name=course.display_name