Improve algorithm to avoid double queries on postgres and mysql

app/models/burndown.rb

 class Burndown
-  attr_accessor :start_date, :end_date, :open_issues_count, :open_issues_weight
+  attr_accessor :start_date, :end_date, :issues_count, :issues_weight
 
   def initialize(milestone)
-    @milestone          = milestone
-    @start_date         = @milestone.start_date
-    @end_date           = @milestone.due_date
+    @milestone     = milestone
+    @start_date    = @milestone.start_date
+    @end_date      = @milestone.due_date
 
-    open_issues         = @milestone.issues.opened
+    issues         = @milestone.issues
 
-    @open_issues_count  = open_issues.count
-    @open_issues_weight = open_issues.sum(:weight)
+    @issues_count  = issues.count
+    @issues_weight = issues.sum(:weight)
   end
 
-  def closed_issues
-    return {} unless @start_date && @end_date
+  # Returns the chart data in the following format:
+  # [date, issue count, issue weight] eg: [["2017-03-01", 33, 127], ["2017-03-02", 35, 73], ["2017-03-03", 28, 50]...]
+  def chart_data
+    return [] unless @start_date && @end_date
 
-    Hash[
-      get_count.map do |row|
-        [row.date.strftime('%d %b'), [row.count, row.weight]]
-      end
-    ]
+    open_count = @issues_count
+    open_weight = @issues_weight
+
+    @start_date.upto(@end_date).each_with_object([]) do |date, chart_data|
+      closed, reopened = opened_and_closed_issues_by(date)
+
+      closed_count, closed_weight = count_and_weight_of(closed)
+      chart_data << [date.strftime("%Y-%m-%d"), open_count -= closed_count, open_weight -= closed_weight]
+
+      reopened_count, reopened_weight = count_and_weight_of(reopened)
+      open_count += reopened_count
+      open_weight += reopened_weight
+    end
   end
 
   private
 
-  def get_count
-    start_date = @start_date.to_time.beginning_of_day
-    end_date = @end_date.to_time.end_of_day
+  def count_and_weight_of(issues)
+    weight = issues.map{ |i| i.weight }.compact.reduce(:+)
+
+    return issues.count, weight || 0
+  end
+
+  def opened_and_closed_issues_by(date)
+    current_date = date.beginning_of_day
+
+    closed   = issues.select { |issue| issue.closed_at.beginning_of_day.to_i == current_date.to_i }
+    reopened = closed.select { |issue| issue.state == 'reopened' }
+
+    return closed, reopened
+  end
 
-    @milestone.issues.
-      select("DATE_TRUNC('day', closed_at) AS date, COUNT(*) AS count, SUM(weight) as weight").
-      where('closed_at BETWEEN (?) AND (?) AND state = ?', start_date, end_date, 'closed').
-      group(1).reorder(1)
+  def issues
+    @issues ||=
+      @milestone.issues.select('closed_at, weight, state').
+      where('closed_at IS NOT NULL').
+      order('closed_at ASC')
   end
 end

app/models/issue.rb

@@ -67,10 +67,6 @@ class Issue < ActiveRecord::Base
     before_transition any => :closed do |issue|
       issue.closed_at = Time.zone.now
     end
-
-    before_transition closed: any do |issue|
-      issue.closed_at = nil
-    end
   end
 
   def hook_attrs

db/fixtures/development/20_burndown.rb

@@ -7,6 +7,8 @@ class Gitlab::Seeder::Burndown
   end
 
   def seed!
+    Timecop.travel 10.days.ago
+
     Sidekiq::Testing.inline! do
       create_milestone
       puts '.'
@@ -16,8 +18,13 @@ class Gitlab::Seeder::Burndown
 
       close_issues
       puts '.'
+
+      reopen_issues
+      puts '.'
     end
 
+    Timecop.return
+
     print '.'
   end
 
@@ -29,20 +36,21 @@ class Gitlab::Seeder::Burndown
       description: FFaker::Lorem.sentence,
       state: 'active',
       start_date: Date.today,
-      due_date: rand(15..30).days.from_now
+      due_date: rand(5..10).days.from_now
     }
 
     @milestone = Milestones::CreateService.new(@project, @project.team.users.sample, milestone_params).execute
   end
 
   def create_issues
-    40.times do
+    20.times do
       issue_params = {
         title: FFaker::Lorem.sentence(6),
         description: FFaker::Lorem.sentence,
         state: 'opened',
         milestone: @milestone,
-        assignee: @project.team.users.sample
+        assignee: @project.team.users.sample,
+        weight: rand(1..9)
       }
 
       Issues::CreateService.new(@project, @project.team.users.sample, issue_params).execute
