kotovalexarian-likes-gitlab/gitlab-org--gitlab-foss
1
0
Fork 0
Code Releases Activity

Revert "Merge branch '41416-making-instance-wide-data-tools-more-accessible' into 'master'"

Browse source 
This reverts merge request !20679
This commit is contained in:
Sean McGivern 2018-07-27 13:12:35 +00:00
parent 636ea40e18
commit ed81ee9ba2
38 changed files with 43 additions and 295 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
app/controllers/instance_statistics/cohorts_controller.rb → app/controllers/admin/cohorts_controller.rb
View file

@ -1,6 +1,4 @@
# frozen_string_literal: true
class InstanceStatistics::CohortsController < InstanceStatistics::ApplicationController
class Admin::CohortsController < Admin::ApplicationController
def index
if Gitlab::CurrentSettings.usage_ping_enabled
cohorts_results = Rails.cache.fetch('cohorts', expires_in: 1.day) do

5
app/controllers/admin/conversational_development_index_controller.rb Normal file
View file

@ -0,0 +1,5 @@
class Admin::ConversationalDevelopmentIndexController < Admin::ApplicationController
def show
@metric = ConversationalDevelopmentIndex::Metric.order(:created_at).last&.present
end
end

8
app/controllers/instance_statistics/application_controller.rb
View file

@ -1,8 +0,0 @@
class InstanceStatistics::ApplicationController < ApplicationController
before_action :authorize_read_instance_statistics!
layout 'instance_statistics'
def authorize_read_instance_statistics!
render_404 unless can?(current_user, :read_instance_statistics)
end
end

5
app/controllers/instance_statistics/conversational_development_index_controller.rb
View file

@ -1,5 +0,0 @@
class InstanceStatistics::ConversationalDevelopmentIndexController < InstanceStatistics::ApplicationController
def index
@metric = ConversationalDevelopmentIndex::Metric.order(:created_at).last&.present
end
end

1
app/helpers/application_settings_helper.rb
View file

@ -247,7 +247,6 @@ module ApplicationSettingsHelper
:unique_ips_limit_per_user,
:unique_ips_limit_time_window,
:usage_ping_enabled,
:instance_statistics_visibility_private,
:user_default_external,
:user_oauth_applications,
:version_check_enabled,

1
app/models/application_setting.rb
View file

@ -290,7 +290,6 @@ class ApplicationSetting < ActiveRecord::Base
user_default_external: false,
polling_interval_multiplier: 1,
usage_ping_enabled: Settings.gitlab['usage_ping_enabled'],
instance_statistics_visibility_private: false,
gitaly_timeout_fast: 10,
gitaly_timeout_medium: 30,
gitaly_timeout_default: 55,

5
app/policies/global_policy.rb
View file

@ -19,11 +19,6 @@ class GlobalPolicy < BasePolicy
@user&.required_terms_not_accepted?
end
condition(:private_instance_statistics, score: 0) { Gitlab::CurrentSettings.instance_statistics_visibility_private? }
rule { admin | (~private_instance_statistics & ~anonymous) }
.enable :read_instance_statistics
rule { anonymous }.policy do
prevent :log_in
prevent :receive_notifications

5
app/views/admin/application_settings/_usage.html.haml
View file

@ -23,16 +23,13 @@
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')
= link_to 'Cohorts', admin_cohorts_path(anchor: 'usage-ping')
to see the JSON payload sent.
- 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
= 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"

0
app/views/instance_statistics/cohorts/_cohorts_table.html.haml → app/views/admin/cohorts/_cohorts_table.html.haml
View file

0
app/views/instance_statistics/cohorts/_usage_ping.html.haml → app/views/admin/cohorts/_usage_ping.html.haml
View file

0
app/views/instance_statistics/cohorts/index.html.haml → app/views/admin/cohorts/index.html.haml
View file

0
app/views/instance_statistics/conversational_development_index/_callout.html.haml → app/views/admin/conversational_development_index/_callout.html.haml
View file

0
app/views/instance_statistics/conversational_development_index/_card.html.haml → app/views/admin/conversational_development_index/_card.html.haml
View file

0
app/views/instance_statistics/conversational_development_index/_disabled.html.haml → app/views/admin/conversational_development_index/_disabled.html.haml
View file

