Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 35 additions & 3 deletions common/lib/xmodule/xmodule/tests/test_xblock_wrappers.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# -*- coding: utf-8 -*-
"""
Tests for the wrapping layer that provides the XBlock API using XModule/Descriptor
functionality
Expand Down Expand Up @@ -27,7 +28,7 @@

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
Expand Down Expand Up @@ -63,8 +64,8 @@
CONTAINER_XMODULES = {
ConditionalDescriptor: [{}],
CourseDescriptor: [{}],
RandomizeDescriptor: [{}],
SequenceDescriptor: [{}],
RandomizeDescriptor: [{'display_name': 'Test String Display'}],
SequenceDescriptor: [{'display_name': u'Test Unicode हिंदी Display'}],
VerticalBlock: [{}],
WrapperBlock: [{}],
}
Expand Down Expand Up @@ -433,3 +434,34 @@ def check_property(self, descriptor):

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
)
18 changes: 16 additions & 2 deletions common/lib/xmodule/xmodule/x_module.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,10 @@
# 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
Expand Down Expand Up @@ -766,7 +769,18 @@ def public_view(self, _context):
u'<span class="icon icon-alert fa fa fa-warning" aria-hidden="true"></span>'
u'<div class="message-content">{}</div></div></div>'
)
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):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ def transform_block_filters(self, usage_info, block_structure):
# 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,
Expand Down
3 changes: 3 additions & 0 deletions lms/djangoapps/course_blocks/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
11 changes: 11 additions & 0 deletions lms/djangoapps/courseware/courses.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
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
Expand Down Expand Up @@ -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
12 changes: 12 additions & 0 deletions lms/djangoapps/courseware/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,18 @@ def get_state_by_params(cls, course_id, module_state_keys, student_id=None):
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
Expand Down
33 changes: 32 additions & 1 deletion lms/djangoapps/courseware/tests/test_about.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
"""
import datetime
import ddt
import mock
import pytz
from ccx_keys.locator import CCXLocator
from django.conf import settings
Expand All @@ -16,14 +17,22 @@
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,
Expand All @@ -41,6 +50,7 @@
SHIB_ERROR_STR = "The currently logged-in user account does not have permission to enroll in this course."


