Merge branch '321392-total-epic-weight' into 'master'

Expose total weight for epic board lists on GraphQL endpoint

See merge request gitlab-org/gitlab!76440

app/services/boards/base_items_list_service.rb

@@ -12,21 +12,32 @@ module Boards
     end
 
     # rubocop: disable CodeReuse/ActiveRecord
-    def metadata
-      issuables = item_model.arel_table
-      keys = metadata_fields.keys
+    def metadata(required_fields = [:issue_count, :total_issue_weight])
+      fields = metadata_fields(required_fields)
+      keys = fields.keys
       # TODO: eliminate need for SQL literal fragment
-      columns = Arel.sql(metadata_fields.values_at(*keys).join(', '))
-      results = item_model.where(id: init_collection.select(issuables[:id])).pluck(columns)
+      columns = Arel.sql(fields.values_at(*keys).join(', '))
+      results = item_model.where(id: collection_ids)
+      results = query_additions(results, required_fields)
+      results = results.select(columns)
 
-      Hash[keys.zip(results.flatten)]
+      Hash[keys.zip(results.pluck(columns).flatten)]
     end
     # rubocop: enable CodeReuse/ActiveRecord
 
     private
 
-    def metadata_fields
-      { size: 'COUNT(*)' }
+    # override if needed
+    def query_additions(items, required_fields)
+      items
+    end
+
+    def collection_ids
+      @collection_ids ||= init_collection.select(item_model.arel_table[:id])
+    end
+
+    def metadata_fields(required_fields)
+      required_fields&.include?(:issue_count) ? { size: 'COUNT(*)' } : {}
     end
 
     def order(items)

db/migrate/20220124200927_add_index_to_issues.rb

+# frozen_string_literal: true
+
+class AddIndexToIssues < Gitlab::Database::Migration[1.0]
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  INDEX_NAME = 'index_issues_on_id_and_weight'
+
+  def up
+    add_concurrent_index :issues, [:id, :weight], name: INDEX_NAME
+  end
+
+  def down
+    remove_concurrent_index_by_name :issues, INDEX_NAME
+  end
+end

db/schema_migrations/20220124200927

+688232dde01ea4e8574dca73459094264bde405d799ecaf1a5867adb72576b98
\ No newline at end of file

db/structure.sql

