Merge branch 'winh-collapse-discussions' into 'master'

Fix collapsing discussion replies

Closes #54646

See merge request gitlab-org/gitlab-ce!23462

4 files changed, 35 insertions, 62 deletions

diff --git a/app/assets/javascripts/notes/components/noteable_discussion.vue b/app/assets/javascripts/notes/components/noteable_discussion.vue
index 4eb3b49392c..f4991a41325 100644
--- a/app/assets/javascripts/notes/components/noteable_discussion.vue
+++ b/app/assets/javascripts/notes/components/noteable_discussion.vue
@@ -66,11 +66,13 @@ export default {
     },
   },
   data() {
+    const { diff_discussion: isDiffDiscussion, resolved } = this.discussion;
+
     return {
       isReplying: false,
       isResolving: false,
       resolveAsThread: true,
-      isRepliesToggledByUser: false,
+      isRepliesCollapsed: Boolean(!isDiffDiscussion && resolved),
     };
   },
   computed: {
@@ -150,15 +152,6 @@ export default {
 
       return expanded || this.alwaysExpanded || isResolvedNonDiffDiscussion;
     },
-    isRepliesCollapsed() {
-      const { discussion, isRepliesToggledByUser } = this;
-      const { resolved, notes } = discussion;
-      const hasReplies = notes.length > 1;
-
-      return (
-        (!discussion.diff_discussion && resolved && hasReplies && !isRepliesToggledByUser) || false
-      );
-    },
     actionText() {
       const commitId = this.discussion.commit_id ? truncateSha(this.discussion.commit_id) : '';
       const linkStart = `<a href="${_.escape(this.discussion.discussion_path)}">`;
@@ -234,7 +227,7 @@ export default {
       this.toggleDiscussion({ discussionId: this.discussion.id });
     },
     toggleReplies() {
-      this.isRepliesToggledByUser = !this.isRepliesToggledByUser;
+      this.isRepliesCollapsed = !this.isRepliesCollapsed;
     },
     showReplyForm() {
       this.isReplying = true;
diff --git a/changelogs/unreleased/winh-collapse-discussions.yml b/changelogs/unreleased/winh-collapse-discussions.yml
new file mode 100644
index 00000000000..19d04506318
--- /dev/null
+++ b/changelogs/unreleased/winh-collapse-discussions.yml
@@ -0,0 +1,5 @@
+---
+title: Fix collapsing discussion replies
+merge_request: 23462
+author:
+type: fixed
diff --git a/spec/javascripts/notes/components/noteable_discussion_spec.js b/spec/javascripts/notes/components/noteable_discussion_spec.js
index 76e9cd03d2d..ab9c52346d6 100644
--- a/spec/javascripts/notes/components/noteable_discussion_spec.js
+++ b/spec/javascripts/notes/components/noteable_discussion_spec.js
@@ -6,7 +6,6 @@ import { noteableDataMock, discussionMock, notesDataMock } from '../mock_data';
 import mockDiffFile from '../../diffs/mock_data/diff_file';
 
 const discussionWithTwoUnresolvedNotes = 'merge_requests/resolved_diff_discussion.json';
-const diffDiscussionFixture = 'merge_requests/diff_discussion.json';
 
 describe('noteable_discussion component', () => {
   const Component = Vue.extend(noteableDiscussion);
@@ -79,51 +78,6 @@ describe('noteable_discussion component', () => {
     });
   });
 
-  describe('computed', () => {
-    describe('isRepliesCollapsed', () => {
-      it('should return false for diff discussions', done => {
-        const diffDiscussion = getJSONFixture(diffDiscussionFixture)[0];
-        vm.$store.dispatch('setInitialNotes', [diffDiscussion]);
-
-        Vue.nextTick()
-          .then(() => {
-            expect(vm.isRepliesCollapsed).toEqual(false);
-            expect(vm.$el.querySelector('.js-toggle-replies')).not.toBeNull();
-            expect(vm.$el.querySelector('.discussion-reply-holder')).not.toBeNull();
-          })
-          .then(done)
-          .catch(done.fail);
-      });
-
-      it('should return false if discussion does not have a reply', () => {
-        const discussion = { ...discussionMock, resolved: true };
-        discussion.notes = discussion.notes.slice(0, 1);
-        const noRepliesVm = new Component({
-          store,
-          propsData: { discussion },
-        }).$mount();
-
-        expect(noRepliesVm.isRepliesCollapsed).toEqual(false);
-        expect(noRepliesVm.$el.querySelector('.js-toggle-replies')).toBeNull();
-        expect(vm.$el.querySelector('.discussion-reply-holder')).not.toBeNull();
-        noRepliesVm.$destroy();
-      });
-
-      it('should return true for resolved non-diff discussion which has replies', () => {
-        const discussion = { ...discussionMock, resolved: true };
-        const resolvedDiscussionVm = new Component({
-          store,
-          propsData: { discussion },
-        }).$mount();
-
-        expect(resolvedDiscussionVm.isRepliesCollapsed).toEqual(true);
-        expect(resolvedDiscussionVm.$el.querySelector('.js-toggle-replies')).not.toBeNull();
-        expect(vm.$el.querySelector('.discussion-reply-holder')).not.toBeNull();
-        resolvedDiscussionVm.$destroy();
-      });
-    });
-  });
-
   describe('methods', () => {
     describe('jumpToNextDiscussion', () => {
       it('expands next unresolved discussion', done => {
diff --git a/spec/support/features/discussion_comments_shared_example.rb b/spec/support/features/discussion_comments_shared_example.rb
index 18cf08f0b9e..922f3df144d 100644
--- a/spec/support/features/discussion_comments_shared_example.rb
+++ b/spec/support/features/discussion_comments_shared_example.rb
@@ -142,6 +142,14 @@ shared_examples 'discussion comments' do |resource_name|
           find(comments_selector, match: :first)
         end
 
+        def submit_reply(text)
+          find("#{comments_selector} .js-vue-discussion-reply").click
+          find("#{comments_selector} .note-textarea").send_keys(text)
+
+          click_button "Comment"
+          wait_for_requests
+        end
+
         it 'clicking "Start discussion" will post a discussion' do
           new_comment = all(comments_selector).last
 
@@ -149,16 +157,29 @@ shared_examples 'discussion comments' do |resource_name|
           expect(new_comment).to have_selector '.discussion'
         end
 
+        if resource_name =~ /(issue|merge request)/
+          it 'can be replied to' do
+            submit_reply('some text')
+
+            expect(page).to have_css('.discussion-notes .note', count: 2)
+            expect(page).to have_content 'Collapse replies'
+          end
+
+          it 'can be collapsed' do
+            submit_reply('another text')
+
+            find('.js-collapse-replies').click
+            expect(page).to have_css('.discussion-notes .note', count: 1)
+            expect(page).to have_content '1 reply'
+          end
+        end
+
         if resource_name == 'merge request'
           let(:note_id) { find("#{comments_selector} .note:first-child", match: :first)['data-note-id'] }
           let(:reply_id) { find("#{comments_selector} .note:last-child", match: :first)['data-note-id'] }
 
           it 'shows resolved discussion when toggled' do
-            find("#{comments_selector} .js-vue-discussion-reply").click
-            find("#{comments_selector} .note-textarea").send_keys('a')
-
-            click_button "Comment"
-            wait_for_requests
+            submit_reply('a')
 
             click_button "Resolve discussion"
             wait_for_requests