0
app/views/instance_statistics/conversational_development_index/_no_data.html.haml → app/views/admin/conversational_development_index/_no_data.html.haml
View file

0
app/views/instance_statistics/conversational_development_index/index.html.haml → app/views/admin/conversational_development_index/show.html.haml
View file

6
app/views/layouts/instance_statistics.html.haml
View file

@ -1,6 +0,0 @@
- page_title "Instance Statistics"
- header_title "Instance Statistics", instance_statistics_root_path
- nav "instance_statistics"
- @left_sidebar = true
= render template: "layouts/application"

8
app/views/layouts/nav/_dashboard.html.haml
View file

@ -64,15 +64,11 @@
= link_to '#', class: 'dashboard-shortcuts-web-ide', title: _('Web IDE') do
= _('Web IDE')
- if Gitlab::Sherlock.enabled? || can?(current_user, :read_instance_statistics)
- if current_user.admin? || Gitlab::Sherlock.enabled?
%li.line-separator.d-none.d-sm-block
- if can?(current_user, :read_instance_statistics)
= nav_link(controller: [:conversational_development_index, :cohorts]) do
= link_to instance_statistics_root_path, title: 'Instance Statistics', aria: { label: 'Instance statistics' }, data: {toggle: 'tooltip', placement: 'bottom', container: 'body'} do
= sprite_icon('chart', size: 18)
- if current_user.admin?
= nav_link(controller: 'admin/dashboard') do
= link_to admin_root_path, class: 'admin-icon qa-admin-area-link', title: _('Admin area'), aria: { label: "Admin area" }, data: {toggle: 'tooltip', placement: 'bottom', container: 'body'} do
= link_to admin_root_path, class: 'admin-icon qa-admin-area-link', title: _('Admin area'), aria: { label: _("Admin area") }, data: {toggle: 'tooltip', placement: 'bottom', container: 'body'} do
= sprite_icon('admin', size: 18)
- if Gitlab::Sherlock.enabled?
%li

12
app/views/layouts/nav/sidebar/_admin.html.haml
View file

@ -7,14 +7,14 @@
.sidebar-context-title
= _('Admin Area')
%ul.sidebar-top-level-items
= nav_link(controller: %w(dashboard admin projects users groups jobs runners gitaly_servers), html_options: {class: 'home'}) do
= nav_link(controller: %w(dashboard admin projects users groups jobs runners gitaly_servers cohorts conversational_development_index), html_options: {class: 'home'}) do
= link_to admin_root_path, class: 'shortcuts-tree' do
.nav-icon-container
= sprite_icon('overview')
%span.nav-item-name
= _('Overview')
%ul.sidebar-sub-level-items
= nav_link(controller: %w(dashboard admin projects users groups jobs runners gitaly_servers), html_options: { class: "fly-out-top-item" } ) do
= nav_link(controller: %w(dashboard admin projects users groups jobs runners gitaly_servers cohorts conversational_development_index), html_options: { class: "fly-out-top-item" } ) do
= link_to admin_root_path do
%strong.fly-out-top-item-name
= _('Overview')
@ -47,6 +47,14 @@
= link_to admin_gitaly_servers_path, title: 'Gitaly Servers' do
%span
= _('Gitaly Servers')
= nav_link path: 'cohorts#index' do
= link_to admin_cohorts_path, title: _('Cohorts') do
%span
= _('Cohorts')
= nav_link(controller: :conversational_development_index) do
= link_to admin_conversational_development_index_path, title: _('ConvDev Index') do
%span
= _('ConvDev Index')
= nav_link(controller: %w(system_info background_jobs logs health_check requests_profiles)) do
= link_to admin_system_info_path do

33
app/views/layouts/nav/sidebar/_instance_statistics.html.haml
View file

@ -1,33 +0,0 @@
.nav-sidebar{ class: ("sidebar-collapsed-desktop" if collapsed_sidebar?) }
.nav-sidebar-inner-scroll
.context-header
= link_to instance_statistics_root_path, title: 'Instance Statistics' do
.avatar-container.s40.settings-avatar
= sprite_icon('chart', size: 24)
.sidebar-context-title Instance Statistics
%ul.sidebar-top-level-items
= nav_link(controller: :conversational_development_index) do
= link_to instance_statistics_conversational_development_index_index_path do
.nav-icon-container
= sprite_icon('comment')
%span.nav-item-name
= _('ConvDev Index')
%ul.sidebar-sub-level-items.is-fly-out-only
= nav_link(controller: :conversational_development_index, html_options: { class: "fly-out-top-item" } ) do
= link_to instance_statistics_conversational_development_index_index_path do
%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')
= render 'shared/sidebar_toggle_button'

