Merge branch '196582-add-top-n-labels-endpoint-to-type-of-work' into 'master'

Top labels endpoint for type of work chart

See merge request gitlab-org/gitlab!24691

app/models/label.rb

@@ -42,6 +42,22 @@ class Label < ApplicationRecord
   scope :order_name_desc, -> { reorder(title: :desc) }
   scope :subscribed_by, ->(user_id) { joins(:subscriptions).where(subscriptions: { user_id: user_id, subscribed: true }) }
 
+  scope :top_labels_by_target, -> (target_relation) {
+    label_id_column = arel_table[:id]
+
+    # Window aggregation to count labels
+    count_by_id = Arel::Nodes::Over.new(
+      Arel::Nodes::NamedFunction.new('count', [label_id_column]),
+      Arel::Nodes::Window.new.partition(label_id_column)
+    ).as('count_by_id')
+
+    select(arel_table[Arel.star], count_by_id)
+      .joins(:label_links)
+      .merge(LabelLink.where(target: target_relation))
+      .reorder(count_by_id: :desc)
+      .distinct
+  }
+
   def self.prioritized(project)
     joins(:priorities)
       .where(label_priorities: { project_id: project })

ee/app/controllers/analytics/tasks_by_type_controller.rb

@@ -6,23 +6,31 @@ class Analytics::TasksByTypeController < Analytics::ApplicationController
   before_action :load_group
   before_action -> { check_feature_availability!(:type_of_work_analytics) }
   before_action -> { authorize_view_by_action!(:view_type_of_work_charts) }
-  before_action :validate_label_ids
+  before_action :validate_label_ids, only: :show
   before_action :prepare_date_range
 
   def show
     render json: Analytics::TasksByTypeLabelEntity.represent(counts_by_labels)
   end
 
+  def top_labels
+    render json: LabelEntity.represent(tasks_by_type.top_labels)
+  end
+
   private
 
   def counts_by_labels
+    tasks_by_type.counts_by_labels
+  end
+
+  def tasks_by_type
     Gitlab::Analytics::TypeOfWork::TasksByType.new(group: @group, current_user: current_user, params: {
       subject: params[:subject],
       label_ids: Array(params[:label_ids]),
       project_ids: Array(params[:project_ids]),
       created_after: @created_after.to_time.utc.beginning_of_day,
       created_before: @created_before.to_time.utc.end_of_day
-    }).counts_by_labels
+    })
   end
 
   def validate_label_ids

ee/config/routes/analytics.rb

@@ -22,7 +22,9 @@ namespace :analytics do
 
   constraints(::Constraints::FeatureConstrainer.new(Gitlab::Analytics::TASKS_BY_TYPE_CHART_FEATURE_FLAG)) do
     scope :type_of_work do
-      resource :tasks_by_type, controller: :tasks_by_type, only: :show
+      resource :tasks_by_type, controller: :tasks_by_type, only: :show do
+        get :top_labels
+      end
     end
   end
 end

ee/lib/gitlab/analytics/type_of_work/tasks_by_type.rb

@@ -11,6 +11,8 @@ module Gitlab
           Issue.to_s => IssuesFinder
         }.freeze
 
+        TOP_LABELS_COUNT = 10
+
         def initialize(group:, params:, current_user:)
           @group = group
           @params = params
@@ -21,12 +23,26 @@ module Gitlab
           format_result(query_result)
         end
 
+        # top N commonly used labels for Issues or MergeRequests ordered by usage
+        # rubocop: disable CodeReuse/ActiveRecord
+        def top_labels(limit = TOP_LABELS_COUNT)
+          targets = finder
+            .execute
+            .order_by(:id_desc)
+            .limit(100)
+
+          GroupLabel
+            .top_labels_by_target(targets)
+            .limit(limit)
+        end
+        # rubocop: enable CodeReuse/ActiveRecord
+
         private
 
         attr_reader :group, :params, :finder
 
         def finder_class
-          FINDER_CLASSES.fetch(params[:subject], FINDER_CLASSES.each_key.first)
+          FINDER_CLASSES.fetch(params[:subject], FINDER_CLASSES.each_value.first)
         end
 
         def format_result(result)

ee/spec/controllers/analytics/tasks_by_type_controller_spec.rb

@@ -19,97 +19,129 @@ describe Analytics::TasksByTypeController do
     sign_in(user)
   end
 
-  context 'when valid parameters are given' do
-    it 'succeeds' do
+  shared_examples 'expects unprocessable_entity response' do
+    it 'returns unprocessable_entity as response' do
       subject
 
-      expect(response).to be_successful
-      expect(response).to match_response_schema('analytics/tasks_by_type', dir: 'ee')
+      expect(response).to have_gitlab_http_status(:unprocessable_entity)
     end
+  end
 
