Skip to content

Commit

Permalink
Use rows from CSV parsed with headers, not array
Browse files Browse the repository at this point in the history
  • Loading branch information
maxkadel committed Sep 30, 2021
1 parent 3009d48 commit 1913fc2
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 31 deletions.
25 changes: 7 additions & 18 deletions app/uploaders/zizia/csv_manifest_validator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,7 @@ def headers

def object_types
return ["work"] unless headers.include?(:object_type)
original_object_types = @rows.map { |row| row[headers.find_index(:object_type)] }
original_object_types.map { |object_type| map_object_type(object_type) }.compact.uniq
@object_types ||= @parsed_csv[:object_type].map { |object_type| map_object_type(object_type) }.compact.uniq
end

def map_object_type(orig_value)
Expand All @@ -90,9 +89,6 @@ def map_object_type(orig_value)
"work"
when "f", "file"
"file"
# Don't return an object type for the header
when "object type"
nil
else
"work"
end
Expand Down Expand Up @@ -143,23 +139,16 @@ def unrecognized_headers
end

def missing_values
@rows.each_with_index do |row, i|
next if i.zero? # Skip the header row
required_column_numbers(row).each_with_index do |required_column_number, j|
next unless row[required_column_number].blank?
@errors << "Missing required metadata in row #{i + 1}: \"#{required_headers(object_type(row))[j].to_s.titleize}\" field cannot be blank"
@parsed_csv.each_with_index do |row, index|
# Skip blank rows
next if row.to_hash.values.all?(&:nil?)
required_headers(object_type(row)).each do |required_header|
next unless row[required_header].blank?
@errors << "Missing required metadata in row #{index + 2}: \"#{required_header.to_s.titleize}\" field cannot be blank"
end
end
end

def required_column_numbers(row)
if headers.include?(:object_type)
required_headers(object_type(row)).map { |header| headers.find_index(header) }.compact
else
required_headers.map { |header| headers.find_index(header) }.compact
end
end

private

def object_type(row)
Expand Down
4 changes: 2 additions & 2 deletions spec/dummy/spec/system/import_from_csv_with_errors_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,9 @@
click_on 'Preview Import'

expect(page).to have_content 'This import will process 13 row(s).'
expect(page).to have_content 'Missing required metadata in row 14: "Visibility" field cannot be blank'
expect(page).to have_content 'Missing required metadata in row 17: "Visibility" field cannot be blank'

expect(page).not_to have_content 'Missing required metadata in row 14: "Creator" field cannot be blank'
expect(page).not_to have_content 'Missing required metadata in row 17: "Creator" field cannot be blank'
end
end
context "with a csv with an unrecognized object_type" do
Expand Down
18 changes: 7 additions & 11 deletions spec/uploaders/zizia/csv_manfest_validator_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,12 @@
subject(:validator) { described_class.new(uploader) }

let(:uploader) { Zizia::CsvManifestUploader.new }
# let(:required_file_plus_work_headers) { ['title', 'creator', 'keyword', 'rights statement', 'visibility', 'files', 'deduplication_key', 'parent'] }
# let(:required_work_headers) { ['title', 'creator', 'keyword', 'rights statement', 'visibility', 'files', 'deduplication_key'] }
# let(:required_collection_headers) { ['title', 'visibility'] }
# let(:required_file_headers) { ["files", "parent"] }

let(:required_file_plus_work_headers) { [:title, :creator, :keyword, :rights_statement, :visibility, :files, :deduplication_key, :parent] }
let(:required_work_headers) { [:title, :creator, :keyword, :rights_statement, :visibility, :files, :deduplication_key] }
let(:required_collection_headers) { [:title, :visibility] }
let(:required_file_headers) { [:files, :parent] }

before do
Zizia::CsvManifestUploader.enable_processing = true
File.open(path_to_file) { |f| uploader.store!(f) }
Expand Down Expand Up @@ -65,7 +63,7 @@

it "gives an error for the missing header based on having a file row" do
validator.validate
expect(validator.errors).to eq(['Missing required column: "Parent". Your spreadsheet must have this column.'])
expect(validator.errors).to eq(['Missing required column: "Parent". Your spreadsheet must have this column.', 'Missing required metadata in row 8: "Parent" field cannot be blank'])
expect(validator.warnings).to eq([])
end
end
Expand Down Expand Up @@ -109,11 +107,6 @@
expect(validator.required_headers('')).to eq(required_work_headers)
expect(validator.required_headers('file')).to eq(required_file_headers)
end

it "returns different required column numbers based on the row" do
expect(validator.required_column_numbers(work_row)).to eq([1, 3, 6, 8, 18, 19, 20])
expect(validator.required_column_numbers(collection_row)).to eq([1, 18])
end
end

context "without an object type column and empty rows" do
Expand All @@ -124,7 +117,10 @@
end
it "still gives required headers and their associated column numbers" do
expect(validator.required_headers).to match_array(required_work_headers)
expect(validator.required_column_numbers(work_row)).to eq([8, 7, 5, 6, 3, 9, 2])
end

it "can read the headers" do
expect(validator.headers).to eq([:identifier, :license, :deduplication_key, :visibility, :location, :keyword, :rights_statement, :creator, :title, :files])
end

it "still validates the file" do
Expand Down

0 comments on commit 1913fc2

Please sign in to comment.