From 248558f1d415fcba787b7746d346b4ddc47fda4a Mon Sep 17 00:00:00 2001 From: Andy Armstrong <andya@edx.org> Date: Thu, 8 Dec 2016 20:30:56 -0500 Subject: [PATCH] Render Discussion tab using web fragments --- .../templates/static_content.html | 2 +- common/lib/xmodule/xmodule/tabs.py | 86 ++-- lms/djangoapps/courseware/tabs.py | 6 +- lms/djangoapps/courseware/tests/test_tabs.py | 12 +- lms/djangoapps/courseware/views/views.py | 192 ++++----- lms/djangoapps/discussion/plugins.py | 9 +- .../discussion/js/discussion_board_factory.js | 14 +- .../js/discussion_profile_page_factory.js | 2 +- .../static/discussion/js/discussion_router.js | 8 +- .../js/spec/discussion_board_factory_spec.js | 8 +- .../js/views/discussion_user_profile_view.js | 2 +- ...rd.html => discussion_board_fragment.html} | 42 +- .../discussion/discussion_board_js.template | 63 +++ ...tenance.html => maintenance_fragment.html} | 12 +- lms/djangoapps/discussion/tests/test_views.py | 8 +- lms/djangoapps/discussion/urls.py | 7 + lms/djangoapps/discussion/views.py | 389 +++++++++++------- .../django_comment_client/base/views.py | 4 +- .../django_comment_client/tests/group_id.py | 7 +- lms/envs/common.py | 7 + lms/templates/courseware/error-message.html | 2 + lms/templates/courseware/static_tab.html | 9 +- lms/templates/courseware/tab-view.html | 32 ++ lms/templates/fragment-view-chromeless.html | 24 ++ lms/templates/main.html | 2 + lms/urls.py | 8 +- .../core/djangoapps/plugin_api/__init__.py | 0 openedx/core/djangoapps/plugin_api/views.py | 99 +++++ openedx/core/lib/tests/test_course_tab_api.py | 4 +- requirements/edx/base.txt | 4 + requirements/edx/github.txt | 1 - 31 files changed, 674 insertions(+), 391 deletions(-) rename lms/djangoapps/discussion/templates/discussion/{discussion_board.html => discussion_board_fragment.html} (59%) create mode 100644 lms/djangoapps/discussion/templates/discussion/discussion_board_js.template rename lms/djangoapps/discussion/templates/discussion/{maintenance.html => maintenance_fragment.html} (66%) create mode 100644 lms/templates/courseware/tab-view.html create mode 100644 lms/templates/fragment-view-chromeless.html create mode 100644 openedx/core/djangoapps/plugin_api/__init__.py create mode 100644 openedx/core/djangoapps/plugin_api/views.py diff --git a/common/djangoapps/pipeline_mako/templates/static_content.html b/common/djangoapps/pipeline_mako/templates/static_content.html index 281ce7d7372..76282a8c4dc 100644 --- a/common/djangoapps/pipeline_mako/templates/static_content.html +++ b/common/djangoapps/pipeline_mako/templates/static_content.html @@ -119,7 +119,7 @@ source, template_path = Loader(engine).load_template_source(path) }).call(this, require || RequireJS.require); % else: ## The "raw" parameter is specified to avoid the URL from being further maninpulated by - ## static_replace calls (as woudl happen if require_module is used within courseware). + ## static_replace calls (as would happen if require_module is used within courseware). ## Without specifying "raw", a call to static_replace would result in the MD5 hash being ## being appended more than once, causing the import to fail in production environments. require(['${staticfiles_storage.url(module_name + ".js") + "?raw" | n, js_escaped_string}'], function () { diff --git a/common/lib/xmodule/xmodule/tabs.py b/common/lib/xmodule/xmodule/tabs.py index 0b0fb6b282b..8e2f83b8b45 100644 --- a/common/lib/xmodule/xmodule/tabs.py +++ b/common/lib/xmodule/xmodule/tabs.py @@ -15,6 +15,9 @@ log = logging.getLogger("edx.courseware") # `django.utils.translation.ugettext_noop` because Django cannot be imported in this file _ = lambda text: text +# A list of attributes on course tabs that can not be updated +READ_ONLY_COURSE_TAB_ATTRIBUTES = ['type'] + class CourseTab(object): """ @@ -33,6 +36,12 @@ class CourseTab(object): # ugettext_noop since the user won't be available in this context. title = None + # HTML class to add to the tab page's body, or None if no class it to be added + body_class = None + + # Token to identify the online help URL, or None if no help is provided + online_help_token = None + # Class property that specifies whether the tab can be hidden for a particular course is_hideable = False @@ -70,7 +79,7 @@ class CourseTab(object): Args: tab_dict (dict) - a dictionary of parameters used to build the tab. """ - + super(CourseTab, self).__init__() self.name = tab_dict.get('name', self.title) self.tab_id = tab_dict.get('tab_id', getattr(self, 'tab_id', self.type)) self.course_staff_only = tab_dict.get('course_staff_only', False) @@ -80,6 +89,13 @@ class CourseTab(object): @property def link_func(self): + """ + Returns a function that will determine a course URL for this tab. + + The returned function takes two arguments: + course (Course) - the course in question. + view_name (str) - the name of the view. + """ return self.tab_dict.get('link_func', link_reverse_func(self.view_name)) @classmethod @@ -107,16 +123,8 @@ class CourseTab(object): This method allows callers to access CourseTab members with the d[key] syntax as is done with Python dictionary objects. """ - if key == 'name': - return self.name - elif key == 'type': - return self.type - elif key == 'tab_id': - return self.tab_id - elif key == 'is_hidden': - return self.is_hidden - elif key == 'course_staff_only': - return self.course_staff_only + if hasattr(self, key): + return getattr(self, key, None) else: raise KeyError('Key {0} not present in tab {1}'.format(key, self.to_json())) @@ -127,14 +135,8 @@ class CourseTab(object): Note: the 'type' member can be 'get', but not 'set'. """ - if key == 'name': - self.name = value - elif key == 'tab_id': - self.tab_id = value - elif key == 'is_hidden': - self.is_hidden = value - elif key == 'course_staff_only': - self.course_staff_only = value + if hasattr(self, key) and key not in READ_ONLY_COURSE_TAB_ATTRIBUTES: + setattr(self, key, value) else: raise KeyError('Key {0} cannot be set in tab {1}'.format(key, self.to_json())) @@ -236,28 +238,52 @@ class CourseTab(object): return tab_type(tab_dict=tab_dict) -class ComponentTabMixin(object): +class TabFragmentViewMixin(object): """ - A mixin for tabs that meet the component API (and can be rendered via Fragments). + A mixin for tabs that render themselves as web fragments. """ - class_name = None + fragment_view_name = None + + def __init__(self, tab_dict): + super(TabFragmentViewMixin, self).__init__(tab_dict) + self._fragment_view = None @property def link_func(self): + """ Returns a function that returns the course tab's URL. """ + + # If a view_name is specified, then use the default link function + if self.view_name: + return super(TabFragmentViewMixin, self).link_func + + # If not, then use the generic course tab URL def link_func(course, reverse_func): - """ Returns a url for a given course and reverse function. """ - return reverse_func("content_tab", args=[course.id.to_deprecated_string(), self.type]) + """ Returns a function that returns the course tab's URL. """ + return reverse_func("course_tab_view", args=[course.id.to_deprecated_string(), self.type]) return link_func @property def url_slug(self): - return "content_tab/"+self.type + """ + Returns the slug to be included in this tab's URL. + """ + return "tab/" + self.type - def render_fragment(self, request, course): - component = get_storage_class(self.class_name)() - fragment = component.render_component(request, course_id=course.id.to_deprecated_string()) - return fragment + @property + def fragment_view(self): + """ + Returns the view that will be used to render the fragment. + """ + if not self._fragment_view: + self._fragment_view = get_storage_class(self.fragment_view_name)() + return self._fragment_view + + def render_to_fragment(self, request, course, **kwargs): + """ + Renders this tab to a web fragment. + """ + return self.fragment_view.render_to_fragment(request, course_id=unicode(course.id), **kwargs) class StaticTab(CourseTab): @@ -270,7 +296,7 @@ class StaticTab(CourseTab): def __init__(self, tab_dict=None, name=None, url_slug=None): def link_func(course, reverse_func): - """ Returns a url for a given course and reverse function. """ + """ Returns a function that returns the static tab's URL. """ return reverse_func(self.type, args=[course.id.to_deprecated_string(), self.url_slug]) self.url_slug = tab_dict.get('url_slug') if tab_dict else url_slug diff --git a/lms/djangoapps/courseware/tabs.py b/lms/djangoapps/courseware/tabs.py index 75ebba93755..cae742e0f40 100644 --- a/lms/djangoapps/courseware/tabs.py +++ b/lms/djangoapps/courseware/tabs.py @@ -9,7 +9,7 @@ from courseware.access import has_access from courseware.entrance_exams import user_must_complete_entrance_exam from openedx.core.lib.course_tabs import CourseTabPluginManager from student.models import CourseEnrollment -from xmodule.tabs import ComponentTabMixin, CourseTab, CourseTabList, key_checker +from xmodule.tabs import CourseTab, CourseTabList, key_checker class EnrolledTab(CourseTab): @@ -70,14 +70,14 @@ class SyllabusTab(EnrolledTab): return getattr(course, 'syllabus_present', False) -class ProgressTab(ComponentTabMixin, EnrolledTab): +class ProgressTab(EnrolledTab): """ The course progress view. """ type = 'progress' title = ugettext_noop('Progress') priority = 40 - class_name="courseware.views.views.ProgressComponentView" + view_name = 'progress' is_hideable = True is_default = False diff --git a/lms/djangoapps/courseware/tests/test_tabs.py b/lms/djangoapps/courseware/tests/test_tabs.py index ed60e003e34..70f4f8454b9 100644 --- a/lms/djangoapps/courseware/tests/test_tabs.py +++ b/lms/djangoapps/courseware/tests/test_tabs.py @@ -14,7 +14,7 @@ from courseware.tabs import ( ) from courseware.tests.helpers import LoginEnrollmentTestCase from courseware.tests.factories import InstructorFactory, StaffFactory -from courseware.views.views import get_static_tab_contents, static_tab +from courseware.views.views import get_static_tab_fragment, StaticCourseTabView from openedx.core.djangolib.testing.utils import get_mock_request from student.models import CourseEnrollment from student.tests.factories import UserFactory @@ -258,16 +258,16 @@ class StaticTabDateTestCase(LoginEnrollmentTestCase, SharedModuleStoreTestCase): self.setup_user() request = get_mock_request(self.user) with self.assertRaises(Http404): - static_tab(request, course_id='edX/toy', tab_slug='new_tab') + StaticCourseTabView().get(request, course_id='edX/toy', tab_slug='new_tab') - def test_get_static_tab_contents(self): + def test_get_static_tab_fragment(self): self.setup_user() course = get_course_by_id(self.course.id) request = get_mock_request(self.user) tab = xmodule_tabs.CourseTabList.get_tab_by_slug(course.tabs, 'new_tab') # Test render works okay - tab_content = get_static_tab_contents(request, course, tab) + tab_content = get_static_tab_fragment(request, course, tab).content self.assertIn(self.course.id.to_deprecated_string(), tab_content) self.assertIn('static_tab', tab_content) @@ -276,8 +276,8 @@ class StaticTabDateTestCase(LoginEnrollmentTestCase, SharedModuleStoreTestCase): mock_module_render.return_value = MagicMock( render=Mock(side_effect=Exception('Render failed!')) ) - static_tab = get_static_tab_contents(request, course, tab) - self.assertIn("this module is temporarily unavailable", static_tab) + static_tab_content = get_static_tab_fragment(request, course, tab).content + self.assertIn("this module is temporarily unavailable", static_tab_content) @attr(shard=1) diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py index 35cb105b590..3983d77024d 100644 --- a/lms/djangoapps/courseware/views/views.py +++ b/lms/djangoapps/courseware/views/views.py @@ -18,7 +18,13 @@ from django.core.urlresolvers import reverse from django.core.context_processors import csrf from django.db import transaction from django.db.models import Q -from django.http import Http404, HttpResponse, HttpResponseBadRequest, HttpResponseForbidden, QueryDict +from django.http import ( + Http404, + HttpResponse, + HttpResponseBadRequest, + HttpResponseForbidden, + QueryDict, +) from django.shortcuts import redirect from django.utils.decorators import method_decorator from django.utils.timezone import UTC @@ -32,7 +38,6 @@ from ipware.ip import get_ip from markupsafe import escape from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey, UsageKey -from opaque_keys.edx.locations import SlashSeparatedCourseKey from rest_framework import status from lms.djangoapps.instructor.views.api import require_global_staff from lms.djangoapps.ccx.utils import prep_course_for_grading @@ -99,8 +104,8 @@ from xmodule.x_module import STUDENT_VIEW from ..entrance_exams import user_must_complete_entrance_exam from ..module_render import get_module_for_descriptor, get_module, get_module_by_usage_id -from web_fragments.views import FragmentView from web_fragments.fragment import Fragment +from web_fragments.views import FragmentView log = logging.getLogger("edx.courseware") @@ -232,7 +237,7 @@ def jump_to_id(request, course_id, module_id): This entry point allows for a shorter version of a jump to where just the id of the element is passed in. This assumes that id is unique within the course_id namespace """ - course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) + course_key = CourseKey.from_string(course_id) items = modulestore().get_items(course_key, qualifiers={'name': module_id}) if len(items) == 0: @@ -443,63 +448,79 @@ def get_last_accessed_courseware(course, request, user): return None -@ensure_csrf_cookie -@ensure_valid_course_key -def static_tab(request, course_id, tab_slug): +class StaticCourseTabView(FragmentView): """ - Display the courses tab with the given name. - - Assumes the course_id is in a valid format. + View that displays a static course tab with a given name. """ + @method_decorator(ensure_csrf_cookie) + @method_decorator(ensure_valid_course_key) + def get(self, request, course_id, tab_slug, **kwargs): + """ + Displays a static course tab page with a given name + """ + course_key = CourseKey.from_string(course_id) + course = get_course_with_access(request.user, 'load', course_key) + tab = CourseTabList.get_tab_by_slug(course.tabs, tab_slug) + if tab is None: + raise Http404 + return super(StaticCourseTabView, self).get(request, course=course, tab=tab, **kwargs) - course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) - - course = get_course_with_access(request.user, 'load', course_key) - - tab = CourseTabList.get_tab_by_slug(course.tabs, tab_slug) - if tab is None: - raise Http404 - - fragment = get_static_tab_fragment( - request, - course, - tab - ) + def render_to_fragment(self, request, course=None, tab=None, **kwargs): + """ + Renders the static tab to a fragment. + """ + return get_static_tab_fragment(request, course, tab) - return render_to_response('courseware/static_tab.html', { - 'course': course, - 'active_page': 'static_tab_{0}'.format(tab['url_slug']), - 'tab': tab, - 'fragment': fragment, - 'uses_pattern_library': False, - 'disable_courseware_js': True - }) + def render_to_standalone_html(self, request, fragment, course=None, tab=None, **kwargs): + """ + Renders this static tab's fragment to HTML for a standalone page. + """ + return render_to_response('courseware/static_tab.html', { + 'course': course, + 'active_page': 'static_tab_{0}'.format(tab['url_slug']), + 'tab': tab, + 'fragment': fragment, + 'uses_pattern_library': False, + 'disable_courseware_js': True, + }) -@ensure_csrf_cookie -@ensure_valid_course_key -def content_tab(request, course_id, tab_type): +class CourseTabView(FragmentView): """ - Display a content tab based on type name. - - Assumes the course_id is in a valid format. + View that displays a course tab page. """ + @method_decorator(ensure_csrf_cookie) + @method_decorator(ensure_valid_course_key) + def get(self, request, course_id, tab_type, **kwargs): + """ + Displays a course tab page that contains a web fragment. + """ + course_key = CourseKey.from_string(course_id) + course = get_course_with_access(request.user, 'load', course_key) + tab = CourseTabList.get_tab_by_type(course.tabs, tab_type) + return super(CourseTabView, self).get(request, course=course, tab=tab, **kwargs) - course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) - course = get_course_with_access(request.user, 'load', course_key) - - content_tab = [tab for tab in course.tabs if tab.type == tab_type][0] - fragment = content_tab.render_fragment(request, course) - + def render_to_fragment(self, request, course=None, tab=None, **kwargs): + """ + Renders the course tab to a fragment. + """ + return tab.render_to_fragment(request, course, **kwargs) - return render_to_response('courseware/static_tab.html', { - 'course': course, - 'active_page': content_tab['type'], - 'tab': content_tab, - 'fragment': fragment, - 'uses_pattern_library': True, - 'disable_courseware_js': True - }) + def render_to_standalone_html(self, request, fragment, course=None, tab=None, **kwargs): + """ + Renders this course tab's fragment to HTML for a standalone page. + """ + return render_to_string( + 'courseware/tab-view.html', + { + 'course': course, + 'active_page': tab['type'], + 'tab': tab, + 'fragment': fragment, + 'uses_pattern_library': True, + 'disable_courseware_js': True, + }, + ) @ensure_csrf_cookie @@ -511,7 +532,7 @@ def syllabus(request, course_id): Assumes the course_id is in a valid format. """ - course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) + course_key = CourseKey.from_string(course_id) course = get_course_with_access(request.user, 'load', course_key) staff_access = bool(has_access(request.user, 'staff', course)) @@ -619,7 +640,7 @@ def course_about(request, course_id): Assumes the course_id is in a valid format. """ - course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) + course_key = CourseKey.from_string(course_id) if hasattr(course_key, 'ccx'): # if un-enrolled/non-registered user try to access CCX (direct for registration) @@ -749,65 +770,6 @@ def course_about(request, course_id): return render_to_response('courseware/course_about.html', context) -class ProgressComponentView(FragmentView): - """ - Component implementation of the discussion board. - """ - def render_fragment(self, request, course_id=None): - """ - Render the component - """ - # nr_transaction = newrelic.agent.current_transaction() - # - course_key = CourseKey.from_string(course_id) - context = _create_progress_context(request, course_key) - html = render_to_string('discussion/discussion_board_component.html', context) - # # inline_js = render_to_string('discussion/discussion_board_js.template', context) - # - # fragment = Fragment(html) - # # fragment.add_javascript(inline_js) - fragment = Fragment() - fragment.content = "Hello World" - return fragment - - -def _create_progress_context(request, course_key): - course = get_course_with_access(request.user, 'load', course_key, depth=None, check_if_enrolled=True) - prep_course_for_grading(course, request) - staff_access = bool(has_access(request.user, 'staff', course)) - student = request.user - - # NOTE: To make sure impersonation by instructor works, use - # student instead of request.user in the rest of the function. - - # The pre-fetching of groups is done to make auth checks not require an - # additional DB lookup (this kills the Progress page in particular). - student = User.objects.prefetch_related("groups").get(id=student.id) - - course_grade = CourseGradeFactory().create(student, course) - courseware_summary = course_grade.chapter_grades - grade_summary = course_grade.summary - - studio_url = get_studio_url(course, 'settings/grading') - - # checking certificate generation configuration - enrollment_mode, is_active = CourseEnrollment.enrollment_mode_for_user(student, course_key) - - context = { - 'course': course, - 'courseware_summary': courseware_summary, - 'studio_url': studio_url, - 'grade_summary': grade_summary, - 'staff_access': staff_access, - 'student': student, - 'passed': is_course_passed(course, grade_summary), - 'credit_course_requirements': _credit_course_requirements(course_key, student), - 'certificate_data': _get_cert_data(student, course, course_key, is_active, enrollment_mode) - } - - return context - - @transaction.non_atomic_requests @login_required @cache_control(no_cache=True, no_store=True, must_revalidate=True) @@ -1081,7 +1043,7 @@ def submission_history(request, course_id, student_username, location): StudentModuleHistory records. """ - course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) + course_key = CourseKey.from_string(course_id) try: usage_key = course_key.make_usage_key_from_deprecated_string(location) @@ -1195,7 +1157,7 @@ def get_course_lti_endpoints(request, course_id): (django response object): HTTP response. 404 if course is not found, otherwise 200 with JSON body. """ - course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) + course_key = CourseKey.from_string(course_id) try: course = get_course(course_key, depth=2) @@ -1244,7 +1206,7 @@ def course_survey(request, course_id): views.py file in the Survey Djangoapp """ - course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) + course_key = CourseKey.from_string(course_id) course = get_course_with_access(request.user, 'load', course_key) redirect_url = reverse('info', args=[course_id]) diff --git a/lms/djangoapps/discussion/plugins.py b/lms/djangoapps/discussion/plugins.py index 440fca766ab..f0096f3f5bf 100644 --- a/lms/djangoapps/discussion/plugins.py +++ b/lms/djangoapps/discussion/plugins.py @@ -7,10 +7,10 @@ from django.utils.translation import ugettext_noop from courseware.tabs import EnrolledTab import django_comment_client.utils as utils -from xmodule.tabs import ComponentTabMixin +from xmodule.tabs import TabFragmentViewMixin -class DiscussionTab(ComponentTabMixin, EnrolledTab): +class DiscussionTab(TabFragmentViewMixin, EnrolledTab): """ A tab for the cs_comments_service forums. """ @@ -18,9 +18,12 @@ class DiscussionTab(ComponentTabMixin, EnrolledTab): type = 'discussion' title = ugettext_noop('Discussion') priority = None - class_name = 'discussion.views.DiscussionBoardComponentView' + view_name = 'discussion.views.forum_form_discussion' + fragment_view_name = 'discussion.views.DiscussionBoardFragmentView' is_hideable = settings.FEATURES.get('ALLOW_HIDING_DISCUSSION_TAB', False) is_default = False + body_class = 'discussion' + online_help_token = 'discussions' @classmethod def is_enabled(cls, course, user=None): diff --git a/lms/djangoapps/discussion/static/discussion/js/discussion_board_factory.js b/lms/djangoapps/discussion/static/discussion/js/discussion_board_factory.js index 6c2c3103434..ff266e6b64b 100644 --- a/lms/djangoapps/discussion/static/discussion/js/discussion_board_factory.js +++ b/lms/djangoapps/discussion/static/discussion/js/discussion_board_factory.js @@ -17,11 +17,11 @@ function($, Backbone, Content, Discussion, DiscussionUtil, DiscussionCourseSettings, DiscussionUser, NewPostView, DiscussionRouter, DiscussionBoardView) { return function(options) { - var userInfo = options.user_info, - sortPreference = options.sort_preference, + var userInfo = options.userInfo, + sortPreference = options.sortPreference, threads = options.threads, - threadPages = options.thread_pages, - contentInfo = options.content_info, + threadPages = options.threadPages, + contentInfo = options.contentInfo, user = new DiscussionUser(userInfo), discussion, courseSettings, @@ -33,14 +33,14 @@ // TODO: eliminate usage of global variables when possible DiscussionUtil.loadRoles(options.roles); window.$$course_id = options.courseId; - window.courseName = options.course_name; + window.courseName = options.courseName; DiscussionUtil.setUser(user); window.user = user; Content.loadContentInfos(contentInfo); // Create a discussion model discussion = new Discussion(threads, {pages: threadPages, sort: sortPreference}); - courseSettings = new DiscussionCourseSettings(options.course_settings); + courseSettings = new DiscussionCourseSettings(options.courseSettings); // Create the discussion board view discussionBoardView = new DiscussionBoardView({ @@ -61,7 +61,7 @@ // Set up a router to manage the page's history router = new DiscussionRouter({ - courseId: options.courseId, + rootUrl: options.rootUrl, discussion: discussion, courseSettings: courseSettings, discussionBoardView: discussionBoardView, diff --git a/lms/djangoapps/discussion/static/discussion/js/discussion_profile_page_factory.js b/lms/djangoapps/discussion/static/discussion/js/discussion_profile_page_factory.js index 469a1a35bd3..313fbe99cc2 100644 --- a/lms/djangoapps/discussion/static/discussion/js/discussion_profile_page_factory.js +++ b/lms/djangoapps/discussion/static/discussion/js/discussion_profile_page_factory.js @@ -35,7 +35,7 @@ DiscussionUtil.loadRoles(options.roles); window.$$course_id = options.courseId; - window.courseName = options.course_name; + window.courseName = options.courseName; DiscussionUtil.setUser(user); window.user = user; Content.loadContentInfos(contentInfo); diff --git a/lms/djangoapps/discussion/static/discussion/js/discussion_router.js b/lms/djangoapps/discussion/static/discussion/js/discussion_router.js index a6bea34cbfc..a9dce42cd9d 100644 --- a/lms/djangoapps/discussion/static/discussion/js/discussion_router.js +++ b/lms/djangoapps/discussion/static/discussion/js/discussion_router.js @@ -18,9 +18,9 @@ initialize: function(options) { Backbone.Router.prototype.initialize.call(this); _.bindAll(this, 'allThreads', 'showThread'); - this.courseId = options.courseId; + this.rootUrl = options.rootUrl; this.discussion = options.discussion; - this.course_settings = options.courseSettings; + this.courseSettings = options.courseSettings; this.discussionBoardView = options.discussionBoardView; this.newPostView = options.newPostView; }, @@ -50,7 +50,7 @@ Backbone.history.start({ pushState: true, - root: '/courses/' + this.courseId + '/discussion/forum/' + root: this.rootUrl }); }, @@ -95,7 +95,7 @@ el: $('.forum-content'), model: this.thread, mode: 'tab', - course_settings: this.course_settings + courseSettings: this.courseSettings }); this.main.render(); this.main.on('thread:responses:rendered', function() { diff --git a/lms/djangoapps/discussion/static/discussion/js/spec/discussion_board_factory_spec.js b/lms/djangoapps/discussion/static/discussion/js/spec/discussion_board_factory_spec.js index cf571920a12..dd383cfe00f 100644 --- a/lms/djangoapps/discussion/static/discussion/js/spec/discussion_board_factory_spec.js +++ b/lms/djangoapps/discussion/static/discussion/js/spec/discussion_board_factory_spec.js @@ -33,14 +33,14 @@ define( DiscussionBoardFactory({ el: $('#discussion-container'), courseId: 'test_course_id', - course_name: 'Test Course', + courseName: 'Test Course', user_info: DiscussionSpecHelper.getTestUserInfo(), roles: DiscussionSpecHelper.getTestRoleInfo(), - sort_preference: null, + sortPreference: null, threads: [], thread_pages: [], - content_info: null, - course_settings: { + contentInfo: null, + courseSettings: { is_cohorted: false, allow_anonymous: false, allow_anonymous_to_peers: false, diff --git a/lms/djangoapps/discussion/static/discussion/js/views/discussion_user_profile_view.js b/lms/djangoapps/discussion/static/discussion/js/views/discussion_user_profile_view.js index 900413ef479..63bb8d92e0c 100644 --- a/lms/djangoapps/discussion/static/discussion/js/views/discussion_user_profile_view.js +++ b/lms/djangoapps/discussion/static/discussion/js/views/discussion_user_profile_view.js @@ -56,7 +56,7 @@ el: this.$('.forum-content'), model: thread, mode: 'inline', - course_settings: this.courseSettings + courseSettings: this.courseSettings }); this.threadView.render(); this.listenTo(this.threadView.showView, 'thread:_delete', this.navigateToAllThreads); diff --git a/lms/djangoapps/discussion/templates/discussion/discussion_board.html b/lms/djangoapps/discussion/templates/discussion/discussion_board_fragment.html similarity index 59% rename from lms/djangoapps/discussion/templates/discussion/discussion_board.html rename to lms/djangoapps/discussion/templates/discussion/discussion_board_fragment.html index 03f07ca09d6..6382f5c8165 100644 --- a/lms/djangoapps/discussion/templates/discussion/discussion_board.html +++ b/lms/djangoapps/discussion/templates/discussion/discussion_board_fragment.html @@ -1,11 +1,9 @@ ## mako -<%! main_css = "style-discussion-main" %> +<%namespace name='static' file='../static_content.html'/> <%page expression_filter="h"/> -<%inherit file="../main.html" /> -<%namespace name='static' file='../static_content.html'/> -<%def name="online_help_token()"><% return "discussions" %></%def> + <%! import json from django.utils.translation import ugettext as _ @@ -14,42 +12,11 @@ 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 %> -<%block name="bodyclass">discussion</%block> -<%block name="pagetitle">${_("Discussion - {course_number}").format(course_number=course.display_number_with_default)}</%block> - -<%block name="headextra"> -<%include file="../discussion/_js_head_dependencies.html" /> -</%block> - -<%block name="base_js_dependencies"> - ## Enable fast preview to fix discussion MathJax rendering bug when page first loads. - <%include file="/discussion/_js_body_dependencies.html" args="disable_fast_preview=False"/> -</%block> - -<%block name="js_extra"> -<%static:require_module module_name="discussion/js/discussion_board_factory" class_name="DiscussionBoardFactory"> -DiscussionBoardFactory({ - courseId: '${unicode(course.id) | n, js_escaped_string}', - $el: $(".discussion-board"), - user_info: ${user_info | n, dump_js_escaped_json}, - roles: ${roles | n, dump_js_escaped_json}, - sort_preference: '${sort_preference | n, js_escaped_string}', - threads: ${threads | n, dump_js_escaped_json}, - thread_pages: '${thread_pages | n, js_escaped_string}', - content_info: ${annotated_content_info | n, dump_js_escaped_json}, - course_name: '${course.display_name_with_default | n, js_escaped_string}', - course_settings: ${course_settings | n, dump_js_escaped_json} -}); -</%static:require_module> -</%block> - -<%include file="../courseware/course_navigation.html" args="active_page='discussion'" /> - -<%block name="content"> <section class="discussion discussion-board container" id="discussion-container" - data-course-id="${course_id}" + data-course-id="${course.id}" data-user-create-comment="${json.dumps(can_create_comment)}" data-user-create-subcomment="${json.dumps(can_create_subcomment)}" data-read-only="false" @@ -88,7 +55,6 @@ DiscussionBoardFactory({ </div> </div> </section> -</%block> <%include file="_underscore_templates.html" /> <%include file="_thread_list_template.html" /> diff --git a/lms/djangoapps/discussion/templates/discussion/discussion_board_js.template b/lms/djangoapps/discussion/templates/discussion/discussion_board_js.template new file mode 100644 index 00000000000..7310e3451b1 --- /dev/null +++ b/lms/djangoapps/discussion/templates/discussion/discussion_board_js.template @@ -0,0 +1,63 @@ +## mako + +<%! +from openedx.core.djangolib.js_utils import dump_js_escaped_json, js_escaped_string +%> + +(function (require, define) { + var registerDiscussionClass = function(moduleName, modulePath) { + define( + modulePath, + [], + function() { + var discussionClass = window[moduleName]; + if (!discussionClass) { + throw new Error('Discussion class not loaded: ' + moduleName); + } + return discussionClass; + } + ); + } + + ## Add RequireJS definitions for each discussion class + <% + discussion_classes = [ + ['Discussion', 'common/js/discussion/discussion'], + ['Content', 'common/js/discussion/content'], + ['DiscussionModuleView', 'common/js/discussion/discussion_module_view'], + ['DiscussionThreadView', 'common/js/discussion/views/discussion_thread_view'], + ['DiscussionThreadListView', 'common/js/discussion/views/discussion_thread_list_view'], + ['DiscussionThreadProfileView', 'common/js/discussion/views/discussion_thread_profile_view'], + ['DiscussionUtil', 'common/js/discussion/utils'], + ['DiscussionCourseSettings', 'common/js/discussion/models/discussion_course_settings'], + ['DiscussionUser', 'common/js/discussion/models/discussion_user'], + ['NewPostView', 'common/js/discussion/views/new_post_view'], + ] + %> + + % for discussion_class_info in discussion_classes: + registerDiscussionClass( + '${discussion_class_info[0] | n, js_escaped_string}', + '${discussion_class_info[1] | n, js_escaped_string}' + ); + % endfor + + ## Install the discussion board once the DOM is ready + $(function() { + require(['discussion/js/discussion_board_factory'], function (DiscussionBoardFactory) { + DiscussionBoardFactory({ + courseId: '${unicode(course.id) | n, js_escaped_string}', + $el: $(".discussion-board"), + rootUrl: '${root_url | n, js_escaped_string}', + userInfo: ${user_info | n, dump_js_escaped_json}, + roles: ${roles | n, dump_js_escaped_json}, + sortPreference: '${sort_preference | n, js_escaped_string}', + threads: ${threads | n, dump_js_escaped_json}, + threadPages: '${thread_pages | n, js_escaped_string}', + contentInfo: ${annotated_content_info | n, dump_js_escaped_json}, + courseName: '${course.display_name_with_default | n, js_escaped_string}', + courseSettings: ${course_settings | n, dump_js_escaped_json} + }); + }); + }); +}).call(this, require || RequireJS.require, define || RequireJS.define); diff --git a/lms/djangoapps/discussion/templates/discussion/maintenance.html b/lms/djangoapps/discussion/templates/discussion/maintenance_fragment.html similarity index 66% rename from lms/djangoapps/discussion/templates/discussion/maintenance.html rename to lms/djangoapps/discussion/templates/discussion/maintenance_fragment.html index 63c55c6dbf6..fddf691bfe5 100644 --- a/lms/djangoapps/discussion/templates/discussion/maintenance.html +++ b/lms/djangoapps/discussion/templates/discussion/maintenance_fragment.html @@ -1,16 +1,10 @@ ## mako -<%! main_css = "style-discussion-main" %> - -<%! from django.utils.translation import ugettext as _ %> <%page expression_filter="h"/> -<%inherit file="../main.html" /> - -<%block name="bodyclass">discussion</%block> -<%block name="headextra"> -<%include file="../discussion/_js_head_dependencies.html" /> -</%block> +<%! +from django.utils.translation import ugettext as _ +%> <%block name="content"> <h2>${_("Discussion unavailable")}</h2> diff --git a/lms/djangoapps/discussion/tests/test_views.py b/lms/djangoapps/discussion/tests/test_views.py index 5ed2bce1c57..cd727a4ced8 100644 --- a/lms/djangoapps/discussion/tests/test_views.py +++ b/lms/djangoapps/discussion/tests/test_views.py @@ -356,11 +356,11 @@ class SingleThreadQueryCountTestCase(ForumsEnableMixin, ModuleStoreTestCase): # course is outside the context manager that is verifying the number of queries, # and with split mongo, that method ends up querying disabled_xblocks (which is then # cached and hence not queried as part of call_single_thread). - (ModuleStoreEnum.Type.mongo, 1, 6, 4, 15, 3), - (ModuleStoreEnum.Type.mongo, 50, 6, 4, 15, 3), + (ModuleStoreEnum.Type.mongo, 1, 5, 3, 13, 1), + (ModuleStoreEnum.Type.mongo, 50, 5, 3, 13, 1), # split mongo: 3 queries, regardless of thread response size. - (ModuleStoreEnum.Type.split, 1, 3, 3, 14, 3), - (ModuleStoreEnum.Type.split, 50, 3, 3, 14, 3), + (ModuleStoreEnum.Type.split, 1, 3, 3, 12, 1), + (ModuleStoreEnum.Type.split, 50, 3, 3, 12, 1), ) @ddt.unpack def test_number_of_mongo_queries( diff --git a/lms/djangoapps/discussion/urls.py b/lms/djangoapps/discussion/urls.py index 4c7939866c3..4295d3a253c 100644 --- a/lms/djangoapps/discussion/urls.py +++ b/lms/djangoapps/discussion/urls.py @@ -3,6 +3,8 @@ Forum urls for the django_comment_client. """ from django.conf.urls import url, patterns +from .views import DiscussionBoardFragmentView + urlpatterns = patterns( 'discussion.views', @@ -10,5 +12,10 @@ urlpatterns = patterns( url(r'users/(?P<user_id>\w+)$', 'user_profile', name='user_profile'), url(r'^(?P<discussion_id>[\w\-.]+)/threads/(?P<thread_id>\w+)$', 'single_thread', name='single_thread'), url(r'^(?P<discussion_id>[\w\-.]+)/inline$', 'inline_discussion', name='inline_discussion'), + url( + r'discussion_board_fragment_view$', + DiscussionBoardFragmentView.as_view(), + name='discussion_board_fragment_view' + ), url(r'', 'forum_form_discussion', name='forum_form_discussion'), ) diff --git a/lms/djangoapps/discussion/views.py b/lms/djangoapps/discussion/views.py index b70c72b1132..db5df8d243c 100644 --- a/lms/djangoapps/discussion/views.py +++ b/lms/djangoapps/discussion/views.py @@ -4,22 +4,33 @@ Views handling read (GET) requests for the Discussion tab and inline discussions from functools import wraps import logging +from sets import Set +from django.conf import settings from django.contrib.auth.decorators import login_required from django.core.context_processors import csrf from django.core.urlresolvers import reverse from django.contrib.auth.models import User -from django.http import Http404, HttpResponseBadRequest +from django.contrib.staticfiles.storage import staticfiles_storage +from django.http import Http404, HttpResponseServerError from django.shortcuts import render_to_response +from django.template.loader import render_to_string +from django.utils.translation import get_language_bidi from django.views.decorators.http import require_GET import newrelic.agent +from rest_framework import status + +from web_fragments.fragment import Fragment from courseware.courses import get_course_with_access +from courseware.views.views import CourseTabView from openedx.core.djangoapps.course_groups.cohorts import ( is_course_cohorted, get_cohort_id, get_course_cohorts, ) +from openedx.core.djangoapps.plugin_api.views import EdxFragmentView + from courseware.access import has_access from student.models import CourseEnrollment from xmodule.modulestore.django import modulestore @@ -51,8 +62,7 @@ def make_course_settings(course, user): Generate a JSON-serializable model for course settings, which will be used to initialize a DiscussionCourseSettings object on the client. """ - - obj = { + return { 'is_cohorted': is_course_cohorted(course.id), 'allow_anonymous': course.allow_anonymous, 'allow_anonymous_to_peers': course.allow_anonymous_to_peers, @@ -60,8 +70,6 @@ def make_course_settings(course, user): 'category_map': utils.get_discussion_category_map(course, user) } - return obj - @newrelic.agent.function_trace() def get_threads(request, course, user_info, discussion_id=None, per_page=THREADS_PER_PAGE): @@ -185,7 +193,7 @@ def inline_discussion(request, course_key, discussion_id): try: threads, query_params = get_threads(request, course, user_info, discussion_id, per_page=INLINE_THREADS_PER_PAGE) except ValueError: - return HttpResponseBadRequest("Invalid group_id") + return HttpResponseServerError("Invalid group_id") with newrelic.agent.FunctionTrace(nr_transaction, "get_metadata_for_threads"): annotated_content_info = utils.get_metadata_for_threads(course_key, threads, request.user, user_info) @@ -214,31 +222,25 @@ def forum_form_discussion(request, course_key): nr_transaction = newrelic.agent.current_transaction() course = get_course_with_access(request.user, 'load', course_key, check_if_enrolled=True) - course_settings = make_course_settings(course, request.user) - - user = cc.User.from_django_user(request.user) - user_info = user.to_dict() + if request.is_ajax(): + user = cc.User.from_django_user(request.user) + user_info = user.to_dict() - try: - unsafethreads, query_params = get_threads(request, course, user_info) # This might process a search query - is_staff = has_permission(request.user, 'openclose_thread', course.id) - threads = [utils.prepare_content(thread, course_key, is_staff) for thread in unsafethreads] - except cc.utils.CommentClientMaintenanceError: - log.warning("Forum is in maintenance mode") - return render_to_response('discussion/maintenance.html', { - 'disable_courseware_js': True, - 'uses_pattern_library': True, - }) - except ValueError: - return HttpResponseBadRequest("Invalid group_id") + try: + unsafethreads, query_params = get_threads(request, course, user_info) # This might process a search query + is_staff = has_permission(request.user, 'openclose_thread', course.id) + threads = [utils.prepare_content(thread, course_key, is_staff) for thread in unsafethreads] + except cc.utils.CommentClientMaintenanceError: + return HttpResponseServerError('Forum is in maintenance mode', status=status.HTTP_503_SERVICE_UNAVAILABLE) + except ValueError: + return HttpResponseServerError("Invalid group_id") - with newrelic.agent.FunctionTrace(nr_transaction, "get_metadata_for_threads"): - annotated_content_info = utils.get_metadata_for_threads(course_key, threads, request.user, user_info) + with newrelic.agent.FunctionTrace(nr_transaction, "get_metadata_for_threads"): + annotated_content_info = utils.get_metadata_for_threads(course_key, threads, request.user, user_info) - with newrelic.agent.FunctionTrace(nr_transaction, "add_courseware_context"): - add_courseware_context(threads, course, request.user) + with newrelic.agent.FunctionTrace(nr_transaction, "add_courseware_context"): + add_courseware_context(threads, course, request.user) - if request.is_ajax(): return utils.JsonResponse({ 'discussion_data': threads, # TODO: Standardize on 'discussion_data' vs 'threads' 'annotated_content_info': annotated_content_info, @@ -247,39 +249,9 @@ def forum_form_discussion(request, course_key): 'corrected_text': query_params['corrected_text'], }) else: - with newrelic.agent.FunctionTrace(nr_transaction, "get_cohort_info"): - user_cohort_id = get_cohort_id(request.user, course_key) - - context = { - 'csrf': csrf(request)['csrf_token'], - 'course': course, - #'recent_active_threads': recent_active_threads, - 'staff_access': bool(has_access(request.user, 'staff', course)), - 'threads': threads, - 'thread_pages': query_params['num_pages'], - 'user_info': user_info, - 'can_create_comment': has_permission(request.user, "create_comment", course.id), - 'can_create_subcomment': has_permission(request.user, "create_sub_comment", course.id), - 'can_create_thread': has_permission(request.user, "create_thread", course.id), - 'flag_moderator': bool( - has_permission(request.user, 'openclose_thread', course.id) or - has_access(request.user, 'staff', course) - ), - 'annotated_content_info': annotated_content_info, - 'course_id': course.id.to_deprecated_string(), - 'roles': utils.get_role_ids(course_key), - 'is_moderator': has_permission(request.user, "see_all_cohorts", course_key), - 'cohorts': course_settings["cohorts"], # still needed to render _thread_list_template - 'user_cohort': user_cohort_id, # read from container in NewPostView - 'is_course_cohorted': is_course_cohorted(course_key), # still needed to render _thread_list_template - 'sort_preference': user.default_sort_key, - 'category_map': course_settings["category_map"], - 'course_settings': course_settings, - 'disable_courseware_js': True, - 'uses_pattern_library': True, - } - # print "start rendering.." - return render_to_response('discussion/discussion_board.html', context) + course_id = unicode(course.id) + tab_view = CourseTabView() + return tab_view.get(request, course_id, 'discussion') @require_GET @@ -296,37 +268,16 @@ def single_thread(request, course_key, discussion_id, thread_id): nr_transaction = newrelic.agent.current_transaction() course = get_course_with_access(request.user, 'load', course_key, check_if_enrolled=True) - course_settings = make_course_settings(course, request.user) - cc_user = cc.User.from_django_user(request.user) - user_info = cc_user.to_dict() - is_moderator = has_permission(request.user, "see_all_cohorts", course_key) - is_staff = has_permission(request.user, 'openclose_thread', course.id) - try: - thread = cc.Thread.find(thread_id).retrieve( - with_responses=request.is_ajax(), - recursive=request.is_ajax(), - user_id=request.user.id, - response_skip=request.GET.get("resp_skip"), - response_limit=request.GET.get("resp_limit") - ) - except cc.utils.CommentClientRequestError as error: - if error.status_code == 404: - raise Http404 - raise - - # Verify that the student has access to this thread if belongs to a course discussion module - thread_context = getattr(thread, "context", "course") - if thread_context == "course" and not utils.discussion_category_id_access(course, request.user, discussion_id): - raise Http404 + if request.is_ajax(): + cc_user = cc.User.from_django_user(request.user) + user_info = cc_user.to_dict() + is_staff = has_permission(request.user, 'openclose_thread', course.id) - # verify that the thread belongs to the requesting student's cohort - if is_commentable_cohorted(course_key, discussion_id) and not is_moderator: - user_group_id = get_cohort_id(request.user, course_key) - if getattr(thread, "group_id", None) is not None and user_group_id != thread.group_id: + thread = _find_thread(request, course, discussion_id=discussion_id, thread_id=thread_id) + if not thread: raise Http404 - if request.is_ajax(): with newrelic.agent.FunctionTrace(nr_transaction, "get_annotated_content_infos"): annotated_content_info = utils.get_annotated_content_infos( course_key, @@ -344,57 +295,136 @@ def single_thread(request, course_key, discussion_id, thread_id): 'annotated_content_info': annotated_content_info, }) else: + course_id = unicode(course.id) + tab_view = CourseTabView() + return tab_view.get(request, course_id, 'discussion', discussion_id=discussion_id, thread_id=thread_id) + + +def _find_thread(request, course, discussion_id, thread_id): + """ + Finds the discussion thread with the specified ID. + + Args: + request: The Django request. + course_id: The ID of the owning course. + discussion_id: The ID of the owning discussion. + thread_id: The ID of the thread. + + Returns: + The thread in question if the user can see it, else None. + """ + try: + thread = cc.Thread.find(thread_id).retrieve( + with_responses=request.is_ajax(), + recursive=request.is_ajax(), + user_id=request.user.id, + response_skip=request.GET.get("resp_skip"), + response_limit=request.GET.get("resp_limit") + ) + except cc.utils.CommentClientRequestError: + return None + + # Verify that the student has access to this thread if belongs to a course discussion module + thread_context = getattr(thread, "context", "course") + if thread_context == "course" and not utils.discussion_category_id_access(course, request.user, discussion_id): + return None + + # verify that the thread belongs to the requesting student's cohort + is_moderator = has_permission(request.user, "see_all_cohorts", course.id) + if is_commentable_cohorted(course.id, discussion_id) and not is_moderator: + user_group_id = get_cohort_id(request.user, course.id) + if getattr(thread, "group_id", None) is not None and user_group_id != thread.group_id: + return None + + return thread + + +def _create_base_discussion_view_context(request, course_key): + """ + Returns the default template context for rendering any discussion view. + """ + user = request.user + cc_user = cc.User.from_django_user(user) + user_info = cc_user.to_dict() + course = get_course_with_access(user, 'load', course_key, check_if_enrolled=True) + course_settings = make_course_settings(course, user) + return { + 'csrf': csrf(request)['csrf_token'], + 'course': course, + 'user': user, + 'user_info': user_info, + 'staff_access': bool(has_access(user, 'staff', course)), + 'roles': utils.get_role_ids(course_key), + 'can_create_comment': has_permission(user, "create_comment", course.id), + 'can_create_subcomment': has_permission(user, "create_sub_comment", course.id), + 'can_create_thread': has_permission(user, "create_thread", course.id), + 'flag_moderator': bool( + has_permission(user, 'openclose_thread', course.id) or + has_access(user, 'staff', course) + ), + 'course_settings': course_settings, + 'disable_courseware_js': True, + 'uses_pattern_library': True, + } + + +def _create_discussion_board_context(request, course_key, discussion_id=None, thread_id=None): + """ + Returns the template context for rendering the discussion board. + """ + nr_transaction = newrelic.agent.current_transaction() + context = _create_base_discussion_view_context(request, course_key) + course = context['course'] + course_settings = context['course_settings'] + user = context['user'] + cc_user = cc.User.from_django_user(user) + user_info = context['user_info'] + if thread_id: + thread = _find_thread(request, course, discussion_id=discussion_id, thread_id=thread_id) + if not thread: + raise Http404 + # Since we're in page render mode, and the discussions UI will request the thread list itself, # we need only return the thread information for this one. threads = [thread.to_dict()] - with newrelic.agent.FunctionTrace(nr_transaction, "add_courseware_context"): - add_courseware_context(threads, course, request.user) - for thread in threads: # patch for backward compatibility with comments service if "pinned" not in thread: thread["pinned"] = False + thread_pages = 1 + root_url = reverse('forum_form_discussion', args=[unicode(course.id)]) + else: + threads, query_params = get_threads(request, course, user_info) # This might process a search query + thread_pages = query_params['num_pages'] + root_url = request.path + is_staff = has_permission(user, 'openclose_thread', course.id) + threads = [utils.prepare_content(thread, course_key, is_staff) for thread in threads] - threads = [utils.prepare_content(thread, course_key, is_staff) for thread in threads] + with newrelic.agent.FunctionTrace(nr_transaction, "get_metadata_for_threads"): + annotated_content_info = utils.get_metadata_for_threads(course_key, threads, user, user_info) - with newrelic.agent.FunctionTrace(nr_transaction, "get_metadata_for_threads"): - annotated_content_info = utils.get_metadata_for_threads(course_key, threads, request.user, user_info) + with newrelic.agent.FunctionTrace(nr_transaction, "add_courseware_context"): + add_courseware_context(threads, course, user) - with newrelic.agent.FunctionTrace(nr_transaction, "get_cohort_info"): - user_cohort = get_cohort_id(request.user, course_key) - - context = { - 'discussion_id': discussion_id, - 'csrf': csrf(request)['csrf_token'], - 'init': '', # TODO: What is this? - 'user_info': user_info, - 'can_create_comment': has_permission(request.user, "create_comment", course.id), - 'can_create_subcomment': has_permission(request.user, "create_sub_comment", course.id), - 'can_create_thread': has_permission(request.user, "create_thread", course.id), - 'annotated_content_info': annotated_content_info, - 'course': course, - #'recent_active_threads': recent_active_threads, - 'course_id': course.id.to_deprecated_string(), # TODO: Why pass both course and course.id to template? - 'thread_id': thread_id, - 'threads': threads, - 'roles': utils.get_role_ids(course_key), - 'is_moderator': is_moderator, - 'thread_pages': 1, - 'is_course_cohorted': is_course_cohorted(course_key), - 'flag_moderator': bool( - has_permission(request.user, 'openclose_thread', course.id) or - has_access(request.user, 'staff', course) - ), - 'cohorts': course_settings["cohorts"], - 'user_cohort': user_cohort, - 'sort_preference': cc_user.default_sort_key, - 'category_map': course_settings["category_map"], - 'course_settings': course_settings, - 'disable_courseware_js': True, - 'uses_pattern_library': True, - } - return render_to_response('discussion/discussion_board.html', context) + with newrelic.agent.FunctionTrace(nr_transaction, "get_cohort_info"): + user_cohort_id = get_cohort_id(user, course_key) + + context.update({ + 'root_url': root_url, + 'discussion_id': discussion_id, + 'thread_id': thread_id, + 'threads': threads, + 'thread_pages': thread_pages, + 'annotated_content_info': annotated_content_info, + 'is_moderator': has_permission(user, "see_all_cohorts", course_key), + 'cohorts': course_settings["cohorts"], # still needed to render _thread_list_template + 'user_cohort': user_cohort_id, # read from container in NewPostView + 'sort_preference': cc_user.default_sort_key, + 'category_map': course_settings["category_map"], + 'course_settings': course_settings, + }) + return context @require_GET @@ -409,9 +439,7 @@ def user_profile(request, course_key, user_id): nr_transaction = newrelic.agent.current_transaction() user = cc.User.from_django_user(request.user) - user_info = user.to_dict() course = get_course_with_access(request.user, 'load', course_key, check_if_enrolled=True) - course_settings = make_course_settings(course, request.user) try: # If user is not enrolled in the course, do not proceed. @@ -427,7 +455,7 @@ def user_profile(request, course_key, user_id): try: group_id = get_group_id_for_comments_service(request, course_key) except ValueError: - return HttpResponseBadRequest("Invalid group_id") + return HttpResponseServerError("Invalid group_id") if group_id is not None: query_params['group_id'] = group_id profiled_user = cc.User(id=user_id, course_id=course_key, group_id=group_id) @@ -437,9 +465,9 @@ def user_profile(request, course_key, user_id): threads, page, num_pages = profiled_user.active_threads(query_params) query_params['page'] = page query_params['num_pages'] = num_pages - user_info = cc.User.from_django_user(request.user).to_dict() with newrelic.agent.FunctionTrace(nr_transaction, "get_metadata_for_threads"): + user_info = cc.User.from_django_user(request.user).to_dict() annotated_content_info = utils.get_metadata_for_threads(course_key, threads, request.user, user_info) is_staff = has_permission(request.user, 'openclose_thread', course.id) @@ -461,32 +489,19 @@ def user_profile(request, course_key, user_id): with newrelic.agent.FunctionTrace(nr_transaction, "get_cohort_info"): user_cohort_id = get_cohort_id(request.user, course_key) - context = { - 'course': course, - 'user': request.user, + context = _create_base_discussion_view_context(request, course_key) + context.update({ 'django_user': django_user, 'django_user_roles': user_roles, 'profiled_user': profiled_user.to_dict(), 'threads': threads, - 'user_info': user_info, - 'roles': utils.get_role_ids(course_key), - 'can_create_comment': has_permission(request.user, "create_comment", course.id), - 'can_create_subcomment': has_permission(request.user, "create_sub_comment", course.id), - 'can_create_thread': has_permission(request.user, "create_thread", course.id), - 'flag_moderator': bool( - has_permission(request.user, 'openclose_thread', course.id) or - has_access(request.user, 'staff', course) - ), 'user_cohort': user_cohort_id, 'annotated_content_info': annotated_content_info, 'page': query_params['page'], 'num_pages': query_params['num_pages'], 'sort_preference': user.default_sort_key, - 'course_settings': course_settings, 'learner_profile_page_url': reverse('learner_profile', kwargs={'username': django_user.username}), - 'disable_courseware_js': True, - 'uses_pattern_library': True, - } + }) return render_to_response('discussion/discussion_profile_page.html', context) except User.DoesNotExist: @@ -531,7 +546,7 @@ def followed_threads(request, course_key, user_id): try: group_id = get_group_id_for_comments_service(request, course_key) except ValueError: - return HttpResponseBadRequest("Invalid group_id") + return HttpResponseServerError("Invalid group_id") if group_id is not None: query_params['group_id'] = group_id @@ -574,3 +589,81 @@ def followed_threads(request, course_key, user_id): return render_to_response('discussion/user_profile.html', context) except User.DoesNotExist: raise Http404 + + +class DiscussionBoardFragmentView(EdxFragmentView): + """ + Component implementation of the discussion board. + """ + def render_to_fragment(self, request, course_id=None, discussion_id=None, thread_id=None, **kwargs): + """ + Render the discussion board to a fragment. + + Args: + request: The Django request. + course_id: The id of the course in question. + discussion_id: An optional discussion ID to be focused upon. + thread_id: An optional ID of the thread to be shown. + + Returns: + Fragment: The fragment representing the discussion board + """ + course_key = CourseKey.from_string(course_id) + try: + context = _create_discussion_board_context( + request, + course_key, + discussion_id=discussion_id, + thread_id=thread_id, + ) + html = render_to_string('discussion/discussion_board_fragment.html', context) + inline_js = render_to_string('discussion/discussion_board_js.template', context) + + fragment = Fragment(html) + self.add_fragment_resource_urls(fragment) + fragment.add_javascript(inline_js) + if not settings.REQUIRE_DEBUG: + fragment.add_javascript_url(staticfiles_storage.url('discussion/js/discussion_board_factory.js')) + return fragment + except cc.utils.CommentClientMaintenanceError: + log.warning('Forum is in maintenance mode') + html = render_to_response('discussion/maintenance_fragment.html', { + 'disable_courseware_js': True, + 'uses_pattern_library': True, + }) + return Fragment(html) + + def vendor_js_dependencies(self): + """ + Returns list of vendor JS files that this view depends on. + + The helper function that it uses to obtain the list of vendor JS files + works in conjunction with the Django pipeline to ensure that in development mode + the files are loaded individually, but in production just the single bundle is loaded. + """ + dependencies = Set() + dependencies.update(self.get_js_dependencies('discussion_vendor')) + return list(dependencies) + + def js_dependencies(self): + """ + Returns list of JS files that this view depends on. + + The helper function that it uses to obtain the list of JS files + works in conjunction with the Django pipeline to ensure that in development mode + the files are loaded individually, but in production just the single bundle is loaded. + """ + return self.get_js_dependencies('discussion') + + def css_dependencies(self): + """ + Returns list of CSS files that this view depends on. + + The helper function that it uses to obtain the list of CSS files + works in conjunction with the Django pipeline to ensure that in development mode + the files are loaded individually, but in production just the single bundle is loaded. + """ + if get_language_bidi(): + return self.get_css_dependencies('style-discussion-main-rtl') + else: + return self.get_css_dependencies('style-discussion-main') diff --git a/lms/djangoapps/django_comment_client/base/views.py b/lms/djangoapps/django_comment_client/base/views.py index 994e585e84f..31cc366c265 100644 --- a/lms/djangoapps/django_comment_client/base/views.py +++ b/lms/djangoapps/django_comment_client/base/views.py @@ -8,7 +8,7 @@ import urlparse from django.contrib.auth.decorators import login_required from django.contrib.auth.models import User from django.core import exceptions -from django.http import Http404, HttpResponseBadRequest, HttpResponse +from django.http import Http404, HttpResponse, HttpResponseServerError from django.utils.translation import ugettext as _ from django.views.decorators import csrf from django.views.decorators.http import require_GET, require_POST @@ -243,7 +243,7 @@ def create_thread(request, course_id, commentable_id): try: group_id = get_group_id_for_comments_service(request, course_key, commentable_id) except ValueError: - return HttpResponseBadRequest("Invalid cohort id") + return HttpResponseServerError("Invalid cohort id") if group_id is not None: thread.group_id = group_id diff --git a/lms/djangoapps/django_comment_client/tests/group_id.py b/lms/djangoapps/django_comment_client/tests/group_id.py index b25e6d091e1..3c7b30d045d 100644 --- a/lms/djangoapps/django_comment_client/tests/group_id.py +++ b/lms/djangoapps/django_comment_client/tests/group_id.py @@ -94,8 +94,11 @@ class CohortedTopicGroupIdTestMixin(GroupIdAssertionMixin): def test_cohorted_topic_moderator_with_invalid_group_id(self, mock_request): invalid_id = self.student_cohort.id + self.moderator_cohort.id - response = self.call_view(mock_request, "cohorted_topic", self.moderator, invalid_id) - self.assertEqual(response.status_code, 400) + try: + response = self.call_view(mock_request, "cohorted_topic", self.moderator, invalid_id) + self.assertEqual(response.status_code, 500) + except ValueError: + pass # In mock request mode, server errors are not captured class NonCohortedTopicGroupIdTestMixin(GroupIdAssertionMixin): diff --git a/lms/envs/common.py b/lms/envs/common.py index 51810353658..fb4dfab1e16 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -544,6 +544,9 @@ TEMPLATES = [ ] DEFAULT_TEMPLATE_ENGINE = TEMPLATES[0] +# The template used to render a web fragment as a standalone page +STANDALONE_FRAGMENT_VIEW_TEMPLATE = 'fragment-view-chromeless.html' + ############################################################################################### # use the ratelimit backend to prevent brute force attacks @@ -1927,6 +1930,10 @@ INSTALLED_APPS = ( 'pipeline', 'static_replace', + # For user interface plugins + 'web_fragments', + 'openedx.core.djangoapps.plugin_api', + # For content serving 'openedx.core.djangoapps.contentserver', diff --git a/lms/templates/courseware/error-message.html b/lms/templates/courseware/error-message.html index 391476569d3..27bb43202f6 100644 --- a/lms/templates/courseware/error-message.html +++ b/lms/templates/courseware/error-message.html @@ -1,3 +1,5 @@ +## mako + <%! from django.utils.translation import ugettext as _ %> <% tech_support_email='<a href=\"mailto:{tech_support_email}\">{tech_support_email}</a>'.format(tech_support_email=settings.TECH_SUPPORT_EMAIL) diff --git a/lms/templates/courseware/static_tab.html b/lms/templates/courseware/static_tab.html index efb16914315..ab18be3961f 100644 --- a/lms/templates/courseware/static_tab.html +++ b/lms/templates/courseware/static_tab.html @@ -1,7 +1,5 @@ ## mako - - <%page expression_filter="h"/> <%! from openedx.core.djangolib.markup import HTML @@ -11,19 +9,18 @@ from openedx.core.djangolib.markup import HTML <%block name="bodyclass">view-in-course view-statictab ${course.css_class or ''}</%block> <%namespace name='static' file='/static_content.html'/> -<%block name="headextra"> +<%block name="head_extra"> <%static:css group='style-course-vendor'/> <%static:css group='style-course'/> ${HTML(fragment.head_html())} </%block> -<%block name="js_extra"> +<%block name="footer_extra"> <%include file="/mathjax_include.html" args="disable_fast_preview=True"/> ${HTML(fragment.foot_html())} </%block> - -<%block name="pagetitle">${tab['name']} | ${course.display_number_with_default | h}</%block> +<%block name="pagetitle">${tab['name']} | ${course.display_number_with_default}</%block> <%include file="/courseware/course_navigation.html" args="active_page=active_page" /> diff --git a/lms/templates/courseware/tab-view.html b/lms/templates/courseware/tab-view.html new file mode 100644 index 00000000000..cbef28204f7 --- /dev/null +++ b/lms/templates/courseware/tab-view.html @@ -0,0 +1,32 @@ +## mako + +<%! main_css = "style-main-v2" %> + +<%page expression_filter="h"/> +<%! +from openedx.core.djangolib.markup import HTML +%> + +<%inherit file="/main.html" /> +<%namespace name='static' file='/static_content.html'/> + +<%block name="bodyclass">${tab['body_class']}</%block> + +<%def name="online_help_token()"><% return "${tab['online_help_token']}" %></%def> + +<%block name="pagetitle">${tab['name']} | ${course.display_number_with_default}</%block> + +<%include file="/courseware/course_navigation.html" args="active_page=active_page" /> + +<%block name="head_extra"> +${HTML(fragment.head_html())} +</%block> + +<%block name="footer_extra"> +<%include file="/mathjax_include.html" args="disable_fast_preview=True"/> +${HTML(fragment.foot_html())} +</%block> + +<%block name="content"> + ${HTML(fragment.body_html())} +</%block> diff --git a/lms/templates/fragment-view-chromeless.html b/lms/templates/fragment-view-chromeless.html new file mode 100644 index 00000000000..68a13c355df --- /dev/null +++ b/lms/templates/fragment-view-chromeless.html @@ -0,0 +1,24 @@ +## mako + +<%! main_css = "style-main-v2" %> + +<%page expression_filter="h"/> +<%inherit file="/main.html" /> + +<%namespace name='static' file='static_content.html'/> + +<%! from openedx.core.djangolib.markup import HTML %> + +<% header_file = None %> + +<%block name="head_extra"> + ${HTML(fragment.head_html())} +</%block> + +<%block name="footer_extra"> + ${HTML(fragment.foot_html())} +</%block> + +<div class="content-wrapper" id="container"> + ${HTML(fragment.body_html())} +</div> diff --git a/lms/templates/main.html b/lms/templates/main.html index 41369b42d1e..5c3d01e2ffe 100644 --- a/lms/templates/main.html +++ b/lms/templates/main.html @@ -96,6 +96,7 @@ from pipeline_mako import render_require_js_path_overrides % endif <%block name="headextra"/> + <%block name="head_extra"/> <%static:optional_include_mako file="head-extra.html" is_theming_enabled="True" /> @@ -148,6 +149,7 @@ from pipeline_mako import render_require_js_path_overrides </div> % endif + <%block name="footer_extra"/> <%block name="js_extra"/> <%include file="widgets/segment-io-footer.html" /> diff --git a/lms/urls.py b/lms/urls.py index 92c00768bfc..13ad85f5ec4 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -8,7 +8,7 @@ from django.views.generic.base import RedirectView from ratelimitbackend import admin from django.conf.urls.static import static -from courseware.views.views import EnrollStaffView +from courseware.views.views import CourseTabView, EnrollStaffView, StaticCourseTabView from config_models.views import ConfigurationModelCurrentAPIView from courseware.views.index import CoursewareIndex from openedx.core.djangoapps.auth_exchange.views import LoginWithAccessTokenView @@ -691,8 +691,8 @@ urlpatterns += ( r'^courses/{}/tab/(?P<tab_type>[^/]+)/$'.format( settings.COURSE_ID_PATTERN, ), - 'courseware.views.views.content_tab', - name='content_tab', + CourseTabView.as_view(), + name='course_tab_view', ), ) @@ -702,7 +702,7 @@ urlpatterns += ( r'^courses/{}/(?P<tab_slug>[^/]+)/$'.format( settings.COURSE_ID_PATTERN, ), - 'courseware.views.views.static_tab', + StaticCourseTabView.as_view(), name='static_tab', ), ) diff --git a/openedx/core/djangoapps/plugin_api/__init__.py b/openedx/core/djangoapps/plugin_api/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/openedx/core/djangoapps/plugin_api/views.py b/openedx/core/djangoapps/plugin_api/views.py new file mode 100644 index 00000000000..668eea6caae --- /dev/null +++ b/openedx/core/djangoapps/plugin_api/views.py @@ -0,0 +1,99 @@ +""" +Views for building plugins. +""" + +from abc import abstractmethod +import logging + +from django.conf import settings +from django.contrib.staticfiles.storage import staticfiles_storage +from django.shortcuts import render_to_response +from web_fragments.views import FragmentView + +log = logging.getLogger('plugin_api') + + +class EdxFragmentView(FragmentView): + """ + The base class of all Open edX fragment views. + """ + USES_PATTERN_LIBRARY = True + + page_title = None + + @staticmethod + def get_css_dependencies(group): + """ + Returns list of CSS dependencies belonging to `group` in settings.PIPELINE_JS. + + Respects `PIPELINE_ENABLED` setting. + """ + if settings.PIPELINE_ENABLED: + return [settings.PIPELINE_CSS[group]['output_filename']] + else: + return settings.PIPELINE_CSS[group]['source_filenames'] + + @staticmethod + def get_js_dependencies(group): + """ + Returns list of JS dependencies belonging to `group` in settings.PIPELINE_JS. + + Respects `PIPELINE_ENABLED` setting. + """ + if settings.PIPELINE_ENABLED: + return [settings.PIPELINE_JS[group]['output_filename']] + else: + return settings.PIPELINE_JS[group]['source_filenames'] + + @abstractmethod + def vendor_js_dependencies(self): + """ + Returns list of the vendor JS files that this view depends on. + """ + return [] + + @abstractmethod + def js_dependencies(self): + """ + Returns list of the JavaScript files that this view depends on. + """ + return [] + + @abstractmethod + def css_dependencies(self): + """ + Returns list of the CSS files that this view depends on. + """ + return [] + + def add_fragment_resource_urls(self, fragment): + """ + Adds URLs for JS and CSS resources needed by this fragment. + """ + # Head dependencies + for vendor_js_file in self.vendor_js_dependencies(): + fragment.add_resource_url(staticfiles_storage.url(vendor_js_file), 'application/javascript', 'head') + + for css_file in self.css_dependencies(): + fragment.add_css_url(staticfiles_storage.url(css_file)) + + # Body dependencies + for js_file in self.js_dependencies(): + fragment.add_javascript_url(staticfiles_storage.url(js_file)) + + def render_to_standalone_html(self, request, fragment, **kwargs): + """ + Renders this fragment to HTML for a standalone page. + """ + context = { + 'uses-pattern-library': self.USES_PATTERN_LIBRARY, + 'settings': settings, + 'fragment': fragment, + 'disable_accordion': True, + 'allow_iframing': True, + 'disable_header': True, + 'disable_footer': True, + 'disable_window_wrap': True, + 'disable_preview_menu': True, + } + return render_to_response(settings.STANDALONE_FRAGMENT_VIEW_TEMPLATE, context) diff --git a/openedx/core/lib/tests/test_course_tab_api.py b/openedx/core/lib/tests/test_course_tab_api.py index 54b7639f44c..44537e295b8 100644 --- a/openedx/core/lib/tests/test_course_tab_api.py +++ b/openedx/core/lib/tests/test_course_tab_api.py @@ -10,9 +10,9 @@ from openedx.core.lib.course_tabs import CourseTabPluginManager @attr(shard=2) -class TestPluginApi(TestCase): +class TestCourseTabApi(TestCase): """ - Unit tests for the plugin API + Unit tests for the course tab plugin API """ def test_get_plugin(self): diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index f8cdc5516a2..5f89005797f 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -206,5 +206,9 @@ py2neo==3.1.2 # for calculating coverage -r coverage.txt +# Support for plugins +web-fragments==0.2.1 +xblock==0.4.14 + # Third Party XBlocks edx-sga==0.6.2 diff --git a/requirements/edx/github.txt b/requirements/edx/github.txt index 86012723627..1e81644d2f5 100644 --- a/requirements/edx/github.txt +++ b/requirements/edx/github.txt @@ -71,7 +71,6 @@ git+https://github.com/edx/rfc6266.git@v0.0.5-edx#egg=rfc6266==0.0.5-edx git+https://github.com/edx/lettuce.git@0.2.20.002#egg=lettuce==0.2.20.002 # Our libraries: -git+https://github.com/edx/XBlock.git@xblock-0.4.13#egg=XBlock==0.4.13 -e git+https://github.com/edx/codejail.git@a320d43ce6b9c93b17636b2491f724d9e433be47#egg=codejail==0.0 -e git+https://github.com/edx/event-tracking.git@0.2.1#egg=event-tracking==0.2.1 -e git+https://github.com/edx/django-splash.git@v0.2#egg=django-splash==0.2 -- GitLab