Merge branch 'mr-image-commenting' into 'master'

Re-implemented image commenting on diffs

Closes #48956

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

10 files changed, 302 insertions, 29 deletions

diff --git a/spec/javascripts/diffs/components/diff_content_spec.js b/spec/javascripts/diffs/components/diff_content_spec.js
index 67f7b569f47..36bd042f3c4 100644
--- a/spec/javascripts/diffs/components/diff_content_spec.js
+++ b/spec/javascripts/diffs/components/diff_content_spec.js
@@ -1,15 +1,24 @@
 import Vue from 'vue';
 import DiffContentComponent from '~/diffs/components/diff_content.vue';
-import store from '~/mr_notes/stores';
+import { createStore } from '~/mr_notes/stores';
 import { mountComponentWithStore } from 'spec/helpers/vue_mount_component_helper';
 import { GREEN_BOX_IMAGE_URL, RED_BOX_IMAGE_URL } from 'spec/test_constants';
+import '~/behaviors/markdown/render_gfm';
 import diffFileMockData from '../mock_data/diff_file';
+import discussionsMockData from '../mock_data/diff_discussions';
 
 describe('DiffContent', () => {
   const Component = Vue.extend(DiffContentComponent);
   let vm;
 
   beforeEach(() => {
+    const store = createStore();
+    store.state.notes.noteableData = {
+      current_user: {
+        can_create_note: false,
+      },
+    };
+
     vm = mountComponentWithStore(Component, {
       store,
       props: {
@@ -46,21 +55,57 @@ describe('DiffContent', () => {
     });
 
     describe('image diff', () => {
-      beforeEach(() => {
+      beforeEach(done => {
         vm.diffFile.newPath = GREEN_BOX_IMAGE_URL;
         vm.diffFile.newSha = 'DEF';
         vm.diffFile.oldPath = RED_BOX_IMAGE_URL;
         vm.diffFile.oldSha = 'ABC';
         vm.diffFile.viewPath = '';
+        vm.diffFile.discussions = [{ ...discussionsMockData }];
+        vm.$store.state.diffs.commentForms.push({
+          fileHash: vm.diffFile.fileHash,
+          x: 10,
+          y: 20,
+          width: 100,
+          height: 200,
+        });
+
+        vm.$nextTick(done);
       });
 
-      it('should have image diff view in place', done => {
-        vm.$nextTick(() => {
-          expect(vm.$el.querySelectorAll('.js-diff-inline-view').length).toEqual(0);
+      it('should have image diff view in place', () => {
+        expect(vm.$el.querySelectorAll('.js-diff-inline-view').length).toEqual(0);
 
-          expect(vm.$el.querySelectorAll('.diff-viewer .image').length).toEqual(1);
+        expect(vm.$el.querySelectorAll('.diff-viewer .image').length).toEqual(1);
+      });
 
-          done();
+      it('renders image diff overlay', () => {
+        expect(vm.$el.querySelector('.image-diff-overlay')).not.toBe(null);
+      });
+
+      it('renders diff file discussions', () => {
+        expect(vm.$el.querySelectorAll('.discussion .note.timeline-entry').length).toEqual(5);
+      });
+
+      describe('handleSaveNote', () => {
+        it('dispatches handleSaveNote', () => {
+          spyOn(vm.$store, 'dispatch').and.stub();
+
+          vm.handleSaveNote('test');
+
+          expect(vm.$store.dispatch).toHaveBeenCalledWith('diffs/saveDiffDiscussion', {
+            note: 'test',
+            formData: {
+              noteableData: jasmine.anything(),
+              noteableType: jasmine.anything(),
+              diffFile: vm.diffFile,
+              positionType: 'image',
+              x: 10,
+              y: 20,
+              width: 100,
+              height: 200,
+            },
+          });
         });
       });
     });
diff --git a/spec/javascripts/diffs/components/diff_discussions_spec.js b/spec/javascripts/diffs/components/diff_discussions_spec.js
index 270f363825f..0bc9da5ad0f 100644
--- a/spec/javascripts/diffs/components/diff_discussions_spec.js
+++ b/spec/javascripts/diffs/components/diff_discussions_spec.js
@@ -1,24 +1,90 @@
 import Vue from 'vue';
 import DiffDiscussions from '~/diffs/components/diff_discussions.vue';
-import store from '~/mr_notes/stores';
+import { createStore } from '~/mr_notes/stores';
 import { createComponentWithStore } from 'spec/helpers/vue_mount_component_helper';
+import '~/behaviors/markdown/render_gfm';
 import discussionsMockData from '../mock_data/diff_discussions';
 
 describe('DiffDiscussions', () => {
-  let component;
+  let vm;
   const getDiscussionsMockData = () => [Object.assign({}, discussionsMockData)];
 
-  beforeEach(() => {
-    component = createComponentWithStore(Vue.extend(DiffDiscussions), store, {
+  function createComponent(props = {}) {
+    const store = createStore();
+
+    vm = createComponentWithStore(Vue.extend(DiffDiscussions), store, {
       discussions: getDiscussionsMockData(),
+      ...props,
     }).$mount();
+  }
+
+  afterEach(() => {
+    vm.$destroy();
   });
 
   describe('template', () => {
     it('should have notes list', () => {
-      const { $el } = component;
+      createComponent();
+
+      expect(vm.$el.querySelectorAll('.discussion .note.timeline-entry').length).toEqual(5);
+    });
+  });
+
+  describe('image commenting', () => {
+    it('renders collapsible discussion button', () => {
+      createComponent({ shouldCollapseDiscussions: true });
+
+      expect(vm.$el.querySelector('.js-diff-notes-toggle')).not.toBe(null);
+      expect(vm.$el.querySelector('.js-diff-notes-toggle svg')).not.toBe(null);
+      expect(vm.$el.querySelector('.js-diff-notes-toggle').classList).toContain(
+        'diff-notes-collapse',
+      );
+    });
+
+    it('dispatches toggleDiscussion when clicking collapse button', () => {
+      createComponent({ shouldCollapseDiscussions: true });
+
+      spyOn(vm.$store, 'dispatch').and.stub();
+
+      vm.$el.querySelector('.js-diff-notes-toggle').click();
+
+      expect(vm.$store.dispatch).toHaveBeenCalledWith('toggleDiscussion', {
+        discussionId: vm.discussions[0].id,
+      });
+    });
+
+    it('renders expand button when discussion is collapsed', done => {
+      createComponent({ shouldCollapseDiscussions: true });
+
+      vm.discussions[0].expanded = false;
+
+      vm.$nextTick(() => {
+        expect(vm.$el.querySelector('.js-diff-notes-toggle').textContent.trim()).toBe('1');
+        expect(vm.$el.querySelector('.js-diff-notes-toggle').className).toContain(
+          'btn-transparent badge badge-pill',
+        );
+
+        done();
+      });
+    });
+
+    it('hides discussion when collapsed', done => {
+      createComponent({ shouldCollapseDiscussions: true });
+
+      vm.discussions[0].expanded = false;
+
+      vm.$nextTick(() => {
+        expect(vm.$el.querySelector('.note-discussion').style.display).toBe('none');
+
+        done();
+      });
+    });
+
+    it('renders badge on avatar', () => {
+      createComponent({ renderAvatarBadge: true, discussions: [{ ...discussionsMockData }] });
 
-      expect($el.querySelectorAll('.discussion .note.timeline-entry').length).toEqual(5);
+      expect(vm.$el.querySelector('.user-avatar-link .badge-pill')).not.toBe(null);
+      expect(vm.$el.querySelector('.user-avatar-link .badge-pill').textContent.trim()).toBe('1');
     });
   });
 });
diff --git a/spec/javascripts/diffs/components/image_diff_overlay_spec.js b/spec/javascripts/diffs/components/image_diff_overlay_spec.js
new file mode 100644
index 00000000000..d76ab745fe1
--- /dev/null
+++ b/spec/javascripts/diffs/components/image_diff_overlay_spec.js
@@ -0,0 +1,146 @@
+import Vue from 'vue';
+import ImageDiffOverlay from '~/diffs/components/image_diff_overlay.vue';
+import { createStore } from '~/mr_notes/stores';
+import { createComponentWithStore } from 'spec/helpers/vue_mount_component_helper';
+import { imageDiffDiscussions } from '../mock_data/diff_discussions';
+
+describe('Diffs image diff overlay component', () => {
+  const dimensions = {
+    width: 100,
+    height: 200,
+  };
+  let Component;
+  let vm;
+
+  function createComponent(props = {}, extendStore = () => {}) {
+    const store = createStore();
+
+    extendStore(store);
+
+    vm = createComponentWithStore(Component, store, {
+      discussions: [...imageDiffDiscussions],
+      fileHash: 'ABC',
+      ...props,
+    });
+  }
+
+  beforeAll(() => {
+    Component = Vue.extend(ImageDiffOverlay);
+  });
+
+  afterEach(() => {
+    vm.$destroy();
+  });
+
+  it('renders comment badges', () => {
+    createComponent();
+    spyOn(vm, 'getImageDimensions').and.returnValue(dimensions);
+    vm.$mount();
+
+    expect(vm.$el.querySelectorAll('.js-image-badge').length).toBe(2);
+  });
+
+  it('renders index of discussion in badge', () => {
+    createComponent();
+    spyOn(vm, 'getImageDimensions').and.returnValue(dimensions);
+    vm.$mount();
+
+    expect(vm.$el.querySelectorAll('.js-image-badge')[0].textContent.trim()).toBe('1');
+    expect(vm.$el.querySelectorAll('.js-image-badge')[1].textContent.trim()).toBe('2');
+  });
+
+  it('renders icon when showCommentIcon is true', () => {
+    createComponent({ showCommentIcon: true });
+    spyOn(vm, 'getImageDimensions').and.returnValue(dimensions);
+    vm.$mount();
+
+    expect(vm.$el.querySelector('.js-image-badge svg')).not.toBe(null);
+  });
+
+  it('sets badge comment positions', () => {
+    createComponent();
+    spyOn(vm, 'getImageDimensions').and.returnValue(dimensions);
+    vm.$mount();
+
+    expect(vm.$el.querySelectorAll('.js-image-badge')[0].style.left).toBe('10px');
+    expect(vm.$el.querySelectorAll('.js-image-badge')[0].style.top).toBe('10px');
+
+    expect(vm.$el.querySelectorAll('.js-image-badge')[1].style.left).toBe('5px');
+    expect(vm.$el.querySelectorAll('.js-image-badge')[1].style.top).toBe('5px');
+  });
+
+  it('renders single badge for discussion object', () => {
+    createComponent({
+      discussions: {
+        ...imageDiffDiscussions[0],
+      },
+    });
+    spyOn(vm, 'getImageDimensions').and.returnValue(dimensions);
+    vm.$mount();
+
+    expect(vm.$el.querySelectorAll('.js-image-badge').length).toBe(1);
+  });
+
+  it('dispatches openDiffFileCommentForm when clicking overlay', () => {
+    createComponent({ canComment: true });
+    spyOn(vm, 'getImageDimensions').and.returnValue(dimensions);
+    vm.$mount();
+
+    spyOn(vm.$store, 'dispatch').and.stub();
+
+    vm.$el.querySelector('.js-add-image-diff-note-button').click();
+
+    expect(vm.$store.dispatch).toHaveBeenCalledWith('diffs/openDiffFileCommentForm', {
+      fileHash: 'ABC',
+      x: 0,
+      y: 0,
+      width: 100,
+      height: 200,
+    });
+  });
+
+  describe('toggle discussion', () => {
+    it('disables buttons when shouldToggleDiscussion is false', () => {
+      createComponent({ shouldToggleDiscussion: false });
+      spyOn(vm, 'getImageDimensions').and.returnValue(dimensions);
+      vm.$mount();
+
+      expect(vm.$el.querySelector('.js-image-badge').hasAttribute('disabled')).toBe(true);
+    });
+
+    it('dispatches toggleDiscussion when clicking image badge', () => {
+      createComponent();
+      spyOn(vm, 'getImageDimensions').and.returnValue(dimensions);
+      vm.$mount();
+
+      spyOn(vm.$store, 'dispatch').and.stub();
+
+      vm.$el.querySelector('.js-image-badge').click();
+
+      expect(vm.$store.dispatch).toHaveBeenCalledWith('toggleDiscussion', { discussionId: '1' });
+    });
+  });
+
+  describe('comment form', () => {
+    beforeEach(() => {
+      createComponent({}, store => {
+        store.state.diffs.commentForms.push({
+          fileHash: 'ABC',
+          x: 20,
+          y: 10,
+        });
+      });
+      spyOn(vm, 'getImageDimensions').and.returnValue(dimensions);
+      vm.$mount();
+    });
+
+    it('renders comment form badge', () => {
+      expect(vm.$el.querySelector('.comment-indicator')).not.toBe(null);
+    });
+
+    it('sets comment form badge position', () => {
+      expect(vm.$el.querySelector('.comment-indicator').style.left).toBe('20px');
+      expect(vm.$el.querySelector('.comment-indicator').style.top).toBe('10px');
+    });
+  });
+});
diff --git a/spec/javascripts/diffs/mock_data/diff_discussions.js b/spec/javascripts/diffs/mock_data/diff_discussions.js
index 0ad214ea4a4..5ffe5a366ba 100644
--- a/spec/javascripts/diffs/mock_data/diff_discussions.js
+++ b/spec/javascripts/diffs/mock_data/diff_discussions.js
@@ -492,3 +492,24 @@ export default {
   image_diff_html:
     '<div class="image js-replaced-image" data="">\n<div class="two-up view">\n<div class="wrap">\n<div class="frame deleted">\n<img alt="CHANGELOG" src="http://localhost:3000/gitlab-org/gitlab-test/raw/e63f41fe459e62e1228fcef60d7189127aeba95a/CHANGELOG" />\n</div>\n<p class="image-info hide">\n<span class="meta-filesize">22.3 KB</span>\n|\n<strong>W:</strong>\n<span class="meta-width"></span>\n|\n<strong>H:</strong>\n<span class="meta-height"></span>\n</p>\n</div>\n<div class="wrap">\n<div class="added frame js-image-frame" data-note-type="DiffNote" data-position="{&quot;base_sha&quot;:&quot;e63f41fe459e62e1228fcef60d7189127aeba95a&quot;,&quot;start_sha&quot;:&quot;d9eaefe5a676b820c57ff18cf5b68316025f7962&quot;,&quot;head_sha&quot;:&quot;c48ee0d1bf3b30453f5b32250ce03134beaa6d13&quot;,&quot;old_path&quot;:&quot;CHANGELOG&quot;,&quot;new_path&quot;:&quot;CHANGELOG&quot;,&quot;position_type&quot;:&quot;text&quot;,&quot;old_line&quot;:null,&quot;new_line&quot;:2}">\n<img alt="CHANGELOG" draggable="false" src="http://localhost:3000/gitlab-org/gitlab-test/raw/c48ee0d1bf3b30453f5b32250ce03134beaa6d13/CHANGELOG" />\n</div>\n\n<p class="image-info hide">\n<span class="meta-filesize">22.3 KB</span>\n|\n<strong>W:</strong>\n<span class="meta-width"></span>\n|\n<strong>H:</strong>\n<span class="meta-height"></span>\n</p>\n</div>\n</div>\n<div class="swipe view hide">\n<div class="swipe-frame">\n<div class="frame deleted">\n<img alt="CHANGELOG" src="http://localhost:3000/gitlab-org/gitlab-test/raw/e63f41fe459e62e1228fcef60d7189127aeba95a/CHANGELOG" />\n</div>\n<div class="swipe-wrap">\n<div class="added frame js-image-frame" data-note-type="DiffNote" data-position="{&quot;base_sha&quot;:&quot;e63f41fe459e62e1228fcef60d7189127aeba95a&quot;,&quot;start_sha&quot;:&quot;d9eaefe5a676b820c57ff18cf5b68316025f7962&quot;,&quot;head_sha&quot;:&quot;c48ee0d1bf3b30453f5b32250ce03134beaa6d13&quot;,&quot;old_path&quot;:&quot;CHANGELOG&quot;,&quot;new_path&quot;:&quot;CHANGELOG&quot;,&quot;position_type&quot;:&quot;text&quot;,&quot;old_line&quot;:null,&quot;new_line&quot;:2}">\n<img alt="CHANGELOG" draggable="false" src="http://localhost:3000/gitlab-org/gitlab-test/raw/c48ee0d1bf3b30453f5b32250ce03134beaa6d13/CHANGELOG" />\n</div>\n\n</div>\n<span class="swipe-bar">\n<span class="top-handle"></span>\n<span class="bottom-handle"></span>\n</span>\n</div>\n</div>\n<div class="onion-skin view hide">\n<div class="onion-skin-frame">\n<div class="frame deleted">\n<img alt="CHANGELOG" src="http://localhost:3000/gitlab-org/gitlab-test/raw/e63f41fe459e62e1228fcef60d7189127aeba95a/CHANGELOG" />\n</div>\n<div class="added frame js-image-frame" data-note-type="DiffNote" data-position="{&quot;base_sha&quot;:&quot;e63f41fe459e62e1228fcef60d7189127aeba95a&quot;,&quot;start_sha&quot;:&quot;d9eaefe5a676b820c57ff18cf5b68316025f7962&quot;,&quot;head_sha&quot;:&quot;c48ee0d1bf3b30453f5b32250ce03134beaa6d13&quot;,&quot;old_path&quot;:&quot;CHANGELOG&quot;,&quot;new_path&quot;:&quot;CHANGELOG&quot;,&quot;position_type&quot;:&quot;text&quot;,&quot;old_line&quot;:null,&quot;new_line&quot;:2}">\n<img alt="CHANGELOG" draggable="false" src="http://localhost:3000/gitlab-org/gitlab-test/raw/c48ee0d1bf3b30453f5b32250ce03134beaa6d13/CHANGELOG" />\n</div>\n\n<div class="controls">\n<div class="transparent"></div>\n<div class="drag-track">\n<div class="dragger" style="left: 0px;"></div>\n</div>\n<div class="opaque"></div>\n</div>\n</div>\n</div>\n</div>\n<div class="view-modes hide">\n<ul class="view-modes-menu">\n<li class="two-up" data-mode="two-up">2-up</li>\n<li class="swipe" data-mode="swipe">Swipe</li>\n<li class="onion-skin" data-mode="onion-skin">Onion skin</li>\n</ul>\n</div>\n',
 };
+
+export const imageDiffDiscussions = [
+  {
+    id: '1',
+    position: {
+      x: 10,
+      y: 10,
+      width: 100,
+      height: 200,
+    },
+  },
+  {
+    id: '2',
+    position: {
+      x: 5,
+      y: 5,
+      width: 100,
+      height: 200,
+    },
+  },
+];
diff --git a/spec/javascripts/diffs/mock_data/diff_file.js b/spec/javascripts/diffs/mock_data/diff_file.js
index d7bc0dbe431..be194ab414f 100644
--- a/spec/javascripts/diffs/mock_data/diff_file.js
+++ b/spec/javascripts/diffs/mock_data/diff_file.js
@@ -237,4 +237,5 @@ export default {
       },
     },
   ],
+  discussions: [],
 };
diff --git a/spec/javascripts/diffs/store/actions_spec.js b/spec/javascripts/diffs/store/actions_spec.js
index bb623953710..17d0f31bdd3 100644
--- a/spec/javascripts/diffs/store/actions_spec.js
+++ b/spec/javascripts/diffs/store/actions_spec.js
@@ -218,6 +218,7 @@ describe('DiffsStoreActions', () => {
         ],
       };
       const singleDiscussion = {
+        id: '1',
         fileHash: 'ABC',
         line_code: 'ABC_1_1',
       };
@@ -230,6 +231,7 @@ describe('DiffsStoreActions', () => {
           {
             type: types.REMOVE_LINE_DISCUSSIONS_FOR_FILE,
             payload: {
+              id: '1',
               fileHash: 'ABC',
               lineCode: 'ABC_1_1',
             },
diff --git a/spec/javascripts/notes/components/diff_with_note_spec.js b/spec/javascripts/notes/components/diff_with_note_spec.js
index 239d7950907..0c16103714a 100644
--- a/spec/javascripts/notes/components/diff_with_note_spec.js
+++ b/spec/javascripts/notes/components/diff_with_note_spec.js
@@ -1,7 +1,7 @@
 import Vue from 'vue';
 import DiffWithNote from '~/notes/components/diff_with_note.vue';
 import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
-import createStore from '~/notes/stores';
+import { createStore } from '~/mr_notes/stores';
 import { mountComponentWithStore } from 'spec/helpers';
 
 const discussionFixture = 'merge_requests/diff_discussion.json';
diff --git a/spec/javascripts/vue_shared/components/content_viewer/content_viewer_spec.js b/spec/javascripts/vue_shared/components/content_viewer/content_viewer_spec.js
index e2c34508b0d..4da8c6196b1 100644
--- a/spec/javascripts/vue_shared/components/content_viewer/content_viewer_spec.js
+++ b/spec/javascripts/vue_shared/components/content_viewer/content_viewer_spec.js
@@ -47,7 +47,7 @@ describe('ContentViewer', () => {
     });
 
     setTimeout(() => {
-      expect(vm.$el.querySelector('.image_file img').getAttribute('src')).toBe(GREEN_BOX_IMAGE_URL);
+      expect(vm.$el.querySelector('img').getAttribute('src')).toBe(GREEN_BOX_IMAGE_URL);
 
       done();
     });
diff --git a/spec/javascripts/vue_shared/components/diff_viewer/diff_viewer_spec.js b/spec/javascripts/vue_shared/components/diff_viewer/diff_viewer_spec.js
index fcd231ec693..67a3a2e08bc 100644
--- a/spec/javascripts/vue_shared/components/diff_viewer/diff_viewer_spec.js
+++ b/spec/javascripts/vue_shared/components/diff_viewer/diff_viewer_spec.js
@@ -30,11 +30,11 @@ describe('DiffViewer', () => {
     });
 
     setTimeout(() => {
-      expect(vm.$el.querySelector('.deleted .image_file img').getAttribute('src')).toBe(
+      expect(vm.$el.querySelector('.deleted img').getAttribute('src')).toBe(
         `//raw/DEF/${RED_BOX_IMAGE_URL}`,
       );
 
-      expect(vm.$el.querySelector('.added .image_file img').getAttribute('src')).toBe(
+      expect(vm.$el.querySelector('.added img').getAttribute('src')).toBe(
         `//raw/ABC/${GREEN_BOX_IMAGE_URL}`,
       );
 
diff --git a/spec/javascripts/vue_shared/components/diff_viewer/viewers/image_diff_viewer_spec.js b/spec/javascripts/vue_shared/components/diff_viewer/viewers/image_diff_viewer_spec.js
index 380effdb669..2d3e178d249 100644
--- a/spec/javascripts/vue_shared/components/diff_viewer/viewers/image_diff_viewer_spec.js
+++ b/spec/javascripts/vue_shared/components/diff_viewer/viewers/image_diff_viewer_spec.js
@@ -52,13 +52,9 @@ describe('ImageDiffViewer', () => {
     });
 
     setTimeout(() => {
-      expect(vm.$el.querySelector('.added .image_file img').getAttribute('src')).toBe(
-        GREEN_BOX_IMAGE_URL,
-      );
+      expect(vm.$el.querySelector('.added img').getAttribute('src')).toBe(GREEN_BOX_IMAGE_URL);
 
-      expect(vm.$el.querySelector('.deleted .image_file img').getAttribute('src')).toBe(
-        RED_BOX_IMAGE_URL,
-      );
+      expect(vm.$el.querySelector('.deleted img').getAttribute('src')).toBe(RED_BOX_IMAGE_URL);
 
       expect(vm.$el.querySelector('.view-modes-menu li.active').textContent.trim()).toBe('2-up');
       expect(vm.$el.querySelector('.view-modes-menu li:nth-child(2)').textContent.trim()).toBe(
@@ -81,9 +77,7 @@ describe('ImageDiffViewer', () => {
     });
 
     setTimeout(() => {
-      expect(vm.$el.querySelector('.added .image_file img').getAttribute('src')).toBe(
-        GREEN_BOX_IMAGE_URL,
-      );
+      expect(vm.$el.querySelector('.added img').getAttribute('src')).toBe(GREEN_BOX_IMAGE_URL);
 
       done();
     });
@@ -97,9 +91,7 @@ describe('ImageDiffViewer', () => {
     });
 
     setTimeout(() => {
-      expect(vm.$el.querySelector('.deleted .image_file img').getAttribute('src')).toBe(
-        RED_BOX_IMAGE_URL,
-      );
+      expect(vm.$el.querySelector('.deleted img').getAttribute('src')).toBe(RED_BOX_IMAGE_URL);
 
       done();
     });