From a8241032c8821afbf7627bf55f36531c35a864fe Mon Sep 17 00:00:00 2001 From: jmilljr24 <16829344+jmilljr24@users.noreply.github.com> Date: Thu, 5 Dec 2024 09:17:47 -0500 Subject: [PATCH 1/6] add validation and error messages --- .../importers/google_csv_import_service.rb | 74 ++++++++++++------- 1 file changed, 49 insertions(+), 25 deletions(-) diff --git a/app/services/organizations/importers/google_csv_import_service.rb b/app/services/organizations/importers/google_csv_import_service.rb index d29723be9..fa9775b96 100644 --- a/app/services/organizations/importers/google_csv_import_service.rb +++ b/app/services/organizations/importers/google_csv_import_service.rb @@ -13,42 +13,54 @@ def initialize(file) end def call - CSV.foreach(@file.to_path, headers: true, skip_blanks: true).with_index(1) do |row, index| - # Using Google Form headers - email = row["Email"].downcase - csv_timestamp = Time.parse(row["Timestamp"]) + catch(:halt_import) do + validate_file - person = Person.find_by(email:, organization: @organization) - previously_matched_form_submission = FormSubmission.where(person:, csv_timestamp:) + CSV.foreach(@file.to_path, headers: true, skip_blanks: true).with_index(1) do |row, index| + # Using Google Form headers + email = row["Email"].downcase + csv_timestamp = Time.parse(row["Timestamp"]) - if person.nil? - @no_match << [index, email] - elsif previously_matched_form_submission.present? - next - else - latest_form_submission = person.latest_form_submission - ActiveRecord::Base.transaction do - # This checks for the empty form submission that is added when a person is created - if latest_form_submission.csv_timestamp.nil? && latest_form_submission.form_answers.empty? - latest_form_submission.update!(csv_timestamp:) - create_form_answers(latest_form_submission, row) - else - # if the person submits a new/updated form, - # i.e. an additional row in the csv with the same email / different timestamp, - # a new form_submission will be created - create_form_answers(FormSubmission.create!(person:, csv_timestamp:), row) + person = Person.find_by(email:, organization: @organization) + previously_matched_form_submission = FormSubmission.where(person:, csv_timestamp:) + + if person.nil? + @no_match << [index, email] + elsif previously_matched_form_submission.present? + next + else + latest_form_submission = person.latest_form_submission + ActiveRecord::Base.transaction do + # This checks for the empty form submission that is added when a person is created + if latest_form_submission.csv_timestamp.nil? && latest_form_submission.form_answers.empty? + latest_form_submission.update!(csv_timestamp:) + create_form_answers(latest_form_submission, row) + else + # if the person submits a new/updated form, + # i.e. an additional row in the csv with the same email / different timestamp, + # a new form_submission will be created + create_form_answers(FormSubmission.create!(person:, csv_timestamp:), row) + end + @count += 1 end - @count += 1 end + rescue => e + @errors << [index, e] end - rescue => e - @errors << [index, e] end Status.new(@errors.empty?, @count, @no_match, @errors) end private + def validate_file + raise FileTypeError unless @file.content_type == "text/csv" + raise EmailColumnError unless CSV.foreach(@file.to_path).first.include?("Email") + rescue EmailColumnError, FileTypeError => e + @errors << e + throw :halt_import + end + def create_form_answers(form_submission, row) row.each do |col| next if col[0] == "Email" || col[0] == "Timestamp" @@ -61,6 +73,18 @@ def create_form_answers(form_submission, row) ) end end + + class EmailColumnError < StandardError + def message + 'The column header "Email" was not found in the attached csv' + end + end + + class FileTypeError < StandardError + def message + "Invalid File Type: File type must be CSV" + end + end end end end From e0e5f8608f022ae8bcccbde7a8c3c61d090496dd Mon Sep 17 00:00:00 2001 From: jmilljr24 <16829344+jmilljr24@users.noreply.github.com> Date: Thu, 5 Dec 2024 10:45:27 -0500 Subject: [PATCH 2/6] add tests --- .../importers/google_csv_import_service.rb | 5 +++- .../organizations/csv_import_service_test.rb | 30 +++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/app/services/organizations/importers/google_csv_import_service.rb b/app/services/organizations/importers/google_csv_import_service.rb index fa9775b96..4262ddef2 100644 --- a/app/services/organizations/importers/google_csv_import_service.rb +++ b/app/services/organizations/importers/google_csv_import_service.rb @@ -55,7 +55,10 @@ def call def validate_file raise FileTypeError unless @file.content_type == "text/csv" - raise EmailColumnError unless CSV.foreach(@file.to_path).first.include?("Email") + + first_row = CSV.foreach(@file.to_path).first + raise EmailColumnError if first_row.nil? + raise EmailColumnError unless first_row.include?("Email") rescue EmailColumnError, FileTypeError => e @errors << e throw :halt_import diff --git a/test/services/organizations/csv_import_service_test.rb b/test/services/organizations/csv_import_service_test.rb index 9794b0bae..92994fb42 100644 --- a/test/services/organizations/csv_import_service_test.rb +++ b/test/services/organizations/csv_import_service_test.rb @@ -8,6 +8,8 @@ class CsvImportServiceTest < ActiveSupport::TestCase Current.organization = adopter.organization @file = Tempfile.new(["test", ".csv"]) + @file.stubs(:content_type).returns("text/csv") + headers = ["Timestamp", "First name", "Last name", "Email", "Address", "Phone number", *Faker::Lorem.questions] @data = [ @@ -110,5 +112,33 @@ class CsvImportServiceTest < ActiveSupport::TestCase refute import.success? assert_equal "mon out of range", import.errors[0][1].message end + + should "validate file type" do + file = Tempfile.new(["test", ".png"]) + file.stubs(:content_type).returns("image/png") + import = Organizations::Importers::GoogleCsvImportService.new(file).call + + assert_equal "Invalid File Type: File type must be CSV", import.errors.first.message + end + + should "validate empty header" do + file = Tempfile.new(["test", ".csv"]) + file.stubs(:content_type).returns("text/csv") + import = Organizations::Importers::GoogleCsvImportService.new(file).call + + assert_equal 'The column header "Email" was not found in the attached csv', import.errors.first.message + end + + should "validate email header" do + file = Tempfile.new(["test", ".csv"]) + headers = ["Timestamp", "First name", "Last name", "Address", "Phone number", *Faker::Lorem.questions] + CSV.open(@file.path, "wb") do |csv| + csv << headers + end + file.stubs(:content_type).returns("text/csv") + import = Organizations::Importers::GoogleCsvImportService.new(file).call + + assert_equal 'The column header "Email" was not found in the attached csv', import.errors.first.message + end end end From 53d1ae7b9ae72362c700488fe6521c6ed0ef641d Mon Sep 17 00:00:00 2001 From: jmilljr24 <16829344+jmilljr24@users.noreply.github.com> Date: Fri, 6 Dec 2024 13:17:29 -0500 Subject: [PATCH 3/6] add error for empty file --- .../importers/google_csv_import_service.rb | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/app/services/organizations/importers/google_csv_import_service.rb b/app/services/organizations/importers/google_csv_import_service.rb index 4262ddef2..e7af8f611 100644 --- a/app/services/organizations/importers/google_csv_import_service.rb +++ b/app/services/organizations/importers/google_csv_import_service.rb @@ -55,11 +55,11 @@ def call def validate_file raise FileTypeError unless @file.content_type == "text/csv" - first_row = CSV.foreach(@file.to_path).first - raise EmailColumnError if first_row.nil? + + raise FileEmptyError if first_row.nil? raise EmailColumnError unless first_row.include?("Email") - rescue EmailColumnError, FileTypeError => e + rescue EmailColumnError, FileTypeError, FileEmptyError => e @errors << e throw :halt_import end @@ -88,6 +88,12 @@ def message "Invalid File Type: File type must be CSV" end end + + class FileEmptyError < StandardError + def message + "File is empty" + end + end end end end From 50ecffc916ca6a5038cab285a0fac96c034660c9 Mon Sep 17 00:00:00 2001 From: jmilljr24 <16829344+jmilljr24@users.noreply.github.com> Date: Fri, 6 Dec 2024 14:06:20 -0500 Subject: [PATCH 4/6] add array of valid email header variations --- .../importers/google_csv_import_service.rb | 12 +++++++++--- .../organizations/csv_import_service_test.rb | 6 +++--- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/app/services/organizations/importers/google_csv_import_service.rb b/app/services/organizations/importers/google_csv_import_service.rb index e7af8f611..3638831bb 100644 --- a/app/services/organizations/importers/google_csv_import_service.rb +++ b/app/services/organizations/importers/google_csv_import_service.rb @@ -18,7 +18,8 @@ def call CSV.foreach(@file.to_path, headers: true, skip_blanks: true).with_index(1) do |row, index| # Using Google Form headers - email = row["Email"].downcase + debugger + email = row[@email_header].downcase csv_timestamp = Time.parse(row["Timestamp"]) person = Person.find_by(email:, organization: @organization) @@ -55,10 +56,15 @@ def call def validate_file raise FileTypeError unless @file.content_type == "text/csv" - first_row = CSV.foreach(@file.to_path).first + first_row = CSV.foreach(@file.to_path).first raise FileEmptyError if first_row.nil? - raise EmailColumnError unless first_row.include?("Email") + + email_headers = ["Email", "email", "Email Address", "email address"] + email_headers.each do |e| + @email_header = e if first_row.include?(e) + end + raise EmailColumnError unless @email_header rescue EmailColumnError, FileTypeError, FileEmptyError => e @errors << e throw :halt_import diff --git a/test/services/organizations/csv_import_service_test.rb b/test/services/organizations/csv_import_service_test.rb index 92994fb42..945fc6246 100644 --- a/test/services/organizations/csv_import_service_test.rb +++ b/test/services/organizations/csv_import_service_test.rb @@ -121,18 +121,18 @@ class CsvImportServiceTest < ActiveSupport::TestCase assert_equal "Invalid File Type: File type must be CSV", import.errors.first.message end - should "validate empty header" do + should "validate empty file" do file = Tempfile.new(["test", ".csv"]) file.stubs(:content_type).returns("text/csv") import = Organizations::Importers::GoogleCsvImportService.new(file).call - assert_equal 'The column header "Email" was not found in the attached csv', import.errors.first.message + assert_equal "File is empty", import.errors.first.message end should "validate email header" do file = Tempfile.new(["test", ".csv"]) headers = ["Timestamp", "First name", "Last name", "Address", "Phone number", *Faker::Lorem.questions] - CSV.open(@file.path, "wb") do |csv| + CSV.open(file.path, "wb") do |csv| csv << headers end file.stubs(:content_type).returns("text/csv") From e93623cee482e88ce69b930db3e18ac24ef676f7 Mon Sep 17 00:00:00 2001 From: jmilljr24 <16829344+jmilljr24@users.noreply.github.com> Date: Fri, 6 Dec 2024 14:07:36 -0500 Subject: [PATCH 5/6] remove debugger --- .../organizations/importers/google_csv_import_service.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/services/organizations/importers/google_csv_import_service.rb b/app/services/organizations/importers/google_csv_import_service.rb index 3638831bb..0274179c9 100644 --- a/app/services/organizations/importers/google_csv_import_service.rb +++ b/app/services/organizations/importers/google_csv_import_service.rb @@ -18,7 +18,6 @@ def call CSV.foreach(@file.to_path, headers: true, skip_blanks: true).with_index(1) do |row, index| # Using Google Form headers - debugger email = row[@email_header].downcase csv_timestamp = Time.parse(row["Timestamp"]) From 3911c0edc8faf66a1b9e51ac0b31a6e4e3eec814 Mon Sep 17 00:00:00 2001 From: jmilljr24 <16829344+jmilljr24@users.noreply.github.com> Date: Fri, 6 Dec 2024 14:35:29 -0500 Subject: [PATCH 6/6] add timestamp header validation --- .../importers/google_csv_import_service.rb | 10 +++++++++- .../organizations/csv_import_service_test.rb | 12 ++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/app/services/organizations/importers/google_csv_import_service.rb b/app/services/organizations/importers/google_csv_import_service.rb index 0274179c9..cc167b58a 100644 --- a/app/services/organizations/importers/google_csv_import_service.rb +++ b/app/services/organizations/importers/google_csv_import_service.rb @@ -59,12 +59,14 @@ def validate_file first_row = CSV.foreach(@file.to_path).first raise FileEmptyError if first_row.nil? + raise TimestampColumnError unless first_row.include?("Timestamp") + email_headers = ["Email", "email", "Email Address", "email address"] email_headers.each do |e| @email_header = e if first_row.include?(e) end raise EmailColumnError unless @email_header - rescue EmailColumnError, FileTypeError, FileEmptyError => e + rescue FileTypeError, FileEmptyError, TimestampColumnError, EmailColumnError => e @errors << e throw :halt_import end @@ -88,6 +90,12 @@ def message end end + class TimestampColumnError < StandardError + def message + 'The column header "Timestamp" was not found in the attached csv' + end + end + class FileTypeError < StandardError def message "Invalid File Type: File type must be CSV" diff --git a/test/services/organizations/csv_import_service_test.rb b/test/services/organizations/csv_import_service_test.rb index 945fc6246..0891f5cee 100644 --- a/test/services/organizations/csv_import_service_test.rb +++ b/test/services/organizations/csv_import_service_test.rb @@ -140,5 +140,17 @@ class CsvImportServiceTest < ActiveSupport::TestCase assert_equal 'The column header "Email" was not found in the attached csv', import.errors.first.message end + + should "validate Timestamp header" do + file = Tempfile.new(["test", ".csv"]) + headers = ["Email", "First name", "Last name", "Address", "Phone number", *Faker::Lorem.questions] + CSV.open(file.path, "wb") do |csv| + csv << headers + end + file.stubs(:content_type).returns("text/csv") + import = Organizations::Importers::GoogleCsvImportService.new(file).call + + assert_equal 'The column header "Timestamp" was not found in the attached csv', import.errors.first.message + end end end