Adjust burndown chart opened issues countce-9578-adjust-opened-issues-count-and-completion-rate

* Adjust opened issues count in the milestone burndown chart
to match current opened issues in the milestone.
* Fix completion rate calculation to include milestones only.

6 files changed, 33 insertions, 96 deletions

diff --git a/app/models/concerns/milestoneish.rb b/app/models/concerns/milestoneish.rb
index e65bbb8ca07..3deb86da6cf 100644
--- a/app/models/concerns/milestoneish.rb
+++ b/app/models/concerns/milestoneish.rb
@@ -1,28 +1,20 @@
 # frozen_string_literal: true
 
 module Milestoneish
-  def closed_items_count(user)
-    memoize_per_user(user, :closed_items_count) do
-      (count_issues_by_state(user)['closed'] || 0) + merge_requests.closed_and_merged.size
-    end
-  end
-
-  def total_items_count(user)
-    memoize_per_user(user, :total_items_count) do
-      total_issues_count(user) + merge_requests.size
-    end
-  end
-
   def total_issues_count(user)
     count_issues_by_state(user).values.sum
   end
 
+  def closed_issues_count(user)
+    count_issues_by_state(user)['closed'].to_i
+  end
+
   def complete?(user)
-    total_items_count(user) > 0 && total_items_count(user) == closed_items_count(user)
+    total_issues_count(user) > 0 && total_issues_count(user) == closed_issues_count(user)
   end
 
   def percent_complete(user)
-    ((closed_items_count(user) * 100) / total_items_count(user)).abs
+    closed_issues_count(user) * 100 / total_issues_count(user)
   rescue ZeroDivisionError
     0
   end
diff --git a/app/views/projects/milestones/show.html.haml b/app/views/projects/milestones/show.html.haml
index 78b416edd5c..1cee8be604a 100644
--- a/app/views/projects/milestones/show.html.haml
+++ b/app/views/projects/milestones/show.html.haml
@@ -59,7 +59,7 @@
 
   = render_if_exists 'shared/milestones/burndown', milestone: @milestone, project: @project
 
-  - if can?(current_user, :read_issue, @project) && @milestone.total_items_count(current_user).zero?
+  - if can?(current_user, :read_issue, @project) && @milestone.total_issues_count(current_user).zero?
     .alert.alert-success.prepend-top-default
       %span= _('Assign some issues to this milestone.')
   - elsif @milestone.complete?(current_user) && @milestone.active?
