diff options
author | Fatih Acet <acetfatih@gmail.com> | 2019-01-29 14:50:22 +0100 |
---|---|---|
committer | Fatih Acet <acetfatih@gmail.com> | 2019-01-30 23:18:20 +0100 |
commit | a3a847f8624b5f5b10d5665725df2090a1f631ba (patch) | |
tree | 834d9ee06381ba88f29531c52566268743360315 | |
parent | c352e7e1621dfb3de97518eb46ca513503995ccf (diff) | |
download | gitlab-ce-a3a847f8624b5f5b10d5665725df2090a1f631ba.tar.gz |
Address review comments and fix commented spec
-rw-r--r-- | app/assets/javascripts/issue_show/components/app.vue | 24 | ||||
-rw-r--r-- | app/assets/javascripts/issue_show/stores/index.js | 9 | ||||
-rw-r--r-- | app/assets/javascripts/task_list.js | 9 | ||||
-rw-r--r-- | spec/javascripts/issue_show/components/app_spec.js | 39 | ||||
-rw-r--r-- | spec/javascripts/task_list_spec.js | 8 |
5 files changed, 47 insertions, 42 deletions
diff --git a/app/assets/javascripts/issue_show/components/app.vue b/app/assets/javascripts/issue_show/components/app.vue index 5dbe206c6f9..8b3e8719247 100644 --- a/app/assets/javascripts/issue_show/components/app.vue +++ b/app/assets/javascripts/issue_show/components/app.vue @@ -1,6 +1,7 @@ <script> import Visibility from 'visibilityjs'; -import { s__, sprintf } from '~/locale'; +import { __, s__, sprintf } from '~/locale'; +import createFlash from '~/flash'; import { visitUrl } from '../../lib/utils/url_utility'; import Poll from '../../lib/utils/poll'; import eventHub from '../event_hub'; @@ -11,7 +12,6 @@ import descriptionComponent from './description.vue'; import editedComponent from './edited.vue'; import formComponent from './form.vue'; import recaptchaModalImplementor from '../../vue_shared/mixins/recaptcha_modal_implementor'; -import { __ } from '~/locale'; export default { components: { @@ -168,7 +168,7 @@ export default { return descriptionChanged || titleChanged; }, defaultErrorMessage() { - return sprintf(s__('Error updating %{issuableType}.'), { issuableType: this.issuableType }); + return sprintf(s__('Error updating %{issuableType}'), { issuableType: this.issuableType }); }, }, created() { @@ -224,7 +224,7 @@ export default { this.store.updateState(data); }) .catch(() => { - window.Flash(this.defaultErrorMessage); + createFlash(this.defaultErrorMessage); }); }, @@ -258,18 +258,20 @@ export default { .then(() => { eventHub.$emit('close.form'); }) - .catch(error => { - if (error && error.name === 'SpamError') { + .catch((error = {}) => { + const { name, response = {} } = error; + + if (name === 'SpamError') { this.openRecaptcha(); } else { let errMsg = this.defaultErrorMessage; - if (error && error.response && error.response.data && error.response.data.errors) { - errMsg += error.response.data.errors.join(' '); + if (response.data && response.data.errors) { + errMsg += `. ${response.data.errors.join(' ')}`; } eventHub.$emit('close.form'); - window.Flash(errMsg); + createFlash(errMsg); } }); }, @@ -294,7 +296,9 @@ export default { }) .catch(() => { eventHub.$emit('close.form'); - window.Flash(`Error deleting ${this.issuableType}`); + createFlash( + sprintf(s__('Error deleting %{issuableType}'), { issuableType: this.issuableType }), + ); }); }, }, diff --git a/app/assets/javascripts/issue_show/stores/index.js b/app/assets/javascripts/issue_show/stores/index.js index 2b3903def6b..3c17e73ccec 100644 --- a/app/assets/javascripts/issue_show/stores/index.js +++ b/app/assets/javascripts/issue_show/stores/index.js @@ -1,3 +1,5 @@ +import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils'; + export default class Store { constructor(initialState) { this.state = initialState; @@ -15,14 +17,9 @@ export default class Store { this.formState.lockedWarningVisible = true; } + Object.assign(this.state, convertObjectPropsToCamelCase(data)); this.state.titleHtml = data.title; - this.state.titleText = data.title_text; this.state.descriptionHtml = data.description; - this.state.descriptionText = data.description_text; - this.state.taskStatus = data.task_status; - this.state.updatedAt = data.updated_at; - this.state.updatedByName = data.updated_by_name; - this.state.updatedByPath = data.updated_by_path; this.state.lock_version = data.lock_version; } diff --git a/app/assets/javascripts/task_list.js b/app/assets/javascripts/task_list.js index 161c44fa156..5172a1ef3d6 100644 --- a/app/assets/javascripts/task_list.js +++ b/app/assets/javascripts/task_list.js @@ -1,5 +1,6 @@ import $ from 'jquery'; import 'deckar01-task_list'; +import { __ } from '~/locale'; import axios from './lib/utils/axios_utils'; import Flash from './flash'; @@ -21,7 +22,7 @@ export default class TaskList { errorMessages = e.response.data.errors.join(' '); } - return new Flash(errorMessages || 'Update failed', 'alert'); + return new Flash(errorMessages || __('Update failed'), 'alert'); }; this.init(); @@ -34,10 +35,8 @@ export default class TaskList { $(document).on('tasklist:changed', this.taskListContainerSelector, this.updateHandler); } - getTaskListTarget(e = {}) { - const $currentTarget = $(e.currentTarget); - - return $currentTarget.taskList ? $currentTarget : $(this.taskListContainerSelector); + getTaskListTarget(e) { + return e && e.currentTarget ? $(e.currentTarget) : $(this.taskListContainerSelector); } disableTaskListItems(e) { diff --git a/spec/javascripts/issue_show/components/app_spec.js b/spec/javascripts/issue_show/components/app_spec.js index 028a36d3496..46f0d3b16b9 100644 --- a/spec/javascripts/issue_show/components/app_spec.js +++ b/spec/javascripts/issue_show/components/app_spec.js @@ -18,9 +18,13 @@ describe('Issuable output', () => { let realtimeRequestCount = 0; let vm; - document.body.innerHTML = '<span id="task_status"></span>'; - beforeEach(done => { + setFixtures(` + <div> + <div class="flash-container"></div> + <span id="task_status"></span> + </div> + `); spyOn(eventHub, '$emit'); const IssuableDescriptionComponent = Vue.extend(issuableApp); @@ -258,18 +262,15 @@ describe('Issuable output', () => { }); describe('error when updating', () => { - beforeEach(() => { - spyOn(window, 'Flash').and.callThrough(); - }); - it('closes form on error', done => { spyOn(vm.service, 'updateIssuable').and.callFake(() => Promise.resolve()); vm.updateIssuable(); setTimeout(() => { expect(eventHub.$emit).toHaveBeenCalledWith('close.form'); - - expect(window.Flash).toHaveBeenCalledWith('Error updating issue'); + expect(document.querySelector('.flash-container .flash-text').innerText.trim()).toBe( + `Error updating issue`, + ); done(); }); @@ -284,8 +285,9 @@ describe('Issuable output', () => { setTimeout(() => { expect(eventHub.$emit).toHaveBeenCalledWith('close.form'); - - expect(window.Flash).toHaveBeenCalledWith('Error updating merge request'); + expect(document.querySelector('.flash-container .flash-text').innerText.trim()).toBe( + `Error updating merge request`, + ); done(); }); @@ -295,12 +297,14 @@ describe('Issuable output', () => { it('shows error mesage from backend if exists', done => { const msg = 'Custom error message from backend'; spyOn(vm.service, 'updateIssuable').and.callFake( - () => Promise.reject({ response: { data: { errors: msg } } }), // eslint-disable-line prefer-promise-reject-errors + () => Promise.reject({ response: { data: { errors: [msg] } } }), // eslint-disable-line prefer-promise-reject-errors ); vm.updateIssuable(); setTimeout(() => { - expect(window.Flash).toHaveBeenCalledWith(msg); + expect(document.querySelector('.flash-container .flash-text').innerText.trim()).toBe( + `${vm.defaultErrorMessage}. ${msg}`, + ); done(); }); @@ -399,7 +403,6 @@ describe('Issuable output', () => { }); it('closes form on error', done => { - spyOn(window, 'Flash').and.callThrough(); spyOn(vm.service, 'deleteIssuable').and.callFake( () => new Promise((resolve, reject) => { @@ -411,8 +414,9 @@ describe('Issuable output', () => { setTimeout(() => { expect(eventHub.$emit).toHaveBeenCalledWith('close.form'); - - expect(window.Flash).toHaveBeenCalledWith('Error deleting issue'); + expect(document.querySelector('.flash-container .flash-text').innerText.trim()).toBe( + 'Error deleting issue', + ); done(); }); @@ -472,12 +476,13 @@ describe('Issuable output', () => { it('should show error message if store update fails', done => { spyOn(vm.service, 'getData').and.returnValue(Promise.reject()); - spyOn(window, 'Flash'); vm.issuableType = 'merge request'; vm.updateStoreState() .then(() => { - expect(window.Flash).toHaveBeenCalledWith(`Error updating ${vm.issuableType}`); + expect(document.querySelector('.flash-container .flash-text').innerText.trim()).toBe( + `Error updating ${vm.issuableType}`, + ); }) .then(done) .catch(done.fail); diff --git a/spec/javascripts/task_list_spec.js b/spec/javascripts/task_list_spec.js index 00f8b2d6d77..563f402de58 100644 --- a/spec/javascripts/task_list_spec.js +++ b/spec/javascripts/task_list_spec.js @@ -50,11 +50,11 @@ describe('TaskList', () => { expect($target).toEqual(currentTarget); }); - // it('should return element of the taskListContainerSelector', () => { - // const $target = taskList.getTaskListTarget(); + it('should return element of the taskListContainerSelector', () => { + const $target = taskList.getTaskListTarget(); - // expect($target).toEqual($(taskList.taskListContainerSelector)); - // }); + expect($target).toEqual($(taskList.taskListContainerSelector)); + }); }); describe('disableTaskListItems', () => { |