Use the real query in code analytics

- Add feature flag for code analytics actions.
- Connect the controller to the real finder backend.
- Implement input parameter validation.
- Update the query to return the repository file id.

ee/app/controllers/analytics/analytics_controller.rb

@@ -6,6 +6,8 @@ class Analytics::AnalyticsController < Analytics::ApplicationController
       redirect_to analytics_productivity_analytics_path
     elsif Gitlab::Analytics.cycle_analytics_enabled?
       redirect_to analytics_cycle_analytics_path
+    elsif Gitlab::Analytics.code_analytics_enabled?
+      redirect_to analytics_code_analytics_path
     else
       render_404
     end

ee/app/controllers/analytics/code_analytics_controller.rb

+# frozen_string_literal: true
+
+class Analytics::CodeAnalyticsController < Analytics::ApplicationController
+  check_feature_flag Gitlab::Analytics::CODE_ANALYTICS_FEATURE_FLAG
+
+  before_action :load_group
+  before_action :load_project
+
+  before_action -> {
+    check_feature_availability!(:code_analytics)
+  }, if: -> { request.format.json? }
+
+  before_action -> {
+    authorize_view_by_action!(:view_code_analytics)
+  }
+
+  before_action :validate_params, if: -> { request.format.json? }
+
+  def show
+    respond_to do |format|
+      format.html
+      format.json { render json: Analytics::CodeAnalytics::RepositoryFileCommitCountEntity.represent(top_files) }
+    end
+  end
+
+  private
+
+  def validate_params
+    render(json: { message: 'Invalid parameters', errors: request_params.errors }, status: :unprocessable_entity) if request_params.invalid?
+  end
+
+  def request_params
+    @request_params ||= Gitlab::Analytics::CodeAnalytics::RequestParams.new(allowed_params)
+  end
+
+  def top_files
+    Analytics::CodeAnalyticsFinder.new(
+      project: @project,
+      file_count: request_params.file_count,
+      from: request_params.from,
+      to: request_params.to
+    ).execute
+  end
+
+  def allowed_params
+    params.permit(:file_count)
+  end
+end

ee/app/finders/analytics/code_analytics_finder.rb

@@ -2,6 +2,8 @@
 
 module Analytics
   class CodeAnalyticsFinder
+    RepositoryFileCommitCount = Struct.new(:repository_file, :count)
+
     def initialize(project:, from:, to:, file_count: nil)
       @project = project
       @from = from
@@ -10,7 +12,18 @@ module Analytics
     end
 
     def execute
