Commit 19c74038 authored by Alper Akgun's avatar Alper Akgun

Merge branch 'clean-up-analytics-feature-flags' into 'master'

Eliminate analytics FF helper methods

See merge request gitlab-org/gitlab!41876
parents fb24b25c 66d24ead
......@@ -73,18 +73,3 @@ The **Merge Request Analytics** feature can be accessed only:
- On [Starter](https://about.gitlab.com/pricing/) and above.
- By users with [Reporter access](../permissions.md) and above.
## Enable and disable related feature flags
Merge Request Analytics is disabled by default but can be enabled using the following
[feature flag](../../development/feature_flags/development.md#enabling-a-feature-flag-locally-in-development):
- `project_merge_request_analytics`
A GitLab administrator can:
- Enable this feature by running the following command in a Rails console:
```ruby
Feature.enable(:project_merge_request_analytics)
```
......@@ -62,29 +62,3 @@ The **Productivity Analytics** dashboard can be accessed only:
- On [Premium or Silver tier](https://about.gitlab.com/pricing/) and above.
- By users with [Reporter access](../permissions.md) and above.
## Enabling and disabling using feature flags
Productivity Analytics is:
- [Enabled by default](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/18754) from GitLab 12.4,
but can be disabled using the following feature flags:
- `productivity_analytics`.
- `productivity_analytics_scatterplot_enabled`.
- Disabled by default in GitLab 12.3, but can be enabled using the following feature flag:
- `productivity_analytics`.
A GitLab administrator can:
- Disable this feature from GitLab 12.4 by running the follow in a Rails console:
```ruby
Feature.disable(:productivity_analytics)
Feature.disable(:productivity_analytics_scatterplot_enabled)
```
- Enable this feature in GitLab 12.3 by running the following in a Rails console:
```ruby
Feature.enable(:productivity_analytics)
```
......@@ -6,12 +6,6 @@ class Groups::Analytics::ApplicationController < ApplicationController
private
def self.check_feature_flag(flag, *args)
before_action(*args) do
render_404 unless Gitlab::Analytics.feature_enabled?(flag)
end
end
def self.increment_usage_counter(counter_klass, counter, *args)
before_action(*args) { counter_klass.count(counter) }
end
......@@ -36,5 +30,5 @@ class Groups::Analytics::ApplicationController < ApplicationController
@project = find_routable!(@group.projects, params['project_id'])
end
private_class_method :check_feature_flag, :increment_usage_counter
private_class_method :increment_usage_counter
end
# frozen_string_literal: true
class Groups::Analytics::CoverageReportsController < Groups::Analytics::ApplicationController
check_feature_flag Gitlab::Analytics::CYCLE_ANALYTICS_FEATURE_FLAG
COVERAGE_PARAM = 'coverage'.freeze
before_action :load_group
before_action -> { check_feature_availability!(:group_coverage_reports) }
before_action :check_feature_flag
def index
respond_to do |format|
......@@ -52,4 +51,8 @@ class Groups::Analytics::CoverageReportsController < Groups::Analytics::Applicat
value: @group.id
}
end
def check_feature_flag
render_404 unless @group.feature_available?(:group_coverage_reports)
end
end
......@@ -7,8 +7,6 @@ module Groups
include CycleAnalyticsParams
extend ::Gitlab::Utils::Override
check_feature_flag Gitlab::Analytics::CYCLE_ANALYTICS_FEATURE_FLAG
before_action :load_group
before_action :load_value_stream
before_action :validate_params, only: %i[median records duration_chart]
......
......@@ -6,8 +6,6 @@ module Groups
class SummaryController < Groups::Analytics::ApplicationController
include CycleAnalyticsParams
check_feature_flag Gitlab::Analytics::CYCLE_ANALYTICS_FEATURE_FLAG
before_action :load_group
before_action :authorize_access
before_action :validate_params
......
......@@ -3,8 +3,6 @@
class Groups::Analytics::CycleAnalytics::ValueStreamsController < Groups::Analytics::ApplicationController
respond_to :json
check_feature_flag Gitlab::Analytics::CYCLE_ANALYTICS_FEATURE_FLAG
before_action :load_group
before_action do
render_403 unless can?(current_user, :read_group_cycle_analytics, @group)
......
......@@ -5,7 +5,6 @@ class Groups::Analytics::CycleAnalyticsController < Groups::Analytics::Applicati
include CycleAnalyticsParams
extend ::Gitlab::Utils::Override
check_feature_flag Gitlab::Analytics::CYCLE_ANALYTICS_FEATURE_FLAG
increment_usage_counter Gitlab::UsageDataCounters::CycleAnalyticsCounter, :views, only: :show
before_action :load_group, only: :show
......
......@@ -3,8 +3,6 @@
class Groups::Analytics::MergeRequestAnalyticsController < Groups::Analytics::ApplicationController
include Analytics::UniqueVisitsHelper
check_feature_flag Gitlab::Analytics::GROUP_MERGE_REQUEST_ANALYTICS_FEATURE_FLAG
layout 'group'
before_action :load_group
......
# frozen_string_literal: true
class Groups::Analytics::ProductivityAnalyticsController < Groups::Analytics::ApplicationController
check_feature_flag Gitlab::Analytics::PRODUCTIVITY_ANALYTICS_FEATURE_FLAG
increment_usage_counter Gitlab::UsageDataCounters::ProductivityAnalyticsCounter,
:views, only: :show, if: -> { request.format.html? }
......
# frozen_string_literal: true
class Groups::Analytics::RepositoryAnalyticsController < Groups::Analytics::ApplicationController
check_feature_flag Gitlab::Analytics::GROUP_COVERAGE_REPORTS_FEATURE_FLAG
layout 'group'
before_action :load_group
before_action :check_feature_flag
before_action -> { check_feature_availability!(:group_repository_analytics) }
before_action -> { authorize_view_by_action!(:read_group_repository_analytics) }
before_action -> { push_frontend_feature_flag(:group_coverage_data_report, @group, default_enabled: false) }
......@@ -22,4 +21,8 @@ class Groups::Analytics::RepositoryAnalyticsController < Groups::Analytics::Appl
value: @group.id
}
end
def check_feature_flag
render_404 unless @group.feature_available?(:group_coverage_reports)
end
end
......@@ -43,7 +43,6 @@ module EE
def project_merge_request_analytics_navbar_link(project, current_user)
return unless project_nav_tab?(:merge_request_analytics)
return unless ::Gitlab::Analytics.project_merge_request_analytics_enabled?
navbar_sub_item(
title: _('Merge Request'),
......@@ -52,9 +51,10 @@ module EE
)
end
# Currently an empty page, so don't show it on the navbar for now
def group_merge_request_analytics_navbar_link(group, current_user)
return unless ::Gitlab::Analytics.group_merge_request_analytics_enabled?
return unless group_sidebar_link?(:merge_request_analytics)
return
return unless group_sidebar_link?(:merge_request_analytics) # rubocop: disable Lint/UnreachableCode
navbar_sub_item(
title: _('Merge Request'),
......@@ -116,7 +116,7 @@ module EE
end
def group_repository_analytics_navbar_link(group, current_user)
return unless ::Gitlab::Analytics.group_coverage_reports_enabled?
return unless group.feature_available?(:group_coverage_reports)
return unless group_sidebar_link?(:repository_analytics)
navbar_sub_item(
......
---
title: Remove `project_merge_request_analytics` feature flag
merge_request: 41876
author:
type: changed
---
name: cycle_analytics
name: group_coverage_reports
introduced_by_url:
rollout_issue_url:
group:
type: development
type: licensed
group: group::analytics
default_enabled: true
......@@ -23,16 +23,20 @@ constraints(::Constraints::GroupUrlConstrainer.new) do
resource :contribution_analytics, only: [:show]
namespace :analytics do
resource :productivity_analytics, only: :show, constraints: -> (req) { Gitlab::Analytics.productivity_analytics_enabled? }
resources :coverage_reports, only: :index, constraints: -> (req) { Gitlab::Analytics.group_coverage_reports_enabled? }
resource :merge_request_analytics, only: :show, constraints: -> (req) { Gitlab::Analytics.group_merge_request_analytics_enabled? }
resource :repository_analytics, only: :show, constraints: -> (req) { Gitlab::Analytics.group_coverage_reports_enabled? }
feature_default_enabled = Gitlab::Analytics.feature_enabled_by_default?(Gitlab::Analytics::CYCLE_ANALYTICS_FEATURE_FLAG)
constrainer = ::Constraints::FeatureConstrainer.new(Gitlab::Analytics::CYCLE_ANALYTICS_FEATURE_FLAG, default_enabled: feature_default_enabled)
constraints(constrainer) do
resource :cycle_analytics, only: :show, path: 'value_stream_analytics'
scope module: :cycle_analytics, as: 'cycle_analytics', path: 'value_stream_analytics' do
resource :productivity_analytics, only: :show
resources :coverage_reports, only: :index
resource :merge_request_analytics, only: :show
resource :repository_analytics, only: :show
resource :cycle_analytics, only: :show, path: 'value_stream_analytics'
scope module: :cycle_analytics, as: 'cycle_analytics', path: 'value_stream_analytics' do
resources :stages, only: [:index, :create, :update, :destroy] do
member do
get :duration_chart
get :median
get :records
end
end
resources :value_streams, only: [:index, :create, :destroy] do
resources :stages, only: [:index, :create, :update, :destroy] do
member do
get :duration_chart
......@@ -40,20 +44,11 @@ constraints(::Constraints::GroupUrlConstrainer.new) do
get :records
end
end
resources :value_streams, only: [:index, :create, :destroy] do
resources :stages, only: [:index, :create, :update, :destroy] do
member do
get :duration_chart
get :median
get :records
end
end
end
resource :summary, controller: :summary, only: :show
get '/time_summary' => 'summary#time_summary'
end
get '/cycle_analytics', to: redirect('-/analytics/value_stream_analytics')
resource :summary, controller: :summary, only: :show
get '/time_summary' => 'summary#time_summary'
end
get '/cycle_analytics', to: redirect('-/analytics/value_stream_analytics')
scope :type_of_work do
resource :tasks_by_type, controller: :tasks_by_type, only: :show do
......
......@@ -84,7 +84,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
namespace :analytics do
resources :code_reviews, only: [:index]
resource :issues_analytics, only: [:show]
resource :merge_request_analytics, only: :show, constraints: -> (req) { Gitlab::Analytics.project_merge_request_analytics_enabled? }
resource :merge_request_analytics, only: :show
end
resources :approvers, only: :destroy
......
......@@ -2,70 +2,5 @@
module Gitlab
module Analytics
# Normally each analytics feature should be guarded with a feature flag.
CYCLE_ANALYTICS_FEATURE_FLAG = :cycle_analytics
PRODUCTIVITY_ANALYTICS_FEATURE_FLAG = :productivity_analytics
GROUP_COVERAGE_REPORTS_FEATURE_FLAG = :group_coverage_reports
GROUP_MERGE_REQUEST_ANALYTICS_FEATURE_FLAG = :group_merge_request_analytics
PROJECT_MERGE_REQUEST_ANALYTICS_FEATURE_FLAG = :project_merge_request_analytics
FEATURE_FLAGS = [
CYCLE_ANALYTICS_FEATURE_FLAG,
GROUP_COVERAGE_REPORTS_FEATURE_FLAG,
PRODUCTIVITY_ANALYTICS_FEATURE_FLAG
].freeze
# Improve that in https://gitlab.com/gitlab-org/gitlab/-/issues/246768
FEATURE_FLAG_DEFAULTS = {
PRODUCTIVITY_ANALYTICS_FEATURE_FLAG => true,
GROUP_COVERAGE_REPORTS_FEATURE_FLAG => true,
CYCLE_ANALYTICS_FEATURE_FLAG => true,
PROJECT_MERGE_REQUEST_ANALYTICS_FEATURE_FLAG => true
}.freeze
FEATURE_FLAGS_TYPE = {
# TODO: it seems that we use a licensed "feature"
PRODUCTIVITY_ANALYTICS_FEATURE_FLAG => :licensed,
GROUP_COVERAGE_REPORTS_FEATURE_FLAG => :licensed,
GROUP_MERGE_REQUEST_ANALYTICS_FEATURE_FLAG => :licensed,
CYCLE_ANALYTICS_FEATURE_FLAG => :development,
PROJECT_MERGE_REQUEST_ANALYTICS_FEATURE_FLAG => :licensed
}.freeze
def self.any_features_enabled?
FEATURE_FLAGS.any? do |flag|
feature_enabled?(flag)
end
end
def self.cycle_analytics_enabled?
feature_enabled?(CYCLE_ANALYTICS_FEATURE_FLAG)
end
def self.productivity_analytics_enabled?
feature_enabled?(PRODUCTIVITY_ANALYTICS_FEATURE_FLAG)
end
def self.group_coverage_reports_enabled?
feature_enabled?(GROUP_COVERAGE_REPORTS_FEATURE_FLAG)
end
def self.group_merge_request_analytics_enabled?
feature_enabled?(GROUP_MERGE_REQUEST_ANALYTICS_FEATURE_FLAG)
end
def self.project_merge_request_analytics_enabled?
feature_enabled?(PROJECT_MERGE_REQUEST_ANALYTICS_FEATURE_FLAG)
end
def self.feature_enabled_by_default?(flag)
!!FEATURE_FLAG_DEFAULTS[flag]
end
def self.feature_enabled?(feature)
Feature.enabled?(feature,
type: FEATURE_FLAGS_TYPE.fetch(feature),
default_enabled: feature_enabled_by_default?(feature))
end
end
end
......@@ -55,21 +55,6 @@ RSpec.describe Groups::Analytics::CoverageReportsController do
end
end
context 'with the feature flag shut off' do
before do
stub_licensed_features(group_coverage_reports: true)
stub_feature_flags(group_coverage_reports: false)
end
describe 'GET index' do
it 'responds 403 because the feature is not licensed' do
get :index, params: valid_request_params
expect(response).to have_gitlab_http_status(:forbidden)
end
end
end
describe 'GET index' do
before do
stub_licensed_features(group_coverage_reports: true)
......
......@@ -8,7 +8,6 @@ RSpec.describe Groups::Analytics::CycleAnalytics::SummaryController do
let(:params) { { group_id: group.full_path, created_after: '2010-01-01', created_before: '2010-01-02' } }
before do
stub_feature_flags(Gitlab::Analytics::CYCLE_ANALYTICS_FEATURE_FLAG => true)
stub_licensed_features(cycle_analytics_for_groups: true)
group.add_reporter(user)
......
......@@ -8,7 +8,6 @@ RSpec.describe Groups::Analytics::CycleAnalytics::ValueStreamsController do
let(:params) { { group_id: group } }
before do
stub_feature_flags(Gitlab::Analytics::CYCLE_ANALYTICS_FEATURE_FLAG => true)
stub_licensed_features(cycle_analytics_for_groups: true)
group.add_maintainer(user)
......
......@@ -31,20 +31,10 @@ RSpec.describe Groups::Analytics::CycleAnalyticsController do
end
it 'renders `show` template when feature flag is enabled' do
stub_feature_flags(Gitlab::Analytics::CYCLE_ANALYTICS_FEATURE_FLAG => true)
get(:show, params: { group_id: group })
expect(response).to render_template :show
end
it 'renders `404` when feature flag is disabled' do
stub_feature_flags(Gitlab::Analytics::CYCLE_ANALYTICS_FEATURE_FLAG => false)
get(:show, params: { group_id: group })
expect(response).to have_gitlab_http_status(:not_found)
end
end
context 'when the license is missing' do
......
......@@ -5,13 +5,11 @@ require 'spec_helper'
RSpec.describe Groups::Analytics::MergeRequestAnalyticsController do
let_it_be(:current_user) { create(:user) }
let_it_be(:group) { create :group }
let_it_be(:feature_flag_name) { Gitlab::Analytics::GROUP_MERGE_REQUEST_ANALYTICS_FEATURE_FLAG }
let_it_be(:feature_name) { :group_merge_request_analytics }
before do
sign_in(current_user)
stub_feature_flags(feature_flag_name => true)
stub_licensed_features(feature_name => true)
end
......@@ -37,14 +35,6 @@ RSpec.describe Groups::Analytics::MergeRequestAnalyticsController do
it { is_expected.to have_gitlab_http_status(:forbidden) }
end
context 'when feature flag is off' do
before do
stub_feature_flags(feature_flag_name => false)
end
it { is_expected.to have_gitlab_http_status(:not_found) }
end
context 'when the user has no access to the group' do
before do
current_user.group_members.delete_all
......@@ -52,17 +42,5 @@ RSpec.describe Groups::Analytics::MergeRequestAnalyticsController do
it { is_expected.to have_gitlab_http_status(:forbidden) }
end
context 'when requesting HTML' do
render_views
before do
get :show, params: { group_id: group }, format: :html
end
it 'renders the side navigation with the correct submenu set as active' do
expect(response.body).to have_active_sub_navigation('Merge Request')
end
end
end
end
......@@ -51,18 +51,6 @@ RSpec.describe Groups::Analytics::ProductivityAnalyticsController do
end
end
context 'when productivity_analytics feature flag is disabled' do
before do
stub_feature_flags(Gitlab::Analytics::PRODUCTIVITY_ANALYTICS_FEATURE_FLAG => false)
end
it 'renders 404, not found error' do
subject
expect(response).to have_gitlab_http_status(:not_found)
end
end
context 'when feature is not licensed' do
before do
stub_licensed_features(productivity_analytics: false)
......
......@@ -5,14 +5,12 @@ require 'spec_helper'
RSpec.describe Groups::Analytics::RepositoryAnalyticsController do
let_it_be(:current_user) { create(:user) }
let_it_be(:group) { create(:group) }
let_it_be(:feature_flag_name) { Gitlab::Analytics::GROUP_COVERAGE_REPORTS_FEATURE_FLAG }
let_it_be(:feature_name) { :group_repository_analytics }
before do
sign_in(current_user)
stub_feature_flags(feature_flag_name => true)
stub_licensed_features(feature_name => true)
stub_licensed_features(feature_name => true, :group_coverage_reports => true)
end
describe 'GET show', :snowplow do
......@@ -37,15 +35,15 @@ RSpec.describe Groups::Analytics::RepositoryAnalyticsController do
context 'when license is missing' do
before do
stub_licensed_features(feature_name => false)
stub_licensed_features(feature_name => false, :group_coverage_reports => true)
end
it { is_expected.to have_gitlab_http_status(:forbidden) }
end
context 'when feature flag is off' do
context 'when `group_repository_analytics` licensed feature flag is off' do
before do
stub_feature_flags(feature_flag_name => false)
stub_feature_flags(group_coverage_reports: false)
end
it { is_expected.to have_gitlab_http_status(:not_found) }
......
......@@ -6,13 +6,11 @@ RSpec.describe Projects::Analytics::MergeRequestAnalyticsController do
let_it_be(:current_user) { create(:user) }
let_it_be(:group) { create(:group) }
let_it_be(:project) { create(:project, group: group) }
let_it_be(:feature_flag_name) { Gitlab::Analytics::PROJECT_MERGE_REQUEST_ANALYTICS_FEATURE_FLAG }
let_it_be(:feature_name) { :project_merge_request_analytics }
before do
sign_in(current_user)
stub_feature_flags(feature_flag_name => true)
stub_licensed_features(feature_name => true)
end
......@@ -38,14 +36,6 @@ RSpec.describe Projects::Analytics::MergeRequestAnalyticsController do
it { is_expected.to have_gitlab_http_status(:not_found) }
end
context 'when feature flag is off' do
before do
stub_feature_flags(feature_flag_name => false)
end
it { is_expected.to have_gitlab_http_status(:not_found) }
end
context 'when the user has no access to the group' do
before do
current_user.project_authorizations.delete_all
......
......@@ -37,32 +37,6 @@ RSpec.describe 'Group navbar' do
it_behaves_like 'verified navigation bar'
end
context 'when merge request analytics is available' do
before do
stub_licensed_features(group_merge_request_analytics: true)
insert_after_sub_nav_item(
_('Contribution'),
within: _('Analytics'),
new_sub_nav_item_name: _('Merge Request')
)
visit group_path(group)
end
it_behaves_like 'verified navigation bar'
end
context 'when merge request analytics is unavailable' do
before do
stub_feature_flags(group_merge_request_analytics: false)
visit group_path(group)
end
it_behaves_like 'verified navigation bar'
end
context 'when value stream analytics is available' do
before do
stub_licensed_features(cycle_analytics_for_groups: true)
......
......@@ -130,7 +130,6 @@ RSpec.describe 'Analytics (JavaScript fixtures)', :sidekiq_inline do
let(:params) { { created_after: 3.months.ago, created_before: Time.now, group_id: group.full_path } }
before do
stub_feature_flags(Gitlab::Analytics::CYCLE_ANALYTICS_FEATURE_FLAG => true)
stub_licensed_features(cycle_analytics_for_groups: true)
# Persist the default stages
......@@ -199,7 +198,6 @@ RSpec.describe 'Analytics (JavaScript fixtures)', :sidekiq_inline do
end
before do
stub_feature_flags(Gitlab::Analytics::CYCLE_ANALYTICS_FEATURE_FLAG => true)
stub_licensed_features(cycle_analytics_for_groups: true)
prepare_cycle_analytics_data
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Gitlab::Analytics do
describe '.productivity_analytics_enabled?' do
it 'is enabled by default' do
expect(described_class).to be_productivity_analytics_enabled
end
end
end
......@@ -2,7 +2,6 @@
RSpec.shared_examples 'cycle analytics stages controller' do
before do
stub_feature_flags(Gitlab::Analytics::CYCLE_ANALYTICS_FEATURE_FLAG => true)
stub_licensed_features(cycle_analytics_for_groups: true)
group.add_reporter(user)
......@@ -242,18 +241,6 @@ RSpec.shared_examples 'group permission check on the controller level' do
end
end
context 'when feature flag is disabled' do
before do
stub_feature_flags(Gitlab::Analytics::CYCLE_ANALYTICS_FEATURE_FLAG => false)
end
it 'renders `not_found` response' do
subject
expect(response).to have_gitlab_http_status(:not_found)
end
end
context 'when user has no lower access level than `reporter`' do
before do
GroupMember.where(user: user).delete_all
......
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