Merge pull request #3167 from bitzesty/pb/fix-report-ordering

Add Reports::CsvHelper#find_each_with_order

app/models/reports/admin/assessor_judge_admin_data_report.rb

@@ -56,7 +56,7 @@ def as_csv
         u = Reports::User.new(user)
 
         csv << mapping.map do |m|
-          sanitize_string(
+          Utils::String.sanitize(
             u.call_method(m[:method]),
           )
         end
@@ -66,7 +66,7 @@ def as_csv
         u = Reports::User.new(user)
 
         csv << mapping.map do |m|
-          sanitize_string(
+          Utils::String.sanitize(
             u.call_method(m[:method]),
           )
         end
@@ -76,7 +76,7 @@ def as_csv
         u = Reports::User.new(user)
 
         csv << mapping.map do |m|
-          sanitize_string(
+          Utils::String.sanitize(
             u.call_method(m[:method]),
           )
         end

app/models/reports/csv_helper.rb

@@ -9,7 +9,7 @@ def build
         form_answer = Reports::FormAnswer.new(form_answer)
 
         csv << mapping.map do |m|
-          sanitize_string(
+          Utils::String.sanitize(
             form_answer.call_method(m[:method]),
           )
         end
@@ -21,7 +21,7 @@ def prepare_response(scope, limited_access = false, builder = nil)
     CSV.generate(encoding: "UTF-8", force_quotes: true) do |csv|
       csv << headers
 
-      scope.find_each do |fa|
+      find_each_with_order(scope) do |fa|
         f = if builder.nil?
           Reports::FormAnswer.new(fa, limited_access)
         else
@@ -40,7 +40,7 @@ def prepare_stream(scope, limited_access = false)
     @_csv_enumerator ||= Enumerator.new do |yielder|
       yielder << CSV.generate_line(headers, encoding: "UTF-8", force_quotes: true)
 
-      scope.find_each do |fa|
+      find_each_with_order(scope) do |fa|
         f = Reports::FormAnswer.new(fa, limited_access)
 
         row = mapping.map do |m|
@@ -55,11 +55,17 @@ def prepare_stream(scope, limited_access = false)
 
   private
 
-  def headers
-    mapping.pluck(:label)
+  def find_each_with_order(scope, batch_size: 100, &block)
+    ordered_ids = scope.pluck(:id)
+
+    ordered_ids.in_groups_of(batch_size, false) do |ids|
+      scope.model.where(id: ids).index_by(&:id).values_at(*ordered_ids).each do |fa|
+        yield fa
+      end
+    end
   end
 
-  def sanitize_string(string)
-    string.present? ? string.to_s.tr("\n", "").squish : ""
+  def headers
+    mapping.pluck(:label)
   end
 end

spec/models/reports/case_index_report_spec.rb

@@ -0,0 +1,122 @@
+require "rails_helper"
+
+describe Reports::CaseIndexReport do
+  let(:year) { create(:award_year) }
+  let(:category) { "innovation" }
+  let(:years_mode) { "all" }
+  let(:options) { { category: category, years_mode: years_mode } }
+  let(:report) { described_class.new(year, options) }
+  let(:expected_headers) { described_class::MAPPING.pluck(:label) }
+
+  describe "#build" do
+    let(:result) { report.build.split("\n") }
+
+    context "when there are no form_answers" do
+      it "only includes the headers" do
+        expect(result.length).to eq(1)
+        expected_headers.each { |h| expect(result.first).to include(h) }
+      end
+    end
+
+    context "with form_answers that are not shortlisted" do
+      let!(:form_answers) { create_list(:form_answer, 3, category.to_sym, award_year: year) }
+
+      it "only includes the headers" do
+        expect(result.length).to eq(1)
+        expected_headers.each { |h| expect(result.first).to include(h) }
+      end
+    end
+
+    context "with form_answers that do not match the category" do
+      let!(:form_answers) { create_list(:form_answer, 3, :recommended, :trade, award_year: year) }
+
+      it "only includes the headers" do
+        expect(result.length).to eq(1)
+        expected_headers.each { |h| expect(result.first).to include(h) }
+      end
+    end
+
+    context "with form_answers that are shortlisted and match the category" do
+      let!(:form_answers) { [form_answer_one, form_answer_two, form_answer_three] }
+
+      let(:form_answer_one) { create(:form_answer, category.to_sym, :recommended, award_year: year) }
+      let(:form_answer_two) { create(:form_answer, category.to_sym, :recommended, award_year: year) }
+      let(:form_answer_three) { create(:form_answer, category.to_sym, :recommended, award_year: year) }
+
+      before do
+        form_answer_one.document["sic_code"] = "1984"
+        form_answer_one.save!
+
+        form_answer_two.document["sic_code"] = "0011"
+        form_answer_two.save!
+
+        form_answer_three.document["sic_code"] = "0030"
+        form_answer_three.save!
+      end
+
+      it "includes the headers and the form_answers in the correct order" do
+        expect(result.length).to eq(4)
+        expected_headers.each { |h| expect(result.first).to include(h) }
+        expect(result.second).to include(form_answer_two.urn)
+        expect(result.third).to include(form_answer_three.urn)
+        expect(result.fourth).to include(form_answer_one.urn)
+      end
+    end
+  end
+
+  describe "#stream" do
+    let(:result) { report.stream.to_a }
+
+    context "when there are no form_answers" do
+      it "only includes the headers" do
+        expect(result.length).to eq(1)
+        expected_headers.each { |h| expect(result.first).to include(h) }
+      end
+    end
+
+    context "with form_answers that are not shortlisted" do
+      let!(:form_answers) { create_list(:form_answer, 3, category.to_sym, award_year: year) }
+
+      it "only includes the headers" do
+        expect(result.split("\n").length).to eq(1)
+        expected_headers.each { |h| expect(result.first).to include(h) }
+      end
+    end
+
+    context "with form_answers that do not match the category" do
+      let!(:form_answers) { create_list(:form_answer, 3, :recommended, :trade, award_year: year) }
+
+      it "only includes the headers" do
+        expect(result.split("\n").length).to eq(1)
+        expected_headers.each { |h| expect(result.first).to include(h) }
+      end
+    end
+
+    context "with form_answers that are shortlisted and match the category" do
+      let!(:form_answers) { [form_answer_one, form_answer_two, form_answer_three] }
+
+      let(:form_answer_one) { create(:form_answer, category.to_sym, :recommended, award_year: year) }
+      let(:form_answer_two) { create(:form_answer, category.to_sym, :recommended, award_year: year) }
+      let(:form_answer_three) { create(:form_answer, category.to_sym, :recommended, award_year: year) }
+
+      before do
+        form_answer_one.document["sic_code"] = "1984"
+        form_answer_one.save!
+
+        form_answer_two.document["sic_code"] = "0011"
+        form_answer_two.save!
+
+        form_answer_three.document["sic_code"] = "0030"
+        form_answer_three.save!
+      end
+
+      it "includes the headers and the form_answers in the correct order" do
+        expect(result.length).to eq(4)
+        expected_headers.each { |h| expect(result.first).to include(h) }
+        expect(result.second).to include(form_answer_two.urn)
+        expect(result.third).to include(form_answer_three.urn)
+        expect(result.fourth).to include(form_answer_one.urn)
+      end
+    end
+  end
+end