5
changelogs/unreleased/41416-making-instance-wide-data-tools-more-accessible.yml
View file

@ -1,5 +0,0 @@
---
title: Allow non-admins to view instance statistics (if permitted by the instance admins)
merge_request: 20679
author:
type: changed

2
config/routes.rb
View file

@ -70,8 +70,6 @@ Rails.application.routes.draw do
get 'ide' => 'ide#index'
get 'ide/*vueroute' => 'ide#index', format: false
draw :instance_statistics
end
# Koding route

4
config/routes/admin.rb
View file

@ -76,6 +76,8 @@ namespace :admin do
resource :system_info, controller: 'system_info', only: [:show]
resources :requests_profiles, only: [:index, :show], param: :name, constraints: { name: /.+\.html/ }
get 'conversational_development_index' => 'conversational_development_index#show'
resources :projects, only: [:index]
scope(path: 'projects/*namespace_id',
@ -121,6 +123,8 @@ namespace :admin do
end
end
resources :cohorts, only: :index
resources :jobs, only: :index do
collection do
post :cancel_all

6
config/routes/instance_statistics.rb
View file

@ -1,6 +0,0 @@
namespace :instance_statistics do
root to: redirect('/-/instance_statistics/conversational_development_index')
resources :cohorts, only: :index
resources :conversational_development_index, only: :index
end

18
db/migrate/20180718005113_add_instance_statistics_visibility_to_application_setting.rb
View file

@ -1,18 +0,0 @@
class AddInstanceStatisticsVisibilityToApplicationSetting < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
def up
add_column_with_default(:application_settings, :instance_statistics_visibility_private,
:boolean,
default: false,
allow_null: false)
end
def down
remove_column(:application_settings, :instance_statistics_visibility_private)
end
end

1
db/schema.rb
View file

@ -168,7 +168,6 @@ ActiveRecord::Schema.define(version: 20180722103201) do
t.boolean "enforce_terms", default: false
t.boolean "mirror_available", default: true, null: false
t.boolean "hide_third_party_offers", default: false, null: false
t.boolean "instance_statistics_visibility_private", default: false, null: false
end
create_table "audit_events", force: :cascade do |t|

7
doc/api/settings.md
View file

@ -55,8 +55,7 @@ Example response:
"ed25519_key_restriction": 0,
"enforce_terms": true,
"terms": "Hello world!",
"performance_bar_allowed_group_id": 42,
"instance_statistics_visibility_private": false
"performance_bar_allowed_group_id": 42
}
```
@ -160,7 +159,6 @@ PUT /application/settings
| `version_check_enabled` | boolean | no | Let GitLab inform you when an update is available. |
| `enforce_terms` | boolean | no | Enforce application ToS to all users |
| `terms` | text | yes (if `enforce_terms` is true) | Markdown content for the ToS |
| `instance_statistics_visibility_private` | boolean | no | When set to `true` Instance statistics will only be available to admins |
```bash
curl --request PUT --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/application/settings?signup_enabled=false&default_project_visibility=internal
@ -205,7 +203,6 @@ Example response:
"ed25519_key_restriction": 0,
"enforce_terms": true,
"terms": "Hello world!",
"performance_bar_allowed_group_id": 42,
"instance_statistics_visibility_private": false
"performance_bar_allowed_group_id": 42
}
```

1
lib/api/settings.rb
View file

@ -130,7 +130,6 @@ module API
optional :gitaly_timeout_medium, type: Integer, desc: 'Medium Gitaly timeout, in seconds. Set to 0 to disable timeouts.'
optional :gitaly_timeout_fast, type: Integer, desc: 'Gitaly fast operation timeout, in seconds. Set to 0 to disable timeouts.'
optional :usage_ping_enabled, type: Boolean, desc: 'Every week GitLab will report license usage back to GitLab, Inc.'
optional :instance_statistics_visibility_private, type: Boolean, desc: 'When set to `true` Instance statistics will only be available to admins'
ApplicationSetting::SUPPORTED_KEY_TYPES.each do |type|
optional :"#{type}_key_restriction",

7
spec/controllers/instance_statistics/cohorts_controller_spec.rb
View file

@ -1,7 +0,0 @@
# frozen_string_literal: true
require 'spec_helper'
describe InstanceStatistics::CohortsController do
it_behaves_like 'instance statistics availability'
end

7
spec/controllers/instance_statistics/conversational_development_index_controller_spec.rb
View file

@ -1,7 +0,0 @@
# frozen_string_literal: true
require 'spec_helper'
describe InstanceStatistics::ConversationalDevelopmentIndexController do
it_behaves_like 'instance statistics availability'
end

4
spec/features/instance_statistics/cohorts_spec.rb → spec/features/admin/admin_cohorts_spec.rb
View file

@ -1,6 +1,6 @@
require 'rails_helper'
describe 'Cohorts page' do
describe 'Admin cohorts page' do
before do
sign_in(create(:admin))
end
@ -8,7 +8,7 @@ describe 'Cohorts page' do
it 'See users count per month' do
2.times { create(:user) }
visit instance_statistics_cohorts_path
visit admin_cohorts_path
expect(page).to have_content("#{Time.now.strftime('%b %Y')} 3 0")
end

8
spec/features/instance_statistics/conversational_development_index_spec.rb → spec/features/admin/admin_conversational_development_index_spec.rb
View file

@ -1,6 +1,6 @@
require 'spec_helper'
describe 'Conversational Development Index' do
describe 'Admin Conversational Development Index' do
before do
sign_in(create(:admin))
end
@ -9,7 +9,7 @@ describe 'Conversational Development Index' do
it 'shows empty state' do
stub_application_setting(usage_ping_enabled: false)
visit instance_statistics_conversational_development_index_index_path
visit admin_conversational_development_index_path
expect(page).to have_content('Usage ping is not enabled')
end
@ -19,7 +19,7 @@ describe 'Conversational Development Index' do
it 'shows empty state' do
stub_application_setting(usage_ping_enabled: true)
visit instance_statistics_conversational_development_index_index_path
visit admin_conversational_development_index_path
expect(page).to have_content('Data is still calculating')
end
@ -30,7 +30,7 @@ describe 'Conversational Development Index' do
stub_application_setting(usage_ping_enabled: true)
create(:conversational_development_index_metric)
visit instance_statistics_conversational_development_index_index_path
visit admin_conversational_development_index_path
expect(page).to have_content(
'Issues created per active user 1.2 You 9.3 Lead 13.3%'

28
spec/features/dashboard/active_tab_spec.rb
View file

@ -7,38 +7,32 @@ RSpec.describe 'Dashboard Active Tab', :js do
shared_examples 'page has active tab' do |title|
it "#{title} tab" do
subject
expect(page).to have_selector('.navbar-sub-nav li.active', count: 1)
expect(find('.navbar-sub-nav li.active')).to have_content(title)
end
end
context 'on dashboard projects' do
it_behaves_like 'page has active tab', 'Projects' do
subject { visit dashboard_projects_path }
before do
visit dashboard_projects_path
end
it_behaves_like 'page has active tab', 'Projects'
end
context 'on dashboard groups' do
it_behaves_like 'page has active tab', 'Groups' do
subject { visit dashboard_groups_path }
before do
visit dashboard_groups_path
end
it_behaves_like 'page has active tab', 'Groups'
end
context 'on activity projects' do
it_behaves_like 'page has active tab', 'Activity' do
subject { visit activity_dashboard_path }
before do
visit activity_dashboard_path
end
end
context 'on instance statistics' do
subject { visit instance_statistics_root_path }
it 'shows Instance Statistics` as active' do
subject
expect(find('.navbar-sub-nav li.active')).to have_link('Instance Statistics')
end
it_behaves_like 'page has active tab', 'Activity'
end
end

