Merge pull request #14366 from edx/release-candidate

Merge Release candidate to Release

common/static/common/js/discussion/discussion.js

@@ -99,7 +99,7 @@
                     data.text = options.search_text;
                     break;
                 case 'commentables':
-                    url = DiscussionUtil.urlFor('search');
+                    url = DiscussionUtil.urlFor('retrieve_discussion', options.commentable_ids);
                     data.commentable_ids = options.commentable_ids;
                     break;
                 case 'all':
@@ -107,6 +107,10 @@
                     break;
                 case 'followed':
                     url = DiscussionUtil.urlFor('followed_threads', options.user_id);
+                    break;
+                case 'user':
+                    url = DiscussionUtil.urlFor('user_profile', options.user_id);
+                    break;
                 }
                 if (options.group_id) {
                     data.group_id = options.group_id;

common/static/common/js/discussion/views/discussion_inline_view.js

@@ -39,6 +39,9 @@
                 this.page = 1;
             }
 
+            this.defaultSortKey = 'activity';
+            this.defaultSortOrder = 'desc';
+
             // By default the view is displayed in a hidden state. If you want it to be shown by default (e.g. in Teams)
             // pass showByDefault as an option. This code will open it on initialization.
             if (this.showByDefault) {
@@ -48,7 +51,8 @@
 
         loadDiscussions: function($elem, error) {
             var discussionId = this.$el.data('discussion-id'),
-                url = DiscussionUtil.urlFor('retrieve_discussion', discussionId) + ('?page=' + this.page),
+                url = DiscussionUtil.urlFor('retrieve_discussion', discussionId) + ('?page=' + this.page)
+                    + ('&sort_key=' + this.defaultSortKey) + ('&sort_order=' + this.defaultSortOrder),
                 self = this;
 
             DiscussionUtil.safeAjax({
@@ -100,8 +104,7 @@
             this.threadListView = new DiscussionThreadListView({
                 el: this.$('.inline-threads'),
                 collection: self.discussion,
-                courseSettings: self.course_settings,
-                hideRefineBar: true  // TODO: re-enable the search/filter bar when it works correctly
+                courseSettings: self.course_settings
             });
 
             this.threadListView.render();

common/static/common/js/discussion/views/discussion_thread_list_view.js

@@ -91,14 +91,13 @@
             DiscussionThreadListView.prototype.initialize = function(options) {
                 var self = this;
                 this.courseSettings = options.courseSettings;
-                this.hideRefineBar = options.hideRefineBar;
                 this.supportsActiveThread = options.supportsActiveThread;
                 this.hideReadState = options.hideReadState || false;
                 this.displayedCollection = new Discussion(this.collection.models, {
                     pages: this.collection.pages
                 });
                 this.collection.on('change', this.reloadDisplayedCollection);
-                this.discussionIds = '';
+                this.discussionIds = this.$el.data('discussion-id') || '';
                 this.collection.on('reset', function(discussion) {
                     self.displayedCollection.current_page = discussion.current_page;
                     self.displayedCollection.pages = discussion.pages;
@@ -109,7 +108,7 @@
                 this.sidebar_padding = 10;
                 this.boardName = null;
                 this.current_search = '';
-                this.mode = 'all';
+                this.mode = options.mode || 'commentables';
                 this.showThreadPreview = true;
                 this.searchAlertCollection = new Backbone.Collection([], {
                     model: Backbone.Model
@@ -199,6 +198,9 @@
                         isPrivilegedUser: DiscussionUtil.isPrivilegedUser()
                     })
                 );
+                if (this.hideReadState) {
+                    this.$('.forum-nav-filter-main').addClass('is-hidden');
+                }
                 this.$('.forum-nav-sort-control option').removeProp('selected');
                 this.$('.forum-nav-sort-control option[value=' + this.collection.sort_preference + ']')
                     .prop('selected', true);
@@ -223,9 +225,6 @@
                 }
                 this.showMetadataAccordingToSort();
                 this.renderMorePages();
-                if (this.hideRefineBar) {
-                    this.$('.forum-nav-refine-bar').addClass('is-hidden');
-                }
                 this.trigger('threads:rendered');
             };
 
@@ -284,6 +283,9 @@
                 case 'followed':
                     options.user_id = window.user.id;
                     break;
+                case 'user':
+                    options.user_id = this.$el.parent().data('user-id');
+                    break;
                 case 'commentables':
                     options.commentable_ids = this.discussionIds;
                     if (this.group_id) {
@@ -319,6 +321,11 @@
                         gettext('Additional posts could not be loaded. Refresh the page and try again.')
                     );
                 };
+                /*
+                The options object is being passed to the function below from discussion/discussion.js
+                which correspondingly forms the ajax url based on the mode via the DiscussionUtil.urlFor
+                from discussion/utils.js
+                */
                 return this.collection.retrieveAnotherPage(this.mode, options, {
                     sort_key: this.$('.forum-nav-sort-control').val()
                 }, error);

common/static/common/js/spec/discussion/view/discussion_thread_list_view_spec.js

@@ -364,6 +364,7 @@
                 });
                 sortControl.val(newType).change();
                 expect($.ajax).toHaveBeenCalled();
+                expect(view.mode).toBe('commentables');
                 checkThreadsOrdering(view, sortOrder, newType);
             };
 

