Skip to content

Commit

Permalink
🐛 [I951] false object import bug (#952)
Browse files Browse the repository at this point in the history
* 🐛 Add guard to prevent method calls on newly created object

Imports were failing for active fedora applications because ordered_memebers is an undefined method for FalseClass. object gets assigned by #find, which intentionally returns false for new objects; we can't find something that doesn't exist.

Issue:
- #951

* 🧹 refactor methods into a shareable module

* 🐛 fix ContentUpdateEventJob retries

ContentUpdateEventJob would fail and retry over and over again. This is because we were passing it a nil user.

Similar issue (previously fixed):
- #656
  • Loading branch information
ShanaLMoore authored Apr 22, 2024
1 parent 855ce39 commit bf7bc5c
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 16 deletions.
25 changes: 25 additions & 0 deletions app/concerns/loggable.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# frozen_string_literal: true

module Loggable
extend ActiveSupport::Concern

def log_created(obj)
log_action('Created', obj)
end

def log_updated(obj)
log_action('Updated', obj)
end

def log_deleted_fs(obj)
msg = "Deleted All Files from #{obj.id}"
Rails.logger.info("#{msg} (#{Array(obj.attributes[work_identifier]).first})")
end

private

def log_action(action, obj)
msg = "#{action} #{obj.class.model_name.human} #{obj.id}"
Rails.logger.info("#{msg} (#{Array(obj.attributes[work_identifier]).first})")
end
end
16 changes: 1 addition & 15 deletions app/factories/bulkrax/object_factory_interface.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ module Bulkrax
class ObjectFactoryInterface
extend ActiveModel::Callbacks
include DynamicRecordLookup
include Loggable

# We're inheriting from an ActiveRecord exception as that is something we
# know will be here; and something that the main_app will be expect to be
Expand Down Expand Up @@ -404,21 +405,6 @@ def add_user_to_collection_permissions(*args)
self.class.add_user_to_collection_permissions(**arguments)
end

def log_created(obj)
msg = "Created #{klass.model_name.human} #{obj.id}"
Rails.logger.info("#{msg} (#{Array(attributes[work_identifier]).first})")
end

def log_updated(obj)
msg = "Updated #{klass.model_name.human} #{obj.id}"
Rails.logger.info("#{msg} (#{Array(attributes[work_identifier]).first})")
end

def log_deleted_fs(obj)
msg = "Deleted All Files from #{obj.id}"
Rails.logger.info("#{msg} (#{Array(attributes[work_identifier]).first})")
end

private

def apply_depositor_metadata
Expand Down
6 changes: 5 additions & 1 deletion app/models/concerns/bulkrax/file_factory.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ def file_set_factory_inner_workings
end

class InnerWorkings
include Loggable

def initialize(object_factory:)
@object_factory = object_factory
end
Expand Down Expand Up @@ -119,7 +121,7 @@ def import_files_filenames
def destroy_existing_files
return unless object.present? && object.file_sets.present?
object.file_sets.each do |fs|
Hyrax::Actors::FileSetActor.new(fs, @user).destroy
Hyrax::Actors::FileSetActor.new(fs, user).destroy
end
@object = object.reload
log_deleted_fs(object)
Expand Down Expand Up @@ -155,6 +157,8 @@ def local_file_sets
end

def ordered_file_sets
return [] if object.blank?

Bulkrax.object_factory.ordered_file_sets_for(object)
end

Expand Down

0 comments on commit bf7bc5c

Please sign in to comment.