diff --git a/changelogs/unreleased/9578-adjust-milestone-completion-rate.yml b/changelogs/unreleased/9578-adjust-milestone-completion-rate.yml
new file mode 100644
index 00000000000..0694e1462cf
--- /dev/null
+++ b/changelogs/unreleased/9578-adjust-milestone-completion-rate.yml
@@ -0,0 +1,5 @@
+---
+title: Adjust milestone completion rate to be based on issues count.
+merge_request: 28777
+author:
+type: changed
diff --git a/doc/user/project/milestones/index.md b/doc/user/project/milestones/index.md
index 0d8ee0a6cd2..6cd866b5c0d 100644
--- a/doc/user/project/milestones/index.md
+++ b/doc/user/project/milestones/index.md
@@ -138,7 +138,7 @@ For group milestones in [GitLab Premium](https://about.gitlab.com/pricing), a [b
 
 The milestone sidebar on the milestone view shows the following:
 
-- Percentage complete, which is calculated as number of closed issues plus number of closed/merged merge requests divided by total number issues and merge requests.
+- Percentage complete, which is calculated as number of closed issues divided by total number of issues.
 - The start date and due date.
 - The total time spent on all issues that have the milestone assigned.
 
diff --git a/spec/models/concerns/milestoneish_spec.rb b/spec/models/concerns/milestoneish_spec.rb
index 22d4b2cc517..7e9a8306612 100644
--- a/spec/models/concerns/milestoneish_spec.rb
+++ b/spec/models/concerns/milestoneish_spec.rb
@@ -194,58 +194,6 @@ describe Milestone, 'Milestoneish' do
     end
   end
 
-  describe '#closed_items_count' do
-    it 'does not count confidential issues for non project members' do
-      expect(milestone.closed_items_count(non_member)).to eq 2
-    end
-
-    it 'does not count confidential issues for project members with guest role' do
-      expect(milestone.closed_items_count(guest)).to eq 2
-    end
-
-    it 'counts confidential issues for author' do
-      expect(milestone.closed_items_count(author)).to eq 4
-    end
-
-    it 'counts confidential issues for assignee' do
-      expect(milestone.closed_items_count(assignee)).to eq 4
-    end
-
-    it 'counts confidential issues for project members' do
-      expect(milestone.closed_items_count(member)).to eq 6
-    end
-
-    it 'counts all issues for admin' do
-      expect(milestone.closed_items_count(admin)).to eq 6
-    end
-  end
-
-  describe '#total_items_count' do
-    it 'does not count confidential issues for non project members' do
-      expect(milestone.total_items_count(non_member)).to eq 4
-    end
-
-    it 'does not count confidential issues for project members with guest role' do
-      expect(milestone.total_items_count(guest)).to eq 4
-    end
-
-    it 'counts confidential issues for author' do
-      expect(milestone.total_items_count(author)).to eq 7
-    end
-
-    it 'counts confidential issues for assignee' do
-      expect(milestone.total_items_count(assignee)).to eq 7
-    end
-
-    it 'counts confidential issues for project members' do
-      expect(milestone.total_items_count(member)).to eq 10
-    end
-
-    it 'counts all issues for admin' do
-      expect(milestone.total_items_count(admin)).to eq 10
-    end
-  end
-
   describe '#complete?' do
     it 'returns false when has items opened' do
       expect(milestone.complete?(non_member)).to eq false
@@ -260,28 +208,42 @@ describe Milestone, 'Milestoneish' do
   end
 
   describe '#percent_complete' do
+    context 'division by zero' do
+      let(:new_milestone) { build_stubbed(:milestone) }
+
+      it { expect(new_milestone.percent_complete(admin)).to eq(0) }
+    end
+  end
+
+  describe '#count_issues_by_state' do
     it 'does not count confidential issues for non project members' do
-      expect(milestone.percent_complete(non_member)).to eq 50
+      expect(milestone.closed_issues_count(non_member)).to eq 2
+      expect(milestone.total_issues_count(non_member)).to eq 3
     end
 
     it 'does not count confidential issues for project members with guest role' do
-      expect(milestone.percent_complete(guest)).to eq 50
+      expect(milestone.closed_issues_count(guest)).to eq 2
+      expect(milestone.total_issues_count(guest)).to eq 3
     end
 
     it 'counts confidential issues for author' do
-      expect(milestone.percent_complete(author)).to eq 57
+      expect(milestone.closed_issues_count(author)).to eq 4
+      expect(milestone.total_issues_count(author)).to eq 6
     end
 
     it 'counts confidential issues for assignee' do
-      expect(milestone.percent_complete(assignee)).to eq 57
+      expect(milestone.closed_issues_count(assignee)).to eq 4
+      expect(milestone.total_issues_count(assignee)).to eq 6
     end
 
     it 'counts confidential issues for project members' do
-      expect(milestone.percent_complete(member)).to eq 60
+      expect(milestone.closed_issues_count(member)).to eq 6
+      expect(milestone.total_issues_count(member)).to eq 9
     end
 
     it 'counts confidential issues for admin' do
-      expect(milestone.percent_complete(admin)).to eq 60
+      expect(milestone.closed_issues_count(admin)).to eq 6
+      expect(milestone.total_issues_count(admin)).to eq 9
     end
   end
 
diff --git a/spec/models/milestone_spec.rb b/spec/models/milestone_spec.rb
index 752a7965704..e47f2de1ae9 100644
--- a/spec/models/milestone_spec.rb
+++ b/spec/models/milestone_spec.rb
@@ -182,32 +182,10 @@ describe Milestone do
     end
   end
 
-  describe '#percent_complete' do
-    before do
-      allow(milestone).to receive_messages(
-        closed_items_count: 3,
-        total_items_count: 4
-      )
-    end
-
-    it { expect(milestone.percent_complete(user)).to eq(75) }
-  end
-
   describe '#can_be_closed?' do
     it { expect(milestone.can_be_closed?).to be_truthy }
   end
 
-  describe '#total_items_count' do
-    before do
-      create :closed_issue, milestone: milestone, project: project
-      create :merge_request, milestone: milestone, source_project: project
-    end
-
-    it 'returns total count of issues and merge requests assigned to milestone' do
-      expect(milestone.total_items_count(user)).to eq 2
-    end
-  end
-
   describe '#can_be_closed?' do
     before do
       milestone = create :milestone, project: project