Add latest changes from gitlab-org/gitlab@master

8 files changed, 137 insertions, 67 deletions

diff --git a/spec/controllers/oauth/token_info_controller_spec.rb b/spec/controllers/oauth/token_info_controller_spec.rb
index 91a986db251..6d01a534673 100644
--- a/spec/controllers/oauth/token_info_controller_spec.rb
+++ b/spec/controllers/oauth/token_info_controller_spec.rb
@@ -5,10 +5,10 @@ require 'spec_helper'
 RSpec.describe Oauth::TokenInfoController do
   describe '#show' do
     context 'when the user is not authenticated' do
-      it 'responds with a 401' do
+      it 'responds with a 400' do
         get :show
 
-        expect(response).to have_gitlab_http_status(:unauthorized)
+        expect(response).to have_gitlab_http_status(:bad_request)
         expect(Gitlab::Json.parse(response.body)).to include('error' => 'invalid_request')
       end
     end
@@ -36,10 +36,10 @@ RSpec.describe Oauth::TokenInfoController do
     end
 
     context 'when the doorkeeper_token is not recognised' do
-      it 'responds with a 401' do
+      it 'responds with a 400' do
         get :show, params: { access_token: 'unknown_token' }
 
-        expect(response).to have_gitlab_http_status(:unauthorized)
+        expect(response).to have_gitlab_http_status(:bad_request)
         expect(Gitlab::Json.parse(response.body)).to include('error' => 'invalid_request')
       end
     end
@@ -49,10 +49,10 @@ RSpec.describe Oauth::TokenInfoController do
         create(:oauth_access_token, created_at: 2.days.ago, expires_in: 10.minutes)
       end
 
-      it 'responds with a 401' do
+      it 'responds with a 400' do
         get :show, params: { access_token: access_token.token }
 
-        expect(response).to have_gitlab_http_status(:unauthorized)
+        expect(response).to have_gitlab_http_status(:bad_request)
         expect(Gitlab::Json.parse(response.body)).to include('error' => 'invalid_request')
       end
     end
@@ -60,10 +60,10 @@ RSpec.describe Oauth::TokenInfoController do
     context 'when the token is revoked' do
       let(:access_token) { create(:oauth_access_token, revoked_at: 2.days.ago) }
 
-      it 'responds with a 401' do
+      it 'responds with a 400' do
         get :show, params: { access_token: access_token.token }
 
-        expect(response).to have_gitlab_http_status(:unauthorized)
+        expect(response).to have_gitlab_http_status(:bad_request)
         expect(Gitlab::Json.parse(response.body)).to include('error' => 'invalid_request')
       end
     end
diff --git a/spec/frontend/design_management/components/design_notes/design_note_spec.js b/spec/frontend/design_management/components/design_notes/design_note_spec.js
index 8eeaa6cd5ab..a59a2223cf3 100644
--- a/spec/frontend/design_management/components/design_notes/design_note_spec.js
+++ b/spec/frontend/design_management/components/design_notes/design_note_spec.js
@@ -91,7 +91,9 @@ describe('Design note component', () => {
       note,
     });
 