@@ -53,9 +61,9 @@ class Gitlab::Seeder::Burndown
     @milestone.start_date.upto(@milestone.due_date) do |date|
       Timecop.travel(date)
 
-      close_number = rand(1..3)
-      open_issues = @milestone.issues.where(state: "opened")
-      open_issues = open_issues.slice(0..close_number)
+      close_number = rand(0..2)
+      open_issues  = @milestone.issues.opened
+      open_issues  = open_issues.slice(0..close_number)
 
       open_issues.each do |issue|
         Issues::CloseService.new(@project, @project.team.users.sample, {}).execute(issue)
@@ -64,6 +72,12 @@ class Gitlab::Seeder::Burndown
 
     Timecop.return
   end
+
+  def reopen_issues
+    count  = @milestone.issues.closed.count / 3
+    issues = @milestone.issues.closed.slice(0..rand(count))
+    issues.each { |i| i.update(state: 'reopened') }
+  end
 end
 
 Gitlab::Seeder.quiet do

spec/models/burndown_spec.rb

 require 'spec_helper'
 
 describe Burndown, models: true do
-  let(:range) { 10 }
-  let(:milestone) { create(:milestone, start_date: range.days.ago, due_date: Date.today) }
-  let(:project) { milestone.project }
-  let(:user) { create(:user) }
+  let(:start_date) { "2017-03-01" }
+  let(:due_date)   { "2017-03-05" }
+  let(:milestone)  { create(:milestone, start_date: start_date, due_date: due_date) }
+  let(:project)    { milestone.project }
+  let(:user)       { create(:user) }
+
+  let(:issue_params) do
+    {
+      title: FFaker::Lorem.sentence(6),
+      description: FFaker::Lorem.sentence,
+      state: 'opened',
+      milestone: milestone,
+      weight: 2
+    }
+  end
 
   before do
     project.add_master(user)
-    @closed_by_date = {}
-
-    range.times do |i|
-      date = i.days.ago
-      @closed_by_date[date] ||= []
-
-      issue_params = {
-        title: FFaker::Lorem.sentence(6),
-        description: FFaker::Lorem.sentence,
-        state: 'closed',
-        milestone: milestone,
-        closed_at: date,
-        weight: rand(9)
-      }
-
-      rand(1..4).times do |i|
-        @closed_by_date[date] << Issues::CreateService.new(project, user, issue_params).execute
-      end
-    end
+    build_sample
+  end
+
+  after do
+    Timecop.return
+  end
+
+  subject { described_class.new(milestone).chart_data }
+
+  it "generates an array with date, issue count and weight" do
+    expect(subject).to eq([
+      ["2017-03-01", 33, 66],
+      ["2017-03-02", 35, 70],
+      ["2017-03-03", 28, 56],
+      ["2017-03-04", 32, 64],
+      ["2017-03-05", 21, 42]
+    ])
+  end
+
+  it "returns empty array if milestone start date is nil" do
+    milestone.update(start_date: nil)
+
+    expect(subject).to eq([])
+  end
+
+  it "returns empty array if milestone due date is nil" do
+    milestone.update(due_date: nil)
+
+    expect(subject).to eq([])
   end
 
-  subject { described_class.new(milestone).closed_issues }
+  # Creates, closes and reopens issues only for odd days numbers
+  def build_sample
+    milestone.start_date.upto(milestone.due_date) do |date|
+      day = date.day
+      next if day.even?
 
-  it "groups count and weight by closed_at" do
-    sample =
-      Hash[
-        @closed_by_date.sort.map do |k, v|
-          [k.strftime('%d %b'), [v.count, v.map { |issues| issues.weight }.sum]]
-        end
-      ]
+      count =  day * 4
+      issues = []
+      Timecop.travel(date)
+
+      # Create issues
+      count.times { issues << Issues::CreateService.new(project, user, issue_params).execute }
+
+      # Close issues
+      closed = issues.slice(0..count / 2)
+      closed.each { |i| Issues::CloseService.new(project, user, {}).execute(i) }
+
+      # Reopen issues
+      closed.slice(0..count / 4).each { |i| i.reopen }
+    end
 
-    expect(sample).to eq(subject)
+    Timecop.travel(due_date)
   end
 end

spec/models/issue_spec.rb

@@ -51,14 +51,6 @@ describe Issue, models: true do
 
       expect(issue.closed_at).to eq(now)
     end
-
-    it 'sets closed_at to nil when issue is reopened' do
-      issue = create(:issue, state: 'closed')
-
-      issue.reopen
-
-      expect(issue.closed_at).to be_nil
-    end
   end
 
   describe '#to_reference' do