Merge branch 'i7410-board-bundown-chart-be' into 'master'

Refactor burndown chart to accept a list of issues as input

See merge request gitlab-org/gitlab!18894

ee/app/helpers/ee/milestones_helper.rb

@@ -3,7 +3,10 @@
 module EE
   module MilestonesHelper
     def burndown_chart(milestone)
-      Burndown.new(milestone, current_user) if milestone.supports_burndown_charts?
+      if milestone.supports_burndown_charts?
+        issues = milestone.issues_visible_to_user(current_user)
+        Burndown.new(issues, milestone.start_date, milestone.due_date)
+      end
     end
 
     def can_generate_chart?(milestone, burndown)

ee/app/models/burndown.rb

@@ -3,17 +3,19 @@
 class Burndown
   include Gitlab::Utils::StrongMemoize
 
-  attr_reader :start_date, :due_date, :end_date, :accurate, :milestone, :current_user
+  attr_reader :issues, :start_date, :end_date, :due_date, :accurate
   alias_method :accurate?, :accurate
 
-  def initialize(milestone, current_user)
-    @milestone = milestone
-    @current_user = current_user
-    @start_date = @milestone.start_date
-    @due_date = @milestone.due_date
-    @end_date = @milestone.due_date
-    @end_date = Date.today if @end_date.present? && @end_date > Date.today
+  def initialize(issues, start_date, due_date)
+    @start_date = start_date
+    @due_date = due_date
+    @end_date = if due_date.blank? || due_date > Date.today
+                  Date.today
+                else
+                  due_date
+                end
 
+    @issues = filter_issues_created_before(@end_date, issues)
     @accurate = true
   end
 
@@ -36,7 +38,7 @@ class Burndown
   # If all closed issues have no closed events, mark burndown chart as containing legacy data
   def legacy_data?
     strong_memoize(:legacy_data) do
-      closed_events = milestone_issues.select(&:closed?)
+      closed_events = issues.select(&:closed?)
       closed_events.any? && !Event.closed.where(target: closed_events, action: Event::CLOSED).exists?
     end
   end
@@ -44,7 +46,7 @@ class Burndown
   private
 
   def burndown_events
-    milestone_issues
+    issues
       .map { |issue| burndown_events_for(issue) }
       .flatten
   end
@@ -57,22 +59,12 @@ class Burndown
     ].compact
   end
 
-  def milestone_issues
-    return [] unless valid?
-
-    strong_memoize(:milestone_issues) do
-      @milestone
-        .issues_visible_to_user(current_user)
-        .where('issues.created_at <= ?', end_date.end_of_day)
-    end
-  end
-
   def milestone_events_per_issue
     return [] unless valid?
 
     strong_memoize(:milestone_events_per_issue) do
       Event
-        .where(target: milestone_issues, action: [Event::CLOSED, Event::REOPENED])
+        .where(target: issues, action: [Event::CLOSED, Event::REOPENED])
         .where('created_at <= ?', end_date.end_of_day)
         .order(:created_at)
         .group_by(&:target_id)
@@ -112,10 +104,16 @@ class Burndown
     # Mark burndown chart as inaccurate
     @accurate = false
 
-    build_burndown_event(milestone.start_date.beginning_of_day, issue.weight, 'closed')
+    build_burndown_event(start_date.beginning_of_day, issue.weight, 'closed')
   end
 
   def build_burndown_event(created_at, issue_weight, action)
     { created_at: created_at, weight: issue_weight, action: action }
   end
+
+  def filter_issues_created_before(date, issues)
+    return [] unless valid?
+
+    issues.where('issues.created_at <= ?', date.end_of_day)
+  end
 end

ee/lib/ee/api/milestone_responses.rb

@@ -11,7 +11,8 @@ module EE
             milestone = parent.milestones.find(params[:milestone_id])
 
             if milestone.supports_burndown_charts?
-              present Burndown.new(milestone, current_user).as_json
+              issues = milestone.issues_visible_to_user(current_user)
+              present Burndown.new(issues, milestone.start_date, milestone.due_date).as_json
             else
               render_api_error!("Milestone does not support burndown chart", 405)
             end

ee/spec/models/burndown_spec.rb

@@ -18,7 +18,7 @@ describe Burndown do
       end
     end
 
-    subject { described_class.new(milestone, user).as_json }
+    subject { described_class.new(milestone.issues_visible_to_user(user), milestone.start_date, milestone.due_date).as_json }
 
     it 'generates an array of issues with date, issue weight and action' do
       expect(subject).to match_array([
@@ -39,7 +39,7 @@ describe Burndown do
 
       subject do
         project.update(visibility_level: Gitlab::VisibilityLevel::PUBLIC)
-        described_class.new(milestone, non_member).as_json.each { |event| event[:created_at] = event[:created_at].to_date }
+        described_class.new(milestone.issues_visible_to_user(non_member), milestone.start_date, milestone.due_date).as_json.each { |event| event[:created_at] = event[:created_at].to_date }
       end
 
       it 'does not include confidential issues for users who are not project members' do
@@ -74,15 +74,16 @@ describe Burndown do
     end
 
     it "sets attribute accurate to true" do
-      burndown = described_class.new(milestone, user)
+      burndown = described_class.new(milestone.issues_visible_to_user(user), milestone.start_date, milestone.due_date)
 
       expect(burndown).to be_accurate
     end
 
     it "is accurate with no issues" do
-      burndown = described_class.new(create(:milestone), user)
+      new_milestone = create(:milestone)
+      burndown = described_class.new(new_milestone.issues_visible_to_user(user), new_milestone.start_date, new_milestone.due_date)
 
-      burndown.milestone.project.add_master(user)
+      new_milestone.project.add_master(user)
 
       expect(burndown).to be_accurate
     end
@@ -94,7 +95,7 @@ describe Burndown do
       end
 
       it "sets attribute empty to false" do
-        burndown = described_class.new(milestone, user)
+        burndown = described_class.new(milestone.issues_visible_to_user(user), milestone.start_date, milestone.due_date)
 
         expect(burndown).not_to be_empty
       end
@@ -128,7 +129,7 @@ describe Burndown do
       end
 
       it "sets attribute empty to true" do
-        burndown = described_class.new(milestone, user)
+        burndown = described_class.new(milestone.issues_visible_to_user(user), milestone.start_date, milestone.due_date)
 
         expect(burndown).to be_empty
       end
@@ -137,7 +138,7 @@ describe Burndown do
     context "when one but not all closed issues does not have a closed event" do
       it "sets attribute accurate to false" do
         Event.where(target: milestone.issues.closed.first, action: Event::CLOSED).destroy_all # rubocop: disable DestroyAll
-        burndown = described_class.new(milestone, user)
+        burndown = described_class.new(milestone.issues_visible_to_user(user), milestone.start_date, milestone.due_date)
 
         aggregate_failures do
           expect(burndown).not_to be_empty