Clarify Embargo/Lease Terminology

[no-reply]

Descriptive summary

Embargoes and Leases use unclear and inconsistent terminology to describe their current and future states. Some important states are completely unnamed and are checked in client code via presence of *_release_date (which it might actually be nice to keep around).

We have:

• #active?(Boolean) which indicates whether a release date is in the future.
• #deactivate! which releases(?) the embargo, and should usually be called when #active? is already false!
• embargo/lease_release_date, hence "release", which is an action that's never taken, but is maybe what's meant by "deactivate"? (edit: oh, and also "expiration" as in #lease_expiration_date and (in Hyrax) Hyrax::EmbargoHelper#assets_with_expired_embargoes.)
• during_embargo/lease which seems to mean "until deactivate is called"
• after_embargo/lease which seems to mean "after deactivate is called"

I might try to draw a state diagram for this later.

In the meanwhile there's an important, unnamed state when the release date has passed but has not been deactivated (under present terminology (maybe) the embargo, is "inactive", but not "deactivated"). The only way to tell whether this is the case is to inspect embargo/lease_release_date and infer that, because it is present the embargo is still... ummmm... not-deactivated.

Rationale

The current state of affairs makes things confusing, even for experienced Samvera devs (:wave:)

Related work

samvera/hyrax#4299 (comment)

[no-reply]

there's also Embargoable#under_embargo? and Embargoable#active_lease? which further muddy the naming waters.

hydra-head/hydra-access-controls/app/models/concerns/hydra/access_controls/embargoable.rb

Lines 51 to 57 in 4ffa873

	def under_embargo?
	embargo && embargo.active?
	end
	def active_lease?
	lease && lease.active?
	end

[jeremyf]

At one point, I was considering a "visibility schedule" abstraction. Fundamentally, we have an embargo. From beginning of time until the embargo date. And during that time frame there are "magic" viewer rights. Perhaps institution? Perhaps only explicitly assigned people. Similarly we have lease. From the begining of time until the lease date, it's open-ish. Then collapses into a dark state.

The logic is leaky AND if an institution gets it wrong there can be punitive consequences. As such, I believe we should spend some energy to design a more clearly articulated implementation.

[no-reply]

And during that time frame there are "magic" viewer rights.

i don't think this is true. Embargo/Lease function as data objects that are used to manage editing ACLs (recall that "visibility" is just a less granular version of ACLs; see also: https://github.com/samvera/hyrax/wiki/Hyrax-Valkyrie-Usage-Guide#permissions). there aren't/shouldn't be any magic permissions, and AFAICT it is totally possible for an embargo to be "active" but for the intended permissions to not be applied.

---

i might suggest a cleaned up version of what exists:

• Embargo/Lease (visibility schedule) has:
  • start date (oh, could we please have a start date?)
  • end date (we have this now)
  • active/applied permissions (maybe an ACL template? or as currently, just a visibility term?)
  • base(?) permissions (same as above but for the "reset" permissions; currently: "visibility after *")
  • an #apply method to set the "active/applied" permissions, normally called at start date.
  • a #revoke method to reset to the base permissions, normally called at end date
  • #current?: is the date now between the start and end dates
  • #active?: are the "active/applied" permissions currently enforced on the object?

all names can probably be refined.

under this model, we have the following states

current?	active?	appropriate action	notes
true	true	🆗	this might be summarized as #enforced?
false	true	#release	this is the state currently represented in the UI using #present? checks, maybe #needs_release?
true	false	#apply	danger zone. objects in this state likely have ACLs in violation of policy
false	false	🆗

---

note that there's very little difference in implementation between the embargo and lease classes as-is. both are more-or-less a set of permissions "during", a set of permissions "after", and a release date, and there's nothing about the data structure that stops an embargo's "during" permissions from being looser than its "after" permissions; i.e. an embargo can be effectively a lease, and vice versa. simplifying to one implementation for these two concepts seems good.