From 00a889ea7a115ebbda95a071dd630f93b79261e3 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Fri, 23 Jul 2021 09:08:49 +0000 Subject: Add latest changes from gitlab-org/gitlab@master --- app/assets/javascripts/diffs/components/app.vue | 21 ++++- .../environments/components/edit_environment.vue | 54 +++++++++++++ app/assets/javascripts/environments/edit.js | 18 +++++ .../pages/projects/environments/edit/index.js | 3 + .../projects/environments_controller.rb | 4 +- app/helpers/environments_helper.rb | 8 ++ .../service_ping/permit_data_categories_service.rb | 12 +-- app/services/service_ping/service_ping_settings.rb | 19 +++++ app/services/service_ping/submit_service.rb | 2 +- app/views/projects/environments/edit.html.haml | 7 +- lib/gitlab/jira_import/issue_serializer.rb | 5 +- .../projects/environments_controller_spec.rb | 24 +++++- .../frontend/environments/edit_environment_spec.js | 90 ++++++++++++++++++++++ spec/helpers/environments_helper_spec.rb | 9 +++ .../gitlab/jira_import/issue_serializer_spec.rb | 13 ++++ .../permit_data_categories_service_spec.rb | 25 ------ .../service_ping/service_ping_settings_spec.rb | 30 ++++++++ .../submit_service_ping_service_spec.rb | 8 +- 18 files changed, 294 insertions(+), 58 deletions(-) create mode 100644 app/assets/javascripts/environments/components/edit_environment.vue create mode 100644 app/assets/javascripts/environments/edit.js create mode 100644 app/assets/javascripts/pages/projects/environments/edit/index.js create mode 100644 app/services/service_ping/service_ping_settings.rb create mode 100644 spec/frontend/environments/edit_environment_spec.js create mode 100644 spec/services/service_ping/service_ping_settings_spec.rb diff --git a/app/assets/javascripts/diffs/components/app.vue b/app/assets/javascripts/diffs/components/app.vue index e33b60f8ab5..35b7ea48958 100644 --- a/app/assets/javascripts/diffs/components/app.vue +++ b/app/assets/javascripts/diffs/components/app.vue @@ -506,9 +506,24 @@ export default { ); } - Mousetrap.bind(['ctrl+f', 'command+f'], () => { - this.disableVirtualScroller = true; - }); + if (window.gon?.features?.diffsVirtualScrolling) { + let keydownTime; + Mousetrap.bind(['mod+f', 'mod+g'], () => { + keydownTime = new Date().getTime(); + }); + + window.addEventListener('blur', () => { + if (keydownTime) { + const delta = new Date().getTime() - keydownTime; + + // To make sure the user is using the find function we need to wait for blur + // and max 1000ms to be sure it the search box is filtered + if (delta >= 0 && delta < 1000) { + this.disableVirtualScroller = true; + } + } + }); + } }, removeEventListeners() { Mousetrap.unbind(keysFor(MR_PREVIOUS_FILE_IN_DIFF)); diff --git a/app/assets/javascripts/environments/components/edit_environment.vue b/app/assets/javascripts/environments/components/edit_environment.vue new file mode 100644 index 00000000000..80355708311 --- /dev/null +++ b/app/assets/javascripts/environments/components/edit_environment.vue @@ -0,0 +1,54 @@ + + diff --git a/app/assets/javascripts/environments/edit.js b/app/assets/javascripts/environments/edit.js new file mode 100644 index 00000000000..dd6680f64bd --- /dev/null +++ b/app/assets/javascripts/environments/edit.js @@ -0,0 +1,18 @@ +import Vue from 'vue'; +import EditEnvironment from './components/edit_environment.vue'; + +export default (el) => + new Vue({ + el, + provide: { + projectEnvironmentsPath: el.dataset.projectEnvironmentsPath, + updateEnvironmentPath: el.dataset.updateEnvironmentPath, + }, + render(h) { + return h(EditEnvironment, { + props: { + environment: JSON.parse(el.dataset.environment), + }, + }); + }, + }); diff --git a/app/assets/javascripts/pages/projects/environments/edit/index.js b/app/assets/javascripts/pages/projects/environments/edit/index.js new file mode 100644 index 00000000000..574963d825a --- /dev/null +++ b/app/assets/javascripts/pages/projects/environments/edit/index.js @@ -0,0 +1,3 @@ +import mountEdit from '~/environments/edit'; + +mountEdit(document.getElementById('js-edit-environment')); diff --git a/app/controllers/projects/environments_controller.rb b/app/controllers/projects/environments_controller.rb index 7e76190f9c4..cac0aa9d513 100644 --- a/app/controllers/projects/environments_controller.rb +++ b/app/controllers/projects/environments_controller.rb @@ -95,9 +95,9 @@ class Projects::EnvironmentsController < Projects::ApplicationController def update if @environment.update(environment_params) - redirect_to project_environment_path(project, @environment) + render json: { environment: @environment, path: project_environment_path(project, @environment) } else - render :edit + render json: { message: @environment.errors.full_messages }, status: :bad_request end end diff --git a/app/helpers/environments_helper.rb b/app/helpers/environments_helper.rb index 62060200698..b2842664879 100644 --- a/app/helpers/environments_helper.rb +++ b/app/helpers/environments_helper.rb @@ -45,6 +45,14 @@ module EnvironmentsHelper can?(current_user, :destroy_environment, environment) end + def environment_data(environment) + Gitlab::Json.generate({ + id: environment.id, + name: environment.name, + external_url: environment.external_url + }) + end + private def project_metrics_data(project) diff --git a/app/services/service_ping/permit_data_categories_service.rb b/app/services/service_ping/permit_data_categories_service.rb index ff48c022b56..49dd2426262 100644 --- a/app/services/service_ping/permit_data_categories_service.rb +++ b/app/services/service_ping/permit_data_categories_service.rb @@ -14,20 +14,10 @@ module ServicePing ].to_set.freeze def execute - return [] unless product_intelligence_enabled? + return [] unless ServicePingSettings.product_intelligence_enabled? CATEGORIES end - - def product_intelligence_enabled? - pings_enabled? && !User.single_user&.requires_usage_stats_consent? - end - - private - - def pings_enabled? - ::Gitlab::CurrentSettings.usage_ping_enabled? - end end end diff --git a/app/services/service_ping/service_ping_settings.rb b/app/services/service_ping/service_ping_settings.rb new file mode 100644 index 00000000000..2b6535d89d1 --- /dev/null +++ b/app/services/service_ping/service_ping_settings.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module ServicePing + module ServicePingSettings + extend self + + def product_intelligence_enabled? + pings_enabled? && !User.single_user&.requires_usage_stats_consent? + end + + private + + def pings_enabled? + ::Gitlab::CurrentSettings.usage_ping_enabled? + end + end +end + +ServicePing::ServicePingSettings.extend_mod_with('ServicePing::ServicePingSettings') diff --git a/app/services/service_ping/submit_service.rb b/app/services/service_ping/submit_service.rb index 5c03aa46e18..09d1670fd1f 100644 --- a/app/services/service_ping/submit_service.rb +++ b/app/services/service_ping/submit_service.rb @@ -18,7 +18,7 @@ module ServicePing SubmissionError = Class.new(StandardError) def execute - return unless ServicePing::PermitDataCategoriesService.new.product_intelligence_enabled? + return unless ServicePing::ServicePingSettings.product_intelligence_enabled? begin usage_data = BuildPayloadService.new.execute diff --git a/app/views/projects/environments/edit.html.haml b/app/views/projects/environments/edit.html.haml index c4e2c1eb63d..dcd5fb2574e 100644 --- a/app/views/projects/environments/edit.html.haml +++ b/app/views/projects/environments/edit.html.haml @@ -1,7 +1,6 @@ - page_title _("Edit"), @environment.name, _("Environments") - add_page_specific_style 'page_bundles/environments' -%h3.page-title - = _('Edit environment') -%hr -= render 'form' +#js-edit-environment{ data: { project_environments_path: project_environments_path(@project), + update_environment_path: project_environment_path(@project, @environment), + environment: environment_data(@environment)} } diff --git a/lib/gitlab/jira_import/issue_serializer.rb b/lib/gitlab/jira_import/issue_serializer.rb index 43280606bb6..ab748d67fbf 100644 --- a/lib/gitlab/jira_import/issue_serializer.rb +++ b/lib/gitlab/jira_import/issue_serializer.rb @@ -52,9 +52,10 @@ module Gitlab end def map_user_id(jira_user) - return unless jira_user&.dig('accountId') + jira_user_identifier = jira_user&.dig('accountId') || jira_user&.dig('key') + return unless jira_user_identifier - Gitlab::JiraImport.get_user_mapping(project.id, jira_user['accountId']) + Gitlab::JiraImport.get_user_mapping(project.id, jira_user_identifier) end def reporter diff --git a/spec/controllers/projects/environments_controller_spec.rb b/spec/controllers/projects/environments_controller_spec.rb index fc7bb63bd11..7103d7df5c5 100644 --- a/spec/controllers/projects/environments_controller_spec.rb +++ b/spec/controllers/projects/environments_controller_spec.rb @@ -200,11 +200,27 @@ RSpec.describe Projects::EnvironmentsController do end describe 'PATCH #update' do - it 'responds with a 302' do - patch_params = environment_params.merge(environment: { external_url: 'https://git.gitlab.com' }) - patch :update, params: patch_params + subject { patch :update, params: params } - expect(response).to have_gitlab_http_status(:found) + context "when environment params are valid" do + let(:params) { environment_params.merge(environment: { external_url: 'https://git.gitlab.com' }) } + + it 'returns ok and the path to the newly created environment' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['path']).to eq("/#{project.full_path}/-/environments/#{environment.id}") + end + end + + context "when environment params are invalid" do + let(:params) { environment_params.merge(environment: { name: '/foo/', external_url: '/git.gitlab.com' }) } + + it 'returns bad request' do + subject + + expect(response).to have_gitlab_http_status(:bad_request) + end end end diff --git a/spec/frontend/environments/edit_environment_spec.js b/spec/frontend/environments/edit_environment_spec.js new file mode 100644 index 00000000000..de497b18dd6 --- /dev/null +++ b/spec/frontend/environments/edit_environment_spec.js @@ -0,0 +1,90 @@ +import MockAdapter from 'axios-mock-adapter'; +import { mountExtended } from 'helpers/vue_test_utils_helper'; +import waitForPromises from 'helpers/wait_for_promises'; +import EditEnvironment from '~/environments/components/edit_environment.vue'; +import createFlash from '~/flash'; +import axios from '~/lib/utils/axios_utils'; +import { visitUrl } from '~/lib/utils/url_utility'; + +jest.mock('~/lib/utils/url_utility'); +jest.mock('~/flash'); + +const DEFAULT_OPTS = { + provide: { + projectEnvironmentsPath: '/projects/environments', + updateEnvironmentPath: '/proejcts/environments/1', + }, + propsData: { environment: { name: 'foo', externalUrl: 'https://foo.example.com' } }, +}; + +describe('~/environments/components/edit.vue', () => { + let wrapper; + let mock; + let name; + let url; + let form; + + const createWrapper = (opts = {}) => + mountExtended(EditEnvironment, { + ...DEFAULT_OPTS, + ...opts, + }); + + beforeEach(() => { + mock = new MockAdapter(axios); + wrapper = createWrapper(); + name = wrapper.findByLabelText('Name'); + url = wrapper.findByLabelText('External URL'); + form = wrapper.findByRole('form', { name: 'Edit environment' }); + }); + + afterEach(() => { + mock.restore(); + wrapper.destroy(); + }); + + const fillForm = async (expected, response) => { + mock + .onPut(DEFAULT_OPTS.provide.updateEnvironmentPath, { + name: expected.name, + external_url: expected.url, + }) + .reply(...response); + await name.setValue(expected.name); + await url.setValue(expected.url); + + await form.trigger('submit'); + await waitForPromises(); + }; + + it('sets the title to Edit environment', () => { + const header = wrapper.findByRole('heading', { name: 'Edit environment' }); + expect(header.exists()).toBe(true); + }); + + it.each` + input | value + ${() => name} | ${'test'} + ${() => url} | ${'https://example.org'} + `('it changes the value of the input to $value', async ({ input, value }) => { + await input().setValue(value); + + expect(input().element.value).toBe(value); + }); + + it('submits the updated environment on submit', async () => { + const expected = { name: 'test', url: 'https://google.ca' }; + + await fillForm(expected, [200, { path: '/test' }]); + + expect(visitUrl).toHaveBeenCalledWith('/test'); + }); + + it('shows errors on error', async () => { + const expected = { name: 'test', url: 'https://google.ca' }; + + await fillForm(expected, [400, { message: ['name taken'] }]); + + expect(createFlash).toHaveBeenCalledWith({ message: 'name taken' }); + }); +}); diff --git a/spec/helpers/environments_helper_spec.rb b/spec/helpers/environments_helper_spec.rb index 22867a5b652..60bed247d85 100644 --- a/spec/helpers/environments_helper_spec.rb +++ b/spec/helpers/environments_helper_spec.rb @@ -199,4 +199,13 @@ RSpec.describe EnvironmentsHelper do expect(helper.environment_logs_data(project, environment)).to eq(expected_data) end end + + describe '#environment_data' do + it 'returns the environment as JSON' do + expected_data = { id: environment.id, + name: environment.name, + external_url: environment.external_url }.to_json + expect(helper.environment_data(environment)).to eq(expected_data) + end + end end diff --git a/spec/lib/gitlab/jira_import/issue_serializer_spec.rb b/spec/lib/gitlab/jira_import/issue_serializer_spec.rb index e57a8457e7c..198d2db234c 100644 --- a/spec/lib/gitlab/jira_import/issue_serializer_spec.rb +++ b/spec/lib/gitlab/jira_import/issue_serializer_spec.rb @@ -192,6 +192,19 @@ RSpec.describe Gitlab::JiraImport::IssueSerializer do expect(subject[:assignee_ids]).to be_nil end end + + context 'with jira server response' do + let(:assignee) { double(attrs: { 'displayName' => 'Solver', 'key' => '1234' }) } + + context 'when assignee maps to a valid GitLab user' do + it 'sets the issue assignees to the mapped user' do + expect(Gitlab::JiraImport).to receive(:get_user_mapping).with(project.id, '1234') + .and_return(user.id) + + expect(subject[:assignee_ids]).to eq([user.id]) + end + end + end end end diff --git a/spec/services/service_ping/permit_data_categories_service_spec.rb b/spec/services/service_ping/permit_data_categories_service_spec.rb index 4fd5c6f9ccb..5b25f216c4d 100644 --- a/spec/services/service_ping/permit_data_categories_service_spec.rb +++ b/spec/services/service_ping/permit_data_categories_service_spec.rb @@ -3,8 +3,6 @@ require 'spec_helper' RSpec.describe ServicePing::PermitDataCategoriesService do - using RSpec::Parameterized::TableSyntax - describe '#execute', :without_license do subject(:permitted_categories) { described_class.new.execute } @@ -41,27 +39,4 @@ RSpec.describe ServicePing::PermitDataCategoriesService do end end end - - describe '#product_intelligence_enabled?' do - where(:usage_ping_enabled, :requires_usage_stats_consent, :expected_product_intelligence_enabled) do - # Usage ping enabled - true | false | true - true | true | false - - # Usage ping disabled - false | false | false - false | true | false - end - - with_them do - before do - allow(User).to receive(:single_user).and_return(double(:user, requires_usage_stats_consent?: requires_usage_stats_consent)) - stub_config_setting(usage_ping_enabled: usage_ping_enabled) - end - - it 'has the correct product_intelligence_enabled?' do - expect(described_class.new.product_intelligence_enabled?).to eq(expected_product_intelligence_enabled) - end - end - end end diff --git a/spec/services/service_ping/service_ping_settings_spec.rb b/spec/services/service_ping/service_ping_settings_spec.rb new file mode 100644 index 00000000000..8446fee36c5 --- /dev/null +++ b/spec/services/service_ping/service_ping_settings_spec.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ServicePing::ServicePingSettings do + using RSpec::Parameterized::TableSyntax + + describe '#product_intelligence_enabled?' do + where(:usage_ping_enabled, :requires_usage_stats_consent, :expected_product_intelligence_enabled) do + # Usage ping enabled + true | false | true + true | true | false + + # Usage ping disabled + false | false | false + false | true | false + end + + with_them do + before do + allow(User).to receive(:single_user).and_return(double(:user, requires_usage_stats_consent?: requires_usage_stats_consent)) + stub_config_setting(usage_ping_enabled: usage_ping_enabled) + end + + it 'has the correct product_intelligence_enabled?' do + expect(described_class.product_intelligence_enabled?).to eq(expected_product_intelligence_enabled) + end + end + end +end diff --git a/spec/services/service_ping/submit_service_ping_service_spec.rb b/spec/services/service_ping/submit_service_ping_service_spec.rb index 8a3065e6bc6..05df4e49014 100644 --- a/spec/services/service_ping/submit_service_ping_service_spec.rb +++ b/spec/services/service_ping/submit_service_ping_service_spec.rb @@ -100,9 +100,7 @@ RSpec.describe ServicePing::SubmitService do context 'when product_intelligence_enabled is false' do before do - allow_next_instance_of(ServicePing::PermitDataCategoriesService) do |service| - allow(service).to receive(:product_intelligence_enabled?).and_return(false) - end + allow(ServicePing::ServicePingSettings).to receive(:product_intelligence_enabled?).and_return(false) end it_behaves_like 'does not run' @@ -112,9 +110,7 @@ RSpec.describe ServicePing::SubmitService do before do stub_usage_data_connections - allow_next_instance_of(ServicePing::PermitDataCategoriesService) do |service| - allow(service).to receive(:product_intelligence_enabled?).and_return(true) - end + allow(ServicePing::ServicePingSettings).to receive(:product_intelligence_enabled?).and_return(true) end it 'generates service ping' do -- cgit v1.2.3