Browse Source

setup errors do not consider threshold

- return approprate intro test depending if there are setup errors or
  not
pull/1522/head
Phil Lee 3 years ago
parent
commit
9182f44b89
  1. 4
      app/components/bulk_upload_error_summary_table_component.html.erb
  2. 9
      app/components/bulk_upload_error_summary_table_component.rb
  3. 20
      app/mailers/bulk_upload_mailer.rb
  4. 4
      app/views/bulk_upload_lettings_results/summary.html.erb
  5. 14
      spec/components/bulk_upload_error_summary_table_component_spec.rb
  6. 10
      spec/mailers/bulk_upload_mailer_spec.rb

4
app/components/bulk_upload_error_summary_table_component.html.erb

@ -1,3 +1,7 @@
<p class="govuk-body">
<%= intro %>
</p>
<% sorted_errors.each do |error| %> <% sorted_errors.each do |error| %>
<%= govuk_table do |table| %> <%= govuk_table do |table| %>
<% table.head do |head| %> <% table.head do |head| %>

9
app/components/bulk_upload_error_summary_table_component.rb

@ -24,6 +24,14 @@ class BulkUploadErrorSummaryTableComponent < ViewComponent::Base
sorted_errors.present? sorted_errors.present?
end end
def intro
if setup_errors.present?
"This summary shows important questions that have errors. See full error report for more details."
else
"This summary shows questions that have more than #{BulkUploadErrorSummaryTableComponent::DISPLAY_THRESHOLD - 1} errors. See full error report for more details."
end
end
private private
def setup_errors def setup_errors
@ -31,7 +39,6 @@ private
.bulk_upload_errors .bulk_upload_errors
.where(category: "setup") .where(category: "setup")
.group(:col, :field, :error) .group(:col, :field, :error)
.having("count(*) >= ?", display_threshold)
.count .count
.sort_by { |el| el[0][0].rjust(3, "0") } .sort_by { |el| el[0][0].rjust(3, "0") }
end end

20
app/mailers/bulk_upload_mailer.rb

@ -72,25 +72,12 @@ class BulkUploadMailer < NotifyMailer
end end
def send_bulk_upload_failed_file_setup_error_mail(bulk_upload:) def send_bulk_upload_failed_file_setup_error_mail(bulk_upload:)
bulk_upload_link = if bulk_upload.lettings? bulk_upload_link = if BulkUploadErrorSummaryTableComponent.new(bulk_upload:).errors?
start_bulk_upload_lettings_logs_url summary_bulk_upload_lettings_result_url(bulk_upload)
else else
start_bulk_upload_sales_logs_url bulk_upload_lettings_result_url(bulk_upload)
end end
row_parser_class = bulk_upload.prefix_namespace::RowParser
errors = bulk_upload
.bulk_upload_errors
.where(category: "setup")
.group(:col, :field)
.count
.keys
.sort_by { |_col, field| field }
.map do |col, field|
"- #{row_parser_class.question_for_field(field.to_sym)} (Column #{col})"
end
send_email( send_email(
bulk_upload.user.email, bulk_upload.user.email,
FAILED_FILE_SETUP_ERROR_TEMPLATE_ID, FAILED_FILE_SETUP_ERROR_TEMPLATE_ID,
@ -99,7 +86,6 @@ class BulkUploadMailer < NotifyMailer
upload_timestamp: bulk_upload.created_at.to_fs(:govuk_date_and_time), upload_timestamp: bulk_upload.created_at.to_fs(:govuk_date_and_time),
lettings_or_sales: bulk_upload.log_type, lettings_or_sales: bulk_upload.log_type,
year_combo: bulk_upload.year_combo, year_combo: bulk_upload.year_combo,
errors_list: errors.join("\n"),
bulk_upload_link:, bulk_upload_link:,
}, },
) )

4
app/views/bulk_upload_lettings_results/summary.html.erb

@ -16,10 +16,6 @@
<div class="govuk-grid-row"> <div class="govuk-grid-row">
<%= govuk_tabs(title: "Error reports") do |c| %> <%= govuk_tabs(title: "Error reports") do |c| %>
<% c.with_tab(label: "Summary") do %> <% c.with_tab(label: "Summary") do %>
<p class="govuk-body">
This summary shows questions that have more than <%= BulkUploadErrorSummaryTableComponent::DISPLAY_THRESHOLD - 1 %> errors. See full error report for more details.
</p>
<%= render BulkUploadErrorSummaryTableComponent.new(bulk_upload: @bulk_upload) %> <%= render BulkUploadErrorSummaryTableComponent.new(bulk_upload: @bulk_upload) %>
<% end %> <% end %>

14
spec/components/bulk_upload_error_summary_table_component_spec.rb

@ -41,6 +41,12 @@ RSpec.describe BulkUploadErrorSummaryTableComponent, type: :component do
result = render_inline(component) result = render_inline(component)
expect(result).to have_selector("table", count: 1) expect(result).to have_selector("table", count: 1)
end end
it "renders intro with threshold" do
result = render_inline(component)
expect(result).to have_content("This summary shows questions that have more than 0 errors. See full error report for more details.")
end
end end
context "when there are 2 independent errors" do context "when there are 2 independent errors" do
@ -112,6 +118,8 @@ RSpec.describe BulkUploadErrorSummaryTableComponent, type: :component do
before do before do
create(:bulk_upload_error, bulk_upload:, col: "B", row: 2, category: nil) create(:bulk_upload_error, bulk_upload:, col: "B", row: 2, category: nil)
stub_const("BulkUploadErrorSummaryTableComponent::DISPLAY_THRESHOLD", 16)
end end
it "only returns the setup errors" do it "only returns the setup errors" do
@ -130,6 +138,12 @@ RSpec.describe BulkUploadErrorSummaryTableComponent, type: :component do
"1 error", "1 error",
]) ])
end end
it "renders intro with setup errors" do
result = render_inline(component)
expect(result).to have_content("This summary shows important questions that have errors. See full error report for more details.")
end
end end
end end

10
spec/mailers/bulk_upload_mailer_spec.rb

@ -19,13 +19,6 @@ RSpec.describe BulkUploadMailer do
create(:bulk_upload_error, bulk_upload:, col: "F", field: "field_5") create(:bulk_upload_error, bulk_upload:, col: "F", field: "field_5")
end end
let(:expected_errors) do
[
"- What is the letting type? (Column A)",
"- Management group code (Column E)",
]
end
it "sends correctly formed email" do it "sends correctly formed email" do
expect(notify_client).to receive(:send_email).with( expect(notify_client).to receive(:send_email).with(
email_address: bulk_upload.user.email, email_address: bulk_upload.user.email,
@ -35,8 +28,7 @@ RSpec.describe BulkUploadMailer do
upload_timestamp: bulk_upload.created_at.to_fs(:govuk_date_and_time), upload_timestamp: bulk_upload.created_at.to_fs(:govuk_date_and_time),
lettings_or_sales: bulk_upload.log_type, lettings_or_sales: bulk_upload.log_type,
year_combo: bulk_upload.year_combo, year_combo: bulk_upload.year_combo,
errors_list: expected_errors.join("\n"), bulk_upload_link: summary_bulk_upload_lettings_result_url(bulk_upload),
bulk_upload_link: start_bulk_upload_lettings_logs_url,
}, },
) )

Loading…
Cancel
Save