From ca48799ecb84e41435fa8005f926bf929b47d6d9 Mon Sep 17 00:00:00 2001 From: Ankit Date: Thu, 11 Jan 2024 00:36:56 +0530 Subject: [PATCH 01/12] Add new checkbox for optional data in service ping Changelog: added --- .../setup_metrics_and_profiling.js | 5 +- .../metrics_and_profiling/constants.js | 25 +++++ .../metrics_and_profiling/usage_statistics.js | 83 ++++++++++++----- app/helpers/application_settings_helper.rb | 1 + .../application_setting_implementation.rb | 16 +++- .../application_settings/_usage.html.haml | 45 ++++----- ...metrics_enabled_to_application_settings.rb | 16 ++++ ...date_optiona_metrics_value_service_ping.rb | 20 ++++ db/schema_migrations/20240110160643 | 1 + db/schema_migrations/20240110160816 | 1 + db/structure.sql | 1 + doc/api/settings.md | 1 + ...ional_metric_in_service_ping_checkbox.haml | 12 +++ .../ee/service_ping/permit_data_categories.rb | 2 +- .../ee/service_ping/service_ping_settings.rb | 5 + .../service_ping_settings_spec.rb | 17 ++++ lib/service_ping/service_ping_settings.rb | 4 + locale/gitlab.pot | 33 +++++-- .../frontend/fixtures/application_settings.rb | 1 + .../usage_statistics_spec.js | 91 +++++++++---------- .../service_ping_settings_spec.rb | 6 ++ 21 files changed, 285 insertions(+), 101 deletions(-) create mode 100644 app/assets/javascripts/pages/admin/application_settings/metrics_and_profiling/constants.js create mode 100644 db/migrate/20240110160643_add_optional_metrics_enabled_to_application_settings.rb create mode 100644 db/migrate/20240110160816_update_optiona_metrics_value_service_ping.rb create mode 100644 db/schema_migrations/20240110160643 create mode 100644 db/schema_migrations/20240110160816 create mode 100644 ee/app/views/admin/application_settings/_include_optional_metric_in_service_ping_checkbox.haml 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 cfa2f4b8762cb0..3da138c256c809 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 00000000000000..e67c35c5e61ee8 --- /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. To continue using Registration Features in the future, you will also need to register with GitLab via a new cloud licensing service.', +); + +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. To continue using Registration Features in the future, you will also need to register with GitLab via a new cloud licensing service.', +); + +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 68849857d0fa6b..ca0e547ae3e2ae 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 d048bfe9ce041a..540f93611c77d9 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 fed0f2b67aa4c3..ea1e708154e0d2 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, @@ -516,12 +517,25 @@ def usage_ping_can_be_configured? end def usage_ping_features_enabled? - usage_ping_enabled? && usage_ping_features_enabled + if Gitlab.ee? + include_optional_metrics_in_service_ping && usage_ping_features_enabled + else + usage_ping_enabled? && usage_ping_features_enabled + end end def usage_ping_enabled + # 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 + + if Gitlab.ee? && ::License.current&.customer_service_enabled? + true + end + 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 dd9820d064a781..5cf9e6a4e3a7ca 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. 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.') + - 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. 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 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: '%{help_text}'.html_safe % { help_text: Gitlab.ee? ? optional_metrics_help_text : service_ping_help_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 00000000000000..4accec63f9e530 --- /dev/null +++ b/db/migrate/20240110160643_add_optional_metrics_enabled_to_application_settings.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +# See https://docs.gitlab.com/ee/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +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 00000000000000..14900541682b19 --- /dev/null +++ b/db/migrate/20240110160816_update_optiona_metrics_value_service_ping.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +# See https://docs.gitlab.com/ee/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +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 00000000000000..f228bf854369af --- /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 00000000000000..c01d2939a782f2 --- /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 8eef253e0d61da..8b39612e659356 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 ea40fdb5d9eb38..d1ff481c63bc1e 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 is enabled in service ping | | `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 00000000000000..37ad446ae17938 --- /dev/null +++ b/ee/app/views/admin/application_settings/_include_optional_metric_in_service_ping_checkbox.haml @@ -0,0 +1,12 @@ +- 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, + s_('AdminSettings|Include Optional data in Service Ping.'), + help_text: _('Include additional optional data in Service Ping. To enable Optional Data, first enable Service Ping.'), + checkbox_options: { disabled: optional_metrics_disabled } + -# TODO fix the link, how to add handbook link here, currently it is pointing to docs.gitlab.com + .form-text.gl-text-gray-500.gl-pl-6 + %p + - service_ping_handbook_link = link_to('the documentation', '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 9dc8dbf67240a8..ff7da97ddcdef9 100644 --- a/ee/lib/ee/service_ping/permit_data_categories.rb +++ b/ee/lib/ee/service_ping/permit_data_categories.rb @@ -15,7 +15,7 @@ def execute return super unless ::License.current.present? return super unless ::ServicePing::ServicePingSettings.enabled_and_consented? - optional_enabled = ::Gitlab::CurrentSettings.usage_ping_enabled? + optional_enabled = ::Gitlab::CurrentSettings.include_optional_metrics_in_service_ping? customer_service_enabled = ::License.current.customer_service_enabled? [STANDARD_CATEGORY, SUBSCRIPTION_CATEGORY].tap do |categories| diff --git a/ee/lib/ee/service_ping/service_ping_settings.rb b/ee/lib/ee/service_ping/service_ping_settings.rb index 05e382cb1309ef..1068cb26d0a097 100644 --- a/ee/lib/ee/service_ping/service_ping_settings.rb +++ b/ee/lib/ee/service_ping/service_ping_settings.rb @@ -5,6 +5,11 @@ module ServicePing module ServicePingSettings extend ::Gitlab::Utils::Override + override :license_operational_metric_enabled? + def license_operational_metric_enabled? + ::License.current&.customer_service_enabled? + end + override :enabled? def enabled? ::License.current&.customer_service_enabled? || super 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 856b6603ad52c2..ab6c45b953d8e7 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 8c99f1138c5acd..2fa1c86ed1d47a 100644 --- a/lib/service_ping/service_ping_settings.rb +++ b/lib/service_ping/service_ping_settings.rb @@ -8,6 +8,10 @@ def enabled_and_consented? enabled? && !User.single_user&.requires_usage_stats_consent? end + def license_operational_metric_enabled? + false + end + def enabled? ::Gitlab::CurrentSettings.usage_ping_enabled? end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 69f58b20995643..387e8b88c73443 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 "" @@ -3586,6 +3589,9 @@ msgstr "" msgid "AdminSettings|Instance runners details" msgstr "" +msgid "AdminSettings|Include Optional data in Service Ping." +msgstr "" + msgid "AdminSettings|Instance runners expiration" msgstr "" @@ -3781,6 +3787,9 @@ msgstr "" msgid "AdminSettings|This limit cannot be disabled. Set to 0 to block all DAG dependencies." msgstr "" +msgid "AdminSettings|To enable Registration Features, first enable Optional data in Service Ping." +msgstr "" + msgid "AdminSettings|To enable Registration Features, first enable Service Ping." msgstr "" @@ -3802,6 +3811,9 @@ msgstr "" msgid "AdminSettings|When to delete inactive projects" msgstr "" +msgid "AdminSettings|You can enable Registration Features because Optional data in 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 "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." msgstr "" @@ -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. To continue using Registration Features in the future, you will also need to register with GitLab via a new cloud licensing service." +msgstr "" + +msgid "ApplicationSettings|You can enable Registration Features because optional data in 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 "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 34e99ec647cd6f..060622d97885d2 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 72d2bb0f983cf9..89ab3d559d7728 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,55 @@ -import htmlApplicationSettingsUsage from 'test_fixtures/application_settings/usage.html'; -import initSetHelperText, { - HELPER_TEXT_SERVICE_PING_DISABLED, - HELPER_TEXT_SERVICE_PING_ENABLED, +import { + 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; +import * as constants from '~/pages/admin/application_settings/metrics_and_profiling/constants'; +describe('Optional Metrics Tests', () => { 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'); - }); - - afterEach(() => { - resetHTMLFixture(); + document.body.innerHTML = ` +
+ + + + + `; }); - const expectEnabledservicePingFeaturesCheckBox = () => { - expect(servicePingFeaturesCheckBox.classList.contains('gl-cursor-not-allowed')).toBe(false); - expect(servicePingFeaturesHelperText.textContent).toEqual(HELPER_TEXT_SERVICE_PING_ENABLED); - }; - - const expectDisabledservicePingFeaturesCheckBox = () => { - expect(servicePingFeaturesLabel.classList.contains('gl-cursor-not-allowed')).toBe(true); - expect(servicePingFeaturesHelperText.textContent).toEqual(HELPER_TEXT_SERVICE_PING_DISABLED); - }; + 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); - describe('Registration Features checkbox', () => { - it('is disabled when Service Ping checkbox is unchecked', () => { - expect(servicePingCheckBox.checked).toBe(false); - expectDisabledservicePingFeaturesCheckBox(); + const helperText = document.getElementById('service_ping_features_helper_text').textContent; + expect(helperText).toEqual(constants.HELPER_TEXT_OPTIONAL_METRICS_ENABLED); }); - it('is enabled when Servie Ping checkbox is checked', () => { - servicePingCheckBox.click(); - expect(servicePingCheckBox.checked).toBe(true); - expectEnabledservicePingFeaturesCheckBox(); - }); + 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); - it('is switched to disabled when Service Ping checkbox is unchecked', () => { - servicePingCheckBox.click(); - servicePingFeaturesCheckBox.click(); - expectEnabledservicePingFeaturesCheckBox(); + const helperText = document.getElementById('service_ping_features_helper_text').textContent; + expect(helperText).toEqual(constants.HELPER_TEXT_OPTIONAL_METRICS_DISABLED); + }); + }); - servicePingCheckBox.click(); - expect(servicePingCheckBox.checked).toBe(false); - expect(servicePingFeaturesCheckBox.checked).toBe(false); - expectDisabledservicePingFeaturesCheckBox(); + 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); }); }); }); diff --git a/spec/lib/service_ping/service_ping_settings_spec.rb b/spec/lib/service_ping/service_ping_settings_spec.rb index 2e61b35a13198a..e920f6a21a1486 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 be_falsey + end + end + describe '#enabled?' do describe 'has the correct enabled' do it 'when false' do -- GitLab From 7638cbda28f8a9c76eec84c5b5d58fccb718c6d6 Mon Sep 17 00:00:00 2001 From: Piotr Skorupa Date: Wed, 7 Feb 2024 04:02:58 +0100 Subject: [PATCH 02/12] Rework categories to support new optional data option --- .../application_setting_implementation.rb | 22 ++++++--------- .../ee/service_ping/permit_data_categories.rb | 9 ++---- .../lib/ee/service_ping/build_payload_spec.rb | 16 +++++++---- .../permit_data_categories_spec.rb | 28 +++++++++++++++++-- lib/service_ping/service_ping_settings.rb | 5 ++++ 5 files changed, 52 insertions(+), 28 deletions(-) diff --git a/app/models/application_setting_implementation.rb b/app/models/application_setting_implementation.rb index ea1e708154e0d2..d212f574b77df0 100644 --- a/app/models/application_setting_implementation.rb +++ b/app/models/application_setting_implementation.rb @@ -516,23 +516,17 @@ def usage_ping_can_be_configured? Settings.gitlab.usage_ping_enabled end - def usage_ping_features_enabled? - if Gitlab.ee? - include_optional_metrics_in_service_ping && usage_ping_features_enabled - else - usage_ping_enabled? && usage_ping_features_enabled - end - end + def usage_ping_features_enabled + return false unless usage_ping_enabled? && super - def usage_ping_enabled - # 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 + return include_optional_metrics_in_service_ping if Gitlab.ee? - if Gitlab.ee? && ::License.current&.customer_service_enabled? - true - end + true + end + alias_method :usage_ping_features_enabled?, :usage_ping_features_enabled + + def usage_ping_enabled usage_ping_can_be_configured? && super end diff --git a/ee/lib/ee/service_ping/permit_data_categories.rb b/ee/lib/ee/service_ping/permit_data_categories.rb index ff7da97ddcdef9..9edcc7fdbe819d 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? - 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/spec/lib/ee/service_ping/build_payload_spec.rb b/ee/spec/lib/ee/service_ping/build_payload_spec.rb index 84c3053b396d93..855b1f5c4a4a96 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 654be80ab112a6..1dd83b89bf0df8 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/lib/service_ping/service_ping_settings.rb b/lib/service_ping/service_ping_settings.rb index 2fa1c86ed1d47a..6f055d5bd4c191 100644 --- a/lib/service_ping/service_ping_settings.rb +++ b/lib/service_ping/service_ping_settings.rb @@ -13,6 +13,11 @@ def license_operational_metric_enabled? end def enabled? + # 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 + return true if Gitlab.ee? && ::License.current&.customer_service_enabled? + ::Gitlab::CurrentSettings.usage_ping_enabled? end end -- GitLab From 68c74dfa395db29071ce36ed17f8bc6616a0fbaa Mon Sep 17 00:00:00 2001 From: Ankit Date: Wed, 7 Feb 2024 18:45:56 +0530 Subject: [PATCH 03/12] worked on feedback --- .../application_settings/_usage.html.haml | 4 +- ...metrics_enabled_to_application_settings.rb | 3 - ...date_optiona_metrics_value_service_ping.rb | 3 - ...ional_metric_in_service_ping_checkbox.haml | 14 ++-- locale/gitlab.pot | 10 +-- .../usage_statistics_spec.js | 75 ++++++++++++++++++- 6 files changed, 86 insertions(+), 23 deletions(-) diff --git a/app/views/admin/application_settings/_usage.html.haml b/app/views/admin/application_settings/_usage.html.haml index 5cf9e6a4e3a7ca..9534a1efd61148 100644 --- a/app/views/admin/application_settings/_usage.html.haml +++ b/app/views/admin/application_settings/_usage.html.haml @@ -44,9 +44,9 @@ - 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') - service_ping_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.') - - 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. 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 Optional data in 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. 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 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: Gitlab.ee? ? optional_metrics_help_text : service_ping_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 index 4accec63f9e530..dd8990ef086679 100644 --- a/db/migrate/20240110160643_add_optional_metrics_enabled_to_application_settings.rb +++ b/db/migrate/20240110160643_add_optional_metrics_enabled_to_application_settings.rb @@ -1,8 +1,5 @@ # frozen_string_literal: true -# See https://docs.gitlab.com/ee/development/migration_style_guide.html -# for more information on how to write migrations for GitLab. - class AddOptionalMetricsEnabledToApplicationSettings < Gitlab::Database::Migration[2.2] milestone '16.9' diff --git a/db/migrate/20240110160816_update_optiona_metrics_value_service_ping.rb b/db/migrate/20240110160816_update_optiona_metrics_value_service_ping.rb index 14900541682b19..e0034c5d2d295a 100644 --- a/db/migrate/20240110160816_update_optiona_metrics_value_service_ping.rb +++ b/db/migrate/20240110160816_update_optiona_metrics_value_service_ping.rb @@ -1,8 +1,5 @@ # frozen_string_literal: true -# See https://docs.gitlab.com/ee/development/migration_style_guide.html -# for more information on how to write migrations for GitLab. - class UpdateOptionaMetricsValueServicePing < Gitlab::Database::Migration[2.2] restrict_gitlab_migration gitlab_schema: :gitlab_main milestone '16.9' 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 index 37ad446ae17938..eeb9e26084f5f1 100644 --- 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 @@ -1,12 +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, - s_('AdminSettings|Include Optional data in Service Ping.'), - help_text: _('Include additional optional data in Service Ping. To enable Optional Data, first enable Service Ping.'), - checkbox_options: { disabled: optional_metrics_disabled } - -# TODO fix the link, how to add handbook link here, currently it is pointing to docs.gitlab.com - .form-text.gl-text-gray-500.gl-pl-6 - %p - - service_ping_handbook_link = link_to('the documentation', 'https://handbook.gitlab.com/handbook/legal/privacy/customer-product-usage-information#service-ping-formerly-known-as-usage-ping', target: '_blank', rel: 'noopener noreferrer') + = 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/locale/gitlab.pot b/locale/gitlab.pot index 387e8b88c73443..cf7715ee707979 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -3787,10 +3787,10 @@ msgstr "" msgid "AdminSettings|This limit cannot be disabled. Set to 0 to block all DAG dependencies." msgstr "" -msgid "AdminSettings|To enable Registration Features, first enable Optional data in Service Ping." +msgid "AdminSettings|To enable Registration Features, first enable Service Ping." msgstr "" -msgid "AdminSettings|To enable Registration Features, first enable Service Ping." +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}" @@ -3811,10 +3811,10 @@ msgstr "" msgid "AdminSettings|When to delete inactive projects" msgstr "" -msgid "AdminSettings|You can enable Registration Features because Optional data in 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. To continue using Registration Features in the future, you will also need to register with GitLab via a new cloud licensing service." 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 optional data in 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 "AdminSettings|You can't delete projects before the warning email is sent." @@ -26140,7 +26140,7 @@ msgstr "" msgid "Incident|Timeline text..." msgstr "" -msgid "Include additional optional data in Service Ping. To enable Optional Data, first enable Service Ping." +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" 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 89ab3d559d7728..fda9066784f8c8 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,10 +1,10 @@ -import { +import initSetHelperText, { setHelperText, checkOptionalMetrics, } from '~/pages/admin/application_settings/metrics_and_profiling/usage_statistics'; import * as constants from '~/pages/admin/application_settings/metrics_and_profiling/constants'; -describe('Optional Metrics Tests', () => { +describe('Optional Metrics Tests for EE', () => { beforeEach(() => { document.body.innerHTML = `
@@ -52,4 +52,75 @@ describe('Optional Metrics Tests', () => { 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); + + optionalMetricsServicePingCheckbox.checked = false; + optionalMetricsServicePingCheckbox.dispatchEvent(new Event('change')); + + 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(); + + 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).toEqual(constants.HELPER_TEXT_SERVICE_PING_ENABLED); + + servicePingCheckbox.checked = false; + servicePingCheckbox.dispatchEvent(new Event('change')); + + helperText = document.getElementById('service_ping_features_helper_text').textContent; + expect(helperText).toEqual(constants.HELPER_TEXT_SERVICE_PING_DISABLED); + }); + 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); + + servicePingCheckbox.checked = false; + servicePingCheckbox.dispatchEvent(new Event('change')); + + expect(servicePingFeaturesCheckbox.disabled).toBe(true); + }); + }); }); -- GitLab From 85a974acd550bae0d8c90226f81ffe725c95134d Mon Sep 17 00:00:00 2001 From: Piotr Skorupa Date: Thu, 8 Feb 2024 06:14:02 +0100 Subject: [PATCH 04/12] Refactor ServicePingSettings conditionals --- ee/lib/ee/service_ping/service_ping_settings.rb | 5 ----- lib/service_ping/service_ping_settings.rb | 10 ++++------ 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/ee/lib/ee/service_ping/service_ping_settings.rb b/ee/lib/ee/service_ping/service_ping_settings.rb index 1068cb26d0a097..7d4cc84a9437fa 100644 --- a/ee/lib/ee/service_ping/service_ping_settings.rb +++ b/ee/lib/ee/service_ping/service_ping_settings.rb @@ -9,11 +9,6 @@ module ServicePingSettings def license_operational_metric_enabled? ::License.current&.customer_service_enabled? end - - override :enabled? - def enabled? - ::License.current&.customer_service_enabled? || super - end end end end diff --git a/lib/service_ping/service_ping_settings.rb b/lib/service_ping/service_ping_settings.rb index 6f055d5bd4c191..c5aed5132c0f34 100644 --- a/lib/service_ping/service_ping_settings.rb +++ b/lib/service_ping/service_ping_settings.rb @@ -12,13 +12,11 @@ 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? - # 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 - return true if Gitlab.ee? && ::License.current&.customer_service_enabled? - - ::Gitlab::CurrentSettings.usage_ping_enabled? + license_operational_metric_enabled? || ::Gitlab::CurrentSettings.usage_ping_enabled? end end end -- GitLab From b209a93cc0bf7da6e85c075c413c91d157c2e531 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Wielich?= Date: Thu, 8 Feb 2024 05:16:45 +0000 Subject: [PATCH 05/12] Fix docs grammar --- doc/api/settings.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/api/settings.md b/doc/api/settings.md index d1ff481c63bc1e..5cd28211405b4f 100644 --- a/doc/api/settings.md +++ b/doc/api/settings.md @@ -624,7 +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 is enabled in service ping | +| `include_optional_metrics_in_service_ping`| boolean | no | Whether or not optional metrics are enabled in service ping | | `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. | -- GitLab From 70390727672ce9ef161e25cac7cf638b5fc57886 Mon Sep 17 00:00:00 2001 From: Piotr Skorupa Date: Thu, 8 Feb 2024 09:46:59 +0100 Subject: [PATCH 06/12] Fix undercoverage for usage_ping_features_enabled --- .../application_setting_shared_examples.rb | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+) 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 70179dd7fc7deb..fb9fd2f0f73f56 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 -- GitLab From 96041ef301b23f4db1e5e1fc56ae408a4cbf919a Mon Sep 17 00:00:00 2001 From: Ankit Date: Fri, 9 Feb 2024 14:47:23 +0530 Subject: [PATCH 07/12] fixed casing --- doc/api/settings.md | 2 +- locale/gitlab.pot | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/api/settings.md b/doc/api/settings.md index 5cd28211405b4f..9a630f4a2c5ab1 100644 --- a/doc/api/settings.md +++ b/doc/api/settings.md @@ -624,7 +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 | +| `include_optional_metrics_in_service_ping`| boolean | no | Whether or not optional metrics are enabled in Service Ping | | `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/locale/gitlab.pot b/locale/gitlab.pot index cf7715ee707979..b1e0c9ba57450b 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -3586,10 +3586,10 @@ msgstr "" msgid "AdminSettings|Inactive project deletion" msgstr "" -msgid "AdminSettings|Instance runners details" +msgid "AdminSettings|Include optional data in Service Ping." msgstr "" -msgid "AdminSettings|Include Optional data in Service Ping." +msgid "AdminSettings|Instance runners details" msgstr "" msgid "AdminSettings|Instance runners expiration" -- GitLab From 617ddf7a0f8396e5b857755282c5f4211447d639 Mon Sep 17 00:00:00 2001 From: Ankit Date: Mon, 12 Feb 2024 15:01:54 +0530 Subject: [PATCH 08/12] worked on feedback --- .../metrics_and_profiling/constants.js | 4 ++-- app/views/admin/application_settings/_usage.html.haml | 4 ++-- locale/gitlab.pot | 8 ++++---- 3 files changed, 8 insertions(+), 8 deletions(-) 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 index e67c35c5e61ee8..e76bed66ced9a3 100644 --- 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 @@ -5,7 +5,7 @@ export const HELPER_TEXT_SERVICE_PING_DISABLED = s__( ); export const HELPER_TEXT_SERVICE_PING_ENABLED = s__( - 'ApplicationSettings|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.', + 'ApplicationSettings|You can enable Registration Features because Service Ping is enabled.', ); export const HELPER_TEXT_OPTIONAL_METRICS_DISABLED = s__( @@ -13,7 +13,7 @@ export const HELPER_TEXT_OPTIONAL_METRICS_DISABLED = s__( ); export const HELPER_TEXT_OPTIONAL_METRICS_ENABLED = s__( - 'ApplicationSettings|You can enable Registration Features because optional data in 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.', + 'ApplicationSettings|You can enable Registration Features because optional data in Service Ping is enabled.', ); export const ELEMENT_IDS = Object.freeze({ diff --git a/app/views/admin/application_settings/_usage.html.haml b/app/views/admin/application_settings/_usage.html.haml index 9534a1efd61148..bdaef614033d4a 100644 --- a/app/views/admin/application_settings/_usage.html.haml +++ b/app/views/admin/application_settings/_usage.html.haml @@ -43,8 +43,8 @@ - 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') - - service_ping_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.') - - 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. 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 optional data in 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: 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' }, diff --git a/locale/gitlab.pot b/locale/gitlab.pot index b1e0c9ba57450b..c676a089458f9d 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -3811,10 +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. 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 optional data in Service Ping is enabled." msgstr "" msgid "AdminSettings|You can't delete projects before the warning email is sent." @@ -6151,10 +6151,10 @@ 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. To continue using Registration Features in the future, you will also need to register with GitLab via a new cloud licensing service." +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. To continue using Registration Features in the future, you will also need to register with GitLab via a new cloud licensing service." +msgid "ApplicationSettings|You can enable Registration Features because optional data in Service Ping is enabled." msgstr "" msgid "ApplicationSettings|domain.com" -- GitLab From ec01e1a5d3826fa4e14e3af0236a879c4b9642c3 Mon Sep 17 00:00:00 2001 From: Amy Qualls Date: Tue, 13 Feb 2024 03:59:24 +0000 Subject: [PATCH 09/12] Apply 1 suggestion(s) to 1 file(s) --- doc/api/settings.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/api/settings.md b/doc/api/settings.md index 9a630f4a2c5ab1..074dc26596fb61 100644 --- a/doc/api/settings.md +++ b/doc/api/settings.md @@ -624,7 +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 | +| `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. | -- GitLab From ffa751ebd379a57db15f4d53f18d3c1047c0ad55 Mon Sep 17 00:00:00 2001 From: Ankit Date: Tue, 13 Feb 2024 09:35:18 +0530 Subject: [PATCH 10/12] worked on suggestion --- .../usage_statistics_spec.js | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) 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 fda9066784f8c8..f57d7ee15b2346 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 @@ -2,7 +2,6 @@ import initSetHelperText, { setHelperText, checkOptionalMetrics, } from '~/pages/admin/application_settings/metrics_and_profiling/usage_statistics'; -import * as constants from '~/pages/admin/application_settings/metrics_and_profiling/constants'; describe('Optional Metrics Tests for EE', () => { beforeEach(() => { @@ -24,7 +23,9 @@ describe('Optional Metrics Tests for EE', () => { setHelperText(optionalMetricsServicePingCheckbox); const helperText = document.getElementById('service_ping_features_helper_text').textContent; - expect(helperText).toEqual(constants.HELPER_TEXT_OPTIONAL_METRICS_ENABLED); + 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', () => { @@ -35,7 +36,9 @@ describe('Optional Metrics Tests for EE', () => { setHelperText(optionalMetricsServicePingCheckbox); const helperText = document.getElementById('service_ping_features_helper_text').textContent; - expect(helperText).toEqual(constants.HELPER_TEXT_OPTIONAL_METRICS_DISABLED); + expect(helperText).toBe( + 'To enable Registration Features, first enable optional data in Service Ping.', + ); }); }); @@ -96,13 +99,15 @@ describe('Without Optional Metrics Checkbox for CE', () => { servicePingCheckbox.dispatchEvent(new Event('change')); let helperText = document.getElementById('service_ping_features_helper_text').textContent; - expect(helperText).toEqual(constants.HELPER_TEXT_SERVICE_PING_ENABLED); + 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).toEqual(constants.HELPER_TEXT_SERVICE_PING_DISABLED); + 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(); -- GitLab From 7e002708a9b981002c1b4e7768f594396b2349df Mon Sep 17 00:00:00 2001 From: Piotr Skorupa Date: Fri, 16 Feb 2024 18:25:13 +0100 Subject: [PATCH 11/12] Add usage_ping_features_enabled default --- app/models/application_setting_implementation.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/models/application_setting_implementation.rb b/app/models/application_setting_implementation.rb index d212f574b77df0..777267cb19565e 100644 --- a/app/models/application_setting_implementation.rb +++ b/app/models/application_setting_implementation.rb @@ -234,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, -- GitLab From 73e9d51c0eb982c960273cbcc24ee9c4fc9e1641 Mon Sep 17 00:00:00 2001 From: Piotr Skorupa Date: Fri, 16 Feb 2024 18:25:42 +0100 Subject: [PATCH 12/12] Fix license_operational_metric_enabled undercoverage --- ee/lib/ee/service_ping/service_ping_settings.rb | 2 +- spec/lib/service_ping/service_ping_settings_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ee/lib/ee/service_ping/service_ping_settings.rb b/ee/lib/ee/service_ping/service_ping_settings.rb index 7d4cc84a9437fa..dee4dc29fa4cd5 100644 --- a/ee/lib/ee/service_ping/service_ping_settings.rb +++ b/ee/lib/ee/service_ping/service_ping_settings.rb @@ -7,7 +7,7 @@ module ServicePingSettings override :license_operational_metric_enabled? def license_operational_metric_enabled? - ::License.current&.customer_service_enabled? + ::License.current&.customer_service_enabled? || super end end end diff --git a/spec/lib/service_ping/service_ping_settings_spec.rb b/spec/lib/service_ping/service_ping_settings_spec.rb index e920f6a21a1486..ccf2564ca8caf3 100644 --- a/spec/lib/service_ping/service_ping_settings_spec.rb +++ b/spec/lib/service_ping/service_ping_settings_spec.rb @@ -31,7 +31,7 @@ describe '#license_operational_metric_enabled?' do it 'returns false' do - expect(described_class.license_operational_metric_enabled?).to be_falsey + expect(described_class.license_operational_metric_enabled?).to eq(false) end end -- GitLab