Merge pull request #14818 from edx/ri/TNL-6740-fix-previously-cohorted-discussion-posts

TNL-6740 Fix previously cohorted discussion posts still show cohorting visibility message.

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

@@ -36,6 +36,7 @@
                 this.pages = options.pages || 1;
                 this.current_page = 1;
                 this.sort_preference = options.sort;
+                this.is_commentable_cohorted = options.is_commentable_cohorted;
                 this.bind('add', function(item) {
                     item.discussion = self;
                 });
@@ -141,6 +142,7 @@
                         Content.loadContentInfos(response.annotated_content_info);
                         self.pages = response.num_pages;
                         self.current_page = response.page;
+                        self.is_commentable_cohorted = response.is_commentable_cohorted;
                         return self.reset(new_collection);
                     },
                     error: error

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

@@ -86,6 +86,7 @@
             DiscussionUtil.loadRoles(response.roles);
 
             this.courseSettings = new DiscussionCourseSettings(response.course_settings);
+            this.is_commentable_cohorted = response.is_commentable_cohorted;
 
             this.discussion = new Discussion(undefined, {pages: response.num_pages});
             this.discussion.reset(response.discussion_data, {
@@ -152,7 +153,8 @@
                 model: thread,
                 mode: 'inline',
                 startHeader: this.startHeader,
-                courseSettings: this.courseSettings
+                courseSettings: this.courseSettings,
+                is_commentable_cohorted: this.is_commentable_cohorted
             });
             this.threadView.render();
             this.listenTo(this.threadView.showView, 'thread:_delete', this.navigateToAllPosts);

common/static/common/js/discussion/views/discussion_thread_show_view.js

@@ -31,6 +31,7 @@
                 DiscussionThreadShowView.__super__.initialize.call(this);
                 this.mode = options.mode || 'inline';
                 this.startHeader = options.startHeader;
+                this.is_commentable_cohorted = options.is_commentable_cohorted;
                 if ((_ref = this.mode) !== 'tab' && _ref !== 'inline') {
                     throw new Error('invalid mode: ' + this.mode);
                 }
@@ -41,6 +42,7 @@
                     mode: this.mode,
                     startHeader: this.startHeader,
                     flagged: this.model.isFlagged(),
+                    is_commentable_cohorted: this.is_commentable_cohorted,
                     author_display: this.getAuthorDisplay(),
                     cid: this.model.cid,
                     readOnly: $('.discussion-module').data('read-only')

common/static/common/js/discussion/views/discussion_thread_view.js

@@ -92,7 +92,7 @@
                         self.model = collection.get(id);
                     }
                 });
-
+                this.is_commentable_cohorted = options.is_commentable_cohorted;
                 this.createShowView();
                 this.responses = new Comments();
                 this.loadedResponses = false;
@@ -422,7 +422,8 @@
                 this.showView = new DiscussionThreadShowView({
                     model: this.model,
                     mode: this.mode,
-                    startHeader: this.startHeader
+                    startHeader: this.startHeader,
+                    is_commentable_cohorted: this.is_commentable_cohorted
                 });
                 this.showView.bind('thread:_delete', this._delete);
                 return this.showView.bind('thread:edit', this.edit);

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

@@ -181,10 +181,19 @@
             it('renders correctly for a cohorted thread', function() {
                 this.thread.set('group_id', '1');
                 this.thread.set('group_name', 'Mock Cohort');
+                this.view.is_commentable_cohorted = true;
                 this.view.render();
                 return expect(this.view.$('.group-visibility-label').text().trim())
                     .toEqual('This post is visible only to Mock Cohort.');
             });
+            it('renders correctly for a grouped uncohorted thread', function() {
+                this.thread.set('group_id', '1');
+                this.thread.set('group_name', 'Mock Cohort');
+                this.view.is_commentable_cohorted = false;
+                this.view.render();
+                return expect(this.view.$('.group-visibility-label').text().trim())
+                    .toEqual('This post is visible to everyone.');
+            });
         });
     });
 }).call(this);

common/static/common/templates/discussion/thread-show.underscore

@@ -69,7 +69,7 @@
             %>
         <% } %>
         <div class="group-visibility-label">
