Resolve "Remove usage ping payload from Cohorts, add to Settings"

11 files changed, 129 insertions, 58 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'