Merge branch '49161-disable-toggle-comments' into 'master'

Resolve "MR refactor: Disable toggle comments button when diff has no discussions"

Closes #49161

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

app/assets/javascripts/diffs/components/diff_file_header.vue

@@ -50,7 +50,7 @@ export default {
     };
   },
   computed: {
-    ...mapGetters('diffs', ['diffHasExpandedDiscussions']),
+    ...mapGetters('diffs', ['diffHasExpandedDiscussions', 'diffHasDiscussions']),
     hasExpandedDiscussions() {
       return this.diffHasExpandedDiscussions(this.diffFile);
     },
@@ -221,6 +221,7 @@ export default {
         v-if="diffFile.blob && diffFile.blob.readableText"
       >
         <button
+          :disabled="!diffHasDiscussions(diffFile)"
           :class="{ active: hasExpandedDiscussions }"
           :title="s__('MergeRequests|Toggle comments for this file')"
           class="js-btn-vue-toggle-comments btn"

app/assets/javascripts/diffs/store/getters.js

@@ -47,6 +47,14 @@ export const diffHasExpandedDiscussions = (state, getters) => diff => {
   );
 };
 
+/**
+ * Checks if the diff has any discussion
+ * @param {Boolean} diff
+ * @returns {Boolean}
+ */
+export const diffHasDiscussions = (state, getters) => diff =>
+  getters.getDiffFileDiscussions(diff).length > 0;
+
 /**
  * Returns an array with the discussions of the given diff
  * @param {Object} diff

changelogs/unreleased/49161-disable-toggle-comments.yml

+---
+title: Disables toggle comments button if diff has no discussions
+merge_request:
+author:
+type: other

spec/javascripts/diffs/components/diff_file_header_spec.js

@@ -11,7 +11,9 @@ const discussionFixture = 'merge_requests/diff_discussion.json';
 describe('diff_file_header', () => {
   let vm;
   let props;
+  const diffDiscussionMock = getJSONFixture(discussionFixture)[0];
   const Component = Vue.extend(DiffFileHeader);
+
   const store = new Vuex.Store({
     modules: {
       diffs: diffsModule,
@@ -20,7 +22,6 @@ describe('diff_file_header', () => {
   });
 
   beforeEach(() => {
-    const diffDiscussionMock = getJSONFixture(discussionFixture)[0];
     const diffFile = convertObjectPropsToCamelCase(diffDiscussionMock.diff_file, { deep: true });
     props = {
       diffFile,
@@ -409,7 +410,7 @@ describe('diff_file_header', () => {
     });
 
     describe('handles toggle discussions', () => {
-      it('dispatches toggleFileDiscussions when user clicks on toggle discussions button', () => {
+      it('renders a disabled button when diff has no discussions', () => {
         const propsCopy = Object.assign({}, props);
         propsCopy.diffFile.submodule = false;
         propsCopy.diffFile.blob = {
@@ -428,6 +429,38 @@ describe('diff_file_header', () => {
           store,
         });
 
+        expect(
+          vm.$el.querySelector('.js-btn-vue-toggle-comments').getAttribute('disabled'),
+        ).toEqual('disabled');
+      });
+
+      describe('with discussions', () => {
+        it('dispatches toggleFileDiscussions when user clicks on toggle discussions button', () => {
+          const propsCopy = Object.assign({}, props);
+          propsCopy.diffFile.submodule = false;
+          propsCopy.diffFile.blob = {
+            id: '848ed9407c6730ff16edb3dd24485a0eea24292a',
+            path: 'lib/base.js',
+            name: 'base.js',
+            mode: '100644',
+            readableText: true,
+            icon: 'file-text-o',
+          };
+          propsCopy.addMergeRequestButtons = true;
+          propsCopy.diffFile.deletedFile = true;
+
+          const discussionGetter = () => [diffDiscussionMock];
+          notesModule.getters.discussions = discussionGetter;
+          vm = mountComponentWithStore(Component, {
+            props: propsCopy,
+            store: new Vuex.Store({
+              modules: {
+                diffs: diffsModule,
+                notes: notesModule,
+              },
+            }),
+          });
+
           spyOn(vm, 'toggleFileDiscussions');
 
           vm.$el.querySelector('.js-btn-vue-toggle-comments').click();
@@ -436,4 +469,5 @@ describe('diff_file_header', () => {
         });
       });
     });
+  });
 });

spec/javascripts/diffs/store/getters_spec.js

@@ -167,6 +167,24 @@ describe('Diffs Module Getters', () => {
     });
   });
 
+  describe('diffHasDiscussions', () => {
+    it('returns true when getDiffFileDiscussions returns discussions', () => {
+      expect(
+        getters.diffHasDiscussions(localState, {
+          getDiffFileDiscussions: () => [discussionMock],
+        })(diffFileMock),
+      ).toEqual(true);
+    });
+
+    it('returns false when getDiffFileDiscussions returns no discussions', () => {
+      expect(
+        getters.diffHasDiscussions(localState, {
+          getDiffFileDiscussions: () => [],
+        })(diffFileMock),
+      ).toEqual(false);
+    });
+  });
+
   describe('getDiffFileDiscussions', () => {
     it('returns an array with discussions when fileHash matches and the discussion belongs to a diff', () => {
       discussionMock.diff_file.file_hash = diffFileMock.fileHash;