@@ -27884,6 +27884,8 @@ CREATE INDEX index_issues_on_description_trigram ON issues USING gin (descriptio
 
 CREATE INDEX index_issues_on_duplicated_to_id ON issues USING btree (duplicated_to_id) WHERE (duplicated_to_id IS NOT NULL);
 
+CREATE INDEX index_issues_on_id_and_weight ON issues USING btree (id, weight);
+
 CREATE INDEX index_issues_on_incident_issue_type ON issues USING btree (issue_type) WHERE (issue_type = 1);
 
 CREATE INDEX index_issues_on_last_edited_by_id ON issues USING btree (last_edited_by_id);

doc/api/graphql/reference/index.md

@@ -10759,10 +10759,11 @@ Represents an epic board list.
 | Name | Type | Description |
 | ---- | ---- | ----------- |
 | <a id="epiclistcollapsed"></a>`collapsed` | [`Boolean`](#boolean) | Indicates if this list is collapsed for this user. |
-| <a id="epiclistepicscount"></a>`epicsCount` | [`Int`](#int) | Count of epics in the list. |
+| <a id="epiclistepicscount"></a>`epicsCount` **{warning-solid}** | [`Int`](#int) | **Deprecated** in 14.9. This was renamed. Use: `metadata`. |
 | <a id="epiclistid"></a>`id` | [`BoardsEpicListID!`](#boardsepiclistid) | Global ID of the board list. |
 | <a id="epiclistlabel"></a>`label` | [`Label`](#label) | Label of the list. |
 | <a id="epiclistlisttype"></a>`listType` | [`String!`](#string) | Type of the list. |
+| <a id="epiclistmetadata"></a>`metadata` | [`EpicListMetadata`](#epiclistmetadata) | Epic list metatada. |
 | <a id="epiclistposition"></a>`position` | [`Int`](#int) | Position of the list within the board. |
 | <a id="epiclisttitle"></a>`title` | [`String!`](#string) | Title of the list. |
 
@@ -10784,6 +10785,17 @@ four standard [pagination arguments](#connection-pagination-arguments):
 | ---- | ---- | ----------- |
 | <a id="epiclistepicsfilters"></a>`filters` | [`EpicFilters`](#epicfilters) | Filters applied when selecting epics in the board list. |
 
+### `EpicListMetadata`
+
+Represents epic board list metadata.
+
+#### Fields
+
+| Name | Type | Description |
+| ---- | ---- | ----------- |
+| <a id="epiclistmetadataepicscount"></a>`epicsCount` | [`Int`](#int) | Count of epics in the list. |
+| <a id="epiclistmetadatatotalweight"></a>`totalWeight` | [`Int`](#int) | Total weight of all issues in the list. Available only when feature flag `epic_board_total_weight` is enabled. This flag is disabled by default, because the feature is experimental and is subject to change without notice. |
+
 ### `EpicPermissions`
 
 Check permissions for the current user on an epic.

ee/app/graphql/types/boards/epic_list_metadata_type.rb

+# frozen_string_literal: true
+
+module Types
+  module Boards
+    # rubocop: disable Graphql/AuthorizeTypes
+    # the board is authorized in `EpicListType`
+
+    class EpicListMetadataType < BaseObject
+      graphql_name 'EpicListMetadata'
+      description 'Represents epic board list metadata'
+
+      field :epics_count, GraphQL::Types::Int, null: true,
+            description: 'Count of epics in the list.'
+
+      field :total_weight, GraphQL::Types::Int, null: true,
+            description: 'Total weight of all issues in the list.',
+            feature_flag: :epic_board_total_weight
+    end
+    # rubocop: enable Graphql/AuthorizeTypes
+  end
+end

ee/app/graphql/types/boards/epic_list_type.rb

@@ -36,24 +36,35 @@ module Types
             description: 'List epics.'
 
       field :epics_count, GraphQL::Types::Int, null: true,
-            description: 'Count of epics in the list.'
+            description: 'Count of epics in the list.',
+            deprecated: { reason: :renamed, replacement: 'metadata', milestone: '14.9' }
+
+      field :metadata, Types::Boards::EpicListMetadataType, null: true,
+          description: 'Epic list metatada.',
+          extras: [:lookahead]
 
       def collapsed
         object.collapsed?(current_user)
       end
 
       def epics_count
-        metadata[:size]
+        list_service.metadata([:epics_count])[:epics_count]
       end
 
-      def metadata
-        strong_memoize(:metadata) do
-          params = (context[:epic_filters] || {}).merge(board_id: list.epic_board_id, id: list.id)
+      def metadata(lookahead: nil)
+        required_metadata = []
+        required_metadata << :epics_count if lookahead&.selects?(:epics_count)
+        required_metadata << :total_weight if lookahead&.selects?(:total_weight)
+
+        list_service.metadata(required_metadata)
+      end
+
+      def list_service
+        ::Boards::Epics::ListService.new(list.epic_board.resource_parent, current_user, params)
+      end
 
-          ::Boards::Epics::ListService
-              .new(list.epic_board.resource_parent, current_user, params)
-              .metadata
-        end
+      def params
+        (context[:epic_filters] || {}).merge(board_id: list.epic_board_id, id: list.id)
       end
     end
     # rubocop: enable Graphql/AuthorizeTypes

ee/app/services/boards/epics/list_service.rb

@@ -16,6 +16,20 @@ module Boards
         super(items)
       end
 
+      def query_additions(items, required_fields)
+        return items unless required_fields&.include?(:total_weight)
+
+        items.left_joins(epic_issues: :issue)
+      end
+
+      def metadata_fields(required_fields)
+        fields = super
+
+        fields[:total_weight] = 'SUM(weight)' if required_fields&.include?(:total_weight)
+        fields[:epics_count] = 'COUNT(distinct epics.id)' if required_fields&.include?(:epics_count)
+        fields
+      end
+
       def filter_by_from_id(items)
         return items unless params[:from_id].present?
 

ee/app/services/ee/boards/issues/list_service.rb

@@ -87,8 +87,12 @@ module EE
         # rubocop: enable CodeReuse/ActiveRecord
 
         override :metadata_fields
-        def metadata_fields
-          super.merge(total_weight: 'COALESCE(SUM(weight), 0)')
+        def metadata_fields(required_fields)
+          fields = super
+
+          fields[:total_weight] = 'COALESCE(SUM(weight), 0)' if required_fields.include?(:total_issue_weight)
+
+          fields
         end
 
         # rubocop: disable CodeReuse/ActiveRecord

ee/config/feature_flags/development/epic_board_total_weight.yml

+---
+name: epic_board_total_weight
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/76440
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/353450
+milestone: '14.7'
+type: development
+group: group::product planning
+default_enabled: false

ee/spec/graphql/types/boards/epic_list_metadata_type_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe GitlabSchema.types['EpicListMetadata'] do
+  specify { expect(described_class.graphql_name).to eq('EpicListMetadata') }
+
+  it 'has specific fields' do
+    expected_fields = %w[epics_count total_weight]
+
+    expect(described_class).to include_graphql_fields(*expected_fields)
+  end
+end

ee/spec/graphql/types/boards/epic_list_type_spec.rb

@@ -6,7 +6,7 @@ RSpec.describe GitlabSchema.types['EpicList'] do
   specify { expect(described_class.graphql_name).to eq('EpicList') }
 
   it 'has specific fields' do
-    expected_fields = %w[id title list_type position label epics epics_count collapsed]
+    expected_fields = %w[id title list_type position label epics epics_count collapsed metadata]
 
     expect(described_class).to include_graphql_fields(*expected_fields)
   end

ee/spec/requests/api/graphql/boards/epic_lists_query_spec.rb

@@ -7,16 +7,19 @@ RSpec.describe 'get list of epic boards' do
 
   let_it_be(:current_user) { create(:user) }
   let_it_be(:group) { create(:group, :private) }
+  let_it_be(:project) { create(:project, group: group) }
   let_it_be(:board) { create(:epic_board, group: group) }
   let_it_be(:list1) { create(:epic_list, epic_board: board) }
   let_it_be(:list2) { create(:epic_list, epic_board: board, list_type: :closed) }
   let_it_be(:list3) { create(:epic_list, epic_board: board, list_type: :backlog) }
 
+  let(:fields) { all_graphql_fields_for('epic_lists'.classify) }
+
   def pagination_query(params = {})
     graphql_query_for(:group, { full_path: group.full_path },
       <<~BOARDS
         epicBoard(id: "#{board.to_global_id}") {
-          #{query_nodes(:lists, all_graphql_fields_for('epic_lists'.classify), include_pagination_info: true, args: params)}
+          #{query_nodes(:lists, fields, include_pagination_info: true, args: params)}
         }
       BOARDS
     )
@@ -79,24 +82,47 @@ RSpec.describe 'get list of epic boards' do
         assert_field_value('collapsed', [false, true, false])
       end
 
-      it 'returns the correct values for count' do
+      it 'returns the correct metadata values' do
         label = create(:group_label, group: group)
         # Epics in backlog, the list which is returned first. The first epic
         # should be ignored because it doesn't have the label by which we are
         # filtering.
         create(:labeled_epic, group: group)
-        create(:labeled_epic, group: group, labels: [label])
         create(:labeled_epic, group: group, labels: [label], confidential: true)
+        epic_with_issue = create(:labeled_epic, group: group, labels: [label])
+
+        create(:issue, project: project, epic: epic_with_issue, weight: 3)
+        create(:issue, project: project, epic: epic_with_issue, weight: 4)
 
         params = { epicFilters: { labelName: label.title, confidential: false } }
         post_graphql(pagination_query(params), current_user: current_user)
 
         assert_field_value('epicsCount', [1, 0, 0])
+        expected_metadata = [
+          { 'epicsCount' => 1, 'totalWeight' => 7 },
+          { 'epicsCount' => 0, 'totalWeight' => nil },
+          { 'epicsCount' => 0, 'totalWeight' => nil }
+        ]
+        assert_field_value('metadata', expected_metadata)
+      end
+
+      context 'when totalWeight not requested' do
+        let(:fields) { "metadata { epicsCount }" }
+
+        it 'does not required the value from the service' do
+          post_graphql(pagination_query, current_user: current_user)
+
+          expect(dig_data('metadata').first.keys).to match_array(['epicsCount'])
+        end
       end
     end
   end
 
   def assert_field_value(field, expected_value)
-    expect(graphql_dig_at(graphql_data, 'group', 'epicBoard', 'lists', 'nodes', field)).to eq(expected_value)
+    expect(dig_data(field)).to eq(expected_value)
+  end
+
+  def dig_data(field)
+    graphql_dig_at(graphql_data, 'group', 'epicBoard', 'lists', 'nodes', field)
   end
 end

ee/spec/services/boards/epics/list_service_spec.rb

@@ -3,34 +3,34 @@
 require 'spec_helper'
 
 RSpec.describe Boards::Epics::ListService do
-  describe '#execute' do
-    let_it_be(:user) { create(:user) }
-    let_it_be(:group) { create(:group) }
-    let_it_be(:board) { create(:epic_board, group: group) }
+  let_it_be(:user) { create(:user) }
+  let_it_be(:group) { create(:group) }
+  let_it_be(:board) { create(:epic_board, group: group) }
 
-    let_it_be(:development) { create(:group_label, group: group, name: 'Development') }
-    let_it_be(:testing) { create(:group_label, group: group, name: 'Testing') }
+  let_it_be(:development) { create(:group_label, group: group, name: 'Development') }
+  let_it_be(:testing) { create(:group_label, group: group, name: 'Testing') }
 
-    let_it_be(:backlog) { create(:epic_list, epic_board: board, list_type: :backlog) }
-    let_it_be(:list1) { create(:epic_list, epic_board: board, label: development, position: 0) }
-    let_it_be(:list2) { create(:epic_list, epic_board: board, label: testing, position: 1) }
-    let_it_be(:closed) { create(:epic_list, epic_board: board, list_type: :closed) }
+  let_it_be(:backlog) { create(:epic_list, epic_board: board, list_type: :backlog) }
+  let_it_be(:list1) { create(:epic_list, epic_board: board, label: development, position: 0) }
+  let_it_be(:list2) { create(:epic_list, epic_board: board, label: testing, position: 1) }
+  let_it_be(:closed) { create(:epic_list, epic_board: board, list_type: :closed) }
 
-    let_it_be(:backlog_epic1) { create(:epic, group: group) }
-    let_it_be(:list1_epic1) { create(:labeled_epic, group: group, labels: [development]) }
-    let_it_be(:list1_epic2) { create(:labeled_epic, group: group, labels: [development]) }
-    let_it_be(:list1_epic3) { create(:labeled_epic, group: group, labels: [development]) }
-    let_it_be(:list2_epic1) { create(:labeled_epic, group: group, labels: [testing]) }
+  let_it_be(:backlog_epic1) { create(:epic, group: group) }
+  let_it_be(:list1_epic1) { create(:labeled_epic, group: group, labels: [development]) }
+  let_it_be(:list1_epic2) { create(:labeled_epic, group: group, labels: [development]) }
+  let_it_be(:list1_epic3) { create(:labeled_epic, group: group, labels: [development]) }
+  let_it_be(:list2_epic1) { create(:labeled_epic, group: group, labels: [testing]) }
 
-    let_it_be(:closed_epic1) { create(:labeled_epic, :closed, group: group, labels: [development], closed_at: 1.day.ago) }
-    let_it_be(:closed_epic2) { create(:labeled_epic, :closed, group: group, labels: [testing], closed_at: 2.days.ago) }
-    let_it_be(:closed_epic3) { create(:epic, :closed, group: group, closed_at: 1.week.ago) }
+  let_it_be(:closed_epic1) { create(:labeled_epic, :closed, group: group, labels: [development], closed_at: 1.day.ago) }
+  let_it_be(:closed_epic2) { create(:labeled_epic, :closed, group: group, labels: [testing], closed_at: 2.days.ago) }
+  let_it_be(:closed_epic3) { create(:epic, :closed, group: group, closed_at: 1.week.ago) }
 
-    before do
-      stub_licensed_features(epics: true)
-      group.add_developer(user)
-    end
+  before do
+    stub_licensed_features(epics: true)
+    group.add_developer(user)
+  end
 
+  describe '#execute' do
     it_behaves_like 'items list service' do
       let(:parent) { group }
       let(:backlog_items) { [backlog_epic1] }
@@ -59,4 +59,48 @@ RSpec.describe Boards::Epics::ListService do
         .execute
     end
   end
+
+  describe '#metadata' do
+    before do
+      project = create(:project, group: group)
+
+      create(:epic_issue, epic: list1_epic1, issue: create(:issue, project: project, weight: 2))
+      create(:epic_issue, epic: list1_epic2, issue: create(:issue, project: project, weight: 3))
+      create(:epic_issue, epic: list1_epic2, issue: create(:issue, project: project, weight: 2))
+    end
+
+    subject { described_class.new(group, user, { board_id: board.id, id: list1.id }).metadata(fields) }
+
+    context 'with all fields included in the required_fields' do
+      let(:fields) { [:total_weight, :epics_count] }
+
+      it 'containes correct data including weight' do
+        expect(subject).to eq({ total_weight: 7, epics_count: 3 })
+      end
+    end
+
+    context 'with total_weight not included in the required_fields' do
+      let(:fields) { [:epics_count] }
+
+      it 'containes correct data without weight' do
+        expect(subject).to eq({ epics_count: 3 })
+      end
+    end
+
+    context 'with epics_countr not included in the required_fields' do
+      let(:fields) { [:total_weight] }
+
+      it 'containes correct data without weight' do
+        expect(subject).to eq({ total_weight: 7 })
+      end
+    end
+
+    context 'with required_fields set to nil' do
+      let(:fields) { nil }
+
+      it 'does not contain any data' do
+        expect(subject).to eq({})
+      end
+    end
+  end
 end

ee/spec/services/ee/boards/issues/list_service_spec.rb

@@ -132,6 +132,26 @@ RSpec.describe Boards::Issues::ListService, services: true do
           expect(metadata[:size]).to eq(0)
           expect(metadata[:total_weight]).to eq(0)
         end
+
+        context 'when total_issue_weight is not included in the required_fields' do
+          it 'returns only issue count' do
+            params = { board_id: board.id, id: backlog.id }
+
+            metadata = described_class.new(parent, user, params).metadata([:issue_count])
+
+            expect(metadata).to eq(size: 3)
+          end
+        end
+
+        context 'when required_fields array is empty' do
+          it 'returns empty hash' do
+            params = { board_id: board.id, id: backlog.id }
+
+            metadata = described_class.new(parent, user, params).metadata([])
+
+            expect(metadata).to eq({})
+          end
+        end
       end
 
       context 'when list_id is missing' do