5844-deactivated-leases #6111
Closed
5844-deactivated-leases #6111
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
technically the lease work is not part of #5844, however, it will need to updated similarly to embargoes and I do not see a ticket for it. so the work is being done here wile the embargo work is fresh on our minds. co-authored-by: braydon <braydon.justice@1268456bcltd.ca>
alishaevn
added
notes-valkyrie
alishaevn
added a commit
that referenced
this pull request
Jul 14, 2023
…. that work can now be found on #6111.
alishaevn
added a commit
that referenced
this pull request
Jul 14, 2023
alishaevn
added a commit
that referenced
this pull request
Jul 17, 2023
co-authored-by: braydon <braydon.justice@1268456bcltd.ca>
alishaevn
force-pushed
the
5844-deactivated-leases
branch
from
a905be9
to
7b19cca
Compare
This was referenced Jul 17, 2023
orangewolf
added a commit
that referenced
this pull request
Jul 20, 2023
* update the solr_params[:fq] value in #with_deactivated_embargos to match whats in the dassie app. ref: #5844 * properly set "embargo.embargo_history" when deactivating an embargo on koppie * save the embargo on the work after its been updated, when deactivating the embargo. remove an errant pry * wip: able to display past embargoes on the edit embargo page, and under the deactivated embargoes tab. the edit page now does not reflect that the embargo is gone though. co-authored-by: braydon <braydon.justice@1268456bcltd.ca> * verify that there is an embargo release date in #enforced? before saying there is an embargo. before this commit if a user simply changed the visibility of a work to "restricted", then the app would think there was an embargo on the work since the embargo visibility was also "restricted". co-authored-by: braydon <braydon.justice@1268456bcltd.ca> * index the "embargo_history_ssim" value in solr when an embargo has been deactivated so that it shows up on the "deactivated embargoes" tab of embargoes/index. co-authored-by: braydon <braydon.justice@1268456bcltd.ca> * stop persisting the work in EmbargoActor#destroy because the AccessControlList persists it for us. something is still going on weird though. it appears that the embargo object in the database is different than the embargo metadata thats getting saved on the work. this means that the "past embargoes" section on the embargo/edit page shows different data than the data in the "deactivated embargoes" tab on the embargo/index page. co-authored-by: braydon <braydon.justice@1268456bcltd.ca> * save the embargo id to the work as a reference instead of the embargo itself. this change fixes the problem where the updated embargo is different than the embargo that got saved on the work. co-authored-by: braydon <braydon.justice@1268456bcltd.ca> * lease structure now matches embargo structure * save the embargo and lease before setting it, so we have access to the id * embargoes and leases are no longer saved to the work itself. instead, we need to find them by their id * rubocop fixes * embargoes and leases now save during changeset sync along with the resource. embargo_manager lazy loads embargo. * update leases to work like embargoes. use the safe navigation operator when calling methods in the valkyrie_work_indexer * fix some specs. co-authored-by: braydon <braydon.justice@1268456bcltd.ca> * fix lease spec issues * embargo actor spec fixes * decouple the deactivated lease work from the deactivated embargo work. that work can now be found on #6111. * fix one more embargo manager spec * move a few more lease related changes to #6111 * Update app/services/hyrax/embargo_manager.rb Co-authored-by: Benjamin Kiah Stroud <32469930+bkiahstroud@users.noreply.github.com> * Update app/models/hyrax/resource.rb Co-authored-by: Benjamin Kiah Stroud <32469930+bkiahstroud@users.noreply.github.com> --------- Co-authored-by: braydon <braydon.justice@1268456bcltd.ca> Co-authored-by: Rob Kaufman <rob@notch8.com> Co-authored-by: braydonjustice <braydonjustice@gmail.com> Co-authored-by: Benjamin Kiah Stroud <32469930+bkiahstroud@users.noreply.github.com>
…hat we can create and deactivate them too.
|
closed in favor of #6127 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
co-authored-by: braydon braydon.justice@1268456bcltd.ca
Related
Summary
decouple the deactivated lease work from the deactivated embargo work.
Guidance for testing, such as acceptance criteria or new user interface behaviors:
Detailed Description
while working on #6098, braydon and I began updating some of the lease code to follow the pattern of the embargo code we were implementing. however, actually fixing the ability to deactivate leases was out of scope for #5844, so the work was moved here. I do not see a ticket for it, but wanted to capture the work somewhere so that we have somewhere to start from when fixing this feature is added to a sprint.
TODO
in my current testing, we cannot deactivate a lease yet. it will probably need something similar to
#deactivatein the embargo pr, but I haven't debugged the issue at all.Changes proposed in this pull request:
@samvera/hyrax-code-reviewers