Skip to content

🚧 Retire FileValidator #1188

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
laritakr wants to merge 1 commit into
base: validate-files-comparable-to-importing
Choose a base branch
from
Draft

🚧 Retire FileValidator #1188

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions app/assets/javascripts/bulkrax/importers_stepper.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1562,10 +1562,6 @@
rowCount: data.rowCount != null ? data.rowCount : data.row_count,
isValid: determineIsValid(data),
hasWarnings: determineHasWarnings(data),
fileReferences: data.fileReferences != null ? data.fileReferences : data.file_references,
missingFiles: data.missingFiles || data.missing_files,
foundFiles: data.foundFiles != null ? data.foundFiles : data.found_files,
zipIncluded: data.zipIncluded != null ? data.zipIncluded : data.zip_included,
messages: data.messages,
validationErrorsCacheKey: data.validationErrorsCacheKey || null
}
Expand Down
6 changes: 2 additions & 4 deletions app/controllers/bulkrax/guided_imports_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,7 @@ def cache_validation_errors(validation_result, raw_csv_data, csv_file)
has_errors = validation_result[:rowErrors]&.any? ||
validation_result[:missingRequired]&.any? ||
validation_result[:unrecognized]&.any? ||
validation_result[:emptyColumns]&.any? ||
validation_result[:missingFiles]&.any?
validation_result[:emptyColumns]&.any?
return nil unless has_errors

key = "guided_import_errors:#{session.id}:#{Time.now.to_i}"
Expand All @@ -126,8 +125,7 @@ def cache_validation_errors(validation_result, raw_csv_data, csv_file)
file_errors: {
missing_required: validation_result[:missingRequired] || [],
unrecognized: validation_result[:unrecognized] || {},
empty_columns: validation_result[:emptyColumns] || [],
missing_files: validation_result[:missingFiles] || []
empty_columns: validation_result[:emptyColumns] || []
},
original_filename: filename_for(csv_file)
},
Expand Down
105 changes: 64 additions & 41 deletions app/controllers/concerns/bulkrax/guided_import_demo_scenarios.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,33 @@ def generate_validation_response(_csv_file, zip_file)
headers = ['source_identifier', 'title', 'creator', 'model', 'parents', 'children', 'file', 'description', 'date_created', 'legacy_id', 'subject']
unrecognized = ['legacy_id']
missing_required = []
missing_files = ['photo_087.tiff', 'letter_scan_12.pdf', 'recording_03.wav']
zip_included = zip_file.present?
missing_file_paths = ['photo_087.tiff', 'letter_scan_12.pdf', 'recording_03.wav']
row_warnings = if zip_included
missing_file_paths.each_with_index.map do |path, i|
{
row: 10 + (i * 15),
severity: 'warning',
category: 'missing_file_reference',
column: 'file',
value: path,
message: I18n.t('bulkrax.importer.guided_import.validation.file_reference_validator.errors.missing_file_reference.message', value: path),
suggestion: I18n.t('bulkrax.importer.guided_import.validation.file_reference_validator.errors.missing_file_reference.suggestion')
}
end
else
[]
end
notices = if zip_included
[]
else
[{
field: 'file',
category: 'files_referenced_no_zip',
message: I18n.t('bulkrax.importer.guided_import.validation.files_referenced_no_zip_notice.message'),
suggestion: I18n.t('bulkrax.importer.guided_import.validation.files_referenced_no_zip_notice.suggestion')
}]
end

{
headers: headers,
Expand All @@ -86,27 +111,26 @@ def generate_validation_response(_csv_file, zip_file)
works: works,
fileSets: file_sets,
totalItems: collections.length + works.length + file_sets.length,
fileReferences: 55,
missingFiles: missing_files,
foundFiles: 52,
zipIncluded: zip_included,
rowErrors: row_warnings,
notices: notices,
messages: build_validation_messages(
headers: headers, unrecognized: unrecognized, missing_required: missing_required,
missing_files: missing_files, zip_included: zip_included, row_count: 247,
is_valid: true, has_warnings: true, file_references: 55
row_warnings: row_warnings, notices: notices, row_count: 247,
is_valid: true, has_warnings: true
)
}
end
# rubocop:enable Metrics/MethodLength

# Builds the structured messages hash from validation results.
# @param results [Hash] with keys: headers, unrecognized, missing_required,
# missing_files, zip_included, row_count, is_valid, has_warnings, file_references
# row_warnings, notices, row_count, is_valid, has_warnings
def build_validation_messages(results)
issues = []
issues << missing_required_issue(results[:missing_required]) if results[:missing_required]&.any?
issues << notices_issue(results[:notices]) if results[:notices]&.any?
issues << unrecognized_fields_issue(results[:unrecognized]) if results[:unrecognized]&.any?
issues << file_references_issue(results) if results[:file_references]&.positive?
issues << row_level_warnings_issue(results[:row_warnings]) if results[:row_warnings]&.any?