-    it 'returns valid count' do
-      subject
+  shared_examples 'parameter validation' do
+    context 'when user access level is lower than reporter' do
+      before do
+        group.add_guest(user)
+      end
 
-      date, count = json_response.first["series"].first
-      expect(Date.parse(date)).to eq(issue.created_at.to_date)
-      expect(count).to eq(1)
-    end
-  end
+      it 'returns forbidden as response' do
+        subject
 
-  context 'when user access level is lower than reporter' do
-    before do
-      group.add_guest(user)
+        expect(response).to have_gitlab_http_status(:forbidden)
+      end
     end
 
-    it do
-      subject
+    context 'when license is missing' do
+      before do
+        stub_licensed_features(type_of_work_analytics: false)
+      end
 
-      expect(response).to have_gitlab_http_status(:forbidden)
-    end
-  end
+      it 'returns forbidden as response' do
+        subject
 
-  context 'when license is missing' do
-    before do
-      stub_licensed_features(type_of_work_analytics: false)
+        expect(response).to have_gitlab_http_status(:forbidden)
+      end
     end
 
-    it 'returns forbidden as response' do
-      subject
+    context 'when feature flag is disabled' do
+      before do
+        stub_feature_flags(Gitlab::Analytics::TASKS_BY_TYPE_CHART_FEATURE_FLAG => false)
+      end
+
+      it 'returns not_found as response' do
+        subject
 
-      expect(response).to have_gitlab_http_status(:forbidden)
+        expect(response).to have_gitlab_http_status(:not_found)
+      end
     end
-  end
 
-  context 'when feature flag is disabled' do
-    before do
-      stub_feature_flags(Gitlab::Analytics::TASKS_BY_TYPE_CHART_FEATURE_FLAG => false)
+    context 'when `created_after` parameter is invalid' do
+      before do
+        params[:created_after] = 'invalid_date'
+      end
+
+      it_behaves_like 'expects unprocessable_entity response'
     end
 
-    it 'returns not_found as response' do
-      subject
+    context 'when `created_after` parameter is missing' do
+      before do
+        params.delete(:created_after)
+      end
 
-      expect(response).to have_gitlab_http_status(:not_found)
+      it_behaves_like 'expects unprocessable_entity response'
     end
-  end
 
-  shared_examples 'expects unprocessable_entity response' do
-    it 'returns unprocessable_entity as resposne' do
-      subject
+    context 'when `created_after` date is later than `created_before` date' do
+      before do
+        params[:created_after] = 1.year.ago.to_date
+        params[:created_before] = 2.years.ago.to_date
+      end
 
-      expect(response).to have_gitlab_http_status(:unprocessable_entity)
+      it_behaves_like 'expects unprocessable_entity response'
     end
   end
 
-  context 'when `label_id` is missing' do
-    before do
-      params.delete(:label_ids)
-    end
+  describe 'GET show' do
+    context 'when valid parameters are given' do
+      it 'succeeds' do
+        subject
 
-    it_behaves_like 'expects unprocessable_entity response'
-  end
+        expect(response).to be_successful
+        expect(response).to match_response_schema('analytics/tasks_by_type', dir: 'ee')
+      end
+
+      it 'returns valid count' do
+        subject
 
-  context 'when `created_after` parameter is invalid' do
-    before do
-      params[:created_after] = 'invalid_date'
+        date, count = json_response.first["series"].first
+        expect(Date.parse(date)).to eq(issue.created_at.to_date)
+        expect(count).to eq(1)
+      end
     end
 
-    it_behaves_like 'expects unprocessable_entity response'
-  end
+    context 'when `label_id` is missing' do
+      before do
+        params.delete(:label_ids)
+      end
 
-  context 'when `created_after` parameter is missing' do
-    before do
-      params.delete(:created_after)
+      it_behaves_like 'expects unprocessable_entity response'
     end
 
-    it_behaves_like 'expects unprocessable_entity response'
+    it_behaves_like 'parameter validation'
   end
 
-  context 'when `created_after` date is later than "created_before" date' do
-    before do
-      params[:created_after] = 1.year.ago.to_date
-      params[:created_before] = 2.years.ago.to_date
+  describe 'GET top_labels' do
+    let(:params) { { group_id: group.full_path, created_after: 10.days.ago, subject: 'Issue' } }
+
+    subject { get :top_labels, params: params }
+
+    context 'when valid parameters are given' do
+      it 'succeeds' do
+        subject
+
+        expect(response).to be_successful
+        expect(response).to match_response_schema('analytics/tasks_by_type_top_labels', dir: 'ee')
+      end
+
+      it 'returns valid count' do
+        subject
+
+        label_item = json_response.first
+
+        expect(label_item['title']).to eq(label.title)
+        expect(json_response.count).to eq(1)
+      end
     end
 
-    it_behaves_like 'expects unprocessable_entity response'
+    it_behaves_like 'parameter validation'
   end
 end

