Change embargo/lease ID types to fix nil values being converted to blank strings

[randalldfloyd]

Fixes

Fixes #6650 ; refs #6521

Summary

When using a Valkyrie adapter, the attribute type for embargo_id and lease_id is currently Valkyrie::Types::ID. Because of the behavior of that type, if those attributes get set to a blank string, they will get persisted as Valkyrie::ID @id="" . This is problematic when round-tripping them through URI/ID conversions in the fedora adapter (see Description section.) This PR changes to Valkyrie::Types::Params::ID, because blank strings are always returned as nil, whereas Valkyrie::Types::ID returns exactly what it is given, and a blank string is still a valid string.

Guidance for testing, such as acceptance criteria or new user interface behaviors:

In a rails console (i.e. in sirenia), create a work and set a blank string on embargo_id:

persister = Hyrax.persister
resource = Monograph.new
resource.embargo_id = ""
monograph1 = persister.save(resource: resource)

Make sure resulting embargo_id is still nil, per the behavior of Valkyrie::Types::Params::ID

monograph1.embargo_id 
=> nil

Detailed Description

Per the summary, this is a problem when/if IDs like embargo_id and lease_id are ever changed from nil to a blank string. But why would we want to do that? We don't, but it is consistently demonstrable when a work is related to an admin set with a one-step workflow. It's not clear where/why this happens.

This isn't a problem for the postgres adapter, but for fedora, those IDs are stored as URIs. When those get ran through URI-to-ID conversion by the fedora metatdata adapter, it returns what's anchored at the end, which is just the fedora base path (i.e. development etc.) because there is no trailing ID. Conversely, when that value is ran through ID-to-URI conversion, it gets appended to the configured base path, so it ends up with http://fedora_url/base_path/base_path .

This introduces breakage because the embargo_id and lease_id is no longer nil or blank, so it triggers methods that it should have short-circuited, with invalid URIs that are guaranteed to produce Ldp::NotFound errors. This produces confusing behavior; in the case of #6521, the error came from loading the object for embargo_id when the work wasn't even under embargo.

The effect of changing the attribute type is that, even though something in Hyrax workflows is inappropriately trying to change nil values to a blank string, the Valkyrie::Types::Params::ID will convert them back to nil when persisted; that's what the type is designed for.

@samvera/hyrax-code-reviewers

[dlpierce]

Thanks for tracking this down!