-            <% if (obj.group_name) { %>
+            <% if (obj.group_name && is_commentable_cohorted) { %>
                 <%-
                 interpolate(
                     gettext('This post is visible only to %(group_name)s.'),

common/test/acceptance/tests/discussion/helpers.py

@@ -6,7 +6,7 @@ from uuid import uuid4
 import json
 
 from common.test.acceptance.fixtures import LMS_BASE_URL
-from common.test.acceptance.fixtures.course import CourseFixture
+from common.test.acceptance.fixtures.course import (CourseFixture, XBlockFixtureDesc)
 from common.test.acceptance.fixtures.discussion import (
     SingleThreadViewFixture,
     Thread,
@@ -74,6 +74,14 @@ class CohortTestMixin(object):
             },
         })
 
+    def enable_cohorting(self, course_fixture):
+        """
+        enables cohorting for the current course fixture.
+        """
+        url = LMS_BASE_URL + "/courses/" + course_fixture._course_key + '/cohorts/settings'  # pylint: disable=protected-access
+        data = json.dumps({'always_cohort_inline_discussions': True})
+        response = course_fixture.session.patch(url, data=data, headers=course_fixture.headers)
+
     def disable_cohorting(self, course_fixture):
         """
         Disables cohorting for the current course fixture.
@@ -111,8 +119,21 @@ class BaseDiscussionTestCase(UniqueCourseTest, ForumsConfigMixin):
 
         self.discussion_id = "test_discussion_{}".format(uuid4().hex)
         self.course_fixture = CourseFixture(**self.course_info)
+        self.course_fixture.add_children(
+            XBlockFixtureDesc("chapter", "Test Section").add_children(
+                XBlockFixtureDesc("sequential", "Test Subsection").add_children(
+                    XBlockFixtureDesc("vertical", "Test Unit").add_children(
+                        XBlockFixtureDesc(
+                            "discussion",
+                            "Test Discussion",
+                            metadata={"discussion_id": self.discussion_id}
+                        )
+                    )
+                )
+            )
+        )
         self.course_fixture.add_advanced_settings(
-            {'discussion_topics': {'value': {'Test Discussion Topic': {'id': self.discussion_id}}}}
+            {'discussion_topics': {'value': {'General': {'id': 'course'}}}}
         )
         self.course_fixture.install()
 

common/test/acceptance/tests/discussion/test_cohorts.py

@@ -49,6 +49,10 @@ class CohortedDiscussionTestMixin(BaseDiscussionMixin, CohortTestMixin):
         # Must be moderator to view content in a cohort other than your own
         AutoAuthPage(self.browser, course_id=self.course_id, roles="Moderator").visit()
         self.thread_id = self.setup_thread(1, group_id=self.cohort_1_id)
+
+        # Enable cohorts and verify that the post shows to cohort only.
+        self.enable_cohorting(self.course_fixture)
+        self.refresh_thread_page(self.thread_id)
         self.assertEquals(
             self.thread_page.get_group_visibility_label(),
             "This post is visible only to {}.".format(self.cohort_1_name)

common/test/acceptance/tests/discussion/test_discussion.py

@@ -270,12 +270,13 @@ class DiscussionNavigationTest(BaseDiscussionTestCase):
             css=".forum-nav-browse-menu-item[data-discussion-id='{}']".format(self.discussion_id)
         )
         self.assertTrue(topic_button.visible)
+
         topic_button.click()
 
         # Verify the thread's topic has been pushed to breadcrumbs
         breadcrumbs = self.thread_page.q(css=".breadcrumbs .nav-item")
-        self.assertEqual(len(breadcrumbs), 2)
-        self.assertEqual(breadcrumbs[1].text, "Test Discussion Topic")
+        self.assertEqual(len(breadcrumbs), 3)
+        self.assertEqual(breadcrumbs[2].text, "Topic-Level Student-Visible Label")
 
     def test_breadcrumbs_back_to_all_topics(self):
         topic_button = self.thread_page.q(

lms/djangoapps/discussion/static/discussion/js/discussion_board_factory.js

@@ -21,6 +21,7 @@
                     sortPreference = options.sortPreference,
                     threads = options.threads,
                     threadPages = options.threadPages,
+                    isCommentableCohorted = options.isCommentableCohorted,
                     contentInfo = options.contentInfo,
                     user = new DiscussionUser(userInfo),
                     discussion,
@@ -39,7 +40,8 @@
                 Content.loadContentInfos(contentInfo);
 
                 // Create a discussion model
-                discussion = new Discussion(threads, {pages: threadPages, sort: sortPreference});
+                discussion = new Discussion(threads, {pages: threadPages, sort: sortPreference,
+                    is_commentable_cohorted: isCommentableCohorted});
                 courseSettings = new DiscussionCourseSettings(options.courseSettings);
 
                 // Create the discussion board view

lms/djangoapps/discussion/static/discussion/js/discussion_router.js

@@ -102,7 +102,8 @@
                         model: this.thread,
                         mode: 'tab',
                         startHeader: this.startHeader,
-                        courseSettings: this.courseSettings
+                        courseSettings: this.courseSettings,
+                        is_commentable_cohorted: this.discussion.is_commentable_cohorted
                     });
                     this.main.render();
                     this.main.on('thread:responses:rendered', function() {

lms/djangoapps/discussion/templates/discussion/discussion_board_js.template

@@ -56,7 +56,8 @@ from openedx.core.djangolib.js_utils import dump_js_escaped_json, js_escaped_str
                 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}
+                courseSettings: ${course_settings | n, dump_js_escaped_json},
+                isCommentableCohorted: ${is_commentable_cohorted | n, dump_js_escaped_json}
             });
         });
     });

lms/djangoapps/discussion/views.py

@@ -438,6 +438,7 @@ def _create_discussion_board_context(request, course_key, discussion_id=None, th
         'sort_preference': cc_user.default_sort_key,
         'category_map': course_settings["category_map"],
         'course_settings': course_settings,
+        'is_commentable_cohorted': is_commentable_cohorted(course_key, discussion_id)
     })
     return context