diff --git a/app/assets/javascripts/admin/application_settings/setup_metrics_and_profiling.js b/app/assets/javascripts/admin/application_settings/setup_metrics_and_profiling.js index cfa2f4b8762cb0c14fd00432482a3fe136b1be18..3da138c256c809f626ce5734a1b7f5abfd8799fd 100644 --- a/app/assets/javascripts/admin/application_settings/setup_metrics_and_profiling.js +++ b/app/assets/javascripts/admin/application_settings/setup_metrics_and_profiling.js @@ -1,4 +1,6 @@ -import initSetHelperText from '~/pages/admin/application_settings/metrics_and_profiling/usage_statistics'; +import initSetHelperText, { + initOptionMetricsState, +} from '~/pages/admin/application_settings/metrics_and_profiling/usage_statistics'; import PayloadPreviewer from '~/pages/admin/application_settings/payload_previewer'; export default () => { @@ -8,3 +10,4 @@ export default () => { }; initSetHelperText(); +initOptionMetricsState(); diff --git a/app/assets/javascripts/pages/admin/application_settings/metrics_and_profiling/constants.js b/app/assets/javascripts/pages/admin/application_settings/metrics_and_profiling/constants.js new file mode 100644 index 0000000000000000000000000000000000000000..e76bed66ced9a36550b4d96cba9bb0db03d49055 --- /dev/null +++ b/app/assets/javascripts/pages/admin/application_settings/metrics_and_profiling/constants.js @@ -0,0 +1,25 @@ +import { s__ } from '~/locale'; + +export const HELPER_TEXT_SERVICE_PING_DISABLED = s__( + 'ApplicationSettings|To enable Registration Features, first enable Service Ping.', +); + +export const HELPER_TEXT_SERVICE_PING_ENABLED = s__( + 'ApplicationSettings|You can enable Registration Features because Service Ping is enabled.', +); + +export const HELPER_TEXT_OPTIONAL_METRICS_DISABLED = s__( + 'ApplicationSettings|To enable Registration Features, first enable optional data in Service Ping.', +); + +export const HELPER_TEXT_OPTIONAL_METRICS_ENABLED = s__( + 'ApplicationSettings|You can enable Registration Features because optional data in Service Ping is enabled.', +); + +export const ELEMENT_IDS = Object.freeze({ + HELPER_TEXT: 'service_ping_features_helper_text', + SERVICE_PING_FEATURES_LABEL: 'service_ping_features_label', + USAGE_PING_FEATURES_ENABLED: 'application_setting_usage_ping_features_enabled', + USAGE_PING_ENABLED: 'application_setting_usage_ping_enabled', + OPTIONAL_METRICS_IN_SERVICE_PING: 'application_setting_include_optional_metrics_in_service_ping', +}); diff --git a/app/assets/javascripts/pages/admin/application_settings/metrics_and_profiling/usage_statistics.js b/app/assets/javascripts/pages/admin/application_settings/metrics_and_profiling/usage_statistics.js index 68849857d0fa6b0582bd2efac67089b1c1155fad..ca0e547ae3e2ae64faaf4c2729caeef502a239be 100644 --- a/app/assets/javascripts/pages/admin/application_settings/metrics_and_profiling/usage_statistics.js +++ b/app/assets/javascripts/pages/admin/application_settings/metrics_and_profiling/usage_statistics.js @@ -1,41 +1,80 @@ -import { __ } from '~/locale'; +import { + ELEMENT_IDS, + HELPER_TEXT_SERVICE_PING_DISABLED, + HELPER_TEXT_SERVICE_PING_ENABLED, + HELPER_TEXT_OPTIONAL_METRICS_DISABLED, + HELPER_TEXT_OPTIONAL_METRICS_ENABLED, +} from './constants'; -export const HELPER_TEXT_SERVICE_PING_DISABLED = __( - 'To enable Registration Features, first enable Service Ping.', -); +export function setHelperText(checkbox) { + const helperTextId = document.getElementById(ELEMENT_IDS.HELPER_TEXT); + const servicePingFeaturesLabel = document.getElementById(ELEMENT_IDS.SERVICE_PING_FEATURES_LABEL); + const servicePingFeaturesCheckbox = document.getElementById( + ELEMENT_IDS.USAGE_PING_FEATURES_ENABLED, + ); + const optionalMetricsServicePingCheckbox = document.getElementById( + ELEMENT_IDS.OPTIONAL_METRICS_IN_SERVICE_PING, + ); -export const HELPER_TEXT_SERVICE_PING_ENABLED = __( - 'You can enable Registration Features because Service Ping is enabled. To continue using Registration Features in the future, you will also need to register with GitLab via a new cloud licensing service.', -); + if (optionalMetricsServicePingCheckbox) { + helperTextId.textContent = checkbox.checked + ? HELPER_TEXT_OPTIONAL_METRICS_ENABLED + : HELPER_TEXT_OPTIONAL_METRICS_DISABLED; + } else { + helperTextId.textContent = checkbox.checked + ? HELPER_TEXT_SERVICE_PING_ENABLED + : HELPER_TEXT_SERVICE_PING_DISABLED; + } -function setHelperText(servicePingCheckbox) { - const helperTextId = document.getElementById('service_ping_features_helper_text'); + servicePingFeaturesLabel.classList.toggle('gl-cursor-not-allowed', !checkbox.checked); + servicePingFeaturesCheckbox.disabled = !checkbox.checked; - const servicePingFeaturesLabel = document.getElementById('service_ping_features_label'); + if (!checkbox.checked) { + servicePingFeaturesCheckbox.disabled = true; + servicePingFeaturesCheckbox.checked = false; + } +} +export function checkOptionalMetrics(servicePingCheckbox) { + const optionalMetricsServicePingCheckbox = document.getElementById( + ELEMENT_IDS.OPTIONAL_METRICS_IN_SERVICE_PING, + ); const servicePingFeaturesCheckbox = document.getElementById( - 'application_setting_usage_ping_features_enabled', + ELEMENT_IDS.USAGE_PING_FEATURES_ENABLED, ); - helperTextId.textContent = servicePingCheckbox.checked - ? HELPER_TEXT_SERVICE_PING_ENABLED - : HELPER_TEXT_SERVICE_PING_DISABLED; - - servicePingFeaturesLabel.classList.toggle('gl-cursor-not-allowed', !servicePingCheckbox.checked); - - servicePingFeaturesCheckbox.disabled = !servicePingCheckbox.checked; + optionalMetricsServicePingCheckbox.disabled = !servicePingCheckbox.checked; if (!servicePingCheckbox.checked) { + optionalMetricsServicePingCheckbox.disabled = true; + optionalMetricsServicePingCheckbox.checked = false; servicePingFeaturesCheckbox.disabled = true; servicePingFeaturesCheckbox.checked = false; } } +export function initOptionMetricsState() { + const servicePingCheckbox = document.getElementById(ELEMENT_IDS.USAGE_PING_ENABLED); + const optionalMetricsServicePingCheckbox = document.getElementById( + ELEMENT_IDS.OPTIONAL_METRICS_IN_SERVICE_PING, + ); + if (servicePingCheckbox && optionalMetricsServicePingCheckbox) { + checkOptionalMetrics(servicePingCheckbox); + servicePingCheckbox.addEventListener('change', () => { + checkOptionalMetrics(servicePingCheckbox); + }); + } +} + export default function initSetHelperText() { - const servicePingCheckbox = document.getElementById('application_setting_usage_ping_enabled'); + const servicePingCheckbox = document.getElementById(ELEMENT_IDS.USAGE_PING_ENABLED); + const optionalMetricsServicePingCheckbox = document.getElementById( + ELEMENT_IDS.OPTIONAL_METRICS_IN_SERVICE_PING, + ); + const checkbox = optionalMetricsServicePingCheckbox || servicePingCheckbox; - setHelperText(servicePingCheckbox); - servicePingCheckbox.addEventListener('change', () => { - setHelperText(servicePingCheckbox); + setHelperText(checkbox); + checkbox.addEventListener('change', () => { + setHelperText(checkbox); }); } diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb index d048bfe9ce041abc569a56c25c1d50f2113a57f6..540f93611c77d993f2b95e46844b77df7aa53030 100644 --- a/app/helpers/application_settings_helper.rb +++ b/app/helpers/application_settings_helper.rb @@ -308,6 +308,7 @@ def visible_attributes :inactive_projects_delete_after_months, :inactive_projects_min_size_mb, :inactive_projects_send_warning_email_after_months, + :include_optional_metrics_in_service_ping, :invisible_captcha_enabled, :jira_connect_application_key, :jira_connect_public_key_storage_enabled, diff --git a/app/models/application_setting_implementation.rb b/app/models/application_setting_implementation.rb index fed0f2b67aa4c3eec5702ce2e09ffda55563aa64..777267cb19565e705562eea1c7f636005ce6c7f6 100644 --- a/app/models/application_setting_implementation.rb +++ b/app/models/application_setting_implementation.rb @@ -111,6 +111,7 @@ def defaults # rubocop:disable Metrics/AbcSize housekeeping_gc_period: 200, housekeeping_incremental_repack_period: 10, import_sources: Settings.gitlab['import_sources'], + include_optional_metrics_in_service_ping: Settings.gitlab['usage_ping_enabled'], instance_level_ai_beta_features_enabled: false, instance_level_code_suggestions_enabled: false, invisible_captcha_enabled: false, @@ -233,6 +234,7 @@ def defaults # rubocop:disable Metrics/AbcSize unique_ips_limit_per_user: 10, unique_ips_limit_time_window: 3600, usage_ping_enabled: Settings.gitlab['usage_ping_enabled'], + usage_ping_features_enabled: false, usage_stats_set_by_user_id: nil, user_default_external: false, user_default_internal_regex: nil, @@ -515,13 +517,20 @@ def usage_ping_can_be_configured? Settings.gitlab.usage_ping_enabled end - def usage_ping_features_enabled? - usage_ping_enabled? && usage_ping_features_enabled + def usage_ping_features_enabled + return false unless usage_ping_enabled? && super + + return include_optional_metrics_in_service_ping if Gitlab.ee? + + true end + alias_method :usage_ping_features_enabled?, :usage_ping_features_enabled + def usage_ping_enabled usage_ping_can_be_configured? && super end + alias_method :usage_ping_enabled?, :usage_ping_enabled def allowed_key_types diff --git a/app/views/admin/application_settings/_usage.html.haml b/app/views/admin/application_settings/_usage.html.haml index dd9820d064a78100f6b37284d51a0602cf10a78c..bdaef614033d4af01b6365fc48460edd71eeac3a 100644 --- a/app/views/admin/application_settings/_usage.html.haml +++ b/app/views/admin/application_settings/_usage.html.haml @@ -17,33 +17,36 @@ - usage_ping_help_text = s_('AdminSettings|To help improve GitLab and its user experience, GitLab periodically collects usage information. %{link_start}What information is shared with GitLab Inc.?%{link_end}').html_safe % { link_start: service_ping_link_start, link_end: link_end } - disabled_help_text = s_('AdminSettings|Service ping is disabled in your configuration file, and cannot be enabled through this form. For more information, see the documentation on %{link_start}deactivating service ping%{link_end}.').html_safe % { link_start: deactivating_service_ping_link_start, link_end: link_end } = f.gitlab_ui_checkbox_component :usage_ping_enabled, s_('AdminSettings|Enable Service Ping'), - help_text: can_be_configured ? usage_ping_help_text : disabled_help_text, - checkbox_options: { disabled: !can_be_configured, data: { testid: 'enable-usage-data-checkbox' } } - .form-text.gl-pl-6 - - if @service_ping_data.present? - = render Pajamas::ButtonComponent.new(button_options: { class: 'js-payload-preview-trigger gl-mr-2', data: { payload_selector: ".#{payload_class}" } }) do - = gl_loading_icon(css_class: 'js-spinner gl-display-none', inline: true) - %span.js-text.gl-display-inline= s_('AdminSettings|Preview payload') - = render Pajamas::ButtonComponent.new(button_options: { class: 'js-payload-download-trigger gl-mr-2', data: { endpoint: usage_data_admin_application_settings_path(format: :json) } }) do - = gl_loading_icon(css_class: 'js-spinner gl-display-none', inline: true) - %span.js-text.gl-display-inline= s_('AdminSettings|Download payload') - %pre.js-syntax-highlight.code.highlight.gl-mt-2.gl-display-none{ class: payload_class, data: { endpoint: usage_data_admin_application_settings_path(format: :html) } } - - else - = render Pajamas::AlertComponent.new(variant: :warning, - dismissible: false, - title: s_('AdminSettings|Service Ping payload not found in the application cache')) do |c| - - - c.with_body do - - generate_manually_link = link_to('', help_page_path('development/internal_analytics/service_ping/troubleshooting', anchor: 'generate-service-ping'), target: '_blank', rel: 'noopener noreferrer') - = safe_format(s_('AdminSettings|%{generate_manually_link_start}Generate%{generate_manually_link_end} Service Ping to preview and download service usage data payload.'), tag_pair(generate_manually_link, :generate_manually_link_start, :generate_manually_link_end)) + help_text: can_be_configured ? usage_ping_help_text : disabled_help_text, + checkbox_options: { disabled: !can_be_configured || ServicePing::ServicePingSettings.license_operational_metric_enabled?, data: { testid: 'enable-usage-data-checkbox' } } + - if Gitlab.ee? + = render_if_exists 'admin/application_settings/include_optional_metric_in_service_ping_checkbox', form: f + .form-text.gl-pl-6.gl-mb-6 + - if @service_ping_data.present? + = render Pajamas::ButtonComponent.new(button_options: { class: 'js-payload-preview-trigger gl-mr-2', data: { payload_selector: ".#{payload_class}" } }) do + = gl_loading_icon(css_class: 'js-spinner gl-display-none', inline: true) + %span.js-text.gl-display-inline= s_('AdminSettings|Preview payload') + = render Pajamas::ButtonComponent.new(button_options: { class: 'js-payload-download-trigger gl-mr-2', data: { endpoint: usage_data_admin_application_settings_path(format: :json) } }) do + = gl_loading_icon(css_class: 'js-spinner gl-display-none', inline: true) + %span.js-text.gl-display-inline= s_('AdminSettings|Download payload') + %pre.js-syntax-highlight.code.highlight.gl-mt-2.gl-display-none{ class: payload_class, data: { endpoint: usage_data_admin_application_settings_path(format: :html) } } + - else + = render Pajamas::AlertComponent.new(variant: :warning, + dismissible: false, + title: s_('AdminSettings|Service Ping payload not found in the application cache')) do |c| + - c.with_body do + - generate_manually_link = link_to('', help_page_path('development/internal_analytics/service_ping/troubleshooting', anchor: 'generate-service-ping'), target: '_blank', rel: 'noopener noreferrer') + = safe_format(s_('AdminSettings|%{generate_manually_link_start}Generate%{generate_manually_link_end} Service Ping to preview and download service usage data payload.'), tag_pair(generate_manually_link, :generate_manually_link_start, :generate_manually_link_end)) .form-group - usage_ping_enabled = @application_setting.usage_ping_enabled? + - include_optional_metrics_in_service_ping = @application_setting.include_optional_metrics_in_service_ping - label = s_('AdminSettings|Enable Registration Features') - label_link = link_to sprite_icon('question-o'), help_page_path('administration/settings/usage_statistics', anchor: 'registration-features-program') - - help_text = usage_ping_enabled ? s_('AdminSettings|You can enable Registration Features because Service Ping is enabled. To continue using Registration Features in the future, you will also need to register with GitLab via a new cloud licensing service.') : s_('AdminSettings|To enable Registration Features, first enable Service Ping.') + - service_ping_help_text = usage_ping_enabled ? s_('AdminSettings|You can enable Registration Features because Service Ping is enabled.') : s_('AdminSettings|To enable Registration Features, first enable Service Ping.') + - optional_metrics_help_text = include_optional_metrics_in_service_ping ? s_('AdminSettings|You can enable Registration Features because optional data in Service Ping is enabled.') : s_('AdminSettings|To enable Registration Features, first enable optional data in Service Ping.') = f.gitlab_ui_checkbox_component :usage_ping_features_enabled?, '%{label} %{label_link}'.html_safe % { label: label, label_link: label_link }, - help_text: '%{help_text}'.html_safe % { help_text: help_text }, + help_text: tag.span(Gitlab.ee? ? optional_metrics_help_text : service_ping_help_text, id: 'service_ping_features_helper_text'), checkbox_options: { id: 'application_setting_usage_ping_features_enabled' }, label_options: { id: 'service_ping_features_label' } .form-text.gl-text-gray-500.gl-pl-6 diff --git a/db/migrate/20240110160643_add_optional_metrics_enabled_to_application_settings.rb b/db/migrate/20240110160643_add_optional_metrics_enabled_to_application_settings.rb new file mode 100644 index 0000000000000000000000000000000000000000..dd8990ef0866794a275091d7fca195ac8d3d2223 --- /dev/null +++ b/db/migrate/20240110160643_add_optional_metrics_enabled_to_application_settings.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class AddOptionalMetricsEnabledToApplicationSettings < Gitlab::Database::Migration[2.2] + milestone '16.9' + + def up + add_column :application_settings, :include_optional_metrics_in_service_ping, :boolean, default: true, null: false + end + + def down + remove_column :application_settings, :include_optional_metrics_in_service_ping + end +end diff --git a/db/migrate/20240110160816_update_optiona_metrics_value_service_ping.rb b/db/migrate/20240110160816_update_optiona_metrics_value_service_ping.rb new file mode 100644 index 0000000000000000000000000000000000000000..e0034c5d2d295a8f9ce8e2f7341def67ae7ea23d --- /dev/null +++ b/db/migrate/20240110160816_update_optiona_metrics_value_service_ping.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class UpdateOptionaMetricsValueServicePing < Gitlab::Database::Migration[2.2] + restrict_gitlab_migration gitlab_schema: :gitlab_main + milestone '16.9' + + def up + execute <<~SQL + UPDATE application_settings + SET include_optional_metrics_in_service_ping = usage_ping_enabled + SQL + end + + def down + # No-op + end +end diff --git a/db/schema_migrations/20240110160643 b/db/schema_migrations/20240110160643 new file mode 100644 index 0000000000000000000000000000000000000000..f228bf854369afc8b22adb603be3cc2ce4e9b0cd --- /dev/null +++ b/db/schema_migrations/20240110160643 @@ -0,0 +1 @@ +2c565f21446c6a7e63d5007a22f064cf18d8cf662b22dfdd0ba08264957e30d9 \ No newline at end of file diff --git a/db/schema_migrations/20240110160816 b/db/schema_migrations/20240110160816 new file mode 100644 index 0000000000000000000000000000000000000000..c01d2939a782f2d1fd59222f9085f7e9d273a174 --- /dev/null +++ b/db/schema_migrations/20240110160816 @@ -0,0 +1 @@ +04719fee22b042b856b719c2cc987582bc6b3ebecf0f8e5d9944765766718c20 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 8eef253e0d61da10e36ef02f38cca25c1be2585d..8b39612e659356c3da681fe4719f8f1913d59df6 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -4108,6 +4108,7 @@ CREATE TABLE application_settings ( lock_toggle_security_policy_custom_ci boolean DEFAULT false NOT NULL, toggle_security_policies_policy_scope boolean DEFAULT false NOT NULL, lock_toggle_security_policies_policy_scope boolean DEFAULT false NOT NULL, + include_optional_metrics_in_service_ping boolean DEFAULT true NOT NULL, rate_limits jsonb DEFAULT '{}'::jsonb NOT NULL, elasticsearch_max_code_indexing_concurrency integer DEFAULT 30 NOT NULL, enable_member_promotion_management boolean DEFAULT false NOT NULL, diff --git a/doc/api/settings.md b/doc/api/settings.md index ea40fdb5d9eb38e742b9fbff89c8dee8d1698c37..074dc26596fb611025d4fc8e5646a9d766b6fc9a 100644 --- a/doc/api/settings.md +++ b/doc/api/settings.md @@ -624,6 +624,7 @@ listed in the descriptions of the relevant settings. | `unique_ips_limit_time_window` | integer | required by: `unique_ips_limit_enabled` | How many seconds an IP is counted towards the limit. | | `update_runner_versions_enabled` | boolean | no | Fetch GitLab Runner release version data from GitLab.com. For more information, see how to [determine which runners need to be upgraded](../ci/runners/runners_scope.md#determine-which-runners-need-to-be-upgraded). | | `usage_ping_enabled` | boolean | no | Every week GitLab reports license usage back to GitLab, Inc. | +| `include_optional_metrics_in_service_ping`| boolean | no | Whether or not optional metrics are enabled in Service Ping. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/141540) in GitLab 16.10. | | `user_deactivation_emails_enabled` | boolean | no | Send an email to users upon account deactivation. | | `user_default_external` | boolean | no | Newly registered users are external by default. | | `user_default_internal_regex` | string | no | Specify an email address regex pattern to identify default internal users. | diff --git a/ee/app/views/admin/application_settings/_include_optional_metric_in_service_ping_checkbox.haml b/ee/app/views/admin/application_settings/_include_optional_metric_in_service_ping_checkbox.haml new file mode 100644 index 0000000000000000000000000000000000000000..eeb9e26084f5f1d6170041e03ce61275b0d90ef0 --- /dev/null +++ b/ee/app/views/admin/application_settings/_include_optional_metric_in_service_ping_checkbox.haml @@ -0,0 +1,10 @@ +- f = local_assigns.fetch(:form) +.form-group + - optional_metrics_disabled = !@application_setting.usage_ping_enabled? + = f.gitlab_ui_checkbox_component :include_optional_metrics_in_service_ping, checkbox_options: { disabled: optional_metrics_disabled } do |c| + = c.with_label do + = s_('AdminSettings|Include optional data in Service Ping.') + = c.with_help_text do + = _('Include additional optional data in Service Ping. To enable optional data, first enable Service Ping.') + - service_ping_handbook_link = link_to('', 'https://handbook.gitlab.com/handbook/legal/privacy/customer-product-usage-information#service-ping-formerly-known-as-usage-ping', target: '_blank', rel: 'noopener noreferrer') + = safe_format(s_('AdminSettings|For more information, see %{link_start}the documentation%{link_end}.'), tag_pair(service_ping_handbook_link, :link_start, :link_end)) diff --git a/ee/lib/ee/service_ping/permit_data_categories.rb b/ee/lib/ee/service_ping/permit_data_categories.rb index 9dc8dbf67240a8ffa5104dff320bc3a525142e40..9edcc7fdbe819ddf67a296bc7d8d1048465be3ca 100644 --- a/ee/lib/ee/service_ping/permit_data_categories.rb +++ b/ee/lib/ee/service_ping/permit_data_categories.rb @@ -12,15 +12,10 @@ module PermitDataCategories override :execute def execute - return super unless ::License.current.present? - return super unless ::ServicePing::ServicePingSettings.enabled_and_consented? + optional_enabled = ::Gitlab::CurrentSettings.include_optional_metrics_in_service_ping? - optional_enabled = ::Gitlab::CurrentSettings.usage_ping_enabled? - customer_service_enabled = ::License.current.customer_service_enabled? - - [STANDARD_CATEGORY, SUBSCRIPTION_CATEGORY].tap do |categories| - categories << OPERATIONAL_CATEGORY << OPTIONAL_CATEGORY if optional_enabled - categories << OPERATIONAL_CATEGORY if customer_service_enabled + [STANDARD_CATEGORY, SUBSCRIPTION_CATEGORY, OPERATIONAL_CATEGORY].tap do |categories| + categories << OPTIONAL_CATEGORY if optional_enabled end.to_set end end diff --git a/ee/lib/ee/service_ping/service_ping_settings.rb b/ee/lib/ee/service_ping/service_ping_settings.rb index 05e382cb1309ef9e74624118db1d9022eaf91008..dee4dc29fa4cd5df11038c9f83ba483b1c1f3bc3 100644 --- a/ee/lib/ee/service_ping/service_ping_settings.rb +++ b/ee/lib/ee/service_ping/service_ping_settings.rb @@ -5,8 +5,8 @@ module ServicePing module ServicePingSettings extend ::Gitlab::Utils::Override - override :enabled? - def enabled? + override :license_operational_metric_enabled? + def license_operational_metric_enabled? ::License.current&.customer_service_enabled? || super end end diff --git a/ee/spec/lib/ee/service_ping/build_payload_spec.rb b/ee/spec/lib/ee/service_ping/build_payload_spec.rb index 84c3053b396d9366ce32c44c283843622acfca8f..855b1f5c4a4a968c30b26ea4f549d412999e5edc 100644 --- a/ee/spec/lib/ee/service_ping/build_payload_spec.rb +++ b/ee/spec/lib/ee/service_ping/build_payload_spec.rb @@ -17,8 +17,8 @@ # License.current.present? == true context 'Instance consented to submit optional product intelligence data' do before do - # Gitlab::CurrentSettings.usage_ping_enabled? == true - stub_config_setting(usage_ping_enabled: true) + # Gitlab::CurrentSettings.include_optional_metrics_in_service_ping? == true + stub_application_setting(include_optional_metrics_in_service_ping: true) end context 'Instance subscribes to free TAM service' do @@ -44,8 +44,8 @@ context 'Instance does NOT consented to submit optional product intelligence data' do before do - # Gitlab::CurrentSettings.usage_ping_enabled? == false - stub_config_setting(usage_ping_enabled: false) + # Gitlab::CurrentSettings.include_optional_metrics_in_service_ping? == false + stub_application_setting(include_optional_metrics_in_service_ping: false) end context 'Instance subscribes to free TAM service' do @@ -69,7 +69,13 @@ create_current_license(operational_metrics_enabled: false) end - it_behaves_like 'complete service ping payload' + it_behaves_like 'service ping payload with all expected metrics' do + let(:expected_metrics) { standard_metrics + operational_metrics } + end + + it_behaves_like 'service ping payload without restricted metrics' do + let(:restricted_metrics) { optional_metrics } + end end end end diff --git a/ee/spec/lib/ee/service_ping/permit_data_categories_spec.rb b/ee/spec/lib/ee/service_ping/permit_data_categories_spec.rb index 654be80ab112a6fbffc84d01ff3619d34aa4eb55..1dd83b89bf0df8146da232374caba1ff18937ef7 100644 --- a/ee/spec/lib/ee/service_ping/permit_data_categories_spec.rb +++ b/ee/spec/lib/ee/service_ping/permit_data_categories_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe ServicePing::PermitDataCategories do +RSpec.describe ServicePing::PermitDataCategories, feature_category: :service_ping do describe '#execute' do subject(:permitted_categories) { described_class.new.execute } @@ -25,6 +25,30 @@ end context 'with current license' do + using RSpec::Parameterized::TableSyntax + + where(:usage_ping_enabled, :operational_metrics_enabled, :optional_metrics_enabled, :expected_data_categories) do + false | false | false | %w[standard subscription operational] + true | false | false | %w[standard subscription operational] + true | false | true | %w[standard subscription operational optional] + true | true | false | %w[standard subscription operational] + true | true | true | %w[standard subscription operational optional] + false | true | true | %w[standard subscription operational optional] + false | true | false | %w[standard subscription operational] + end + + with_them do + before do + stub_config_setting(usage_ping_enabled: usage_ping_enabled) + create_current_license(operational_metrics_enabled: operational_metrics_enabled) + stub_application_setting(include_optional_metrics_in_service_ping: optional_metrics_enabled) + end + + it 'returns expected categories' do + expect(permitted_categories).to match_array(expected_data_categories) + end + end + context 'when usage ping setting is set to true' do before do stub_config_setting(usage_ping_enabled: true) @@ -77,7 +101,7 @@ end it 'returns all categories' do - expect(permitted_categories).to match_array(%w[standard subscription operational]) + expect(permitted_categories).to match_array(%w[standard subscription operational optional]) end end diff --git a/ee/spec/lib/ee/service_ping/service_ping_settings_spec.rb b/ee/spec/lib/ee/service_ping/service_ping_settings_spec.rb index 856b6603ad52c24fddbeed6eec087ec930c77383..ab6c45b953d8e78cd9008caf7481e1f2b894dcd8 100644 --- a/ee/spec/lib/ee/service_ping/service_ping_settings_spec.rb +++ b/ee/spec/lib/ee/service_ping/service_ping_settings_spec.rb @@ -45,6 +45,23 @@ end end + describe '#license_operational_metric_enabled?' do + where(:customer_service_enabled, :expected_license_operational_metric_enabled) do + true | true + false | false + end + + with_them do + before do + create_current_license(operational_metrics_enabled: customer_service_enabled) + end + + it 'returns the correct value for license_operational_metric_enabled?' do + expect(described_class.license_operational_metric_enabled?).to eq(expected_license_operational_metric_enabled) + end + end + end + describe '#enabled?' do where(:usage_ping_enabled, :customer_service_enabled, :expected_enabled) do true | true | true diff --git a/lib/service_ping/service_ping_settings.rb b/lib/service_ping/service_ping_settings.rb index 8c99f1138c5acd81dcb0b4ad3e04cfa39d23a593..c5aed5132c0f34dcad984ae97371f01ad4bb5584 100644 --- a/lib/service_ping/service_ping_settings.rb +++ b/lib/service_ping/service_ping_settings.rb @@ -8,8 +8,15 @@ def enabled_and_consented? enabled? && !User.single_user&.requires_usage_stats_consent? end + def license_operational_metric_enabled? + false + end + + # If it is EE and license operational metric is true, + # then we will show enable service ping checkbox checked, + # as it will always send service ping def enabled? - ::Gitlab::CurrentSettings.usage_ping_enabled? + license_operational_metric_enabled? || ::Gitlab::CurrentSettings.usage_ping_enabled? end end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 69f58b209956438f4c65f4717ac478010af8dc13..c676a089458f9d5210aa1c25879fdc87906abd37 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -3541,6 +3541,9 @@ msgstr "" msgid "AdminSettings|For a list of included Registration Features, see %{link_start}the documentation%{link_end}." msgstr "" +msgid "AdminSettings|For more information, see %{link_start}the documentation%{link_end}." +msgstr "" + msgid "AdminSettings|Git abuse rate limit" msgstr "" @@ -3583,6 +3586,9 @@ msgstr "" msgid "AdminSettings|Inactive project deletion" msgstr "" +msgid "AdminSettings|Include optional data in Service Ping." +msgstr "" + msgid "AdminSettings|Instance runners details" msgstr "" @@ -3784,6 +3790,9 @@ msgstr "" msgid "AdminSettings|To enable Registration Features, first enable Service Ping." msgstr "" +msgid "AdminSettings|To enable Registration Features, first enable optional data in Service Ping." +msgstr "" + msgid "AdminSettings|To help improve GitLab and its user experience, GitLab periodically collects usage information. %{link_start}What information is shared with GitLab Inc.?%{link_end}" msgstr "" @@ -3802,7 +3811,10 @@ msgstr "" msgid "AdminSettings|When to delete inactive projects" msgstr "" -msgid "AdminSettings|You can enable Registration Features because Service Ping is enabled. To continue using Registration Features in the future, you will also need to register with GitLab via a new cloud licensing service." +msgid "AdminSettings|You can enable Registration Features because Service Ping is enabled." +msgstr "" + +msgid "AdminSettings|You can enable Registration Features because optional data in Service Ping is enabled." msgstr "" msgid "AdminSettings|You can't delete projects before the warning email is sent." @@ -6109,6 +6121,12 @@ msgstr "" msgid "ApplicationSettings|Text shown after a user signs up. Markdown enabled." msgstr "" +msgid "ApplicationSettings|To enable Registration Features, first enable Service Ping." +msgstr "" + +msgid "ApplicationSettings|To enable Registration Features, first enable optional data in Service Ping." +msgstr "" + msgid "ApplicationSettings|Upload denylist file" msgstr "" @@ -6133,6 +6151,12 @@ msgstr "" msgid "ApplicationSettings|When enabled, new passwords must contain at least one uppercase letter (A-Z)." msgstr "" +msgid "ApplicationSettings|You can enable Registration Features because Service Ping is enabled." +msgstr "" + +msgid "ApplicationSettings|You can enable Registration Features because optional data in Service Ping is enabled." +msgstr "" + msgid "ApplicationSettings|domain.com" msgstr "" @@ -26116,6 +26140,9 @@ msgstr "" msgid "Incident|Timeline text..." msgstr "" +msgid "Include additional optional data in Service Ping. To enable optional data, first enable Service Ping." +msgstr "" + msgid "Include author name in notification email body" msgstr "" @@ -51933,9 +51960,6 @@ msgstr "" msgid "To edit the pipeline configuration, you must go to the project or external site that hosts the file." msgstr "" -msgid "To enable Registration Features, first enable Service Ping." -msgstr "" - msgid "To ensure %{project_link} is unscheduled for deletion, check that activity has been logged by GitLab. For example:" msgstr "" @@ -57281,9 +57305,6 @@ msgstr "" msgid "You can easily contribute to them by requesting to join these groups." msgstr "" -msgid "You can enable Registration Features because Service Ping is enabled. To continue using Registration Features in the future, you will also need to register with GitLab via a new cloud licensing service." -msgstr "" - msgid "You can enable group access token creation in %{link_start}group settings%{link_end}." msgstr "" diff --git a/spec/frontend/fixtures/application_settings.rb b/spec/frontend/fixtures/application_settings.rb index 34e99ec647cd6f0bdc83dd62da53ddfd5a62c0b1..060622d97885d224079e9e8c4955e4bcd678e728 100644 --- a/spec/frontend/fixtures/application_settings.rb +++ b/spec/frontend/fixtures/application_settings.rb @@ -34,6 +34,7 @@ it 'application_settings/usage.html' do stub_application_setting(usage_ping_enabled: false) + stub_application_setting(include_optional_metrics_in_service_ping: false) get :metrics_and_profiling diff --git a/spec/frontend/pages/admin/application_settings/metrics_and_profiling/usage_statistics_spec.js b/spec/frontend/pages/admin/application_settings/metrics_and_profiling/usage_statistics_spec.js index 72d2bb0f983cf9603bad5f3e4bf7c4af99c77b1a..f57d7ee15b2346285a54c99de504a5883204c48e 100644 --- a/spec/frontend/pages/admin/application_settings/metrics_and_profiling/usage_statistics_spec.js +++ b/spec/frontend/pages/admin/application_settings/metrics_and_profiling/usage_statistics_spec.js @@ -1,62 +1,131 @@ -import htmlApplicationSettingsUsage from 'test_fixtures/application_settings/usage.html'; import initSetHelperText, { - HELPER_TEXT_SERVICE_PING_DISABLED, - HELPER_TEXT_SERVICE_PING_ENABLED, + setHelperText, + checkOptionalMetrics, } from '~/pages/admin/application_settings/metrics_and_profiling/usage_statistics'; -import { setHTMLFixture, resetHTMLFixture } from 'helpers/fixtures'; - -describe('UsageStatistics', () => { - let servicePingCheckBox; - let servicePingFeaturesCheckBox; - let servicePingFeaturesLabel; - let servicePingFeaturesHelperText; +describe('Optional Metrics Tests for EE', () => { beforeEach(() => { - setHTMLFixture(htmlApplicationSettingsUsage); - initSetHelperText(); - servicePingCheckBox = document.getElementById('application_setting_usage_ping_enabled'); - servicePingFeaturesCheckBox = document.getElementById( - 'application_setting_usage_ping_features_enabled', - ); - servicePingFeaturesLabel = document.getElementById('service_ping_features_label'); - servicePingFeaturesHelperText = document.getElementById('service_ping_features_helper_text'); + document.body.innerHTML = ` +
+ + + + + `; }); - afterEach(() => { - resetHTMLFixture(); + describe('setHelperText Functionality', () => { + it('should enable helper text when optional metrics are enabled', () => { + const optionalMetricsServicePingCheckbox = document.getElementById( + 'application_setting_include_optional_metrics_in_service_ping', + ); + optionalMetricsServicePingCheckbox.checked = true; + setHelperText(optionalMetricsServicePingCheckbox); + + const helperText = document.getElementById('service_ping_features_helper_text').textContent; + expect(helperText).toBe( + 'You can enable Registration Features because optional data in Service Ping is enabled.', + ); + }); + + it('should disable helper text when optional metrics are disabled', () => { + const optionalMetricsServicePingCheckbox = document.getElementById( + 'application_setting_include_optional_metrics_in_service_ping', + ); + optionalMetricsServicePingCheckbox.checked = false; + setHelperText(optionalMetricsServicePingCheckbox); + + const helperText = document.getElementById('service_ping_features_helper_text').textContent; + expect(helperText).toBe( + 'To enable Registration Features, first enable optional data in Service Ping.', + ); + }); }); - const expectEnabledservicePingFeaturesCheckBox = () => { - expect(servicePingFeaturesCheckBox.classList.contains('gl-cursor-not-allowed')).toBe(false); - expect(servicePingFeaturesHelperText.textContent).toEqual(HELPER_TEXT_SERVICE_PING_ENABLED); - }; + describe('checkOptionalMetrics Functionality', () => { + it('should disable optional metrics when service ping is disabled', () => { + const servicePingCheckbox = document.getElementById('application_setting_usage_ping_enabled'); + servicePingCheckbox.checked = false; + checkOptionalMetrics(servicePingCheckbox); + + const optionalMetricsCheckbox = document.getElementById( + 'application_setting_include_optional_metrics_in_service_ping', + ); + expect(optionalMetricsCheckbox.disabled).toBe(true); + expect(optionalMetricsCheckbox.checked).toBe(false); + }); + }); + + describe('Features checkbox state', () => { + it('should enable/disable features checkbox when optional metrics checkbox is enabled or disabled', () => { + initSetHelperText(); + + const optionalMetricsServicePingCheckbox = document.getElementById( + 'application_setting_include_optional_metrics_in_service_ping', + ); + optionalMetricsServicePingCheckbox.checked = true; + optionalMetricsServicePingCheckbox.dispatchEvent(new Event('change')); + + const servicePingFeaturesCheckbox = document.getElementById( + 'application_setting_usage_ping_features_enabled', + ); + + expect(servicePingFeaturesCheckbox.disabled).toBe(false); - const expectDisabledservicePingFeaturesCheckBox = () => { - expect(servicePingFeaturesLabel.classList.contains('gl-cursor-not-allowed')).toBe(true); - expect(servicePingFeaturesHelperText.textContent).toEqual(HELPER_TEXT_SERVICE_PING_DISABLED); - }; + optionalMetricsServicePingCheckbox.checked = false; + optionalMetricsServicePingCheckbox.dispatchEvent(new Event('change')); - describe('Registration Features checkbox', () => { - it('is disabled when Service Ping checkbox is unchecked', () => { - expect(servicePingCheckBox.checked).toBe(false); - expectDisabledservicePingFeaturesCheckBox(); + expect(servicePingFeaturesCheckbox.disabled).toBe(true); }); + }); +}); + +describe('Without Optional Metrics Checkbox for CE', () => { + beforeEach(() => { + document.body.innerHTML = ` +
+ + + + `; + }); + + describe('initSetHelperText Functionality Without Optional Metrics Checkbox for CE', () => { + it('should set helper text for service ping when optional metrics checkbox is missing', () => { + initSetHelperText(); - it('is enabled when Servie Ping checkbox is checked', () => { - servicePingCheckBox.click(); - expect(servicePingCheckBox.checked).toBe(true); - expectEnabledservicePingFeaturesCheckBox(); + const servicePingCheckbox = document.getElementById('application_setting_usage_ping_enabled'); + servicePingCheckbox.checked = true; + servicePingCheckbox.dispatchEvent(new Event('change')); + + let helperText = document.getElementById('service_ping_features_helper_text').textContent; + expect(helperText).toBe( + 'You can enable Registration Features because Service Ping is enabled.', + ); + + servicePingCheckbox.checked = false; + servicePingCheckbox.dispatchEvent(new Event('change')); + + helperText = document.getElementById('service_ping_features_helper_text').textContent; + expect(helperText).toBe('To enable Registration Features, first enable Service Ping.'); }); + it('should enable/disable features checkbox when enable service ping checkbox is enabled or disabled', () => { + initSetHelperText(); + + const servicePingCheckbox = document.getElementById('application_setting_usage_ping_enabled'); + servicePingCheckbox.checked = true; + servicePingCheckbox.dispatchEvent(new Event('change')); + + const servicePingFeaturesCheckbox = document.getElementById( + 'application_setting_usage_ping_features_enabled', + ); + + expect(servicePingFeaturesCheckbox.disabled).toBe(false); - it('is switched to disabled when Service Ping checkbox is unchecked', () => { - servicePingCheckBox.click(); - servicePingFeaturesCheckBox.click(); - expectEnabledservicePingFeaturesCheckBox(); + servicePingCheckbox.checked = false; + servicePingCheckbox.dispatchEvent(new Event('change')); - servicePingCheckBox.click(); - expect(servicePingCheckBox.checked).toBe(false); - expect(servicePingFeaturesCheckBox.checked).toBe(false); - expectDisabledservicePingFeaturesCheckBox(); + expect(servicePingFeaturesCheckbox.disabled).toBe(true); }); }); }); diff --git a/spec/lib/service_ping/service_ping_settings_spec.rb b/spec/lib/service_ping/service_ping_settings_spec.rb index 2e61b35a13198af818064b4c372fa52d629bea42..ccf2564ca8caf3d5875dceaf0bbd186026c5640b 100644 --- a/spec/lib/service_ping/service_ping_settings_spec.rb +++ b/spec/lib/service_ping/service_ping_settings_spec.rb @@ -29,6 +29,12 @@ end end + describe '#license_operational_metric_enabled?' do + it 'returns false' do + expect(described_class.license_operational_metric_enabled?).to eq(false) + end + end + describe '#enabled?' do describe 'has the correct enabled' do it 'when false' do diff --git a/spec/support/shared_examples/models/application_setting_shared_examples.rb b/spec/support/shared_examples/models/application_setting_shared_examples.rb index 70179dd7fc7deb5ef116346a95edbe92b3c8a1ab..fb9fd2f0f73f56f819b4ec27bfda4819299a475c 100644 --- a/spec/support/shared_examples/models/application_setting_shared_examples.rb +++ b/spec/support/shared_examples/models/application_setting_shared_examples.rb @@ -193,6 +193,10 @@ it 'returns false for usage_ping_enabled' do expect(setting.usage_ping_enabled).to be_falsey end + + it 'returns false for usage_ping_features_enabled' do + expect(setting.usage_ping_features_enabled).to be_falsey + end end context 'when the usage ping is enabled in the DB' do @@ -203,6 +207,10 @@ it 'returns false for usage_ping_enabled' do expect(setting.usage_ping_enabled).to be_falsey end + + it 'returns false for usage_ping_features_enabled' do + expect(setting.usage_ping_features_enabled).to be_falsey + end end end @@ -223,6 +231,10 @@ it 'returns false for usage_ping_enabled' do expect(setting.usage_ping_enabled).to be_falsey end + + it 'returns false for usage_ping_features_enabled' do + expect(setting.usage_ping_features_enabled).to be_falsey + end end context 'when the usage ping is enabled in the DB' do @@ -233,6 +245,48 @@ it 'returns true for usage_ping_enabled' do expect(setting.usage_ping_enabled).to be_truthy end + + context 'when usage_ping_features_enabled is enabled in db' do + before do + setting.usage_ping_features_enabled = true + end + + it 'returns true for usage_ping_features_enabled' do + expect(setting.usage_ping_features_enabled).to be_truthy + end + + context 'when Gitlab.ee? is true', if: Gitlab.ee? do + context 'when include_optional_metrics_in_service_ping is true' do + before do + setting.include_optional_metrics_in_service_ping = true + end + + it 'returns true for usage_ping_features_enabled' do + expect(setting.usage_ping_features_enabled).to be_truthy + end + end + + context 'when include_optional_metrics_in_service_ping is false' do + before do + setting.include_optional_metrics_in_service_ping = false + end + + it 'returns false for usage_ping_features_enabled' do + expect(setting.usage_ping_features_enabled).to be_falsey + end + end + end + end + + context 'when usage_ping_features_enabled is disabled in db' do + before do + setting.usage_ping_features_enabled = false + end + + it 'returns false for usage_ping_features_enabled' do + expect(setting.usage_ping_features_enabled).to be_falsey + end + end end end end