Skip to content
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.

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

Cleans ID array attributes before persisting with pair-tree Valkyrie Fedora. #6884

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
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
7 changes: 7 additions & 0 deletions app/models/hyrax/change_set.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,5 +45,12 @@ class ChangeSet < Valkyrie::ChangeSet
def self.for(resource)
Hyrax::ChangeSet(resource.class).new(resource)
end

def clean_id_fields_before_sync
id_fields = fields.keys.select { |k| k.include?('_id') }
multiple_id_fields_with_empties = id_fields.select { |f| fields[f].is_a?(Array) && fields[f].any?(&:empty?) }

multiple_id_fields_with_empties.each { |f| fields[f] = fields[f].reject(&:empty?) }
end
end
end
1 change: 1 addition & 0 deletions lib/hyrax/transactions/steps/save.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ def call(change_set, user: nil)
valid_future_date?(change_set.lease, 'lease_expiration_date') if change_set.respond_to?(:lease) && change_set.lease
valid_future_date?(change_set.embargo, 'embargo_release_date') if change_set.respond_to?(:embargo) && change_set.embargo
new_collections = changed_collection_membership(change_set)
change_set.clean_id_fields_before_sync

unsaved = change_set.sync
save_lease_or_embargo(unsaved)
Expand Down
17 changes: 16 additions & 1 deletion spec/models/hyrax/change_set_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

RSpec.describe Hyrax::ChangeSet do
subject(:change_set) { described_class.for(resource) }
let(:resource) { build(:hyrax_work) }
let(:resource) { build(:hyrax_work, :with_default_admin_set) }
let(:titles) { ['comet in moominland', 'finn family moomintroll'] }

it_behaves_like 'a Hyrax::ChangeSet'
Expand Down Expand Up @@ -34,6 +34,21 @@
end
end

describe '#clean_id_fields_before_sync' do
# NOTE: A pair-tree'ed Fedora storage scheme requires the elimiation of any
# empty string values within a multivalued id attribute. If it passes along
# to a parser that takes any value present in an array and converts it into
# a Valkyrie::ID instance, Valkyrie's persister will attempt to pairtree that value.
it 'will eliminate empty string values from a change set' do
change_set.member_of_collection_ids = [""]

expect { change_set.clean_id_fields_before_sync }
.to change { change_set.member_of_collection_ids }
.from([""])
.to([])
end
end

describe '#sync' do
it 'applies changeset attributes' do
change_set.title = titles
Expand Down
Loading