@ddt.ddt
@attr(shard=1)
class AboutTestCase(LoginEnrollmentTestCase, SharedModuleStoreTestCase, EventTrackingTestCase, MilestonesTestCaseMixin):
"""
Expand Down Expand Up @@ -222,6 +232,27 @@ def test_about_page_unfulfilled_prereqs(self):
resp = self.client.get(url)
self.assertEqual(resp.status_code, 200)

@ddt.data(
[COURSE_VISIBILITY_PRIVATE],
[COURSE_VISIBILITY_PUBLIC_OUTLINE],
[COURSE_VISIBILITY_PUBLIC],
)
@ddt.unpack
def test_about_page_public_view(self, course_visibility):
"""
Assert that anonymous or unenrolled users see View Course option
when unenrolled access flag is set
"""
with mock.patch('xmodule.course_module.CourseDescriptor.course_visibility', course_visibility):
with override_waffle_flag(COURSE_ENABLE_UNENROLLED_ACCESS_FLAG, active=True):
url = reverse('about_course', args=[text_type(self.course.id)])
resp = self.client.get(url)
self.assertEqual(resp.status_code, 200)
if course_visibility == COURSE_VISIBILITY_PUBLIC or course_visibility == COURSE_VISIBILITY_PUBLIC_OUTLINE:
self.assertIn("View Course", resp.content)
else:
self.assertIn("Enroll in", resp.content)


@attr(shard=1)
class AboutTestCaseXML(LoginEnrollmentTestCase, ModuleStoreTestCase):
Expand Down
4 changes: 4 additions & 0 deletions lms/djangoapps/courseware/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -2472,6 +2472,10 @@ def test_courseware_access(self, waffle_override, course_visibility, user_type,
self.assertIn('xblock-public_view-vertical', response.content)
self.assertIn('xblock-public_view-html', response.content)
self.assertIn('xblock-public_view-video', response.content)
if user_type == CourseUserType.ANONYMOUS and course_visibility == COURSE_VISIBILITY_PRIVATE:
self.assertIn('To see course content', response.content)
if user_type == CourseUserType.UNENROLLED and course_visibility == COURSE_VISIBILITY_PRIVATE:
self.assertIn('You must be enrolled', response.content)
else:
self.assertIn('data-save-position="true"', response.content)
self.assertIn('data-show-completion="true"', response.content)
Expand Down
30 changes: 17 additions & 13 deletions lms/djangoapps/courseware/views/index.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

from edxmako.shortcuts import render_to_response, render_to_string

from lms.djangoapps.courseware.courses import allow_public_access
from lms.djangoapps.courseware.exceptions import CourseAccessRedirect
from lms.djangoapps.experiments.utils import get_experiment_user_metadata_context
from lms.djangoapps.gating.api import get_entrance_exam_score_ratio, get_entrance_exam_usage_key
Expand Down Expand Up @@ -195,20 +196,23 @@ def render(self, request):
'email_opt_in': False,
})

PageLevelMessages.register_warning_message(
request,
Text(_(u"You are not signed in. To see additional course content, {sign_in_link} or "
u"{register_link}, and enroll in this course.")).format(
sign_in_link=HTML(u'<a href="{url}">{sign_in_label}</a>').format(
sign_in_label=_('sign in'),
url='{}?{}'.format(reverse('signin_user'), qs),
),
register_link=HTML(u'<a href="/{url}">{register_label}</a>').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('<a href="{url}">{sign_in_label}</a>').format(
sign_in_label=_('sign in'),
url='{}?{}'.format(reverse('signin_user'), qs),
),
register_link=HTML('<a href="/{url}">{register_label}</a>').format(
register_label=_('register'),
url='{}?{}'.format(reverse('register_user'), qs),
),
)
)
)

return render_to_response('courseware/courseware.html', self._create_courseware_context(request))

Expand Down
25 changes: 18 additions & 7 deletions lms/djangoapps/courseware/views/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
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
Expand All @@ -87,7 +88,11 @@
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
Expand All @@ -101,6 +106,7 @@
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
Expand Down Expand Up @@ -459,7 +465,7 @@ def get(self, request, course_id, tab_slug, **kwargs):
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)

Expand Down Expand Up @@ -504,7 +510,7 @@ def get(self, request, course_id, tab_type, **kwargs):

# 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)
Expand All @@ -522,11 +528,13 @@ def url_to_enroll(course_key):
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:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: It seems like it might be useful to have a helper method that answers allow_anonymous for some group of visibility flags (e.g. allow_anonymous(user, course, course_visibilities))?

Performance warning: Calling get_course like this is a modulestore call that can potentially make this method much more expensive. If we're already calling get_course in some fashion elsewhere in the view where this is invoked, we're fine, because modulestore will cache it. If we're not, it can potentially make the access a lot more expensive. I think we're okay here, but I'm guessing that you know where this is invoked better than I do. Just wanted to make sure you were aware.

@pkulkark pkulkark Jan 30, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for pointing this out. I have modified the function to use course passed in from the calling function since they were already fetching it. I've also added a helper method for checking anonymous access.

PageLevelMessages.register_warning_message(
request,
Text(_(u"To see course content, {sign_in_link} or {register_link}.")).format(
Expand All @@ -541,9 +549,9 @@ def register_user_access_warning_messages(request, course_key):
)
)
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(
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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)
Expand Down
6 changes: 6 additions & 0 deletions lms/templates/courseware/course_about.html
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,12 @@ <h1>
.format(course_name=course.display_number_with_default, price=course_price)}
</a>
<div id="register_error"></div>
%elif allow_anonymous:
%if show_courseware_link:
<a href="${course_target}">
<strong>${_("View Course")}</strong>
</a>
%endif
%else:
<%
if ecommerce_checkout:
Expand Down
4 changes: 4 additions & 0 deletions openedx/core/djangoapps/course_groups/tests/test_cohorts.py
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,10 @@ def test_anonymous_user_cohort(self):
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,
Expand Down
Loading