diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2023-10-18 18:10:24 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2023-10-18 18:10:24 +0300 |
commit | b81ffd93854bcc57c493745137578f141ac8a78f (patch) | |
tree | 3ceaaffa155813c40272927c6aef0dfdf04ac155 | |
parent | 91ca0550e00347bb6db5aba9f039be9137f3a254 (diff) |
Add latest changes from gitlab-org/gitlab@master
33 files changed, 213 insertions, 485 deletions
diff --git a/.rubocop_todo/gitlab/feature_available_usage.yml b/.rubocop_todo/gitlab/feature_available_usage.yml index 302bf8d17c4..45240467627 100644 --- a/.rubocop_todo/gitlab/feature_available_usage.yml +++ b/.rubocop_todo/gitlab/feature_available_usage.yml @@ -72,7 +72,6 @@ Gitlab/FeatureAvailableUsage: - 'ee/app/services/ee/boards/lists/create_service.rb' - 'ee/app/services/ee/boards/update_service.rb' - 'ee/app/services/ee/ide/schemas_config_service.rb' - - 'ee/app/services/ee/issuable_base_service.rb' - 'ee/app/services/ee/issue_links/create_service.rb' - 'ee/app/services/ee/issues/build_service.rb' - 'ee/app/services/ee/lfs/lock_file_service.rb' diff --git a/app/controllers/projects/prometheus/metrics_controller.rb b/app/controllers/projects/prometheus/metrics_controller.rb deleted file mode 100644 index 396841e667d..00000000000 --- a/app/controllers/projects/prometheus/metrics_controller.rb +++ /dev/null @@ -1,137 +0,0 @@ -# frozen_string_literal: true - -module Projects - module Prometheus - class MetricsController < Projects::ApplicationController - before_action :check_feature_availability! - before_action :authorize_admin_project! - before_action :require_prometheus_metrics! - - feature_category :metrics - urgency :low - - def active_common - respond_to do |format| - format.json do - matched_metrics = prometheus_adapter.query(:matched_metrics) || {} - - if matched_metrics.any? - render json: matched_metrics - else - head :no_content - end - end - end - end - - def validate_query - respond_to do |format| - format.json do - result = prometheus_adapter.query(:validate, params[:query]) - - if result - render json: result - else - head :accepted - end - end - end - end - - def new - @metric = project.prometheus_metrics.new - end - - def index - respond_to do |format| - format.json do - metrics = ::PrometheusMetricsFinder.new( - project: project, - ordered: true - ).execute.to_a - - response = {} - if metrics.any? - response[:metrics] = ::PrometheusMetricSerializer - .new(project: project) - .represent(metrics) - end - - render json: response - end - end - end - - def create - @metric = project.prometheus_metrics.create( - metrics_params.to_h.symbolize_keys - ) - - if @metric.persisted? - redirect_to edit_project_settings_integration_path(project, ::Integrations::Prometheus), - notice: _('Metric was successfully added.') - else - render 'new' - end - end - - def update - @metric = prometheus_metric - - if @metric.update(metrics_params) - redirect_to edit_project_settings_integration_path(project, ::Integrations::Prometheus), - notice: _('Metric was successfully updated.') - else - render 'edit' - end - end - - def edit - @metric = prometheus_metric - end - - def destroy - destroy_metrics_service(prometheus_metric).execute - - respond_to do |format| - format.html do - redirect_to edit_project_settings_integration_path(project, ::Integrations::Prometheus), status: :see_other - end - format.json do - head :ok - end - end - end - - private - - def prometheus_adapter - @prometheus_adapter ||= ::Gitlab::Prometheus::Adapter.new(project, project.deployment_platform&.cluster).prometheus_adapter - end - - def require_prometheus_metrics! - render_404 unless prometheus_adapter&.can_query? - end - - def prometheus_metric - @prometheus_metric ||= ::PrometheusMetricsFinder.new(id: params[:id]).execute.first - end - - def update_metrics_service(metric) - ::Projects::Prometheus::Metrics::UpdateService.new(metric, metrics_params) - end - - def destroy_metrics_service(metric) - ::Projects::Prometheus::Metrics::DestroyService.new(metric) - end - - def metrics_params - params.require(:prometheus_metric).permit(:title, :query, :y_label, :unit, :legend, :group) - end - - def check_feature_availability! - render_404 if Feature.enabled?(:remove_monitor_metrics) - end - end - end -end diff --git a/app/graphql/mutations/work_items/update.rb b/app/graphql/mutations/work_items/update.rb index 1115dc74a9b..228a9e52355 100644 --- a/app/graphql/mutations/work_items/update.rb +++ b/app/graphql/mutations/work_items/update.rb @@ -28,7 +28,7 @@ module Mutations raise_resource_not_available_error! unless attributes.empty? || can_update?(work_item) update_result = ::WorkItems::UpdateService.new( - container: work_item.project, + container: work_item.resource_parent, current_user: current_user, params: attributes, widget_params: widget_params, diff --git a/app/helpers/projects/ml/experiments_helper.rb b/app/helpers/projects/ml/experiments_helper.rb index d5b2c3cd36a..6c7b6eb6fbc 100644 --- a/app/helpers/projects/ml/experiments_helper.rb +++ b/app/helpers/projects/ml/experiments_helper.rb @@ -14,17 +14,17 @@ module Projects Gitlab::Json.generate(data) end - def candidates_table_items(candidates, user) + def candidates_table_items(candidates, current_user) items = candidates.map do |candidate| { **candidate.params.to_h { |p| [p.name, p.value] }, **candidate.latest_metrics.to_h { |m| [m.name, number_with_precision(m.value, precision: 4)] }, - ci_job: job_info(candidate, user), + ci_job: job_info(candidate, current_user), artifact: link_to_artifact(candidate), details: link_to_details(candidate), name: candidate.name, created_at: candidate.created_at, - user: user_info(candidate) + user: user_info(candidate, current_user) } end @@ -87,8 +87,13 @@ module Projects project_ml_experiment_path(project, experiment.iid) end - def user_info(candidate) - user = candidate.user + def user_info(candidate, current_user) + user = + if candidate.from_ci? + candidate.ci_build.user if can?(current_user, :read_build, candidate.ci_build) + else + candidate.user + end return unless user.present? diff --git a/app/services/concerns/users/participable_service.rb b/app/services/concerns/users/participable_service.rb index 1a03b444b68..a54c4947b0b 100644 --- a/app/services/concerns/users/participable_service.rb +++ b/app/services/concerns/users/participable_service.rb @@ -34,7 +34,7 @@ module Users def groups return [] unless current_user - current_user.authorized_groups.with_route.sort_by(&:path) + current_user.authorized_groups.with_route.sort_by(&:full_path) end def render_participants_as_hash(participants) diff --git a/app/services/import/validate_remote_git_endpoint_service.rb b/app/services/import/validate_remote_git_endpoint_service.rb index a994072c4aa..2177238fddf 100644 --- a/app/services/import/validate_remote_git_endpoint_service.rb +++ b/app/services/import/validate_remote_git_endpoint_service.rb @@ -13,6 +13,8 @@ module Import GIT_PROTOCOL_PKT_LEN = 4 GIT_MINIMUM_RESPONSE_LENGTH = GIT_PROTOCOL_PKT_LEN + GIT_EXPECTED_FIRST_PACKET_LINE.length EXPECTED_CONTENT_TYPE = "application/x-#{GIT_SERVICE_NAME}-advertisement" + INVALID_BODY_MESSAGE = 'Not a git repository: Invalid response body' + INVALID_CONTENT_TYPE_MESSAGE = 'Not a git repository: Invalid content-type' def initialize(params) @params = params @@ -30,32 +32,35 @@ module Import uri.fragment = nil url = Gitlab::Utils.append_path(uri.to_s, "/info/refs?service=#{GIT_SERVICE_NAME}") - response_body = '' - result = nil - Gitlab::HTTP.try_get(url, stream_body: true, follow_redirects: false, basic_auth: auth) do |fragment| - response_body += fragment - next if response_body.length < GIT_MINIMUM_RESPONSE_LENGTH - - result = if status_code_is_valid(fragment) && content_type_is_valid(fragment) && response_body_is_valid(response_body) - :success - else - :error - end - - # We are interested only in the first chunks of the response - # So we're using stream_body: true and breaking when receive enough body - break - end + response, response_body = http_get_and_extract_first_chunks(url) - if result == :success - ServiceResponse.success - else - ServiceResponse.error(message: "#{uri} is not a valid HTTP Git repository") - end + validate(uri, response, response_body) + rescue *Gitlab::HTTP::HTTP_ERRORS => err + error_result("HTTP #{err.class.name.underscore} error: #{err.message}") + rescue StandardError => err + ServiceResponse.error( + message: "Internal #{err.class.name.underscore} error: #{err.message}", + reason: 500 + ) end private + def http_get_and_extract_first_chunks(url) + # We are interested only in the first chunks of the response + # So we're using stream_body: true and breaking when receive enough body + response = nil + response_body = '' + + Gitlab::HTTP.get(url, stream_body: true, follow_redirects: false, basic_auth: auth) do |response_chunk| + response = response_chunk + response_body += response_chunk + break if GIT_MINIMUM_RESPONSE_LENGTH <= response_body.length + end + + [response, response_body] + end + def auth unless @params[:user].to_s.blank? { @@ -65,16 +70,38 @@ module Import end end - def status_code_is_valid(fragment) - fragment.http_response.code == '200' + def validate(uri, response, response_body) + return status_code_error(uri, response) unless status_code_is_valid?(response) + return error_result(INVALID_CONTENT_TYPE_MESSAGE) unless content_type_is_valid?(response) + return error_result(INVALID_BODY_MESSAGE) unless response_body_is_valid?(response_body) + + ServiceResponse.success + end + + def status_code_error(uri, response) + http_code = response.http_response.code.to_i + message = response.http_response.message || Rack::Utils::HTTP_STATUS_CODES[http_code] + + error_result( + "#{uri} endpoint error: #{http_code}#{message.presence&.prepend(' ')}", + http_code + ) + end + + def error_result(message, reason = nil) + ServiceResponse.error(message: message, reason: reason) + end + + def status_code_is_valid?(response) + response.http_response.code == '200' end - def content_type_is_valid(fragment) - fragment.http_response['content-type'] == EXPECTED_CONTENT_TYPE + def content_type_is_valid?(response) + response.http_response['content-type'] == EXPECTED_CONTENT_TYPE end - def response_body_is_valid(response_body) - response_body.match?(GIT_BODY_MESSAGE_REGEXP) + def response_body_is_valid?(response_body) + response_body.length <= GIT_MINIMUM_RESPONSE_LENGTH && response_body.match?(GIT_BODY_MESSAGE_REGEXP) end end end diff --git a/app/services/projects/participants_service.rb b/app/services/projects/participants_service.rb index 458eaec4e2e..fe19d1f051d 100644 --- a/app/services/projects/participants_service.rb +++ b/app/services/projects/participants_service.rb @@ -11,8 +11,8 @@ module Projects noteable_owner + participants_in_noteable + all_members + - groups + - project_members + project_members + + groups render_participants_as_hash(participants.uniq) end diff --git a/app/workers/issuable/related_links_create_worker.rb b/app/workers/issuable/related_links_create_worker.rb index a306c23ec3e..7cbf70fd5ab 100644 --- a/app/workers/issuable/related_links_create_worker.rb +++ b/app/workers/issuable/related_links_create_worker.rb @@ -13,8 +13,8 @@ module Issuable urgency :high idempotent! - def perform(params) - @params = params.with_indifferent_access + def perform(args) + @params = args.with_indifferent_access @user = User.find_by_id(params[:user_id]) @issuable = issuable_class.find_by_id(params[:issuable_id]) @links = issuable_class.related_link_class&.where(id: params[:link_ids]) diff --git a/doc/administration/postgresql/replication_and_failover.md b/doc/administration/postgresql/replication_and_failover.md index c3d972d4d33..8403177952a 100644 --- a/doc/administration/postgresql/replication_and_failover.md +++ b/doc/administration/postgresql/replication_and_failover.md @@ -18,7 +18,7 @@ replication and failover for GitLab. ## Architecture -The Linux pacakage-recommended configuration for a PostgreSQL cluster with +The Linux package-recommended configuration for a PostgreSQL cluster with replication failover requires: - A minimum of three PostgreSQL nodes. diff --git a/doc/api/pipeline_schedules.md b/doc/api/pipeline_schedules.md index 871d6d42885..bfcc4ee4613 100644 --- a/doc/api/pipeline_schedules.md +++ b/doc/api/pipeline_schedules.md @@ -95,7 +95,8 @@ curl --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/a { "key": "TEST_VARIABLE_1", "variable_type": "env_var", - "value": "TEST_1" + "value": "TEST_1", + "raw": false } ] } diff --git a/doc/architecture/blueprints/_template.md b/doc/architecture/blueprints/_template.md index e22cc2e6857..18f88322906 100644 --- a/doc/architecture/blueprints/_template.md +++ b/doc/architecture/blueprints/_template.md @@ -9,9 +9,13 @@ participating-stages: [] --- <!-- -**Note:** Please remove comment blocks for sections you've filled in. -When your blueprint ready for review, all of these comment blocks should be -removed. +Before you start: + +- Copy this file to a sub-directory and call it `index.md` for it to appear in + the blueprint directory. +- Please remove comment blocks for sections you've filled in. + When your blueprint ready for review, all of these comment blocks should be + removed. To get started with a blueprint you can use this template to inform you about what you may want to document in it at the beginning. This content will change diff --git a/doc/architecture/blueprints/cloud_connector/service_gateway.md b/doc/architecture/blueprints/cloud_connector/index.md index 840e17a438a..840e17a438a 100644 --- a/doc/architecture/blueprints/cloud_connector/service_gateway.md +++ b/doc/architecture/blueprints/cloud_connector/index.md diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 0ce96134f1d..a77507eaf08 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -29874,12 +29874,6 @@ msgstr "" msgid "Metric" msgstr "" -msgid "Metric was successfully added." -msgstr "" - -msgid "Metric was successfully updated." -msgstr "" - msgid "Metric:" msgstr "" diff --git a/package.json b/package.json index 680d06414ed..6c6e174f7ee 100644 --- a/package.json +++ b/package.json @@ -234,7 +234,7 @@ }, "devDependencies": { "@gitlab/eslint-plugin": "19.2.0", - "@gitlab/stylelint-config": "5.0.0", + "@gitlab/stylelint-config": "5.0.1", "@graphql-eslint/eslint-plugin": "3.20.1", "@originjs/vite-plugin-commonjs": "^1.0.3", "@rollup/plugin-graphql": "^2.0.3", diff --git a/qa/qa/specs/features/browser_ui/14_analytics/service_ping_default_enabled_spec.rb b/qa/qa/specs/features/browser_ui/14_analytics/service_ping_default_enabled_spec.rb index 8e4b76cdb7c..120da2990dd 100644 --- a/qa/qa/specs/features/browser_ui/14_analytics/service_ping_default_enabled_spec.rb +++ b/qa/qa/specs/features/browser_ui/14_analytics/service_ping_default_enabled_spec.rb @@ -2,7 +2,7 @@ module QA RSpec.describe 'Analytics' do - describe 'Service ping default enabled', product_group: :product_intelligence do + describe 'Service ping default enabled', product_group: :analytics_instrumentation do context 'when using default enabled from gitlab.yml config', :requires_admin, except: { job: 'review-qa-*' } do before do Flow::Login.sign_in_as_admin diff --git a/qa/qa/specs/features/browser_ui/14_analytics/service_ping_disabled_spec.rb b/qa/qa/specs/features/browser_ui/14_analytics/service_ping_disabled_spec.rb index e25bba97288..6d99943ce30 100644 --- a/qa/qa/specs/features/browser_ui/14_analytics/service_ping_disabled_spec.rb +++ b/qa/qa/specs/features/browser_ui/14_analytics/service_ping_disabled_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module QA - RSpec.describe 'Analytics', product_group: :product_intelligence do + RSpec.describe 'Analytics', product_group: :analytics_instrumentation do describe 'Service ping disabled', :orchestrated, :service_ping_disabled, :requires_admin do context 'when disabled from gitlab.yml config' do before do diff --git a/qa/qa/specs/features/browser_ui/6_release/deploy_key/add_deploy_key_spec.rb b/qa/qa/specs/features/browser_ui/6_release/deploy_key/add_deploy_key_spec.rb index 05dfc0c572e..66906c46e21 100644 --- a/qa/qa/specs/features/browser_ui/6_release/deploy_key/add_deploy_key_spec.rb +++ b/qa/qa/specs/features/browser_ui/6_release/deploy_key/add_deploy_key_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module QA - RSpec.describe 'Release', product_group: :release do + RSpec.describe 'Release', product_group: :environments do describe 'Deploy key creation' do it 'user adds a deploy key', testcase: 'https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/348023' do Flow::Login.sign_in diff --git a/qa/qa/specs/features/browser_ui/6_release/deploy_key/clone_using_deploy_key_spec.rb b/qa/qa/specs/features/browser_ui/6_release/deploy_key/clone_using_deploy_key_spec.rb index 44cb8e7fc03..f6bea2535a3 100644 --- a/qa/qa/specs/features/browser_ui/6_release/deploy_key/clone_using_deploy_key_spec.rb +++ b/qa/qa/specs/features/browser_ui/6_release/deploy_key/clone_using_deploy_key_spec.rb @@ -3,7 +3,7 @@ require 'digest/sha1' module QA - RSpec.describe 'Release', :runner, product_group: :release do + RSpec.describe 'Release', :runner, product_group: :environments do describe 'Git clone using a deploy key' do let(:runner_name) { "qa-runner-#{SecureRandom.hex(4)}" } let(:repository_location) { project.repository_ssh_location } diff --git a/qa/qa/specs/features/browser_ui/6_release/deploy_token/add_deploy_token_spec.rb b/qa/qa/specs/features/browser_ui/6_release/deploy_token/add_deploy_token_spec.rb index b0b1fa2b68d..109c153ea48 100644 --- a/qa/qa/specs/features/browser_ui/6_release/deploy_token/add_deploy_token_spec.rb +++ b/qa/qa/specs/features/browser_ui/6_release/deploy_token/add_deploy_token_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module QA - RSpec.describe 'Release', product_group: :release do + RSpec.describe 'Release', product_group: :environments do describe 'Deploy token creation' do it 'user adds a deploy token', testcase: 'https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/348028' do Flow::Login.sign_in diff --git a/qa/qa/specs/features/browser_ui/7_configure/auto_devops/auto_devops_templates_spec.rb b/qa/qa/specs/features/browser_ui/7_configure/auto_devops/auto_devops_templates_spec.rb index 4cadeb4b842..820fde7ad3d 100644 --- a/qa/qa/specs/features/browser_ui/7_configure/auto_devops/auto_devops_templates_spec.rb +++ b/qa/qa/specs/features/browser_ui/7_configure/auto_devops/auto_devops_templates_spec.rb @@ -2,7 +2,7 @@ module QA RSpec.describe 'Configure' do - describe 'AutoDevOps Templates', only: { subdomain: %i[staging staging-canary] }, product_group: :configure do + describe 'AutoDevOps Templates', only: { subdomain: %i[staging staging-canary] }, product_group: :environments do using RSpec::Parameterized::TableSyntax # specify jobs to be disabled in the pipeline. diff --git a/qa/qa/specs/features/browser_ui/7_configure/auto_devops/create_project_with_auto_devops_spec.rb b/qa/qa/specs/features/browser_ui/7_configure/auto_devops/create_project_with_auto_devops_spec.rb index 1eaeea8fe23..a94317d3463 100644 --- a/qa/qa/specs/features/browser_ui/7_configure/auto_devops/create_project_with_auto_devops_spec.rb +++ b/qa/qa/specs/features/browser_ui/7_configure/auto_devops/create_project_with_auto_devops_spec.rb @@ -2,7 +2,7 @@ module QA RSpec.describe 'Configure', - only: { pipeline: %i[staging staging-canary canary production] }, product_group: :configure do + only: { pipeline: %i[staging staging-canary canary production] }, product_group: :environments do describe 'Auto DevOps with a Kubernetes Agent' do let!(:app_project) { create(:project, :auto_devops, name: 'autodevops-app-project', template_name: 'express') } let!(:cluster) { Service::KubernetesCluster.new(provider_class: Service::ClusterProvider::Gcloud).create! } diff --git a/spec/controllers/projects/prometheus/metrics_controller_spec.rb b/spec/controllers/projects/prometheus/metrics_controller_spec.rb deleted file mode 100644 index 8f8edebbc30..00000000000 --- a/spec/controllers/projects/prometheus/metrics_controller_spec.rb +++ /dev/null @@ -1,230 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Projects::Prometheus::MetricsController, feature_category: :metrics do - let_it_be(:user) { create(:user) } - let_it_be(:project) { create(:project, :with_prometheus_integration) } - - let(:prometheus_adapter) { double('prometheus_adapter', can_query?: true) } - - before do - stub_feature_flags(remove_monitor_metrics: false) - project.add_maintainer(user) - sign_in(user) - end - - describe 'GET #active_common' do - context 'when prometheus_adapter can query' do - before do - allow(controller).to receive(:prometheus_adapter).and_return(prometheus_adapter) - end - - context 'when prometheus metrics are enabled' do - context 'when data is not present' do - before do - allow(prometheus_adapter).to receive(:query).with(:matched_metrics).and_return({}) - end - - it 'returns no content response' do - get :active_common, params: project_params(format: :json) - - expect(response).to have_gitlab_http_status(:no_content) - end - end - - context 'when data is available' do - let(:sample_response) { { some_data: 1 } } - - before do - allow(prometheus_adapter).to receive(:query).with(:matched_metrics).and_return(sample_response) - end - - it 'returns no content response' do - get :active_common, params: project_params(format: :json) - - expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to eq(sample_response.deep_stringify_keys) - end - end - - context 'when requesting non json response' do - it 'returns not found response' do - get :active_common, params: project_params - - expect(response).to have_gitlab_http_status(:not_found) - end - end - end - end - - context 'when prometheus_adapter cannot query' do - it 'renders 404' do - prometheus_adapter = double('prometheus_adapter', can_query?: false) - - allow(controller).to receive(:prometheus_adapter).and_return(prometheus_adapter) - allow(prometheus_adapter).to receive(:query).with(:matched_metrics).and_return({}) - - get :active_common, params: project_params(format: :json) - - expect(response).to have_gitlab_http_status(:not_found) - end - end - - context 'when prometheus_adapter is disabled' do - let(:project) { create(:project) } - - it 'renders 404' do - get :active_common, params: project_params(format: :json) - - expect(response).to have_gitlab_http_status(:not_found) - end - end - - context 'when metrics dashboard feature is unavailable' do - before do - stub_feature_flags(remove_monitor_metrics: true) - end - - it 'renders 404' do - get :active_common, params: project_params(format: :json) - - expect(response).to have_gitlab_http_status(:not_found) - end - end - end - - describe 'POST #validate_query' do - before do - allow(controller).to receive(:prometheus_adapter).and_return(prometheus_adapter) - allow(prometheus_adapter).to receive(:query).with(:validate, query) { validation_result } - end - - let(:query) { 'avg(metric)' } - - context 'validation information is ready' do - let(:validation_result) { { valid: true } } - - it 'validation data is returned' do - post :validate_query, params: project_params(format: :json, query: query) - - expect(json_response).to eq('valid' => true) - end - end - - context 'validation information is not ready' do - let(:validation_result) { nil } - - it 'validation data is returned' do - post :validate_query, params: project_params(format: :json, query: query) - - expect(response).to have_gitlab_http_status(:accepted) - end - end - end - - describe 'GET #index' do - context 'with custom metric present' do - let!(:prometheus_metric) { create(:prometheus_metric, project: project) } - - it 'returns a list of metrics' do - get :index, params: project_params(format: :json) - - expect(response).to have_gitlab_http_status(:ok) - expect(response).to match_response_schema('prometheus/metrics') - end - end - - context 'without custom metrics ' do - it 'returns an empty json' do - get :index, params: project_params(format: :json) - - expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to eq({}) - end - end - end - - describe 'POST #create' do - context 'metric is valid' do - let(:valid_metric) { { prometheus_metric: { title: 'title', query: 'query', group: 'business', y_label: 'label', unit: 'u', legend: 'legend' } } } - - it 'shows a success flash message' do - post :create, params: project_params(valid_metric) - - expect(flash[:notice]).to include('Metric was successfully added.') - - expect(response).to redirect_to(edit_project_settings_integration_path(project, ::Integrations::Prometheus)) - end - end - - context 'metric is invalid' do - let(:invalid_metric) { { prometheus_metric: { title: 'title' } } } - - it 'renders new metric page' do - post :create, params: project_params(invalid_metric) - - expect(response).to have_gitlab_http_status(:ok) - expect(response).to render_template('new') - end - end - end - - describe 'PUT #update' do - context 'metric is updated' do - let_it_be(:metric) { create(:prometheus_metric, project: project) } - - let(:metric_params) { { prometheus_metric: { title: 'new_title' }, id: metric.id } } - - it 'shows a success flash message' do - put :update, params: project_params(metric_params) - - expect(metric.reload.title).to eq('new_title') - expect(flash[:notice]).to include('Metric was successfully updated.') - expect(response).to redirect_to(edit_project_settings_integration_path(project, ::Integrations::Prometheus)) - end - end - end - - describe 'DELETE #destroy' do - context 'format html' do - let!(:metric) { create(:prometheus_metric, project: project) } - - it 'destroys the metric' do - delete :destroy, params: project_params(id: metric.id) - - expect(response).to redirect_to(edit_project_settings_integration_path(project, ::Integrations::Prometheus)) - expect(PrometheusMetric.find_by(id: metric.id)).to be_nil - end - end - - context 'format json' do - let!(:metric) { create(:prometheus_metric, project: project) } - - it 'destroys the metric' do - delete :destroy, params: project_params(id: metric.id, format: :json) - - expect(response).to have_gitlab_http_status(:ok) - expect(PrometheusMetric.find_by(id: metric.id)).to be_nil - end - end - end - - describe '#prometheus_adapter' do - before do - allow(controller).to receive(:project).and_return(project) - end - - it 'calls prometheus adapter service' do - expect_next_instance_of(::Gitlab::Prometheus::Adapter) do |instance| - expect(instance).to receive(:prometheus_adapter) - end - - subject.__send__(:prometheus_adapter) - end - end - - def project_params(opts = {}) - opts.reverse_merge(namespace_id: project.namespace, project_id: project) - end -end diff --git a/spec/features/projects/integrations/user_activates_prometheus_spec.rb b/spec/features/projects/integrations/user_activates_prometheus_spec.rb deleted file mode 100644 index db71256b294..00000000000 --- a/spec/features/projects/integrations/user_activates_prometheus_spec.rb +++ /dev/null @@ -1,21 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe 'User activates Prometheus', feature_category: :integrations do - include_context 'project integration activation' - - before do - stub_feature_flags(remove_monitor_metrics: false) - stub_request(:get, /.*prometheus.example.com.*/) - end - - it 'saves and activates integration', :js do - visit_project_integration('Prometheus') - check('Active') - - click_button('Save changes') - - expect(page).to have_content('Prometheus settings saved and active.') - end -end diff --git a/spec/helpers/projects/ml/experiments_helper_spec.rb b/spec/helpers/projects/ml/experiments_helper_spec.rb index 569fd0f9ec5..9ac518f664d 100644 --- a/spec/helpers/projects/ml/experiments_helper_spec.rb +++ b/spec/helpers/projects/ml/experiments_helper_spec.rb @@ -9,7 +9,7 @@ RSpec.describe Projects::Ml::ExperimentsHelper, feature_category: :mlops do let_it_be(:project) { create(:project, :private) } let_it_be(:experiment) { create(:ml_experiments, user_id: project.creator, project: project) } let_it_be(:pipeline) { create(:ci_pipeline, project: project) } - let_it_be(:build) { create(:ci_build, pipeline: pipeline) } + let_it_be(:build) { create(:ci_build, user: project.creator, pipeline: pipeline) } let_it_be(:candidate0) do create(:ml_candidates, :with_artifact, @@ -46,7 +46,7 @@ RSpec.describe Projects::Ml::ExperimentsHelper, feature_category: :mlops do 'ci_job' => { 'path' => "/#{project.full_path}/-/jobs/#{build.id}", 'name' => 'test' }, 'name' => candidate0.name, 'created_at' => candidate0.created_at.strftime('%Y-%m-%dT%H:%M:%S.%LZ'), - 'user' => { 'username' => candidate0.user.username, 'path' => "/#{candidate0.user.username}" } }, + 'user' => { 'username' => build.user.username, 'path' => "/#{build.user.username}" } }, { 'param2' => 'p3', 'param3' => 'p4', 'metric3' => '0.4000', 'artifact' => nil, 'details' => "/#{project.full_path}/-/ml/candidates/#{candidate1.iid}", 'ci_job' => nil, @@ -66,6 +66,7 @@ RSpec.describe Projects::Ml::ExperimentsHelper, feature_category: :mlops do before do allow(candidate0).to receive(:user).and_return(nil) + allow(candidate0.ci_build).to receive(:user).and_return(nil) end it 'has the user property, but is nil' do @@ -80,8 +81,9 @@ RSpec.describe Projects::Ml::ExperimentsHelper, feature_category: :mlops do .and_return(false) end - it 'does not include ci info' do + it 'does not include ci info and user for candidate created through CI' do expect(subject[0]['ci_job']).to be_nil + expect(subject[0]['user']).to be_nil end end end diff --git a/spec/requests/api/graphql/mutations/work_items/update_spec.rb b/spec/requests/api/graphql/mutations/work_items/update_spec.rb index b4261fb83c7..cb6571c2c93 100644 --- a/spec/requests/api/graphql/mutations/work_items/update_spec.rb +++ b/spec/requests/api/graphql/mutations/work_items/update_spec.rb @@ -7,14 +7,13 @@ RSpec.describe 'Update a work item', feature_category: :team_planning do let_it_be(:group) { create(:group) } let_it_be(:project) { create(:project, group: group) } - let_it_be(:author) { create(:user).tap { |user| project.add_reporter(user) } } - let_it_be(:developer) { create(:user).tap { |user| project.add_developer(user) } } - let_it_be(:reporter) { create(:user).tap { |user| project.add_reporter(user) } } - let_it_be(:guest) { create(:user).tap { |user| project.add_guest(user) } } + let_it_be(:author) { create(:user).tap { |user| group.add_reporter(user) } } + let_it_be(:developer) { create(:user).tap { |user| group.add_developer(user) } } + let_it_be(:reporter) { create(:user).tap { |user| group.add_reporter(user) } } + let_it_be(:guest) { create(:user).tap { |user| group.add_guest(user) } } let_it_be(:work_item, refind: true) { create(:work_item, project: project, author: author) } - let(:work_item_event) { 'CLOSE' } - let(:input) { { 'stateEvent' => work_item_event, 'title' => 'updated title' } } + let(:input) { { 'stateEvent' => 'CLOSE', 'title' => 'updated title' } } let(:fields) do <<~FIELDS workItem { @@ -25,7 +24,8 @@ RSpec.describe 'Update a work item', feature_category: :team_planning do FIELDS end - let(:mutation) { graphql_mutation(:workItemUpdate, input.merge('id' => work_item.to_global_id.to_s), fields) } + let(:mutation_work_item) { work_item } + let(:mutation) { graphql_mutation(:workItemUpdate, input.merge('id' => mutation_work_item.to_gid.to_s), fields) } let(:mutation_response) { graphql_mutation_response(:work_item_update) } @@ -60,7 +60,7 @@ RSpec.describe 'Update a work item', feature_category: :team_planning do end context 'when the work item is closed' do - let(:work_item_event) { 'REOPEN' } + let(:input) { { 'stateEvent' => 'REOPEN' } } before do work_item.close! @@ -155,10 +155,10 @@ RSpec.describe 'Update a work item', feature_category: :team_planning do it 'updates labels' do expect do post_graphql_mutation(mutation, current_user: current_user) - work_item.reload - end.to change { work_item.labels.count }.to(expected_labels.count) + mutation_work_item.reload + end.to change { mutation_work_item.labels.count }.to(expected_labels.count) - expect(work_item.labels).to match_array(expected_labels) + expect(mutation_work_item.labels).to match_array(expected_labels) expect(mutation_response['workItem']['widgets']).to include( 'labels' => { 'nodes' => match_array(expected_labels.map { |l| { 'id' => l.to_gid.to_s } }) @@ -168,9 +168,9 @@ RSpec.describe 'Update a work item', feature_category: :team_planning do end end - let_it_be(:existing_label) { create(:label, project: project) } - let_it_be(:label1) { create(:label, project: project) } - let_it_be(:label2) { create(:label, project: project) } + let_it_be(:existing_label) { create(:group_label, group: group) } + let_it_be(:label1) { create(:group_label, group: group) } + let_it_be(:label2) { create(:group_label, group: group) } let(:fields) do <<~FIELDS @@ -197,9 +197,11 @@ RSpec.describe 'Update a work item', feature_category: :team_planning do let(:add_label_ids) { [] } let(:remove_label_ids) { [] } + let_it_be(:group_work_item) { create(:work_item, :task, :group_level, namespace: group) } before_all do work_item.update!(labels: [existing_label]) + group_work_item.update!(labels: [existing_label]) end context 'when only removing labels' do @@ -213,6 +215,12 @@ RSpec.describe 'Update a work item', feature_category: :team_planning do it_behaves_like 'mutation updating work item labels' end + + context 'when work item belongs directly to the group' do + let(:mutation_work_item) { group_work_item } + + it_behaves_like 'mutation updating work item labels' + end end context 'when only adding labels' do @@ -228,6 +236,12 @@ RSpec.describe 'Update a work item', feature_category: :team_planning do it_behaves_like 'mutation updating work item labels' end + + context 'when work item belongs directly to the group' do + let(:mutation_work_item) { group_work_item } + + it_behaves_like 'mutation updating work item labels' + end end context 'when adding and removing labels' do @@ -245,6 +259,12 @@ RSpec.describe 'Update a work item', feature_category: :team_planning do it_behaves_like 'mutation updating work item labels' end + + context 'when work item belongs directly to the group' do + let(:mutation_work_item) { group_work_item } + + it_behaves_like 'mutation updating work item labels' + end end context 'when the work item type does not support labels widget' do diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 64e010aa50f..aa7120e482a 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -1400,13 +1400,14 @@ RSpec.describe API::Projects, :aggregate_failures, feature_category: :groups_and it 'disallows creating a project with an import_url that is not reachable' do url = 'http://example.com' endpoint_url = "#{url}/info/refs?service=git-upload-pack" - stub_full_request(endpoint_url, method: :get).to_return({ status: 301, body: '', headers: nil }) + error_response = { status: 301, body: '', headers: nil } + stub_full_request(endpoint_url, method: :get).to_return(error_response) project_params = { import_url: url, path: 'path-project-Foo', name: 'Foo Project' } expect { post api(path, user), params: project_params }.not_to change { Project.count } expect(response).to have_gitlab_http_status(:unprocessable_entity) - expect(json_response['message']).to eq("#{url} is not a valid HTTP Git repository") + expect(json_response['message']).to eq("#{url} endpoint error: #{error_response[:status]}") end it 'creates a project with an import_url that is valid' do diff --git a/spec/services/import/validate_remote_git_endpoint_service_spec.rb b/spec/services/import/validate_remote_git_endpoint_service_spec.rb index 1d2b3975832..15e80f2c85d 100644 --- a/spec/services/import/validate_remote_git_endpoint_service_spec.rb +++ b/spec/services/import/validate_remote_git_endpoint_service_spec.rb @@ -7,7 +7,9 @@ RSpec.describe Import::ValidateRemoteGitEndpointService, feature_category: :impo let_it_be(:base_url) { 'http://demo.host/path' } let_it_be(:endpoint_url) { "#{base_url}/info/refs?service=git-upload-pack" } - let_it_be(:error_message) { "#{base_url} is not a valid HTTP Git repository" } + let_it_be(:endpoint_error_message) { "#{base_url} endpoint error:" } + let_it_be(:body_error_message) { described_class::INVALID_BODY_MESSAGE } + let_it_be(:content_type_error_message) { described_class::INVALID_CONTENT_TYPE_MESSAGE } describe '#execute' do let(:valid_response) do @@ -70,13 +72,14 @@ RSpec.describe Import::ValidateRemoteGitEndpointService, feature_category: :impo end it 'reports error when status code is not 200' do - stub_full_request(endpoint_url, method: :get).to_return(valid_response.merge({ status: 301 })) + error_response = { status: 401 } + stub_full_request(endpoint_url, method: :get).to_return(error_response) result = subject.execute expect(result).to be_a(ServiceResponse) expect(result.error?).to be(true) - expect(result.message).to eq(error_message) + expect(result.message).to eq("#{endpoint_error_message} #{error_response[:status]}") end it 'reports error when invalid URL is provided' do @@ -94,27 +97,49 @@ RSpec.describe Import::ValidateRemoteGitEndpointService, feature_category: :impo expect(result).to be_a(ServiceResponse) expect(result.error?).to be(true) - expect(result.message).to eq(error_message) + expect(result.message).to eq(content_type_error_message) end - it 'reports error when body is in invalid format' do + it 'reports error when body is too short' do stub_full_request(endpoint_url, method: :get).to_return(valid_response.merge({ body: 'invalid content' })) result = subject.execute expect(result).to be_a(ServiceResponse) expect(result.error?).to be(true) - expect(result.message).to eq(error_message) + expect(result.message).to eq(body_error_message) + end + + it 'reports error when body is in invalid format' do + stub_full_request(endpoint_url, method: :get).to_return(valid_response.merge({ body: 'invalid long content with no git respons whatshowever' })) + + result = subject.execute + + expect(result).to be_a(ServiceResponse) + expect(result.error?).to be(true) + expect(result.message).to eq(body_error_message) + end + + it 'reports error when http exceptions are raised' do + err = SocketError.new('dummy message') + stub_full_request(endpoint_url, method: :get).to_raise(err) + + result = subject.execute + + expect(result).to be_a(ServiceResponse) + expect(result.error?).to be(true) + expect(result.message).to eq("HTTP #{err.class.name.underscore} error: #{err.message}") end - it 'reports error when exception is raised' do - stub_full_request(endpoint_url, method: :get).to_raise(SocketError.new('dummy message')) + it 'reports error when other exceptions are raised' do + err = StandardError.new('internal dummy message') + stub_full_request(endpoint_url, method: :get).to_raise(err) result = subject.execute expect(result).to be_a(ServiceResponse) expect(result.error?).to be(true) - expect(result.message).to eq(error_message) + expect(result.message).to eq("Internal #{err.class.name.underscore} error: #{err.message}") end end diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb index c02310f019a..c4012e2a98f 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -14,6 +14,7 @@ RSpec.describe Issues::UpdateService, :mailer, feature_category: :team_planning let_it_be(:label3) { create(:label, title: 'c', project: project) } let_it_be(:milestone) { create(:milestone, project: project) } + let(:container) { project } let(:issue) do create( :issue, @@ -49,7 +50,7 @@ RSpec.describe Issues::UpdateService, :mailer, feature_category: :team_planning end def update_issue(opts) - described_class.new(container: project, current_user: user, params: opts).execute(issue) + described_class.new(container: container, current_user: user, params: opts).execute(issue) end it_behaves_like 'issuable update service updating last_edited_at values' do @@ -1006,6 +1007,12 @@ RSpec.describe Issues::UpdateService, :mailer, feature_category: :team_planning it_behaves_like 'keeps issuable labels sorted after update' it_behaves_like 'broadcasting issuable labels updates' + context 'when the issue belongs directly to a group' do + let(:container) { group } + + it_behaves_like 'updating issuable labels' + end + def update_issuable(update_params) update_issue(update_params) end diff --git a/spec/services/projects/participants_service_spec.rb b/spec/services/projects/participants_service_spec.rb index b01e64439ec..692f43eb205 100644 --- a/spec/services/projects/participants_service_spec.rb +++ b/spec/services/projects/participants_service_spec.rb @@ -18,6 +18,14 @@ RSpec.describe Projects::ParticipantsService, feature_category: :groups_and_proj described_class.new(project, user).execute(noteable) end + it 'returns results in correct order' do + group = create(:group).tap { |g| g.add_owner(user) } + + expect(run_service.pluck(:username)).to eq([ + noteable.author.username, 'all', user.username, group.full_path + ]) + end + it 'includes `All Project and Group Members`' do expect(run_service).to include(a_hash_including({ username: "all", name: "All Project and Group Members" })) end @@ -104,6 +112,24 @@ RSpec.describe Projects::ParticipantsService, feature_category: :groups_and_proj expect(group_items.first[:avatar_url]).to eq("/gitlab/uploads/-/system/group/avatar/#{group.id}/dk.png") end end + + context 'with subgroups' do + let(:group_1) { create(:group, path: 'bb') } + let(:group_2) { create(:group, path: 'zz') } + let(:subgroup) { create(:group, path: 'aa', parent: group_1) } + + before do + group_1.add_owner(user) + group_2.add_owner(user) + subgroup.add_owner(user) + end + + it 'returns results ordered by full path' do + expect(group_items.pluck(:username)).to eq([ + group_1.full_path, subgroup.full_path, group_2.full_path + ]) + end + end end context 'when `disable_all_mention` FF is enabled' do diff --git a/spec/support/db_cleaner.rb b/spec/support/db_cleaner.rb index 3131a22a20b..a1579ad1685 100644 --- a/spec/support/db_cleaner.rb +++ b/spec/support/db_cleaner.rb @@ -12,7 +12,10 @@ module DbCleaner end def deletion_except_tables - %w[work_item_types work_item_hierarchy_restrictions work_item_widget_definitions] + %w[ + work_item_types work_item_hierarchy_restrictions + work_item_widget_definitions work_item_related_link_restrictions + ] end def setup_database_cleaner diff --git a/spec/support/helpers/test_env.rb b/spec/support/helpers/test_env.rb index fb663011e8f..740abdb6cfa 100644 --- a/spec/support/helpers/test_env.rb +++ b/spec/support/helpers/test_env.rb @@ -394,6 +394,8 @@ module TestEnv end def seed_db + # Adjust `deletion_except_tables` method to exclude seeded tables from + # record deletions. Gitlab::DatabaseImporters::WorkItems::BaseTypeImporter.upsert_types Gitlab::DatabaseImporters::WorkItems::HierarchyRestrictionsImporter.upsert_restrictions Gitlab::DatabaseImporters::WorkItems::RelatedLinksRestrictionsImporter.upsert_restrictions diff --git a/spec/workers/issuable/related_links_create_worker_spec.rb b/spec/workers/issuable/related_links_create_worker_spec.rb index 46c5e980d93..d2d0aeb7081 100644 --- a/spec/workers/issuable/related_links_create_worker_spec.rb +++ b/spec/workers/issuable/related_links_create_worker_spec.rb @@ -18,7 +18,7 @@ RSpec.describe Issuable::RelatedLinksCreateWorker, feature_category: :portfolio_ link_ids: [link1.id, link2.id], link_type: 'relates_to', user_id: user.id - } + }.transform_keys(&:to_s) end before_all do @@ -54,28 +54,28 @@ RSpec.describe Issuable::RelatedLinksCreateWorker, feature_category: :portfolio_ context 'when params contain errors' do it 'does nothing when user is not found' do - params[:user_id] = non_existing_record_id + params['user_id'] = non_existing_record_id expect(Sidekiq.logger).not_to receive(:error) expect { subject }.not_to change { Note.count } end it 'does nothing when issuable is not found' do - params[:issuable_id] = non_existing_record_id + params['issuable_id'] = non_existing_record_id expect(Sidekiq.logger).not_to receive(:error) expect { subject }.not_to change { Note.count } end it 'does nothing when links are not found' do - params[:link_ids] = [non_existing_record_id] + params['link_ids'] = [non_existing_record_id] expect(Sidekiq.logger).not_to receive(:error) expect { subject }.not_to change { Note.count } end it 'logs error when issuable_class is invalid' do - params[:issuable_class] = 'FooBar' + params['issuable_class'] = 'FooBar' expect(Sidekiq.logger).to receive(:error).with({ worker: described_class.to_s, diff --git a/yarn.lock b/yarn.lock index 432e21f0731..82b320a5b4e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1260,10 +1260,10 @@ resolved "https://registry.yarnpkg.com/@gitlab/fonts/-/fonts-1.3.0.tgz#df89c1bb6714e4a8a5d3272568aa4de7fb337267" integrity sha512-DoMUIN3DqjEn7wvcxBg/b7Ite5fTdF5EmuOZoBRo2j0UBGweDXmNBi+9HrTZs4cBU660dOxcf1hATFcG3npbPg== -"@gitlab/stylelint-config@5.0.0": - version "5.0.0" - resolved "https://registry.yarnpkg.com/@gitlab/stylelint-config/-/stylelint-config-5.0.0.tgz#1f644bcf2cf11d77a77bfbe06c7ac79519684580" - integrity sha512-uNMAJkH4C33XfBZDVAZTluaTOU9vEQ6uwUiKBgTsklH3bDPw39jiNvHOwTib956NiY74OwXU3fL2poMFdDgKcg== +"@gitlab/stylelint-config@5.0.1": + version "5.0.1" + resolved "https://registry.yarnpkg.com/@gitlab/stylelint-config/-/stylelint-config-5.0.1.tgz#fba9fb855865c6a3c58f9b0e49656619f01e18e3" + integrity sha512-MftcTHvhPVKNzubCQk8gO33Ps1iwJywqBryJJLPCPlLQq1YY3wUF1Z5x6CVKqvoe2IU/+/Puf4LMsbHp9VFPYg== dependencies: postcss-scss "4.0.7" stylelint-declaration-strict-value "1.9.2" |