diff options
author | Illya Klymov <xanf@xanf.me> | 2019-09-10 13:17:25 +0300 |
---|---|---|
committer | Illya Klymov <xanf@xanf.me> | 2019-09-10 15:53:01 +0300 |
commit | 1d73d647b06eb6314bfeaf8b5926a08ff161258a (patch) | |
tree | 322a68047265ef3e6f51d8f99f5dd29a4df2dac5 | |
parent | 2319892654fb44ea4895dd38795de0f215e49208 (diff) | |
download | gitlab-ce-fix-image-spec-recursive.tar.gz |
Fix recursive dependency in image_difffix-image-spec-recursive
utilsHelper was requiring ImageDiff and vice-versa forming a loop
Break the loop by separating initImageDiff function to separate file
9 files changed, 90 insertions, 88 deletions
diff --git a/app/assets/javascripts/diff.js b/app/assets/javascripts/diff.js index 245f1a7c558..a343138a9e1 100644 --- a/app/assets/javascripts/diff.js +++ b/app/assets/javascripts/diff.js @@ -5,7 +5,7 @@ import { __ } from '~/locale'; import { getLocationHash } from './lib/utils/url_utility'; import FilesCommentButton from './files_comment_button'; import SingleFileDiff from './single_file_diff'; -import imageDiffHelper from './image_diff/helpers/index'; +import initImageDiffHelper from './image_diff/helpers/init_image_diff'; const UNFOLD_COUNT = 20; let isBound = false; @@ -28,7 +28,7 @@ export default class Diff { .first() .get(0); const canCreateNote = firstFile && firstFile.hasAttribute('data-can-create-note'); - $diffFile.each((index, file) => imageDiffHelper.initImageDiff(file, canCreateNote)); + $diffFile.each((index, file) => initImageDiffHelper.initImageDiff(file, canCreateNote)); if (!isBound) { $(document) diff --git a/app/assets/javascripts/image_diff/helpers/index.js b/app/assets/javascripts/image_diff/helpers/index.js index 4a100631003..feeb0e8fa16 100644 --- a/app/assets/javascripts/image_diff/helpers/index.js +++ b/app/assets/javascripts/image_diff/helpers/index.js @@ -21,5 +21,4 @@ export default { resizeCoordinatesToImageElement: utilsHelper.resizeCoordinatesToImageElement, generateBadgeFromDiscussionDOM: utilsHelper.generateBadgeFromDiscussionDOM, getTargetSelection: utilsHelper.getTargetSelection, - initImageDiff: utilsHelper.initImageDiff, }; diff --git a/app/assets/javascripts/image_diff/helpers/init_image_diff.js b/app/assets/javascripts/image_diff/helpers/init_image_diff.js new file mode 100644 index 00000000000..8eef930c372 --- /dev/null +++ b/app/assets/javascripts/image_diff/helpers/init_image_diff.js @@ -0,0 +1,27 @@ +import ImageDiff from '../image_diff'; +import ReplacedImageDiff from '../replaced_image_diff'; +import ImageFile from '../../commit/image_file'; + +function initImageDiff(fileEl, canCreateNote, renderCommentBadge) { + const options = { + canCreateNote, + renderCommentBadge, + }; + let diff; + + // ImageFile needs to be invoked before initImageDiff so that badges + // can mount to the correct location + new ImageFile(fileEl); // eslint-disable-line no-new + + if (fileEl.querySelector('.diff-file .js-single-image')) { + diff = new ImageDiff(fileEl, options); + diff.init(); + } else if (fileEl.querySelector('.diff-file .js-replaced-image')) { + diff = new ReplacedImageDiff(fileEl, options); + diff.init(); + } + + return diff; +} + +export default { initImageDiff }; diff --git a/app/assets/javascripts/image_diff/helpers/utils_helper.js b/app/assets/javascripts/image_diff/helpers/utils_helper.js index beec99e6934..4b383b42dff 100644 --- a/app/assets/javascripts/image_diff/helpers/utils_helper.js +++ b/app/assets/javascripts/image_diff/helpers/utils_helper.js @@ -1,7 +1,4 @@ import ImageBadge from '../image_badge'; -import ImageDiff from '../image_diff'; -import ReplacedImageDiff from '../replaced_image_diff'; -import ImageFile from '../../commit/image_file'; export function resizeCoordinatesToImageElement(imageEl, meta) { const { x, y, width, height } = meta; @@ -70,25 +67,3 @@ export function getTargetSelection(event) { }, }; } - -export function initImageDiff(fileEl, canCreateNote, renderCommentBadge) { - const options = { - canCreateNote, - renderCommentBadge, - }; - let diff; - - // ImageFile needs to be invoked before initImageDiff so that badges - // can mount to the correct location - new ImageFile(fileEl); // eslint-disable-line no-new - - if (fileEl.querySelector('.diff-file .js-single-image')) { - diff = new ImageDiff(fileEl, options); - diff.init(); - } else if (fileEl.querySelector('.diff-file .js-replaced-image')) { - diff = new ReplacedImageDiff(fileEl, options); - diff.init(); - } - - return diff; -} diff --git a/app/assets/javascripts/image_diff/init_discussion_tab.js b/app/assets/javascripts/image_diff/init_discussion_tab.js index dbe4c06a4e9..54ff2858206 100644 --- a/app/assets/javascripts/image_diff/init_discussion_tab.js +++ b/app/assets/javascripts/image_diff/init_discussion_tab.js @@ -1,4 +1,4 @@ -import imageDiffHelper from './helpers/index'; +import initImageDiffHelper from './helpers/init_image_diff'; export default () => { // Always pass can-create-note as false because a user @@ -8,6 +8,6 @@ export default () => { const diffFileEls = document.querySelectorAll('.timeline-content .diff-file.js-image-file'); [...diffFileEls].forEach(diffFileEl => - imageDiffHelper.initImageDiff(diffFileEl, canCreateNote, renderCommentBadge), + initImageDiffHelper.initImageDiff(diffFileEl, canCreateNote, renderCommentBadge), ); }; diff --git a/app/assets/javascripts/single_file_diff.js b/app/assets/javascripts/single_file_diff.js index f2b9d75dd00..b70e384fae5 100644 --- a/app/assets/javascripts/single_file_diff.js +++ b/app/assets/javascripts/single_file_diff.js @@ -5,7 +5,7 @@ import { __ } from './locale'; import axios from './lib/utils/axios_utils'; import createFlash from './flash'; import FilesCommentButton from './files_comment_button'; -import imageDiffHelper from './image_diff/helpers/index'; +import initImageDiffHelper from './image_diff/helpers/init_image_diff'; import syntaxHighlight from './syntax_highlight'; const WRAPPER = '<div class="diff-content"></div>'; @@ -101,7 +101,7 @@ export default class SingleFileDiff { FilesCommentButton.init($file); const canCreateNote = $file.closest('.files').is('[data-can-create-note]'); - imageDiffHelper.initImageDiff($file[0], canCreateNote); + initImageDiffHelper.initImageDiff($file[0], canCreateNote); if (cb) cb(); }) diff --git a/spec/javascripts/image_diff/helpers/init_image_diff_spec.js b/spec/javascripts/image_diff/helpers/init_image_diff_spec.js new file mode 100644 index 00000000000..ba501d58965 --- /dev/null +++ b/spec/javascripts/image_diff/helpers/init_image_diff_spec.js @@ -0,0 +1,52 @@ +import initImageDiffHelper from '~/image_diff/helpers/init_image_diff'; +import ImageDiff from '~/image_diff/image_diff'; +import ReplacedImageDiff from '~/image_diff/replaced_image_diff'; + +describe('initImageDiff', () => { + let glCache; + let fileEl; + + beforeEach(() => { + window.gl = window.gl || (window.gl = {}); + glCache = window.gl; + fileEl = document.createElement('div'); + fileEl.innerHTML = ` + <div class="diff-file"></div> + `; + + spyOn(ReplacedImageDiff.prototype, 'init').and.callFake(() => {}); + spyOn(ImageDiff.prototype, 'init').and.callFake(() => {}); + }); + + afterEach(() => { + window.gl = glCache; + }); + + it('should initialize ImageDiff if js-single-image', () => { + const diffFileEl = fileEl.querySelector('.diff-file'); + diffFileEl.innerHTML = ` + <div class="js-single-image"> + </div> + `; + + const imageDiff = initImageDiffHelper.initImageDiff(fileEl, true, false); + + expect(ImageDiff.prototype.init).toHaveBeenCalled(); + expect(imageDiff.canCreateNote).toEqual(true); + expect(imageDiff.renderCommentBadge).toEqual(false); + }); + + it('should initialize ReplacedImageDiff if js-replaced-image', () => { + const diffFileEl = fileEl.querySelector('.diff-file'); + diffFileEl.innerHTML = ` + <div class="js-replaced-image"> + </div> + `; + + const replacedImageDiff = initImageDiffHelper.initImageDiff(fileEl, false, true); + + expect(ReplacedImageDiff.prototype.init).toHaveBeenCalled(); + expect(replacedImageDiff.canCreateNote).toEqual(false); + expect(replacedImageDiff.renderCommentBadge).toEqual(true); + }); +}); diff --git a/spec/javascripts/image_diff/helpers/utils_helper_spec.js b/spec/javascripts/image_diff/helpers/utils_helper_spec.js index 6f62ee3f93b..3b6378be883 100644 --- a/spec/javascripts/image_diff/helpers/utils_helper_spec.js +++ b/spec/javascripts/image_diff/helpers/utils_helper_spec.js @@ -1,6 +1,4 @@ import * as utilsHelper from '~/image_diff/helpers/utils_helper'; -import ImageDiff from '~/image_diff/image_diff'; -import ReplacedImageDiff from '~/image_diff/replaced_image_diff'; import ImageBadge from '~/image_diff/image_badge'; import * as mockData from '../mock_data'; @@ -151,53 +149,4 @@ describe('utilsHelper', () => { }); }); }); - - describe('initImageDiff', () => { - let glCache; - let fileEl; - - beforeEach(() => { - window.gl = window.gl || (window.gl = {}); - glCache = window.gl; - fileEl = document.createElement('div'); - fileEl.innerHTML = ` - <div class="diff-file"></div> - `; - - spyOn(ReplacedImageDiff.prototype, 'init').and.callFake(() => {}); - spyOn(ImageDiff.prototype, 'init').and.callFake(() => {}); - }); - - afterEach(() => { - window.gl = glCache; - }); - - it('should initialize ImageDiff if js-single-image', () => { - const diffFileEl = fileEl.querySelector('.diff-file'); - diffFileEl.innerHTML = ` - <div class="js-single-image"> - </div> - `; - - const imageDiff = utilsHelper.initImageDiff(fileEl, true, false); - - expect(ImageDiff.prototype.init).toHaveBeenCalled(); - expect(imageDiff.canCreateNote).toEqual(true); - expect(imageDiff.renderCommentBadge).toEqual(false); - }); - - it('should initialize ReplacedImageDiff if js-replaced-image', () => { - const diffFileEl = fileEl.querySelector('.diff-file'); - diffFileEl.innerHTML = ` - <div class="js-replaced-image"> - </div> - `; - - const replacedImageDiff = utilsHelper.initImageDiff(fileEl, false, true); - - expect(ReplacedImageDiff.prototype.init).toHaveBeenCalled(); - expect(replacedImageDiff.canCreateNote).toEqual(false); - expect(replacedImageDiff.renderCommentBadge).toEqual(true); - }); - }); }); diff --git a/spec/javascripts/image_diff/init_discussion_tab_spec.js b/spec/javascripts/image_diff/init_discussion_tab_spec.js index 7cacc45ab62..5eb87e1df25 100644 --- a/spec/javascripts/image_diff/init_discussion_tab_spec.js +++ b/spec/javascripts/image_diff/init_discussion_tab_spec.js @@ -1,5 +1,5 @@ import initDiscussionTab from '~/image_diff/init_discussion_tab'; -import imageDiffHelper from '~/image_diff/helpers/index'; +import initImageDiffHelper from '~/image_diff/helpers/init_image_diff'; describe('initDiscussionTab', () => { beforeEach(() => { @@ -12,7 +12,7 @@ describe('initDiscussionTab', () => { }); it('should pass canCreateNote as false to initImageDiff', done => { - spyOn(imageDiffHelper, 'initImageDiff').and.callFake((diffFileEl, canCreateNote) => { + spyOn(initImageDiffHelper, 'initImageDiff').and.callFake((diffFileEl, canCreateNote) => { expect(canCreateNote).toEqual(false); done(); }); @@ -21,7 +21,7 @@ describe('initDiscussionTab', () => { }); it('should pass renderCommentBadge as true to initImageDiff', done => { - spyOn(imageDiffHelper, 'initImageDiff').and.callFake( + spyOn(initImageDiffHelper, 'initImageDiff').and.callFake( (diffFileEl, canCreateNote, renderCommentBadge) => { expect(renderCommentBadge).toEqual(true); done(); @@ -32,9 +32,9 @@ describe('initDiscussionTab', () => { }); it('should call initImageDiff for each diffFileEls', () => { - spyOn(imageDiffHelper, 'initImageDiff').and.callFake(() => {}); + spyOn(initImageDiffHelper, 'initImageDiff').and.callFake(() => {}); initDiscussionTab(); - expect(imageDiffHelper.initImageDiff.calls.count()).toEqual(2); + expect(initImageDiffHelper.initImageDiff.calls.count()).toEqual(2); }); }); |