Merge pull request #17743 from edx/sstudent/EDUCATOR-2444

Show inline discussions by default

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

@@ -26,7 +26,6 @@
             var match;
             this.$el = options.el;
             this.readOnly = options.readOnly;
-            this.showByDefault = options.showByDefault || false;
             this.toggleDiscussionBtn = this.$('.discussion-show');
             this.listenTo(this.model, 'change', this.render);
             this.escKey = 27;
@@ -46,12 +45,8 @@
 
             this.defaultSortKey = 'activity';
             this.defaultSortOrder = 'desc';
-
+            // discussions are open by default
-            // By default the view is displayed in a hidden state. If you want it to be shown by default (e.g. in Teams)
+            this.toggleDiscussion();
-            // pass showByDefault as an option. This code will open it on initialization.
-            if (this.showByDefault) {
-                this.toggleDiscussion();
-            }
         },
 
         loadDiscussions: function($elem, error) {

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

@@ -34,7 +34,34 @@
             spyOn(DiscussionThreadShowView.prototype, 'convertMath');
         });
 
-        createTestView = function() {
+        createTestView = function(test) {
+            var courseSettings = DiscussionSpecHelper.createTestCourseSettings({
+                groups: [
+                    {
+                        id: 1,
+                        name: 'Cohort1'
+                    }, {
+                        id: 2,
+                        name: 'Cohort2'
+                    }
+                ]
+            });
+            setNextAjaxResult(test, {
+                user_info: DiscussionSpecHelper.getTestUserInfo(),
+                roles: DiscussionSpecHelper.getTestRoleInfo(),
+                course_settings: courseSettings.attributes,
+                discussion_data: DiscussionViewSpecHelper.makeThreadWithProps({
+                    commentable_id: 'test-topic',
+                    title: TEST_THREAD_TITLE
+                }),
+                page: 1,
+                num_pages: 1,
+                content: {
+                    endorsed_responses: [],
+                    non_endorsed_responses: [],
+                    children: []
+                }
+            });
             var testView = new DiscussionInlineView({
                 el: $('.discussion-module')
             });
@@ -83,12 +110,24 @@
         };
 
         describe('inline discussion', function() {
-            it('is shown after "Show Discussion" is clicked', function() {
+            it('is shown by default', function() {
                 var testView = createTestView(this),
                     showButton = testView.$('.discussion-show');
+
+                // Verify that the discussion is shown without clicking anything
+                expect(showButton).toHaveClass('shown');
+                expect(showButton.text().trim()).toEqual('Hide Discussion');
+                expect(testView.$('.inline-discussion:visible')).not.toHaveClass('is-hidden');
+            });
+            it('is shown after "Show Discussion" is clicked while discussions are hidden', function() {
+                var testView = createTestView(this),
+                    showButton = testView.$('.discussion-show');
+
+                // hide the discussion; discussions are loaded by default
+                testView.$('.discussion-show').click();
                 showDiscussion(this, testView);
 
-                // Verify that the discussion is now shown
+                // Verify that the discussion is now shown again
                 expect(showButton).toHaveClass('shown');
                 expect(showButton.text().trim()).toEqual('Hide Discussion');
                 expect(testView.$('.inline-discussion:visible')).not.toHaveClass('is-hidden');
@@ -97,9 +136,8 @@
             it('is hidden after "Hide Discussion" is clicked', function() {
                 var testView = createTestView(this),
                     showButton = testView.$('.discussion-show');
-                showDiscussion(this, testView);
 
-                // Hide the discussion by clicking the toggle button again
+                // Hide the discussion by clicking the toggle button
                 testView.$('.discussion-show').click();
 
                 // Verify that the discussion is now hidden

common/test/acceptance/fixtures/discussion.py

@@ -83,7 +83,7 @@ class DiscussionContentFixture(object):
         """
         Push the data to the stub comments service.
         """
-        requests.put(
+        return requests.put(
             '{}/set_config'.format(COMMENTS_STUB_URL),
             data=self.get_config_data()
         )

common/test/acceptance/pages/lms/discussion.py

@@ -581,6 +581,7 @@ class InlineDiscussionPage(PageObject, DiscussionPageMixin):
         """
         Clicks the link for the specified thread to show the detailed view.
         """
+        self.wait_for_element_presence('.forum-nav-thread-link', 'Thread list has loaded')
         thread_selector = ".forum-nav-thread[data-id='{thread_id}'] .forum-nav-thread-link".format(thread_id=thread_id)
         self._find_within(thread_selector).first.click()
         self.thread_page = InlineDiscussionThreadPage(self.browser, thread_id)  # pylint: disable=attribute-defined-outside-init

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

@@ -34,7 +34,8 @@ class BaseDiscussionMixin(object):
         )
         for i in range(num_responses):
             thread_fixture.addResponse(Response(id=str(i), body=str(i)))
-        thread_fixture.push()
+        response = thread_fixture.push()
+        self.assertTrue(response.ok, "Failed to push discussion content")
         self.setup_thread_page(thread_id)
         return thread_id
 
@@ -52,7 +53,8 @@ class BaseDiscussionMixin(object):
             )
             self.thread_ids.append(thread_id)
         thread_fixture = MultipleThreadFixture(threads)
-        thread_fixture.push()
+        response = thread_fixture.push()
+        self.assertTrue(response.ok, "Failed to push discussion content")
 
 
 class CohortTestMixin(object):

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

@@ -127,7 +127,8 @@ class InlineDiscussionTest(UniqueCourseTest):
 
     def show_thread(self, thread_id):
         discussion_page = InlineDiscussionPage(self.browser, self.discussion_id)
-        discussion_page.expand_discussion()
+        if not discussion_page.is_discussion_expanded():
+            discussion_page.expand_discussion()
         self.assertEqual(discussion_page.get_num_displayed_threads(), 1)
         discussion_page.show_thread(thread_id)
         self.thread_page = discussion_page.thread_page  # pylint: disable=attribute-defined-outside-init

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

@@ -1053,7 +1053,9 @@ class InlineDiscussionTest(UniqueCourseTest, DiscussionResponsePaginationTestMix
         self.additional_discussion_page = InlineDiscussionPage(self.browser, self.additional_discussion_id)
 
     def setup_thread_page(self, thread_id):
-        self.discussion_page.expand_discussion()
+        self.browser.refresh()
+        if not self.discussion_page.is_discussion_expanded():
+            self.discussion_page.expand_discussion()
         self.discussion_page.show_thread(thread_id)
         self.thread_page = self.discussion_page.thread_page  # pylint: disable=attribute-defined-outside-init
 
@@ -1098,10 +1100,9 @@ class InlineDiscussionTest(UniqueCourseTest, DiscussionResponsePaginationTestMix
         self.assertFalse(self.discussion_page.is_new_post_button_visible())
 
     def test_initial_render(self):
-        self.assertFalse(self.discussion_page.is_discussion_expanded())
+        self.assertTrue(self.discussion_page.is_discussion_expanded())
 
-    def test_expand_discussion_empty(self):
+    def test_discussion_empty(self):
-        self.discussion_page.expand_discussion()
         self.assertEqual(self.discussion_page.get_num_displayed_threads(), 0)
 
     def check_anonymous_to_peers(self, is_staff):
@@ -1151,16 +1152,14 @@ class InlineDiscussionTest(UniqueCourseTest, DiscussionResponsePaginationTestMix
         self.discussion_page.wait_for_page()
         self.additional_discussion_page.wait_for_page()
 
-        # Expand the first discussion, click to add a post
+        # Click to add a post to the first discussion
-        self.discussion_page.expand_discussion()
         self.discussion_page.click_new_post_button()
 
         # Verify that only the first discussion's form is shown
         self.assertIsNotNone(self.discussion_page.new_post_form)
         self.assertIsNone(self.additional_discussion_page.new_post_form)
 
-        # Expand the second discussion, click to add a post
+        # Click to add a post to the second discussion
-        self.additional_discussion_page.expand_discussion()
         self.additional_discussion_page.click_new_post_button()
 
         # Verify that both discussion's forms are shown

lms/djangoapps/teams/static/teams/js/views/team_discussion.js

@@ -14,7 +14,6 @@
                 render: function() {
                     var discussionInlineView = new DiscussionInlineView({
                         el: this.$el,
-                        showByDefault: true,
                         readOnly: this.readOnly,
                         startHeader: 3
                     });