60
spec/features/dashboard/instance_statistics_spec.rb
View file

@ -1,60 +0,0 @@
# frozen_string_literal: true
require 'spec_helper'
describe 'Showing instance statistics' do
before do
sign_in user if user
end
# Using a path that is publicly accessible
subject { visit explore_projects_path }
context 'for unauthenticated users' do
let(:user) { nil }
it 'does not show the instance statistics link' do
subject
expect(page).not_to have_link('Instance Statistics')
end
end
context 'for regular users' do
let(:user) { create(:user) }
context 'when instance statistics are publicly available' do
before do
stub_application_setting(instance_statistics_visibility_private: false)
end
it 'shows the instance statistics link' do
subject
expect(page).to have_link('Instance Statistics')
end
end
context 'when instance statistics are not publicly available' do
before do
stub_application_setting(instance_statistics_visibility_private: true)
end
it 'shows the instance statistics link' do
subject
expect(page).not_to have_link('Instance Statistics')
end
end
end
context 'for admins' do
let(:user) { create(:admin) }
it 'shows the instance statistics link' do
subject
expect(page).to have_link('Instance Statistics')
end
end
end

34
spec/policies/global_policy_spec.rb
View file

@ -180,38 +180,4 @@ describe GlobalPolicy do
end
end
end
describe 'read instance statistics' do
context 'regular user' do
it { is_expected.to be_allowed(:read_instance_statistics) }
context 'when instance statistics are set to private' do
before do
stub_application_setting(instance_statistics_visibility_private: true)
end
it { is_expected.not_to be_allowed(:read_instance_statistics) }
end
end
context 'admin' do
let(:current_user) { create(:admin) }
it { is_expected.to be_allowed(:read_instance_statistics) }
context 'when instance statistics are set to private' do
before do
stub_application_setting(instance_statistics_visibility_private: true)
end
it { is_expected.to be_allowed(:read_instance_statistics) }
end
end
context 'anonymous' do
let(:current_user) { nil }
it { is_expected.not_to be_allowed(:read_instance_statistics) }
end
end
end

