Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
65 changes: 62 additions & 3 deletions app/factories/bulkrax/valkyrie_object_factory.rb
Original file line number Diff line number Diff line change
Expand Up @@ -335,9 +335,35 @@ def conditionally_set_reindex_extent
nil
end

# rubocop:disable Metrics/AbcSize
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we break this method up, rather than disabling the cop?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes by all means!

def create_file_set(attrs)
# TODO: Make it work for Valkyrie
raise NotImplementedError, __method__.to_s
work = find_record(attributes[related_parents_parsed_mapping].first, importer_run_id).last
attrs = clean_attrs(attrs)
file_set_attrs = attrs.slice(*object.attributes.keys) || {}

file = if attrs[:uploaded_files].present?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we could split the file-finding chunk into its own pair of methods? One for uploaded files and one for remote files?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup absolutely, this is just a quick draft to get started, feel free to change as much as you need

uploaded_file_id = attrs[:uploaded_files].first # currently assuming one file per FileSet
Hyrax::UploadedFile.find(uploaded_file_id)

return Hyrax.query_service.find_by(id: file.file_set_uri) if file.file_set_uri.present?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm confused - what is "file" here if we're setting it to the result of this method on line 344?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, what happens if file.file_set_uri.present? is false - won't it just return nil? Is that definitely what we want to do?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm on mobile so I can't see it all but I think file should be a Hyrax::UploadedFile

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah good question about the nil

elsif attrs[:remote_files].present?
remote_file = attrs[:remote_files].first # currently assuming one file per FileSet
file_path = download_file(remote_file["url"])

raise "Failed to download file from #{remote_file['url']}" unless file_path

create_uploaded_file(file_path, remote_file["file_name"])
end

file_set = Hyrax.persister.save(resource: object.class.new(file_set_args(file).merge(file_set_attrs)))
Hyrax.publisher.publish('object.deposited', object: file_set, user: file.user)
file.add_file_set!(file_set)

attach_to_work(file_set: file_set, work: work, attrs: attrs)

ValkyrieIngestJob.new(file).perform_now

file_set
end

def create_work(attrs)
Expand Down Expand Up @@ -498,7 +524,17 @@ def update_collection(attrs)
end

def update_file_set(attrs)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When does this update_file_set get called vs. the FileFactoryInnerWorkings#update_file_set version? Should we actually have both?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good question, I didn't know the other one existed

# TODO: Make it work
change_set = Hyrax::Forms::ResourceForm.for(resource: object)

attributes = clean_attrs(attrs)

result =
change_set.validate(attributes) && transactions['change_set.update_file_set']
.with_step_args(
'file_set.save_acl' => { permissions_params: change_set.input_params["permissions"] }
).call(change_set).value_or { false }

result if result
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if there's no result? Will the method just return nil? Is that what we really want?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup that looks like what it would do

end

def uploaded_local_files(uploaded_files: [])
Expand Down Expand Up @@ -578,6 +614,29 @@ def transform_attributes(update: false)
attrs[:creator] = [''] if attrs[:creator].blank?
attrs
end

def file_set_args(file)
{ depositor: file.user.user_key,
creator: file.user.user_key,
date_uploaded: file.created_at,
date_modified: Hyrax::TimeService.time_in_utc,
label: file.uploader.filename,
title: file.uploader.filename }
end

def attach_to_work(file_set:, work:, attrs:)
Hyrax::AccessControlList.copy_permissions(source: Hyrax::AccessControlList.new(resource: work), target: file_set)

file_set.visibility = attrs[:visibility] if attrs[:visibility].present?
file_set.permission_manager.acl.save if file_set.permission_manager.acl.pending_changes?

work.member_ids += [file_set.id]
work.representative_id = file_set.id if work.respond_to?(:representative_id) && work.representative_id.blank?
work.thumbnail_id = file_set.id if work.respond_to?(:thumbnail_id) && work.thumbnail_id.blank?
Hyrax.persister.save(resource: work)

Hyrax.publisher.publish('object.metadata.updated', object: work, user: @user)
end
end
# rubocop:enable Metrics/ClassLength
end
2 changes: 1 addition & 1 deletion app/models/bulkrax/csv_entry.rb
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ def add_metadata_for_model
if factory_class.present? && factory_class == Bulkrax.collection_model_class
add_collection_type_gid if defined?(::Hyrax)
# add any additional collection metadata methods here
elsif factory_class == Bulkrax.file_model_class
elsif factory_class == Bulkrax.file_model_class && @new_record
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if it's a FileSet and not a new record? Does it actually hit the update code?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it should since an update should only happen on an already existing record

validate_presence_of_filename!
add_path_to_file
validate_presence_of_parent!
Expand Down
Loading