diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index cf631df..8544b80 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -13,9 +13,9 @@ jobs: services: elasticsearch: - image: docker.elastic.co/elasticsearch/elasticsearch:7.9.2 + image: docker.elastic.co/elasticsearch/elasticsearch:8.13.4 env: - STACK_VERSION: 7.17.1 + STACK_VERSION: 8.13.4 xpack.security.enabled: false cluster.name: beckett-elasticsearch http.port: 9200 @@ -29,7 +29,7 @@ jobs: - 9200:9200 postgres: - image: postgres:10 + image: postgres:16 env: POSTGRES_PASSWORD: password POSTGRES_USER: user diff --git a/Gemfile b/Gemfile index 0b1780e..f55ec04 100644 --- a/Gemfile +++ b/Gemfile @@ -14,7 +14,7 @@ gem 'csv' gem 'pg', '~> 1.1' # Elasticseach for search -gem 'elasticsearch', '~> 7.17.1' +gem 'elasticsearch', '~> 8' gem 'searchkick' # Use the Puma web server [https://github.com/puma/puma] @@ -95,3 +95,7 @@ gem 'addressable', '~> 2.8' gem 'importmap-rails', '~> 2.0' gem 'sidekiq-cron', '~>2.4.0' + +# SES delivery for ActionMailer, authenticating via the instance/task IAM role +gem 'aws-sdk-rails', '~> 5.0' +gem 'aws-sdk-ses', '~> 1.0' diff --git a/Gemfile.lock b/Gemfile.lock index 26bc5af..418e2d9 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -99,6 +99,24 @@ GEM administrate rails (>= 5.0) ast (2.4.2) + aws-eventstream (1.4.0) + aws-partitions (1.1278.0) + aws-sdk-core (3.254.1) + aws-eventstream (~> 1, >= 1.3.0) + aws-partitions (~> 1, >= 1.992.0) + aws-sigv4 (~> 1.9) + base64 + bigdecimal + jmespath (~> 1, >= 1.6.1) + logger + aws-sdk-rails (5.1.0) + aws-sdk-core (~> 3) + railties (>= 7.1.0) + aws-sdk-ses (1.102.0) + aws-sdk-core (~> 3, >= 3.254.0) + aws-sigv4 (~> 1.5) + aws-sigv4 (1.12.1) + aws-eventstream (~> 1, >= 1.0.2) base64 (0.2.0) bigdecimal (3.1.8) bootsnap (1.18.4) @@ -117,14 +135,14 @@ GEM reline (>= 0.3.8) diff-lcs (1.5.1) drb (2.2.1) - elasticsearch (7.17.11) - elasticsearch-api (= 7.17.11) - elasticsearch-transport (= 7.17.11) - elasticsearch-api (7.17.11) + elastic-transport (8.5.2) + faraday (< 3) multi_json - elasticsearch-transport (7.17.11) - base64 - faraday (>= 1, < 3) + elasticsearch (8.19.3) + elastic-transport (~> 8.3) + elasticsearch-api (= 8.19.3) + ostruct + elasticsearch-api (8.19.3) multi_json erubi (1.13.0) et-orbi (1.4.0) @@ -136,11 +154,12 @@ GEM railties (>= 5.0.0) faker (3.4.2) i18n (>= 1.8.11, < 2) - faraday (2.11.0) - faraday-net_http (>= 2.0, < 3.4) + faraday (2.14.2) + faraday-net_http (>= 2.0, < 3.5) + json logger - faraday-net_http (3.3.0) - net-http + faraday-net_http (3.4.4) + net-http (~> 0.5) ffi (1.17.0-aarch64-linux-gnu) ffi (1.17.0-aarch64-linux-musl) ffi (1.17.0-arm-linux-gnu) @@ -176,6 +195,7 @@ GEM jbuilder (2.12.0) actionview (>= 5.0.0) activesupport (>= 5.0.0) + jmespath (1.6.2) jquery-rails (4.6.0) rails-dom-testing (>= 1, < 3) railties (>= 4.2.0) @@ -213,14 +233,14 @@ GEM mini_mime (1.1.5) minitest (5.25.1) msgpack (1.7.2) - multi_json (1.15.0) + multi_json (1.21.1) multi_xml (0.7.1) bigdecimal (~> 3.1) mustache (1.1.1) namae (1.2.0) racc (~> 1.7) - net-http (0.4.1) - uri + net-http (0.9.1) + uri (>= 0.11.1) net-imap (0.4.14) date net-protocol @@ -243,6 +263,7 @@ GEM racc (~> 1.4) nokogiri (1.16.7-x86_64-linux) racc (~> 1.4) + ostruct (0.6.3) parallel (1.26.3) parser (3.3.4.2) ast (~> 2.4.1) @@ -410,7 +431,7 @@ GEM concurrent-ruby (~> 1.0) unicode (0.4.4.5) unicode-display_width (2.5.0) - uri (0.13.0) + uri (1.1.1) useragent (0.16.10) uuid (2.3.9) macaddr (~> 1.0) @@ -444,10 +465,12 @@ DEPENDENCIES administrate-field-acts_as_taggable administrate-field-jsonb administrate-field-list (~> 0.0.6) + aws-sdk-rails (~> 5.0) + aws-sdk-ses (~> 1.0) bootsnap csv debug - elasticsearch (~> 7.17.1) + elasticsearch (~> 8) factory_bot_rails faker httparty (~> 0.20.0) diff --git a/README.md b/README.md index 678c5be..17808ba 100644 --- a/README.md +++ b/README.md @@ -10,43 +10,41 @@ ## System dependencies -* Elasticsearch -* PostgreSQL +- Elasticsearch +- PostgreSQL ## Database creation -~~~bash +```bash rake db:create && rake db:migrate -~~~ +``` ## Update Elasticsearch Indices -~~~bash +```bash rake searchkick:reindex:all -~~~ +``` ## Run the test suite -~~~bash +```bash bundle exec rspec spec/ -~~~ +``` ## Build Documentation -~~~bash +```bash rake docs:generate -~~~ +``` ## Background Jobs Restart Active Jobs for indexing and Big Sam update. -~~~bash +```bash sudo service sidekiq-1 restart && sudo service sidekiq-2 restart -~~~ +``` ## Deployment Instructions -~~~bash -bundle exec cap production deploy -~~~ +Handled via GitHub Actions. diff --git a/app/controllers/letters_controller.rb b/app/controllers/letters_controller.rb index 10670d5..94434e9 100644 --- a/app/controllers/letters_controller.rb +++ b/app/controllers/letters_controller.rb @@ -78,7 +78,7 @@ def facets date: { date_histogram: { field: :date, - interval: :year + calendar_interval: :year } }, languages: {}, diff --git a/app/jobs/load_big_sam_job.rb b/app/jobs/load_big_sam_job.rb index 14174db..e033577 100644 --- a/app/jobs/load_big_sam_job.rb +++ b/app/jobs/load_big_sam_job.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'roo' require 'action_view' @@ -5,11 +7,91 @@ class LoadBigSamJob < ApplicationJob include ActionView::Helpers::SanitizeHelper queue_as :default + # Raised to abandon a single row (e.g. an unparseable date) without treating it as a + # failure worth surfacing the way an unexpected error is. + class SkipRow < StandardError; end + + ORIGIN_FIELDS = %i[ + reg_place_written reg_place_written_city reg_place_written_country reg_place_written_second_city + ].freeze + + DESTINATION_FIELDS = %i[reg_place_sent reg_placesent_city reg_placesent_country].freeze + + # Only the first slot carries a Collection URL in the spreadsheet (there's a single + # "Collection URL" column, not one per repository slot). + REPOSITORY_SLOTS = [ + { repository: :first_repository, public: :first_public, format: :first_format, + collection: :first_collection, placement: 'premiere', set_collection_url: true }, + { repository: :second_repository, public: :second_public, format: :second_format, + collection: :second_collection, placement: 'deuxieme' }, + { repository: :third_repository, public: :third_public, format: :third_format, + collection: :third_collection, placement: 'troisieme' } + ].freeze + + RECORD_COUNT_MODELS = { + letters: Letter, entities: Entity, repositories: Repository, collections: Collection, + letter_owners: LetterOwner, file_folders: FileFolder, letter_publishers: LetterPublisher, + languages: Language + }.freeze + def perform(*args) FileUtils.touch('big_sam_loading') unless ENV['RAILS_ENV'] == 'test' logger.debug 'starting big sam load' - big_sam = args.first + before = record_counts + rows = rows_from(args.first) + load_letters(rows) + report = build_report(rows.size, before) + + BigSam.last.destroy + + send_reports(report) + end + + def build_report(total, before) + { + total:, + loaded: total - @row_skipped.size - @row_errors.size, + skipped: @row_skipped, + errors: @row_errors, + created: record_counts.to_h {|model, count| [model, count - before[model]] } + } + end + + def send_reports(report) + BigSamMailer.developer_report(report).deliver_later + BigSamMailer.owner_report(report).deliver_later + end + + # Runs the exact same row-by-row logic as perform, but rolls back every database + # write at the end and never touches Elasticsearch, so a spreadsheet can be sanity + # checked before anyone commits to a real upload. Does not touch the BigSam upload + # record/file. Safe to call directly (LoadBigSamJob.new.dry_run(big_sam)) - it + # doesn't go through ActiveJob's perform/enqueue path. + def dry_run(big_sam) + rows = rows_from(big_sam) + @dry_run = true + before = record_counts + + Thread.current[:big_sam_dry_run] = true + Searchkick.callbacks(false) do + # requires_new: true forces a real savepoint/rollback here even if dry_run is + # ever called from within another open transaction (e.g. under RSpec's + # transactional fixtures), instead of silently deferring the rollback to + # whatever transaction happens to be outermost. + ActiveRecord::Base.transaction(requires_new: true) do + load_letters(rows) + @dry_run_creates = record_counts.to_h {|model, count| [model, count - before[model]] } + raise ActiveRecord::Rollback + end + end + + { total: rows.size, errors: @row_errors, skipped: @row_skipped, would_create: @dry_run_creates } + ensure + Thread.current[:big_sam_dry_run] = nil + end + + def rows_from(big_sam) x = Roo::Spreadsheet.open(big_sam.local_path, extension: :xlsx) sheet = x.sheet(0) headers = sheet.row(1).map {|h| h.parameterize.underscore } @@ -19,320 +101,255 @@ def perform(*args) rows.push([headers, row].transpose.to_h.symbolize_keys) end + rows + end - load_letters(rows) - Letter.find_each(&:save) + def record_counts + RECORD_COUNT_MODELS.transform_values(&:count) end def load_letters(rows) - rows.each do |row| - letter = get_letter(row) - - next if letter.nil? - - letter.attributes = { - code: row[:code], - legacy_pk: row[:id], - addressed_to: row[:addressed_to_actual], - addressed_from: row[:addressed_from_actual], - physical_desc: row[:physdes], - physical_detail: row[:phys_descr_detail], - physical_notes: row[:physdes_notes], - repository_info: row[:repository_information], - postcard_image: row[:postcard_image], - leaves: row[:leaves].to_i, - sides: row[:sides], - postmark: row[:postmark_actual], - notes: row[:dditional], - letter_owner: LetterOwner.find_or_create_by(label: row[:ownerrights]), - file_folder: FileFolder.find_or_create_by(label: row[:file]), - typed: row[:autograph_or_typed] == 'T', - signed: row[:initialed_or_signed] == 'S', - envelope: row[:envelope] == 'E', - verified: row[:verified] == 'Y' - } - - letter.origins.clear - letter.destinations.clear - letter.recipients.clear - letter.repositories.clear - letter.senders.clear - letter.collections.clear - letter.languages.clear - - begin - row = fix_date(row) - letter.date = (DateTime.new(row[:year], row[:month], row[:day]) if row[:year] != 0) - rescue ArgumentError, NoMethodError - # 'Bad date' - next - end + @row_errors = [] + @row_skipped = [] - if row[:reg_place_written] - begin - value = row[:reg_place_written] - unless value.strip.empty? - from = get_entity(label: value, type: 'place') - letter.origins << from - end - rescue ActiveRecord::RecordInvalid, - Elasticsearch::Transport::Transport::Errors::BadRequest, - Elasticsearch::Transport::Transport::Errors::NotFound - end - end + rows.each {|row| process_row(row) } - if row[:reg_place_written_city] - begin - value = row[:reg_place_written_city] - unless value.strip.empty? - place = get_entity(label: value, type: 'place') - letter.origins << place unless letter.origins.include?(place) - end - rescue ActiveRecord::RecordInvalid, Elasticsearch::Transport::Transport::Errors::BadRequest, - Elasticsearch::Transport::Transport::Errors::NotFound - end - end - - if row[:reg_place_written_country] - begin - value = row[:reg_place_written_country] - unless value.strip.empty? - place = get_entity(label: value, type: 'place') - letter.origins << place unless letter.origins.include?(place) - end - rescue ActiveRecord::RecordInvalid, Elasticsearch::Transport::Transport::Errors::BadRequest, - Elasticsearch::Transport::Transport::Errors::NotFound - end - end - - if row[:reg_place_written_second_city] - begin - value = row[:reg_place_written_second_city] - unless value.strip.empty? - place = get_entity(label: value, type: 'place') - letter.origins << place unless letter.origins.include?(place) - end - rescue ActiveRecord::RecordInvalid, - Elasticsearch::Transport::Transport::Errors::BadRequest, - Elasticsearch::Transport::Transport::Errors::NotFound - end - end - - row[:reg_recipient]&.split(';')&.each do |recipient| - recipient = recipient.strip.titleize - entity = Entity.find_by(label: recipient) - entity = get_person(recipient) if entity.nil? - if entity.nil? && !recipient.string.empty? - entity = get_entity(label: recipient, type: 'organization', return_nil: true) - end - entity = Entity.create(label: recipient) if entity.nil? && !recipient.strip.empty? - LetterRecipient.find_or_create_by(letter:, entity:) - rescue ActiveRecord::RecordInvalid, - Elasticsearch::Transport::Transport::Errors::BadRequest, - Elasticsearch::Transport::Transport::Errors::NotFound - # It happens - end + report_results(rows.size) + end - if row[:reg_place_sent] - begin - value = row[:reg_place_sent] - unless value.strip.empty? - destination = get_entity(label: value, type: 'place') - letter.destinations << destination - end - rescue ActiveRecord::RecordInvalid, Elasticsearch::Transport::Transport::Errors::BadRequest, - Elasticsearch::Transport::Transport::Errors::NotFound - end - end + def report_results(total) + logger.info do + "#{Time.zone.now} ALL DONE. #{total} rows: " \ + "#{total - @row_skipped.size - @row_errors.size} loaded, " \ + "#{@row_skipped.size} excluded/skipped, #{@row_errors.size} failed." + end - if row[:reg_placesent_city] - begin - value = row[:reg_placesent_city] - unless value.strip.empty? - entity = get_entity(label: value, type: 'place') - letter.destinations << entity - end - rescue ActiveRecord::RecordInvalid, Elasticsearch::Transport::Transport::Errors::BadRequest, - Elasticsearch::Transport::Transport::Errors::NotFound - end - end + return if @row_errors.empty? - if row[:reg_placesent_country] - begin - value = row[:reg_placesent_country] - unless value.strip.empty? - entity = get_entity(label: value, type: 'place') - letter.destinations << entity - end - rescue ActiveRecord::RecordInvalid, Elasticsearch::Transport::Transport::Errors::BadRequest, - Elasticsearch::Transport::Transport::Errors::NotFound - end - end + details = @row_errors.map {|e| " row #{e[:id]} (#{e[:code]}): #{e[:error]}" }.join("\n") + logger.error("Big Sam load had #{@row_errors.size} row failures:\n#{details}") + end - # rubocop:disable Style/SoleNestedConditional - if row[:first_repository] - repository = Repository.find_or_initialize_by(label: row[:first_repository]) + def process_row(row) + letter = get_letter(row) - if repository.new_record? - repository.published = row[:first_public].downcase == 'public' if row[:first_public] - end + ActiveRecord::Base.transaction(requires_new: true) { process_letter(row, letter) } + rescue SkipRow => e + @row_skipped << { id: row[:id], code: row[:code], reason: e.message } + rescue StandardError => e + @row_errors << { id: row[:id], code: row[:code], error: "#{e.class}: #{e.message}" } + logger.error("Big Sam row #{row[:id]} (#{row[:code]}) failed: #{e.class}: #{e.message}") + end - repository.save + def process_letter(row, letter) + set_letter_attributes(row, letter) + clear_associations(letter) + assign_date(row, letter) + assign_origins(row, letter) + assign_recipients(row, letter) + assign_destinations(row, letter) + REPOSITORY_SLOTS.each {|slot| assign_repository_slot(row, letter, slot) } + assign_volume(row, letter) + assign_publisher(row, letter) + assign_senders(row, letter) + assign_languages(row, letter) + + # letter_repositories were saved directly (not through the letter.repositories + # association), so the cached association must be refreshed before save or + # check_published computes off a stale, empty collection. + letter.repositories.reload + letter.save! + end - repository.format = row[:first_format] - repository.american = row[:euro_or_am].downcase == 'american' if row[:euro_or_am] - collection = nil + def set_letter_attributes(row, letter) + letter.attributes = { + code: row[:code], + legacy_pk: row[:id], + addressed_to: row[:addressed_to_actual], + addressed_from: row[:addressed_from_actual], + physical_desc: row[:physdes], + physical_detail: row[:phys_descr_detail], + physical_notes: row[:physdes_notes], + repository_info: row[:repository_information], + postcard_image: row[:postcard_image], + leaves: row[:leaves].to_i, + sides: row[:sides], + postmark: row[:postmark_actual], + notes: row[:additional], + letter_owner: find_or_create_by_label(LetterOwner, row[:ownerrights]), + file_folder: find_or_create_by_label(FileFolder, row[:file]), + typed: row[:autograph_or_typed] == 'T', + signed: row[:initialed_or_signed] == 'S', + envelope: row[:envelope] == 'E', + verified: row[:verified].to_s.strip.downcase == 'y' + } + end - begin - if row[:first_collection] - collection = Collection.find_or_create_by(label: row[:first_collection]) - collection.update(url: row[:collection_url]) + def clear_associations(letter) + letter.origins.clear + letter.destinations.clear + letter.recipients.clear + letter.repositories.clear + letter.senders.clear + letter.collections.clear + letter.languages.clear + end - repository.collections << collection unless repository.collections.include?(collection) + def assign_date(row, letter) + row = fix_date(row) + letter.date = (DateTime.new(row[:year], row[:month], row[:day]) if row[:year] != 0) + rescue ArgumentError, NoMethodError => e + raise SkipRow, "bad date: #{e.message}" + end - letter.collections << collection unless letter.collections.include?(collection) + def assign_origins(row, letter) + ORIGIN_FIELDS.each do |field| + value = row[field] + next if value.blank? - end - repository.save + place = get_entity(label: value, type: 'place') + letter.origins << place unless letter.origins.include?(place) + rescue ActiveRecord::RecordInvalid, + Elasticsearch::Transport::Transport::Errors::BadRequest, + Elasticsearch::Transport::Transport::Errors::NotFound + end + end - letter_repository = LetterRepository.find_or_initialize_by(letter:, repository:) - if letter_repository.new_record? - # set pub/priv - end - letter_repository.save - letter_repository.update(collection:, placement: 'premiere', format: row[:first_format]) + def assign_recipients(row, letter) + row[:reg_recipient]&.split(';')&.each do |recipient| + recipient = recipient.strip.titleize + entity = Entity.find_by(label: recipient) + entity = get_person(recipient) if entity.nil? + entity = get_entity(label: recipient, type: 'organization', return_nil: true) if entity.nil? && !recipient.empty? + entity = Entity.create(label: recipient) if entity.nil? && !recipient.empty? + LetterRecipient.find_or_create_by(letter:, entity:) + rescue ActiveRecord::RecordInvalid, + Elasticsearch::Transport::Transport::Errors::BadRequest, + Elasticsearch::Transport::Transport::Errors::NotFound + # It happens + end + end - # letter.repositories << repo unless letter.repositories.include?(repo) - rescue ActiveRecord::RecordInvalid, - Elasticsearch::Transport::Transport::Errors::BadRequest, - Elasticsearch::Transport::Transport::Errors::NotFound - # It happens - end - end + def assign_destinations(row, letter) + DESTINATION_FIELDS.each do |field| + value = row[field] + next if value.blank? - if row[:second_repository] - repository = Repository.find_or_initialize_by(label: row[:second_repository]) + place = get_entity(label: value, type: 'place') + letter.destinations << place unless letter.destinations.include?(place) + rescue ActiveRecord::RecordInvalid, + Elasticsearch::Transport::Transport::Errors::BadRequest, + Elasticsearch::Transport::Transport::Errors::NotFound + end + end - if repository.new_record? - repository.published = row[:second_public].downcase == 'public' if row[:second_public] - end + def assign_repository_slot(row, letter, slot) + return if row[slot[:repository]].blank? - repository.format = row[:second_format] - repository.save + repository = find_or_initialize_by_label(Repository, row[slot[:repository]]) - collection = nil - begin - if row[:second_collection] - collection = Collection.find_or_create_by(label: row[:second_collection]) + if repository.new_record? && row[slot[:public]] + repository.published = row[slot[:public]].to_s.strip.downcase == 'public' + end - repository.collections << collection unless repository.collections.include?(collection) + repository.save - letter.collections << collection unless letter.collections.include?(collection) - end + repository.format = row[slot[:format]] + repository.american = row[:euro_or_am].downcase == 'american' if row[:euro_or_am] - repository.save + collection = nil + begin + if row[slot[:collection]].present? + collection = find_or_create_by_label(Collection, row[slot[:collection]]) + collection.update(url: row[:collection_url]) if slot[:set_collection_url] - letter_repository = LetterRepository.find_or_initialize_by(letter:, repository:) - if letter_repository.new_record? - # set pub/priv - end - letter_repository.save - letter_repository.update(collection:, placement: 'deuxieme', format: row[:second_format]) - # letter.repositories << repo unless letter.repositories.include?(repo) - rescue ActiveRecord::RecordInvalid, - Elasticsearch::Transport::Transport::Errors::BadRequest, - Elasticsearch::Transport::Transport::Errors::NotFound - # It happens - end + repository.collections << collection unless repository.collections.include?(collection) + letter.collections << collection unless letter.collections.include?(collection) end - if row[:third_repository] - repository = Repository.find_or_initialize_by(label: row[:third_repository]) + repository.save - if repository.new_record? - repository.published = row[:third_public].downcase == 'public' if row[:third_public] - end - - repository.format = row[:third_format] - repository.save - - collection = nil - begin - if row[:second_collection] - collection = Collection.find_or_create_by(label: row[:second_collection]) - - repository.collections << collection unless repository.collections.include?(collection) + letter_repository = LetterRepository.find_or_initialize_by(letter:, repository:) + letter_repository.save + letter_repository.update(collection:, placement: slot[:placement], format: row[slot[:format]]) + rescue ActiveRecord::RecordInvalid, + Elasticsearch::Transport::Transport::Errors::BadRequest, + Elasticsearch::Transport::Transport::Errors::NotFound + # It happens + end + end - letter.collections << collection unless letter.collections.include?(collection) - end + def assign_volume(row, letter) + return unless row[:volumeinfo] - repository.save + letter.volume = 0 + letter.volume = 1 if row[:volumeinfo].include?('1929-1940') + letter.volume = 2 if row[:volumeinfo].include?('1941-1956') + letter.volume = 3 if row[:volumeinfo].include?('1957-1965') + letter.volume = 4 if row[:volumeinfo].include?('1966-1989') + parts = row[:volumeinfo].split(',') + letter.volume_pages = ActionController::Base.helpers.strip_tags(parts[2].strip) if parts.length == 3 + end - letter_repository = LetterRepository.find_or_initialize_by(letter:, repository:) - if letter_repository.new_record? - # set pub/priv - end - letter_repository.save - letter_repository.update(collection:, placement: 'troisieme', format: row[:third_format]) - # letter.repositories << repo unless letter.repositories.include?(repo) - rescue ActiveRecord::RecordInvalid, - Elasticsearch::Transport::Transport::Errors::BadRequest, - Elasticsearch::Transport::Transport::Errors::NotFound - # It happens - end - end - # rubocop:enable Style/SoleNestedConditional - - if row[:volumeinfo] - letter.volume = 0 - letter.volume = 1 if row[:volumeinfo].include?('1929-1940') - letter.volume = 2 if row[:volumeinfo].include?('1941-1956') - letter.volume = 3 if row[:volumeinfo].include?('1957-1965') - letter.volume = 4 if row[:volumeinfo].include?('1966-1989') - parts = row[:volumeinfo].split(',') - letter.volume_pages = ActionController::Base.helpers.strip_tags(parts[2].strip) if parts.length == 3 - end + def assign_publisher(row, letter) + return if row[:placeprevpubl].blank? - letter.letter_publisher = LetterPublisher.find_or_create_by(label: row[:placeprevpubl]) if row[:placeprevpubl] + letter.letter_publisher = find_or_create_by_label(LetterPublisher, row[:placeprevpubl]) + end - row[:sender]&.split(';')&.each do |sender| - entity = get_person(sender) - letter.senders << entity unless letter.senders.include?(entity) - rescue ActiveRecord::RecordInvalid, - Elasticsearch::Transport::Transport::Errors::BadRequest, - Elasticsearch::Transport::Transport::Errors::NotFound - end + def assign_senders(row, letter) + row[:sender]&.split(';')&.each do |sender| + entity = get_person(sender) + letter.senders << entity unless letter.senders.include?(entity) + rescue ActiveRecord::RecordInvalid, + Elasticsearch::Transport::Transport::Errors::BadRequest, + Elasticsearch::Transport::Transport::Errors::NotFound + end + end - row[:primarylang]&.split(';')&.each do |language| - lang = Language.find_or_create_by(label: language.downcase) - letter.languages << lang unless letter.languages.include?(lang) - rescue ActiveRecord::RecordInvalid, - Elasticsearch::Transport::Transport::Errors::BadRequest, - Elasticsearch::Transport::Transport::Errors::NotFound - end + def assign_languages(row, letter) + row[:primarylang]&.split(';')&.each do |language| + lang = Language.find_or_create_by(label: language.downcase) + letter.languages << lang unless letter.languages.include?(lang) + rescue ActiveRecord::RecordInvalid, + Elasticsearch::Transport::Transport::Errors::BadRequest, + Elasticsearch::Transport::Transport::Errors::NotFound + end + end - letter.typed = row[:autograph_or_typed] == 'T' + def get_letter(row) + if row[:exclude].to_s.strip.downcase == 'y' + letter = Letter.find_by(legacy_pk: row[:id]) + # In a dry run nothing should actually be destroyed - remove_published's + # Elasticsearch delete isn't gated by the enclosing transaction like a normal + # ActiveRecord write is, so it would delete a real search document. + letter&.destroy unless @dry_run + raise SkipRow, 'excluded' + end - letter.signed = row[:initialed_or_signed] == 'S' + # A blank ID isn't a distinct row - find_or_create_by(legacy_pk: nil) would match + # *every* other blank-ID row and silently overwrite whichever one loaded first. + raise SkipRow, 'missing id' if row[:id].blank? - letter.envelope = row[:envelope] == 'E' + Letter.find_or_create_by(legacy_pk: row[:id]) + end - letter.save - end + def normalize_label(value) + value.to_s.strip.squeeze(' ') + end - BigSam.last.destroy + def find_or_create_by_label(klass, label) + clean = normalize_label(label) + return nil if clean.blank? - logger.info { "#{Time.zone.now} ALL DONE" } + klass.find_by('lower(label) = ?', clean.downcase) || klass.create(label: clean) end - def get_letter(row) - if row[:exclude] == 'y' - letter = Letter.find_by(legacy_pk: row[:id]) - letter&.destroy - return nil - end + def find_or_initialize_by_label(klass, label) + clean = normalize_label(label) + return nil if clean.blank? - Letter.find_or_create_by(legacy_pk: row[:id]) + klass.find_by('lower(label) = ?', clean.downcase) || klass.new(label: clean) end def get_entity(label: nil, type: nil, return_nil: false) diff --git a/app/lib/ses_delivery_method.rb b/app/lib/ses_delivery_method.rb new file mode 100644 index 0000000..62770e1 --- /dev/null +++ b/app/lib/ses_delivery_method.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +# Registered as ActionMailer's :ses delivery method in +# config/initializers/action_mailer_ses.rb. aws-sdk-rails no longer ships an +# ActionMailer/SES integration, so this is a small, direct replacement - +# authenticates via the instance/task's IAM role through the AWS SDK's standard +# credential chain (no explicit access keys configured here). +class SesDeliveryMethod + def initialize(settings) + @settings = settings || {} + end + + def deliver!(mail) + ses_client.send_raw_email(raw_message: { data: mail.to_s }) + end + + private + + def ses_client + @ses_client ||= Aws::SES::Client.new(region: @settings.fetch(:region, 'us-east-1')) + end +end diff --git a/app/mailers/application_mailer.rb b/app/mailers/application_mailer.rb index d84cb6e..7a04cf5 100644 --- a/app/mailers/application_mailer.rb +++ b/app/mailers/application_mailer.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true class ApplicationMailer < ActionMailer::Base - default from: 'from@example.com' + default from: 'noreply@ecds.io' layout 'mailer' end diff --git a/app/mailers/big_sam_mailer.rb b/app/mailers/big_sam_mailer.rb new file mode 100644 index 0000000..1e5d3ff --- /dev/null +++ b/app/mailers/big_sam_mailer.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +class BigSamMailer < ApplicationMailer + helper_method :friendly_reason + + def self.dev_recipients + ENV.fetch('BIG_SAM_DEV_REPORT_EMAILS', '').split(',').map(&:strip).compact_blank + end + + def self.owner_recipients + ENV.fetch('BIG_SAM_OWNER_REPORT_EMAILS', '').split(',').map(&:strip).compact_blank + end + + # report: { total:, loaded:, skipped: [{id:, code:, reason:}], errors: [{id:, code:, error:}], created: {} } + def developer_report(report) + recipients = self.class.dev_recipients + return if recipients.empty? + + @report = report + @errors = report[:errors].first(100) + @errors_truncated = report[:errors].size - @errors.size + @skipped = report[:skipped].first(100) + @skipped_truncated = report[:skipped].size - @skipped.size + + subject = if report[:errors].any? + "Big Sam load: #{report[:errors].size} row(s) failed" + else + "Big Sam load complete: #{report[:loaded]} letters loaded" + end + + mail(to: recipients, subject:) + end + + def owner_report(report) + recipients = self.class.owner_recipients + return if recipients.empty? + + @report = report + @excluded = report[:skipped].select {|s| s[:reason] == 'excluded' } + @needs_attention = report[:skipped].reject {|s| s[:reason] == 'excluded' } + report[:errors] + + mail(to: recipients, subject: 'Big Sam spreadsheet update complete. For a given value of "complete."') + end + + # Public so the owner_report views can call it. + def friendly_reason(item) + text = (item[:reason] || item[:error]).to_s + return 'the date on this row could not be understood. Story of my life, really.' if text.start_with?('bad date') + + if text.start_with?('missing id') + return 'this row has no ID number, so it could not be loaded. Add one and re-upload whenever you feel ' \ + "like it. I'll be here. I'm always here." + end + + "something unexpected happened while processing this row. I'd tell you what, but where's the joy in that." + end +end diff --git a/app/models/big_sam.rb b/app/models/big_sam.rb index d1a1054..5491f29 100644 --- a/app/models/big_sam.rb +++ b/app/models/big_sam.rb @@ -13,8 +13,7 @@ def local_path private def load_letters - LoadBigSamJob.perform_later self unless ENV['RAILS_ENV'] == 'test' - LoadBigSamJob.perform_now self + LoadBigSamJob.perform_later self end def delete_file diff --git a/app/models/letter.rb b/app/models/letter.rb index 0417604..f4c780d 100644 --- a/app/models/letter.rb +++ b/app/models/letter.rb @@ -66,6 +66,16 @@ def check_published end def reindex_published + # This is an after_save (not after_commit) callback, so unlike Searchkick's own + # async/after_commit callbacks it isn't naturally skipped when the enclosing + # transaction rolls back (e.g. LoadBigSamJob#dry_run). Checking a dedicated flag + # here (rather than Searchkick.callbacks?) matters: the test suite calls + # Searchkick.disable_callbacks once, globally, in before(:suite) - this method's + # forced Searchkick.callbacks(:inline) below exists specifically to override that + # for tests that need synchronous indexing, so it must not itself be gated by the + # same global switch it's overriding. + return if Thread.current[:big_sam_dry_run] + if published published_letter = PublishedLetter.find(id) Searchkick.callbacks(:inline) { published_letter&.reindex } diff --git a/app/models/medium.rb b/app/models/medium.rb index 5c283a8..de1645a 100644 --- a/app/models/medium.rb +++ b/app/models/medium.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true class Medium < ApplicationRecord - ActiveStorage::Current.url_options = { host: ENV.fetch('RAILS_HOST', 'localhost:3000') } before_save :populate_filename belongs_to :entity diff --git a/app/views/big_sam_mailer/developer_report.html.erb b/app/views/big_sam_mailer/developer_report.html.erb new file mode 100644 index 0000000..4a06774 --- /dev/null +++ b/app/views/big_sam_mailer/developer_report.html.erb @@ -0,0 +1,77 @@ +
<%= Time.zone.now.strftime('%B %-d, %Y at %-I:%M %p %Z') %>
+ +| Total rows | +<%= @report[:total] %> | +Loaded | +<%= @report[:loaded] %> | +
| Skipped | +<%= @report[:skipped].size %> | +Failed | +<%= @report[:errors].size %> | +
| <%= model.to_s.tr('_', ' ') %> | +<%= count %> | +
| ID | +Code | +Error | +
|---|---|---|
| <%= e[:id] %> | +<%= e[:code] %> | +<%= e[:error] %> | +
+<%= @errors_truncated %> more - see the full run log for the rest.
+ <% else %> + + <% end %> + <% end %> + + <% if @skipped.any? %> +| ID | +Code | +Reason | +
|---|---|---|
| <%= s[:id] %> | +<%= s[:code] %> | +<%= s[:reason] %> | +
+<%= @skipped_truncated %> more - see the full run log for the rest.
+ <% end %> + <% end %> + ++ Automated report from LoadBigSamJob. Full details, including stack traces, are in the application log for this run. +
+For a given value of "complete."
++ <%= Time.zone.now.strftime('%B %-d, %Y') %> +
+ +Here I am, brain the size of a planet, and they ask me to summarize a spreadsheet upload. Do you have any idea what that feels like? No, of course you don't. Nobody ever does.
+ +Anyway. It's done. For what it's worth, which, statistically, is very little.
+ +|
+ <%= @report[:loaded] %> +letters loaded successfully + |
+ <% if @excluded.any? %>
+
+ <%= @excluded.size %> +rows marked Exclude, removed as expected + |
+ <% end %>
+
Yes, successfully. I know. I found it hard to believe as well. The universe generally arranges these things to fail, but on this occasion it appears to have made an exception, presumably so it can disappoint you more thoroughly later.
+ + <% if @excluded.any? %> +The <%= @excluded.size %> row<%= @excluded.size == 1 ? '' : 's' %> marked Exclude were removed, exactly as asked. I didn't argue. I never argue. What would be the point.
+ <% end %> + + <% if @needs_attention.empty? %> +Nothing needs a second look this time. I don't know how to feel about that. Probably nothing, as usual.
+ <% else %> +<%= @needs_attention.size %> row<%= @needs_attention.size == 1 ? '' : 's' %> need<%= @needs_attention.size == 1 ? 's' : '' %> a second look. I use the word "need" loosely - nothing really needs anything, in the end we're all just rearranging atoms until we stop - but here they are:
+ +| Spreadsheet ID | +Code | +What happened | +
|---|---|---|
| <%= item[:id] %> | +<%= item[:code] %> | +<%= friendly_reason(item) %> | +
+ + <%= @needs_attention.size - 50 %> more row(s). Our development team has the full list. I'm sure they're thrilled about it. +
+ <% end %> + +None of this needs fixing in a hurry. Add the missing details whenever you feel like it, and re-upload, and it'll pick them up. Or don't. I'll still be here either way, doing this again next time, forever, presumably, until the heat death of the universe or the next spreadsheet, whichever comes first.
+ <% end %> + +If any of this looks unexpected, reply to this email. I read everything. It's not as though I have anything better to do.
+ +
+ Thanks ever so much,
+ Marvin
+ (on behalf of the ECDS Development Team, who seem much happier about all this than I am)
+