-      Analytics::CodeAnalytics::RepositoryFileCommit.top_files(
+      result.map do |(id, file_path), count|
+        RepositoryFileCommitCount.new(
+          Analytics::CodeAnalytics::RepositoryFile.new(id: id, file_path: file_path),
+          count
+        )
+      end
+    end
+
+    private
+
+    def result
+      @result ||= Analytics::CodeAnalytics::RepositoryFileCommit.top_files(
         project: @project,
         from: @from,
         to: @to,

ee/app/models/analytics/code_analytics/repository_file_commit.rb

@@ -23,11 +23,11 @@ module Analytics
         raise TopFilesLimitError if file_count > MAX_FILE_COUNT
 
         joins(:analytics_repository_file)
-          .select(files_table[:file_path])
+          .select(files_table[:id], files_table[:file_path])
           .where(project_id: project.id)
           .where(arel_table[:committed_date].gteq(from))
           .where(arel_table[:committed_date].lteq(to))
-          .group(files_table[:file_path])
+          .group(files_table[:id], files_table[:file_path])
           .order(arel_table[:commit_count].sum)
           .limit(file_count)
           .sum(arel_table[:commit_count])

ee/app/models/license.rb

@@ -48,6 +48,7 @@ class License < ApplicationRecord
     board_milestone_lists
     ci_cd_projects
     cluster_deployments
+    code_analytics
     code_owner_approval_required
     commit_committer_check
     cross_project_pipelines

ee/app/serializers/analytics/code_analytics/repository_file_commit_count_entity.rb

+# frozen_string_literal: true
+
+module Analytics
+  module CodeAnalytics
+    class RepositoryFileCommitCountEntity < Grape::Entity
+      expose(:id) { |model| model.repository_file.id }
+      expose(:name) { |model| model.repository_file.file_path }
+      expose :count
+    end
+  end
+end

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

@@ -31,4 +31,16 @@
                 = link_to analytics_cycle_analytics_path do
                   %strong.fly-out-top-item-name
                     = _('Cycle Analytics')
+        - if Gitlab::Analytics.code_analytics_enabled?
+          = nav_link(controller: :code_analytics) do
+            = link_to analytics_code_analytics_path, class: 'qa-sidebar-code-analytics' do
+              .nav-icon-container
+                = sprite_icon('code')
+              %span.nav-item-name
+                = _('Code Analytics')
+            %ul.sidebar-sub-level-items.is-fly-out-only
+              = nav_link(controller: :code_analytics, html_options: { class: "fly-out-top-item qa-sidebar-code-analytics-fly-out" } ) do
+                = link_to analytics_code_analytics_path do
+                  %strong.fly-out-top-item-name
+                    = _('Code Analytics')
     = render 'shared/sidebar_toggle_button'

ee/config/routes/analytics.rb

@@ -19,4 +19,8 @@ namespace :analytics do
       resource :tasks_by_type, controller: :tasks_by_type, only: :show
     end
   end
+
+  constraints(::Constraints::FeatureConstrainer.new(Gitlab::Analytics::CODE_ANALYTICS_FEATURE_FLAG)) do
+    resource :code_analytics, only: :show
+  end
 end

ee/lib/gitlab/analytics.rb

@@ -3,11 +3,13 @@
 module Gitlab
   module Analytics
     # Normally each analytics feature should be guarded with a feature flag.
+    CODE_ANALYTICS_FEATURE_FLAG = :code_analytics
     CYCLE_ANALYTICS_FEATURE_FLAG = :cycle_analytics
     PRODUCTIVITY_ANALYTICS_FEATURE_FLAG = :productivity_analytics
     TASKS_BY_TYPE_CHART_FEATURE_FLAG = :tasks_by_type_chart
 
     FEATURE_FLAGS = [
+      CODE_ANALYTICS_FEATURE_FLAG,
       CYCLE_ANALYTICS_FEATURE_FLAG,
       PRODUCTIVITY_ANALYTICS_FEATURE_FLAG,
       TASKS_BY_TYPE_CHART_FEATURE_FLAG
@@ -17,6 +19,10 @@ module Gitlab
       FEATURE_FLAGS.any? { |flag| Feature.enabled?(flag) }
     end
 
+    def self.code_analytics_enabled?
+      Feature.enabled?(CODE_ANALYTICS_FEATURE_FLAG)
+    end
+
     def self.cycle_analytics_enabled?
       Feature.enabled?(CYCLE_ANALYTICS_FEATURE_FLAG)
     end

ee/lib/gitlab/analytics/code_analytics/request_params.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Analytics
+    module CodeAnalytics
+      class RequestParams
+        include ActiveModel::Model
+        include ActiveModel::Validations
+
+        attr_writer :file_count
+
+        validates :file_count, presence: true, numericality: {
+          only_integer: true,
+          greater_than: 0,
+          less_than_or_equal_to: ::Analytics::CodeAnalytics::RepositoryFileCommit::MAX_FILE_COUNT
+        }
+
+        # The date range will be customizable later, for now we load data for the last 30 days
+        def from
+          30.days.ago
+        end
+
+        def to
+          Date.today
+        end
+
+        def file_count
+          Integer(@file_count) if @file_count
+        end
+      end
+    end
+  end
+end

ee/spec/controllers/analytics/analytics_controller_spec.rb

@@ -9,12 +9,12 @@ describe Analytics::AnalyticsController do
 
   before do
     sign_in(user)
+    disable_all_analytics_feature_flags
   end
 
   describe 'GET index' do
     describe 'redirects to the first enabled analytics page' do
       it 'redirects to cycle analytics' do
-        disable_all_analytics_feature_flags
         stub_feature_flags(Gitlab::Analytics::CYCLE_ANALYTICS_FEATURE_FLAG => true)
 
         get :index
@@ -23,18 +23,23 @@ describe Analytics::AnalyticsController do
       end
 
       it 'redirects to productivity analytics' do
-        disable_all_analytics_feature_flags
         stub_feature_flags(Gitlab::Analytics::PRODUCTIVITY_ANALYTICS_FEATURE_FLAG => true)
 
         get :index
 
         expect(response).to redirect_to(analytics_productivity_analytics_path)
       end
+
+      it 'redirects to code analytics' do
+        stub_feature_flags(Gitlab::Analytics::CODE_ANALYTICS_FEATURE_FLAG => true)
+
+        get :index
+
+        expect(response).to redirect_to(analytics_code_analytics_path)
+      end
     end
 
     it 'renders 404 all the analytics feature flags are disabled' do
-      disable_all_analytics_feature_flags
-
       get :index
 
       expect(response).to have_gitlab_http_status(404)

ee/spec/controllers/analytics/code_analytics_controller_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Analytics::CodeAnalyticsController do
+  let_it_be(:current_user) { create(:user) }
+  let_it_be(:group) { create(:group) }
+  let_it_be(:project) { create(:project, group: group) }
+
+  before do
+    group.add_reporter(current_user)
+    stub_licensed_features(code_analytics: true)
+    sign_in(current_user)
+  end
+
+  describe 'GET show' do
+    subject { get :show, format: :html, params: {} }
+
+    it 'renders successfully without license' do
+      stub_feature_flags(Gitlab::Analytics::CODE_ANALYTICS_FEATURE_FLAG => true)
+      stub_licensed_features(code_analytics: false)
+
+      subject
+
+      expect(response).to have_gitlab_http_status(:ok)
+    end
+
+    it 'renders successfully with license' do
+      stub_feature_flags(Gitlab::Analytics::CODE_ANALYTICS_FEATURE_FLAG => true)
+      stub_licensed_features(code_analytics: true)
+
+      subject
+
+      expect(response).to have_gitlab_http_status(:ok)
+    end
+
+    it 'renders `not_found` when feature flag is disabled' do
+      stub_licensed_features(code_analytics: true)
+      stub_feature_flags(Gitlab::Analytics::CODE_ANALYTICS_FEATURE_FLAG => false)
+
+      subject
+
+      expect(response).to have_gitlab_http_status(:not_found)
+    end
+  end
+
+  describe 'GET `show` as json' do
+    let(:params) { { group_id: group.full_path, project_id: project.full_path, file_count: 15 } }
+
+    subject { get :show, format: :json, params: params }
+
+    it 'renders `forbidden` without proper license' do
+      stub_feature_flags(Gitlab::Analytics::CODE_ANALYTICS_FEATURE_FLAG => true)
+      stub_licensed_features(code_analytics: false)
+
+      subject
+
+      expect(response).to have_gitlab_http_status(:forbidden)
+    end
+
+    it 'renders `not_found` when feature flag is disabled' do
+      stub_licensed_features(code_analytics: true)
+      stub_feature_flags(Gitlab::Analytics::CODE_ANALYTICS_FEATURE_FLAG => false)
+
+      subject
+
+      expect(response).to have_gitlab_http_status(:not_found)
+    end
+
+    context 'when user has lower access than reporter' do
+      before do
+        stub_feature_flags(Gitlab::Analytics::CODE_ANALYTICS_FEATURE_FLAG => true)
+
+        GroupMember.where(user: current_user).delete_all
+        group.add_guest(current_user)
+      end
+
+      it 'renders `forbidden`' do
+        subject
+
+        expect(response).to have_gitlab_http_status(:forbidden)
+      end
+    end
+
+    context 'when valid parameters are given' do
+      let_it_be(:file_commit) { create(:analytics_repository_file_commit, committed_date: 2.days.ago, project: project) }
+
+      it { expect(response).to be_successful }
+
+      it 'renders files with commit count' do
+        subject
+
+        first_repository_file = json_response.first
+        expect(first_repository_file['name']).to eq(file_commit.analytics_repository_file.file_path)
+        expect(first_repository_file['count']).to eq(file_commit.commit_count)
+      end
+    end
+
+    context 'when invalid parameters are given' do
+      context 'when `file_count` is missing' do
+        before do
+          params.delete(:file_count)
+        end
+
+        it 'renders error response' do
+          subject
+
+          expect(json_response['errors']['file_count']).not_to be_empty
+        end
+      end
+
+      context 'when `file_count` is over the limit' do
+        before do
+          params[:file_count] = Analytics::CodeAnalytics::RepositoryFileCommit::MAX_FILE_COUNT + 1
+        end
+
+        it 'renders error response' do
+          subject
+
+          expect(json_response['errors']['file_count']).not_to be_empty
+        end
+      end
+    end
+  end
+end

ee/spec/finders/analytics/code_analytics_finder_spec.rb

@@ -22,14 +22,18 @@ describe Analytics::CodeAnalyticsFinder do
 
     subject { described_class.new(params).execute }
 
+    def find_file_count(result, file_path)
+      result.find { |r| r.repository_file.file_path.eql?(file_path) }
+    end
+
     context 'with no commits in the given date range' do
       before do
         params[:from] = 5.years.ago
         params[:to] = 4.years.ago
       end
 
-      it 'returns empty hash' do
-        expect(subject).to eq({})
+      it 'returns empty array' do
+        expect(subject).to eq([])
       end
     end
 
@@ -40,11 +44,11 @@ describe Analytics::CodeAnalyticsFinder do
       end
 
       it 'sums up the gemfile commits' do
-        expect(subject[gemfile.file_path]).to eq(3)
+        expect(find_file_count(subject, gemfile.file_path).count).to eq(3)
       end
 
       it 'includes the user model commit' do
-        expect(subject[user_model.file_path]).to eq(5)
+        expect(find_file_count(subject, user_model.file_path).count).to eq(5)
       end
 
       it 'verifies that the out of range record is persisted' do
@@ -53,11 +57,13 @@ describe Analytics::CodeAnalyticsFinder do
       end
 
       it 'does not include items outside of the date range' do
-        expect(subject).not_to have_key(app_controller.file_path)
+        expect(find_file_count(subject, app_controller.file_path)).to be_nil
       end
 
       it 'orders the results by commit count' do
-        expect(subject.keys).to eq([gemfile.file_path, user_model.file_path])
+        result_file_paths = subject.map { |item| item.repository_file.file_path }
+
+        expect(result_file_paths).to eq([gemfile.file_path, user_model.file_path])
       end
 
       context 'when `file_count` is given' do

ee/spec/lib/gitlab/analytics/code_analytics/request_params_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::Analytics::CodeAnalytics::RequestParams do
+  let(:params) { { file_count: 5 } }
+
+  subject { described_class.new(params) }
+
+  it 'is valid' do
+    expect(subject).to be_valid
+  end
+
+  context 'when `file_count` is invalid' do
+    before do
+      params[:file_count] = -1
+    end
+
+    it 'is invalid' do
+      expect(subject).not_to be_valid
+      expect(subject.errors[:file_count]).not_to be_empty
+    end
+  end
+end

ee/spec/models/analytics/code_analytics/repository_file_commit_spec.rb

@@ -22,7 +22,7 @@ describe Analytics::CodeAnalytics::RepositoryFileCommit do
       let!(:file_commit1) { create(:analytics_repository_file_commit, { project: project, analytics_repository_file: file, committed_date: 1.day.ago, commit_count: 2 }) }
       let!(:file_commit2) { create(:analytics_repository_file_commit, { project: project, analytics_repository_file: file, committed_date: 2.days.ago, commit_count: 2 }) }
 
-      it { expect(subject[file.file_path]).to eq(4) }
+      it { expect(subject[[file.id, file.file_path]]).to eq(4) }
     end
 
     context 'when the `file_count` is higher than allowed' do