Commit a415a904 authored by Jan Provaznik's avatar Jan Provaznik Committed by Douwe Maan

Ask user explicitly about usage stats agreement

parent 1f765ce9
......@@ -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 || '/';
......
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.');
});
});
};
......@@ -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;
......
......@@ -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,
......
......@@ -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
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
......@@ -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
......
......@@ -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),
......
......@@ -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?
......
......@@ -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,
......
......@@ -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"
......
- 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'
---
title: Ask user explicitly about usage stats agreement on single user deployments.
merge_request: 21423
author:
type: added
# 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
......@@ -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
......
......@@ -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 ""
......
# 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
......@@ -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) }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment