summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--app/assets/javascripts/main.js2
-rw-r--r--app/assets/javascripts/usage_ping_consent.js30
-rw-r--r--app/assets/stylesheets/framework/layout.scss19
-rw-r--r--app/controllers/admin/application_settings_controller.rb24
-rw-r--r--app/controllers/application_controller.rb26
-rw-r--r--app/helpers/version_check_helper.rb10
-rw-r--r--app/models/application_setting.rb3
-rw-r--r--app/models/user.rb22
-rw-r--r--app/services/application_settings/update_service.rb8
-rw-r--r--app/services/submit_usage_ping_service.rb1
-rw-r--r--app/views/layouts/_page.html.haml1
-rw-r--r--app/views/shared/_ping_consent.html.haml12
-rw-r--r--changelogs/unreleased/usage_consent.yml5
-rw-r--r--db/migrate/20180906101639_add_user_ping_consent_to_application_settings.rb19
-rw-r--r--db/schema.rb4
-rw-r--r--locale/gitlab.pot9
-rw-r--r--spec/features/usage_stats_consent_spec.rb45
-rw-r--r--spec/models/user_spec.rb42
18 files changed, 267 insertions, 15 deletions
diff --git a/app/assets/javascripts/main.js b/app/assets/javascripts/main.js
index 2718f73a830..c5a5f64abac 100644
--- a/app/assets/javascripts/main.js
+++ b/app/assets/javascripts/main.js
@@ -29,6 +29,7 @@ import './milestone_select';
import './frequent_items';
import initBreadcrumbs from './breadcrumb';
import initDispatcher from './dispatcher';
+import initUsagePingConsent from './usage_ping_consent';
// expose jQuery as global (TODO: remove these)
window.jQuery = jQuery;
@@ -78,6 +79,7 @@ document.addEventListener('DOMContentLoaded', () => {
initImporterStatus();
initTodoToggle();
initLogoAnimation();
+ initUsagePingConsent();
// Set the default path for all cookies to GitLab's root directory
Cookies.defaults.path = gon.relative_url_root || '/';
diff --git a/app/assets/javascripts/usage_ping_consent.js b/app/assets/javascripts/usage_ping_consent.js
new file mode 100644
index 00000000000..ae3fde190e3
--- /dev/null
+++ b/app/assets/javascripts/usage_ping_consent.js
@@ -0,0 +1,30 @@
+import $ from 'jquery';
+import axios from './lib/utils/axios_utils';
+import Flash, { hideFlash } from './flash';
+import { convertPermissionToBoolean } from './lib/utils/common_utils';
+
+export default () => {
+ $('body').on('click', '.js-usage-consent-action', (e) => {
+ e.preventDefault();
+ e.stopImmediatePropagation(); // overwrite rails listener
+
+ const { url, checkEnabled, pingEnabled } = e.target.dataset;
+ const data = {
+ application_setting: {
+ version_check_enabled: convertPermissionToBoolean(checkEnabled),
+ usage_ping_enabled: convertPermissionToBoolean(pingEnabled),
+ },
+ };
+
+ const hideConsentMessage = () => hideFlash(document.querySelector('.ping-consent-message'));
+
+ axios.put(url, data)
+ .then(() => {
+ hideConsentMessage();
+ })
+ .catch(() => {
+ hideConsentMessage();
+ Flash('Something went wrong. Try again later.');
+ });
+ });
+};
diff --git a/app/assets/stylesheets/framework/layout.scss b/app/assets/stylesheets/framework/layout.scss
index d4bae4cb137..9218df9b40f 100644
--- a/app/assets/stylesheets/framework/layout.scss
+++ b/app/assets/stylesheets/framework/layout.scss
@@ -69,10 +69,14 @@ body {
float: right;
}
- /* Center alert text and alert action links on smaller screens */
- @include media-breakpoint-down(sm) {
- .alert {
- text-align: center;
+ .flex-alert {
+ @include media-breakpoint-up(lg) {
+ display: flex;
+
+ .alert-message {
+ flex: 1;
+ padding-right: 40px;
+ }
}
.alert-link-group {
@@ -80,6 +84,13 @@ body {
}
}
+ @include media-breakpoint-down(sm) {
+ .alert-link-group {
+ float: none;
+ margin-top: $gl-padding-8;
+ }
+ }
+
/* Stripe the background colors so that adjacent alert-warnings are distinct from one another */
.alert-warning {
transition: background-color 0.15s, border-color 0.15s;
diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb
index 9723e400574..869213d61f1 100644
--- a/app/controllers/admin/application_settings_controller.rb
+++ b/app/controllers/admin/application_settings_controller.rb
@@ -9,11 +9,18 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
.new(@application_setting, current_user, application_setting_params)
.execute
- if successful
- redirect_to admin_application_settings_path,
- notice: 'Application settings saved successfully'
- else
- render :show
+ if recheck_user_consent?
+ session[:ask_for_usage_stats_consent] = current_user.requires_usage_stats_consent?
+ end
+
+ respond_to do |format|
+ if successful
+ format.json { head :ok }
+ format.html { redirect_to admin_application_settings_path, notice: 'Application settings saved successfully' }
+ else
+ format.json { head :bad_request }
+ format.html { render :show }
+ end
end
end
@@ -76,6 +83,13 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
)
end
+ def recheck_user_consent?
+ return false unless session[:ask_for_usage_stats_consent]
+ return false unless params[:application_setting]
+
+ params[:application_setting].key?(:usage_ping_enabled) || params[:application_setting].key?(:version_check_enabled)
+ end
+
def visible_application_setting_attributes
ApplicationSettingsHelper.visible_attributes + [
:domain_blacklist_file,
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 7cd68d6b92a..7e2b2cf3ad3 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -22,6 +22,7 @@ class ApplicationController < ActionController::Base
before_action :add_gon_variables, unless: [:peek_request?, :json_request?]
before_action :configure_permitted_parameters, if: :devise_controller?
before_action :require_email, unless: :devise_controller?
+ before_action :set_usage_stats_consent_flag
around_action :set_locale
@@ -434,4 +435,29 @@ class ApplicationController < ActionController::Base
!(peek_request? || devise_controller?)
end
+
+ def set_usage_stats_consent_flag
+ return unless current_user
+ return if sessionless_user?
+ return if session.has_key?(:ask_for_usage_stats_consent)
+
+ session[:ask_for_usage_stats_consent] = current_user.requires_usage_stats_consent?
+
+ if session[:ask_for_usage_stats_consent]
+ disable_usage_stats
+ end
+ end
+
+ def disable_usage_stats
+ application_setting_params = {
+ usage_ping_enabled: false,
+ version_check_enabled: false,
+ skip_usage_stats_user: true
+ }
+ settings = Gitlab::CurrentSettings.current_application_settings
+
+ ApplicationSettings::UpdateService
+ .new(settings, current_user, application_setting_params)
+ .execute
+ end
end
diff --git a/app/helpers/version_check_helper.rb b/app/helpers/version_check_helper.rb
index c20753ece72..a7fef93e7f1 100644
--- a/app/helpers/version_check_helper.rb
+++ b/app/helpers/version_check_helper.rb
@@ -1,8 +1,10 @@
module VersionCheckHelper
def version_status_badge
- if Rails.env.production? && Gitlab::CurrentSettings.version_check_enabled
- image_url = VersionCheck.new.url
- image_tag image_url, class: 'js-version-status-badge'
- end
+ return unless Rails.env.production?
+ return unless Gitlab::CurrentSettings.version_check_enabled
+ return if User.single_user&.requires_usage_stats_consent?
+
+ image_url = VersionCheck.new.url
+ image_tag image_url, class: 'js-version-status-badge'
end
end
diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb
index 03bd7fa016e..d8536c5512d 100644
--- a/app/models/application_setting.rb
+++ b/app/models/application_setting.rb
@@ -302,7 +302,8 @@ class ApplicationSetting < ActiveRecord::Base
instance_statistics_visibility_private: false,
user_default_external: false,
user_default_internal_regex: nil,
- user_show_add_ssh_key_message: true
+ user_show_add_ssh_key_message: true,
+ usage_stats_set_by_user_id: nil
}
end
diff --git a/app/models/user.rb b/app/models/user.rb
index 0fcc952b5cd..568ec101016 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -490,6 +490,16 @@ class User < ActiveRecord::Base
u.name = 'Ghost User'
end
end
+
+ # Return true if there is only single non-internal user in the deployment,
+ # ghost user is ignored.
+ def single_user?
+ User.non_internal.limit(2).count == 1
+ end
+
+ def single_user
+ User.non_internal.first if single_user?
+ end
end
def full_path
@@ -1287,6 +1297,10 @@ class User < ActiveRecord::Base
!terms_accepted?
end
+ def requires_usage_stats_consent?
+ !consented_usage_stats? && 7.days.ago > self.created_at && !has_current_license? && User.single_user?
+ end
+
# @deprecated
alias_method :owned_or_masters_groups, :owned_or_maintainers_groups
@@ -1301,6 +1315,14 @@ class User < ActiveRecord::Base
private
+ def has_current_license?
+ false
+ end
+
+ def consented_usage_stats?
+ Gitlab::CurrentSettings.usage_stats_set_by_user_id == self.id
+ end
+
def owned_projects_union
Gitlab::SQL::Union.new([
Project.where(namespace: namespace),
diff --git a/app/services/application_settings/update_service.rb b/app/services/application_settings/update_service.rb
index 19cf34e2ac4..2e4643ed668 100644
--- a/app/services/application_settings/update_service.rb
+++ b/app/services/application_settings/update_service.rb
@@ -11,11 +11,19 @@ module ApplicationSettings
params[:performance_bar_allowed_group_id] = performance_bar_allowed_group_id
end
+ if usage_stats_updated? && !params.delete(:skip_usage_stats_user)
+ params[:usage_stats_set_by_user_id] = current_user.id
+ end
+
@application_setting.update(@params)
end
private
+ def usage_stats_updated?
+ params.key?(:usage_ping_enabled) || params.key?(:version_check_enabled)
+ end
+
def update_terms(terms)
return unless terms.present?
diff --git a/app/services/submit_usage_ping_service.rb b/app/services/submit_usage_ping_service.rb
index 93c2e222963..62222d3fd2a 100644
--- a/app/services/submit_usage_ping_service.rb
+++ b/app/services/submit_usage_ping_service.rb
@@ -15,6 +15,7 @@ class SubmitUsagePingService
def execute
return false unless Gitlab::CurrentSettings.usage_ping_enabled?
+ return false if User.single_user&.requires_usage_stats_consent?
response = Gitlab::HTTP.post(
URL,
diff --git a/app/views/layouts/_page.html.haml b/app/views/layouts/_page.html.haml
index f67a8878c80..a41d30da450 100644
--- a/app/views/layouts/_page.html.haml
+++ b/app/views/layouts/_page.html.haml
@@ -8,6 +8,7 @@
= render "layouts/broadcast"
= render 'layouts/header/read_only_banner'
= yield :flash_message
+ = render "shared/ping_consent"
- unless @hide_breadcrumbs
= render "layouts/nav/breadcrumbs"
= render "layouts/flash"
diff --git a/app/views/shared/_ping_consent.html.haml b/app/views/shared/_ping_consent.html.haml
new file mode 100644
index 00000000000..f8eb2b2833b
--- /dev/null
+++ b/app/views/shared/_ping_consent.html.haml
@@ -0,0 +1,12 @@
+- if session[:ask_for_usage_stats_consent]
+ .ping-consent-message.alert.alert-warning.flex-alert
+ - settings_link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer" class="alert-link">'.html_safe % { url: admin_application_settings_path(anchor: 'js-usage-settings') }
+ - info_link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer" class="alert-link">'.html_safe % { url: help_page_path('user/admin_area/settings/usage_statistics.md') }
+ .alert-message
+ = s_('To help improve GitLab, we would like to periodically collect usage information. This can be changed at any time in %{settings_link_start}Settings%{link_end}. %{info_link_start}More Information%{link_end}').html_safe % { settings_link_start: settings_link_start, info_link_start: info_link_start, link_end: '</a>'.html_safe }
+ .alert-link-group
+ - send_usage_data_path = admin_application_settings_path(application_setting: { version_check_enabled: 1, usage_ping_enabled: 1 })
+ - not_now_path = admin_application_settings_path(application_setting: { version_check_enabled: 0, usage_ping_enabled: 0 })
+ = link_to _("Send usage data"), send_usage_data_path, 'data-url' => admin_application_settings_path, method: :put, 'data-check-enabled': true, 'data-ping-enabled': true, class: 'alert-link js-usage-consent-action'
+ |
+ = link_to _('Not now'), not_now_path, 'data-url' => admin_application_settings_path, method: :put, 'data-check-enabled': false, 'data-ping-enabled': false, class: 'hide-ping-consent-message alert-link js-usage-consent-action'
diff --git a/changelogs/unreleased/usage_consent.yml b/changelogs/unreleased/usage_consent.yml
new file mode 100644
index 00000000000..56e00879b9a
--- /dev/null
+++ b/changelogs/unreleased/usage_consent.yml
@@ -0,0 +1,5 @@
+---
+title: Ask user explicitly about usage stats agreement on single user deployments.
+merge_request: 21423
+author:
+type: added
diff --git a/db/migrate/20180906101639_add_user_ping_consent_to_application_settings.rb b/db/migrate/20180906101639_add_user_ping_consent_to_application_settings.rb
new file mode 100644
index 00000000000..5d0e67d2648
--- /dev/null
+++ b/db/migrate/20180906101639_add_user_ping_consent_to_application_settings.rb
@@ -0,0 +1,19 @@
+# frozen_string_literal: true
+
+class AddUserPingConsentToApplicationSettings < ActiveRecord::Migration
+ include Gitlab::Database::MigrationHelpers
+
+ DOWNTIME = false
+
+ disable_ddl_transaction!
+
+ def up
+ add_column :application_settings, :usage_stats_set_by_user_id, :integer
+ add_concurrent_foreign_key :application_settings, :users, column: :usage_stats_set_by_user_id, on_delete: :nullify
+ end
+
+ def down
+ remove_foreign_key :application_settings, column: :usage_stats_set_by_user_id
+ remove_column :application_settings, :usage_stats_set_by_user_id
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index e0b704f82d4..a417d0bc70a 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,7 +11,7 @@
#
# It's strongly recommended that you check this file into your version control system.
-ActiveRecord::Schema.define(version: 20180901200537) do
+ActiveRecord::Schema.define(version: 20180906101639) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
@@ -172,6 +172,7 @@ ActiveRecord::Schema.define(version: 20180901200537) do
t.boolean "instance_statistics_visibility_private", default: false, null: false
t.boolean "web_ide_clientside_preview_enabled", default: false, null: false
t.boolean "user_show_add_ssh_key_message", default: true, null: false
+ t.integer "usage_stats_set_by_user_id"
end
create_table "audit_events", force: :cascade do |t|
@@ -2275,6 +2276,7 @@ ActiveRecord::Schema.define(version: 20180901200537) do
add_index "web_hooks", ["project_id"], name: "index_web_hooks_on_project_id", using: :btree
add_index "web_hooks", ["type"], name: "index_web_hooks_on_type", using: :btree
+ add_foreign_key "application_settings", "users", column: "usage_stats_set_by_user_id", name: "fk_964370041d", on_delete: :nullify
add_foreign_key "badges", "namespaces", column: "group_id", on_delete: :cascade
add_foreign_key "badges", "projects", on_delete: :cascade
add_foreign_key "boards", "namespaces", column: "group_id", on_delete: :cascade
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 491da5d9631..8b19e2bec61 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -4004,6 +4004,9 @@ msgstr ""
msgid "Not enough data"
msgstr ""
+msgid "Not now"
+msgstr ""
+
msgid "Note that the master branch is automatically protected. %{link_to_protected_branches}"
msgstr ""
@@ -5177,6 +5180,9 @@ msgstr ""
msgid "Send email"
msgstr ""
+msgid "Send usage data"
+msgstr ""
+
msgid "Sep"
msgstr ""
@@ -6145,6 +6151,9 @@ msgstr ""
msgid "To help improve GitLab and its user experience, GitLab will periodically collect usage information."
msgstr ""
+msgid "To help improve GitLab, we would like to periodically collect usage information. This can be changed at any time in %{settings_link_start}Settings%{link_end}. %{info_link_start}More Information%{link_end}"
+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 ""
diff --git a/spec/features/usage_stats_consent_spec.rb b/spec/features/usage_stats_consent_spec.rb
new file mode 100644
index 00000000000..dd8f3179895
--- /dev/null
+++ b/spec/features/usage_stats_consent_spec.rb
@@ -0,0 +1,45 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe 'Usage stats consent' do
+ context 'when signed in' do
+ let(:user) { create(:admin, created_at: 8.days.ago) }
+ let(:message) { 'To help improve GitLab, we would like to periodically collect usage information.' }
+
+ before do
+ allow(user).to receive(:has_current_license?).and_return false
+
+ gitlab_sign_in(user)
+ end
+
+ it 'hides the banner permanently when sets usage stats' do
+ visit root_dashboard_path
+
+ expect(page).to have_content(message)
+
+ click_link 'Send usage data'
+
+ expect(page).not_to have_content(message)
+ expect(page).to have_content('Application settings saved successfully')
+
+ gitlab_sign_out
+ gitlab_sign_in(user)
+ visit root_dashboard_path
+
+ expect(page).not_to have_content(message)
+ end
+
+ it 'shows banner on next session if user did not set usage stats' do
+ visit root_dashboard_path
+
+ expect(page).to have_content(message)
+
+ gitlab_sign_out
+ gitlab_sign_in(user)
+ visit root_dashboard_path
+
+ expect(page).to have_content(message)
+ end
+ end
+end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 2a7aff39240..bee4a3d24a7 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -2957,6 +2957,48 @@ describe User do
end
end
+ describe '#requires_usage_stats_consent?' do
+ let(:user) { create(:user, created_at: 8.days.ago) }
+
+ before do
+ allow(user).to receive(:has_current_license?).and_return false
+ end
+
+ context 'in single-user environment' do
+ it 'requires user consent after one week' do
+ create(:user, ghost: true)
+
+ expect(user.requires_usage_stats_consent?).to be true
+ end
+
+ it 'requires user consent after one week if there is another ghost user' do
+ expect(user.requires_usage_stats_consent?).to be true
+ end
+
+ it 'does not require consent in the first week' do
+ user.created_at = 6.days.ago
+
+ expect(user.requires_usage_stats_consent?).to be false
+ end
+
+ it 'does not require consent if usage stats were set by this user' do
+ allow(Gitlab::CurrentSettings).to receive(:usage_stats_set_by_user_id).and_return(user.id)
+
+ expect(user.requires_usage_stats_consent?).to be false
+ end
+ end
+
+ context 'in multi-user environment' do
+ before do
+ create(:user)
+ end
+
+ it 'does not require consent' do
+ expect(user.requires_usage_stats_consent?).to be false
+ end
+ end
+ end
+
context 'with uploads' do
it_behaves_like 'model with mounted uploader', false do
let(:model_object) { create(:user, :with_avatar) }