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

6126 deactivated leases #6127

Merged
merged 57 commits into from
Aug 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
57 commits
Select commit Hold shift + click to select a range
6cc1544
decouple the deactivated lease work from the deactivated embargo work.
alishaevn Jul 14, 2023
7b19cca
move over a few more lease related changes from #6098.
alishaevn Jul 17, 2023
b8789d1
Merge branch 'main' into 5844-deactivated-leases
alishaevn Jul 25, 2023
42d504b
Merge branch 'main' into 5844-deactivated-leases
alishaevn Jul 26, 2023
ed8f13a
merge upstream changes
alishaevn Jul 26, 2023
d26467d
make the lease code in the resource and save files follow the updated…
alishaevn Jul 26, 2023
16cdbfa
wip: update more of the lease code to look like the embargo code so t…
alishaevn Jul 26, 2023
8061f33
embargo_release_date is now a DateTime, so we do not need to parse it.
alishaevn Jul 26, 2023
bf1b1eb
rubocop fixes
alishaevn Jul 26, 2023
d413323
revert Hyrax::LeaseManager#lease to what it was so that we can deacti…
alishaevn Jul 26, 2023
fac77c3
fix the Hyrax::LeaseManager comments
alishaevn Jul 26, 2023
0362456
update the specs to expect a DateTime for `lease.lease_expiration_date`
alishaevn Jul 26, 2023
c886e64
Merge branch 'main' into 6126-deactivated-leases
alishaevn Jul 28, 2023
0c86b0f
verify that the `lease_expiration_date` and `embargo_release_date` ar…
alishaevn Jul 28, 2023
1f979dc
Merge branch 'main' into 6126-deactivated-leases
alishaevn Jul 28, 2023
4b5b054
rubocop fixes
alishaevn Jul 28, 2023
6314351
only check for the embargo and lease, if they exist on the change_set…
alishaevn Jul 31, 2023
33ebb3d
allow for Wings::ModelTransformer unsaved leases to also have the cor…
alishaevn Jul 31, 2023
9975ac8
convert the embargo/lease DateTime to Date on the work/edit screen fo…
alishaevn Jul 31, 2023
3154b19
also make sure that pcdm_object.lease is available when working with …
alishaevn Jul 31, 2023
6a59d45
remove outdated comment
alishaevn Jul 31, 2023
5a8398f
rubocop
alishaevn Jul 31, 2023
9b64c54
disable validation on the Active Fedora object in Wings::Valkyrie::Pe…
alishaevn Aug 1, 2023
530750c
allow for spec/wings/active_fedora_converter_spec.rb:37 to pass.
alishaevn Aug 1, 2023
cfc5d91
skip spec/wings/active_fedora_converter_spec.rb:36 when using valkyri…
alishaevn Aug 1, 2023
cef578a
make sure that `af_object` responds to `lease` before fetching it ins…
alishaevn Aug 1, 2023
10215f0
update spec/services/hyrax/admin_set_create_service_spec.rb:294to exp…
alishaevn Aug 1, 2023
6b76967
we no longer need to set `embargo_history` and `lease_history` to emp…
alishaevn Aug 1, 2023
753d216
add a lease to the file_set if applicable
alishaevn Aug 1, 2023
82bb390
rubocop
alishaevn Aug 1, 2023
cee247a
more rubocop things
alishaevn Aug 1, 2023
5fdc19f
spec fixes related to lease time stamps
alishaevn Aug 2, 2023
ff52b95
try again to get this conditional right in the active_fedora_converter
alishaevn Aug 2, 2023
d315ec2
reindex the file set with the correct lease related properties when a…
alishaevn Aug 2, 2023
8ea729a
wip: developing a spec for Hyrax::ValkyrieFileSetIndexer#index_lease
alishaevn Aug 2, 2023
b3bd3cc
rubocop
alishaevn Aug 2, 2023
ad4f247
also remove the `:id` attribute when coercing the resource lease into…
alishaevn Aug 3, 2023
5c4486d
yet again...update #apply_attributes_to_model. make sure that the res…
alishaevn Aug 3, 2023
ef3f8e2
update the lease_actor_spec for the new lease deactivation process
alishaevn Aug 3, 2023
6c071cb
update the active_fedora_converter_spec example that expected the con…
alishaevn Aug 3, 2023
3c503bf
adjust Hyrax::WorksControllerBehavior#create_valkyrie_work error hand…
alishaevn Aug 3, 2023
45f586b
rubocop
alishaevn Aug 3, 2023
77f7ac0
update TODO in spec/controllers/hyrax/generic_works_controller_json_s…
alishaevn Aug 3, 2023
720b140
thought I deleted this comment already
alishaevn Aug 3, 2023
796a1f6
remove another comment
alishaevn Aug 3, 2023
9896d7a
conditionally validate Wings::Valkyrie::Persister#save.
alishaevn Aug 3, 2023
04c9c2e
also skip the Hyrax::Actors::CollectionsMembershipActor specs if Hyra…
alishaevn Aug 3, 2023
9d54048
add specs for Hyrax::ValkyrieFileSetIndexer#index_lease
alishaevn Aug 3, 2023
a1ea69c
better comments
alishaevn Aug 4, 2023
b2a9f23
able to add leases to newly added file sets too, not just preexisting…
alishaevn Aug 4, 2023
3e28096
more semantic method/parameter names and add param details
alishaevn Aug 4, 2023
8970a44
restore spec/services/hyrax/admin_set_create_service_spec.rb:294 to i…
alishaevn Aug 4, 2023
580d817
I am not sure what changed along the way....but we no longer need thi…
alishaevn Aug 4, 2023
04fac65
update Hyrax::Actors::BaseActor#save to pass the correct argument/par…
alishaevn Aug 4, 2023
b16bace
remove the no longer necessary rubocop disable comment from active_fe…
alishaevn Aug 4, 2023
f992e10
revert the changes from 8970a4460080f11a2cf93ed5dfa396a0f52526bd and …
alishaevn Aug 4, 2023
96f5719
converting a valkyrie resource lease into an active fedora lease is r…
alishaevn Aug 4, 2023
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: 4 additions & 3 deletions app/actors/hyrax/actors/base_actor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,8 @@ def apply_deposit_date(env)
def save(env, use_valkyrie: false)
return env.curation_concern.save unless use_valkyrie