{
validationStatus: validation_status(results),
Expand Down Expand Up @@ -164,38 +188,37 @@ def unrecognized_fields_issue(unrecognized)
}
end

# rubocop:disable Metrics/MethodLength
def file_references_issue(results)
file_references = results[:file_references]
missing_files = results[:missing_files] || []
found_files = file_references - missing_files.length

if missing_files.any? && results[:zip_included]
{
type: 'file_references',
severity: 'warning',
icon: 'fa-info-circle',
title: I18n.t('bulkrax.importer.guided_import.validation.file_references_title'),
count: file_references,
summary: I18n.t('bulkrax.importer.guided_import.validation.files_found_in_zip', found: found_files, total: file_references),
description: I18n.t('bulkrax.importer.guided_import.validation.files_missing_from_zip', count: missing_files.length, files_word: 'file'.pluralize(missing_files.length)),
items: missing_files.map { |file| { field: file, message: I18n.t('bulkrax.importer.guided_import.validation.missing_from_zip') } },
defaultOpen: false
}
elsif !results[:zip_included]
{
type: 'file_references',
severity: 'warning',
icon: 'fa-exclamation-triangle',
title: I18n.t('bulkrax.importer.guided_import.validation.file_references_title'),
count: file_references,
summary: I18n.t('bulkrax.importer.guided_import.validation.files_referenced', count: file_references),
description: I18n.t('bulkrax.importer.guided_import.validation.no_zip_desc'),
items: [],
defaultOpen: false
}
end
end # rubocop:enable Metrics/MethodLength
def notices_issue(notices)
{
type: 'notices',
severity: 'warning',
icon: 'fa-info-circle',
title: I18n.t('bulkrax.importer.guided_import.validation.notices_title'),
count: notices.length,
description: I18n.t('bulkrax.importer.guided_import.validation.notices_desc'),
items: notices.map { |n| { field: n[:field], message: [n[:message], n[:suggestion]].compact.join(' ') } },
defaultOpen: false
}
end

def row_level_warnings_issue(row_warnings)
{
type: 'row_level_warnings',
severity: 'warning',
icon: 'fa-exclamation-triangle',
title: I18n.t('bulkrax.importer.guided_import.stepper_response_formatter.row_errors_issue.title_warnings'),
count: row_warnings.length,
description: I18n.t('bulkrax.importer.guided_import.stepper_response_formatter.row_errors_issue.description'),
items: row_warnings.map do |error|
{
field: I18n.t('bulkrax.importer.guided_import.stepper_response_formatter.row_errors_issue.row_label', row: error[:row], column: error[:column]),
message: [error[:message], error[:suggestion]].compact.join(' '),
category: error[:category]
}
end,
defaultOpen: false
}
end
end
# rubocop:enable Metrics/ModuleLength
end
22 changes: 11 additions & 11 deletions app/parsers/concerns/bulkrax/csv_parser/csv_validation.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,12 @@ def validate_csv(csv_file:, zip_file: nil, admin_set_id: nil)
all_ids = csv_data.map { |r| r[:source_identifier] }.compact.to_set
header_issues = check_headers(headers, raw_csv, mapping_manager, mappings, field_metadata, field_analyzer)
missing_required = header_issues[:missing_required]
notices, row_errors, file_validator, collections, works, file_sets =
run_validations(csv_data, all_ids, headers, source_id_key, mappings, field_metadata, missing_required, zip_file, admin_set_id, mapping_manager: mapping_manager)
notices, row_errors, collections, works, file_sets =
run_validations(csv_data, all_ids, headers, source_id_key, mappings, field_metadata, missing_required, zip_file, mapping_manager: mapping_manager)

result = assemble_result(
headers: headers, missing_required: missing_required, header_issues: header_issues,
row_errors: row_errors, csv_data: csv_data, file_validator: file_validator,
row_errors: row_errors, csv_data: csv_data,
collections: collections, works: works, file_sets: file_sets, notices: notices
)
result[:raw_csv_data] = csv_data
Expand Down Expand Up @@ -68,19 +68,19 @@ def macos_junk_entry?(name)
name.start_with?('__MACOSX/') || File.basename(name) == '.DS_Store' || File.basename(name).start_with?('._')
end

# Builds notices, runs row validators, file validator, and hierarchy extraction.
# Returns [notices, row_errors, file_validator, collections, works, file_sets].
def run_validations(csv_data, all_ids, headers, source_id_key, mappings, field_metadata, missing_required, zip_file, admin_set_id, mapping_manager: nil) # rubocop:disable Metrics/ParameterLists
# Builds notices, runs row validators, and hierarchy extraction.
# Returns [notices, row_errors, collections, works, file_sets].
def run_validations(csv_data, all_ids, headers, source_id_key, mappings, field_metadata, missing_required, zip_file, mapping_manager: nil) # rubocop:disable Metrics/ParameterLists
find_record = build_find_record
notices = []
append_missing_source_id!(missing_required, headers, source_id_key, csv_data.map { |r| r[:model] }.compact.uniq)
append_missing_model_notice!(notices, headers, csv_data)
append_files_referenced_no_zip_notice!(notices, csv_data, zip_file)

