diff options
| -rw-r--r-- | app/assets/javascripts/merge_request_tabs.js | 39 | ||||
| -rw-r--r-- | spec/javascripts/merge_request_tabs_spec.js | 96 |
2 files changed, 84 insertions, 51 deletions
diff --git a/app/assets/javascripts/merge_request_tabs.js b/app/assets/javascripts/merge_request_tabs.js index acfc62fe5cb..f69506a0471 100644 --- a/app/assets/javascripts/merge_request_tabs.js +++ b/app/assets/javascripts/merge_request_tabs.js @@ -1,6 +1,7 @@ /* eslint-disable no-new, class-methods-use-this */ import Cookies from 'js-cookie'; +import axios from './lib/utils/axios_utils'; import Flash from './flash'; import BlobForkSuggestion from './blob/blob_fork_suggestion'; import initChangesDropdown from './init_changes_dropdown'; @@ -244,15 +245,19 @@ export default class MergeRequestTabs { if (this.commitsLoaded) { return; } - this.ajaxGet({ - url: `${source}.json`, - success: (data) => { + + this.toggleLoading(true) + + axios.get(`${source}.json`) + .then(({ data }) => { document.querySelector('div#commits').innerHTML = data.html; localTimeAgo($('.js-timeago', 'div#commits')); this.commitsLoaded = true; this.scrollToElement('#commits'); - }, - }); + + this.toggleLoading(false); + }) + .catch(() => new Flash('An error occurred while fetching this tab.', 'alert')); } mountPipelinesView() { @@ -283,9 +288,10 @@ export default class MergeRequestTabs { // some pages like MergeRequestsController#new has query parameters on that anchor const urlPathname = parseUrlPathname(source); - this.ajaxGet({ - url: `${urlPathname}.json${location.search}`, - success: (data) => { + this.toggleLoading(true); + + axios.get(`${urlPathname}.json${location.search}`) + .then(({ data }) => { const $container = $('#diffs'); $container.html(data.html); @@ -335,8 +341,10 @@ export default class MergeRequestTabs { // (discussion and diff tabs) and `:target` only applies to the first anchor.addClass('target'); } - }, - }); + + this.toggleLoading(false); + }) + .catch(() => Flash('An error occurred while fetching this tab.', 'alert')); } // Show or hide the loading spinner @@ -346,17 +354,6 @@ export default class MergeRequestTabs { $('.mr-loading-status .loading').toggle(status); } - ajaxGet(options) { - const defaults = { - beforeSend: () => this.toggleLoading(true), - error: () => new Flash('An error occurred while fetching this tab.', 'alert'), - complete: () => this.toggleLoading(false), - dataType: 'json', - type: 'GET', - }; - $.ajax($.extend({}, defaults, options)); - } - diffViewType() { return $('.inline-parallel-buttons a.active').data('view-type'); } diff --git a/spec/javascripts/merge_request_tabs_spec.js b/spec/javascripts/merge_request_tabs_spec.js index a6be474805b..116c4e8f0e4 100644 --- a/spec/javascripts/merge_request_tabs_spec.js +++ b/spec/javascripts/merge_request_tabs_spec.js @@ -1,5 +1,6 @@ /* eslint-disable no-var, comma-dangle, object-shorthand */ - +import MockAdaptor from 'axios-mock-adapter'; +import axios from '~/lib/utils/axios_utils'; import * as urlUtils from '~/lib/utils/url_utility'; import MergeRequestTabs from '~/merge_request_tabs'; import '~/commit/pipelines/pipelines_bundle'; @@ -46,7 +47,7 @@ import 'vendor/jquery.scrollTo'; describe('activateTab', function () { beforeEach(function () { - spyOn($, 'ajax').and.callFake(function () {}); + spyOn(axios, 'get').and.returnValue(Promise.resolve({ data: {} })); loadFixtures('merge_requests/merge_request_with_task_list.html.raw'); this.subject = this.class.activateTab; }); @@ -148,7 +149,7 @@ import 'vendor/jquery.scrollTo'; describe('setCurrentAction', function () { beforeEach(function () { - spyOn($, 'ajax').and.callFake(function () {}); + spyOn(axios, 'get').and.returnValue(Promise.resolve({ data: {} })); this.subject = this.class.setCurrentAction; }); @@ -214,13 +215,21 @@ import 'vendor/jquery.scrollTo'; }); describe('tabShown', () => { + let mock; + beforeEach(function () { - spyOn($, 'ajax').and.callFake(function (options) { - options.success({ html: '' }); + mock = new MockAdaptor(axios); + mock.onGet(/(.*)\/diffs\.json/).reply(200, { + data: { html: '' }, }); + loadFixtures('merge_requests/merge_request_with_task_list.html.raw'); }); + afterEach(() => { + mock.restore(); + }); + describe('with "Side-by-side"/parallel diff view', () => { beforeEach(function () { this.class.diffViewType = () => 'parallel'; @@ -292,16 +301,20 @@ import 'vendor/jquery.scrollTo'; it('triggers Ajax request to JSON endpoint', function (done) { const url = '/foo/bar/merge_requests/1/diffs'; - spyOn(this.class, 'ajaxGet').and.callFake((options) => { - expect(options.url).toEqual(`${url}.json`); + + spyOn(axios, 'get').and.callFake((reqUrl) => { + expect(reqUrl).toBe(`${url}.json`); + done(); + + return Promise.resolve({ data: {} }); }); this.class.loadDiff(url); }); it('triggers scroll event when diff already loaded', function (done) { - spyOn(this.class, 'ajaxGet').and.callFake(() => done.fail()); + spyOn(axios, 'get').and.callFake(done.fail); spyOn(document, 'dispatchEvent'); this.class.diffsLoaded = true; @@ -316,6 +329,7 @@ import 'vendor/jquery.scrollTo'; describe('with inline diff', () => { let noteId; let noteLineNumId; + let mock; beforeEach(() => { const diffsResponse = getJSONFixture(inlineChangesTabJsonFixture); @@ -330,29 +344,40 @@ import 'vendor/jquery.scrollTo'; .attr('href') .replace('#', ''); - spyOn($, 'ajax').and.callFake(function (options) { - options.success(diffsResponse); - }); + mock = new MockAdaptor(axios); + mock.onGet(/(.*)\/diffs\.json/).reply(200, diffsResponse); + }); + + afterEach(() => { + mock.restore(); }); describe('with note fragment hash', () => { - it('should expand and scroll to linked fragment hash #note_xxx', function () { + it('should expand and scroll to linked fragment hash #note_xxx', function (done) { spyOn(urlUtils, 'getLocationHash').and.returnValue(noteId); this.class.loadDiff('/foo/bar/merge_requests/1/diffs'); - expect(noteId.length).toBeGreaterThan(0); - expect(Notes.instance.toggleDiffNote).toHaveBeenCalledWith({ - target: jasmine.any(Object), - lineType: 'old', - forceShow: true, + setTimeout(() => { + expect(noteId.length).toBeGreaterThan(0); + expect(Notes.instance.toggleDiffNote).toHaveBeenCalledWith({ + target: jasmine.any(Object), + lineType: 'old', + forceShow: true, + }); + + done(); }); }); - it('should gracefully ignore non-existant fragment hash', function () { + it('should gracefully ignore non-existant fragment hash', function (done) { spyOn(urlUtils, 'getLocationHash').and.returnValue('note_something-that-does-not-exist'); this.class.loadDiff('/foo/bar/merge_requests/1/diffs'); - expect(Notes.instance.toggleDiffNote).not.toHaveBeenCalled(); + setTimeout(() => { + expect(Notes.instance.toggleDiffNote).not.toHaveBeenCalled(); + + done(); + }); }); }); @@ -370,6 +395,7 @@ import 'vendor/jquery.scrollTo'; describe('with parallel diff', () => { let noteId; let noteLineNumId; + let mock; beforeEach(() => { const diffsResponse = getJSONFixture(parallelChangesTabJsonFixture); @@ -384,30 +410,40 @@ import 'vendor/jquery.scrollTo'; .attr('href') .replace('#', ''); - spyOn($, 'ajax').and.callFake(function (options) { - options.success(diffsResponse); - }); + mock = new MockAdaptor(axios); + mock.onGet(/(.*)\/diffs\.json/).reply(200, diffsResponse); + }); + + afterEach(() => { + mock.restore(); }); describe('with note fragment hash', () => { - it('should expand and scroll to linked fragment hash #note_xxx', function () { + it('should expand and scroll to linked fragment hash #note_xxx', function (done) { spyOn(urlUtils, 'getLocationHash').and.returnValue(noteId); this.class.loadDiff('/foo/bar/merge_requests/1/diffs'); - expect(noteId.length).toBeGreaterThan(0); - expect(Notes.instance.toggleDiffNote).toHaveBeenCalledWith({ - target: jasmine.any(Object), - lineType: 'new', - forceShow: true, + setTimeout(() => { + expect(noteId.length).toBeGreaterThan(0); + expect(Notes.instance.toggleDiffNote).toHaveBeenCalledWith({ + target: jasmine.any(Object), + lineType: 'new', + forceShow: true, + }); + + done(); }); }); - it('should gracefully ignore non-existant fragment hash', function () { + it('should gracefully ignore non-existant fragment hash', function (done) { spyOn(urlUtils, 'getLocationHash').and.returnValue('note_something-that-does-not-exist'); this.class.loadDiff('/foo/bar/merge_requests/1/diffs'); - expect(Notes.instance.toggleDiffNote).not.toHaveBeenCalled(); + setTimeout(() => { + expect(Notes.instance.toggleDiffNote).not.toHaveBeenCalled(); + done(); + }); }); }); |