ee/spec/fixtures/api/schemas/analytics/tasks_by_type_top_labels.json

+{
+  "type": "array",
+  "items": {
+    "type": "object" ,
+    "required": ["id", "title", "color", "text_color"],
+    "properties": {
+      "label": {
+        "type": "object",
+        "properties": {
+          "id": {
+            "type": "integer"
+          },
+          "title": {
+            "type": "string"
+          },
+          "color": {
+            "type": "string"
+          },
+          "text_color": {
+            "type": "string"
+          }
+        }
+      }
+    }
+  }
+}

ee/spec/lib/gitlab/analytics/type_of_work/tasks_by_type_spec.rb

@@ -8,6 +8,8 @@ describe Gitlab::Analytics::TypeOfWork::TasksByType do
   let_it_be(:other_group) { create(:group) }
   let_it_be(:subgroup) { create(:group, parent: group) }
   let_it_be(:label) { create(:group_label, group: group) }
+  let_it_be(:other_label) { create(:group_label, group: group) }
+  let_it_be(:not_used_label) { create(:group_label, group: group) }
   let_it_be(:label_for_subgroup) { create(:group_label, group: group) }
   let_it_be(:other_label) { create(:group_label, group: other_group) }
   let_it_be(:project) { create(:project, group: group) }
@@ -118,6 +120,34 @@ describe Gitlab::Analytics::TypeOfWork::TasksByType do
     end
   end
 
+  shared_examples '#top_labels' do
+    let(:top_labels) { described_class.new(params).top_labels }
+
+    let!(:with_label) do
+      create(factory_name, {
+        :created_at => 3.days.ago,
+        :labels => [label, other_label],
+        project_attribute_name => project
+      })
+    end
+
+    let!(:with_other_label_only) do
+      create(factory_name, {
+        :created_at => 3.days.ago,
+        :labels => [other_label],
+        project_attribute_name => create(:project, group: group)
+      })
+    end
+
+    it 'sorts by descending order' do
+      expect(top_labels).to eq([other_label, label])
+    end
+
+    it 'limits the the size of the results' do
+      expect(described_class.new(params).top_labels(1)).to eq([other_label])
+    end
+  end
+
   context 'when subject is `Issue`' do
     let(:factory_name) { :labeled_issue }
     let(:project_attribute_name) { :project }
@@ -126,7 +156,8 @@ describe Gitlab::Analytics::TypeOfWork::TasksByType do
       params[:params][:subject] = Issue.to_s
     end
 
-    include_examples '#counts_by_labels'
+    it_behaves_like '#counts_by_labels'
+    it_behaves_like '#top_labels'
   end
 
   context 'when subject is `MergeRequest`' do
@@ -137,6 +168,23 @@ describe Gitlab::Analytics::TypeOfWork::TasksByType do
       params[:params][:subject] = MergeRequest.to_s
     end
 
-    include_examples '#counts_by_labels'
+    it_behaves_like '#counts_by_labels'
+    it_behaves_like '#top_labels'
+  end
+
+  context 'when unknown `subject` is given' do
+    before do
+      params[:params][:subject] = 'invalid'
+
+      create(:merge_request, {
+        created_at: 3.days.ago,
+        labels: [label],
+        source_project: project
+      })
+    end
+
+    it 'falls back to `MergeRequestFinder`' do
+      expect(subject.map(&:label)).to eq([label])
+    end
   end
 end

spec/controllers/projects/deployments_controller_spec.rb

@@ -59,7 +59,7 @@ describe Projects::DeploymentsController do
           end
         end
 
-        it 'returns a empty response 204 resposne' do
+        it 'returns an empty 204 response' do
           get :metrics, params: deployment_params(id: deployment.to_param)
           expect(response).to have_gitlab_http_status(:no_content)
           expect(response.body).to eq('')

spec/models/label_spec.rb

@@ -183,6 +183,31 @@ describe Label do
     end
   end
 
+  describe '.top_labels_by_target' do
+    let(:label) { create(:label) }
+    let(:popular_label) { create(:label) }
+    let(:merge_request1) { create(:merge_request) }
+    let(:merge_request2) { create(:merge_request) }
+
+    before do
+      merge_request1.labels = [label, popular_label]
+      merge_request2.labels = [popular_label]
+    end
+
+    it 'returns distinct labels, ordered by usage in the given target relation' do
+      top_labels = described_class.top_labels_by_target(MergeRequest.all)
+
+      expect(top_labels).to match_array([popular_label, label])
+    end
+
+    it 'excludes labels that are not assigned to any records in the given target relation' do
+      merge_requests = MergeRequest.where(id: merge_request2.id)
+      top_labels = described_class.top_labels_by_target(merge_requests)
+
+      expect(top_labels).to match_array([popular_label])
+    end
+  end
+
   describe '.optionally_subscribed_by' do
     let!(:user)   { create(:user) }
     let!(:label)  { create(:label) }