Address review comments and fix commented spec

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', () => {