-    expect(scrollIntoViewMock).toHaveBeenCalled();
+    return wrapper.vm.$nextTick().then(() => {
+      expect(scrollIntoViewMock).toHaveBeenCalled();
+    });
   });
 
   it('should not render edit icon when user does not have a permission', () => {
diff --git a/spec/frontend/design_management/pages/design/__snapshots__/index_spec.js.snap b/spec/frontend/design_management/pages/design/__snapshots__/index_spec.js.snap
index 5fdc41bbfbf..3c5de543eb3 100644
--- a/spec/frontend/design_management/pages/design/__snapshots__/index_spec.js.snap
+++ b/spec/frontend/design_management/pages/design/__snapshots__/index_spec.js.snap
@@ -57,7 +57,7 @@ exports[`Design management design index page renders design index 1`] = `
      
     <design-discussion-stub
       data-testid="unresolved-discussion"
-      designid="test"
+      designid="design-id"
       discussion="[object Object]"
       discussionwithopenform=""
       markdownpreviewpath="/project-path/preview_markdown?target_type=Issue"
@@ -105,7 +105,7 @@ exports[`Design management design index page renders design index 1`] = `
     >
       <design-discussion-stub
         data-testid="resolved-discussion"
-        designid="test"
+        designid="design-id"
         discussion="[object Object]"
         discussionwithopenform=""
         markdownpreviewpath="/project-path/preview_markdown?target_type=Issue"
diff --git a/spec/frontend/design_management/pages/design/index_spec.js b/spec/frontend/design_management/pages/design/index_spec.js
index 369c8667f4d..d189bdb4345 100644
--- a/spec/frontend/design_management/pages/design/index_spec.js
+++ b/spec/frontend/design_management/pages/design/index_spec.js
@@ -7,18 +7,19 @@ import DesignIndex from '~/design_management/pages/design/index.vue';
 import DesignSidebar from '~/design_management/components/design_sidebar.vue';
 import DesignPresentation from '~/design_management/components/design_presentation.vue';
 import createImageDiffNoteMutation from '~/design_management/graphql/mutations/create_image_diff_note.mutation.graphql';
-import design from '../../mock_data/design';
-import mockResponseWithDesigns from '../../mock_data/designs';
-import mockResponseNoDesigns from '../../mock_data/no_designs';
-import mockAllVersions from '../../mock_data/all_versions';
+import updateActiveDiscussion from '~/design_management/graphql/mutations/update_active_discussion.mutation.graphql';
 import {
   DESIGN_NOT_FOUND_ERROR,
   DESIGN_VERSION_NOT_EXIST_ERROR,
 } from '~/design_management/utils/error_messages';
-import { DESIGNS_ROUTE_NAME } from '~/design_management/router/constants';
+import { DESIGNS_ROUTE_NAME, DESIGN_ROUTE_NAME } from '~/design_management/router/constants';
 import createRouter from '~/design_management/router';
 import * as utils from '~/design_management/utils/design_management_utils';
 import { DESIGN_DETAIL_LAYOUT_CLASSLIST } from '~/design_management/constants';
+import design from '../../mock_data/design';
+import mockResponseWithDesigns from '../../mock_data/designs';
+import mockResponseNoDesigns from '../../mock_data/no_designs';
+import mockAllVersions from '../../mock_data/all_versions';
 
 jest.mock('~/flash');
 jest.mock('mousetrap', () => ({
@@ -34,6 +35,12 @@ const DesignReplyForm = {
     focusInput,
   },
 };
+const mockDesignNoDiscussions = {
+  ...design,
+  discussions: {
+    nodes: [],
+  },
+};
 
 const localVue = createLocalVue();
 localVue.use(VueRouter);
@@ -75,7 +82,7 @@ describe('Design management design index page', () => {
   const findSidebar = () => wrapper.find(DesignSidebar);
   const findDesignPresentation = () => wrapper.find(DesignPresentation);
 
-  function createComponent(loading = false, data = {}) {
+  function createComponent({ loading = false } = {}, { data = {}, intialRouteOptions = {} } = {}) {
     const $apollo = {
       queries: {
         design: {
@@ -87,6 +94,8 @@ describe('Design management design index page', () => {
 
     router = createRouter();
 
+    router.push({ name: DESIGN_ROUTE_NAME, params: { id: design.id }, ...intialRouteOptions });
+
     wrapper = shallowMount(DesignIndex, {
       propsData: { id: '1' },
       mocks: { $apollo },
@@ -126,29 +135,28 @@ describe('Design management design index page', () => {
         },
       };
       jest.spyOn(utils, 'getPageLayoutElement').mockReturnValue(mockEl);
-      createComponent(true);
+      createComponent({ loading: true });
 
-      wrapper.vm.$router.push('/designs/test');
       expect(mockEl.classList.add).toHaveBeenCalledTimes(1);
       expect(mockEl.classList.add).toHaveBeenCalledWith(...DESIGN_DETAIL_LAYOUT_CLASSLIST);
     });
   });
 
   it('sets loading state', () => {
-    createComponent(true);
+    createComponent({ loading: true });
 
     expect(wrapper.element).toMatchSnapshot();
   });
 
   it('renders design index', () => {
-    createComponent(false, { design });
+    createComponent({ loading: false }, { data: { design } });
 
     expect(wrapper.element).toMatchSnapshot();
     expect(wrapper.find(GlAlert).exists()).toBe(false);
   });
 
   it('passes correct props to sidebar component', () => {
-    createComponent(false, { design });
+    createComponent({ loading: false }, { data: { design } });
 
     expect(findSidebar().props()).toEqual({
       design,
@@ -158,14 +166,14 @@ describe('Design management design index page', () => {
   });
 
   it('opens a new discussion form', () => {
-    createComponent(false, {
-      design: {
-        ...design,
-        discussions: {
-          nodes: [],
+    createComponent(
+      { loading: false },
+      {
+        data: {
+          design,
         },
       },
-    });
+    );
 
     findDesignPresentation().vm.$emit('openCommentForm', { x: 0, y: 0 });
 
@@ -175,15 +183,15 @@ describe('Design management design index page', () => {
   });
 
   it('keeps new discussion form focused', () => {
-    createComponent(false, {
-      design: {
-        ...design,
-        discussions: {
-          nodes: [],
+    createComponent(
+      { loading: false },
+      {
+        data: {
+          design,
+          annotationCoordinates,
         },
       },
-      annotationCoordinates,
-    });
+    );
 
     findDesignPresentation().vm.$emit('openCommentForm', { x: 10, y: 10 });
 
@@ -191,16 +199,16 @@ describe('Design management design index page', () => {
   });
 
   it('sends a mutation on submitting form and closes form', () => {
-    createComponent(false, {
-      design: {
-        ...design,
-        discussions: {
-          nodes: [],
+    createComponent(
+      { loading: false },
+      {
+        data: {
+          design,
+          annotationCoordinates,
+          comment: newComment,
         },
       },
-      annotationCoordinates,
-      comment: newComment,
-    });
+    );
 
     findDiscussionForm().vm.$emit('submitForm');
     expect(mutate).toHaveBeenCalledWith(createDiscussionMutationVariables);
@@ -216,16 +224,16 @@ describe('Design management design index page', () => {
   });
 
   it('closes the form and clears the comment on canceling form', () => {
-    createComponent(false, {
-      design: {
-        ...design,
-        discussions: {
-          nodes: [],
+    createComponent(
+      { loading: false },
+      {
+        data: {
+          design,
+          annotationCoordinates,
+          comment: newComment,
         },
       },
-      annotationCoordinates,
-      comment: newComment,
-    });
+    );
 
     findDiscussionForm().vm.$emit('cancelForm');
 
@@ -238,15 +246,15 @@ describe('Design management design index page', () => {
 
   describe('with error', () => {
     beforeEach(() => {
-      createComponent(false, {
-        design: {
-          ...design,
-          discussions: {
-            nodes: [],
+      createComponent(
+        { loading: false },
+        {
+          data: {
+            design: mockDesignNoDiscussions,
+            errorMessage: 'woops',
           },
         },
-        errorMessage: 'woops',
-      });
+      );
     });
 
     it('GlAlert is rendered in correct position with correct content', () => {
@@ -257,7 +265,7 @@ describe('Design management design index page', () => {
   describe('onDesignQueryResult', () => {
     describe('with no designs', () => {
       it('redirects to /designs', () => {
-        createComponent(true);
+        createComponent({ loading: true });
         router.push = jest.fn();
 
         wrapper.vm.onDesignQueryResult({ data: mockResponseNoDesigns, loading: false });
@@ -272,7 +280,7 @@ describe('Design management design index page', () => {
 
     describe('when no design exists for given version', () => {
       it('redirects to /designs', () => {
-        createComponent(true);
+        createComponent({ loading: true });
         wrapper.setData({
           allVersions: mockAllVersions,
         });
@@ -291,4 +299,24 @@ describe('Design management design index page', () => {
       });
     });
   });
+
+  describe('when hash present in current route', () => {
+    it('calls updateActiveDiscussion mutation', () => {
+      createComponent(
+        { loading: false },
+        {
+          data: {
+            design,
+          },
+          intialRouteOptions: { hash: '#note_123' },
+        },
+      );
+
+      expect(mutate).toHaveBeenCalledTimes(1);
+      expect(mutate).toHaveBeenCalledWith({
+        mutation: updateActiveDiscussion,
+        variables: { id: 'gid://gitlab/DiffNote/123', source: 'url' },
+      });
+    });
+  });
 });
diff --git a/spec/frontend/design_management/pages/index_spec.js b/spec/frontend/design_management/pages/index_spec.js
index 4728648b49c..2da02732b1e 100644
--- a/spec/frontend/design_management/pages/index_spec.js
+++ b/spec/frontend/design_management/pages/index_spec.js
@@ -579,7 +579,9 @@ describe('Design management index page', () => {
       });
       createComponent(true);
 
-      expect(scrollIntoViewMock).toHaveBeenCalled();
+      return wrapper.vm.$nextTick().then(() => {
+        expect(scrollIntoViewMock).toHaveBeenCalled();
+      });
     });
   });
 });
diff --git a/spec/frontend/design_management/utils/design_management_utils_spec.js b/spec/frontend/design_management/utils/design_management_utils_spec.js
index e6d836b9157..7e857d08d25 100644
--- a/spec/frontend/design_management/utils/design_management_utils_spec.js
+++ b/spec/frontend/design_management/utils/design_management_utils_spec.js
@@ -6,6 +6,7 @@ import {
   updateImageDiffNoteOptimisticResponse,
   isValidDesignFile,
   extractDesign,
+  extractDesignNoteId,
 } from '~/design_management/utils/design_management_utils';
 import mockResponseNoDesigns from '../mock_data/no_designs';
 import mockResponseWithDesigns from '../mock_data/designs';
@@ -171,3 +172,19 @@ describe('extractDesign', () => {
     });
   });
 });
+
+describe('extractDesignNoteId', () => {
+  it.each`
+    hash            | expectedNoteId
+    ${'#note_0'}    | ${'0'}
+    ${'#note_1'}    | ${'1'}
+    ${'#note_23'}   | ${'23'}
+    ${'#note_456'}  | ${'456'}
+    ${'note_1'}     | ${null}
+    ${'#note_'}     | ${null}
+    ${'#note_asd'}  | ${null}
+    ${'#note_1asd'} | ${null}
+  `('returns $expectedNoteId when hash is $hash', ({ hash, expectedNoteId }) => {
+    expect(extractDesignNoteId(hash)).toBe(expectedNoteId);
+  });
+});
diff --git a/spec/requests/api/commits_spec.rb b/spec/requests/api/commits_spec.rb
index 21ff0a94db9..d34244771ad 100644
--- a/spec/requests/api/commits_spec.rb
+++ b/spec/requests/api/commits_spec.rb
@@ -367,10 +367,31 @@ RSpec.describe API::Commits do
         end
       end
 
-      it 'does not increment the usage counters using access token authentication' do
-        expect(::Gitlab::UsageDataCounters::WebIdeCounter).not_to receive(:increment_commits_count)
+      context 'when using access token authentication' do
+        it 'does not increment the usage counters' do
+          expect(::Gitlab::UsageDataCounters::WebIdeCounter).not_to receive(:increment_commits_count)
+          expect(::Gitlab::UsageDataCounters::EditorUniqueCounter).not_to receive(:track_web_ide_edit_action)
 
-        post api(url, user), params: valid_c_params
+          post api(url, user), params: valid_c_params
+        end
+      end
+
+      context 'when using warden' do
+        it 'increments usage counters', :clean_gitlab_redis_shared_state do
+          session_id = Rack::Session::SessionId.new('6919a6f1bb119dd7396fadc38fd18d0d')
+          session_hash = { 'warden.user.user.key' => [[user.id], user.encrypted_password[0, 29]] }
+
+          Gitlab::Redis::SharedState.with do |redis|
+            redis.set("session:gitlab:#{session_id.private_id}", Marshal.dump(session_hash))
+          end
+
+          cookies[Gitlab::Application.config.session_options[:key]] = session_id.public_id
+
+          expect(::Gitlab::UsageDataCounters::WebIdeCounter).to receive(:increment_commits_count)
+          expect(::Gitlab::UsageDataCounters::EditorUniqueCounter).to receive(:track_web_ide_edit_action)
+
+          post api(url), params: valid_c_params
+        end
       end
 
       context 'a new file in project repo' do
diff --git a/spec/requests/api/oauth_tokens_spec.rb b/spec/requests/api/oauth_tokens_spec.rb
index f5971054b3c..23d5df873d4 100644
--- a/spec/requests/api/oauth_tokens_spec.rb
+++ b/spec/requests/api/oauth_tokens_spec.rb
@@ -20,7 +20,7 @@ RSpec.describe 'OAuth tokens' do
 
         request_oauth_token(user, client_basic_auth_header(client))
 
-        expect(response).to have_gitlab_http_status(:unauthorized)
+        expect(response).to have_gitlab_http_status(:bad_request)
         expect(json_response['error']).to eq('invalid_grant')
       end
     end
@@ -62,7 +62,7 @@ RSpec.describe 'OAuth tokens' do
 
             request_oauth_token(user, basic_auth_header(client.uid, 'invalid secret'))
 
-            expect(response).to have_gitlab_http_status(:unauthorized)
+            expect(response).to have_gitlab_http_status(:bad_request)
             expect(json_response['error']).to eq('invalid_client')
           end
         end
@@ -72,7 +72,7 @@ RSpec.describe 'OAuth tokens' do
     shared_examples 'does not create an access token' do
       let(:user) { create(:user) }
 
-      it { expect(response).to have_gitlab_http_status(:unauthorized) }
+      it { expect(response).to have_gitlab_http_status(:bad_request) }
     end
 
     context 'when user is blocked' do