5
spec/requests/api/settings_spec.rb
View file

@ -25,7 +25,6 @@ describe API::Settings, 'Settings' do
expect(json_response['ed25519_key_restriction']).to eq(0)
expect(json_response['circuitbreaker_failure_count_threshold']).not_to be_nil
expect(json_response['performance_bar_allowed_group_id']).to be_nil
expect(json_response['instance_statistics_visibility_private']).to be(false)
expect(json_response).not_to have_key('performance_bar_allowed_group_path')
expect(json_response).not_to have_key('performance_bar_enabled')
end
@ -65,8 +64,7 @@ describe API::Settings, 'Settings' do
circuitbreaker_check_interval: 2,
enforce_terms: true,
terms: 'Hello world!',
performance_bar_allowed_group_path: group.full_path,
instance_statistics_visibility_private: true
performance_bar_allowed_group_path: group.full_path
expect(response).to have_gitlab_http_status(200)
expect(json_response['default_projects_limit']).to eq(3)
@ -91,7 +89,6 @@ describe API::Settings, 'Settings' do
expect(json_response['enforce_terms']).to be(true)
expect(json_response['terms']).to eq('Hello world!')
expect(json_response['performance_bar_allowed_group_id']).to eq(group.id)
expect(json_response['instance_statistics_visibility_private']).to be(true)
end
end

11
spec/routing/instance_statistics_routing_spec.rb
View file

@ -1,11 +0,0 @@
# frozen_string_literal: true
require 'spec_helper'
describe 'Instance Statistics', 'routing' do
include RSpec::Rails::RequestExampleGroup
it "routes '/-/instance_statistics' to conversational development index" do
expect(get('/-/instance_statistics')).to redirect_to('/-/instance_statistics/conversational_development_index')
end
end

37
spec/support/shared_examples/instance_statistics_controllers_shared_examples.rb
View file

@ -1,37 +0,0 @@
# frozen_string_literal: true
shared_examples 'instance statistics availability' do
let(:user) { create(:user) }
before do
sign_in(user)
end
describe 'GET #index' do
it 'is available when the feature is available publicly' do
get :index
expect(response).to have_gitlab_http_status(:success)
end
it 'renders a 404 when the feature is not available publicly' do
stub_application_setting(instance_statistics_visibility_private: true)
get :index
expect(response).to have_gitlab_http_status(:not_found)
end
context 'for admins' do
let(:user) { create(:admin) }
it 'allows access when the feature is not available publicly' do
stub_application_setting(instance_statistics_visibility_private: true)
get :index
expect(response).to have_gitlab_http_status(:success)
end
end
end
end