resource = valkyrie_save(resource: env.curation_concern.valkyrie_resource)
# don't run validations again on the converted object if they've already passed
resource = valkyrie_save(resource: env.curation_concern.valkyrie_resource, is_valid: env.curation_concern.save)

# we need to manually set the id and reload, because the actor stack requires
# `env.curation_concern` to be the exact same instance throughout.
Expand Down Expand Up @@ -115,9 +116,9 @@ def multivalued_form_attributes(attributes)
attributes.select { |_, v| v.respond_to?(:select) && !v.respond_to?(:read) }
end

def valkyrie_save(resource:)
def valkyrie_save(resource:, is_valid:)
permissions = resource.permission_manager.acl.permissions
resource = Hyrax.persister.save(resource: resource)
resource = Hyrax.persister.save(resource: resource, perform_af_validation: !is_valid)

resource.permission_manager.acl.permissions = permissions
resource.permission_manager.acl.save
Expand Down
7 changes: 5 additions & 2 deletions app/actors/hyrax/actors/lease_actor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,11 @@ def destroy
case work
when Valkyrie::Resource
lease_manager = Hyrax::LeaseManager.new(resource: work)
lease_manager.release && Hyrax::AccessControlList(work).save
lease_manager.nullify
return if lease_manager.lease.lease_expiration_date.blank?

lease_manager.deactivate!
work.lease = Hyrax.persister.save(resource: lease_manager.lease)
Hyrax::AccessControlList(work).save
else
work.lease_visibility! # If the lease has lapsed, update the current visibility.
work.deactivate_lease!
Expand Down
8 changes: 6 additions & 2 deletions app/controllers/concerns/hyrax/works_controller_behavior.rb
Original file line number Diff line number Diff line change
Expand Up @@ -180,15 +180,18 @@ def actor

##
# @return [#errors]
# rubocop:disable Metrics/MethodLength
def create_valkyrie_work
form = build_form
return after_create_error(form_err_msg(form)) unless form.validate(params[hash_key_for_curation_concern])
# fallback to an empty hash to avoid: # NoMethodError: undefined method `has_key?` for nil:NilClass
original_input_params_for_form = params[hash_key_for_curation_concern] ? params[hash_key_for_curation_concern] : {}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

adjusted this error handling so that generic_works_controller_json_spec.rb:58 passes. this is also how the #create method in this same file handles the params

return after_create_error(form_err_msg(form), original_input_params_for_form) unless form.validate(original_input_params_for_form)

