From 7c659e5b533d4b2ee73a3ff0a013cac9366f6ace Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Tue, 30 Jan 2018 14:53:28 +0000 Subject: Converted issue.js to axios --- app/assets/javascripts/issue.js | 40 +++++++++----------- spec/javascripts/issue_spec.js | 82 +++++++++++++++++++++++++++++------------ 2 files changed, 76 insertions(+), 46 deletions(-) diff --git a/app/assets/javascripts/issue.js b/app/assets/javascripts/issue.js index 411c820cc43..ff65ea99e9a 100644 --- a/app/assets/javascripts/issue.js +++ b/app/assets/javascripts/issue.js @@ -1,7 +1,8 @@ /* eslint-disable func-names, space-before-function-paren, no-var, prefer-rest-params, wrap-iife, one-var, no-underscore-dangle, one-var-declaration-per-line, object-shorthand, no-unused-vars, no-new, comma-dangle, consistent-return, quotes, dot-notation, quote-props, prefer-arrow-callback, max-len */ import 'vendor/jquery.waitforimages'; +import axios from './lib/utils/axios_utils'; import { addDelimiter } from './lib/utils/text_utility'; -import Flash from './flash'; +import flash from './flash'; import TaskList from './task_list'; import CreateMergeRequestDropdown from './create_merge_request_dropdown'; import IssuablesHelper from './helpers/issuables_helper'; @@ -42,12 +43,8 @@ export default class Issue { this.disableCloseReopenButton($button); url = $button.attr('href'); - return $.ajax({ - type: 'PUT', - url: url - }) - .fail(() => new Flash(issueFailMessage)) - .done((data) => { + return axios.put(url) + .then(({ data }) => { const isClosedBadge = $('div.status-box-issue-closed'); const isOpenBadge = $('div.status-box-open'); const projectIssuesCounter = $('.issue_counter'); @@ -74,9 +71,10 @@ export default class Issue { } } } else { - new Flash(issueFailMessage); + flash(issueFailMessage); } }) + .catch(() => flash(issueFailMessage)) .then(() => { this.disableCloseReopenButton($button, false); }); @@ -115,24 +113,22 @@ export default class Issue { static initMergeRequests() { var $container; $container = $('#merge-requests'); - return $.getJSON($container.data('url')).fail(function() { - return new Flash('Failed to load referenced merge requests'); - }).done(function(data) { - if ('html' in data) { - return $container.html(data.html); - } - }); + return axios.get($container.data('url')) + .then(({ data }) => { + if ('html' in data) { + $container.html(data.html); + } + }).catch(() => flash('Failed to load referenced merge requests')); } static initRelatedBranches() { var $container; $container = $('#related-branches'); - return $.getJSON($container.data('url')).fail(function() { - return new Flash('Failed to load related branches'); - }).done(function(data) { - if ('html' in data) { - return $container.html(data.html); - } - }); + return axios.get($container.data('url')) + .then(({ data }) => { + if ('html' in data) { + $container.html(data.html); + } + }).catch(() => flash('Failed to load related branches')); } } diff --git a/spec/javascripts/issue_spec.js b/spec/javascripts/issue_spec.js index 2cd2e63b15d..2da7cede510 100644 --- a/spec/javascripts/issue_spec.js +++ b/spec/javascripts/issue_spec.js @@ -1,4 +1,6 @@ /* eslint-disable space-before-function-paren, one-var, one-var-declaration-per-line, no-use-before-define, comma-dangle, max-len */ +import MockAdaptor from 'axios-mock-adapter'; +import axios from '~/lib/utils/axios_utils'; import Issue from '~/issue'; import '~/lib/utils/text_utility'; @@ -88,6 +90,7 @@ describe('Issue', function() { [true, false].forEach((isIssueInitiallyOpen) => { describe(`with ${isIssueInitiallyOpen ? 'open' : 'closed'} issue`, function() { const action = isIssueInitiallyOpen ? 'close' : 'reopen'; + let mock; function ajaxSpy(req) { if (req.url === this.$triggeredButton.attr('href')) { @@ -104,6 +107,18 @@ describe('Issue', function() { return null; } + function mockCloseButtonResponseSuccess(url, response) { + mock.onPut(url).reply(() => { + expectNewBranchButtonState(true, false); + + return [200, response]; + }); + } + + function mockCloseButtonResponseError(url) { + mock.onPut(url).networkError(); + } + beforeEach(function() { if (isIssueInitiallyOpen) { loadFixtures('issues/open-issue.html.raw'); @@ -123,68 +138,87 @@ describe('Issue', function() { this.issueStateDeferred = new jQuery.Deferred(); this.canCreateBranchDeferred = new jQuery.Deferred(); + mock = new MockAdaptor(axios); + spyOn(jQuery, 'ajax').and.callFake(ajaxSpy.bind(this)); }); - it(`${action}s the issue`, function() { - this.$triggeredButton.trigger('click'); - this.issueStateDeferred.resolve({ + afterEach(() => { + mock.restore(); + }); + + it(`${action}s the issue`, function(done) { + mockCloseButtonResponseSuccess(this.$triggeredButton.attr('href'), { id: 34 }); + + this.$triggeredButton.trigger('click'); this.canCreateBranchDeferred.resolve({ can_create_branch: !isIssueInitiallyOpen }); - expectIssueState(!isIssueInitiallyOpen); - expect(this.$triggeredButton.get(0).getAttribute('disabled')).toBeNull(); - expect(this.$projectIssuesCounter.text()).toBe(isIssueInitiallyOpen ? '1,000' : '1,002'); - expectNewBranchButtonState(false, !isIssueInitiallyOpen); + setTimeout(() => { + expectIssueState(!isIssueInitiallyOpen); + expect(this.$triggeredButton.get(0).getAttribute('disabled')).toBeNull(); + expect(this.$projectIssuesCounter.text()).toBe(isIssueInitiallyOpen ? '1,000' : '1,002'); + expectNewBranchButtonState(false, !isIssueInitiallyOpen); + + done(); + }); }); - it(`fails to ${action} the issue if saved:false`, function() { - this.$triggeredButton.trigger('click'); - this.issueStateDeferred.resolve({ + it(`fails to ${action} the issue if saved:false`, function(done) { + mockCloseButtonResponseSuccess(this.$triggeredButton.attr('href'), { saved: false }); + this.$triggeredButton.trigger('click'); this.canCreateBranchDeferred.resolve({ can_create_branch: isIssueInitiallyOpen }); - expectIssueState(isIssueInitiallyOpen); - expect(this.$triggeredButton.get(0).getAttribute('disabled')).toBeNull(); - expectErrorMessage(); - expect(this.$projectIssuesCounter.text()).toBe('1,001'); - expectNewBranchButtonState(false, isIssueInitiallyOpen); + setTimeout(() => { + expectIssueState(isIssueInitiallyOpen); + expect(this.$triggeredButton.get(0).getAttribute('disabled')).toBeNull(); + expectErrorMessage(); + expect(this.$projectIssuesCounter.text()).toBe('1,001'); + expectNewBranchButtonState(false, isIssueInitiallyOpen); + + done(); + }); }); - it(`fails to ${action} the issue if HTTP error occurs`, function() { + it(`fails to ${action} the issue if HTTP error occurs`, function(done) { + mockCloseButtonResponseError(this.$triggeredButton.attr('href')); this.$triggeredButton.trigger('click'); - this.issueStateDeferred.reject(); this.canCreateBranchDeferred.resolve({ can_create_branch: isIssueInitiallyOpen }); - expectIssueState(isIssueInitiallyOpen); - expect(this.$triggeredButton.get(0).getAttribute('disabled')).toBeNull(); - expectErrorMessage(); - expect(this.$projectIssuesCounter.text()).toBe('1,001'); - expectNewBranchButtonState(false, isIssueInitiallyOpen); + setTimeout(() => { + expectIssueState(isIssueInitiallyOpen); + expect(this.$triggeredButton.get(0).getAttribute('disabled')).toBeNull(); + expectErrorMessage(); + expect(this.$projectIssuesCounter.text()).toBe('1,001'); + expectNewBranchButtonState(false, isIssueInitiallyOpen); + + done(); + }); }); it('disables the new branch button if Ajax call fails', function() { + mockCloseButtonResponseError(this.$triggeredButton.attr('href')); this.$triggeredButton.trigger('click'); - this.issueStateDeferred.reject(); this.canCreateBranchDeferred.reject(); expectNewBranchButtonState(false, false); }); it('does not trigger Ajax call if new branch button is missing', function() { + mockCloseButtonResponseError(this.$triggeredButton.attr('href')); Issue.$btnNewBranch = $(); this.canCreateBranchDeferred = null; this.$triggeredButton.trigger('click'); - this.issueStateDeferred.reject(); }); }); }); -- cgit v1.2.1 From d504118402668872d4acc67c10a40cd03d8f8e99 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Tue, 30 Jan 2018 16:33:23 +0000 Subject: Converted job.js to axios --- app/assets/javascripts/job.js | 12 +- spec/javascripts/job_spec.js | 263 ++++++++++++++++++++++-------------------- 2 files changed, 144 insertions(+), 131 deletions(-) diff --git a/app/assets/javascripts/job.js b/app/assets/javascripts/job.js index 9b5092c5e3f..d2cbc9b82b3 100644 --- a/app/assets/javascripts/job.js +++ b/app/assets/javascripts/job.js @@ -1,4 +1,5 @@ import _ from 'underscore'; +import axios from './lib/utils/axios_utils'; import { visitUrl } from './lib/utils/url_utility'; import bp from './breakpoints'; import { numberToHumanSize } from './lib/utils/number_utils'; @@ -171,11 +172,12 @@ export default class Job { } getBuildTrace() { - return $.ajax({ - url: `${this.pagePath}/trace.json`, - data: { state: this.state }, + return axios.get(`${this.pagePath}/trace.json`, { + params: { state: this.state }, }) - .done((log) => { + .then((res) => { + const log = res.data; + setCiStatusFavicon(`${this.pagePath}/status.json`); if (log.state) { @@ -217,7 +219,7 @@ export default class Job { visitUrl(this.pagePath); } }) - .fail(() => { + .catch(() => { this.$buildRefreshAnimation.remove(); }) .then(() => { diff --git a/spec/javascripts/job_spec.js b/spec/javascripts/job_spec.js index feb341d22e6..c3d8d821ac4 100644 --- a/spec/javascripts/job_spec.js +++ b/spec/javascripts/job_spec.js @@ -1,3 +1,5 @@ +import MockAdaptor from 'axios-mock-adapter'; +import axios from '~/lib/utils/axios_utils'; import { numberToHumanSize } from '~/lib/utils/number_utils'; import * as urlUtils from '~/lib/utils/url_utility'; import '~/lib/utils/datetime_utility'; @@ -6,11 +8,31 @@ import '~/breakpoints'; describe('Job', () => { const JOB_URL = `${gl.TEST_HOST}/frontend-fixtures/builds-project/-/jobs/1`; + let mock; + let response; + + function waitForPromise() { + return new Promise(resolve => requestAnimationFrame(resolve)); + } preloadFixtures('builds/build-with-artifacts.html.raw'); beforeEach(() => { loadFixtures('builds/build-with-artifacts.html.raw'); + + spyOn(urlUtils, 'visitUrl'); + + mock = new MockAdaptor(axios); + + mock.onGet(new RegExp(`${JOB_URL}/trace.json?(.*)`)).reply(() => { + return [200, response]; + }); + }); + + afterEach(() => { + mock.restore(); + + response = {}; }); describe('class constructor', () => { @@ -55,170 +77,159 @@ describe('Job', () => { }); describe('running build', () => { - it('updates the build trace on an interval', function () { - const deferred1 = $.Deferred(); - const deferred2 = $.Deferred(); - const deferred3 = $.Deferred(); - spyOn($, 'ajax').and.returnValues(deferred1.promise(), deferred2.promise(), deferred3.promise()); - spyOn(urlUtils, 'visitUrl'); - - deferred1.resolve({ + it('updates the build trace on an interval', function (done) { + response = { html: 'Update', status: 'running', state: 'newstate', append: true, complete: false, - }); - - deferred2.resolve(); - - deferred3.resolve({ - html: 'More', - status: 'running', - state: 'finalstate', - append: true, - complete: true, - }); + }; this.job = new Job(); - expect($('#build-trace .js-build-output').text()).toMatch(/Update/); - expect(this.job.state).toBe('newstate'); - - jasmine.clock().tick(4001); - - expect($('#build-trace .js-build-output').text()).toMatch(/UpdateMore/); - expect(this.job.state).toBe('finalstate'); + waitForPromise() + .then(() => { + expect($('#build-trace .js-build-output').text()).toMatch(/Update/); + expect(this.job.state).toBe('newstate'); + + response = { + html: 'More', + status: 'running', + state: 'finalstate', + append: true, + complete: true, + }; + }) + .then(() => jasmine.clock().tick(4001)) + .then(waitForPromise) + .then(() => { + expect($('#build-trace .js-build-output').text()).toMatch(/UpdateMore/); + expect(this.job.state).toBe('finalstate'); + }) + .then(done) + .catch(done.fail); }); - it('replaces the entire build trace', () => { - const deferred1 = $.Deferred(); - const deferred2 = $.Deferred(); - const deferred3 = $.Deferred(); - - spyOn($, 'ajax').and.returnValues(deferred1.promise(), deferred2.promise(), deferred3.promise()); - - spyOn(urlUtils, 'visitUrl'); - - deferred1.resolve({ + it('replaces the entire build trace', (done) => { + response = { html: 'Update', status: 'running', append: false, complete: false, - }); - - deferred2.resolve(); - - deferred3.resolve({ - html: 'Different', - status: 'running', - append: false, - }); + }; this.job = new Job(); - expect($('#build-trace .js-build-output').text()).toMatch(/Update/); - - jasmine.clock().tick(4001); - - expect($('#build-trace .js-build-output').text()).not.toMatch(/Update/); - expect($('#build-trace .js-build-output').text()).toMatch(/Different/); + waitForPromise() + .then(() => { + expect($('#build-trace .js-build-output').text()).toMatch(/Update/); + + response = { + html: 'Different', + status: 'running', + append: false, + }; + }) + .then(() => jasmine.clock().tick(4001)) + .then(waitForPromise) + .then(() => { + expect($('#build-trace .js-build-output').text()).not.toMatch(/Update/); + expect($('#build-trace .js-build-output').text()).toMatch(/Different/); + }) + .then(done) + .catch(done.fail); }); }); describe('truncated information', () => { describe('when size is less than total', () => { - it('shows information about truncated log', () => { - spyOn(urlUtils, 'visitUrl'); - const deferred = $.Deferred(); - spyOn($, 'ajax').and.returnValue(deferred.promise()); - - deferred.resolve({ + it('shows information about truncated log', (done) => { + response = { html: 'Update', status: 'success', append: false, size: 50, total: 100, - }); + }; this.job = new Job(); - expect(document.querySelector('.js-truncated-info').classList).not.toContain('hidden'); + waitForPromise() + .then(() => { + expect(document.querySelector('.js-truncated-info').classList).not.toContain('hidden'); + }) + .then(done) + .catch(done.fail); }); - it('shows the size in KiB', () => { + it('shows the size in KiB', (done) => { const size = 50; - spyOn(urlUtils, 'visitUrl'); - const deferred = $.Deferred(); - spyOn($, 'ajax').and.returnValue(deferred.promise()); - deferred.resolve({ + response = { html: 'Update', status: 'success', append: false, size, total: 100, - }); + }; this.job = new Job(); - expect( - document.querySelector('.js-truncated-info-size').textContent.trim(), - ).toEqual(`${numberToHumanSize(size)}`); + waitForPromise() + .then(() => { + expect( + document.querySelector('.js-truncated-info-size').textContent.trim(), + ).toEqual(`${numberToHumanSize(size)}`); + }) + .then(done) + .catch(done.fail); }); - it('shows incremented size', () => { - const deferred1 = $.Deferred(); - const deferred2 = $.Deferred(); - const deferred3 = $.Deferred(); - - spyOn($, 'ajax').and.returnValues(deferred1.promise(), deferred2.promise(), deferred3.promise()); - - spyOn(urlUtils, 'visitUrl'); - - deferred1.resolve({ + it('shows incremented size', (done) => { + response = { html: 'Update', status: 'success', append: false, size: 50, total: 100, - }); - - deferred2.resolve(); + }; this.job = new Job(); - expect( - document.querySelector('.js-truncated-info-size').textContent.trim(), - ).toEqual(`${numberToHumanSize(50)}`); - - jasmine.clock().tick(4001); - - deferred3.resolve({ - html: 'Update', - status: 'success', - append: true, - size: 10, - total: 100, - }); - - expect( - document.querySelector('.js-truncated-info-size').textContent.trim(), - ).toEqual(`${numberToHumanSize(60)}`); + waitForPromise() + .then(() => { + expect( + document.querySelector('.js-truncated-info-size').textContent.trim(), + ).toEqual(`${numberToHumanSize(50)}`); + + response = { + html: 'Update', + status: 'success', + append: true, + size: 10, + total: 100, + }; + }) + .then(() => jasmine.clock().tick(4001)) + .then(waitForPromise) + .then(() => { + expect( + document.querySelector('.js-truncated-info-size').textContent.trim(), + ).toEqual(`${numberToHumanSize(60)}`); + }) + .then(done) + .catch(done.fail); }); it('renders the raw link', () => { - const deferred = $.Deferred(); - spyOn(urlUtils, 'visitUrl'); - - spyOn($, 'ajax').and.returnValue(deferred.promise()); - deferred.resolve({ + response = { html: 'Update', status: 'success', append: false, size: 50, total: 100, - }); + }; this.job = new Job(); @@ -229,50 +240,50 @@ describe('Job', () => { }); describe('when size is equal than total', () => { - it('does not show the trunctated information', () => { - const deferred = $.Deferred(); - spyOn(urlUtils, 'visitUrl'); - - spyOn($, 'ajax').and.returnValue(deferred.promise()); - deferred.resolve({ + it('does not show the trunctated information', (done) => { + response = { html: 'Update', status: 'success', append: false, size: 100, total: 100, - }); + }; this.job = new Job(); - expect(document.querySelector('.js-truncated-info').classList).toContain('hidden'); + waitForPromise() + .then(() => { + expect(document.querySelector('.js-truncated-info').classList).toContain('hidden'); + }) + .then(done) + .catch(done.fail); }); }); }); describe('output trace', () => { - beforeEach(() => { - const deferred = $.Deferred(); - spyOn(urlUtils, 'visitUrl'); - - spyOn($, 'ajax').and.returnValue(deferred.promise()); - deferred.resolve({ + beforeEach((done) => { + response = { html: 'Update', status: 'success', append: false, size: 50, total: 100, - }); + }; this.job = new Job(); + + waitForPromise() + .then(done) + .catch(done.fail); }); it('should render trace controls', () => { const controllers = document.querySelector('.controllers'); - expect(controllers.querySelector('.js-raw-link-controller')).toBeDefined(); - expect(controllers.querySelector('.js-erase-link')).toBeDefined(); - expect(controllers.querySelector('.js-scroll-up')).toBeDefined(); - expect(controllers.querySelector('.js-scroll-down')).toBeDefined(); + expect(controllers.querySelector('.js-raw-link-controller')).not.toBeNull(); + expect(controllers.querySelector('.js-scroll-up')).not.toBeNull(); + expect(controllers.querySelector('.js-scroll-down')).not.toBeNull(); }); it('should render received output', () => { @@ -285,13 +296,13 @@ describe('Job', () => { describe('getBuildTrace', () => { it('should request build trace with state parameter', (done) => { - spyOn(jQuery, 'ajax').and.callThrough(); + spyOn(axios, 'get').and.callThrough(); // eslint-disable-next-line no-new new Job(); setTimeout(() => { - expect(jQuery.ajax).toHaveBeenCalledWith( - { url: `${JOB_URL}/trace.json`, data: { state: '' } }, + expect(axios.get).toHaveBeenCalledWith( + `${JOB_URL}/trace.json`, { params: { state: '' } }, ); done(); }, 0); -- cgit v1.2.1 From 05a3479c9efe81c1ced7bba37ec4da37e4a9fcd6 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Tue, 30 Jan 2018 16:40:47 +0000 Subject: Converted labels_select.js to axios --- app/assets/javascripts/labels_select.js | 170 ++++++++++++++++---------------- 1 file changed, 85 insertions(+), 85 deletions(-) diff --git a/app/assets/javascripts/labels_select.js b/app/assets/javascripts/labels_select.js index 664e793fc8e..06b8333db27 100644 --- a/app/assets/javascripts/labels_select.js +++ b/app/assets/javascripts/labels_select.js @@ -2,9 +2,12 @@ /* global Issuable */ /* global ListLabel */ import _ from 'underscore'; +import { __ } from './locale'; +import axios from './lib/utils/axios_utils'; import IssuableBulkUpdateActions from './issuable_bulk_update_actions'; import DropdownUtils from './filtered_search/dropdown_utils'; import CreateLabelDropdown from './create_label'; +import flash from './flash'; export default class LabelsSelect { constructor(els, options = {}) { @@ -82,99 +85,96 @@ export default class LabelsSelect { } $loading.removeClass('hidden').fadeIn(); $dropdown.trigger('loading.gl.dropdown'); - return $.ajax({ - type: 'PUT', - url: issueUpdateURL, - dataType: 'JSON', - data: data - }).done(function(data) { - var labelCount, template, labelTooltipTitle, labelTitles; - $loading.fadeOut(); - $dropdown.trigger('loaded.gl.dropdown'); - $selectbox.hide(); - data.issueURLSplit = issueURLSplit; - labelCount = 0; - if (data.labels.length) { - template = labelHTMLTemplate(data); - labelCount = data.labels.length; - } - else { - template = labelNoneHTMLTemplate; - } - $value.removeAttr('style').html(template); - $sidebarCollapsedValue.text(labelCount); - - if (data.labels.length) { - labelTitles = data.labels.map(function(label) { - return label.title; - }); - - if (labelTitles.length > 5) { - labelTitles = labelTitles.slice(0, 5); - labelTitles.push('and ' + (data.labels.length - 5) + ' more'); + axios.put(issueUpdateURL, data) + .then(({ data }) => { + var labelCount, template, labelTooltipTitle, labelTitles; + $loading.fadeOut(); + $dropdown.trigger('loaded.gl.dropdown'); + $selectbox.hide(); + data.issueURLSplit = issueURLSplit; + labelCount = 0; + if (data.labels.length) { + template = labelHTMLTemplate(data); + labelCount = data.labels.length; } - - labelTooltipTitle = labelTitles.join(', '); - } - else { - labelTooltipTitle = ''; - $sidebarLabelTooltip.tooltip('destroy'); - } - - $sidebarLabelTooltip - .attr('title', labelTooltipTitle) - .tooltip('fixTitle'); - - $('.has-tooltip', $value).tooltip({ - container: 'body' - }); - }); + else { + template = labelNoneHTMLTemplate; + } + $value.removeAttr('style').html(template); + $sidebarCollapsedValue.text(labelCount); + + if (data.labels.length) { + labelTitles = data.labels.map(function(label) { + return label.title; + }); + + if (labelTitles.length > 5) { + labelTitles = labelTitles.slice(0, 5); + labelTitles.push('and ' + (data.labels.length - 5) + ' more'); + } + + labelTooltipTitle = labelTitles.join(', '); + } + else { + labelTooltipTitle = ''; + $sidebarLabelTooltip.tooltip('destroy'); + } + + $sidebarLabelTooltip + .attr('title', labelTooltipTitle) + .tooltip('fixTitle'); + + $('.has-tooltip', $value).tooltip({ + container: 'body' + }); + }) + .catch(() => flash(__('Error saving label update.'))); }; $dropdown.glDropdown({ showMenuAbove: showMenuAbove, data: function(term, callback) { - return $.ajax({ - url: labelUrl - }).done(function(data) { - data = _.chain(data).groupBy(function(label) { - return label.title; - }).map(function(label) { - var color; - color = _.map(label, function(dup) { - return dup.color; - }); - return { - id: label[0].id, - title: label[0].title, - color: color, - duplicate: color.length > 1 - }; - }).value(); - if ($dropdown.hasClass('js-extra-options')) { - var extraData = []; - if (showNo) { - extraData.unshift({ - id: 0, - title: 'No Label' - }); - } - if (showAny) { - extraData.unshift({ - isAny: true, - title: 'Any Label' + axios.get(labelUrl) + .then((res) => { + let data = _.chain(res.data).groupBy(function(label) { + return label.title; + }).map(function(label) { + var color; + color = _.map(label, function(dup) { + return dup.color; }); + return { + id: label[0].id, + title: label[0].title, + color: color, + duplicate: color.length > 1 + }; + }).value(); + if ($dropdown.hasClass('js-extra-options')) { + var extraData = []; + if (showNo) { + extraData.unshift({ + id: 0, + title: 'No Label' + }); + } + if (showAny) { + extraData.unshift({ + isAny: true, + title: 'Any Label' + }); + } + if (extraData.length) { + extraData.push('divider'); + data = extraData.concat(data); + } } - if (extraData.length) { - extraData.push('divider'); - data = extraData.concat(data); + + callback(data); + if (showMenuAbove) { + $dropdown.data('glDropdown').positionMenuAbove(); } - } - - callback(data); - if (showMenuAbove) { - $dropdown.data('glDropdown').positionMenuAbove(); - } - }); + }) + .catch(() => flash(__('Error fetching labels.'))); }, renderRow: function(label, instance) { var $a, $li, color, colorEl, indeterminate, removesAll, selectedClass, spacing, i, marked, dropdownName, dropdownValue; -- cgit v1.2.1 From 8750507b5db62e7821380d075d46d1a1c4eaca9d Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Tue, 30 Jan 2018 16:47:19 +0000 Subject: Convert merge_conflict_service.js to axios --- .../merge_conflicts/merge_conflict_service.js | 14 +++----------- .../merge_conflicts/merge_conflicts_bundle.js | 18 ++++++++---------- 2 files changed, 11 insertions(+), 21 deletions(-) diff --git a/app/assets/javascripts/merge_conflicts/merge_conflict_service.js b/app/assets/javascripts/merge_conflicts/merge_conflict_service.js index c012b77e0bf..c68b47c9348 100644 --- a/app/assets/javascripts/merge_conflicts/merge_conflict_service.js +++ b/app/assets/javascripts/merge_conflicts/merge_conflict_service.js @@ -1,4 +1,5 @@ /* eslint-disable no-param-reassign, comma-dangle */ +import axios from '../lib/utils/axios_utils'; ((global) => { global.mergeConflicts = global.mergeConflicts || {}; @@ -10,20 +11,11 @@ } fetchConflictsData() { - return $.ajax({ - dataType: 'json', - url: this.conflictsPath - }); + return axios.get(this.conflictsPath); } submitResolveConflicts(data) { - return $.ajax({ - url: this.resolveConflictsPath, - data: JSON.stringify(data), - contentType: 'application/json', - dataType: 'json', - method: 'POST' - }); + return axios.post(this.resolveConflictsPath, data); } } diff --git a/app/assets/javascripts/merge_conflicts/merge_conflicts_bundle.js b/app/assets/javascripts/merge_conflicts/merge_conflicts_bundle.js index 792b7523889..bc805047f73 100644 --- a/app/assets/javascripts/merge_conflicts/merge_conflicts_bundle.js +++ b/app/assets/javascripts/merge_conflicts/merge_conflicts_bundle.js @@ -38,24 +38,22 @@ $(() => { showDiffViewTypeSwitcher() { return mergeConflictsStore.fileTextTypePresent(); } }, created() { - mergeConflictsService - .fetchConflictsData() - .done((data) => { + mergeConflictsService.fetchConflictsData() + .then(({ data }) => { if (data.type === 'error') { mergeConflictsStore.setFailedRequest(data.message); } else { mergeConflictsStore.setConflictsData(data); } - }) - .error(() => { - mergeConflictsStore.setFailedRequest(); - }) - .always(() => { + mergeConflictsStore.setLoadingState(false); this.$nextTick(() => { syntaxHighlight($('.js-syntax-highlight')); }); + }) + .catch(() => { + mergeConflictsStore.setFailedRequest(); }); }, methods: { @@ -82,10 +80,10 @@ $(() => { mergeConflictsService .submitResolveConflicts(mergeConflictsStore.getCommitData()) - .done((data) => { + .then(({ data }) => { window.location.href = data.redirect_to; }) - .error(() => { + .catch(() => { mergeConflictsStore.setSubmitState(false); new Flash('Failed to save merge conflicts resolutions. Please try again!'); }); -- cgit v1.2.1 From 58eb3c55c9d15bd604b926ffeae9401f0b70c53c Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Tue, 30 Jan 2018 17:23:27 +0000 Subject: Converted merge_request_tabs.js to axios --- app/assets/javascripts/merge_request_tabs.js | 39 ++++++----- 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(); + }); }); }); -- cgit v1.2.1 From f165bda4ae2a92528e6f4da25825c1e441094c54 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 31 Jan 2018 09:27:30 +0000 Subject: fixed failing specs --- app/assets/javascripts/labels_select.js | 12 ++++---- app/assets/javascripts/merge_request_tabs.js | 2 +- spec/javascripts/issue_spec.js | 4 +-- spec/javascripts/job_spec.js | 8 ++--- spec/javascripts/labels_issue_sidebar_spec.js | 43 +++++++++++++++++---------- spec/javascripts/merge_request_tabs_spec.js | 8 ++--- 6 files changed, 43 insertions(+), 34 deletions(-) diff --git a/app/assets/javascripts/labels_select.js b/app/assets/javascripts/labels_select.js index 06b8333db27..5ecf81ad11d 100644 --- a/app/assets/javascripts/labels_select.js +++ b/app/assets/javascripts/labels_select.js @@ -102,28 +102,28 @@ export default class LabelsSelect { } $value.removeAttr('style').html(template); $sidebarCollapsedValue.text(labelCount); - + if (data.labels.length) { labelTitles = data.labels.map(function(label) { return label.title; }); - + if (labelTitles.length > 5) { labelTitles = labelTitles.slice(0, 5); labelTitles.push('and ' + (data.labels.length - 5) + ' more'); } - + labelTooltipTitle = labelTitles.join(', '); } else { labelTooltipTitle = ''; $sidebarLabelTooltip.tooltip('destroy'); } - + $sidebarLabelTooltip .attr('title', labelTooltipTitle) .tooltip('fixTitle'); - + $('.has-tooltip', $value).tooltip({ container: 'body' }); @@ -168,7 +168,7 @@ export default class LabelsSelect { data = extraData.concat(data); } } - + callback(data); if (showMenuAbove) { $dropdown.data('glDropdown').positionMenuAbove(); diff --git a/app/assets/javascripts/merge_request_tabs.js b/app/assets/javascripts/merge_request_tabs.js index f69506a0471..6151e90aa04 100644 --- a/app/assets/javascripts/merge_request_tabs.js +++ b/app/assets/javascripts/merge_request_tabs.js @@ -246,7 +246,7 @@ export default class MergeRequestTabs { return; } - this.toggleLoading(true) + this.toggleLoading(true); axios.get(`${source}.json`) .then(({ data }) => { diff --git a/spec/javascripts/issue_spec.js b/spec/javascripts/issue_spec.js index 2da7cede510..7eaaa0ff027 100644 --- a/spec/javascripts/issue_spec.js +++ b/spec/javascripts/issue_spec.js @@ -1,5 +1,5 @@ /* eslint-disable space-before-function-paren, one-var, one-var-declaration-per-line, no-use-before-define, comma-dangle, max-len */ -import MockAdaptor from 'axios-mock-adapter'; +import MockAdapter from 'axios-mock-adapter'; import axios from '~/lib/utils/axios_utils'; import Issue from '~/issue'; import '~/lib/utils/text_utility'; @@ -138,7 +138,7 @@ describe('Issue', function() { this.issueStateDeferred = new jQuery.Deferred(); this.canCreateBranchDeferred = new jQuery.Deferred(); - mock = new MockAdaptor(axios); + mock = new MockAdapter(axios); spyOn(jQuery, 'ajax').and.callFake(ajaxSpy.bind(this)); }); diff --git a/spec/javascripts/job_spec.js b/spec/javascripts/job_spec.js index c3d8d821ac4..03b58e9c1d0 100644 --- a/spec/javascripts/job_spec.js +++ b/spec/javascripts/job_spec.js @@ -1,4 +1,4 @@ -import MockAdaptor from 'axios-mock-adapter'; +import MockAdapter from 'axios-mock-adapter'; import axios from '~/lib/utils/axios_utils'; import { numberToHumanSize } from '~/lib/utils/number_utils'; import * as urlUtils from '~/lib/utils/url_utility'; @@ -22,11 +22,9 @@ describe('Job', () => { spyOn(urlUtils, 'visitUrl'); - mock = new MockAdaptor(axios); + mock = new MockAdapter(axios); - mock.onGet(new RegExp(`${JOB_URL}/trace.json?(.*)`)).reply(() => { - return [200, response]; - }); + mock.onGet(new RegExp(`${JOB_URL}/trace.json?(.*)`)).reply(() => [200, response]); }); afterEach(() => { diff --git a/spec/javascripts/labels_issue_sidebar_spec.js b/spec/javascripts/labels_issue_sidebar_spec.js index a197b35f6fb..7d992f62f64 100644 --- a/spec/javascripts/labels_issue_sidebar_spec.js +++ b/spec/javascripts/labels_issue_sidebar_spec.js @@ -1,4 +1,6 @@ /* eslint-disable no-new */ +import MockAdapter from 'axios-mock-adapter'; +import axios from '~/lib/utils/axios_utils'; import IssuableContext from '~/issuable_context'; import LabelsSelect from '~/labels_select'; @@ -10,35 +12,44 @@ import '~/users_select'; (() => { let saveLabelCount = 0; + let mock; + describe('Issue dropdown sidebar', () => { preloadFixtures('static/issue_sidebar_label.html.raw'); beforeEach(() => { loadFixtures('static/issue_sidebar_label.html.raw'); + + mock = new MockAdapter(axios); + new IssuableContext('{"id":1,"name":"Administrator","username":"root"}'); new LabelsSelect(); - spyOn(jQuery, 'ajax').and.callFake((req) => { - const d = $.Deferred(); - let LABELS_DATA = []; + mock.onGet('/root/test/labels.json').reply(() => { + const labels = Array(10).fill().map((_, i) => ({ + id: i, + title: `test ${i}`, + color: '#5CB85C', + })); - if (req.url === '/root/test/labels.json') { - for (let i = 0; i < 10; i += 1) { - LABELS_DATA.push({ id: i, title: `test ${i}`, color: '#5CB85C' }); - } - } else if (req.url === '/root/test/issues/2.json') { - const tmp = []; - for (let i = 0; i < saveLabelCount; i += 1) { - tmp.push({ id: i, title: `test ${i}`, color: '#5CB85C' }); - } - LABELS_DATA = { labels: tmp }; - } + return [200, labels]; + }); + + mock.onPut('/root/test/issues/2.json').reply(() => { + const labels = Array(saveLabelCount).fill().map((_, i) => ({ + id: i, + title: `test ${i}`, + color: '#5CB85C', + })); - d.resolve(LABELS_DATA); - return d.promise(); + return [200, { labels }]; }); }); + afterEach(() => { + mock.restore(); + }); + it('changes collapsed tooltip when changing labels when less than 5', (done) => { saveLabelCount = 5; $('.edit-link').get(0).click(); diff --git a/spec/javascripts/merge_request_tabs_spec.js b/spec/javascripts/merge_request_tabs_spec.js index 116c4e8f0e4..fda24db98b4 100644 --- a/spec/javascripts/merge_request_tabs_spec.js +++ b/spec/javascripts/merge_request_tabs_spec.js @@ -1,5 +1,5 @@ /* eslint-disable no-var, comma-dangle, object-shorthand */ -import MockAdaptor from 'axios-mock-adapter'; +import MockAdapter 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'; @@ -218,7 +218,7 @@ import 'vendor/jquery.scrollTo'; let mock; beforeEach(function () { - mock = new MockAdaptor(axios); + mock = new MockAdapter(axios); mock.onGet(/(.*)\/diffs\.json/).reply(200, { data: { html: '' }, }); @@ -344,7 +344,7 @@ import 'vendor/jquery.scrollTo'; .attr('href') .replace('#', ''); - mock = new MockAdaptor(axios); + mock = new MockAdapter(axios); mock.onGet(/(.*)\/diffs\.json/).reply(200, diffsResponse); }); @@ -410,7 +410,7 @@ import 'vendor/jquery.scrollTo'; .attr('href') .replace('#', ''); - mock = new MockAdaptor(axios); + mock = new MockAdapter(axios); mock.onGet(/(.*)\/diffs\.json/).reply(200, diffsResponse); }); -- cgit v1.2.1 From d58ff9433db2e737329d7aea436d086a133bdfe0 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 31 Jan 2018 09:29:29 +0000 Subject: Converted milestone.js to axios --- app/assets/javascripts/milestone.js | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/app/assets/javascripts/milestone.js b/app/assets/javascripts/milestone.js index dd6c6b854bc..b1d74250dfd 100644 --- a/app/assets/javascripts/milestone.js +++ b/app/assets/javascripts/milestone.js @@ -1,4 +1,5 @@ -import Flash from './flash'; +import axios from './lib/utils/axios_utils'; +import flash from './flash'; export default class Milestone { constructor() { @@ -33,15 +34,12 @@ export default class Milestone { const tabElId = $target.attr('href'); if (endpoint && !$target.hasClass('is-loaded')) { - $.ajax({ - url: endpoint, - dataType: 'JSON', - }) - .fail(() => new Flash('Error loading milestone tab')) - .done((data) => { - $(tabElId).html(data.html); - $target.addClass('is-loaded'); - }); + axios.get(endpoint) + .then(({ data }) => { + $(tabElId).html(data.html); + $target.addClass('is-loaded'); + }) + .catch(() => flash('Error loading milestone tab')); } } } -- cgit v1.2.1 From 3ae2d90b49f727096fb7848bb904951db2179c68 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 31 Jan 2018 09:34:25 +0000 Subject: Converted milestone_select.js to axios --- app/assets/javascripts/milestone_select.js | 119 ++++++++++++++--------------- 1 file changed, 58 insertions(+), 61 deletions(-) diff --git a/app/assets/javascripts/milestone_select.js b/app/assets/javascripts/milestone_select.js index 0e854295fe3..6581be606eb 100644 --- a/app/assets/javascripts/milestone_select.js +++ b/app/assets/javascripts/milestone_select.js @@ -2,6 +2,7 @@ /* global Issuable */ /* global ListMilestone */ import _ from 'underscore'; +import axios from './lib/utils/axios_utils'; import { timeFor } from './lib/utils/datetime_utility'; export default class MilestoneSelect { @@ -52,48 +53,47 @@ export default class MilestoneSelect { } return $dropdown.glDropdown({ showMenuAbove: showMenuAbove, - data: (term, callback) => $.ajax({ - url: milestonesUrl - }).done((data) => { - const extraOptions = []; - if (showAny) { - extraOptions.push({ - id: 0, - name: '', - title: 'Any Milestone' - }); - } - if (showNo) { - extraOptions.push({ - id: -1, - name: 'No Milestone', - title: 'No Milestone' - }); - } - if (showUpcoming) { - extraOptions.push({ - id: -2, - name: '#upcoming', - title: 'Upcoming' - }); - } - if (showStarted) { - extraOptions.push({ - id: -3, - name: '#started', - title: 'Started' - }); - } - if (extraOptions.length) { - extraOptions.push('divider'); - } + data: (term, callback) => axios.get(milestonesUrl) + .then(({ data }) => { + const extraOptions = []; + if (showAny) { + extraOptions.push({ + id: 0, + name: '', + title: 'Any Milestone' + }); + } + if (showNo) { + extraOptions.push({ + id: -1, + name: 'No Milestone', + title: 'No Milestone' + }); + } + if (showUpcoming) { + extraOptions.push({ + id: -2, + name: '#upcoming', + title: 'Upcoming' + }); + } + if (showStarted) { + extraOptions.push({ + id: -3, + name: '#started', + title: 'Started' + }); + } + if (extraOptions.length) { + extraOptions.push('divider'); + } - callback(extraOptions.concat(data)); - if (showMenuAbove) { - $dropdown.data('glDropdown').positionMenuAbove(); - } - $(`[data-milestone-id="${selectedMilestone}"] > a`).addClass('is-active'); - }), + callback(extraOptions.concat(data)); + if (showMenuAbove) { + $dropdown.data('glDropdown').positionMenuAbove(); + } + $(`[data-milestone-id="${selectedMilestone}"] > a`).addClass('is-active'); + }), renderRow: milestone => `
  • @@ -200,26 +200,23 @@ export default class MilestoneSelect { data[abilityName].milestone_id = selected != null ? selected : null; $loading.removeClass('hidden').fadeIn(); $dropdown.trigger('loading.gl.dropdown'); - return $.ajax({ - type: 'PUT', - url: issueUpdateURL, - data: data - }).done((data) => { - $dropdown.trigger('loaded.gl.dropdown'); - $loading.fadeOut(); - $selectBox.hide(); - $value.css('display', ''); - if (data.milestone != null) { - data.milestone.full_path = this.currentProject.full_path; - data.milestone.remaining = timeFor(data.milestone.due_date); - data.milestone.name = data.milestone.title; - $value.html(milestoneLinkTemplate(data.milestone)); - return $sidebarCollapsedValue.find('span').html(collapsedSidebarLabelTemplate(data.milestone)); - } else { - $value.html(milestoneLinkNoneTemplate); - return $sidebarCollapsedValue.find('span').text('No'); - } - }); + return axios.put(issueUpdateURL, data) + .then(({ data }) => { + $dropdown.trigger('loaded.gl.dropdown'); + $loading.fadeOut(); + $selectBox.hide(); + $value.css('display', ''); + if (data.milestone != null) { + data.milestone.full_path = this.currentProject.full_path; + data.milestone.remaining = timeFor(data.milestone.due_date); + data.milestone.name = data.milestone.title; + $value.html(milestoneLinkTemplate(data.milestone)); + return $sidebarCollapsedValue.find('span').html(collapsedSidebarLabelTemplate(data.milestone)); + } else { + $value.html(milestoneLinkNoneTemplate); + return $sidebarCollapsedValue.find('span').text('No'); + } + }); } } }); -- cgit v1.2.1 From ae401d03d3261cda379ccd3367a877fb095c4e59 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 31 Jan 2018 09:58:14 +0000 Subject: Converted ajax_cache to axios --- app/assets/javascripts/lib/utils/ajax_cache.js | 32 +++++------- spec/javascripts/lib/utils/ajax_cache_spec.js | 69 +++++++++++--------------- 2 files changed, 41 insertions(+), 60 deletions(-) diff --git a/app/assets/javascripts/lib/utils/ajax_cache.js b/app/assets/javascripts/lib/utils/ajax_cache.js index 629d8f44e18..616d8952ada 100644 --- a/app/assets/javascripts/lib/utils/ajax_cache.js +++ b/app/assets/javascripts/lib/utils/ajax_cache.js @@ -1,3 +1,4 @@ +import axios from './axios_utils'; import Cache from './cache'; class AjaxCache extends Cache { @@ -18,25 +19,18 @@ class AjaxCache extends Cache { let pendingRequest = this.pendingRequests[endpoint]; if (!pendingRequest) { - pendingRequest = new Promise((resolve, reject) => { - // jQuery 2 is not Promises/A+ compatible (missing catch) - $.ajax(endpoint) // eslint-disable-line promise/catch-or-return - .then(data => resolve(data), - (jqXHR, textStatus, errorThrown) => { - const error = new Error(`${endpoint}: ${errorThrown}`); - error.textStatus = textStatus; - reject(error); - }, - ); - }) - .then((data) => { - this.internalStorage[endpoint] = data; - delete this.pendingRequests[endpoint]; - }) - .catch((error) => { - delete this.pendingRequests[endpoint]; - throw error; - }); + pendingRequest = axios.get(endpoint) + .then(({ data }) => { + this.internalStorage[endpoint] = data; + delete this.pendingRequests[endpoint]; + }) + .catch((e) => { + const error = new Error(`${endpoint}: ${e.message}`); + error.textStatus = e.message; + + delete this.pendingRequests[endpoint]; + throw error; + }); this.pendingRequests[endpoint] = pendingRequest; } diff --git a/spec/javascripts/lib/utils/ajax_cache_spec.js b/spec/javascripts/lib/utils/ajax_cache_spec.js index 49971bd91e2..be137ad8167 100644 --- a/spec/javascripts/lib/utils/ajax_cache_spec.js +++ b/spec/javascripts/lib/utils/ajax_cache_spec.js @@ -1,3 +1,5 @@ +import MockAdapter from 'axios-mock-adapter'; +import axios from '~/lib/utils/axios_utils'; import AjaxCache from '~/lib/utils/ajax_cache'; describe('AjaxCache', () => { @@ -86,67 +88,55 @@ describe('AjaxCache', () => { }); }); - describe('retrieve', () => { + fdescribe('retrieve', () => { let ajaxSpy; + let mock; beforeEach(() => { - spyOn(jQuery, 'ajax').and.callFake(url => ajaxSpy(url)); + mock = new MockAdapter(axios); + + spyOn(axios, 'get').and.callThrough(); + }); + + afterEach(() => { + mock.restore(); }); it('stores and returns data from Ajax call if cache is empty', (done) => { - ajaxSpy = (url) => { - expect(url).toBe(dummyEndpoint); - const deferred = $.Deferred(); - deferred.resolve(dummyResponse); - return deferred.promise(); - }; + mock.onGet(dummyEndpoint).reply(200, dummyResponse); AjaxCache.retrieve(dummyEndpoint) .then((data) => { - expect(data).toBe(dummyResponse); - expect(AjaxCache.internalStorage[dummyEndpoint]).toBe(dummyResponse); + expect(data).toEqual(dummyResponse); + expect(AjaxCache.internalStorage[dummyEndpoint]).toEqual(dummyResponse); }) .then(done) .catch(fail); }); - it('makes no Ajax call if request is pending', () => { - const responseDeferred = $.Deferred(); - - ajaxSpy = (url) => { - expect(url).toBe(dummyEndpoint); - // neither reject nor resolve to keep request pending - return responseDeferred.promise(); - }; - - const unexpectedResponse = data => fail(`Did not expect response: ${data}`); + it('makes no Ajax call if request is pending', (done) => { + mock.onGet(dummyEndpoint).reply(200, dummyResponse); AjaxCache.retrieve(dummyEndpoint) - .then(unexpectedResponse) + .then(done) .catch(fail); AjaxCache.retrieve(dummyEndpoint) - .then(unexpectedResponse) + .then(done) .catch(fail); - expect($.ajax.calls.count()).toBe(1); + expect(axios.get.calls.count()).toBe(1); }); it('returns undefined if Ajax call fails and cache is empty', (done) => { - const dummyStatusText = 'exploded'; - const dummyErrorMessage = 'server exploded'; - ajaxSpy = (url) => { - expect(url).toBe(dummyEndpoint); - const deferred = $.Deferred(); - deferred.reject(null, dummyStatusText, dummyErrorMessage); - return deferred.promise(); - }; + const errorMessage = 'Network Error'; + mock.onGet(dummyEndpoint).networkError(); AjaxCache.retrieve(dummyEndpoint) .then(data => fail(`Received unexpected data: ${JSON.stringify(data)}`)) .catch((error) => { - expect(error.message).toBe(`${dummyEndpoint}: ${dummyErrorMessage}`); - expect(error.textStatus).toBe(dummyStatusText); + expect(error.message).toBe(`${dummyEndpoint}: ${errorMessage}`); + expect(error.textStatus).toBe(errorMessage); done(); }) .catch(fail); @@ -154,7 +144,9 @@ describe('AjaxCache', () => { it('makes no Ajax call if matching data exists', (done) => { AjaxCache.internalStorage[dummyEndpoint] = dummyResponse; - ajaxSpy = () => fail(new Error('expected no Ajax call!')); + mock.onGet(dummyEndpoint).reply(() => { + fail(new Error('expected no Ajax call!')); + }); AjaxCache.retrieve(dummyEndpoint) .then((data) => { @@ -171,12 +163,7 @@ describe('AjaxCache', () => { AjaxCache.internalStorage[dummyEndpoint] = oldDummyResponse; - ajaxSpy = (url) => { - expect(url).toBe(dummyEndpoint); - const deferred = $.Deferred(); - deferred.resolve(dummyResponse); - return deferred.promise(); - }; + mock.onGet(dummyEndpoint).reply(200, dummyResponse); // Call without forceRetrieve param AjaxCache.retrieve(dummyEndpoint) @@ -189,7 +176,7 @@ describe('AjaxCache', () => { // Call with forceRetrieve param AjaxCache.retrieve(dummyEndpoint, true) .then((data) => { - expect(data).toBe(dummyResponse); + expect(data).toEqual(dummyResponse); }) .then(done) .catch(fail); -- cgit v1.2.1 From 641f1d2c4b210026fd40c4c3d3acc1bd41dc9adc Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 31 Jan 2018 10:14:55 +0000 Subject: Converted common_utils to axios --- app/assets/javascripts/lib/utils/common_utils.js | 16 +++++++--------- app/assets/javascripts/notes.js | 12 +++++++----- spec/javascripts/lib/utils/common_utils_spec.js | 8 +++++--- 3 files changed, 19 insertions(+), 17 deletions(-) diff --git a/app/assets/javascripts/lib/utils/common_utils.js b/app/assets/javascripts/lib/utils/common_utils.js index 03918762842..0fcf8b410af 100644 --- a/app/assets/javascripts/lib/utils/common_utils.js +++ b/app/assets/javascripts/lib/utils/common_utils.js @@ -1,3 +1,4 @@ +import axios from './axios_utils'; import { getLocationHash } from './url_utility'; export const getPagePath = (index = 0) => $('body').attr('data-page').split(':')[index]; @@ -27,17 +28,14 @@ export const isInIssuePage = () => { return page === 'issues' && action === 'show'; }; -export const ajaxGet = url => $.ajax({ - type: 'GET', - url, - dataType: 'script', +export const ajaxGet = url => axios.get(url, { + params: { format: 'js' }, + responseType: 'text', +}).then(({ data }) => { + $.globalEval(data); }); -export const ajaxPost = (url, data) => $.ajax({ - type: 'POST', - url, - data, -}); +export const ajaxPost = (url, data) => axios.post(url, data); export const rstrip = (val) => { if (val) { diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index a2b8e6f6495..2f37d40ebad 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -1487,7 +1487,9 @@ export default class Notes { /* eslint-disable promise/catch-or-return */ // Make request to submit comment on server ajaxPost(formAction, formData) - .then((note) => { + .then((res) => { + const note = res.data; + // Submission successful! remove placeholder $notesContainer.find(`#${noteUniqueId}`).remove(); @@ -1560,7 +1562,7 @@ export default class Notes { } $form.trigger('ajax:success', [note]); - }).fail(() => { + }).catch(() => { // Submission failed, remove placeholder note and show Flash error message $notesContainer.find(`#${noteUniqueId}`).remove(); @@ -1631,11 +1633,11 @@ export default class Notes { /* eslint-disable promise/catch-or-return */ // Make request to update comment on server ajaxPost(formAction, formData) - .then((note) => { + .then(({ data }) => { // Submission successful! render final note element - this.updateNote(note, $editingNote); + this.updateNote(data, $editingNote); }) - .fail(() => { + .catch(() => { // Submission failed, revert back to original note $noteBodyText.html(_.escape(cachedNoteBodyText)); $editingNote.removeClass('being-posted fade-in'); diff --git a/spec/javascripts/lib/utils/common_utils_spec.js b/spec/javascripts/lib/utils/common_utils_spec.js index 0a9d815f469..bb71c181b3d 100644 --- a/spec/javascripts/lib/utils/common_utils_spec.js +++ b/spec/javascripts/lib/utils/common_utils_spec.js @@ -1,5 +1,5 @@ /* eslint-disable promise/catch-or-return */ - +import axios from '~/lib/utils/axios_utils'; import * as commonUtils from '~/lib/utils/common_utils'; describe('common_utils', () => { @@ -451,10 +451,12 @@ describe('common_utils', () => { it('should perform `$.ajax` call and do `POST` request', () => { const requestURL = '/some/random/api'; const data = { keyname: 'value' }; - const ajaxSpy = spyOn($, 'ajax').and.callFake(() => {}); + const ajaxSpy = spyOn(axios, 'post').and.callFake(() => {}); commonUtils.ajaxPost(requestURL, data); - expect(ajaxSpy.calls.allArgs()[0][0].type).toEqual('POST'); + + expect(ajaxSpy.calls.allArgs()[0][0]).toEqual(requestURL); + expect(ajaxSpy.calls.allArgs()[0][1]).toEqual(data); }); }); -- cgit v1.2.1 From 7bd66940d9f1d79e7277dedfa6cb2749857660d6 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 31 Jan 2018 15:12:10 +0000 Subject: fixed eslint --- spec/javascripts/lib/utils/ajax_cache_spec.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/spec/javascripts/lib/utils/ajax_cache_spec.js b/spec/javascripts/lib/utils/ajax_cache_spec.js index be137ad8167..7603400b55e 100644 --- a/spec/javascripts/lib/utils/ajax_cache_spec.js +++ b/spec/javascripts/lib/utils/ajax_cache_spec.js @@ -88,8 +88,7 @@ describe('AjaxCache', () => { }); }); - fdescribe('retrieve', () => { - let ajaxSpy; + describe('retrieve', () => { let mock; beforeEach(() => { -- cgit v1.2.1 From a3356bce09d1a5e1222254a8e193b98f90d71e4e Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 31 Jan 2018 15:48:12 +0000 Subject: fixed eslint :see_no_evil: --- app/assets/javascripts/lib/utils/common_utils.js | 1 - spec/javascripts/lib/utils/common_utils_spec.js | 1 - 2 files changed, 2 deletions(-) diff --git a/app/assets/javascripts/lib/utils/common_utils.js b/app/assets/javascripts/lib/utils/common_utils.js index 13e0b0de437..c27c4c6e621 100644 --- a/app/assets/javascripts/lib/utils/common_utils.js +++ b/app/assets/javascripts/lib/utils/common_utils.js @@ -1,6 +1,5 @@ import axios from './axios_utils'; import { getLocationHash } from './url_utility'; -import axios from './axios_utils'; export const getPagePath = (index = 0) => $('body').attr('data-page').split(':')[index]; diff --git a/spec/javascripts/lib/utils/common_utils_spec.js b/spec/javascripts/lib/utils/common_utils_spec.js index 50dac3def60..864ee4995ea 100644 --- a/spec/javascripts/lib/utils/common_utils_spec.js +++ b/spec/javascripts/lib/utils/common_utils_spec.js @@ -1,7 +1,6 @@ /* eslint-disable promise/catch-or-return */ import axios from '~/lib/utils/axios_utils'; import * as commonUtils from '~/lib/utils/common_utils'; -import axios from '~/lib/utils/axios_utils'; import MockAdapter from 'axios-mock-adapter'; describe('common_utils', () => { -- cgit v1.2.1 From 0f895147ff52d71c7ced5db536d2be6154839112 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 31 Jan 2018 17:51:53 +0000 Subject: fixed issue_spec transient failure --- spec/javascripts/issue_spec.js | 59 +++++++++++++++++++----------------------- 1 file changed, 27 insertions(+), 32 deletions(-) diff --git a/spec/javascripts/issue_spec.js b/spec/javascripts/issue_spec.js index 7eaaa0ff027..5245bf6455c 100644 --- a/spec/javascripts/issue_spec.js +++ b/spec/javascripts/issue_spec.js @@ -92,21 +92,6 @@ describe('Issue', function() { const action = isIssueInitiallyOpen ? 'close' : 'reopen'; let mock; - function ajaxSpy(req) { - if (req.url === this.$triggeredButton.attr('href')) { - expect(req.type).toBe('PUT'); - expectNewBranchButtonState(true, false); - return this.issueStateDeferred; - } else if (req.url === Issue.createMrDropdownWrap.dataset.canCreatePath) { - expect(req.type).toBe('GET'); - expectNewBranchButtonState(true, false); - return this.canCreateBranchDeferred; - } - - expect(req.url).toBe('unexpected'); - return null; - } - function mockCloseButtonResponseSuccess(url, response) { mock.onPut(url).reply(() => { expectNewBranchButtonState(true, false); @@ -119,6 +104,12 @@ describe('Issue', function() { mock.onPut(url).networkError(); } + function mockCanCreateBranch(canCreateBranch) { + mock.onGet(/(.*)\/can_create_branch$/).reply(200, { + can_create_branch: canCreateBranch, + }); + } + beforeEach(function() { if (isIssueInitiallyOpen) { loadFixtures('issues/open-issue.html.raw'); @@ -126,6 +117,11 @@ describe('Issue', function() { loadFixtures('issues/closed-issue.html.raw'); } + mock = new MockAdapter(axios); + + mock.onGet(/(.*)\/related_branches$/).reply(200, {}); + mock.onGet(/(.*)\/referenced_merge_requests$/).reply(200, {}); + findElements(isIssueInitiallyOpen); this.issue = new Issue(); expectIssueState(isIssueInitiallyOpen); @@ -135,27 +131,21 @@ describe('Issue', function() { this.$projectIssuesCounter = $('.issue_counter').first(); this.$projectIssuesCounter.text('1,001'); - this.issueStateDeferred = new jQuery.Deferred(); - this.canCreateBranchDeferred = new jQuery.Deferred(); - - mock = new MockAdapter(axios); - - spyOn(jQuery, 'ajax').and.callFake(ajaxSpy.bind(this)); + spyOn(axios, 'get').and.callThrough(); }); afterEach(() => { mock.restore(); + $('div.flash-alert').remove(); }); it(`${action}s the issue`, function(done) { mockCloseButtonResponseSuccess(this.$triggeredButton.attr('href'), { id: 34 }); + mockCanCreateBranch(!isIssueInitiallyOpen); this.$triggeredButton.trigger('click'); - this.canCreateBranchDeferred.resolve({ - can_create_branch: !isIssueInitiallyOpen - }); setTimeout(() => { expectIssueState(!isIssueInitiallyOpen); @@ -171,10 +161,9 @@ describe('Issue', function() { mockCloseButtonResponseSuccess(this.$triggeredButton.attr('href'), { saved: false }); + mockCanCreateBranch(isIssueInitiallyOpen); + this.$triggeredButton.trigger('click'); - this.canCreateBranchDeferred.resolve({ - can_create_branch: isIssueInitiallyOpen - }); setTimeout(() => { expectIssueState(isIssueInitiallyOpen); @@ -189,10 +178,9 @@ describe('Issue', function() { it(`fails to ${action} the issue if HTTP error occurs`, function(done) { mockCloseButtonResponseError(this.$triggeredButton.attr('href')); + mockCanCreateBranch(isIssueInitiallyOpen); + this.$triggeredButton.trigger('click'); - this.canCreateBranchDeferred.resolve({ - can_create_branch: isIssueInitiallyOpen - }); setTimeout(() => { expectIssueState(isIssueInitiallyOpen); @@ -207,18 +195,25 @@ describe('Issue', function() { it('disables the new branch button if Ajax call fails', function() { mockCloseButtonResponseError(this.$triggeredButton.attr('href')); + mock.onGet(/(.*)\/can_create_branch$/).networkError(); + this.$triggeredButton.trigger('click'); - this.canCreateBranchDeferred.reject(); expectNewBranchButtonState(false, false); }); - it('does not trigger Ajax call if new branch button is missing', function() { + it('does not trigger Ajax call if new branch button is missing', function(done) { mockCloseButtonResponseError(this.$triggeredButton.attr('href')); Issue.$btnNewBranch = $(); this.canCreateBranchDeferred = null; this.$triggeredButton.trigger('click'); + + setTimeout(() => { + expect(axios.get).not.toHaveBeenCalled(); + + done(); + }); }); }); }); -- cgit v1.2.1 From 988747df49d9441f539c51efd6f808f77173c3e0 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 1 Feb 2018 09:07:16 +0000 Subject: fixed notes_spec.js --- spec/javascripts/notes_spec.js | 182 ++++++++++++++++++++++++++++------------- 1 file changed, 124 insertions(+), 58 deletions(-) diff --git a/spec/javascripts/notes_spec.js b/spec/javascripts/notes_spec.js index a40821a5693..808e0f6ca31 100644 --- a/spec/javascripts/notes_spec.js +++ b/spec/javascripts/notes_spec.js @@ -1,11 +1,14 @@ /* eslint-disable space-before-function-paren, no-unused-expressions, no-var, object-shorthand, comma-dangle, max-len */ import _ from 'underscore'; +import MockAdapter from 'axios-mock-adapter'; +import axios from '~/lib/utils/axios_utils'; import * as urlUtils from '~/lib/utils/url_utility'; import 'autosize'; import '~/gl_form'; import '~/lib/utils/text_utility'; import '~/render_gfm'; import Notes from '~/notes'; +import timeoutPromise from './helpers/set_timeout_promise_helper'; (function() { window.gon || (window.gon = {}); @@ -119,6 +122,7 @@ import Notes from '~/notes'; let noteEntity; let $form; let $notesContainer; + let mock; beforeEach(() => { this.notes = new Notes('', []); @@ -136,24 +140,28 @@ import Notes from '~/notes'; $form = $('form.js-main-target-form'); $notesContainer = $('ul.main-notes-list'); $form.find('textarea.js-note-text').val(sampleComment); + + mock = new MockAdapter(axios); + mock.onPost(/(.*)\/notes$/).reply(200, noteEntity); }); - it('updates note and resets edit form', () => { - const deferred = $.Deferred(); - spyOn($, 'ajax').and.returnValue(deferred.promise()); + it('updates note and resets edit form', (done) => { spyOn(this.notes, 'revertNoteEditForm'); spyOn(this.notes, 'setupNewNote'); $('.js-comment-button').click(); - deferred.resolve(noteEntity); - const $targetNote = $notesContainer.find(`#note_${noteEntity.id}`); - const updatedNote = Object.assign({}, noteEntity); - updatedNote.note = 'bar'; - this.notes.updateNote(updatedNote, $targetNote); + setTimeout(() => { + const $targetNote = $notesContainer.find(`#note_${noteEntity.id}`); + const updatedNote = Object.assign({}, noteEntity); + updatedNote.note = 'bar'; + this.notes.updateNote(updatedNote, $targetNote); + + expect(this.notes.revertNoteEditForm).toHaveBeenCalledWith($targetNote); + expect(this.notes.setupNewNote).toHaveBeenCalled(); - expect(this.notes.revertNoteEditForm).toHaveBeenCalledWith($targetNote); - expect(this.notes.setupNewNote).toHaveBeenCalled(); + done(); + }); }); }); @@ -479,8 +487,19 @@ import Notes from '~/notes'; }; let $form; let $notesContainer; + let mock; + + function mockNotesPost() { + mock.onPost(/(.*)\/notes$/).reply(200, note); + } + + function mockNotesPostError() { + mock.onPost(/(.*)\/notes$/).networkError(); + } beforeEach(() => { + mock = new MockAdapter(axios); + this.notes = new Notes('', []); window.gon.current_username = 'root'; window.gon.current_user_fullname = 'Administrator'; @@ -489,63 +508,92 @@ import Notes from '~/notes'; $form.find('textarea.js-note-text').val(sampleComment); }); + afterEach(() => { + mock.restore(); + }); + it('should show placeholder note while new comment is being posted', () => { + mockNotesPost(); + $('.js-comment-button').click(); expect($notesContainer.find('.note.being-posted').length > 0).toEqual(true); }); - it('should remove placeholder note when new comment is done posting', () => { - const deferred = $.Deferred(); - spyOn($, 'ajax').and.returnValue(deferred.promise()); + it('should remove placeholder note when new comment is done posting', (done) => { + mockNotesPost(); + $('.js-comment-button').click(); - deferred.resolve(note); - expect($notesContainer.find('.note.being-posted').length).toEqual(0); + setTimeout(() => { + expect($notesContainer.find('.note.being-posted').length).toEqual(0); + + done(); + }); }); - it('should show actual note element when new comment is done posting', () => { - const deferred = $.Deferred(); - spyOn($, 'ajax').and.returnValue(deferred.promise()); + it('should show actual note element when new comment is done posting', (done) => { + mockNotesPost(); + $('.js-comment-button').click(); - deferred.resolve(note); - expect($notesContainer.find(`#note_${note.id}`).length > 0).toEqual(true); + setTimeout(() => { + expect($notesContainer.find(`#note_${note.id}`).length > 0).toEqual(true); + + done(); + }); }); - it('should reset Form when new comment is done posting', () => { - const deferred = $.Deferred(); - spyOn($, 'ajax').and.returnValue(deferred.promise()); + it('should reset Form when new comment is done posting', (done) => { + mockNotesPost(); + $('.js-comment-button').click(); - deferred.resolve(note); - expect($form.find('textarea.js-note-text').val()).toEqual(''); + setTimeout(() => { + expect($form.find('textarea.js-note-text').val()).toEqual(''); + + done(); + }); }); - it('should show flash error message when new comment failed to be posted', () => { - const deferred = $.Deferred(); - spyOn($, 'ajax').and.returnValue(deferred.promise()); + it('should show flash error message when new comment failed to be posted', (done) => { + mockNotesPostError(); + $('.js-comment-button').click(); - deferred.reject(); - expect($notesContainer.parent().find('.flash-container .flash-text').is(':visible')).toEqual(true); + setTimeout(() => { + expect($notesContainer.parent().find('.flash-container .flash-text').is(':visible')).toEqual(true); + + done(); + }); }); - it('should show flash error message when comment failed to be updated', () => { - const deferred = $.Deferred(); - spyOn($, 'ajax').and.returnValue(deferred.promise()); + it('should show flash error message when comment failed to be updated', (done) => { + mockNotesPost(); + $('.js-comment-button').click(); - deferred.resolve(note); - const $noteEl = $notesContainer.find(`#note_${note.id}`); - $noteEl.find('.js-note-edit').click(); - $noteEl.find('textarea.js-note-text').val(updatedComment); - $noteEl.find('.js-comment-save-button').click(); + timeoutPromise() + .then(() => { + const $noteEl = $notesContainer.find(`#note_${note.id}`); + $noteEl.find('.js-note-edit').click(); + $noteEl.find('textarea.js-note-text').val(updatedComment); - deferred.reject(); - const $updatedNoteEl = $notesContainer.find(`#note_${note.id}`); - expect($updatedNoteEl.hasClass('.being-posted')).toEqual(false); // Remove being-posted visuals - expect($updatedNoteEl.find('.note-text').text().trim()).toEqual(sampleComment); // See if comment reverted back to original - expect($('.flash-container').is(':visible')).toEqual(true); // Flash error message shown + mock.restore(); + + mockNotesPostError(); + + $noteEl.find('.js-comment-save-button').click(); + }) + .then(timeoutPromise) + .then(() => { + const $updatedNoteEl = $notesContainer.find(`#note_${note.id}`); + expect($updatedNoteEl.hasClass('.being-posted')).toEqual(false); // Remove being-posted visuals + expect($updatedNoteEl.find('.note-text').text().trim()).toEqual(sampleComment); // See if comment reverted back to original + expect($('.flash-container').is(':visible')).toEqual(true); // Flash error message shown + + done(); + }) + .catch(done.fail); }); }); @@ -563,8 +611,12 @@ import Notes from '~/notes'; }; let $form; let $notesContainer; + let mock; beforeEach(() => { + mock = new MockAdapter(axios); + mock.onPost(/(.*)\/notes$/).reply(200, note); + this.notes = new Notes('', []); window.gon.current_username = 'root'; window.gon.current_user_fullname = 'Administrator'; @@ -582,15 +634,20 @@ import Notes from '~/notes'; $form.find('textarea.js-note-text').val(sampleComment); }); - it('should remove slash command placeholder when comment with slash commands is done posting', () => { - const deferred = $.Deferred(); - spyOn($, 'ajax').and.returnValue(deferred.promise()); + afterEach(() => { + mock.restore(); + }); + + it('should remove slash command placeholder when comment with slash commands is done posting', (done) => { spyOn(gl.awardsHandler, 'addAwardToEmojiBar').and.callThrough(); $('.js-comment-button').click(); expect($notesContainer.find('.system-note.being-posted').length).toEqual(1); // Placeholder shown - deferred.resolve(note); - expect($notesContainer.find('.system-note.being-posted').length).toEqual(0); // Placeholder removed + + setTimeout(() => { + expect($notesContainer.find('.system-note.being-posted').length).toEqual(0); // Placeholder removed + done(); + }); }); }); @@ -607,8 +664,12 @@ import Notes from '~/notes'; }; let $form; let $notesContainer; + let mock; beforeEach(() => { + mock = new MockAdapter(axios); + mock.onPost(/(.*)\/notes$/).reply(200, note); + this.notes = new Notes('', []); window.gon.current_username = 'root'; window.gon.current_user_fullname = 'Administrator'; @@ -617,19 +678,24 @@ import Notes from '~/notes'; $form.find('textarea.js-note-text').html(sampleComment); }); - it('should not render a script tag', () => { - const deferred = $.Deferred(); - spyOn($, 'ajax').and.returnValue(deferred.promise()); + afterEach(() => { + mock.restore(); + }); + + it('should not render a script tag', (done) => { $('.js-comment-button').click(); - deferred.resolve(note); - const $noteEl = $notesContainer.find(`#note_${note.id}`); - $noteEl.find('.js-note-edit').click(); - $noteEl.find('textarea.js-note-text').html(updatedComment); - $noteEl.find('.js-comment-save-button').click(); + setTimeout(() => { + const $noteEl = $notesContainer.find(`#note_${note.id}`); + $noteEl.find('.js-note-edit').click(); + $noteEl.find('textarea.js-note-text').html(updatedComment); + $noteEl.find('.js-comment-save-button').click(); + + const $updatedNoteEl = $notesContainer.find(`#note_${note.id}`).find('.js-task-list-container'); + expect($updatedNoteEl.find('.note-text').text().trim()).toEqual(''); - const $updatedNoteEl = $notesContainer.find(`#note_${note.id}`).find('.js-task-list-container'); - expect($updatedNoteEl.find('.note-text').text().trim()).toEqual(''); + done(); + }); }); }); -- cgit v1.2.1 From 4377b4795fedcb5145d76fab09ccd468ce3a2138 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 1 Feb 2018 09:14:53 +0000 Subject: remove useless ajaxPost method --- app/assets/javascripts/lib/utils/common_utils.js | 3 --- app/assets/javascripts/notes.js | 11 ++++++----- spec/javascripts/lib/utils/common_utils_spec.js | 13 ------------- 3 files changed, 6 insertions(+), 21 deletions(-) diff --git a/app/assets/javascripts/lib/utils/common_utils.js b/app/assets/javascripts/lib/utils/common_utils.js index c27c4c6e621..5811d059e0b 100644 --- a/app/assets/javascripts/lib/utils/common_utils.js +++ b/app/assets/javascripts/lib/utils/common_utils.js @@ -35,8 +35,6 @@ export const ajaxGet = url => axios.get(url, { $.globalEval(data); }); -export const ajaxPost = (url, data) => axios.post(url, data); - export const rstrip = (val) => { if (val) { return val.replace(/\s+$/, ''); @@ -409,7 +407,6 @@ window.gl.utils = { getGroupSlug, isInIssuePage, ajaxGet, - ajaxPost, rstrip, updateTooltipTitle, disableButtonIfEmptyField, diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index 2f37d40ebad..429d50f5463 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -17,13 +17,14 @@ import 'vendor/jquery.caret'; // required by jquery.atwho import 'vendor/jquery.atwho'; import AjaxCache from '~/lib/utils/ajax_cache'; import { getLocationHash } from './lib/utils/url_utility'; +import axios from './lib/utils/axios_utils'; import Flash from './flash'; import CommentTypeToggle from './comment_type_toggle'; import GLForm from './gl_form'; import loadAwardsHandler from './awards_handler'; import Autosave from './autosave'; import TaskList from './task_list'; -import { ajaxPost, isInViewport, getPagePath, scrollToElement, isMetaKey } from './lib/utils/common_utils'; +import { isInViewport, getPagePath, scrollToElement, isMetaKey } from './lib/utils/common_utils'; import imageDiffHelper from './image_diff/helpers/index'; import { localTimeAgo } from './lib/utils/datetime_utility'; @@ -1404,7 +1405,7 @@ export default class Notes { * 2) Identify comment type; a) Main thread b) Discussion thread c) Discussion resolve * 3) Build temporary placeholder element (using `createPlaceholderNote`) * 4) Show placeholder note on UI - * 5) Perform network request to submit the note using `ajaxPost` + * 5) Perform network request to submit the note using `axios.post` * a) If request is successfully completed * 1. Remove placeholder element * 2. Show submitted Note element @@ -1486,7 +1487,7 @@ export default class Notes { /* eslint-disable promise/catch-or-return */ // Make request to submit comment on server - ajaxPost(formAction, formData) + axios.post(formAction, formData) .then((res) => { const note = res.data; @@ -1601,7 +1602,7 @@ export default class Notes { * * 1) Get Form metadata * 2) Update note element with new content - * 3) Perform network request to submit the updated note using `ajaxPost` + * 3) Perform network request to submit the updated note using `axios.post` * a) If request is successfully completed * 1. Show submitted Note element * b) If request failed @@ -1632,7 +1633,7 @@ export default class Notes { /* eslint-disable promise/catch-or-return */ // Make request to update comment on server - ajaxPost(formAction, formData) + axios.post(formAction, formData) .then(({ data }) => { // Submission successful! render final note element this.updateNote(data, $editingNote); diff --git a/spec/javascripts/lib/utils/common_utils_spec.js b/spec/javascripts/lib/utils/common_utils_spec.js index 864ee4995ea..80430011aed 100644 --- a/spec/javascripts/lib/utils/common_utils_spec.js +++ b/spec/javascripts/lib/utils/common_utils_spec.js @@ -459,19 +459,6 @@ describe('common_utils', () => { }); }); - describe('ajaxPost', () => { - it('should perform `$.ajax` call and do `POST` request', () => { - const requestURL = '/some/random/api'; - const data = { keyname: 'value' }; - const ajaxSpy = spyOn(axios, 'post').and.callFake(() => {}); - - commonUtils.ajaxPost(requestURL, data); - - expect(ajaxSpy.calls.allArgs()[0][0]).toEqual(requestURL); - expect(ajaxSpy.calls.allArgs()[0][1]).toEqual(data); - }); - }); - describe('spriteIcon', () => { let beforeGon; -- cgit v1.2.1 From b1c8b3aadb202898a1e9342706e9e6f46bbb3612 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 1 Feb 2018 11:59:59 +0000 Subject: fixed axios mock not being restored --- spec/javascripts/notes_spec.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/spec/javascripts/notes_spec.js b/spec/javascripts/notes_spec.js index 808e0f6ca31..2fb385bd79f 100644 --- a/spec/javascripts/notes_spec.js +++ b/spec/javascripts/notes_spec.js @@ -145,6 +145,10 @@ import timeoutPromise from './helpers/set_timeout_promise_helper'; mock.onPost(/(.*)\/notes$/).reply(200, noteEntity); }); + afterEach(() => { + mock.restore(); + }); + it('updates note and resets edit form', (done) => { spyOn(this.notes, 'revertNoteEditForm'); spyOn(this.notes, 'setupNewNote'); -- cgit v1.2.1 From 645d635976ef48905c0416bd2a4a25025fef9ee4 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Fri, 2 Feb 2018 10:05:37 +0000 Subject: fixed issue with axios_utils not reducing activeVueResources variable when request fails --- app/assets/javascripts/job.js | 13 ++++++++++++- app/assets/javascripts/lib/utils/axios_utils.js | 4 ++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/app/assets/javascripts/job.js b/app/assets/javascripts/job.js index d2cbc9b82b3..d0b7ea75082 100644 --- a/app/assets/javascripts/job.js +++ b/app/assets/javascripts/job.js @@ -9,6 +9,7 @@ export default class Job { constructor(options) { this.timeout = null; this.state = null; + this.fetchingStatusFavicon = false; this.options = options || $('.js-build-options').data(); this.pagePath = this.options.pagePath; @@ -178,7 +179,17 @@ export default class Job { .then((res) => { const log = res.data; - setCiStatusFavicon(`${this.pagePath}/status.json`); + if (!this.fetchingStatusFavicon) { + this.fetchingStatusFavicon = true; + + setCiStatusFavicon(`${this.pagePath}/status.json`) + .then(() => { + this.fetchingStatusFavicon = false; + }) + .catch(() => { + this.fetchingStatusFavicon = false; + }); + } if (log.state) { this.state = log.state; diff --git a/app/assets/javascripts/lib/utils/axios_utils.js b/app/assets/javascripts/lib/utils/axios_utils.js index 585214049c7..792871e2ecf 100644 --- a/app/assets/javascripts/lib/utils/axios_utils.js +++ b/app/assets/javascripts/lib/utils/axios_utils.js @@ -19,6 +19,10 @@ axios.interceptors.response.use((config) => { window.activeVueResources -= 1; return config; +}, (e) => { + window.activeVueResources -= 1; + + return Promise.reject(e); }); export default axios; -- cgit v1.2.1 From 965ff9653dee8cc24c13502714e1dad9a2cdb3a2 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Fri, 2 Feb 2018 16:33:38 +0000 Subject: added missing calls in catch statements --- .../javascripts/merge_conflicts/merge_conflicts_bundle.js | 1 + app/assets/javascripts/merge_request_tabs.js | 12 +++++++++--- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/app/assets/javascripts/merge_conflicts/merge_conflicts_bundle.js b/app/assets/javascripts/merge_conflicts/merge_conflicts_bundle.js index bc805047f73..b4b3c15108d 100644 --- a/app/assets/javascripts/merge_conflicts/merge_conflicts_bundle.js +++ b/app/assets/javascripts/merge_conflicts/merge_conflicts_bundle.js @@ -53,6 +53,7 @@ $(() => { }); }) .catch(() => { + mergeConflictsStore.setLoadingState(false); mergeConflictsStore.setFailedRequest(); }); }, diff --git a/app/assets/javascripts/merge_request_tabs.js b/app/assets/javascripts/merge_request_tabs.js index 6151e90aa04..3e97a8c758d 100644 --- a/app/assets/javascripts/merge_request_tabs.js +++ b/app/assets/javascripts/merge_request_tabs.js @@ -2,7 +2,7 @@ import Cookies from 'js-cookie'; import axios from './lib/utils/axios_utils'; -import Flash from './flash'; +import flash from './flash'; import BlobForkSuggestion from './blob/blob_fork_suggestion'; import initChangesDropdown from './init_changes_dropdown'; import bp from './breakpoints'; @@ -257,7 +257,10 @@ export default class MergeRequestTabs { this.toggleLoading(false); }) - .catch(() => new Flash('An error occurred while fetching this tab.', 'alert')); + .catch(() => { + this.toggleLoading(false); + flash('An error occurred while fetching this tab.'); + }); } mountPipelinesView() { @@ -344,7 +347,10 @@ export default class MergeRequestTabs { this.toggleLoading(false); }) - .catch(() => Flash('An error occurred while fetching this tab.', 'alert')); + .catch(() => { + this.toggleLoading(false); + flash('An error occurred while fetching this tab.'); + }); } // Show or hide the loading spinner -- cgit v1.2.1