From dea6cedc0a0bb4ca118896440329b0fbde436591 Mon Sep 17 00:00:00 2001 From: Graham Pengelly Date: Fri, 19 Jan 2024 16:37:38 +0000 Subject: [PATCH] Pass source_column_count from CSV row As we create a ChildrensBarredListEntry from each CSV row we want to check that the row has the correct number of columns to avoid errors where the data is a column out if the CSV has been incorrectly created. Pass the row column count with the params. --- .../create_childrens_barred_list_entries.rb | 2 ++ config/locales/en.yml | 8 +++++- spec/fixtures/example-incorrect-columns.csv | 4 +++ .../support_interface/upload_form_spec.rb | 4 +-- ...eate_childrens_barred_list_entries_spec.rb | 6 ++--- .../support_user_uploads_a_csv_file_spec.rb | 27 ++++++++++++++++++- 6 files changed, 44 insertions(+), 7 deletions(-) create mode 100644 spec/fixtures/example-incorrect-columns.csv diff --git a/app/services/create_childrens_barred_list_entries.rb b/app/services/create_childrens_barred_list_entries.rb index 24671006..04c394da 100644 --- a/app/services/create_childrens_barred_list_entries.rb +++ b/app/services/create_childrens_barred_list_entries.rb @@ -15,6 +15,7 @@ def call CSV .parse(@raw_data, encoding: "ISO8859-1:utf-8") .each do |row| + source_column_count = row.count entry = ChildrensBarredListEntry.create( trn: pad_trn(row[0]), last_name: format_names(row[1]), @@ -22,6 +23,7 @@ def call date_of_birth: format_date_of_birth(row[3]), national_insurance_number: row[4], upload_file_hash:, + source_column_count: ) @failed_entries << entry unless entry.persisted? diff --git a/config/locales/en.yml b/config/locales/en.yml index f1335d7c..094a5993 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -30,6 +30,12 @@ en: blank: Select a file invalid_csv: The selected file must be a CSV activerecord: + models: + childrens_barred_list_entry: Children’s Barred List entry + feedback: Feedback + attributes: + childrens_barred_list_entry: + source_column_count: The uploaded file errors: models: childrens_barred_list_entry: @@ -37,7 +43,7 @@ en: last_name: taken: with same first names and date of birth already exists source_column_count: - invalid: The uploaded file must have %{required_count} columns + invalid: must have %{required_count} columns feedback: attributes: satisfaction_rating: diff --git a/spec/fixtures/example-incorrect-columns.csv b/spec/fixtures/example-incorrect-columns.csv new file mode 100644 index 00000000..06c02c65 --- /dev/null +++ b/spec/fixtures/example-incorrect-columns.csv @@ -0,0 +1,4 @@ +Darby,James,10/11/1979,,N +Jones,Tim,01/12/2001,,N +Clongbong,Joe,07/06/1989,,N +Kramer,Richie,18/09/1982,,N diff --git a/spec/forms/support_interface/upload_form_spec.rb b/spec/forms/support_interface/upload_form_spec.rb index 890b9c27..1b9f8ffc 100644 --- a/spec/forms/support_interface/upload_form_spec.rb +++ b/spec/forms/support_interface/upload_form_spec.rb @@ -15,8 +15,8 @@ context "when the form is valid" do let(:csv_data) do [ - ["12567", "Smith", "John James", "01/02/1990", "AB123456C"].join(","), - ["1234568", "Smith", "Jane Jemima", "07/05/1980", "AB123456D"].join( + ["12567", "Smith", "John James", "01/02/1990", "AB123456C", "N"].join(","), + ["1234568", "Smith", "Jane Jemima", "07/05/1980", "AB123456D", "N"].join( "," ) ].join("\n") diff --git a/spec/services/create_childrens_barred_list_entries_spec.rb b/spec/services/create_childrens_barred_list_entries_spec.rb index 51e05c70..42bb5666 100644 --- a/spec/services/create_childrens_barred_list_entries_spec.rb +++ b/spec/services/create_childrens_barred_list_entries_spec.rb @@ -3,8 +3,8 @@ RSpec.describe CreateChildrensBarredListEntries do let(:csv_data) do [ - ["12567", "SMITH ", "DR. JOHN JAMES ", "01/02/1990", "AB123456C"].join(","), - ["1234568", " jones ", " mrs jane jemima", "07/05/1980", "AB123456D"].join(",") + ["12567", "SMITH ", "DR. JOHN JAMES ", "01/02/1990", "AB123456C, N"].join(","), + ["1234568", " jones ", " mrs jane jemima", "07/05/1980", "AB123456D, N"].join(",") ].join("\n") end @@ -78,7 +78,7 @@ context "when a row contains non-UTF-8 characters" do let(:csv_data) do [ - ["12567", "Sánchezera-blobbá", "Angélina", "01/11/1990", "AB123456C"].join(","), + ["12567", "Sánchezera-blobbá", "Angélina", "01/11/1990", "AB123456C", "N"].join(","), ].join("\n").encode("ISO-8859-1") end diff --git a/spec/system/support_interface/support_user_uploads_a_csv_file_spec.rb b/spec/system/support_interface/support_user_uploads_a_csv_file_spec.rb index f10d1567..55527fd2 100644 --- a/spec/system/support_interface/support_user_uploads_a_csv_file_spec.rb +++ b/spec/system/support_interface/support_user_uploads_a_csv_file_spec.rb @@ -20,6 +20,14 @@ then_i_see_a_success_message end + scenario "Support user uploads an CSV file that contains rows with an incorrect number of columnss", test: :with_stubbed_auth do + given_the_service_is_open + and_i_am_signed_in_as_an_internal_user_via_dsi + and_i_am_on_the_upload_page + when_i_upload_a_csv_file_with_an_incorrect_number_of_columns + then_i_see_failed_entries + end + def given_the_service_is_open FeatureFlags::FeatureFlag.activate(:service_open) end @@ -74,7 +82,6 @@ def and_unconfirmed_entries_are_removed def and_i_can_see_what_will_not_be_saved expect(page).to have_content("1 entry will not be saved") - invalid_entries_table = page.all("table")[0] within(invalid_entries_table) do expect(page).to have_content("Simpson") expect(page).to have_content("Homer Duff") @@ -91,6 +98,23 @@ def then_i_see_a_success_message expect(page).to have_content("Records uploaded") end + def when_i_upload_a_csv_file_with_an_incorrect_number_of_columns + attach_file "support_interface_upload_form[file]", + Rails.root.join("spec/fixtures/example-incorrect-columns.csv") + click_on "Upload file" + end + + def then_i_see_failed_entries + expect(page).to have_content("4 entries will not be saved") + + within(invalid_entries_table) do + expect(page).to have_content("Darby") + expect(page).to have_content("James") + expect(page).to have_content("10/11/1979") + expect(page).to have_content("The uploaded file must have 6 columns") + end + end + def valid_entries_table page.all("table")[1] end @@ -98,4 +122,5 @@ def valid_entries_table def invalid_entries_table page.all("table")[0] end + end