common/static/common/templates/discussion/inline-discussion.underscore

@@ -6,7 +6,7 @@
     <article class="new-post-article is-hidden"></article>
 
     <div class="inline-discussion-thread-container">
-        <section class="inline-threads">
+        <section class="inline-threads" data-discussion-id="<%- discussionId %>">
         </section>
 
         <div class="inline-thread">

lms/djangoapps/courseware/access.py

@@ -469,6 +469,10 @@ def _has_group_access(descriptor, user, course_key):
         # via updating the children of the split_test module.
         return ACCESS_GRANTED
 
+    # Allow staff and instructors roles group access, as they are not masquerading as a student.
+    if get_user_role(user, course_key) in ['staff', 'instructor']:
+        return ACCESS_GRANTED
+
     # use merged_group_access which takes group access on the block's
     # parents / ancestors into account
     merged_access = descriptor.merged_group_access
@@ -550,14 +554,20 @@ def _has_access_descriptor(user, action, descriptor, course_key=None):
         students to see modules.  If not, views should check the course, so we
         don't have to hit the enrollments table on every module load.
         """
+        # If the user (or the role the user is currently masquerading as) does not have
+        # access to this content, then deny access. The problem with calling _has_staff_access_to_descriptor
+        # before this method is that _has_staff_access_to_descriptor short-circuits and returns True
+        # for staff users in preview mode.
+        if not _has_group_access(descriptor, user, course_key):
+            return ACCESS_DENIED
+
+        # If the user has staff access, they can load the module and checks below are not needed.
         if _has_staff_access_to_descriptor(user, descriptor, course_key):
             return ACCESS_GRANTED
 
-        # if the user has staff access, they can load the module so this code doesn't need to run
         return (
             _visible_to_nonstaff_users(descriptor) and
             _can_access_descriptor_with_milestones(user, descriptor, course_key) and
-            _has_group_access(descriptor, user, course_key) and
             (
                 _has_detached_class_tag(descriptor) or
                 _can_access_descriptor_with_start_date(user, descriptor, course_key)

lms/djangoapps/courseware/tests/helpers.py

 """
 Helpers for courseware tests.
 """
-import crum
 import json
 
 from django.contrib.auth.models import User
@@ -10,6 +9,10 @@ from django.test import TestCase
 from django.test.client import RequestFactory
 
 from courseware.access import has_access
+from courseware.masquerade import (
+    handle_ajax,
+    setup_masquerade
+)
 from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
 from student.models import Registration
 
@@ -178,3 +181,39 @@ class CourseAccessTestMixin(TestCase):
         """
         self.assertFalse(has_access(user, action, course))
         self.assertFalse(has_access(user, action, CourseOverview.get_from_id(course.id)))
