Merge branch '196454-move-productivity-analytics-to-group-level' into 'master'

Move Productivity Analytics to the group level

See merge request gitlab-org/gitlab!24148

ee/app/assets/javascripts/analytics/productivity_analytics/components/filter_dropdowns.vue

@@ -21,6 +21,10 @@ export default {
       required: false,
       default: null,
     },
+    hideGroupDropDown: {
+      type: Boolean,
+      default: false,
+    },
   },
   data() {
     return {
@@ -76,6 +80,7 @@ export default {
 <template>
   <div class="dropdown-container d-flex flex-column flex-lg-row">
     <groups-dropdown-filter
+      v-if="!hideGroupDropDown"
       class="group-select"
       :query-params="$options.groupsQueryParams"
       :default-group="group"

ee/app/assets/javascripts/analytics/productivity_analytics/index.js

@@ -11,6 +11,7 @@ import {
   buildGroupFromDataset,
   buildProjectFromDataset,
 } from './utils';
+import { parseBoolean } from '~/lib/utils/common_utils';
 
 export default () => {
   const container = document.getElementById('js-productivity-analytics');
@@ -39,6 +40,8 @@ export default () => {
     : null;
 
   const group = buildGroupFromDataset(container.dataset);
+  const hideGroupDropDown = parseBoolean(container.dataset.hideGroupDropDown);
+
   let project = null;
 
   let initialData = {
@@ -126,6 +129,7 @@ export default () => {
         props: {
           group,
           project,
+          hideGroupDropDown,
         },
         on: {
           groupSelected: this.onGroupSelected,

ee/app/assets/javascripts/pages/groups/analytics/productivity_analytics/index.js

+import initProductivityAnalyticsApp from 'ee/analytics/productivity_analytics';
+
+document.addEventListener('DOMContentLoaded', () => {
+  initProductivityAnalyticsApp();
+});

ee/app/controllers/analytics/analytics_controller.rb

@@ -2,7 +2,7 @@
 
 class Analytics::AnalyticsController < Analytics::ApplicationController
   def index
-    if Gitlab::Analytics.productivity_analytics_enabled?
+    if Feature.disabled?(:group_level_productivity_analytics) && Gitlab::Analytics.productivity_analytics_enabled?
       redirect_to analytics_productivity_analytics_path
     elsif Gitlab::Analytics.cycle_analytics_enabled?
       redirect_to analytics_cycle_analytics_path

ee/app/controllers/analytics/productivity_analytics_controller.rb

@@ -15,6 +15,7 @@ class Analytics::ProductivityAnalyticsController < Analytics::ApplicationControl
   before_action -> {
     authorize_view_by_action!(:view_productivity_analytics)
   }
+
   before_action -> {
     push_frontend_feature_flag(:productivity_analytics_scatterplot_enabled, default_enabled: true)
   }

ee/app/controllers/groups/analytics/application_controller.rb

+# frozen_string_literal: true
+
+class Groups::Analytics::ApplicationController < ApplicationController
+  include RoutableActions
+
+  private
+
+  def self.check_feature_flag(flag, *args)
+    before_action(*args) { render_404 unless Feature.enabled?(flag, default_enabled: Gitlab::Analytics.feature_enabled_by_default?(flag)) }
+  end
+
+  def self.increment_usage_counter(counter_klass, counter, *args)
+    before_action(*args) { counter_klass.count(counter) }
+  end
+
+  def authorize_view_by_action!(action)
+    return render_403 unless can?(current_user, action, @group)
+  end
+
+  def check_feature_availability!(feature)
+    return render_403 unless @group && @group.feature_available?(feature)
+  end
+
+  def load_group
+    return unless params['group_id']
+
+    @group = find_routable!(Group, params['group_id'])
+  end
+
+  def load_project
+    return unless @group && params['project_id']
+
+    @project = find_routable!(@group.projects, params['project_id'])
+  end
+
+  private_class_method :check_feature_flag, :increment_usage_counter
+end

ee/app/controllers/groups/analytics/productivity_analytics_controller.rb

+# 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? }
+
+  layout 'group'
+
+  before_action :load_group
+  before_action :load_project
+  before_action :build_request_params
+  before_action -> {
+    check_feature_availability!(:productivity_analytics)
+  }
+
+  before_action -> {
+    authorize_view_by_action!(:view_productivity_analytics)
+  }
+  before_action -> {
+    push_frontend_feature_flag(:productivity_analytics_scatterplot_enabled, default_enabled: true)
+  }
+
+  before_action :validate_params, only: :show, if: -> { request.format.json? }
+
+  include IssuableCollections
+
+  def show
+    respond_to do |format|
+      format.html
+      format.json do
+        metric = params.fetch('metric_type', ProductivityAnalytics::DEFAULT_TYPE)
+
+        data = case params['chart_type']
+               when 'scatterplot'
+                 productivity_analytics.scatterplot_data(type: metric)
+               when 'histogram'
+                 productivity_analytics.histogram_data(type: metric)
+               else
+                 include_relations(paginate(productivity_analytics.merge_requests_extended)).map do |merge_request|
+                   serializer.represent(merge_request, {}, ProductivityAnalyticsMergeRequestEntity)
+                 end
+               end
+
+        render json: data, status: :ok
+      end
+    end
+  end
+
+  private
+
+  def paginate(merge_requests)
+    merge_requests.page(params[:page]).per(params[:per_page]).tap do |paginated_data|
+      response.set_header('X-Per-Page', paginated_data.limit_value.to_s)
+      response.set_header('X-Page', paginated_data.current_page.to_s)
+      response.set_header('X-Next-Page', paginated_data.next_page.to_s)
+      response.set_header('X-Prev-Page', paginated_data.prev_page.to_s)
+      response.set_header('X-Total', paginated_data.total_count.to_s)
+      response.set_header('X-Total-Pages', paginated_data.total_pages.to_s)
+    end
+  end
+
+  def serializer
+    @serializer ||= BaseSerializer.new(current_user: current_user)
+  end
+
+  def finder_type
+    ProductivityAnalyticsFinder
+  end
+
+  def default_state
+    'merged'
+  end
+
+  def validate_params
+    if @request_params.invalid?
+      render(
+        json: { message: 'Invalid parameters', errors: @request_params.errors },
+        status: :unprocessable_entity
+      )
+    end
+  end
+
+  def build_request_params
+    @request_params ||= ::Analytics::ProductivityAnalyticsRequestParams.new(allowed_request_params.merge(group: @group, project: @project))
+  end
+
+  def allowed_request_params
+    params.permit(
+      :merged_after,
+      :merged_before,
+      :author_username,
+      :milestone_title,
+      label_name: []
+    )
+  end
+
+  def productivity_analytics
+    @productivity_analytics ||= ProductivityAnalytics.new(merge_requests: finder.execute, sort: params[:sort])
+  end
+
+  # rubocop: disable CodeReuse/ActiveRecord
+  def include_relations(paginated_mrs)
+    # Due to Rails bug: https://github.com/rails/rails/issues/34889 we can't use .includes statement
+    # to avoid N+1 call when we load custom columns.
+    # So we load relations manually here.
+    preloader = ActiveRecord::Associations::Preloader.new
+    preloader.preload(paginated_mrs, { author: [], target_project: { namespace: :route } })
+    paginated_mrs
+  end
+  # rubocop: enable CodeReuse/ActiveRecord
+end

ee/app/helpers/ee/analytics_navbar_helper.rb

@@ -17,12 +17,25 @@ module EE
       super + [
         contribution_analytics_navbar_link(group, current_user),
         group_insights_navbar_link(group, current_user),
-        issues_analytics_navbar_link(group, current_user)
+        issues_analytics_navbar_link(group, current_user),
+        productivity_analytics_navbar_link(group, current_user)
       ].compact
     end
 
     private
 
+    def productivity_analytics_navbar_link(group, current_user)
+      return unless ::Feature.enabled?(:analytics_pages_under_group_analytics_sidebar, group)
+      return unless ::Feature.enabled?(:group_level_productivity_analytics)
+      return unless group_sidebar_link?(:productivity_analytics)
+
+      navbar_sub_item(
+        title: _('Productivity Analytics'),
+        path: 'groups/analytics/productivity_analytics#show',
+        link: group_analytics_productivity_analytics_path(@group)
+      )
+    end
+
     def contribution_analytics_navbar_link(group, current_user)
       return unless ::Feature.enabled?(:analytics_pages_under_group_analytics_sidebar, group)
       return unless group_sidebar_link?(:contribution_analytics)

ee/app/helpers/ee/groups_helper.rb

@@ -112,6 +112,10 @@ module EE
         links << :group_insights
       end
 
+      if @group.feature_available?(:productivity_analytics) && can?(current_user, :view_productivity_analytics, @group)
+        links << :productivity_analytics
+      end
+
       links
     end
   end

ee/app/views/groups/analytics/productivity_analytics/show.html.haml

+- page_title _('Productivity Analytics')
+
+- data_attributes = @request_params.valid? ? @request_params.to_data_attributes : @request_params.to_default_data_attributes
+
+#js-productivity-analytics{ data: data_attributes.merge(hide_group_drop_down: 'true') }
+  .page-title-holder.d-flex.align-items-center
+    .page-title
+      = _('Productivity Analytics')
+  .row-content-block.second-block.d-flex.flex-column.flex-lg-row.mt-3.py-2.px-3
+    .js-group-project-select-container
+    .js-search-bar.filter-container.hide
+      = render 'shared/issuable/search_bar', type: :productivity_analytics
+    .js-timeframe-container{ data: { start_date: ProductivityAnalytics.start_date } }
+  .js-productivity-analytics-app-container{ data: { endpoint: group_analytics_productivity_analytics_path(@group), empty_state_svg_path: image_path('illustrations/productivity-analytics-empty-state.svg'), no_access_svg_path: image_path('illustrations/analytics/no-access.svg') } }

ee/app/views/layouts/nav/sidebar/_analytics.html.haml

@@ -6,7 +6,7 @@
           = sprite_icon('chart', size: 24)
         .sidebar-context-title= _('Analytics')
     %ul.sidebar-top-level-items
-      - if Gitlab::Analytics.productivity_analytics_enabled?
+      - if Feature.disabled?(:group_level_productivity_analytics) && Gitlab::Analytics.productivity_analytics_enabled?
         = nav_link(controller: :productivity_analytics) do
           = link_to analytics_productivity_analytics_path, class: 'qa-sidebar-productivity-analytics' do
             .nav-icon-container

ee/config/routes/analytics.rb

@@ -3,7 +3,7 @@
 namespace :analytics do
   root to: 'analytics#index'
 
-  resource :productivity_analytics, only: :show, constraints: -> (req) { Gitlab::Analytics.productivity_analytics_enabled? }
+  resource :productivity_analytics, only: :show, constraints: -> (req) { Feature.disabled?(:group_level_productivity_analytics) && Gitlab::Analytics.productivity_analytics_enabled? }
 
   constraints(-> (req) { Gitlab::Analytics.cycle_analytics_enabled? }) do
     resource :cycle_analytics, only: :show

ee/config/routes/group.rb

@@ -31,6 +31,9 @@ constraints(::Constraints::GroupUrlConstrainer.new) do
         get :production
       end
     end
+    namespace :analytics do
+      resource :productivity_analytics, only: :show, constraints: -> (req) { Feature.enabled?(:group_level_productivity_analytics) && Gitlab::Analytics.productivity_analytics_enabled? }
+    end
 
     resource :ldap, only: [] do
       member do

ee/spec/controllers/analytics/analytics_controller_spec.rb

@@ -8,6 +8,8 @@ describe Analytics::AnalyticsController do
   let(:user) { create(:user) }
 
   before do
+    stub_feature_flags(group_level_productivity_analytics: false)
+
     sign_in(user)
     disable_all_analytics_feature_flags
   end

ee/spec/controllers/analytics/productivity_analytics_controller_spec.rb

@@ -9,6 +9,7 @@ describe Analytics::ProductivityAnalyticsController do
   before do
     sign_in(current_user) if current_user
 
+    stub_feature_flags(group_level_productivity_analytics: false)
     stub_licensed_features(productivity_analytics: true)
   end
 

ee/spec/controllers/groups/analytics/productivity_analytics_controller_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Groups::Analytics::ProductivityAnalyticsController do
+  let_it_be(:current_user) { create(:user) }
+  let_it_be(:group) { create :group }
+
+  before do
+    sign_in(current_user)
+
+    stub_licensed_features(productivity_analytics: true)
+  end
+
+  describe 'usage counter' do
+    before do
+      group.add_owner(current_user)
+    end
+
+    it 'increments usage counter' do
+      expect(Gitlab::UsageDataCounters::ProductivityAnalyticsCounter).to receive(:count).with(:views)
+
+      get :show, format: :html, params: { group_id: group }
+
+      expect(response).to be_successful
+    end
+
+    it "doesn't increment the usage counter when JSON request is sent" do
+      expect(Gitlab::UsageDataCounters::ProductivityAnalyticsCounter).not_to receive(:count).with(:views)
+
+      get :show, format: :json, params: { group_id: group }
+
+      expect(response).to be_successful
+    end
+  end
+
+  describe 'GET show' do
+    subject { get :show, params: { group_id: group } }
+
+    context 'when user is not authorized to view productivity analytics' do
+      before do
+        expect(Ability).to receive(:allowed?).with(current_user, :read_group, group).and_return(true)
+        expect(Ability).to receive(:allowed?).with(current_user, :view_productivity_analytics, group).and_return(false)
+      end
+
+      it 'renders 403, forbidden error' do
+        subject
+
+        expect(response).to have_gitlab_http_status(:forbidden)
+      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)
+      end
+
+      it 'renders forbidden error' do
+        subject
+
+        expect(response).to have_gitlab_http_status(:forbidden)
+      end
+    end
+  end
+
+  describe 'GET show.json' do
+    subject { get :show, format: :json, params: params }
+
+    let(:params) { { group_id: group } }
+    let(:analytics_mock) { instance_double('ProductivityAnalytics') }
+
+    before do
+      merge_requests = double
+      allow_any_instance_of(ProductivityAnalyticsFinder).to receive(:execute).and_return(merge_requests)
+      allow(ProductivityAnalytics)
+        .to receive(:new)
+              .with(merge_requests: merge_requests, sort: params[:sort])
+              .and_return(analytics_mock)
+    end
+
+    context 'when feature is not licensed' do
+      before do
+        stub_licensed_features(productivity_analytics: false)
+      end
+
+      it 'renders forbidden error' do
+        subject
+
+        expect(response).to have_gitlab_http_status(:forbidden)
+      end
+    end
+
+    context 'when invalid params are given' do
+      let(:params) { { group_id: group, merged_before: 10.days.ago, merged_after: 5.days.ago } }
+
+      before do
+        group.add_owner(current_user)
+      end
+
+      it 'returns 422, unprocessable_entity' do
+        subject
+
+        expect(response).to have_gitlab_http_status(:unprocessable_entity)
+        expect(response).to match_response_schema('analytics/cycle_analytics/validation_error', dir: 'ee')
+      end
+    end
+
+    context 'without group_id specified' do
+      it 'renders 403, forbidden' do
+        subject
+
+        expect(response).to have_gitlab_http_status(:forbidden)
+      end
+    end
+
+    context 'with non-existing group_id' do
+      let(:params) { { group_id: 'SOMETHING_THAT_DOES_NOT_EXIST' } }
+
+      it 'renders 404, not_found' do
+        subject
+
+        expect(response).to have_gitlab_http_status(:not_found)
+      end
+    end
+
+    context 'with non-existing project_id' do
+      let(:params) { { group_id: group, project_id: 'SOMETHING_THAT_DOES_NOT_EXIST' } }
+
+      it 'renders 404, not_found' do
+        subject
+
+        expect(response).to have_gitlab_http_status(:not_found)
+      end
+    end
+
+    context 'with group specified' do
+      let(:params) { { group_id: group } }
+
+      before do
+        group.add_owner(current_user)
+      end
+
+      context 'for list of MRs' do
+        let!(:merge_request ) { create :merge_request, :merged}
+
+        let(:serializer_mock) { instance_double('BaseSerializer') }
+
+        before do
+          allow(BaseSerializer).to receive(:new).with(current_user: current_user).and_return(serializer_mock)
+          allow(analytics_mock).to receive(:merge_requests_extended).and_return(MergeRequest.all)
+          allow(serializer_mock).to receive(:represent)
+                                      .with(merge_request, {}, ProductivityAnalyticsMergeRequestEntity)
+                                      .and_return('mr_representation')
+        end
+
+        it 'serializes whatever analytics returns with ProductivityAnalyticsMergeRequestEntity' do
+          subject
+
+          expect(response.body).to eq '["mr_representation"]'
+        end
+
+        it 'sets pagination headers' do
+          subject
+
+          expect(response.headers['X-Per-Page']).to eq '20'
+          expect(response.headers['X-Page']).to eq '1'
+          expect(response.headers['X-Next-Page']).to eq ''
+          expect(response.headers['X-Prev-Page']).to eq ''
+          expect(response.headers['X-Total']).to eq '1'
+          expect(response.headers['X-Total-Pages']).to eq '1'
+        end
+      end
+
+      context 'for scatterplot charts' do
+        let(:params) { super().merge({ chart_type: 'scatterplot', metric_type: 'commits_count' }) }
+
+        it 'renders whatever analytics returns for scatterplot' do
+          allow(analytics_mock).to receive(:scatterplot_data).with(type: 'commits_count').and_return('scatterplot_data')
+
+          subject
+
+          expect(response.body).to eq 'scatterplot_data'
+        end
+      end
+
+      context 'for histogram charts' do
+        let(:params) { super().merge({ chart_type: 'histogram', metric_type: 'commits_count' }) }
+
+        it 'renders whatever analytics returns for histogram' do
+          allow(analytics_mock).to receive(:histogram_data).with(type: 'commits_count').and_return('histogram_data')
+
+          subject
+
+          expect(response.body).to eq 'histogram_data'
+        end
+      end
+    end
+  end
+end

ee/spec/features/analytics/productivity_analytics_spec.rb

@@ -20,6 +20,7 @@ describe 'ProductivityAnalytics' do
   end
 
   before do
+    stub_feature_flags(group_level_productivity_analytics: false)
     stub_licensed_features(productivity_analytics: true)
 
     sign_in(user)

ee/spec/features/groups/active_tabs_spec.rb

@@ -89,5 +89,16 @@ describe 'Group active tab' do
       it_behaves_like 'page has active tab', _('Analytics')
       it_behaves_like 'page has active sub tab', _('Contribution Analytics')
     end
+
+    context 'on group Productivity Analytics' do
+      before do
+        stub_licensed_features(productivity_analytics: true)
+
+        visit group_analytics_productivity_analytics_path(group)
+      end
+
+      it_behaves_like 'page has active tab', _('Analytics')
+      it_behaves_like 'page has active sub tab', _('Productivity Analytics')
+    end
   end
 end

ee/spec/routing/analytics_routing_spec.rb

@@ -13,6 +13,8 @@ describe 'Analytics' do
     let(:user) { create(:user) }
 
     before do
+      stub_feature_flags(group_level_productivity_analytics: false)
+
       login_as(user)
     end
 

ee/spec/views/layouts/nav/sidebar/_analytics.html.haml_spec.rb

@@ -7,6 +7,10 @@ describe 'layouts/nav/sidebar/_analytics' do
 
   it_behaves_like 'has nav sidebar'
 
+  before do
+    stub_feature_flags(group_level_productivity_analytics: false)
+  end
+
   context 'top-level items' do
     context 'when feature flags are enabled' do
       it 'has `Analytics` link' do

spec/features/groups/navbar_spec.rb

@@ -3,7 +3,6 @@
 require 'spec_helper'
 
 describe 'Group navbar' do
-  it_behaves_like 'verified navigation bar' do
   let(:user) { create(:user) }
   let(:group) { create(:group) }
 
@@ -50,11 +49,29 @@ describe 'Group navbar' do
     ]
   end
 
+  it_behaves_like 'verified navigation bar' do
+    before do
+      group.add_maintainer(user)
+      sign_in(user)
+
+      visit group_path(group)
+    end
+  end
+
+  if Gitlab.ee?
+    context 'when productivity analytics is available' do
       before do
+        stub_licensed_features(productivity_analytics: true)
+
+        analytics_nav_item[:nav_sub_items] << _('Productivity Analytics')
+
         group.add_maintainer(user)
         sign_in(user)
 
         visit group_path(group)
       end
+
+      it_behaves_like 'verified navigation bar'
+    end
   end
 end