From 47dc3f4335c4a4949c280d8adb1ac5190ce5eb6a Mon Sep 17 00:00:00 2001 From: charlieablett Date: Wed, 30 Oct 2019 19:44:46 +1300 Subject: [PATCH] Expose epic weights sum [skip-ci] - add DescendentService abstract class - add issue weights sum scope - add descendent weight service - Refactor common spec shared examples into separate file - Add weight filter to scope (exclude nil or 0 weight issues) - Remove group filter to include all issues --- .../graphql/reference/gitlab_schema.graphql | 17 ++++++ doc/api/graphql/reference/gitlab_schema.json | 55 +++++++++++++++++++ doc/api/graphql/reference/index.md | 8 +++ .../types/epic_descendant_weight_sum_type.rb | 14 +++++ ee/app/graphql/types/epic_type.rb | 6 ++ ee/app/models/ee/issue.rb | 2 + .../epics/descendant_count_service.rb | 20 +------ ee/app/services/epics/descendant_service.rb | 22 ++++++++ .../epics/descendant_weight_service.rb | 21 +++++++ ...gress-information-in-roadmap-epic-bars.yml | 5 ++ ee/spec/graphql/types/epic_type_spec.rb | 2 +- ee/spec/models/issue_spec.rb | 33 +++++++++++ .../epics/descendant_count_service_spec.rb | 48 ++-------------- .../epics/descendant_weight_service_spec.rb | 13 +++++ .../services/descendants_service_examples.rb | 43 +++++++++++++++ 15 files changed, 245 insertions(+), 64 deletions(-) create mode 100644 ee/app/graphql/types/epic_descendant_weight_sum_type.rb create mode 100644 ee/app/services/epics/descendant_service.rb create mode 100644 ee/app/services/epics/descendant_weight_service.rb create mode 100644 ee/changelogs/unreleased/5164-weight-and-progress-information-in-roadmap-epic-bars.yml create mode 100644 ee/spec/services/epics/descendant_weight_service_spec.rb create mode 100644 ee/spec/support/shared_examples/services/descendants_service_examples.rb diff --git a/doc/api/graphql/reference/gitlab_schema.graphql b/doc/api/graphql/reference/gitlab_schema.graphql index 816512c83bbd63..26f9fb28e8a3b9 100644 --- a/doc/api/graphql/reference/gitlab_schema.graphql +++ b/doc/api/graphql/reference/gitlab_schema.graphql @@ -1862,6 +1862,11 @@ type Epic implements Noteable { """ descendantCounts: EpicDescendantCount + """ + Total weight of open and closed descendant epic's issues + """ + descendantWeightSum: EpicDescendantWeights + """ Description of the epic """ @@ -2178,6 +2183,18 @@ type EpicDescendantCount { openedIssues: Int } +type EpicDescendantWeights { + """ + Total weight of completed (closed) issues in this epic, including epic descendants + """ + closedIssues: Int + + """ + Total weight of opened issues in this epic, including epic descendants + """ + openedIssues: Int +} + """ An edge in a connection. """ diff --git a/doc/api/graphql/reference/gitlab_schema.json b/doc/api/graphql/reference/gitlab_schema.json index 2053bdb940413d..18e8872b02da69 100644 --- a/doc/api/graphql/reference/gitlab_schema.json +++ b/doc/api/graphql/reference/gitlab_schema.json @@ -4889,6 +4889,20 @@ "isDeprecated": false, "deprecationReason": null }, + { + "name": "descendantWeightSum", + "description": "Total weight of open and closed descendant epic's issues", + "args": [ + + ], + "type": { + "kind": "OBJECT", + "name": "EpicDescendantWeights", + "ofType": null + }, + "isDeprecated": false, + "deprecationReason": null + }, { "name": "description", "description": "Description of the epic", @@ -12990,6 +13004,47 @@ "enumValues": null, "possibleTypes": null }, + { + "kind": "OBJECT", + "name": "EpicDescendantWeights", + "description": null, + "fields": [ + { + "name": "closedIssues", + "description": "Total weight of completed (closed) issues in this epic, including epic descendants", + "args": [ + + ], + "type": { + "kind": "SCALAR", + "name": "Int", + "ofType": null + }, + "isDeprecated": false, + "deprecationReason": null + }, + { + "name": "openedIssues", + "description": "Total weight of opened issues in this epic, including epic descendants", + "args": [ + + ], + "type": { + "kind": "SCALAR", + "name": "Int", + "ofType": null + }, + "isDeprecated": false, + "deprecationReason": null + } + ], + "inputFields": null, + "interfaces": [ + + ], + "enumValues": null, + "possibleTypes": null + }, { "kind": "OBJECT", "name": "TimelogConnection", diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 978fbc35125e77..09a914d9ff5e39 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -294,6 +294,7 @@ Represents an epic. | `closedAt` | Time | Timestamp of the epic's closure | | `createdAt` | Time | Timestamp of the epic's creation | | `descendantCounts` | EpicDescendantCount | Number of open and closed descendant epics and issues | +| `descendantWeightSum` | EpicDescendantWeights | Total weight of open and closed descendant epic's issues | | `description` | String | Description of the epic | | `downvotes` | Int! | Number of downvotes the epic has received | | `dueDate` | Time | Due date of the epic | @@ -334,6 +335,13 @@ Counts of descendent epics. | `openedEpics` | Int | Number of opened sub-epics | | `openedIssues` | Int | Number of opened epic issues | +## EpicDescendantWeights + +| Name | Type | Description | +| --- | ---- | ---------- | +| `closedIssues` | Int | Total weight of completed (closed) issues in this epic, including epic descendants | +| `openedIssues` | Int | Total weight of opened issues in this epic, including epic descendants | + ## EpicIssue Relationship between an epic and an issue diff --git a/ee/app/graphql/types/epic_descendant_weight_sum_type.rb b/ee/app/graphql/types/epic_descendant_weight_sum_type.rb new file mode 100644 index 00000000000000..722813eaa04d7a --- /dev/null +++ b/ee/app/graphql/types/epic_descendant_weight_sum_type.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +module Types + # rubocop: disable Graphql/AuthorizeTypes + class EpicDescendantWeightSumType < BaseObject + graphql_name 'EpicDescendantWeights' + + field :opened_issues, GraphQL::INT_TYPE, null: true, + description: 'Total weight of opened issues in this epic, including epic descendants' + field :closed_issues, GraphQL::INT_TYPE, null: true, + description: 'Total weight of completed (closed) issues in this epic, including epic descendants' + end + # rubocop: enable Graphql/AuthorizeTypes +end diff --git a/ee/app/graphql/types/epic_type.rb b/ee/app/graphql/types/epic_type.rb index 74b4d60a9daec8..19bcb254c807b7 100644 --- a/ee/app/graphql/types/epic_type.rb +++ b/ee/app/graphql/types/epic_type.rb @@ -126,6 +126,12 @@ class EpicType < BaseObject Epics::DescendantCountService.new(epic, ctx[:current_user]) end + field :descendant_weight_sum, Types::EpicDescendantWeightSumType, null: true, complexity: 10, + description: "Total weight of open and closed descendant epic's issues", + resolve: -> (epic, args, ctx) do + Epics::DescendantWeightService.new(epic, ctx[:current_user]) + end + field :health_status, ::Types::HealthStatusEnum, null: true, diff --git a/ee/app/models/ee/issue.rb b/ee/app/models/ee/issue.rb index 322e0e5a182d26..3b55df5861942c 100644 --- a/ee/app/models/ee/issue.rb +++ b/ee/app/models/ee/issue.rb @@ -21,6 +21,8 @@ module Issue scope :order_weight_asc, -> { reorder ::Gitlab::Database.nulls_last_order('weight') } scope :order_created_at_desc, -> { reorder(created_at: :desc) } scope :service_desk, -> { where(author: ::User.support_bot) } + scope :weight_total_by_state, -> { reorder(nil).where('weight IS NOT NULL AND weight > 0').group(:state_id).sum(:weight) } + scope :no_epic, -> { left_outer_joins(:epic_issue).where(epic_issues: { epic_id: nil }) } scope :in_epics, ->(epics) do issue_ids = EpicIssue.where(epic_id: epics).select(:issue_id) diff --git a/ee/app/services/epics/descendant_count_service.rb b/ee/app/services/epics/descendant_count_service.rb index 90f213d32ee6b4..9c7db9f4d61bc8 100644 --- a/ee/app/services/epics/descendant_count_service.rb +++ b/ee/app/services/epics/descendant_count_service.rb @@ -1,16 +1,7 @@ # frozen_string_literal: true module Epics - class DescendantCountService - include Gitlab::Utils::StrongMemoize - - attr_reader :epic, :current_user - - def initialize(epic, current_user) - @epic = epic - @current_user = current_user - end - + class DescendantCountService < DescendantService def opened_epics epics_count.fetch(Epic.state_ids[:opened], 0) end @@ -44,14 +35,5 @@ def issues_count IssuesFinder.new(current_user).execute.in_epics(accessible_epics).counts_by_state end end - - def accessible_epics - strong_memoize(:epics) do - epics = epic.base_and_descendants - epic_groups = Group.for_epics(epics) - groups = Group.groups_user_can_read_epics(epic_groups, current_user, same_root: true) - epics.in_selected_groups(groups) - end - end end end diff --git a/ee/app/services/epics/descendant_service.rb b/ee/app/services/epics/descendant_service.rb new file mode 100644 index 00000000000000..1af14276cc0809 --- /dev/null +++ b/ee/app/services/epics/descendant_service.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +module Epics + class DescendantService + include Gitlab::Utils::StrongMemoize + + def initialize(epic, current_user) + @epic = epic + @current_user = current_user + end + + private + + attr_reader :epic, :current_user + + def accessible_epics + strong_memoize(:epics) do + epic.base_and_descendants + end + end + end +end diff --git a/ee/app/services/epics/descendant_weight_service.rb b/ee/app/services/epics/descendant_weight_service.rb new file mode 100644 index 00000000000000..7cf650437ce151 --- /dev/null +++ b/ee/app/services/epics/descendant_weight_service.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +module Epics + class DescendantWeightService < DescendantService + def opened_issues + issues_weight_total.fetch(Epic.state_ids[:opened], 0) + end + + def closed_issues + issues_weight_total.fetch(Epic.state_ids[:closed], 0) + end + + private + + def issues_weight_total + strong_memoize(:issues_weight_total) do + IssuesFinder.new(current_user).execute.in_epics(accessible_epics).weight_total_by_state + end + end + end +end diff --git a/ee/changelogs/unreleased/5164-weight-and-progress-information-in-roadmap-epic-bars.yml b/ee/changelogs/unreleased/5164-weight-and-progress-information-in-roadmap-epic-bars.yml new file mode 100644 index 00000000000000..ee022cf6f2393a --- /dev/null +++ b/ee/changelogs/unreleased/5164-weight-and-progress-information-in-roadmap-epic-bars.yml @@ -0,0 +1,5 @@ +--- +title: Show weight and progress information in roadmap epic bars +merge_request: 22284 +author: +type: added diff --git a/ee/spec/graphql/types/epic_type_spec.rb b/ee/spec/graphql/types/epic_type_spec.rb index 5cde7f872c4b2a..844f6df868e71e 100644 --- a/ee/spec/graphql/types/epic_type_spec.rb +++ b/ee/spec/graphql/types/epic_type_spec.rb @@ -11,7 +11,7 @@ closed_at created_at updated_at children has_children has_issues web_path web_url relation_path reference issues user_permissions notes discussions relative_position subscribed participants - descendant_counts upvotes downvotes health_status + descendant_counts upvotes downvotes descendant_weight_sum health_status ] end diff --git a/ee/spec/models/issue_spec.rb b/ee/spec/models/issue_spec.rb index 158689cd572577..0712f78eab40fe 100644 --- a/ee/spec/models/issue_spec.rb +++ b/ee/spec/models/issue_spec.rb @@ -615,4 +615,37 @@ end it_behaves_like 'having health status' + + describe ".weight_total_by_state" do + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, namespace: group) } + let_it_be(:subgroup) { create(:group, :private, parent: group)} + let_it_be(:subproject) { create(:project, namespace: subgroup) } + # open, public + let_it_be(:issue1) { create(:issue, project: project, weight: 1) } + let_it_be(:issue2) { create(:issue, project: project, weight: 1) } + # closed + let_it_be(:issue3) { create(:issue, project: project, weight: 1, state: :closed) } + let_it_be(:issue4) { create(:issue, project: project, weight: 1, state: :closed) } + # confidential + let_it_be(:issue5) { create(:issue, project: project, weight: 1, confidential: true) } + let_it_be(:issue6) { create(:issue, project: project, weight: 1, confidential: true) } + # in private subgroup + let_it_be(:issue7) { create(:issue, project: project, weight: 1) } + let_it_be(:issue8) { create(:issue, project: project, weight: 1) } + # subgroup, confidential + let_it_be(:issue9) { create(:issue, project: project, weight: 1, confidential: true) } + let_it_be(:issue10) { create(:issue, project: project, weight: 1, confidential: true) } + # nil weight doesn't break it + let_it_be(:issue1) { create(:issue, project: project, weight: 0) } + let_it_be(:issue2) { create(:issue, project: project, weight: nil) } + + it 'sums all the weights, even confidential, or in private groups' do + opened_state_id = described_class.available_states[:opened] + closed_state_id = described_class.available_states[:closed] + expected_result = { opened_state_id => 8, closed_state_id => 2 } + + expect(project.issues.weight_total_by_state).to eq expected_result + end + end end diff --git a/ee/spec/services/epics/descendant_count_service_spec.rb b/ee/spec/services/epics/descendant_count_service_spec.rb index 3ad98ac31a875c..3045f9e55c4556 100644 --- a/ee/spec/services/epics/descendant_count_service_spec.rb +++ b/ee/spec/services/epics/descendant_count_service_spec.rb @@ -3,59 +3,19 @@ require 'spec_helper' describe Epics::DescendantCountService do - let_it_be(:group) { create(:group, :public)} - let_it_be(:subgroup) { create(:group, :private, parent: group)} - let_it_be(:user) { create(:user) } - let_it_be(:parent_epic) { create(:epic, group: group) } - let_it_be(:epic1) { create(:epic, group: subgroup, parent: parent_epic, state: :opened) } - let_it_be(:epic2) { create(:epic, group: subgroup, parent: parent_epic, state: :closed) } - - let_it_be(:project) { create(:project, :private, group: group)} - let_it_be(:issue1) { create(:issue, project: project, state: :opened) } - let_it_be(:issue2) { create(:issue, project: project, state: :closed) } - let_it_be(:issue3) { create(:issue, project: project, state: :opened) } - let_it_be(:issue4) { create(:issue, project: project, state: :closed) } - let_it_be(:epic_issue1) { create(:epic_issue, epic: parent_epic, issue: issue1) } - let_it_be(:epic_issue2) { create(:epic_issue, epic: parent_epic, issue: issue2) } - let_it_be(:epic_issue3) { create(:epic_issue, epic: epic1, issue: issue3) } - let_it_be(:epic_issue4) { create(:epic_issue, epic: epic2, issue: issue4) } - - subject { described_class.new(parent_epic, user) } - - shared_examples 'descendants state count' do |method, expected_count| - before do - stub_licensed_features(epics: true) - end - - it 'does not count inaccessible epics' do - expect(subject.public_send(method)).to eq 0 - end - - context 'when authorized' do - before do - subgroup.add_developer(user) - project.add_developer(user) - end - - it 'returns correct number of epics' do - expect(subject.public_send(method)).to eq expected_count - end - end - end - describe '#opened_epics' do - it_behaves_like 'descendants state count', :opened_epics, 1 + it_behaves_like 'descendants total', :opened_epics, 1 end describe '#closed_epics' do - it_behaves_like 'descendants state count', :closed_epics, 1 + it_behaves_like 'descendants total', :closed_epics, 1 end describe '#opened_issues' do - it_behaves_like 'descendants state count', :opened_issues, 2 + it_behaves_like 'descendants total', :opened_issues, 2 end describe '#closed_issues' do - it_behaves_like 'descendants state count', :closed_issues, 2 + it_behaves_like 'descendants total', :closed_issues, 2 end end diff --git a/ee/spec/services/epics/descendant_weight_service_spec.rb b/ee/spec/services/epics/descendant_weight_service_spec.rb new file mode 100644 index 00000000000000..84659902119299 --- /dev/null +++ b/ee/spec/services/epics/descendant_weight_service_spec.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Epics::DescendantWeightService do + describe '#opened_issues' do + it_behaves_like 'descendants total', :opened_issues, 10 + end + + describe '#closed_issues' do + it_behaves_like 'descendants total', :closed_issues, 14 + end +end diff --git a/ee/spec/support/shared_examples/services/descendants_service_examples.rb b/ee/spec/support/shared_examples/services/descendants_service_examples.rb new file mode 100644 index 00000000000000..cbb589e9ff39d4 --- /dev/null +++ b/ee/spec/support/shared_examples/services/descendants_service_examples.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +require 'spec_helper' + +shared_examples 'descendants total' do |method, expected_result| + let_it_be(:group) { create(:group, :public)} + let_it_be(:subgroup) { create(:group, :private, parent: group)} + let_it_be(:user) { create(:user) } + let_it_be(:parent_epic) { create(:epic, group: group) } + let_it_be(:epic1) { create(:epic, group: subgroup, parent: parent_epic, state: :opened) } + let_it_be(:epic2) { create(:epic, group: subgroup, parent: parent_epic, state: :closed) } + + let_it_be(:project) { create(:project, :private, group: group)} + let_it_be(:issue1) { create(:issue, project: project, state: :opened, weight: 3) } + let_it_be(:issue2) { create(:issue, project: project, state: :closed, weight: 5) } + let_it_be(:issue3) { create(:issue, project: project, state: :opened, weight: 7) } + let_it_be(:issue4) { create(:issue, project: project, state: :closed, weight: 9) } + let_it_be(:epic_issue1) { create(:epic_issue, epic: parent_epic, issue: issue1) } + let_it_be(:epic_issue2) { create(:epic_issue, epic: parent_epic, issue: issue2) } + let_it_be(:epic_issue3) { create(:epic_issue, epic: epic1, issue: issue3) } + let_it_be(:epic_issue4) { create(:epic_issue, epic: epic2, issue: issue4) } + + subject { described_class.new(parent_epic, user) } + + before do + stub_licensed_features(epics: true) + end + + it 'counts inaccessible epics' do + expect(subject.public_send(method)).to eq 1 + end + + context 'when authorized' do + before do + subgroup.add_developer(user) + project.add_developer(user) + end + + it "returns correct #{method}" do + expect(subject.public_send(method)).to eq expected_result + end + end +end -- GitLab