+
+
+def masquerade_as_group_member(user, course, partition_id, group_id):
+    """
+    Installs a masquerade for the specified user and course, to enable
+    the user to masquerade as belonging to the specific partition/group
+    combination.
+
+    Arguments:
+        user (User): a user.
+        course (CourseDescriptor): a course.
+        partition_id (int): the integer partition id, referring to partitions already
+           configured in the course.
+        group_id (int); the integer group id, within the specified partition.
+
+    Returns: the status code for the AJAX response to update the user's masquerade for
+        the specified course.
+    """
+    request = _create_mock_json_request(
+        user,
+        data={"role": "student", "user_partition_id": partition_id, "group_id": group_id}
+    )
+    response = handle_ajax(request, unicode(course.id))
+    setup_masquerade(request, course.id, True)
+    return response.status_code
+
+
+def _create_mock_json_request(user, data, method='POST'):
+    """
+    Returns a mock JSON request for the specified user.
+    """
+    factory = RequestFactory()
+    request = factory.generic(method, '/', content_type='application/json', data=json.dumps(data))
+    request.user = user
+    request.session = {}
+    return request

lms/djangoapps/courseware/tests/test_access.py

@@ -27,7 +27,7 @@ from courseware.tests.factories import (
     StaffFactory,
     UserFactory,
 )
-from courseware.tests.helpers import LoginEnrollmentTestCase
+from courseware.tests.helpers import LoginEnrollmentTestCase, masquerade_as_group_member
 from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
 from student.models import CourseEnrollment
 from student.roles import CourseCcxCoachRole, CourseStaffRole
@@ -44,6 +44,9 @@ from xmodule.course_module import (
     CATALOG_VISIBILITY_NONE,
 )
 from xmodule.error_module import ErrorDescriptor
+from xmodule.partitions.partitions import Group, UserPartition
+from xmodule.modulestore import ModuleStoreEnum
+from xmodule.modulestore.django import modulestore
 from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
 from xmodule.modulestore.tests.django_utils import (
     ModuleStoreTestCase,
@@ -293,6 +296,57 @@ class AccessTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase, MilestonesTes
                 bool(access.has_access(self.student, 'staff', self.course, course_key=self.course.id))
             )
 
