diff --git a/cms/envs/common.py b/cms/envs/common.py index eaffb2ea3d..a9028cc74d 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -897,7 +897,6 @@ INSTALLED_APPS = ( 'openedx.core.djangoapps.external_auth', 'student', # misleading name due to sharing with lms 'openedx.core.djangoapps.course_groups', # not used in cms (yet), but tests run - 'openedx.core.djangoapps.coursetalk', # not used in cms (yet), but tests run 'xblock_config', # Maintenance tools diff --git a/common/test/acceptance/tests/lms/test_lms_course_home.py b/common/test/acceptance/tests/lms/test_lms_course_home.py index dd6ecb568d..0b9e39b7c5 100644 --- a/common/test/acceptance/tests/lms/test_lms_course_home.py +++ b/common/test/acceptance/tests/lms/test_lms_course_home.py @@ -7,7 +7,7 @@ from nose.plugins.attrib import attr from ...fixtures.course import CourseFixture, XBlockFixtureDesc from ...pages.lms.bookmarks import BookmarksPage -from ...pages.lms.course_home import CourseHomePage, CourseSearchResultsPage +from ...pages.lms.course_home import CourseHomePage from ...pages.lms.courseware import CoursewarePage from ..helpers import UniqueCourseTest, auto_auth, load_data_str @@ -124,7 +124,7 @@ class CourseHomeTest(CourseHomeBaseTest): @attr('a11y') class CourseHomeA11yTest(CourseHomeBaseTest): """ - Tests the accessibility of the course home page with course outline. + Tests the accessibility of the course home page """ def test_course_home_a11y(self): diff --git a/lms/djangoapps/courseware/tests/test_course_info.py b/lms/djangoapps/courseware/tests/test_course_info.py index c02c029093..48b0775b07 100644 --- a/lms/djangoapps/courseware/tests/test_course_info.py +++ b/lms/djangoapps/courseware/tests/test_course_info.py @@ -385,7 +385,7 @@ class SelfPacedCourseInfoTestCase(LoginEnrollmentTestCase, SharedModuleStoreTest self.assertEqual(resp.status_code, 200) def test_num_queries_instructor_paced(self): - self.fetch_course_info_with_queries(self.instructor_paced_course, 24, 4) + self.fetch_course_info_with_queries(self.instructor_paced_course, 26, 4) def test_num_queries_self_paced(self): - self.fetch_course_info_with_queries(self.self_paced_course, 24, 4) + self.fetch_course_info_with_queries(self.self_paced_course, 26, 4) diff --git a/lms/djangoapps/courseware/views/index.py b/lms/djangoapps/courseware/views/index.py index 62e8593081..742203bbbc 100644 --- a/lms/djangoapps/courseware/views/index.py +++ b/lms/djangoapps/courseware/views/index.py @@ -32,8 +32,8 @@ from openedx.core.djangoapps.monitoring_utils import set_custom_metrics_for_cour from openedx.core.djangoapps.user_api.preferences.api import get_user_preference from openedx.core.djangoapps.waffle_utils import WaffleSwitchNamespace from openedx.features.course_experience import UNIFIED_COURSE_VIEW_FLAG, default_course_url_name -from openedx.features.enterprise_support.api import data_sharing_consent_required from openedx.features.course_experience.views.course_sock import CourseSockFragmentView +from openedx.features.enterprise_support.api import data_sharing_consent_required from request_cache.middleware import RequestCache from shoppingcart.models import CourseRegistrationCode from student.views import is_course_blocked diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py index 9529b8f415..8a0a76035d 100644 --- a/lms/djangoapps/courseware/views/views.py +++ b/lms/djangoapps/courseware/views/views.py @@ -6,7 +6,6 @@ import logging import urllib from collections import OrderedDict, namedtuple from datetime import datetime, timedelta -from pytz import utc import analytics from django.conf import settings @@ -30,6 +29,7 @@ from ipware.ip import get_ip from markupsafe import escape from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey, UsageKey +from pytz import utc from rest_framework import status from web_fragments.fragment import Fragment @@ -73,7 +73,6 @@ from lms.djangoapps.instructor.views.api import require_global_staff from lms.djangoapps.verify_student.models import SoftwareSecurePhotoVerification from openedx.core.djangoapps.catalog.utils import get_programs, get_programs_with_type from openedx.core.djangoapps.content.course_overviews.models import CourseOverview -from openedx.core.djangoapps.coursetalk.helpers import inject_coursetalk_keys_into_context from openedx.core.djangoapps.credit.api import ( get_credit_requirement_status, is_credit_course, @@ -85,7 +84,7 @@ from openedx.core.djangoapps.plugin_api.views import EdxFragmentView from openedx.core.djangoapps.programs.utils import ProgramMarketingDataExtender from openedx.core.djangoapps.self_paced.models import SelfPacedConfiguration from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers -from openedx.features.course_experience import UNIFIED_COURSE_TAB_FLAG, UNIFIED_COURSE_VIEW_FLAG, course_home_url_name +from openedx.features.course_experience import UNIFIED_COURSE_TAB_FLAG, course_home_url_name from openedx.features.course_experience.views.course_dates import CourseDatesFragmentView from openedx.features.enterprise_support.api import data_sharing_consent_required from shoppingcart.utils import is_shopping_cart_enabled @@ -324,6 +323,14 @@ def course_info(request, course_id): if SelfPacedConfiguration.current().enable_course_home_improvements: dates_fragment = CourseDatesFragmentView().render_to_fragment(request, course_id=course_id) + # 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. + from openedx.features.course_experience.views.course_reviews import CourseReviewsModuleFragmentView + + # Decide whether or not to show the reviews link in the course tools bar + show_reviews_link = CourseReviewsModuleFragmentView.is_configured() + context = { 'request': request, 'masquerade_user': user, @@ -338,6 +345,7 @@ def course_info(request, course_id): 'user_is_enrolled': user_is_enrolled, 'dates_fragment': dates_fragment, 'url_to_enroll': url_to_enroll, + 'show_reviews_link': show_reviews_link, # TODO: (Experimental Code). See https://openedx.atlassian.net/wiki/display/RET/2.+In-course+Verification+Prompts 'upgrade_link': check_and_get_upgrade_link(request, user, course.id), 'upgrade_price': get_cosmetic_verified_display_price(course), @@ -355,9 +363,6 @@ def course_info(request, course_id): context['disable_student_access'] = True context['supports_preview_menu'] = False - if CourseEnrollment.is_enrolled(request.user, course.id): - inject_coursetalk_keys_into_context(context, course_key) - return render_to_response('courseware/info.html', context) @@ -694,7 +699,6 @@ def course_about(request, course_id): """ Display the course's about page. """ - course_key = CourseKey.from_string(course_id) if hasattr(course_key, 'ccx'): @@ -776,7 +780,7 @@ def course_about(request, course_id): # - Student is already registered for course # - Course is already full # - Student cannot enroll in course - active_reg_button = not(registered or is_course_full or not can_enroll) + active_reg_button = not (registered or is_course_full or not can_enroll) is_shib_course = uses_shib(course) @@ -786,6 +790,14 @@ def course_about(request, course_id): # Overview overview = CourseOverview.get_from_id(course.id) + # 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. + from openedx.features.course_experience.views.course_reviews import CourseReviewsModuleFragmentView + + # Embed the course reviews tool + reviews_fragment_view = CourseReviewsModuleFragmentView().render_to_fragment(request, course=course) + context = { 'course': course, 'course_details': course_details, @@ -814,8 +826,8 @@ def course_about(request, course_id): 'cart_link': reverse('shoppingcart.views.show_cart'), 'pre_requisite_courses': pre_requisite_courses, 'course_image_urls': overview.image_urls, + 'reviews_fragment_view': reviews_fragment_view, } - inject_coursetalk_keys_into_context(context, course_key) return render_to_response('courseware/course_about.html', context) diff --git a/lms/envs/common.py b/lms/envs/common.py index bec8589139..6582d06b04 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -389,6 +389,10 @@ FEATURES = { 'DISPLAY_ACCOUNT_ACTIVATION_MESSAGE_ON_SIDEBAR': False, } +# Settings for the course reviews tool template and identification key, set either to None to disable course reviews +COURSE_REVIEWS_TOOL_PROVIDER_FRAGMENT_NAME = 'coursetalk-reviews-fragment.html' +COURSE_REVIEWS_TOOL_PROVIDER_PLATFORM_KEY = 'edx' + # Ignore static asset files on import which match this pattern ASSET_IGNORE_REGEX = r"(^\._.*$)|(^\.DS_Store$)|(^.*~$)" @@ -2172,9 +2176,6 @@ INSTALLED_APPS = ( # Static i18n support 'statici18n', - # Review widgets - 'openedx.core.djangoapps.coursetalk', - # API access administration 'openedx.core.djangoapps.api_admin', diff --git a/lms/static/sass/course/_info.scss b/lms/static/sass/course/_info.scss index 9e571fb343..60bbeb52bf 100644 --- a/lms/static/sass/course/_info.scss +++ b/lms/static/sass/course/_info.scss @@ -295,6 +295,17 @@ div.info-wrapper { a { color: $link-color; + display: block; + font-size: 16px; + + span { + width: 20px; + text-align: center; + } + + &:not(:first-child) { + margin-top: 10px; + } } &:after { @@ -307,7 +318,7 @@ div.info-wrapper { @extend %t-strong; @extend %t-title6; margin-bottom: 0; - padding: 12px 26px 20px 0; + padding: 12px 26px 10px 0; } ul { diff --git a/lms/static/sass/features/_course-experience.scss b/lms/static/sass/features/_course-experience.scss index e1d040717d..3ecb26aca8 100644 --- a/lms/static/sass/features/_course-experience.scss +++ b/lms/static/sass/features/_course-experience.scss @@ -20,6 +20,10 @@ .course-sidebar { @include margin-left(0); @include padding-left($baseline); + + .section-tools li:not(:first-child) { + margin-top: ($baseline / 5); + } } // Course outline @@ -194,3 +198,8 @@ } } } + +// Course Reviews Page +.course-reviews-tool { + margin: ($baseline * 2) ($baseline * 3); +} diff --git a/lms/static/sass/multicourse/_course_about.scss b/lms/static/sass/multicourse/_course_about.scss index 833593ec01..1e1e014096 100644 --- a/lms/static/sass/multicourse/_course_about.scss +++ b/lms/static/sass/multicourse/_course_about.scss @@ -392,11 +392,6 @@ } } - >.coursetalk-read-reviews { - margin-top: -200px; - margin-bottom: 220px; - } - header { margin-bottom: 30px; padding-bottom: 16px; diff --git a/lms/templates/courseware/course_about.html b/lms/templates/courseware/course_about.html index 8d6b0ae5e8..8ec15898e2 100644 --- a/lms/templates/courseware/course_about.html +++ b/lms/templates/courseware/course_about.html @@ -5,6 +5,7 @@ from django.core.urlresolvers import reverse from courseware.courses import get_course_about_section from django.conf import settings from edxmako.shortcuts import marketing_link +from openedx.core.djangolib.markup import HTML from openedx.core.lib.courses import course_image_url %> @@ -17,10 +18,6 @@ from openedx.core.lib.courses import course_image_url <%block name="js_extra"> - ## CourseTalk widget js script - % if show_coursetalk_widget: - - % endif - % endif - - <%block name="bodyclass">view-in-course view-course-info ${course.css_class or ''}
@@ -89,14 +83,7 @@ from openedx.core.djangolib.markup import HTML, Text

${_("Course Updates and News")}

${HTML(get_course_info_section(request, masquerade_user, course, 'updates'))} - ## CourseTalk widget - % if show_coursetalk_widget: -
-
-
- % endif -

${_("Course Tools")}

@@ -104,6 +91,12 @@ from openedx.core.djangolib.markup import HTML, Text ${_("Bookmarks")} + % if SHOW_REVIEWS_TOOL_FLAG.is_enabled(course.id) and show_reviews_link: + + + ${_("Reviews")} + + % endif
% if SelfPacedConfiguration.current().enable_course_home_improvements: diff --git a/openedx/core/djangoapps/coursetalk/admin.py b/openedx/core/djangoapps/coursetalk/admin.py deleted file mode 100644 index 1c1da65601..0000000000 --- a/openedx/core/djangoapps/coursetalk/admin.py +++ /dev/null @@ -1,7 +0,0 @@ -"""Manage coursetalk configuration. """ -from config_models.admin import ConfigurationModelAdmin -from django.contrib import admin - -from openedx.core.djangoapps.coursetalk.models import CourseTalkWidgetConfiguration - -admin.site.register(CourseTalkWidgetConfiguration, ConfigurationModelAdmin) diff --git a/openedx/core/djangoapps/coursetalk/helpers.py b/openedx/core/djangoapps/coursetalk/helpers.py deleted file mode 100644 index 69824f6b1f..0000000000 --- a/openedx/core/djangoapps/coursetalk/helpers.py +++ /dev/null @@ -1,35 +0,0 @@ -""" -CourseTalk widget helpers -""" -from __future__ import unicode_literals - -from openedx.core.djangoapps.coursetalk import models - - -def get_coursetalk_course_key(course_key): - """ - Return course key for coursetalk widget - - CourseTalk unique key for a course contains only organization and course code. - :param course_key: SlashSeparatedCourseKey instance - :type course_key: SlashSeparatedCourseKey - :return: CourseTalk course key - :rtype: str - """ - return '{0.org}_{0.course}'.format(course_key) - - -def inject_coursetalk_keys_into_context(context, course_key): - """ - Set params to view context based on course_key and CourseTalkWidgetConfiguration - - :param context: view context - :type context: dict - :param course_key: SlashSeparatedCourseKey instance - :type course_key: SlashSeparatedCourseKey - """ - show_coursetalk_widget = models.CourseTalkWidgetConfiguration.is_enabled() - if show_coursetalk_widget: - context['show_coursetalk_widget'] = True - context['platform_key'] = models.CourseTalkWidgetConfiguration.get_platform_key() - context['course_review_key'] = get_coursetalk_course_key(course_key) diff --git a/openedx/core/djangoapps/coursetalk/models.py b/openedx/core/djangoapps/coursetalk/models.py deleted file mode 100644 index aee503d952..0000000000 --- a/openedx/core/djangoapps/coursetalk/models.py +++ /dev/null @@ -1,38 +0,0 @@ -""" -Models for CourseTalk configurations -""" -from __future__ import unicode_literals - -from config_models.models import ConfigurationModel -from django.db import models -from django.utils.translation import ugettext_lazy as _ - - -class CourseTalkWidgetConfiguration(ConfigurationModel): - """ - This model represents Enable Configuration for CourseTalk widget. - If the setting enabled, widget will will be available on course - info page and on course about page. - """ - platform_key = models.fields.CharField( - max_length=50, - help_text=_( - "The platform key associates CourseTalk widgets with your platform. " - "Generally, it is the domain name for your platform. For example, " - "if your platform is http://edx.org, the platform key is \"edx\"." - ) - ) - - @classmethod - def get_platform_key(cls): - """ - Return platform_key for current active configuration. - If current configuration is not enabled - return empty string - - :return: Platform key - :rtype: unicode - """ - return cls.current().platform_key if cls.is_enabled() else '' - - def __unicode__(self): - return 'CourseTalkWidgetConfiguration - {0}'.format(self.enabled) diff --git a/openedx/core/djangoapps/coursetalk/tests/__init__.py b/openedx/core/djangoapps/coursetalk/tests/__init__.py deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/openedx/core/djangoapps/coursetalk/tests/test_helpers.py b/openedx/core/djangoapps/coursetalk/tests/test_helpers.py deleted file mode 100644 index 379f4ea4f8..0000000000 --- a/openedx/core/djangoapps/coursetalk/tests/test_helpers.py +++ /dev/null @@ -1,56 +0,0 @@ -""" CourseTalk widget helpers tests """ -from __future__ import unicode_literals - -from django import test - -from opaque_keys.edx.locations import SlashSeparatedCourseKey -from openedx.core.djangoapps.coursetalk import helpers -from openedx.core.djangoapps.coursetalk import models -from openedx.core.djangolib.testing.utils import skip_unless_lms - - -@skip_unless_lms -class CourseTalkKeyTests(test.TestCase): - """ - CourseTalkKeyTests: - tests for function get_coursetalk_course_key - tests for function inject_coursetalk_keys_into_context - """ - - PLATFORM_KEY = 'some_platform' - - def setUp(self): - super(CourseTalkKeyTests, self).setUp() - self.course_key = SlashSeparatedCourseKey('org', 'course', 'run') - self.context = {} - - def db_set_up(self, enabled): - """ - Setup database for this test: - Create CourseTalkWidgetConfiguration - """ - config = models.CourseTalkWidgetConfiguration.current() - config.enabled = enabled - config.platform_key = self.PLATFORM_KEY - config.save() - - def test_simple_key(self): - coursetalk_course_key = helpers.get_coursetalk_course_key(self.course_key) - self.assertEqual(coursetalk_course_key, 'org_course') - - def test_inject_coursetalk_keys_when_widget_not_enabled(self): - self.db_set_up(False) - helpers.inject_coursetalk_keys_into_context(self.context, self.course_key) - self.assertNotIn('show_coursetalk_widget', self.context) - self.assertNotIn('platform_key', self.context) - self.assertNotIn('course_review_key', self.context) - - def test_inject_coursetalk_keys_when_widget_enabled(self): - self.db_set_up(True) - helpers.inject_coursetalk_keys_into_context(self.context, self.course_key) - self.assertIn('show_coursetalk_widget', self.context) - self.assertIn('platform_key', self.context) - self.assertIn('course_review_key', self.context) - self.assertEqual(self.context.get('show_coursetalk_widget'), True) - self.assertEqual(self.context.get('platform_key'), self.PLATFORM_KEY) - self.assertEqual(self.context.get('course_review_key'), 'org_course') diff --git a/openedx/features/course_experience/__init__.py b/openedx/features/course_experience/__init__.py index 2fa75ecee2..af72e3d662 100644 --- a/openedx/features/course_experience/__init__.py +++ b/openedx/features/course_experience/__init__.py @@ -20,7 +20,10 @@ WAFFLE_FLAG_NAMESPACE = WaffleFlagNamespace(name='course_experience') UNIFIED_COURSE_TAB_FLAG = CourseWaffleFlag(WAFFLE_FLAG_NAMESPACE, 'unified_course_tab') # Waffle flag to enable the sock on the footer of the home and courseware pages -DISPLAY_COURSE_SOCK = CourseWaffleFlag(WAFFLE_FLAG_NAMESPACE, 'display_course_sock') +DISPLAY_COURSE_SOCK_FLAG = CourseWaffleFlag(WAFFLE_FLAG_NAMESPACE, 'display_course_sock') + +# Waffle flag to enable a review page link from the unified home page +SHOW_REVIEWS_TOOL_FLAG = CourseWaffleFlag(WAFFLE_FLAG_NAMESPACE, 'show_reviews_tool') def course_home_page_title(course): # pylint: disable=unused-argument diff --git a/openedx/features/course_experience/templates/course_experience/course-home-fragment.html b/openedx/features/course_experience/templates/course_experience/course-home-fragment.html index 5aa13184ae..9b0a513307 100644 --- a/openedx/features/course_experience/templates/course_experience/course-home-fragment.html +++ b/openedx/features/course_experience/templates/course_experience/course-home-fragment.html @@ -14,7 +14,7 @@ from django.core.urlresolvers import reverse from django_comment_client.permissions import has_permission from openedx.core.djangolib.js_utils import dump_js_escaped_json, js_escaped_string from openedx.core.djangolib.markup import HTML -from openedx.features.course_experience import UNIFIED_COURSE_TAB_FLAG +from openedx.features.course_experience import UNIFIED_COURSE_TAB_FLAG, SHOW_REVIEWS_TOOL_FLAG %> <%block name="content"> @@ -75,6 +75,14 @@ from openedx.features.course_experience import UNIFIED_COURSE_TAB_FLAG ${_("Bookmarks")} + % if SHOW_REVIEWS_TOOL_FLAG.is_enabled(course.id) and show_reviews_link: +
  • + + + ${_("Reviews")} + +
    • + % endif % if UNIFIED_COURSE_TAB_FLAG.is_enabled(course.id):
  • diff --git a/openedx/features/course_experience/templates/course_experience/course-reviews-fragment.html b/openedx/features/course_experience/templates/course_experience/course-reviews-fragment.html new file mode 100644 index 0000000000..a0ef0bd8e0 --- /dev/null +++ b/openedx/features/course_experience/templates/course_experience/course-reviews-fragment.html @@ -0,0 +1,37 @@ +## mako + +<%page expression_filter="h"/> + +<%namespace name='static' file='../static_content.html'/> + +<%! +from django.utils.translation import ugettext as _ + +from openedx.core.djangolib.markup import HTML +from openedx.features.course_experience import course_home_page_title +%> + + +
    +
    + ## Breadcrumb navigation +
    + +
    +
    +
    + % if course_reviews_provider_fragment: + ${HTML(course_reviews_provider_fragment.body_html())} + % endif +
    +
    diff --git a/openedx/features/course_experience/templates/course_experience/course-sock-fragment.html b/openedx/features/course_experience/templates/course_experience/course-sock-fragment.html index 9b3f89b599..f0fb39a40f 100644 --- a/openedx/features/course_experience/templates/course_experience/course-sock-fragment.html +++ b/openedx/features/course_experience/templates/course_experience/course-sock-fragment.html @@ -5,11 +5,11 @@ <%! from openedx.core.djangolib.markup import HTML -from openedx.features.course_experience import DISPLAY_COURSE_SOCK +from openedx.features.course_experience import DISPLAY_COURSE_SOCK_FLAG %> <%block name="content"> - % if show_course_sock and DISPLAY_COURSE_SOCK.is_enabled(course_id): + % if show_course_sock and DISPLAY_COURSE_SOCK_FLAG.is_enabled(course_id):