From e374f6b2297582fde956350e92c19d1ae93ddfc8 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Fri, 9 Jun 2023 21:09:51 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- app/controllers/graphql_controller.rb | 6 + .../use_pipeline_over_multikey.yml | 8 - .../004_zeitwerk.rb | 1 + ...ode_review_total_unique_counts_monthly.yml | 1 + ...xtension_category_monthly_active_users.yml | 1 + ...code_review_group_monthly_active_users.yml | 1 + ..._jetbrains_bundled_api_request_monthly.yml | 25 +++ ...code_review_total_unique_counts_weekly.yml | 1 + ...code_review_group_monthly_active_users.yml | 1 + ...xtension_category_monthly_active_users.yml | 1 + ...r_jetbrains_bundled_api_request_weekly.yml | 25 +++ ...egistrations_u2f_registration_id_column.rb | 21 ++ db/schema_migrations/20230607093222 | 1 + db/structure.sql | 3 - doc/user/discussions/index.md | 18 ++ .../project/merge_requests/approvals/index.md | 6 +- lib/api/api.rb | 4 + lib/gitlab/avatar_cache.rb | 2 +- .../discussions_diff/highlight_cache.rb | 4 +- lib/gitlab/reactive_cache_set_cache.rb | 3 +- lib/gitlab/set_cache.rb | 2 +- ..._bundled_plugin_activity_unique_counter.rb | 29 +++ .../known_events/code_review_events.yml | 2 + locale/gitlab.pot | 2 +- qa/qa/page/project/web_ide/vscode.rb | 177 +++++++++++++--- .../add_new_directory_in_web_ide_spec.rb | 10 +- .../upload_new_file_in_web_ide_spec.rb | 71 +++++++ .../metrics/every_metric_definition_spec.rb | 38 ++++ spec/controllers/graphql_controller_spec.rb | 20 ++ spec/lib/gitlab/avatar_cache_spec.rb | 72 +++---- .../discussions_diff/highlight_cache_spec.rb | 96 ++++----- .../gitlab/reactive_cache_set_cache_spec.rb | 34 ++- spec/lib/gitlab/repository_set_cache_spec.rb | 90 ++++---- ...led_plugin_activity_unique_counter_spec.rb | 19 ++ spec/services/notification_service_spec.rb | 194 ++++++++++++------ ...every_metric_definition_shared_examples.rb | 161 +++++++++++++++ 36 files changed, 857 insertions(+), 293 deletions(-) delete mode 100644 config/feature_flags/development/use_pipeline_over_multikey.yml create mode 100644 config/metrics/counts_28d/20230607203257_i_user_jetbrains_bundled_api_request_monthly.yml create mode 100644 config/metrics/counts_7d/20230607203256_i_user_jetbrains_bundled_api_request_weekly.yml create mode 100644 db/post_migrate/20230607093222_remove_webauthn_registrations_u2f_registration_id_column.rb create mode 100644 db/schema_migrations/20230607093222 create mode 100644 lib/gitlab/usage_data_counters/jetbrains_bundled_plugin_activity_unique_counter.rb create mode 100644 qa/qa/specs/features/browser_ui/3_create/web_ide/upload_new_file_in_web_ide_spec.rb create mode 100644 spec/config/metrics/every_metric_definition_spec.rb create mode 100644 spec/lib/gitlab/usage_data_counters/jetbrains_bundled_plugin_activity_unique_counter_spec.rb create mode 100644 spec/support/shared_examples/config/metrics/every_metric_definition_shared_examples.rb diff --git a/app/controllers/graphql_controller.rb b/app/controllers/graphql_controller.rb index f8967cf023f..0d348624864 100644 --- a/app/controllers/graphql_controller.rb +++ b/app/controllers/graphql_controller.rb @@ -35,6 +35,7 @@ class GraphqlController < ApplicationController before_action :set_user_last_activity before_action :track_vs_code_usage before_action :track_jetbrains_usage + before_action :track_jetbrains_bundled_usage before_action :track_gitlab_cli_usage before_action :disable_query_limiting before_action :limit_query_size @@ -177,6 +178,11 @@ class GraphqlController < ApplicationController .track_api_request_when_trackable(user_agent: request.user_agent, user: current_user) end + def track_jetbrains_bundled_usage + Gitlab::UsageDataCounters::JetBrainsBundledPluginActivityUniqueCounter + .track_api_request_when_trackable(user_agent: request.user_agent, user: current_user) + end + def track_gitlab_cli_usage Gitlab::UsageDataCounters::GitLabCliActivityUniqueCounter .track_api_request_when_trackable(user_agent: request.user_agent, user: current_user) diff --git a/config/feature_flags/development/use_pipeline_over_multikey.yml b/config/feature_flags/development/use_pipeline_over_multikey.yml deleted file mode 100644 index 35a84868fe4..00000000000 --- a/config/feature_flags/development/use_pipeline_over_multikey.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: use_pipeline_over_multikey -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/118884 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/409436 -milestone: '16.0' -type: development -group: group::scalability -default_enabled: false diff --git a/config/initializers_before_autoloader/004_zeitwerk.rb b/config/initializers_before_autoloader/004_zeitwerk.rb index 72e471d25f2..2d54ab87dca 100644 --- a/config/initializers_before_autoloader/004_zeitwerk.rb +++ b/config/initializers_before_autoloader/004_zeitwerk.rb @@ -41,6 +41,7 @@ Rails.autoloaders.each do |autoloader| 'chunked_io' => 'ChunkedIO', 'http_io' => 'HttpIO', 'jetbrains_plugin_activity_unique_counter' => 'JetBrainsPluginActivityUniqueCounter', + 'jetbrains_bundled_plugin_activity_unique_counter' => 'JetBrainsBundledPluginActivityUniqueCounter', 'json_formatter' => 'JSONFormatter', 'json_web_token' => 'JSONWebToken', 'as_json' => 'AsJSON', diff --git a/config/metrics/counts_28d/20210216184454_code_review_total_unique_counts_monthly.yml b/config/metrics/counts_28d/20210216184454_code_review_total_unique_counts_monthly.yml index 9f1c46764f3..ea392c605a0 100644 --- a/config/metrics/counts_28d/20210216184454_code_review_total_unique_counts_monthly.yml +++ b/config/metrics/counts_28d/20210216184454_code_review_total_unique_counts_monthly.yml @@ -107,6 +107,7 @@ options: - i_code_review_user_edit_multiline_mr_comment - i_code_review_user_gitlab_cli_api_request - i_code_review_user_jetbrains_api_request + - i_editor_extensions_user_jetbrains_bundled_api_request - i_code_review_user_labels_changed - i_code_review_user_load_conflict_ui - i_code_review_user_marked_as_draft diff --git a/config/metrics/counts_28d/20210427103010_code_review_extension_category_monthly_active_users.yml b/config/metrics/counts_28d/20210427103010_code_review_extension_category_monthly_active_users.yml index f5ab886d5cf..3d832f1fc18 100644 --- a/config/metrics/counts_28d/20210427103010_code_review_extension_category_monthly_active_users.yml +++ b/config/metrics/counts_28d/20210427103010_code_review_extension_category_monthly_active_users.yml @@ -19,6 +19,7 @@ options: events: - 'i_code_review_user_vs_code_api_request' - 'i_code_review_user_jetbrains_api_request' + - 'i_editor_extensions_user_jetbrains_bundled_api_request' - 'i_code_review_user_gitlab_cli_api_request' distribution: - ce diff --git a/config/metrics/counts_28d/20210427103119_code_review_group_monthly_active_users.yml b/config/metrics/counts_28d/20210427103119_code_review_group_monthly_active_users.yml index 16e077875d4..63cd37460b2 100644 --- a/config/metrics/counts_28d/20210427103119_code_review_group_monthly_active_users.yml +++ b/config/metrics/counts_28d/20210427103119_code_review_group_monthly_active_users.yml @@ -87,6 +87,7 @@ options: - 'i_code_review_post_merge_submit_revert_modal' - 'i_code_review_post_merge_submit_cherry_pick_modal' - 'i_code_review_user_jetbrains_api_request' + - 'i_editor_extensions_user_jetbrains_bundled_api_request' - 'i_code_review_user_gitlab_cli_api_request' - 'i_code_review_user_create_note_in_ipynb_diff' - 'i_code_review_user_create_note_in_ipynb_diff_mr' diff --git a/config/metrics/counts_28d/20230607203257_i_user_jetbrains_bundled_api_request_monthly.yml b/config/metrics/counts_28d/20230607203257_i_user_jetbrains_bundled_api_request_monthly.yml new file mode 100644 index 00000000000..ac3a69c8388 --- /dev/null +++ b/config/metrics/counts_28d/20230607203257_i_user_jetbrains_bundled_api_request_monthly.yml @@ -0,0 +1,25 @@ +--- +key_path: redis_hll_counters.code_review.i_user_jetbrains_bundled_api_request_monthly +description: Count of unique users per month who use the bundled GitLab plugin in JetBrains IDEs +product_section: dev +product_stage: create +product_group: code_review +value_type: number +status: active +milestone: "16.1" +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/122955 +time_frame: 28d +data_source: redis_hll +data_category: optional +instrumentation_class: RedisHLLMetric +options: + events: + - i_editor_extensions_user_jetbrains_bundled_api_request +performance_indicator_type: [] +distribution: +- ce +- ee +tier: +- free +- premium +- ultimate diff --git a/config/metrics/counts_7d/20210216184452_code_review_total_unique_counts_weekly.yml b/config/metrics/counts_7d/20210216184452_code_review_total_unique_counts_weekly.yml index 154806d8d70..223f0a74b1e 100644 --- a/config/metrics/counts_7d/20210216184452_code_review_total_unique_counts_weekly.yml +++ b/config/metrics/counts_7d/20210216184452_code_review_total_unique_counts_weekly.yml @@ -107,6 +107,7 @@ options: - i_code_review_user_edit_multiline_mr_comment - i_code_review_user_gitlab_cli_api_request - i_code_review_user_jetbrains_api_request + - i_editor_extensions_user_jetbrains_bundled_api_request - i_code_review_user_labels_changed - i_code_review_user_load_conflict_ui - i_code_review_user_marked_as_draft diff --git a/config/metrics/counts_7d/20210427103328_code_review_group_monthly_active_users.yml b/config/metrics/counts_7d/20210427103328_code_review_group_monthly_active_users.yml index 7743cfd2e0c..18674b5db27 100644 --- a/config/metrics/counts_7d/20210427103328_code_review_group_monthly_active_users.yml +++ b/config/metrics/counts_7d/20210427103328_code_review_group_monthly_active_users.yml @@ -85,6 +85,7 @@ options: - 'i_code_review_post_merge_submit_revert_modal' - 'i_code_review_post_merge_submit_cherry_pick_modal' - 'i_code_review_user_jetbrains_api_request' + - 'i_editor_extensions_user_jetbrains_bundled_api_request' - 'i_code_review_user_gitlab_cli_api_request' - 'i_code_review_user_create_note_in_ipynb_diff' - 'i_code_review_user_create_note_in_ipynb_diff_mr' diff --git a/config/metrics/counts_7d/20210427103452_code_review_extension_category_monthly_active_users.yml b/config/metrics/counts_7d/20210427103452_code_review_extension_category_monthly_active_users.yml index 2229e132eb4..b211ed2d8c8 100644 --- a/config/metrics/counts_7d/20210427103452_code_review_extension_category_monthly_active_users.yml +++ b/config/metrics/counts_7d/20210427103452_code_review_extension_category_monthly_active_users.yml @@ -19,6 +19,7 @@ options: events: - 'i_code_review_user_vs_code_api_request' - 'i_code_review_user_jetbrains_api_request' + - 'i_editor_extensions_user_jetbrains_bundled_api_request' - 'i_code_review_user_gitlab_cli_api_request' distribution: - ce diff --git a/config/metrics/counts_7d/20230607203256_i_user_jetbrains_bundled_api_request_weekly.yml b/config/metrics/counts_7d/20230607203256_i_user_jetbrains_bundled_api_request_weekly.yml new file mode 100644 index 00000000000..237190e386d --- /dev/null +++ b/config/metrics/counts_7d/20230607203256_i_user_jetbrains_bundled_api_request_weekly.yml @@ -0,0 +1,25 @@ +--- +key_path: redis_hll_counters.code_review.i_user_jetbrains_bundled_api_request_weekly +description: Count of unique users per week who use the bundled GitLab plugin in JetBrains IDEs +product_section: dev +product_stage: create +product_group: code_review +value_type: number +status: active +milestone: "16.1" +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/122955 +time_frame: 7d +data_source: redis_hll +data_category: optional +instrumentation_class: RedisHLLMetric +options: + events: + - i_editor_extensions_user_jetbrains_bundled_api_request +performance_indicator_type: [] +distribution: +- ce +- ee +tier: +- free +- premium +- ultimate diff --git a/db/post_migrate/20230607093222_remove_webauthn_registrations_u2f_registration_id_column.rb b/db/post_migrate/20230607093222_remove_webauthn_registrations_u2f_registration_id_column.rb new file mode 100644 index 00000000000..6e861fc258a --- /dev/null +++ b/db/post_migrate/20230607093222_remove_webauthn_registrations_u2f_registration_id_column.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +class RemoveWebauthnRegistrationsU2fRegistrationIdColumn < Gitlab::Database::Migration[2.1] + disable_ddl_transaction! + + INDEX_NAME = 'index_webauthn_registrations_on_u2f_registration_id' + + def up + remove_column :webauthn_registrations, :u2f_registration_id + end + + def down + add_column :webauthn_registrations, :u2f_registration_id, :integer + + add_concurrent_index( + :webauthn_registrations, + :u2f_registration_id, + name: INDEX_NAME, + where: 'u2f_registration_id IS NOT NULL') + end +end diff --git a/db/schema_migrations/20230607093222 b/db/schema_migrations/20230607093222 new file mode 100644 index 00000000000..94230685b2c --- /dev/null +++ b/db/schema_migrations/20230607093222 @@ -0,0 +1 @@ +f2357daf5c6e21e2695987894aec547b44bb792b087dd0a46eee0b51f66285fe \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index a220ee97b84..932fb53f050 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -24579,7 +24579,6 @@ CREATE TABLE webauthn_registrations ( credential_xid text NOT NULL, name text NOT NULL, public_key text NOT NULL, - u2f_registration_id integer, CONSTRAINT check_2f02e74321 CHECK ((char_length(name) <= 255)), CONSTRAINT check_f5ab2b551a CHECK ((char_length(credential_xid) <= 1364)) ); @@ -33377,8 +33376,6 @@ CREATE INDEX index_web_hooks_on_type ON web_hooks USING btree (type); CREATE UNIQUE INDEX index_webauthn_registrations_on_credential_xid ON webauthn_registrations USING btree (credential_xid); -CREATE INDEX index_webauthn_registrations_on_u2f_registration_id ON webauthn_registrations USING btree (u2f_registration_id) WHERE (u2f_registration_id IS NOT NULL); - CREATE INDEX index_webauthn_registrations_on_user_id ON webauthn_registrations USING btree (user_id); CREATE INDEX index_wiki_page_meta_on_project_id ON wiki_page_meta USING btree (project_id); diff --git a/doc/user/discussions/index.md b/doc/user/discussions/index.md index 096b4dc2cc5..77b585c17db 100644 --- a/doc/user/discussions/index.md +++ b/doc/user/discussions/index.md @@ -95,6 +95,24 @@ it's converted to a link in the context of the merge request. For example, `28719b171a056960dfdc0012b625d0b47b123196` becomes `28719b17` that links to `https://gitlab.example.com/example-group/example-project/-/merge_requests/12345/diffs?commit_id=28719b171a056960dfdc0012b625d0b47b123196`. +## Add a comment to a merge request file + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/121429) in GitLab 16.1 [with a flag](../../administration/feature_flags.md) named `comment_on_files`. Disabled by default. + +FLAG: +On self-managed GitLab, by default this feature is not available. To make it available, ask an administrator to [enable the feature flag](../../administration/feature_flags.md) named `comment_on_files`. +On GitLab.com, this feature is not available. + +You can add comments to a merge request diff file. These comments persist across +rebases and file changes. + +To add a comment a merge request file: + +1. On the top bar, select **Main menu > Projects** and find your project. +1. On the left sidebar, select **Settings > Merge requests** and find your merge request. +1. Select **Changes**. +1. In the header for the file you want to comment on, select **Comment** (**{comment}**). + ## Add a comment to a commit You can add comments and threads to a particular commit. diff --git a/doc/user/project/merge_requests/approvals/index.md b/doc/user/project/merge_requests/approvals/index.md index 717358df9f3..8c1f09387ce 100644 --- a/doc/user/project/merge_requests/approvals/index.md +++ b/doc/user/project/merge_requests/approvals/index.md @@ -109,11 +109,11 @@ Without the approvals, the work cannot merge. Required approvals enable multiple > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/334698) in GitLab 15.1. > - [Changed](https://gitlab.com/gitlab-org/gitlab/-/issues/389905) in GitLab 15.11 [with a flag](../../../../administration/feature_flags.md) named `invalid_scan_result_policy_prevents_merge`. Disabled by default. +> - [Enabled by default on GitLab.com and self-managed](https://gitlab.com/gitlab-org/gitlab/-/issues/405023) in GitLab 16.1. FLAG: -On self-managed GitLab, by default this feature is not available. To make it available per project or for your entire instance, -ask an administrator to [enable the feature flag](../../../../administration/feature_flags.md) named `invalid_scan_result_policy_prevents_merge`. -On GitLab.com, this feature is available but can be configured by GitLab.com administrators only. +On self-managed GitLab, by default this feature is available. To hide the feature, +ask an administrator to [disable the feature flag](../../../../administration/feature_flags.md) named `invalid_scan_result_policy_prevents_merge`. Whenever an approval rule cannot be satisfied, the rule is displayed as **(!) Auto approved**. This applies to the following conditions: diff --git a/lib/api/api.rb b/lib/api/api.rb index 2e3bd1d6892..60a12ee7145 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -90,6 +90,10 @@ module API Gitlab::UsageDataCounters::JetBrainsPluginActivityUniqueCounter.track_api_request_when_trackable(user_agent: request&.user_agent, user: @current_user) end + after do + Gitlab::UsageDataCounters::JetBrainsBundledPluginActivityUniqueCounter.track_api_request_when_trackable(user_agent: request&.user_agent, user: @current_user) + end + after do Gitlab::UsageDataCounters::GitLabCliActivityUniqueCounter.track_api_request_when_trackable(user_agent: request&.user_agent, user: @current_user) end diff --git a/lib/gitlab/avatar_cache.rb b/lib/gitlab/avatar_cache.rb index c220acda80b..f4dcd6f7910 100644 --- a/lib/gitlab/avatar_cache.rb +++ b/lib/gitlab/avatar_cache.rb @@ -65,7 +65,7 @@ module Gitlab keys = emails.map { |email| email_key(email) } Gitlab::Instrumentation::RedisClusterValidator.allow_cross_slot_commands do - if ::Feature.enabled?(:use_pipeline_over_multikey) || Gitlab::Redis::ClusterUtil.cluster?(redis) + if Gitlab::Redis::ClusterUtil.cluster?(redis) Gitlab::Redis::ClusterUtil.batch_unlink(keys, redis) else redis.unlink(*keys) diff --git a/lib/gitlab/discussions_diff/highlight_cache.rb b/lib/gitlab/discussions_diff/highlight_cache.rb index f90698e41c3..18ff7c28e17 100644 --- a/lib/gitlab/discussions_diff/highlight_cache.rb +++ b/lib/gitlab/discussions_diff/highlight_cache.rb @@ -41,7 +41,7 @@ module Gitlab content = with_redis do |redis| Gitlab::Instrumentation::RedisClusterValidator.allow_cross_slot_commands do - if ::Feature.enabled?(:use_pipeline_over_multikey) || Gitlab::Redis::ClusterUtil.cluster?(redis) + if Gitlab::Redis::ClusterUtil.cluster?(redis) Gitlab::Redis::CrossSlot::Pipeline.new(redis).pipelined do |pipeline| keys.each { |key| pipeline.get(key) } end @@ -72,7 +72,7 @@ module Gitlab with_redis do |redis| Gitlab::Instrumentation::RedisClusterValidator.allow_cross_slot_commands do - if ::Feature.enabled?(:use_pipeline_over_multikey) || Gitlab::Redis::ClusterUtil.cluster?(redis) + if Gitlab::Redis::ClusterUtil.cluster?(redis) Gitlab::Redis::ClusterUtil.batch_unlink(keys, redis) else redis.del(keys) diff --git a/lib/gitlab/reactive_cache_set_cache.rb b/lib/gitlab/reactive_cache_set_cache.rb index 7b58f87308e..9f5769a5c97 100644 --- a/lib/gitlab/reactive_cache_set_cache.rb +++ b/lib/gitlab/reactive_cache_set_cache.rb @@ -11,13 +11,12 @@ module Gitlab end def clear_cache!(key) - use_pipeline = ::Feature.enabled?(:use_pipeline_over_multikey) with do |redis| keys = read(key).map { |value| "#{cache_namespace}:#{value}" } keys << cache_key(key) Gitlab::Instrumentation::RedisClusterValidator.allow_cross_slot_commands do - if use_pipeline || Gitlab::Redis::ClusterUtil.cluster?(redis) + if Gitlab::Redis::ClusterUtil.cluster?(redis) Gitlab::Redis::ClusterUtil.batch_unlink(keys, redis) else redis.pipelined do |pipeline| diff --git a/lib/gitlab/set_cache.rb b/lib/gitlab/set_cache.rb index 8e9de58f31d..eb73a0a3d31 100644 --- a/lib/gitlab/set_cache.rb +++ b/lib/gitlab/set_cache.rb @@ -22,7 +22,7 @@ module Gitlab keys_to_expire = keys.map { |key| cache_key(key) } Gitlab::Instrumentation::RedisClusterValidator.allow_cross_slot_commands do - if ::Feature.enabled?(:use_pipeline_over_multikey) || Gitlab::Redis::ClusterUtil.cluster?(redis) + if Gitlab::Redis::ClusterUtil.cluster?(redis) Gitlab::Redis::ClusterUtil.batch_unlink(keys_to_expire, redis) else redis.unlink(*keys_to_expire) diff --git a/lib/gitlab/usage_data_counters/jetbrains_bundled_plugin_activity_unique_counter.rb b/lib/gitlab/usage_data_counters/jetbrains_bundled_plugin_activity_unique_counter.rb new file mode 100644 index 00000000000..a9e8d9bf0cb --- /dev/null +++ b/lib/gitlab/usage_data_counters/jetbrains_bundled_plugin_activity_unique_counter.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +module Gitlab + module UsageDataCounters + module JetBrainsBundledPluginActivityUniqueCounter + JETBRAINS_BUNDLED_API_REQUEST_ACTION = 'i_editor_extensions_user_jetbrains_bundled_api_request' + JETBRAINS_BUNDLED_USER_AGENT_REGEX = /\AIntelliJ-GitLab-Plugin/ + + class << self + def track_api_request_when_trackable(user_agent:, user:) + user_agent&.match?(JETBRAINS_BUNDLED_USER_AGENT_REGEX) && + track_unique_action_by_user(JETBRAINS_BUNDLED_API_REQUEST_ACTION, user) + end + + private + + def track_unique_action_by_user(action, user) + return unless user + + track_unique_action(action, user.id) + end + + def track_unique_action(action, value) + Gitlab::UsageDataCounters::HLLRedisCounter.track_usage_event(action, value) + end + end + end + end +end diff --git a/lib/gitlab/usage_data_counters/known_events/code_review_events.yml b/lib/gitlab/usage_data_counters/known_events/code_review_events.yml index db0c0653f63..bd8c79f4801 100644 --- a/lib/gitlab/usage_data_counters/known_events/code_review_events.yml +++ b/lib/gitlab/usage_data_counters/known_events/code_review_events.yml @@ -79,6 +79,8 @@ aggregation: weekly - name: i_code_review_user_jetbrains_api_request aggregation: weekly +- name: i_editor_extensions_user_jetbrains_bundled_api_request + aggregation: weekly - name: i_code_review_user_gitlab_cli_api_request aggregation: weekly - name: i_code_review_user_create_mr_from_issue diff --git a/locale/gitlab.pot b/locale/gitlab.pot index d38db8931dd..ed511e9f43d 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -47618,7 +47618,7 @@ msgstr "" msgid "To remove the %{link_start}read-only%{link_end} state and regain write access, ask your top-level group owner(s) to reduce the number of users in your top-level group to %{free_limit} users or less, or to upgrade to a paid tier which do not have user limits." msgstr "" -msgid "To remove the %{link_start}read-only%{link_end} state and regain write access, you can reduce the number of users in your top-level group to %{free_limit} users or less. You can also upgrade to a paid tier, which do not have user limits. If you need additional time, you can start a free 30-day trial which includes unlimited users. To minimize the impact to operations, for a limited time, GitLab is offering a %{promotion_link_start}one-time 70 percent discount%{link_end} off the list price at time of purchase for a new, one year subscription of GitLab Premium SaaS to %{offer_availability_link_start}qualifying top-level groups%{link_end}. The offer is valid until 2023-08-13." +msgid "To remove the %{link_start}read-only%{link_end} state and regain write access, you can reduce the number of users in your top-level group to %{free_limit} users or less. You can also upgrade to a paid tier, which do not have user limits. If you need additional time, you can start a free 30-day trial which includes unlimited users. To minimize the impact to operations, for a limited time, GitLab is offering a %{promotion_link_start}one-time 70 percent discount%{link_end} off the list price at time of purchase for a new, one year subscription of GitLab Premium SaaS to %{offer_availability_link_start}qualifying top-level groups%{link_end}. The offer is valid until %{offer_date}." msgstr "" msgid "To resolve this, try to:" diff --git a/qa/qa/page/project/web_ide/vscode.rb b/qa/qa/page/project/web_ide/vscode.rb index 1a9fad56a10..6a570978dbe 100644 --- a/qa/qa/page/project/web_ide/vscode.rb +++ b/qa/qa/page/project/web_ide/vscode.rb @@ -1,64 +1,183 @@ # frozen_string_literal: true -# VSCode WebIDE is built off an iFrame application therefore we are uanble to use `qa-selectors` +# VSCode WebIDE is built off an iFrame application therefore we are unable to use `qa-selectors` module QA module Page module Project module WebIDE class VSCode < Page::Base - # Use to Pass Test::Sanity::Selectors temporarily until iframe [data-qa-* selector added view 'app/views/shared/_broadcast_message.html.haml' do element :broadcast_notification_container element :close_button end - # Used for stablility, due to feature_caching of vscode_web_ide - def wait_for_ide_to_load - page.driver.browser.switch_to.window(page.driver.browser.window_handles.last) - wait_for_requests - Support::Waiter.wait_until(max_duration: 60, reload_page: page, retry_on_exception: true) do + def has_file_explorer? + page.has_css?('.explorer-folders-view', visible: true) + end + + def right_click_file_explorer + page.find('.explorer-folders-view', visible: true).right_click + end + + def has_new_folder_menu_item? + page.has_css?('[aria-label="New Folder..."]', visible: true) + end + + def click_new_folder_menu_item + page.find('[aria-label="New Folder..."]').click + end + + def enter_new_folder_text_input(name) + page.find('.explorer-item-edited', visible: true) + send_keys(name, :enter) + end + + def has_upload_menu_item? + page.has_css?('[aria-label="Upload..."]', visible: true) + end + + def click_upload_menu_item + page.find('[aria-label="Upload..."]').click + end + + def enter_file_input(file) + page.find('input[type="file"]', visible: false).send_keys(file) + end + + def has_commit_pending_tab? + page.has_css?('.scm-viewlet-label', visible: true) + end + + def click_commit_pending_tab + page.find('.scm-viewlet-label', visible: true).click + end + + def click_commit_tab + page.find('a.codicon-source-control-view-icon', visible: true).click + end + + def has_commit_message_box? + page.has_css?('div.view-lines.monaco-mouse-cursor-text', visible: true) + end + + def enter_commit_message(message) + page.find('div.view-lines.monaco-mouse-cursor-text', visible: true).send_keys(message) + end + + def click_commit_button + page.find('a.monaco-text-button', visible: true).click + end + + def has_notification_box? + page.has_css?('a.monaco-text-button', visible: true) + end + + def create_merge_request + Support::Waiter.wait_until(max_duration: 10, retry_on_exception: true) do within_vscode_editor do - # vscode file_explorer element - page.has_css?('.explorer-folders-view', visible: true) + page.find('.monaco-button[title="Create MR"]').click end end end + def click_new_branch + page.find('.monaco-button[title="Create new branch"]').click + end + + def has_branch_input_field? + page.has_css?('.monaco-findInput', visible: true) + end + + def has_message?(content) + within_vscode_editor do + page.has_content?(content) + end + end + def within_vscode_editor(&block) iframe = find('#ide iframe') page.within_frame(iframe, &block) end + # Used for stablility, due to feature_caching of vscode_web_ide + def wait_for_ide_to_load + page.driver.browser.switch_to.window(page.driver.browser.window_handles.last) + # On test environments we have a broadcast message that can cover the buttons + if has_element?(:broadcast_notification_container, wait: 5) + within_element(:broadcast_notification_container) do + click_element(:close_button) + end + end + + wait_for_requests + Support::Waiter.wait_until(max_duration: 10, reload_page: page, retry_on_exception: true) do + within_vscode_editor do + # Check for webide file_explorer element + has_file_explorer? + end + end + end + def create_new_folder(name) within_vscode_editor do - # Use for stability, WebIDE inside an iframe is finnicky - Support::Waiter.wait_until(max_duration: 60, retry_on_exception: true) do - page.find('.explorer-folders-view').right_click - # new_folder_button - page.has_css?('[aria-label="New Folder..."]', visible: true) - end + right_click_file_explorer + has_new_folder_menu_item? - # Additonal wait for stability, webdriver sometimes moves too fast - Support::Waiter.wait_until(max_duration: 60, retry_on_exception: true) do - page.find('[aria-label="New Folder..."]').click + # Use for stability, WebIDE inside an iframe is finnicky, webdriver sometimes moves too fast + Support::Waiter.wait_until(max_duration: 20, retry_on_exception: true) do + click_new_folder_menu_item # Verify New Folder button is triggered and textbox is waiting for input - page.find('.explorer-item-edited', visible: true) - send_keys(name, :enter) + enter_new_folder_text_input(name) page.has_content?(name) end end end - def commit_and_push(folder_name) - within_vscode_editor do - # Commit Tab - page.find('a.codicon-source-control-view-icon').click - send_keys(folder_name) - page.has_content?(folder_name) + def commit_and_push(file_name) + commit_toggle(file_name) + push_to_new_branch + end - # Commit Button - page.find('a.monaco-text-button').click - page.has_css?('.notification-list-item-details-row', visible: true) + def commit_toggle(message) + within_vscode_editor do + if has_commit_pending_tab? + click_commit_pending_tab + else + click_commit_tab + end + + has_commit_message_box? + send_keys(message) + page.has_content?(message) + click_commit_button + has_notification_box? + end + end + + def push_to_new_branch + within_vscode_editor do + click_new_branch + has_branch_input_field? + # Typing enter to 'New branch name' popup to take the default branch name + send_keys(:enter) + end + end + + def upload_file(file_path) + wait_for_ide_to_load + within_vscode_editor do + # VSCode eagerly removes the input[type='file'] from click on Upload. + # We need to execute a script on the iframe to stub out the iframes body.removeChild to add it back in. + page.execute_script("document.body.removeChild = function(){};") + + right_click_file_explorer + has_upload_menu_item? + + # Use for stability, WebIDE inside an iframe is finnicky, webdriver sometimes moves too fast + Support::Waiter.wait_until(max_duration: 20, retry_on_exception: true) do + click_upload_menu_item + enter_file_input(file_path) + end end end end diff --git a/qa/qa/specs/features/browser_ui/3_create/web_ide/add_new_directory_in_web_ide_spec.rb b/qa/qa/specs/features/browser_ui/3_create/web_ide/add_new_directory_in_web_ide_spec.rb index af3d064a37c..3e8f9307cec 100644 --- a/qa/qa/specs/features/browser_ui/3_create/web_ide/add_new_directory_in_web_ide_spec.rb +++ b/qa/qa/specs/features/browser_ui/3_create/web_ide/add_new_directory_in_web_ide_spec.rb @@ -42,9 +42,7 @@ module QA Page::Project::WebIDE::VSCode.perform do |ide| ide.wait_for_ide_to_load ide.create_new_folder(directory_name) - ide.within_vscode_editor do - expect(page).to have_content('A file or folder first_directory already exists at this location.') - end + ide.has_message?('A file or folder first_directory already exists at this location.') end end end @@ -61,10 +59,8 @@ module QA Page::Project::WebIDE::VSCode.perform do |ide| ide.wait_for_ide_to_load ide.create_new_folder(directory_name) - ide.commit_and_push(directory_name) - ide.within_vscode_editor do - expect(page).to have_content('No changes found. Not able to commit.') - end + ide.commit_toggle(directory_name) + ide.has_message?('No changes found. Not able to commit.') end end end diff --git a/qa/qa/specs/features/browser_ui/3_create/web_ide/upload_new_file_in_web_ide_spec.rb b/qa/qa/specs/features/browser_ui/3_create/web_ide/upload_new_file_in_web_ide_spec.rb new file mode 100644 index 00000000000..336d32bcc35 --- /dev/null +++ b/qa/qa/specs/features/browser_ui/3_create/web_ide/upload_new_file_in_web_ide_spec.rb @@ -0,0 +1,71 @@ +# frozen_string_literal: true + +module QA + RSpec.describe 'Create', product_group: :ide, + quarantine: { + only: { job: 'slow-network' }, + issue: 'https://gitlab.com/gitlab-org/gitlab/-/issues/387609', + type: :flaky + } do + describe 'Upload a file in Web IDE' do + let(:file_path) { File.absolute_path(File.join('qa', 'fixtures', 'web_ide', file_name)) } + + let(:project) do + Resource::Project.fabricate_via_api! do |project| + project.name = 'upload-file-project' + project.initialize_with_readme = true + end + end + + before do + Flow::Login.sign_in + project.visit! + end + + context 'when a file with the same name already exists' do + let(:file_name) { 'README.md' } + + it 'throws an error', testcase: 'https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/390005' do + Page::Project::Show.perform(&:open_web_ide!) + Page::Project::WebIDE::VSCode.perform do |ide| + ide.upload_file(file_path) + ide.has_message?("A file or folder with the name 'README.md' already exists in the destination folder") + end + end + end + + shared_examples 'upload a file' do + it "verifies it successfully uploads and commits to a MR" do + Page::Project::Show.perform(&:open_web_ide!) + Page::Project::WebIDE::VSCode.perform do |ide| + ide.upload_file(file_path) + ide.has_message?(file_name) + ide.commit_and_push(file_name) + ide.has_message?('Success! Your changes have been committed.') + ide.create_merge_request + end + + # Opens the MR in new tab and verify the file is in the MR + page.driver.browser.switch_to.window(page.driver.browser.window_handles.last) + Page::MergeRequest::Show.perform do |merge_request| + expect(merge_request).to have_content(file_name) + end + end + end + + context 'when the file is a text file', + testcase: 'https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/390006' do + let(:file_name) { 'text_file.txt' } + + it_behaves_like 'upload a file' + end + + context 'when the file is an image', + testcase: 'https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/390007' do + let(:file_name) { 'dk.png' } + + it_behaves_like 'upload a file' + end + end + end +end diff --git a/spec/config/metrics/every_metric_definition_spec.rb b/spec/config/metrics/every_metric_definition_spec.rb new file mode 100644 index 00000000000..7492d6cdae5 --- /dev/null +++ b/spec/config/metrics/every_metric_definition_spec.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Every metric definition', feature_category: :service_ping, unless: Gitlab.ee? do + include_examples "every metric definition" do + let(:ce_key_paths_mistakenly_defined_in_ee) do + %w[ + counts.assignee_lists + counts.milestone_lists + counts.projects_with_repositories_enabled + counts.protected_branches + ].freeze + end + + let(:ee_key_paths_mistakenly_defined_in_ce) do + %w[ + counts.operations_dashboard_default_dashboard + counts.operations_dashboard_users_with_projects_added + usage_activity_by_stage.create.projects_imported_from_github + usage_activity_by_stage.monitor.operations_dashboard_users_with_projects_added + usage_activity_by_stage.plan.epics + usage_activity_by_stage.plan.label_lists + usage_activity_by_stage_monthly.create.projects_imported_from_github + usage_activity_by_stage_monthly.create.protected_branches + usage_activity_by_stage_monthly.monitor.operations_dashboard_users_with_projects_added + usage_activity_by_stage_monthly.plan.epics + usage_activity_by_stage_monthly.plan.label_lists + usage_activity_by_stage_monthly.secure.sast_pipeline + usage_activity_by_stage_monthly.secure.secret_detection_pipeline + ].freeze + end + + let(:expected_metric_files_key_paths) do + metric_files_key_paths - ee_key_paths_mistakenly_defined_in_ce + ce_key_paths_mistakenly_defined_in_ee + end + end +end diff --git a/spec/controllers/graphql_controller_spec.rb b/spec/controllers/graphql_controller_spec.rb index 8281cd595f8..b1399835b45 100644 --- a/spec/controllers/graphql_controller_spec.rb +++ b/spec/controllers/graphql_controller_spec.rb @@ -224,6 +224,16 @@ RSpec.describe GraphqlController, feature_category: :integrations do post :execute end + it 'calls the track jetbrains bundled third party api when trackable method' do + agent = 'IntelliJ-GitLab-Plugin PhpStorm/PS-232.6734.11 (JRE 17.0.7+7-b966.2; Linux 6.2.0-20-generic; amd64)' + request.env['HTTP_USER_AGENT'] = agent + + expect(Gitlab::UsageDataCounters::JetBrainsBundledPluginActivityUniqueCounter) + .to receive(:track_api_request_when_trackable).with(user_agent: agent, user: user) + + post :execute + end + context 'if using the GitLab CLI' do it 'call trackable for the old UserAgent' do agent = 'GLab - GitLab CLI' @@ -359,6 +369,16 @@ RSpec.describe GraphqlController, feature_category: :integrations do subject end + it 'calls the track jetbrains bundled third party api when trackable method' do + agent = 'IntelliJ-GitLab-Plugin PhpStorm/PS-232.6734.11 (JRE 17.0.7+7-b966.2; Linux 6.2.0-20-generic; amd64)' + request.env['HTTP_USER_AGENT'] = agent + + expect(Gitlab::UsageDataCounters::JetBrainsBundledPluginActivityUniqueCounter) + .to receive(:track_api_request_when_trackable).with(user_agent: agent, user: user) + + subject + end + it 'calls the track gitlab cli when trackable method' do agent = 'GLab - GitLab CLI' request.env['HTTP_USER_AGENT'] = agent diff --git a/spec/lib/gitlab/avatar_cache_spec.rb b/spec/lib/gitlab/avatar_cache_spec.rb index 35ba767a5c5..c959c5d80b2 100644 --- a/spec/lib/gitlab/avatar_cache_spec.rb +++ b/spec/lib/gitlab/avatar_cache_spec.rb @@ -62,62 +62,54 @@ RSpec.describe Gitlab::AvatarCache, :clean_gitlab_redis_cache do end describe "#delete_by_email" do - shared_examples 'delete emails' do - subject { described_class.delete_by_email(*emails) } + subject { described_class.delete_by_email(*emails) } - before do - perform_fetch + before do + perform_fetch + end + + context "no emails, somehow" do + let(:emails) { [] } + + it { is_expected.to eq(0) } + end + + context "single email" do + let(:emails) { "foo@bar.com" } + + it "removes the email" do + expect(read(key, "20:2:true")).to eq(avatar_path) + + expect(subject).to eq(1) + + expect(read(key, "20:2:true")).to eq(nil) end + end - context "no emails, somehow" do - let(:emails) { [] } + context "multiple emails" do + let(:emails) { ["foo@bar.com", "missing@baz.com"] } - it { is_expected.to eq(0) } - end + it "removes the emails it finds" do + expect(read(key, "20:2:true")).to eq(avatar_path) - context "single email" do - let(:emails) { "foo@bar.com" } + expect(subject).to eq(1) - it "removes the email" do - expect(read(key, "20:2:true")).to eq(avatar_path) - - expect(subject).to eq(1) - - expect(read(key, "20:2:true")).to eq(nil) - end - end - - context "multiple emails" do - let(:emails) { ["foo@bar.com", "missing@baz.com"] } - - it "removes the emails it finds" do - expect(read(key, "20:2:true")).to eq(avatar_path) - - expect(subject).to eq(1) - - expect(read(key, "20:2:true")).to eq(nil) - end + expect(read(key, "20:2:true")).to eq(nil) end end context 'when deleting over 1000 emails' do it 'deletes in batches of 1000' do Gitlab::Redis::Cache.with do |redis| - expect(redis).to receive(:pipelined).at_least(2).and_call_original + if Gitlab::Redis::ClusterUtil.cluster?(redis) + expect(redis).to receive(:pipelined).at_least(2).and_call_original + else + expect(redis).to receive(:unlink).and_call_original + end end described_class.delete_by_email(*(Array.new(1001) { |i| i })) end end - - context 'when feature flag disabled' do - before do - stub_feature_flags(use_pipeline_over_multikey: false) - end - - it_behaves_like 'delete emails' - end - - it_behaves_like 'delete emails' end end diff --git a/spec/lib/gitlab/discussions_diff/highlight_cache_spec.rb b/spec/lib/gitlab/discussions_diff/highlight_cache_spec.rb index 0dc0f50b104..30981e4bd7d 100644 --- a/spec/lib/gitlab/discussions_diff/highlight_cache_spec.rb +++ b/spec/lib/gitlab/discussions_diff/highlight_cache_spec.rb @@ -41,81 +41,57 @@ RSpec.describe Gitlab::DiscussionsDiff::HighlightCache, :clean_gitlab_redis_cach end describe '#read_multiple' do - shared_examples 'read multiple keys' do - it 'reads multiple keys and serializes content into Gitlab::Diff::Line objects' do - described_class.write_multiple(mapping) + it 'reads multiple keys and serializes content into Gitlab::Diff::Line objects' do + described_class.write_multiple(mapping) - found = described_class.read_multiple(mapping.keys) + found = described_class.read_multiple(mapping.keys) - expect(found.size).to eq(2) - expect(found.first.size).to eq(2) - expect(found.first).to all(be_a(Gitlab::Diff::Line)) - end - - it 'returns nil when cached key is not found' do - described_class.write_multiple(mapping) - - found = described_class.read_multiple([2, 3]) - - expect(found.size).to eq(2) - - expect(found.first).to eq(nil) - expect(found.second.size).to eq(2) - expect(found.second).to all(be_a(Gitlab::Diff::Line)) - end - - it 'returns lines which rich_text are HTML-safe' do - described_class.write_multiple(mapping) - - found = described_class.read_multiple(mapping.keys) - rich_texts = found.flatten.map(&:rich_text) - - expect(rich_texts).to all(be_html_safe) - end + expect(found.size).to eq(2) + expect(found.first.size).to eq(2) + expect(found.first).to all(be_a(Gitlab::Diff::Line)) end - context 'when feature flag is disabled' do - before do - stub_feature_flags(use_pipeline_over_multikey: false) - end + it 'returns nil when cached key is not found' do + described_class.write_multiple(mapping) - it_behaves_like 'read multiple keys' + found = described_class.read_multiple([2, 3]) + + expect(found.size).to eq(2) + + expect(found.first).to eq(nil) + expect(found.second.size).to eq(2) + expect(found.second).to all(be_a(Gitlab::Diff::Line)) end - it_behaves_like 'read multiple keys' + it 'returns lines which rich_text are HTML-safe' do + described_class.write_multiple(mapping) + + found = described_class.read_multiple(mapping.keys) + rich_texts = found.flatten.map(&:rich_text) + + expect(rich_texts).to all(be_html_safe) + end end describe '#clear_multiple' do - shared_examples 'delete multiple keys' do - it 'removes all named keys' do - described_class.write_multiple(mapping) + it 'removes all named keys' do + described_class.write_multiple(mapping) - described_class.clear_multiple(mapping.keys) + described_class.clear_multiple(mapping.keys) - expect(described_class.read_multiple(mapping.keys)).to all(be_nil) - end - - it 'only removed named keys' do - to_clear, to_leave = mapping.keys - - described_class.write_multiple(mapping) - described_class.clear_multiple([to_clear]) - - cleared, left = described_class.read_multiple([to_clear, to_leave]) - - expect(cleared).to be_nil - expect(left).to all(be_a(Gitlab::Diff::Line)) - end + expect(described_class.read_multiple(mapping.keys)).to all(be_nil) end - context 'when feature flag is disabled' do - before do - stub_feature_flags(use_pipeline_over_multikey: false) - end + it 'only removed named keys' do + to_clear, to_leave = mapping.keys - it_behaves_like 'delete multiple keys' + described_class.write_multiple(mapping) + described_class.clear_multiple([to_clear]) + + cleared, left = described_class.read_multiple([to_clear, to_leave]) + + expect(cleared).to be_nil + expect(left).to all(be_a(Gitlab::Diff::Line)) end - - it_behaves_like 'delete multiple keys' end end diff --git a/spec/lib/gitlab/reactive_cache_set_cache_spec.rb b/spec/lib/gitlab/reactive_cache_set_cache_spec.rb index 9956078999f..44bbe888c64 100644 --- a/spec/lib/gitlab/reactive_cache_set_cache_spec.rb +++ b/spec/lib/gitlab/reactive_cache_set_cache_spec.rb @@ -46,26 +46,16 @@ RSpec.describe Gitlab::ReactiveCacheSetCache, :clean_gitlab_redis_cache do end describe '#clear_cache!', :use_clean_rails_redis_caching do - shared_examples 'clears cache' do - it 'deletes the cached items' do - # Cached key and value - Rails.cache.write('test_item', 'test_value') - # Add key to set - cache.write(cache_prefix, 'test_item') + it 'deletes the cached items' do + # Cached key and value + Rails.cache.write('test_item', 'test_value') + # Add key to set + cache.write(cache_prefix, 'test_item') - expect(cache.read(cache_prefix)).to contain_exactly('test_item') - cache.clear_cache!(cache_prefix) + expect(cache.read(cache_prefix)).to contain_exactly('test_item') + cache.clear_cache!(cache_prefix) - expect(cache.read(cache_prefix)).to be_empty - end - end - - context 'when featuer flag disabled' do - before do - stub_feature_flags(use_pipeline_over_multikey: false) - end - - it_behaves_like 'clears cache' + expect(cache.read(cache_prefix)).to be_empty end context 'when key size is large' do @@ -75,14 +65,16 @@ RSpec.describe Gitlab::ReactiveCacheSetCache, :clean_gitlab_redis_cache do it 'sends multiple pipelines of 1000 unlinks' do Gitlab::Redis::Cache.with do |redis| - expect(redis).to receive(:pipelined).at_least(2).and_call_original + if Gitlab::Redis::ClusterUtil.cluster?(redis) + expect(redis).to receive(:pipelined).at_least(2).and_call_original + else + expect(redis).to receive(:pipelined).once.and_call_original + end end cache.clear_cache!(cache_prefix) end end - - it_behaves_like 'clears cache' end describe '#include?' do diff --git a/spec/lib/gitlab/repository_set_cache_spec.rb b/spec/lib/gitlab/repository_set_cache_spec.rb index dc2081fe3a9..23b2a2b9493 100644 --- a/spec/lib/gitlab/repository_set_cache_spec.rb +++ b/spec/lib/gitlab/repository_set_cache_spec.rb @@ -77,70 +77,64 @@ RSpec.describe Gitlab::RepositorySetCache, :clean_gitlab_redis_repository_cache, end describe '#expire' do - shared_examples 'expires varying amount of keys' do - subject { cache.expire(*keys) } + subject { cache.expire(*keys) } - before do - cache.write(:foo, ['value']) - cache.write(:bar, ['value2']) + before do + cache.write(:foo, ['value']) + cache.write(:bar, ['value2']) + end + + it 'actually wrote the values' do + expect(cache.read(:foo)).to contain_exactly('value') + expect(cache.read(:bar)).to contain_exactly('value2') + end + + context 'single key' do + let(:keys) { %w(foo) } + + it { is_expected.to eq(1) } + + it 'deletes the given key from the cache' do + subject + + expect(cache.read(:foo)).to be_empty end + end - it 'actually wrote the values' do - expect(cache.read(:foo)).to contain_exactly('value') - expect(cache.read(:bar)).to contain_exactly('value2') + context 'multiple keys' do + let(:keys) { %w(foo bar) } + + it { is_expected.to eq(2) } + + it 'deletes the given keys from the cache' do + subject + + expect(cache.read(:foo)).to be_empty + expect(cache.read(:bar)).to be_empty end + end - context 'single key' do - let(:keys) { %w(foo) } + context 'no keys' do + let(:keys) { [] } - it { is_expected.to eq(1) } - - it 'deletes the given key from the cache' do - subject - - expect(cache.read(:foo)).to be_empty - end - end - - context 'multiple keys' do - let(:keys) { %w(foo bar) } - - it { is_expected.to eq(2) } - - it 'deletes the given keys from the cache' do - subject - - expect(cache.read(:foo)).to be_empty - expect(cache.read(:bar)).to be_empty - end - end - - context 'no keys' do - let(:keys) { [] } - - it { is_expected.to eq(0) } - end + it { is_expected.to eq(0) } end context 'when deleting over 1000 keys' do it 'deletes in batches of 1000' do Gitlab::Redis::RepositoryCache.with do |redis| - expect(redis).to receive(:pipelined).at_least(2).and_call_original + # In a Redis Cluster, we do not want a pipeline to have too many keys + # but in a standalone Redis, multi-key commands can be used. + if ::Gitlab::Redis::ClusterUtil.cluster?(redis) + expect(redis).to receive(:pipelined).at_least(2).and_call_original + else + expect(redis).to receive(:unlink).and_call_original + end end cache.expire(*(Array.new(1001) { |i| i })) end end - - context 'when feature flag is disabled' do - before do - stub_feature_flags(use_pipeline_over_multikey: false) - end - - it_behaves_like 'expires varying amount of keys' - end - - it_behaves_like 'expires varying amount of keys' end describe '#exist?' do diff --git a/spec/lib/gitlab/usage_data_counters/jetbrains_bundled_plugin_activity_unique_counter_spec.rb b/spec/lib/gitlab/usage_data_counters/jetbrains_bundled_plugin_activity_unique_counter_spec.rb new file mode 100644 index 00000000000..e034f04ff92 --- /dev/null +++ b/spec/lib/gitlab/usage_data_counters/jetbrains_bundled_plugin_activity_unique_counter_spec.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::UsageDataCounters::JetBrainsBundledPluginActivityUniqueCounter, :clean_gitlab_redis_shared_state, feature_category: :editor_extensions do # rubocop:disable RSpec/FilePath + let(:user1) { build(:user, id: 1) } + let(:user2) { build(:user, id: 2) } + let(:time) { Time.current } + let(:action) { described_class::JETBRAINS_BUNDLED_API_REQUEST_ACTION } + let(:user_agent_string) do + 'IntelliJ-GitLab-Plugin PhpStorm/PS-232.6734.11 (JRE 17.0.7+7-b966.2; Linux 6.2.0-20-generic; amd64)' + end + + let(:user_agent) { { user_agent: user_agent_string } } + + context 'when tracking a jetbrains bundled api request' do + it_behaves_like 'a request from an extension' + end +end diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index f63f982708d..0e1afaa8378 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -800,7 +800,21 @@ RSpec.describe NotificationService, :mailer, feature_category: :team_planning do let_it_be(:mentioned_issue) { create(:issue, assignees: issue.assignees) } let_it_be(:author) { create(:user) } - let(:note) { create(:note_on_issue, author: author, noteable: issue, project_id: issue.project_id, note: '@all mentioned') } + let(:user_mentions) do + other_members = [ + @unsubscribed_mentioned, + @u_guest_watcher, + @pg_watcher, + @u_mentioned, + @u_not_mentioned, + @u_disabled, + @pg_disabled + ] + + (issue.project.team.members + other_members).map(&:to_reference).join(' ') + end + + let(:note) { create(:note_on_issue, author: author, noteable: issue, project_id: issue.project_id, note: note_content) } before_all do build_team(project) @@ -815,33 +829,6 @@ RSpec.describe NotificationService, :mailer, feature_category: :team_planning do end describe '#new_note' do - it 'notifies the team members' do - notification.new_note(note) - - # Make sure @unsubscribed_mentioned is part of the team - expect(note.project.team.members).to include(@unsubscribed_mentioned) - - # Notify all team members - note.project.team.members.each do |member| - # User with disabled notification should not be notified - next if member.id == @u_disabled.id - # Author should not be notified - next if member.id == note.author.id - - should_email(member) - end - - should_email(@u_guest_watcher) - should_email(note.noteable.author) - should_email(note.noteable.assignees.first) - should_email_nested_group_user(@pg_watcher) - should_email(@u_mentioned) - should_email(@u_not_mentioned) - should_not_email(note.author) - should_not_email(@u_disabled) - should_not_email_nested_group_user(@pg_disabled) - end - it 'notifies parent group members with mention level' do note = create(:note_on_issue, noteable: issue, project_id: issue.project_id, note: "@#{@pg_mention.username}") @@ -850,38 +837,95 @@ RSpec.describe NotificationService, :mailer, feature_category: :team_planning do should_email_nested_group_user(@pg_mention) end - it 'filters out "mentioned in" notes' do - mentioned_note = SystemNoteService.cross_reference(mentioned_issue, issue, issue.author) - - expect(Notify).not_to receive(:note_issue_email) - notification.new_note(mentioned_note) - end - - it_behaves_like 'project emails are disabled' do - let(:notification_target) { note } - let(:notification_trigger) { notification.new_note(note) } - end - - context 'when note is confidential' do - let(:note) { create(:note_on_issue, author: author, noteable: issue, project_id: issue.project_id, note: '@all mentioned', confidential: true) } - let(:guest) { create(:user) } - - it 'does not notify users that cannot read note' do - project.add_guest(guest) - reset_delivered_emails! - + shared_examples 'correct team members are notified' do + it 'notifies the team members' do notification.new_note(note) - should_not_email(guest) + # Make sure @unsubscribed_mentioned is part of the team + expect(note.project.team.members).to include(@unsubscribed_mentioned) + + # Notify all team members + note.project.team.members.each do |member| + # User with disabled notification should not be notified + next if member.id == @u_disabled.id + # Author should not be notified + next if member.id == note.author.id + + should_email(member) + end + + should_email(@u_guest_watcher) + should_email(note.noteable.author) + should_email(note.noteable.assignees.first) + should_email_nested_group_user(@pg_watcher) + should_email(@u_mentioned) + should_email(@u_not_mentioned) + should_not_email(note.author) + should_not_email(@u_disabled) + should_not_email_nested_group_user(@pg_disabled) end + + it 'filters out "mentioned in" notes' do + mentioned_note = SystemNoteService.cross_reference(mentioned_issue, issue, issue.author) + + expect(Notify).not_to receive(:note_issue_email) + notification.new_note(mentioned_note) + end + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { note } + let(:notification_trigger) { notification.new_note(note) } + end + + context 'when note is confidential' do + let(:note) { create(:note_on_issue, author: author, noteable: issue, project_id: issue.project_id, note: note_content, confidential: true) } + let(:guest) { create(:user) } + + it 'does not notify users that cannot read note' do + project.add_guest(guest) + reset_delivered_emails! + + notification.new_note(note) + + should_not_email(guest) + end + end + end + + context 'when `@all` mention is used' do + let(:note_content) { "@all mentioned" } + + it_behaves_like 'correct team members are notified' + end + + context 'when users are individually mentioned' do + # `user_mentions` is concatenanting individual user mentions + # so that the end result is the same as `@all`. + let(:note_content) { "#{user_mentions} mentioned" } + + it_behaves_like 'correct team members are notified' end end end context 'project snippet note', :deliver_mails_inline do + let(:user_mentions) do + other_members = [ + @u_custom_global, + @u_guest_watcher, + snippet.author, # snippet = note.noteable's author + author, # note's author + @u_disabled, + @u_mentioned, + @u_not_mentioned + ] + + (snippet.project.team.members + other_members).map(&:to_reference).join(' ') + end + let(:snippet) { create(:project_snippet, project: project, author: create(:user)) } let(:author) { create(:user) } - let(:note) { create(:note_on_project_snippet, author: author, noteable: snippet, project_id: project.id, note: '@all mentioned') } + let(:note) { create(:note_on_project_snippet, author: author, noteable: snippet, project_id: project.id, note: note_content) } before do build_team(project) @@ -896,27 +940,43 @@ RSpec.describe NotificationService, :mailer, feature_category: :team_planning do end describe '#new_note' do - it 'notifies the team members' do - notification.new_note(note) - # Notify all team members - note.project.team.members.each do |member| - # User with disabled notification should not be notified - next if member.id == @u_disabled.id - # Author should not be notified - next if member.id == note.author.id + shared_examples 'correct team members are notified' do + it 'notifies the team members' do + notification.new_note(note) + # Notify all team members + note.project.team.members.each do |member| + # User with disabled notification should not be notified + next if member.id == @u_disabled.id + # Author should not be notified + next if member.id == note.author.id - should_email(member) + should_email(member) + end + + # it emails custom global users on mention + should_email(@u_custom_global) + + should_email(@u_guest_watcher) + should_email(note.noteable.author) + should_not_email(note.author) + should_email(@u_mentioned) + should_not_email(@u_disabled) + should_email(@u_not_mentioned) end + end - # it emails custom global users on mention - should_email(@u_custom_global) + context 'when `@all` mention is used' do + let(:note_content) { "@all mentioned" } - should_email(@u_guest_watcher) - should_email(note.noteable.author) - should_not_email(note.author) - should_email(@u_mentioned) - should_not_email(@u_disabled) - should_email(@u_not_mentioned) + it_behaves_like 'correct team members are notified' + end + + context 'when users are individually mentioned' do + # `user_mentions` is concatenanting individual user mentions + # so that the end result is the same as `@all`. + let(:note_content) { "#{user_mentions} mentioned" } + + it_behaves_like 'correct team members are notified' end end end diff --git a/spec/support/shared_examples/config/metrics/every_metric_definition_shared_examples.rb b/spec/support/shared_examples/config/metrics/every_metric_definition_shared_examples.rb new file mode 100644 index 00000000000..c8eaef764af --- /dev/null +++ b/spec/support/shared_examples/config/metrics/every_metric_definition_shared_examples.rb @@ -0,0 +1,161 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'every metric definition' do + include UsageDataHelpers + + let(:usage_ping) { Gitlab::Usage::ServicePingReport.for(output: :all_metrics_values, cached: false) } + let(:ignored_usage_ping_key_patterns) do + %w[ + testing_total_unique_counts + user_auth_by_provider + ].freeze + end + + let(:usage_ping_key_paths) do + parse_usage_ping_keys(usage_ping) + .flatten + .grep_v(Regexp.union(ignored_usage_ping_key_patterns)) + .sort + end + + let(:ignored_metric_files_key_patterns) do + %w[ + ci_runners_online + mock_ci + mock_monitoring + user_auth_by_provider + p_ci_templates_5_min_production_app + p_ci_templates_aws_cf_deploy_ec2 + p_ci_templates_auto_devops_build + p_ci_templates_auto_devops_deploy + p_ci_templates_auto_devops_deploy_latest + p_ci_templates_implicit_auto_devops_build + p_ci_templates_implicit_auto_devops_deploy_latest + p_ci_templates_implicit_auto_devops_deploy + ].freeze + end + + let(:metric_files_key_paths) do + Gitlab::Usage::MetricDefinition + .definitions + .reject { |_, v| v.status == 'removed' || v.key_path =~ Regexp.union(ignored_metric_files_key_patterns) } + .keys + .sort + end + + let(:metric_files_with_schema) do + Gitlab::Usage::MetricDefinition + .definitions + .select { |_, v| v.respond_to?(:value_json_schema) } + end + + let(:expected_metric_files_key_paths) { metric_files_key_paths } + + # Recursively traverse nested Hash of a generated Usage Ping to return an Array of key paths + # in the dotted format used in metric definition YAML files, e.g.: 'count.category.metric_name' + def parse_usage_ping_keys(object, key_path = []) + if object.is_a?(Hash) && !object_with_schema?(key_path.join('.')) + object.each_with_object([]) do |(key, value), result| + result.append parse_usage_ping_keys(value, key_path + [key]) + end + else + key_path.join('.') + end + end + + def object_with_schema?(key_path) + metric_files_with_schema.key?(key_path) + end + + before do + allow(Gitlab::UsageData).to receive_messages(count: -1, distinct_count: -1, estimate_batch_distinct_count: -1, + sum: -1) + allow(Gitlab::UsageData).to receive(:alt_usage_data).and_wrap_original do |_m, *_args, **kwargs| + kwargs[:fallback] || Gitlab::Utils::UsageData::FALLBACK + end + stub_licensed_features(requirements: true) + stub_prometheus_queries + stub_usage_data_connections + end + + it 'is included in the Usage Ping hash structure' do + msg = "see https://docs.gitlab.com/ee/development/service_ping/metrics_dictionary.html#metrics-added-dynamic-to-service-ping-payload" + expect(expected_metric_files_key_paths).to match_array(usage_ping_key_paths), msg + end + + it 'only uses .yml and .json formats from metric related files in (ee/)config/metrics directory' do + metric_definition_format = '.yml' + object_schema_format = '.json' + allowed_formats = [metric_definition_format, object_schema_format] + glob_paths = Gitlab::Usage::MetricDefinition.paths.map do |glob_path| + File.join(File.dirname(glob_path), '*.*') + end + + files_with_wrong_extensions = glob_paths.each_with_object([]) do |glob_path, array| + Dir.glob(glob_path).each do |path| + array << path unless allowed_formats.include? File.extname(path) + end + end + + msg = <<~MSG + The only supported file extensions are: #{allowed_formats.join(', ')}. + The following files has the wrong extension: #{files_with_wrong_extensions}" + MSG + + expect(files_with_wrong_extensions).to be_empty, msg + end + + describe 'metrics classes' do + let(:parent_metric_classes) do + [ + Gitlab::Usage::Metrics::Instrumentations::BaseMetric, + Gitlab::Usage::Metrics::Instrumentations::GenericMetric, + Gitlab::Usage::Metrics::Instrumentations::DatabaseMetric, + Gitlab::Usage::Metrics::Instrumentations::RedisMetric, + Gitlab::Usage::Metrics::Instrumentations::RedisHLLMetric, + Gitlab::Usage::Metrics::Instrumentations::NumbersMetric + ] + end + + let(:ignored_classes) do + [ + Gitlab::Usage::Metrics::Instrumentations::IssuesWithAlertManagementAlertsMetric, + Gitlab::Usage::Metrics::Instrumentations::IssuesWithPrometheusAlertEvents, + Gitlab::Usage::Metrics::Instrumentations::IssuesWithSelfManagedPrometheusAlertEvents + ].freeze + end + + def assert_uses_all_nested_classes(parent_module) + parent_module.constants(false).each do |const_name| + constant = parent_module.const_get(const_name, false) + next if parent_metric_classes.include?(constant) || ignored_classes.include?(constant) + + case constant + when Class + metric_class_instance = instance_double(constant) + expect(constant).to receive(:new).at_least(:once).and_return(metric_class_instance) + allow(metric_class_instance).to receive(:available?).and_return(true) + allow(metric_class_instance).to receive(:value).and_return(-1) + expect(metric_class_instance).to receive(:value).at_least(:once) + when Module + assert_uses_all_nested_classes(constant) + end + end + end + + it 'uses all metrics classes' do + assert_uses_all_nested_classes(Gitlab::Usage::Metrics::Instrumentations) + usage_ping + end + end + + context 'with value json schema' do + it 'has a valid structure', :aggregate_failures do + metric_files_with_schema.each do |key_path, metric| + structure = usage_ping.dig(*key_path.split('.').map(&:to_sym)) + + expect(structure).to match_metric_definition_schema(metric.value_json_schema) + end + end + end +end