+    @patch('courseware.access.in_preview_mode', Mock(return_value=True))
+    def test_has_access_in_preview_mode_with_group(self):
+        """
+        Test that a user masquerading as a member of a group sees appropriate content in preview mode.
+        """
+        partition_id = 0
+        group_0_id = 0
+        group_1_id = 1
+        user_partition = UserPartition(
+            partition_id, 'Test User Partition', '',
+            [Group(group_0_id, 'Group 1'), Group(group_1_id, 'Group 2')],
+            scheme_id='cohort'
+        )
+        self.course.user_partitions.append(user_partition)
+        self.course.cohort_config = {'cohorted': True}
+
+        chapter = ItemFactory.create(category="chapter", parent_location=self.course.location)
+        chapter.group_access = {partition_id: [group_0_id]}
+        chapter.user_partitions = self.course.user_partitions
+
+        modulestore().update_item(self.course, ModuleStoreEnum.UserID.test)
+
+        # User should not be able to preview when masquerading as student (and not in the group above).
+        with patch('courseware.access.get_user_role') as mock_user_role:
+            mock_user_role.return_value = 'student'
+            self.assertFalse(
+                bool(access.has_access(self.global_staff, 'load', chapter, course_key=self.course.id))
+            )
+
+        # Should be able to preview when in staff or instructor role.
+        for mocked_role in ['staff', 'instructor']:
+            with patch('courseware.access.get_user_role') as mock_user_role:
+                mock_user_role.return_value = mocked_role
+                self.assertTrue(
+                    bool(access.has_access(self.global_staff, 'load', chapter, course_key=self.course.id))
+                )
+
+        # Now install masquerade group and set staff as a member of that.
+        self.assertEqual(200, masquerade_as_group_member(self.global_staff, self.course, partition_id, group_0_id))
+        # Can load the chapter since user is in the group.
+        self.assertTrue(
+            bool(access.has_access(self.global_staff, 'load', chapter, course_key=self.course.id))
+        )
+
+        # Move the user to be a part of the second group.
+        self.assertEqual(200, masquerade_as_group_member(self.global_staff, self.course, partition_id, group_1_id))
+        # Cannot load the chapter since user is in a different group.
+        self.assertFalse(
+            bool(access.has_access(self.global_staff, 'load', chapter, course_key=self.course.id))
+        )
+
     def test_has_access_to_course(self):
         self.assertFalse(access._has_access_to_course(
             None, 'staff', self.course.id

lms/djangoapps/courseware/tests/test_masquerade.py

@@ -8,7 +8,7 @@ from nose.plugins.attrib import attr
 from datetime import datetime
 
 from django.core.urlresolvers import reverse
-from django.test import TestCase, RequestFactory
+from django.test import TestCase
 from django.utils.timezone import UTC
 
 from capa.tests.response_xml_factory import OptionResponseXMLFactory
@@ -20,7 +20,7 @@ from courseware.masquerade import (
     get_masquerading_group_info
 )
 from courseware.tests.factories import StaffFactory
-from courseware.tests.helpers import LoginEnrollmentTestCase
+from courseware.tests.helpers import LoginEnrollmentTestCase, masquerade_as_group_member
 from courseware.tests.test_submitting_problems import ProblemSubmissionTestMixin
 from student.tests.factories import UserFactory
 from xblock.runtime import DictKeyValueStore
@@ -107,16 +107,6 @@ class MasqueradeTestCase(SharedModuleStoreTestCase, LoginEnrollmentTestCase):
         )
         return self.client.get(url)
 
-    def _create_mock_json_request(self, user, data, method='POST', session=None):
-        """
-        Returns a mock JSON request for the specified user
-        """
-        factory = RequestFactory()
-        request = factory.generic(method, '/', content_type='application/json', data=json.dumps(data))
-        request.user = user
-        request.session = session or {}
-        return request
-
     def verify_staff_debug_present(self, staff_debug_expected):
         """
         Verifies that the staff debug control visibility is as expected (for staff only).
@@ -162,6 +152,19 @@ class MasqueradeTestCase(SharedModuleStoreTestCase, LoginEnrollmentTestCase):
             "Profile link should point to real user",
         )
 
+    def ensure_masquerade_as_group_member(self, partition_id, group_id):
+        """
+        Installs a masquerade for the test_user and test course, to enable the
+        user to masquerade as belonging to the specific partition/group combination.
+        Also verifies that the call to install the masquerade was successful.
+
+        Arguments:
+            partition_id (int): the integer partition id, referring to partitions already
+               configured in the course.
+            group_id (int); the integer group id, within the specified partition.
+        """
+        self.assertEqual(200, masquerade_as_group_member(self.test_user, self.course, partition_id, group_id))
+
 
 @attr(shard=1)
 class NormalStudentVisibilityTest(MasqueradeTestCase):
@@ -405,13 +408,7 @@ class TestGetMasqueradingGroupId(StaffMasqueradeTestCase):
         self.assertIsNone(user_partition_id)
 
         # Install a masquerading group
-        request = self._create_mock_json_request(
-            self.test_user,
-            data={"role": "student", "user_partition_id": 0, "group_id": 1}
-        )
-        response = handle_ajax(request, unicode(self.course.id))
-        self.assertEquals(response.status_code, 200)
-        setup_masquerade(request, self.course.id, True)
+        self.ensure_masquerade_as_group_member(0, 1)
 
         # Verify that the masquerading group is returned
         group_id, user_partition_id = get_masquerading_group_info(self.test_user, self.course.id)

lms/djangoapps/discussion/static/discussion/js/spec/discussion_board_view_spec.js

@@ -30,6 +30,14 @@
                     return discussionBoardView;
                 };
 
+                describe('Thread List View', function() {
+                    it('should ensure the mode is all', function() {
+                        var discussionBoardView = createDiscussionBoardView().render(),
+                            threadListView = discussionBoardView.discussionThreadListView;
+                        expect(threadListView.mode).toBe('all');
+                    });
+                });
+
                 describe('Search events', function() {
                     it('perform search when enter pressed inside search textfield', function() {
                         var discussionBoardView = createDiscussionBoardView(),

lms/djangoapps/discussion/static/discussion/js/spec/views/discussion_user_profile_view_spec.js

@@ -30,13 +30,22 @@ DiscussionSpecHelper, DiscussionUserProfileView) {
 
         describe('thread list in user profile page', function() {
             it('should render', function() {
-                var discussionUserProfileView = createDiscussionUserProfileView(),
-                    threadListView;
-                discussionUserProfileView.render();
-                threadListView = discussionUserProfileView.discussionThreadListView;
-                threadListView.render();
+                var discussionUserProfileView = createDiscussionUserProfileView().render(),
+                    threadListView = discussionUserProfileView.discussionThreadListView.render();
                 expect(threadListView.$('.forum-nav-thread-list').length).toBe(1);
             });
+
+            it('should ensure discussion thread list view mode is all', function() {
+                var discussionUserProfileView = createDiscussionUserProfileView().render(),
+                    threadListView = discussionUserProfileView.discussionThreadListView.render();
+                expect(threadListView.mode).toBe('user');
+            });
+
+            it('should not show the thread list unread unanswered filter', function() {
+                var discussionUserProfileView = createDiscussionUserProfileView().render(),
+                    threadListView = discussionUserProfileView.discussionThreadListView.render();
+                expect(threadListView.$('.forum-nav-filter-main')).toHaveClass('is-hidden');
+            });
         });
     });
 });

lms/djangoapps/discussion/static/discussion/js/views/discussion_board_view.js

@@ -46,7 +46,8 @@
                     collection: this.discussion,
                     el: this.$('.discussion-thread-list-container'),
                     courseSettings: this.courseSettings,
-                    supportsActiveThread: true
+                    supportsActiveThread: true,
+                    mode: this.mode
                 }).render();
                 this.searchView = new DiscussionSearchView({
                     el: this.$('.forum-search')

lms/djangoapps/discussion/static/discussion/js/views/discussion_user_profile_view.js

@@ -26,7 +26,7 @@
                 initialize: function(options) {
                     this.courseSettings = options.courseSettings;
                     this.discussion = options.discussion;
-                    this.mode = 'all';
+                    this.mode = 'user';
                     this.listenTo(this.model, 'change', this.render);
                 },
 
@@ -39,7 +39,7 @@
                         collection: this.discussion,
                         el: this.$('.inline-threads'),
                         courseSettings: this.courseSettings,
-                        hideRefineBar: true,  // TODO: re-enable the search/filter bar when it works correctly
+                        mode: this.mode,
                         // @TODO: On the profile page, thread read state for the viewing user is not accessible via API.
                         // Fix this when the Discussions API can support this query. Until then, hide read state.
                         hideReadState: true

lms/djangoapps/discussion/templates/discussion/discussion_profile_page.html

@@ -76,6 +76,7 @@ from openedx.core.djangolib.js_utils import dump_js_escaped_json, js_escaped_str
                 data-course-name="${course.display_name_with_default}"
                 data-threads="${threads}"
                 data-user-info="${user_info}"
+                data-user-id="${django_user.id}"
                 data-page="${page}"
                 data-num-pages="${num_pages}"
                 data-user-create-comment="${json.dumps(can_create_comment)}"

lms/djangoapps/teams/static/teams/js/spec/views/team_profile_spec.js

@@ -59,7 +59,8 @@ define([
                 requests,
                 'GET',
                 interpolate(
-                    '/courses/%(courseID)s/discussion/forum/%(topicID)s/inline?page=1&ajax=1',
+                    '/courses/%(courseID)s/discussion/forum/%(topicID)s/inline' +
+                    '?page=1&sort_key=activity&sort_order=desc&ajax=1',
                     {
                         courseID: TeamSpecHelpers.testCourseID,
                         topicID: TeamSpecHelpers.testTeamDiscussionID

lms/static/sass/discussion/elements/_navigation.scss

@@ -137,21 +137,26 @@
     @include text-align(left);
     @include float(left);
     box-sizing: border-box;
-    display: inline-block;
     width: 50%;
 }
 
 .forum-nav-filter-cohort, .forum-nav-sort {
     @include text-align(right);
-    @include float(right);
     box-sizing: border-box;
-    display: inline-block;
+}
 
-    @media (min-width: $bp-screen-md) {
+.forum-nav-filter-cohort {
+    .discussion-board & {
+        @include float(right);
+        @include text-align(right);
         width: 50%;
     }
 }
 
+.forum-nav-sort {
+    @include float(right);
+}
+
 %forum-nav-select {
     border: none;
     max-width: 100%;

openedx/core/djangoapps/course_groups/tests/test_partition_scheme.py

@@ -6,7 +6,6 @@ import django.test
 from mock import patch
 from nose.plugins.attrib import attr
 
-from courseware.masquerade import handle_ajax, setup_masquerade
 from courseware.tests.test_masquerade import StaffMasqueradeTestCase
 from student.tests.factories import UserFactory
 from xmodule.partitions.partitions import Group, UserPartition, UserPartitionError
@@ -341,30 +340,17 @@ class TestMasqueradedGroup(StaffMasqueradeTestCase):
             scheme_id='cohort'
         )
         self.course.user_partitions.append(self.user_partition)
-        self.session = {}
         modulestore().update_item(self.course, self.test_user.id)
 
     def _verify_masquerade_for_group(self, group):
         """
         Verify that the masquerade works for the specified group id.
         """
-        # Send the request to set the masquerade
-        request_json = {
-            "role": "student",
-            "user_partition_id": self.user_partition.id,
-            "group_id": group.id if group is not None else None
-        }
-        request = self._create_mock_json_request(
-            self.test_user,
-            data=request_json,
-            session=self.session
+        self.ensure_masquerade_as_group_member(  # pylint: disable=no-member
+            self.user_partition.id,
+            group.id if group is not None else None
         )
-        response = handle_ajax(request, unicode(self.course.id))
-        # pylint has issues analyzing this class (maybe due to circular imports?)
-        self.assertEquals(response.status_code, 200)  # pylint: disable=no-member
 
-        # Now setup the masquerade for the test user
-        setup_masquerade(request, self.course.id, True)
         scheme = self.user_partition.scheme
         self.assertEqual(
             scheme.get_group_for_user(self.course.id, self.test_user, self.user_partition),

requirements/edx/github.txt

@@ -90,7 +90,7 @@ git+https://github.com/edx/xblock-utils.git@v1.0.3#egg=xblock-utils==1.0.3
 -e git+https://github.com/edx-solutions/xblock-google-drive.git@138e6fa0bf3a2013e904a085b9fed77dab7f3f21#egg=xblock-google-drive
 -e git+https://github.com/edx/edx-reverification-block.git@0.0.5#egg=edx-reverification-block==0.0.5
 git+https://github.com/edx/edx-user-state-client.git@1.0.1#egg=edx-user-state-client==1.0.1
-git+https://github.com/edx/xblock-lti-consumer.git@v1.1.0#egg=xblock-lti-consumer==1.1.0
+git+https://github.com/edx/xblock-lti-consumer.git@v1.1.1#egg=xblock-lti-consumer==1.1.1
 git+https://github.com/edx/edx-proctoring.git@0.17.0#egg=edx-proctoring==0.17.0
 
 # Third Party XBlocks