CLDC-2184 Fix ordering for bulk upload errors by cell (#1544)

# Context

- https://digital.dclg.gov.uk/jira/browse/CLDC-2184
- bulk upload error reports were not sorting by cells correctly

# Changes

- there was an issue where `Z100` would come after `AA100` which is not correct
- fix ordering issue above
- also made change so this is now delegated to the database rather than sorting in ruby

app/components/bulk_upload_error_row_component.rb

@@ -2,7 +2,7 @@ class BulkUploadErrorRowComponent < ViewComponent::Base
   attr_reader :bulk_upload_errors
 
   def initialize(bulk_upload_errors:)
-    @bulk_upload_errors = sorted_errors(bulk_upload_errors)
+    @bulk_upload_errors = bulk_upload_errors
 
     super
   end
@@ -62,10 +62,4 @@ class BulkUploadErrorRowComponent < ViewComponent::Base
   def sales?
     bulk_upload.log_type == "sales"
   end
-
-private
-
-  def sorted_errors(errors)
-    errors.sort_by { |e| e.cell.rjust(3, "0") }
-  end
 end

app/components/bulk_upload_error_summary_table_component.rb

@@ -16,8 +16,8 @@ class BulkUploadErrorSummaryTableComponent < ViewComponent::Base
       .bulk_upload_errors
       .group(:col, :field, :error)
       .having("count(*) >= ?", display_threshold)
+      .order_by_col
       .count
-      .sort_by { |el| el[0][0].rjust(3, "0") }
   end
 
   def errors?
@@ -39,8 +39,8 @@ private
       .bulk_upload_errors
       .where(category: "setup")
       .group(:col, :field, :error)
+      .order_by_col
       .count
-      .sort_by { |el| el[0][0].rjust(3, "0") }
   end
 
   def display_threshold

app/models/bulk_upload_error.rb

@@ -1,3 +1,6 @@
 class BulkUploadError < ApplicationRecord
   belongs_to :bulk_upload
+
+  scope :order_by_cell, -> { order(Arel.sql("LPAD(cell, 10, '0')")) }
+  scope :order_by_col, -> { order(Arel.sql("LPAD(col, 10, '0')")) }
 end

app/views/bulk_upload_lettings_results/show.html.erb

@@ -13,7 +13,7 @@
 
 <div class="govuk-grid-row">
   <div class="govuk-grid-column-full">
-    <% @bulk_upload.bulk_upload_errors.group_by(&:row).each do |_row, errors_for_row| %>
+    <% @bulk_upload.bulk_upload_errors.order_by_cell.group_by(&:row).each do |_row, errors_for_row| %>
       <%= render BulkUploadErrorRowComponent.new(bulk_upload_errors: errors_for_row) %>
     <% end %>
   </div>

spec/components/bulk_upload_error_row_component_spec.rb

@@ -78,22 +78,6 @@ RSpec.describe BulkUploadErrorRowComponent, type: :component do
       end
     end
 
-    context "when multiple errors for a row" do
-      subject(:component) { described_class.new(bulk_upload_errors:) }
-
-      let(:bulk_upload_errors) do
-        [
-          build(:bulk_upload_error, cell: "Z1"),
-          build(:bulk_upload_error, cell: "AB1"),
-          build(:bulk_upload_error, cell: "A1"),
-        ]
-      end
-
-      it "is sorted by cell" do
-        expect(component.bulk_upload_errors.map(&:cell)).to eql(%w[A1 Z1 AB1])
-      end
-    end
-
     context "when a sales bulk upload" do
       let(:bulk_upload) { create(:bulk_upload, :sales) }
       let(:field) { :field_87 }

spec/views/bulk_upload_lettings_results/show.html.erb_spec.rb

@@ -0,0 +1,20 @@
+require "rails_helper"
+
+RSpec.describe "bulk_upload_lettings_results/show.html.erb" do
+  let(:bulk_upload) { create(:bulk_upload, :lettings) }
+
+  before do
+    create(:bulk_upload_error, bulk_upload:, cell: "AA100", row: "100", col: "AA")
+    create(:bulk_upload_error, bulk_upload:, cell: "Z100", row: "100", col: "Z")
+  end
+
+  it "renders errors ordered by cell" do
+    assign(:bulk_upload, bulk_upload)
+
+    render
+
+    fragment = Capybara::Node::Simple.new(rendered)
+
+    expect(fragment.find_css("table tbody th").map(&:inner_text)).to eql(%w[Z100 AA100])
+  end
+end