Add latest changes from gitlab-org/gitlab@master

10 files changed, 47 insertions, 79 deletions

diff --git a/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb b/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb
index eb86b1e33af..5a7edd37182 100644
--- a/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb
+++ b/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb
@@ -368,16 +368,6 @@ describe 'Merge request > User resolves diff notes and threads', :js do
         end
       end
 
-      it 'shows jump to next discussion button on all discussions' do
-        wait_for_requests
-
-        all_discussion_replies = page.all('.discussion-reply-holder')
-
-        expect(all_discussion_replies.count).to eq(2)
-        expect(all_discussion_replies.first.all('.discussion-next-btn').count).to eq(1)
-        expect(all_discussion_replies.last.all('.discussion-next-btn').count).to eq(1)
-      end
-
       it 'displays next thread even if hidden' do
         page.all('.note-discussion', count: 2).each do |discussion|
           page.within discussion do
diff --git a/spec/frontend/notes/components/__snapshots__/discussion_jump_to_next_button_spec.js.snap b/spec/frontend/notes/components/__snapshots__/discussion_jump_to_next_button_spec.js.snap
index 1e466f266ed..2f4c114dd3d 100644
--- a/spec/frontend/notes/components/__snapshots__/discussion_jump_to_next_button_spec.js.snap
+++ b/spec/frontend/notes/components/__snapshots__/discussion_jump_to_next_button_spec.js.snap
@@ -7,7 +7,7 @@ exports[`JumpToNextDiscussionButton matches the snapshot 1`] = `
 >
   <button
     class="btn btn-default discussion-next-btn"
-    title="Jump to next unresolved discussion"
+    title="Jump to next unresolved thread"
   >
     <icon-stub
       name="comment-next"
diff --git a/spec/frontend/notes/components/discussion_actions_spec.js b/spec/frontend/notes/components/discussion_actions_spec.js
index 2d95a86d8a6..5101b81e3ee 100644
--- a/spec/frontend/notes/components/discussion_actions_spec.js
+++ b/spec/frontend/notes/components/discussion_actions_spec.js
@@ -120,14 +120,5 @@ describe('DiscussionActions', () => {
         .trigger('click');
       expect(wrapper.vm.$emit).toHaveBeenCalledWith('resolve');
     });
-
-    it('emits jumpToNextDiscussion event when clicking on jump to next discussion button', () => {
-      jest.spyOn(wrapper.vm, '$emit');
-      wrapper
-        .find(JumpToNextDiscussionButton)
-        .find('button')
-        .trigger('click');
-      expect(wrapper.vm.$emit).toHaveBeenCalledWith('jumpToNextDiscussion');
-    });
   });
 });
diff --git a/spec/frontend/notes/components/discussion_jump_to_next_button_spec.js b/spec/frontend/notes/components/discussion_jump_to_next_button_spec.js
index 58cdf3cb57e..a00dd445c4f 100644
--- a/spec/frontend/notes/components/discussion_jump_to_next_button_spec.js
+++ b/spec/frontend/notes/components/discussion_jump_to_next_button_spec.js
@@ -15,15 +15,4 @@ describe('JumpToNextDiscussionButton', () => {
   it('matches the snapshot', () => {
     expect(wrapper.vm.$el).toMatchSnapshot();
   });
-
-  it('emits onClick event on button click', () => {
-    const button = wrapper.find({ ref: 'button' });
-
-    button.trigger('click');
-
-    return wrapper.vm.$nextTick().then(() => {
-      expect(wrapper.emitted().onClick).toBeTruthy();
-      expect(wrapper.emitted().onClick.length).toBe(1);
-    });
-  });
 });
diff --git a/spec/frontend/notes/components/discussion_keyboard_navigator_spec.js b/spec/frontend/notes/components/discussion_keyboard_navigator_spec.js
index b38cfa8fb4a..74e827784ec 100644
--- a/spec/frontend/notes/components/discussion_keyboard_navigator_spec.js
+++ b/spec/frontend/notes/components/discussion_keyboard_navigator_spec.js
@@ -53,13 +53,15 @@ describe('notes/components/discussion_keyboard_navigator', () => {
   });
 
   describe.each`
