Automatic merge of gitlab-org/gitlab-ce master

app/assets/javascripts/notes/components/notes_app.vue

@@ -90,8 +90,15 @@ export default {
         this.fetchNotes();
       }
     },
+    allDiscussions() {
+      if (this.discussonsCount) {
+        this.discussonsCount.textContent = this.allDiscussions.length;
+      }
+    },
   },
   created() {
+    this.discussonsCount = document.querySelector('.js-discussions-count');
+
     this.setNotesData(this.notesData);
     this.setNoteableData(this.noteableData);
     this.setUserData(this.userData);

app/views/projects/issues/_discussion.html.haml

@@ -5,7 +5,7 @@
     = link_to 'Reopen issue', issue_path(@issue, issue: {state_event: :reopen}, format: 'json'), data: {original_text: "Reopen issue", alternative_text: "Comment & reopen issue"}, class: "btn btn-nr btn-reopen btn-comment js-note-target-reopen #{issue_button_visibility(@issue, false)}", title: 'Reopen issue'
     = link_to 'Close issue', issue_path(@issue, issue: {state_event: :close}, format: 'json'), data: {original_text: "Close issue", alternative_text: "Comment & close issue"}, class: "btn btn-nr btn-close btn-comment js-note-target-close #{issue_button_visibility(@issue, true)}", title: 'Close issue'
 
-%section.js-vue-notes-event
+%section.issuable-discussion.js-vue-notes-event
   #js-vue-notes{ data: { notes_data: notes_data(@issue).to_json,
     noteable_data: serialize_issuable(@issue),
     noteable_type: 'Issue',

app/views/projects/issues/show.html.haml

@@ -98,7 +98,6 @@
         #js-vue-discussion-filter{ data: { default_filter: current_user&.notes_filter_for(@issue), notes_filters: UserPreference.notes_filters.to_json } }
         = render 'new_branch' unless @issue.confidential?
 
-  %section.issuable-discussion
-    = render 'projects/issues/discussion'
+  = render_if_exists 'projects/issues/discussion'
 
 = render 'shared/issuable/sidebar', issuable_sidebar: @issuable_sidebar, assignees: @issue.assignees

spec/javascripts/lib/utils/ajax_cache_spec.js → spec/frontend/lib/utils/ajax_cache_spec.js

@@ -94,68 +94,54 @@ describe('AjaxCache', () => {
     beforeEach(() => {
       mock = new MockAdapter(axios);
 
-      spyOn(axios, 'get').and.callThrough();
+      jest.spyOn(axios, 'get');
     });
 
     afterEach(() => {
       mock.restore();
     });
 
-    it('stores and returns data from Ajax call if cache is empty', done => {
+    it('stores and returns data from Ajax call if cache is empty', () => {
       mock.onGet(dummyEndpoint).reply(200, dummyResponse);
 
-      AjaxCache.retrieve(dummyEndpoint)
-        .then(data => {
-          expect(data).toEqual(dummyResponse);
-          expect(AjaxCache.internalStorage[dummyEndpoint]).toEqual(dummyResponse);
-        })
-        .then(done)
-        .catch(fail);
+      return AjaxCache.retrieve(dummyEndpoint).then(data => {
+        expect(data).toEqual(dummyResponse);
+        expect(AjaxCache.internalStorage[dummyEndpoint]).toEqual(dummyResponse);
+      });
     });
 
-    it('makes no Ajax call if request is pending', done => {
+    it('makes no Ajax call if request is pending', () => {
       mock.onGet(dummyEndpoint).reply(200, dummyResponse);
 
-      AjaxCache.retrieve(dummyEndpoint)
-        .then(done)
-        .catch(fail);
-
-      AjaxCache.retrieve(dummyEndpoint)
-        .then(done)
-        .catch(fail);
-
-      expect(axios.get.calls.count()).toBe(1);
+      return Promise.all([
+        AjaxCache.retrieve(dummyEndpoint),
+        AjaxCache.retrieve(dummyEndpoint),
+      ]).then(() => {
+        expect(axios.get).toHaveBeenCalledTimes(1);
+      });
     });
 
-    it('returns undefined if Ajax call fails and cache is empty', done => {
+    it('returns undefined if Ajax call fails and cache is empty', () => {
       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}: ${errorMessage}`);
-          expect(error.textStatus).toBe(errorMessage);
-          done();
-        })
-        .catch(fail);
+      expect.assertions(2);
+      return AjaxCache.retrieve(dummyEndpoint).catch(error => {
+        expect(error.message).toBe(`${dummyEndpoint}: ${errorMessage}`);
+        expect(error.textStatus).toBe(errorMessage);
+      });
     });
 
-    it('makes no Ajax call if matching data exists', done => {
+    it('makes no Ajax call if matching data exists', () => {
       AjaxCache.internalStorage[dummyEndpoint] = dummyResponse;
-      mock.onGet(dummyEndpoint).reply(() => {
-        fail(new Error('expected no Ajax call!'));
-      });
 
-      AjaxCache.retrieve(dummyEndpoint)
-        .then(data => {
-          expect(data).toBe(dummyResponse);
-        })
-        .then(done)
-        .catch(fail);
+      return AjaxCache.retrieve(dummyEndpoint).then(data => {
+        expect(data).toBe(dummyResponse);
+        expect(axios.get).not.toHaveBeenCalled();
+      });
     });
 
-    it('makes Ajax call even if matching data exists when forceRequest parameter is provided', done => {
+    it('makes Ajax call even if matching data exists when forceRequest parameter is provided', () => {
       const oldDummyResponse = {
         important: 'old dummy data',
       };
@@ -164,21 +150,12 @@ describe('AjaxCache', () => {
 
       mock.onGet(dummyEndpoint).reply(200, dummyResponse);
 
-      // Call without forceRetrieve param
-      AjaxCache.retrieve(dummyEndpoint)
-        .then(data => {
-          expect(data).toBe(oldDummyResponse);
-        })
-        .then(done)
-        .catch(fail);
-
-      // Call with forceRetrieve param
-      AjaxCache.retrieve(dummyEndpoint, true)
-        .then(data => {
-          expect(data).toEqual(dummyResponse);
-        })
-        .then(done)
-        .catch(fail);
+      return Promise.all([
+        AjaxCache.retrieve(dummyEndpoint),
+        AjaxCache.retrieve(dummyEndpoint, true),
+      ]).then(data => {
+        expect(data).toEqual([oldDummyResponse, dummyResponse]);
+      });
     });
   });
 });

spec/frontend/test_setup.js

+import axios from '~/lib/utils/axios_utils';
+
 const testTimeoutInMs = 300;
 jest.setTimeout(testTimeoutInMs);
 
@@ -14,3 +16,15 @@ afterEach(() => {
     throw new Error(`Test took too long (${elapsedTimeInMs}ms > ${testTimeoutInMs}ms)!`);
   }
 });
+
+// fail tests for unmocked requests
+beforeEach(done => {
+  axios.defaults.adapter = config => {
+    const error = new Error(`Unexpected unmocked request: ${JSON.stringify(config, null, 2)}`);
+    error.config = config;
+    done.fail(error);
+    return Promise.reject(error);
+  };
+
+  done();
+});

spec/javascripts/notes/components/note_app_spec.js

@@ -83,6 +83,8 @@ describe('note_app', () => {
 
   describe('render', () => {
     beforeEach(() => {
+      setFixtures('<div class="js-discussions-count"></div>');
+
       Vue.http.interceptors.push(mockData.individualNoteInterceptor);
       wrapper = mountComponent();
     });
@@ -127,6 +129,14 @@ describe('note_app', () => {
     it('should render form comment button as disabled', () => {
       expect(wrapper.find('.js-note-new-discussion').attributes('disabled')).toEqual('disabled');
     });
+
+    it('updates discussions badge', done => {
+      setTimeout(() => {
+        expect(document.querySelector('.js-discussions-count').textContent).toEqual('2');
+
+        done();
+      });
+    });
   });
 
   describe('while fetching data', () => {