zip_plan = build_zip_plan(zip_file)
row_errors = run_row_validators(csv_data, all_ids, source_id_key, mappings, field_metadata, find_record, notices, mapping_manager: mapping_manager, zip_plan: zip_plan)
file_validator = Bulkrax::FileValidator.new(csv_data, zip_file, admin_set_id)
collections, works, file_sets = extract_hierarchy_items(csv_data, all_ids, find_record, mappings)
[notices, row_errors, file_validator, collections, works, file_sets]
zip_plan = build_zip_plan(zip_file)
row_errors = run_row_validators(csv_data, all_ids, source_id_key, mappings, field_metadata, find_record, notices, mapping_manager: mapping_manager, zip_plan: zip_plan)
collections, works, file_sets = extract_hierarchy_items(csv_data, all_ids, find_record, mappings)
[notices, row_errors, collections, works, file_sets]
end

# Reads the CSV, resolves mappings, parses rows, and builds field metadata.
Expand Down
35 changes: 24 additions & 11 deletions app/parsers/concerns/bulkrax/csv_parser/csv_validation_helpers.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,24 @@ def append_missing_source_id!(missing_required, headers, source_id_key, all_mode
all_models.each { |model| missing_required << { model: model, field: source_id_key.to_s } }
end

# Adds a notice when the CSV references files but no zip was uploaded.
# Per-row file-reference validation can't run without a zip plan, so
# this nudges the user without blocking validation. (Files may still
# exist on the server at import time; this is intentionally a notice,
# not an error.)
def append_files_referenced_no_zip_notice!(notices, csv_data, zip_file)
return if zip_file
return unless csv_data.any? { |r| r[:file].present? }

base_key = 'bulkrax.importer.guided_import.validation.files_referenced_no_zip_notice'
notices << {
field: 'file',
category: 'files_referenced_no_zip',
message: I18n.t("#{base_key}.message"),
suggestion: I18n.t("#{base_key}.suggestion")
}
end

# Adds a file-level notice when the model column is absent or every row has a blank
# model value, indicating that the default work type will be used for all rows.
# When this notice is present the per-row default_work_type_used warnings are
Expand All @@ -164,10 +182,10 @@ def append_missing_model_notice!(notices, headers, csv_data)
end

# Assembles the final result hash returned to the guided import UI.
def assemble_result(headers:, missing_required:, header_issues:, row_errors:, csv_data:, file_validator:, collections:, works:, file_sets:, notices: []) # rubocop:disable Metrics/ParameterLists
def assemble_result(headers:, missing_required:, header_issues:, row_errors:, csv_data:, collections:, works:, file_sets:, notices: []) # rubocop:disable Metrics/ParameterLists
is_valid, has_warnings = determine_validity(
headers: headers, missing_required: missing_required, header_issues: header_issues,
row_errors: row_errors, csv_data: csv_data, file_validator: file_validator, notices: notices
row_errors: row_errors, csv_data: csv_data, notices: notices
)

{
Expand All @@ -183,19 +201,15 @@ def assemble_result(headers:, missing_required:, header_issues:, row_errors:, cs
collections: collections,
works: works,
fileSets: file_sets,
totalItems: csv_data.length,
fileReferences: file_validator.count_references,
missingFiles: file_validator.missing_files,
foundFiles: file_validator.found_files_count,
zipIncluded: file_validator.zip_included?
totalItems: csv_data.length
}
end

# Returns [is_valid, has_warnings] for the assembled result.
# rights_statement can be supplied on Step 2, so a CSV missing ONLY the
# rights_statement column is valid-with-warnings rather than a blocker;
# the display formatter styles that case as a warning accordion.
def determine_validity(headers:, missing_required:, header_issues:, row_errors:, csv_data:, file_validator:, notices:) # rubocop:disable Metrics/ParameterLists
def determine_validity(headers:, missing_required:, header_issues:, row_errors:, csv_data:, notices:) # rubocop:disable Metrics/ParameterLists
row_error_entries = row_errors.select { |e| e[:severity] == 'error' }
row_warning_entries = row_errors.select { |e| e[:severity] == 'warning' }

Expand All @@ -204,10 +218,9 @@ def determine_validity(headers:, missing_required:, header_issues:, row_errors:,
blocking_missing_required = missing_required.any? && !only_rights_missing

has_errors = blocking_missing_required || headers.blank? || csv_data.empty? ||
file_validator.missing_files.any? || row_error_entries.any?
row_error_entries.any?
has_warnings = header_issues[:unrecognized].any? || header_issues[:empty_columns].any? ||
file_validator.possible_missing_files? || row_warning_entries.any? ||
notices.any? || only_rights_missing
row_warning_entries.any? || notices.any? || only_rights_missing

[!has_errors, has_warnings]
end
Expand Down
68 changes: 0 additions & 68 deletions app/services/bulkrax/file_validator.rb
View file
Open in desktop

This file was deleted.

Loading
Loading