result =
transactions['change_set.create_work']
.with_step_args(
'work_resource.add_to_parent' => { parent_id: params[:parent_id], user: current_user },
'work_resource.add_file_sets' => { uploaded_files: uploaded_files, file_set_params: params[hash_key_for_curation_concern][:file_set] },
'work_resource.add_file_sets' => { uploaded_files: uploaded_files, file_set_params: original_input_params_for_form[:file_set] },
'change_set.set_user_as_depositor' => { user: current_user },
'work_resource.change_depositor' => { user: ::User.find_by_user_key(form.on_behalf_of) },
'work_resource.save_acl' => { permissions_params: form.input_params["permissions"] }
Expand All @@ -197,6 +200,7 @@ def create_valkyrie_work
@curation_concern = result.value_or { return after_create_error(transaction_err_msg(result)) }
after_create_response
end
# rubocop:enable Metrics/MethodLength

def update_valkyrie_work
form = build_form
Expand Down
1 change: 1 addition & 0 deletions app/forms/hyrax/forms/work_lease_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ module Forms
# +LeasesControllerBehavior+.
class WorkLeaseForm < Hyrax::ChangeSet
property :lease, form: Hyrax::Forms::Lease, populator: :lease_populator, prepopulator: :lease_populator
property :lease_history, virtual: true, prepopulator: proc { |_opts| self.lease_history = model.lease&.lease_history }
property :lease_expiration_date, virtual: true, prepopulator: proc { |_opts| self.lease_expiration_date = model.lease&.lease_expiration_date }
property :visibility_after_lease, virtual: true, prepopulator: proc { |_opts| self.visibility_after_lease = model.lease&.visibility_after_lease }
property :visibility_during_lease, virtual: true, prepopulator: proc { |_opts| self.visibility_during_lease = model.lease&.visibility_during_lease }
Expand Down
14 changes: 14 additions & 0 deletions app/indexers/hyrax/valkyrie_file_set_indexer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ def to_solr # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Met
solr_doc['hasRelatedMediaFragment_ssim'] = resource.representative_id.to_s
solr_doc['hasRelatedImage_ssim'] = resource.thumbnail_id.to_s

index_lease(solr_doc)

# Add in metadata from the original file.
file_metadata = Hyrax::FileSetFileService.new(file_set: resource).original_file
return solr_doc unless file_metadata
Expand Down Expand Up @@ -116,5 +118,17 @@ def file_format(file)
file.format_label
end
end

def index_lease(doc)
if resource.lease&.active?
doc['lease_expiration_date_dtsi'] = resource.lease.lease_expiration_date&.to_datetime
doc['visibility_after_lease_ssim'] = resource.lease.visibility_after_lease
doc['visibility_during_lease_ssim'] = resource.lease.visibility_during_lease
else
doc['lease_history_ssim'] = resource&.lease&.lease_history
end

doc
end
end
end
4 changes: 3 additions & 1 deletion app/indexers/hyrax/valkyrie_work_indexer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,12 @@ def index_embargo(doc)
end

def index_lease(doc)
if Hyrax::LeaseManager.new(resource: resource).under_lease?
if resource.lease&.active?
doc['lease_expiration_date_dtsi'] = resource.lease.lease_expiration_date&.to_datetime
doc['visibility_after_lease_ssim'] = resource.lease.visibility_after_lease
doc['visibility_during_lease_ssim'] = resource.lease.visibility_during_lease
else
doc['lease_history_ssim'] = resource&.lease&.lease_history
end

doc
Expand Down
14 changes: 13 additions & 1 deletion app/models/hyrax/resource.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class Resource < Valkyrie::Resource

attribute :alternate_ids, Valkyrie::Types::Array.of(Valkyrie::Types::ID)
attribute :embargo_id, Valkyrie::Types::ID
attribute :lease, Hyrax::Lease.optional
attribute :lease_id, Valkyrie::Types::ID

delegate :edit_groups, :edit_groups=,
:edit_users, :edit_users=,
Expand Down Expand Up @@ -119,6 +119,18 @@ def embargo
@embargo = Hyrax.query_service.find_by(id: embargo_id) if embargo_id.present?
end

def lease=(value)
raise TypeError "can't convert #{value.class} into Hyrax::Lease" unless value.is_a? Hyrax::Lease

@lease = value
self.lease_id = @lease.id
end

def lease
return @lease if @lease
@lease = Hyrax.query_service.find_by(id: lease_id) if lease_id.present?
end

protected

def visibility_writer
Expand Down
9 changes: 6 additions & 3 deletions app/services/hyrax/embargo_manager.rb
Original file line number Diff line number Diff line change
Expand Up @@ -126,13 +126,13 @@ def deactivate!
embargo_record = embargo_history_message(
embargo_state,
Time.zone.today,
DateTime.parse(embargo.embargo_release_date),
embargo.embargo_release_date,
embargo.visibility_during_embargo,
embargo.visibility_after_embargo
)

nullify(force: true)
release(force: true)
nullify(force: true)
embargo.embargo_history += [embargo_record]
end

Expand Down Expand Up @@ -180,14 +180,17 @@ def embargo
end

##
# Drop the embargo by setting its release date to `nil`.
# Drop the embargo by setting its release date and visibility settings to `nil`.
#
# @param force [boolean] force the nullify even when the embargo period is current
#
# @return [void]
def nullify(force: false)
return false if !force && under_embargo?

embargo.embargo_release_date = nil
embargo.visibility_during_embargo = nil
embargo.visibility_after_embargo = nil
end

##
Expand Down
96 changes: 88 additions & 8 deletions app/services/hyrax/lease_manager.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,22 @@ module Hyrax
##
# Provides utilities for managing the lifecycle of an `Hyrax::Lease` on a
# `Hyrax::Resource`.
class LeaseManager
#
# The lease terminology used here is as follows:
#
# - "Expiration Date" is the day a lease is scheduled to expire.
# - "Under Lease" means the lease is "active"; i.e. that its expiration
# date is today or later.
# - "Applied" means the lease's pre-expiration visibility has been set on
# the resource.
# - "Released" means the lease's post-expiration visibility has been set on
# the resource.
# - "Enforced" means the object's visibility matches the pre-expiration
# visibility of the lease; i.e. the lease has been applied,
# but not released.
# - "Deactivate" means that the existing lease will be removed
#
class LeaseManager # rubocop:disable Metrics/ClassLength
##
# @!attribute [rw] resource
# @return [Hyrax::Resource]
Expand Down Expand Up @@ -47,6 +62,51 @@ def release_lease_for!(resource:, query_service: Hyrax.query_service)
new(resource: resource, query_service: query_service)
.release!
end

# Creates or updates an existing lease on a member to match the lease on the parent work
# @param [Array<Valkyrie::Resource>] members
# @param [Hyrax::Work] work
# rubocop:disable Metrics/AbcSize, Metrics/MethodLength
def create_or_update_lease_on_members(members, work)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

putting this method in the LeaseManger feels like a better place for it to be than in "transactions/save.rb". it also makes it easier to use when #6131 gets worked on.

# TODO: account for all members and levels, not just file sets. ref: #6131

members.each do |member|
member_lease_needs_updating = work.lease.updated_at > member.lease&.updated_at if member.lease

if member.lease && member_lease_needs_updating
member.lease.lease_expiration_date = work.lease['lease_expiration_date']
member.lease.visibility_during_lease = work.lease['visibility_during_lease']
member.lease.visibility_after_lease = work.lease['visibility_after_lease']
member.lease = Hyrax.persister.save(resource: member.lease)
else
work_lease_manager = Hyrax::LeaseManager.new(resource: work)
work_lease_manager.copy_lease_to(target: member)
member = Hyrax.persister.save(resource: member)
end

user ||= ::User.find_by_user_key(member.depositor)
# the line below works in that it indexes the file set with the necessary lease properties
# I do not know however if this is the best event_id to pass
Hyrax.publisher.publish('object.metadata.updated', object: member, user: user)
end
end
# rubocop:enable Metrics/AbcSize, Metrics/MethodLength
end

# Deactivates the lease and logs a message to the lease_history property
def deactivate!
lease_state = lease.active? ? 'active' : 'expired'
lease_record = lease_history_message(
lease_state,
Time.zone.today,
lease.lease_expiration_date,
lease.visibility_during_lease,
lease.visibility_after_lease
)

release(force: true)
nullify(force: true)
lease.lease_history += [lease_record]
end

##
Expand All @@ -58,7 +118,7 @@ def release_lease_for!(resource:, query_service: Hyrax.query_service)
def copy_lease_to(target:)
return false unless under_lease?

target.lease = Lease.new(clone_attributes)
target.lease = Hyrax.persister.save(resource: Lease.new(clone_attributes))
self.class.apply_lease_for(resource: target)
end

Expand All @@ -80,7 +140,8 @@ def apply!
##
# @return [Boolean]
def enforced?
lease.visibility_during_lease.to_s == resource.visibility
lease.lease_expiration_date.present? &&
lease.visibility_during_lease.to_s == resource.visibility
end

##
Expand All @@ -90,18 +151,27 @@ def lease
end

##
# Drop the lease by setting its release date to `nil`.
# Drop the lease by setting its release date and visibility settings to `nil`.
#
# @param force [boolean] force the nullify even when the lease period is current
# @return [void]
def nullify
return unless under_lease?
def nullify(force: false)
return false if !force && under_lease?

lease.lease_expiration_date = nil
lease.visibility_during_lease = nil
lease.visibility_after_lease = nil
end

##
# Sets the visibility of the resource to the lease's after lease visibility.
# no-op if the lease period is current and the force flag is false.
#
# @param force [boolean] force the release even when the lease period is current
#
# @return [Boolean]
def release
return false if under_lease?
def release(force: false)
return false if !force && under_lease?
return true if lease.visibility_after_lease.nil?

resource.visibility = lease.visibility_after_lease
Expand Down Expand Up @@ -131,5 +201,15 @@ def clone_attributes
def core_attribute_keys
[:visibility_after_lease, :visibility_during_lease, :lease_expiration_date]
end

# Create the log message used when deactivating a lease
def lease_history_message(state, deactivate_date, expiration_date, visibility_during, visibility_after)
I18n.t 'hydra.lease.history_message',
state: state,
deactivate_date: deactivate_date,
expiration_date: expiration_date,
visibility_during: visibility_during,
visibility_after: visibility_after
end
end
end
2 changes: 1 addition & 1 deletion app/views/hyrax/base/_form_permission_lease.html.erb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<div class="form-inline">
<%= visibility_badge(Hydra::AccessControls::AccessRight::VISIBILITY_TEXT_VALUE_LEASE) %>
<%= f.input :visibility_during_lease, wrapper: :inline, collection: visibility_options(:loosen), include_blank: false %>
<%= f.input :lease_expiration_date, wrapper: :inline, input_html: { value: f.object.lease_expiration_date || Date.tomorrow, class: 'datepicker' } %>
<%= f.input :lease_expiration_date, wrapper: :inline, input_html: { value: f.object.lease_expiration_date&.to_date || Date.tomorrow, class: 'datepicker' } %>
<%= f.input :visibility_after_lease, wrapper: :inline, collection: visibility_options(:restrict), include_blank: false %>
</div>
4 changes: 2 additions & 2 deletions app/views/hyrax/base/_form_visibility_component.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
<div class="form-inline">
<%= f.input :visibility_during_embargo, wrapper: :inline, collection: visibility_options(:restrict), include_blank: false %>
<%= t('hyrax.works.form.visibility_until') %>
<%= f.date_field :embargo_release_date, wrapper: :inline, value: f.object.embargo_release_date || Date.tomorrow, class: 'datepicker form-control' %>
<%= f.date_field :embargo_release_date, wrapper: :inline, value: f.object.embargo_release_date&.to_date || Date.tomorrow, class: 'datepicker form-control' %>
<%= f.input :visibility_after_embargo, wrapper: :inline, collection: visibility_options(:loosen), include_blank: false %>
</div>
</div>
Expand All @@ -55,7 +55,7 @@
<div class="form-inline">
<%= f.input :visibility_during_lease, wrapper: :inline, collection: visibility_options(:loosen), include_blank: false %>
<%= t('hyrax.works.form.visibility_until') %>
<%= f.date_field :lease_expiration_date, wrapper: :inline, value: f.object.lease_expiration_date || Date.tomorrow, class: 'datepicker form-control' %>
<%= f.date_field :lease_expiration_date, wrapper: :inline, value: f.object.lease_expiration_date&.to_date || Date.tomorrow, class: 'datepicker form-control' %>
<%= f.input :visibility_after_lease, wrapper: :inline, collection: visibility_options(:restrict), include_blank: false %>
</div>
</div>
Expand Down
8 changes: 8 additions & 0 deletions lib/hyrax/transactions/steps/add_file_sets.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,14 @@ def initialize(handler: Hyrax::WorkUploadsHandler)
# @return [Dry::Monads::Result]
def call(obj, uploaded_files: [], file_set_params: [])
if @handler.new(work: obj).add(files: uploaded_files, file_set_params: file_set_params).attach
if obj.lease
Copy link
Contributor Author

Choose a reason for hiding this comment

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

a new file set gets added to the work in this #call method, which happens after "steps/save.rb". doing this call here correctly puts the lease on new files and existing ones.

file_sets = obj.member_ids.map do |member|
Hyrax.query_service.find_by(id: member) if Hyrax.query_service.find_by(id: member).is_a? Hyrax::FileSet
end

Hyrax::LeaseManager.create_or_update_lease_on_members(file_sets, obj)
end

Success(obj)
else
Failure[:failed_to_attach_file_sets, uploaded_files]
Expand Down
Loading