diff options
author | Luke Bennett <lbennett@gitlab.com> | 2018-09-06 12:43:14 +0000 |
---|---|---|
committer | Phil Hughes <me@iamphill.com> | 2018-09-06 12:43:14 +0000 |
commit | 7e7ca9faf1f8270d54f7731747db0d44c928492c (patch) | |
tree | 64294529de4536a0cc2e66dbc6bdd6c9f5547ae3 | |
parent | 55b5264043bdecbbb5971c55c27ccbbeb0eb041e (diff) | |
download | gitlab-ce-7e7ca9faf1f8270d54f7731747db0d44c928492c.tar.gz |
Resolve "Remove usage ping payload from Cohorts, add to Settings"
19 files changed, 225 insertions, 70 deletions
diff --git a/app/assets/javascripts/pages/admin/application_settings/index.js b/app/assets/javascripts/pages/admin/application_settings/index.js index 47bd70537f1..069f8ce55d0 100644 --- a/app/assets/javascripts/pages/admin/application_settings/index.js +++ b/app/assets/javascripts/pages/admin/application_settings/index.js @@ -1,8 +1,13 @@ import initSettingsPanels from '~/settings_panels'; import projectSelect from '~/project_select'; +import UsagePingPayload from './usage_ping_payload'; document.addEventListener('DOMContentLoaded', () => { // Initialize expandable settings panels initSettingsPanels(); projectSelect(); + new UsagePingPayload( + document.querySelector('.js-usage-ping-payload-trigger'), + document.querySelector('.js-usage-ping-payload'), + ).init(); }); diff --git a/app/assets/javascripts/pages/admin/application_settings/usage_ping_payload.js b/app/assets/javascripts/pages/admin/application_settings/usage_ping_payload.js new file mode 100644 index 00000000000..9a1bc46bf4a --- /dev/null +++ b/app/assets/javascripts/pages/admin/application_settings/usage_ping_payload.js @@ -0,0 +1,62 @@ +import axios from '../../../lib/utils/axios_utils'; +import { __ } from '../../../locale'; +import flash from '../../../flash'; + +export default class UsagePingPayload { + constructor(trigger, container) { + this.trigger = trigger; + this.container = container; + this.isVisible = false; + this.isInserted = false; + } + + init() { + this.spinner = this.trigger.querySelector('.js-spinner'); + this.text = this.trigger.querySelector('.js-text'); + + this.trigger.addEventListener('click', event => { + event.preventDefault(); + + if (this.isVisible) return this.hidePayload(); + + return this.requestPayload(); + }); + } + + requestPayload() { + if (this.isInserted) return this.showPayload(); + + this.spinner.classList.add('d-inline'); + + return axios + .get(this.container.dataset.endpoint, { + responseType: 'text', + }) + .then(({ data }) => { + this.spinner.classList.remove('d-inline'); + this.insertPayload(data); + }) + .catch(() => { + this.spinner.classList.remove('d-inline'); + flash(__('Error fetching usage ping data.')); + }); + } + + hidePayload() { + this.isVisible = false; + this.container.classList.add('d-none'); + this.text.textContent = __('Preview payload'); + } + + showPayload() { + this.isVisible = true; + this.container.classList.remove('d-none'); + this.text.textContent = __('Hide payload'); + } + + insertPayload(data) { + this.isInserted = true; + this.container.innerHTML = data; + this.showPayload(); + } +} diff --git a/app/assets/javascripts/pages/instance_statistics/cohorts/index.js b/app/assets/javascripts/pages/instance_statistics/cohorts/index.js deleted file mode 100644 index 2d5020dbef4..00000000000 --- a/app/assets/javascripts/pages/instance_statistics/cohorts/index.js +++ /dev/null @@ -1,3 +0,0 @@ -import initUsagePing from './usage_ping'; - -document.addEventListener('DOMContentLoaded', initUsagePing); diff --git a/app/assets/javascripts/pages/instance_statistics/cohorts/usage_ping.js b/app/assets/javascripts/pages/instance_statistics/cohorts/usage_ping.js deleted file mode 100644 index 914a9661c27..00000000000 --- a/app/assets/javascripts/pages/instance_statistics/cohorts/usage_ping.js +++ /dev/null @@ -1,13 +0,0 @@ -import axios from '../../../lib/utils/axios_utils'; -import { __ } from '../../../locale'; -import flash from '../../../flash'; - -export default function UsagePing() { - const el = document.querySelector('.usage-data'); - - axios.get(el.dataset.endpoint, { - responseType: 'text', - }).then(({ data }) => { - el.innerHTML = data; - }).catch(() => flash(__('Error fetching usage ping data.'))); -} diff --git a/app/assets/stylesheets/pages/admin.scss b/app/assets/stylesheets/pages/admin.scss index 6c555aee20a..f0acb78f731 100644 --- a/app/assets/stylesheets/pages/admin.scss +++ b/app/assets/stylesheets/pages/admin.scss @@ -4,3 +4,7 @@ padding-bottom: 46px; } } + +.usage-data { + max-height: 400px; +} diff --git a/app/controllers/instance_statistics/cohorts_controller.rb b/app/controllers/instance_statistics/cohorts_controller.rb index 7eba0a5ecdd..4b4e39db2e1 100644 --- a/app/controllers/instance_statistics/cohorts_controller.rb +++ b/app/controllers/instance_statistics/cohorts_controller.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class InstanceStatistics::CohortsController < InstanceStatistics::ApplicationController + before_action :authenticate_usage_ping_enabled_or_admin! + def index if Gitlab::CurrentSettings.usage_ping_enabled cohorts_results = Rails.cache.fetch('cohorts', expires_in: 1.day) do @@ -10,4 +12,8 @@ class InstanceStatistics::CohortsController < InstanceStatistics::ApplicationCon @cohorts = CohortsSerializer.new.represent(cohorts_results) end end + + def authenticate_usage_ping_enabled_or_admin! + render_404 unless Gitlab::CurrentSettings.usage_ping_enabled || current_user.admin? + end end diff --git a/app/views/admin/application_settings/_usage.html.haml b/app/views/admin/application_settings/_usage.html.haml index 2495defb6a7..788595877ea 100644 --- a/app/views/admin/application_settings/_usage.html.haml +++ b/app/views/admin/application_settings/_usage.html.haml @@ -2,7 +2,7 @@ = form_errors(@application_setting) %fieldset - .form-group + .form-group.mb-2 .form-check = f.check_box :version_check_enabled, class: 'form-check-input' = f.label :version_check_enabled, class: 'form-check-label' do @@ -16,23 +16,26 @@ .form-check = f.check_box :usage_ping_enabled, disabled: !can_be_configured, class: 'form-check-input' = f.label :usage_ping_enabled, class: 'form-check-label' do - Enable usage ping + = _('Enable usage ping') .form-text.text-muted - if can_be_configured - To help improve GitLab and its user experience, GitLab will - periodically collect usage information. - = link_to 'Learn more', help_page_path("user/admin_area/settings/usage_statistics", anchor: "usage-ping") - about what information is shared with GitLab Inc. Visit - = link_to _('Cohorts'), instance_statistics_cohorts_path(anchor: 'usage-ping') - to see the JSON payload sent. + %p.mb-2= _('To help improve GitLab and its user experience, GitLab will periodically collect usage information.') + + - usage_ping_path = help_page_path('user/admin_area/settings/usage_statistics', anchor: 'usage-ping') + - usage_ping_link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe % { url: usage_ping_path } + %p.mb-2= s_('%{usage_ping_link_start}Learn more%{usage_ping_link_end} about what information is shared with GitLab Inc.').html_safe % { usage_ping_link_start: usage_ping_link_start, usage_ping_link_end: '</a>'.html_safe } + + %button.btn.js-usage-ping-payload-trigger{ type: 'button' } + .js-spinner.d-none= icon('spinner spin') + .js-text.d-inline= _('Preview payload') + %pre.usage-data.js-usage-ping-payload.js-syntax-highlight.code.highlight.mt-2.d-none{ data: { endpoint: usage_data_admin_application_settings_path(format: :html) } } - else - The usage ping is disabled, and cannot be configured through this - form. For more information, see the documentation on - = succeed '.' do - = link_to 'deactivating the usage ping', help_page_path('user/admin_area/settings/usage_statistics', anchor: 'deactivate-the-usage-ping') - .form-group + = _('The usage ping is disabled, and cannot be configured through this form.') + - deactivating_usage_ping_path = help_page_path('user/admin_area/settings/usage_statistics', anchor: 'deactivate-the-usage-ping') + - deactivating_usage_ping_link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe % { url: deactivating_usage_ping_path } + = s_('For more information, see the documentation on %{deactivating_usage_ping_link_start}deactivating the usage ping%{deactivating_usage_ping_link_end}.').html_safe % { deactivating_usage_ping_link_start: deactivating_usage_ping_link_start, deactivating_usage_ping_link_end: '</a>'.html_safe } + .form-group.mt-3 = f.label :instance_statistics_visibility_private, _('Instance Statistics visibility') = f.select :instance_statistics_visibility_private, options_for_select({_('All users') => false, _('Only admins') => true}, Gitlab::CurrentSettings.instance_statistics_visibility_private?), {}, class: 'form-control' = f.submit 'Save changes', class: "btn btn-success" - diff --git a/app/views/instance_statistics/cohorts/index.html.haml b/app/views/instance_statistics/cohorts/index.html.haml index 5e9a8c083af..e135bab10d8 100644 --- a/app/views/instance_statistics/cohorts/index.html.haml +++ b/app/views/instance_statistics/cohorts/index.html.haml @@ -1,16 +1,16 @@ -- breadcrumb_title "Cohorts" +- breadcrumb_title _("Cohorts") - @no_container = true %div{ class: container_class } - if @cohorts = render 'cohorts_table' - = render 'usage_ping' - else .bs-callout.bs-callout-warning.clearfix %p - User cohorts are only shown when the - = link_to 'usage ping', help_page_path('user/admin_area/settings/usage_statistics', anchor: 'usage-ping'), target: '_blank' - is enabled. To enable it and see user cohorts, - visit - = succeed '.' do - = link_to 'application settings', admin_application_settings_path(anchor: 'usage-statistics') + - usage_ping_path = help_page_path('user/admin_area/settings/usage_statistics', anchor: 'usage-ping') + - usage_ping_link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe % { url: usage_ping_path } + = s_('User Cohorts are only shown when the %{usage_ping_link_start}usage ping%{usage_ping_link_end} is enabled.').html_safe % { usage_ping_link_start: usage_ping_link_start, usage_ping_link_end: '</a>'.html_safe } + - if current_user.admin? + - application_settings_path = admin_application_settings_path(anchor: 'usage-statistics') + - application_settings_link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe % { url: application_settings_path } + = s_('To enable it and see User Cohorts, visit %{application_settings_link_start}application settings%{application_settings_link_end}.').html_safe % { application_settings_link_start: application_settings_link_start, application_settings_link_end: '</a>'.html_safe } diff --git a/app/views/instance_statistics/conversational_development_index/_disabled.html.haml b/app/views/instance_statistics/conversational_development_index/_disabled.html.haml index 0a741b50960..0a5717f75e1 100644 --- a/app/views/instance_statistics/conversational_development_index/_disabled.html.haml +++ b/app/views/instance_statistics/conversational_development_index/_disabled.html.haml @@ -1,9 +1,14 @@ .container.convdev-empty .col-sm-12.justify-content-center.text-center = custom_icon('convdev_no_index') - %h4 Usage ping is not enabled - %p - ConvDev is only shown when the - = link_to 'usage ping', help_page_path('user/admin_area/settings/usage_statistics'), target: '_blank' - is enabled. Enable usage ping to get an overview of how you are using GitLab from a feature perspective - = link_to 'Enable usage ping', admin_application_settings_path(anchor: 'usage-statistics'), class: 'btn btn-primary' + %h4= _('Usage ping is not enabled') + - if !current_user.admin? + %p + - usage_ping_path = help_page_path('user/admin_area/settings/usage_statistics', anchor: 'usage-ping') + - usage_ping_link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe % { url: usage_ping_path } + = s_('In order to enable instance-level analytics, please ask an admin to enable %{usage_ping_link_start}usage ping%{usage_ping_link_end}.').html_safe % { usage_ping_link_start: usage_ping_link_start, usage_ping_link_end: '</a>'.html_safe } + - if current_user.admin? + %p + = _('Enable usage ping to get an overview of how you are using GitLab from a feature perspective.') + - if current_user.admin? + = link_to _('Enable usage ping'), admin_application_settings_path(anchor: 'usage-statistics'), class: 'btn btn-primary' diff --git a/app/views/instance_statistics/conversational_development_index/index.html.haml b/app/views/instance_statistics/conversational_development_index/index.html.haml index dd63b98376f..1e7db4982d6 100644 --- a/app/views/instance_statistics/conversational_development_index/index.html.haml +++ b/app/views/instance_statistics/conversational_development_index/index.html.haml @@ -1,12 +1,13 @@ - @no_container = true -- page_title 'ConvDev Index' +- page_title _('ConvDev Index') +- usage_ping_enabled = Gitlab::CurrentSettings.usage_ping_enabled .container - - if show_callout?('convdev_intro_callout_dismissed') + - if usage_ping_enabled && show_callout?('convdev_intro_callout_dismissed') = render 'callout' .prepend-top-default - - if !Gitlab::CurrentSettings.usage_ping_enabled + - if !usage_ping_enabled = render 'disabled' - elsif @metric.blank? = render 'no_data' diff --git a/app/views/layouts/nav/sidebar/_instance_statistics.html.haml b/app/views/layouts/nav/sidebar/_instance_statistics.html.haml index b8ff448f261..57180f27146 100644 --- a/app/views/layouts/nav/sidebar/_instance_statistics.html.haml +++ b/app/views/layouts/nav/sidebar/_instance_statistics.html.haml @@ -18,16 +18,17 @@ %strong.fly-out-top-item-name = _('ConvDev Index') - = nav_link(controller: :cohorts) do - = link_to instance_statistics_cohorts_path do - .nav-icon-container - = sprite_icon('users') - %span.nav-item-name - = _('Cohorts') - %ul.sidebar-sub-level-items.is-fly-out-only - = nav_link(controller: :cohorts, html_options: { class: "fly-out-top-item" } ) do - = link_to instance_statistics_cohorts_path do - %strong.fly-out-top-item-name - = _('Cohorts') + - if Gitlab::CurrentSettings.usage_ping_enabled + = nav_link(controller: :cohorts) do + = link_to instance_statistics_cohorts_path do + .nav-icon-container + = sprite_icon('users') + %span.nav-item-name + = _('Cohorts') + %ul.sidebar-sub-level-items.is-fly-out-only + = nav_link(controller: :cohorts, html_options: { class: "fly-out-top-item" } ) do + = link_to instance_statistics_cohorts_path do + %strong.fly-out-top-item-name + = _('Cohorts') = render 'shared/sidebar_toggle_button' diff --git a/changelogs/unreleased/50567-remove-usage-ping-payload-from-cohorts-add-to-settings.yml b/changelogs/unreleased/50567-remove-usage-ping-payload-from-cohorts-add-to-settings.yml new file mode 100644 index 00000000000..2a6666e362c --- /dev/null +++ b/changelogs/unreleased/50567-remove-usage-ping-payload-from-cohorts-add-to-settings.yml @@ -0,0 +1,5 @@ +--- +title: Move usage ping payload from User Cohorts page to admin application settings +merge_request: 21343 +author: +type: other diff --git a/doc/user/instance_statistics/index.md b/doc/user/instance_statistics/index.md index a4eca89b7fe..22f76f728e3 100644 --- a/doc/user/instance_statistics/index.md +++ b/doc/user/instance_statistics/index.md @@ -10,9 +10,6 @@ and can be accessed via the top bar. ![Instance Statistics button](img/instance_statistics_button.png) -For the statistics to show up, [usage ping must be enabled](../admin_area/settings/usage_statistics.md#usage-ping) -by an admin in the admin settings area. - There are two kinds of statistics: - [Conversational Development (ConvDev) Index](convdev.md): Provides an overview of your entire instance's feature usage. diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 2302bdcde8f..00f9c6aa95a 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -150,6 +150,9 @@ msgstr "" msgid "%{unstaged} unstaged and %{staged} staged changes" msgstr "" +msgid "%{usage_ping_link_start}Learn more%{usage_ping_link_end} about what information is shared with GitLab Inc." +msgstr "" + msgid "+ %{count} more" msgstr "" @@ -2430,6 +2433,12 @@ msgstr "" msgid "Enable the Performance Bar for a given group." msgstr "" +msgid "Enable usage ping" +msgstr "" + +msgid "Enable usage ping to get an overview of how you are using GitLab from a feature perspective." +msgstr "" + msgid "Ends at (UTC)" msgstr "" @@ -2742,6 +2751,9 @@ msgstr "" msgid "For internal projects, any logged in user can view pipelines and access job details (output logs and artifacts)" msgstr "" +msgid "For more information, see the documentation on %{deactivating_usage_ping_link_start}deactivating the usage ping%{deactivating_usage_ping_link_end}." +msgstr "" + msgid "For private projects, any member (guest or higher) can view pipelines and access job details (output logs and artifacts)" msgstr "" @@ -3042,6 +3054,9 @@ msgstr "" msgid "Help page text and support page url." msgstr "" +msgid "Hide payload" +msgstr "" + msgid "Hide value" msgid_plural "Hide values" msgstr[0] "" @@ -3182,6 +3197,9 @@ msgstr "" msgid "Import repository" msgstr "" +msgid "In order to enable instance-level analytics, please ask an admin to enable %{usage_ping_link_start}usage ping%{usage_ping_link_end}." +msgstr "" + msgid "In the next step, you'll be able to select the projects you want to import." msgstr "" @@ -4384,6 +4402,9 @@ msgstr "" msgid "Preview" msgstr "" +msgid "Preview payload" +msgstr "" + msgid "Prioritize" msgstr "" @@ -5673,6 +5694,9 @@ msgstr "" msgid "The update action will time out after %{number_of_minutes} minutes. For big repositories, use a clone/push combination." msgstr "" +msgid "The usage ping is disabled, and cannot be configured through this form." +msgstr "" + msgid "The user map is a JSON document mapping the Google Code users that participated on your projects to the way their email addresses and usernames will be imported into GitLab. You can change this by changing the value on the right hand side of <code>:</code>. Be sure to preserve the surrounding double quotes, other punctuation and the email address or username on the left hand side." msgstr "" @@ -6037,12 +6061,18 @@ msgstr "" msgid "To define internal users, first enable new users set to external" msgstr "" +msgid "To enable it and see User Cohorts, visit %{application_settings_link_start}application settings%{application_settings_link_end}." +msgstr "" + msgid "To get started you enter your FogBugz URL and login information below. In the next steps, you'll be able to map users and select the projects you want to import." msgstr "" msgid "To get started, please enter your Gitea Host URL and a %{link_to_personal_token}." msgstr "" +msgid "To help improve GitLab and its user experience, GitLab will periodically collect usage information." +msgstr "" + msgid "To import GitHub repositories, you can use a %{personal_access_token_link}. When you create your Personal Access Token, you will need to select the <code>repo</code> scope, so we can display a list of your public and private repositories which are available to import." msgstr "" @@ -6202,6 +6232,9 @@ msgstr "" msgid "Upvotes" msgstr "" +msgid "Usage ping is not enabled" +msgstr "" + msgid "Usage statistics" msgstr "" @@ -6223,6 +6256,9 @@ msgstr "" msgid "Use your global notification setting" msgstr "" +msgid "User Cohorts are only shown when the %{usage_ping_link_start}usage ping%{usage_ping_link_end} is enabled." +msgstr "" + msgid "User Settings" msgstr "" diff --git a/spec/controllers/instance_statistics/cohorts_controller_spec.rb b/spec/controllers/instance_statistics/cohorts_controller_spec.rb index e4eedede93a..596d3c7abe5 100644 --- a/spec/controllers/instance_statistics/cohorts_controller_spec.rb +++ b/spec/controllers/instance_statistics/cohorts_controller_spec.rb @@ -3,5 +3,19 @@ require 'spec_helper' describe InstanceStatistics::CohortsController do + let(:user) { create(:user) } + + before do + sign_in(user) + end + it_behaves_like 'instance statistics availability' + + it 'renders a 404 when the usage ping is disabled' do + stub_application_setting(usage_ping_enabled: false) + + get :index + + expect(response).to have_gitlab_http_status(:not_found) + end end diff --git a/spec/features/admin/admin_settings_spec.rb b/spec/features/admin/admin_settings_spec.rb index a3229fe1741..3c65b5898b4 100644 --- a/spec/features/admin/admin_settings_spec.rb +++ b/spec/features/admin/admin_settings_spec.rb @@ -336,6 +336,15 @@ describe 'Admin updates settings' do expect(find_field('ED25519 SSH keys').value).to eq(forbidden) end + it 'loads usage ping payload on click', :js do + expect(page).to have_button 'Preview payload' + + find('.js-usage-ping-payload-trigger').click + + expect(page).to have_selector '.js-usage-ping-payload' + expect(page).to have_button 'Hide payload' + end + def check_all_events page.check('Active') page.check('Push') diff --git a/spec/features/instance_statistics/cohorts_spec.rb b/spec/features/instance_statistics/cohorts_spec.rb index 573f8600be1..81fc5eff980 100644 --- a/spec/features/instance_statistics/cohorts_spec.rb +++ b/spec/features/instance_statistics/cohorts_spec.rb @@ -12,12 +12,4 @@ describe 'Cohorts page' do expect(page).to have_content("#{Time.now.strftime('%b %Y')} 3 0") end - - it 'shows usage data', :js do - visit instance_statistics_cohorts_path - - wait_for_requests - - expect(find('.js-syntax-highlight').text).not_to eq('') - end end diff --git a/spec/features/instance_statistics/conversational_development_index_spec.rb b/spec/features/instance_statistics/conversational_development_index_spec.rb index a6c16b6a2a3..d8be554d734 100644 --- a/spec/features/instance_statistics/conversational_development_index_spec.rb +++ b/spec/features/instance_statistics/conversational_development_index_spec.rb @@ -16,13 +16,21 @@ describe 'Conversational Development Index' do end context 'when usage ping is disabled' do - it 'shows empty state' do + before do stub_application_setting(usage_ping_enabled: false) + end + it 'shows empty state' do visit instance_statistics_conversational_development_index_index_path expect(page).to have_content('Usage ping is not enabled') end + + it 'hides the intro callout' do + visit instance_statistics_conversational_development_index_index_path + + expect(page).not_to have_content 'Introducing Your Conversational Development Index' + end end context 'when there is no data to display' do diff --git a/spec/features/instance_statistics/instance_statistics.rb b/spec/features/instance_statistics/instance_statistics.rb new file mode 100644 index 00000000000..d03e6e68075 --- /dev/null +++ b/spec/features/instance_statistics/instance_statistics.rb @@ -0,0 +1,23 @@ +require 'rails_helper' + +describe 'Cohorts page', :js do + before do + sign_in(create(:admin)) + end + + it 'hides cohorts nav button when usage ping is disabled' do + stub_application_setting(usage_ping_enabled: false) + + visit instance_statistics_root_path + + expect(find('.nav-sidebar')).not_to have_content('Cohorts') + end + + it 'shows cohorts nav button when usage ping is enabled' do + stub_application_setting(usage_ping_enabled: true) + + visit instance_statistics_root_path + + expect(find('.nav-sidebar')).to have_content('Cohorts') + end +end |