-    isDiffView | expectedNextId  | expectedPrevId
-    ${true}    | ${NEXT_DIFF_ID} | ${PREV_DIFF_ID}
-    ${false}   | ${NEXT_ID}      | ${PREV_ID}
-  `('when isDiffView is $isDiffView', ({ isDiffView, expectedNextId, expectedPrevId }) => {
+    currentAction | expectedNextId  | expectedPrevId
+    ${'diffs'}    | ${NEXT_DIFF_ID} | ${PREV_DIFF_ID}
+    ${'show'}     | ${NEXT_ID}      | ${PREV_ID}
+  `('when isDiffView is $isDiffView', ({ currentAction, expectedNextId, expectedPrevId }) => {
     beforeEach(() => {
-      createComponent({ propsData: { isDiffView } });
+      window.mrTabs = { currentAction };
+      createComponent();
     });
+    afterEach(() => delete window.mrTabs);
 
     it('calls jumpToNextDiscussion when pressing `n`', () => {
       Mousetrap.trigger('n');
diff --git a/spec/frontend/notes/stores/mutation_spec.js b/spec/frontend/notes/stores/mutation_spec.js
index 49debe348e2..46b4081f6f6 100644
--- a/spec/frontend/notes/stores/mutation_spec.js
+++ b/spec/frontend/notes/stores/mutation_spec.js
@@ -501,7 +501,6 @@ describe('Notes Store mutations', () => {
         expect.objectContaining({
           resolvableDiscussionsCount: 1,
           unresolvedDiscussionsCount: 1,
-          hasUnresolvedDiscussions: false,
         }),
       );
     });
@@ -538,7 +537,6 @@ describe('Notes Store mutations', () => {
         expect.objectContaining({
           resolvableDiscussionsCount: 4,
           unresolvedDiscussionsCount: 2,
-          hasUnresolvedDiscussions: true,
         }),
       );
     });
diff --git a/spec/helpers/avatars_helper_spec.rb b/spec/helpers/avatars_helper_spec.rb
index 8b6817efcc4..2a030742cb9 100644
--- a/spec/helpers/avatars_helper_spec.rb
+++ b/spec/helpers/avatars_helper_spec.rb
@@ -22,15 +22,41 @@ describe AvatarsHelper do
       end
     end
 
-    context 'when providing a project' do
-      it_behaves_like 'resource with a default avatar', 'project' do
-        let(:resource) { create(:project, name: 'foo') }
-        let(:helper_args) { [resource] }
+    shared_examples 'Gitaly exception handling' do
+      before do
+        allow(resource).to receive(:avatar_url).and_raise(error_class)
       end
 
+      it 'handles Gitaly exception gracefully' do
+        expect(Gitlab::ErrorTracking).to receive(:track_exception).with(
+          an_instance_of(error_class), source_type: 'Project', source_id: resource.id
+        )
+        expect { project_icon(resource) }.not_to raise_error
+      end
+
+      it_behaves_like 'resource with a default avatar', 'project'
+    end
+
+    context 'when providing a project' do
+      let(:helper_args) { [resource] }
+      let(:resource) { create(:project, name: 'foo') }
+
+      it_behaves_like 'resource with a default avatar', 'project'
+
       it_behaves_like 'resource with a custom avatar', 'project' do
         let(:resource) { create(:project, :public, avatar: File.open(uploaded_image_temp_path)) }
-        let(:helper_args) { [resource] }
+      end
+
+      context 'when Gitaly is unavailable' do
+        let(:error_class) { GRPC::Unavailable }
+
+        include_examples 'Gitaly exception handling'
+      end
+
+      context 'when Gitaly request is taking too long' do
+        let(:error_class) { GRPC::DeadlineExceeded }
+
+        include_examples 'Gitaly exception handling'
       end
     end
 
diff --git a/spec/javascripts/notes/components/discussion_counter_spec.js b/spec/javascripts/notes/components/discussion_counter_spec.js
index de1fd87517a..9c7aed43a3b 100644
--- a/spec/javascripts/notes/components/discussion_counter_spec.js
+++ b/spec/javascripts/notes/components/discussion_counter_spec.js
@@ -7,6 +7,7 @@ import { noteableDataMock, discussionMock, notesDataMock } from '../mock_data';
 describe('DiscussionCounter component', () => {
   let store;
   let vm;
+  const notes = { currentDiscussionId: null };
 
   beforeEach(() => {
     window.mrTabs = {};
@@ -25,7 +26,7 @@ describe('DiscussionCounter component', () => {
   });
 
   describe('methods', () => {
-    describe('jumpToFirstUnresolvedDiscussion', () => {
+    describe('jumpToNextDiscussion', () => {
       it('expands unresolved discussion', () => {
         window.mrTabs.currentAction = 'show';
 
@@ -48,13 +49,14 @@ describe('DiscussionCounter component', () => {
         store.replaceState({
           ...store.state,
           discussions,
+          notes,
         });
-        vm.jumpToFirstUnresolvedDiscussion();
+        vm.jumpToNextDiscussion();
 
         expect(vm.expandDiscussion).toHaveBeenCalledWith({ discussionId: firstDiscussionId });
       });
 
-      it('jumps to first unresolved discussion from diff tab if all diff discussions are resolved', () => {
+      it('jumps to next unresolved discussion from diff tab if all diff discussions are resolved', () => {
         window.mrTabs.currentAction = 'diff';
         spyOn(vm, 'switchToDiscussionsTabAndJumpTo').and.stub();
 
@@ -77,8 +79,9 @@ describe('DiscussionCounter component', () => {
         store.replaceState({
           ...store.state,
           discussions,
+          notes,
         });
-        vm.jumpToFirstUnresolvedDiscussion();
+        vm.jumpToNextDiscussion();
 
         expect(vm.switchToDiscussionsTabAndJumpTo).toHaveBeenCalledWith(unresolvedId);
       });
diff --git a/spec/javascripts/notes/components/noteable_discussion_spec.js b/spec/javascripts/notes/components/noteable_discussion_spec.js
index 6efc6485b9c..ee84fd2b091 100644
--- a/spec/javascripts/notes/components/noteable_discussion_spec.js
+++ b/spec/javascripts/notes/components/noteable_discussion_spec.js
@@ -101,37 +101,6 @@ describe('noteable_discussion component', () => {
     });
   });
 
-  describe('methods', () => {
-    describe('jumpToNextDiscussion', () => {
-      it('expands next unresolved thread', done => {
-        const discussion2 = getJSONFixture(discussionWithTwoUnresolvedNotes)[0];
-        discussion2.resolved = false;
-        discussion2.active = true;
-        discussion2.id = 'next'; // prepare this for being identified as next one (to be jumped to)
-        store.dispatch('setInitialNotes', [discussionMock, discussion2]);
-        window.mrTabs.currentAction = 'show';
-
-        wrapper.vm
-          .$nextTick()
-          .then(() => {
-            spyOn(wrapper.vm, 'expandDiscussion').and.stub();
-
-            const nextDiscussionId = discussion2.id;
-
-            setFixtures(`<div class="discussion" data-discussion-id="${nextDiscussionId}"></div>`);
-
-            wrapper.vm.jumpToNextDiscussion();
-
-            expect(wrapper.vm.expandDiscussion).toHaveBeenCalledWith({
-              discussionId: nextDiscussionId,
-            });
-          })
-          .then(done)
-          .catch(done.fail);
-      });
-    });
-  });
-
   describe('for resolved thread', () => {
     beforeEach(() => {
       const discussion = getJSONFixture(discussionWithTwoUnresolvedNotes)[0];
diff --git a/spec/support/shared_examples/models/mentionable_shared_examples.rb b/spec/support/shared_examples/models/mentionable_shared_examples.rb
index 0e8ee6f66f5..29c7e65fc21 100644
--- a/spec/support/shared_examples/models/mentionable_shared_examples.rb
+++ b/spec/support/shared_examples/models/mentionable_shared_examples.rb
@@ -263,7 +263,7 @@ RSpec.shared_examples 'mentions in notes' do |mentionable_type|
 
     it 'returns all mentionable mentions' do
       expect(mentionable.user_mentions.count).to eq 1
-      expect(mentionable.referenced_users).to eq [user, user2]
+      expect(mentionable.referenced_users).to match_array([user, user2])
       expect(mentionable.referenced_projects(user)).to eq [mentionable.project].compact # epic.project is nil, and we want empty []
       expect(mentionable.referenced_groups(user)).to eq [group]
     end