🧹 Make CreateRelationshipJob work for Valkyrie #908

kirkkwang · 2024-02-02T00:18:54Z

This will add a relationships path for Valkyrie objects. It also will add a transactions call so set child flag will fire off in IIIF Print.

ref:

IiifPrint x Valkyrie Bulkrax scientist-softserv/hykuup_knapsack#141

Child links to parent work now, after successful job completion:

This will add a relationships path for Valkyrie objects. It also will add a transactions call so set child flag will fire off in IIIF Print. ref: - scientist-softserv/hykuup_knapsack#141

Co-Authored-By: Kirk Wang <[email protected]>

jeremyf · 2024-02-02T16:53:35Z

lib/bulkrax/persistence_layer/active_fedora_adapter.rb

@@ -6,7 +6,7 @@ class ActiveFedoraAdapter < AbstractAdapter
      def self.find(id)
        ActiveFedora::Base.find(id)
      rescue ActiveFedora::ObjectNotFoundError => e
-        raise PersistenceLayer::RecordNotFound, e.message
+        raise PersistenceLayer::ObjectNotFoundError, e.message


Good catch!

This also adds some consideration for refactoring the queries to instead use the persistence layer.

ShanaLMoore · 2024-02-02T17:19:58Z

app/factories/bulkrax/object_factory.rb

@@ -90,17 +90,18 @@ def update
    def find
      found = find_by_id if attributes[:id].present?
      return found if found.present?
-    rescue Valkyrie::Persistence::ObjectNotFoundError
+      return search_by_identifier if attributes[work_identifier].present?


This change fixed the reason why a link was not being displayed on a successful import. Before it would leave a message that the work didn't successfully import when that wasn't true.

Pull in fixes to make bulkrax relationships work for valkyrie. It also fixed the setting of is_child and the linking of relationships Related: - samvera/bulkrax#908 - scientist-softserv/iiif_print#328

no-reply · 2024-02-05T18:27:55Z

app/jobs/bulkrax/create_relationships_job.rb

+      #      Hyrax's `./app/controllers/concerns/hyrax/works_controller_behavior.rb`
+      #
+      change_set = Hyrax::ChangeSet.for(parent_record)
+      Hyrax::Transactions::Container['change_set.update_work'].call(change_set)


i'm curious about this choice. won't this ensure that the entire update work process (the same one that happens via the controller action) takes place just to change the work membership?

Hyrax::Publisher provides an object.membership.updated event, and i wonder whether it wouldn't be better to save the work with the updated membership and publish that? https://github.com/samvera/hyrax/blob/fdcabe651850b1885d7bd3e1b36777bf5f336ee8/lib/hyrax/publisher.rb#L183-L192

@no-reply, for our uses, we're trying to hit this transaction, so you're suggesting we put a publisher here and then that means this can become a listener?

here was the original idea of the listener

@kirkkwang yes. i think what i'm suggesting (and i'm just popping in as an outsider here), is that you want this behavior to trigger whenever an object's membership changes, not only when a given transaction runs.

making it a listener means you'll trigger these changes when outside callers (in Hyrax or in the downstream application) change membership, as long as they dutifully publish the expected message.

i think a helpful line to draw around transactions and listeners is:

use a transaction step when you want to EXPLICITLY implement some behavior as part of a given operation, but you might want to (explicitly) implement different behavior in a different context.

e.g. i might want to set the current_user as the "depositor" every time a create action happens in the hyrax works controller, but handle this matter differently if i create an object through an API endpoint.

it would be normal for the Hyrax engine, Bulkrax's own code, or the downstream application to call different transactions that may or may not include steps the others might use.

use a listener when you want to ensure some behavior always runs in response to some event, regardless of that event's context.

e.g. whenever metadata updates for an object, i want to reindex that object.

another way of thinking about it is:

if in a naive MVC architecture i would have just written the code in the controller, or as a model method, like do_this; do_that; do_other, then you want that to be a transaction step.

if in a naive MVC architecture i would have written a callback, you want a listener. (it might be helpful to think of a listener as a multiplexed callback, decoupled from the model layer).

Thanks @no-reply that's super helpful!

FYI I made a ticket here scientist-softserv/iiif_print#330

@no-reply should this publish 'object.membership.updated'

@orangewolf

* create an object factory that supports Valkyrie All code in this commit has been adapted from Surfliner: https://github.com/surfliner/surfliner-mirror * temp gem conflict workaround * ⚙️ upgrade dry-monads dependency to ~> 1.5.0 Hyrax 4.0.0 requires a dependency upgrade for dry-monads. I could not upgrade GBH's bulkrax without doing this change. - Issue: - scientist-softserv/ams#77 - Ref: - https://github.com/samvera/hyrax/blob/cbe9278b919485f90a37630d3f3157ecef59cd7c/hyrax.gemspec#L48 * 🧹 Add extra parameter for fill_in_blank_source_identifiers gbh got an error that we were passing too many arguments when setting the source_identifier in the bulkrax config. ref: - https://github.com/samvera-labs/bulkrax/wiki/Configuring-Bulkrax#source-identifier * Revert ":broom: Add extra parameter for fill_in_blank_source_identifiers" This reverts commit df96de6. * 🧹 delegate create_parent_child_relationships from importer to parser * allow ruby 3 syntax in migrations * 🧹 change exists? to exist? to support Ruby 3.2 * 🚧 add support for Hyrax 5, valkyrie and ruby 3.2 * add temp workaround for blank title and creator * ⚙️ Switch find methods with custom queries for Valkyrie * hyrax 4 permission service does both valk and non-valk * new bagit * handle validation failure * better failure detection for vaklyrie object * fix validation message * importer failure helpers * improve multiple detection in matchers * fix matcher on missing field * rob cant remember that its include? * Appeasing rubocop * ♻️ Handle exist? and/or exists? for finding objects See inline comments * Add dry/monads require for specs * I897 Bulkrax readiness for Hyku 6 and Hyrax 4 & 5 (#898) * 🧹 relocates transactions from inititalizer file Issue: - #897 Co-Authored-By: LaRita Robinson <[email protected]> * 🧹 Add specs for container.rb, relocate files Co-Authored-By: LaRita Robinson <[email protected]> * 🧹 normalize magic strings into constants for referencing later Convert the create_with_bulk_behavior and update_with_bulk_behavior to a constant; that way we can reference it in IiifPrint and document the “magic” string. Co-Authored-By: LaRita Robinson <[email protected]> * 🧹 correct camel case to constant notation for easier referencing Co-Authored-By: LaRita Robinson <[email protected]> * 💄 rubocop fixes Co-Authored-By: LaRita Robinson <[email protected]> * Update app/factories/bulkrax/valkyrie_object_factory.rb * Update spec/bulkrax/transactions/container_spec.rb * 🧹 Move container & steps Match Hyrax convention by using bulkrax/transactions. * restructure org to run specs locally receiving error when trying to run the entire spec suite due to restructuring files but not moving the spec file. * 🚧 WIP: Consolidate HasMatchers with HasMappingExt Remove HasMappingExt and consolidate logic within HasMatchers. HasMatchers should handle both cases, when objects are ActiveFedora vs Valkyrie. * 🧹 Fix Specs & add Valkyrie Specs * 🧹 Fix Rubocop complaint * 🧹 Address Valkyrie's determination of multiple? * 🧹 Address permitted attributes In Valkyrie, we use the schema to identify the permitted attributes. All allowed attributes should be on the schema, so no additional attributes should be required. Also add a fallback for permitted attributes in case an ActiveFedora model class goes through the ValkyrieObjectFactory. This supports the case where we want to always force a Valkyrie resource to be created, regardless of the model name given. * 🧹 Update TODO comment Adjust TODO message because referring to a handler that doesn't exist anywhere is confusing. We may need to register steps for file sets once the behavior is implemented. --------- Co-authored-by: LaRita Robinson <[email protected]> Co-authored-by: Jeremy Friesen <[email protected]> Co-authored-by: LaRita Robinson <[email protected]> * 📚 Adding documentation for configuration (#896) This builds on a [question asked in Slack][1] [1]: https://samvera.slack.com/archives/C03S9FS60KW/p1705681632335919 * ♻️ Extract Bulkrax::FactoryClassFinder (#900) This refactor introduces consolidating logic for determining an entry's factory_class. The goal is to begin to allow for us to have a CSV record that says "model = Work" and to use a "WorkResource". Note, there are downstream implementations that overwrite `factory_class` and we'll need to consider how we approach that. * 🐛 [i134] - Fix missing translations Missing translations were evaluating to false. Issue: - scientist-softserv/hykuup_knapsack#134 * Renaming method for parity * ♻️ Favor Bulkrax's persistence layer Instead of direct calls to a deprecated service favor a persistence layer call; one that defines an interface. Note this means we need to implement the methods in the Valkyrie adapter; but those should be trivial. * ♻️ Favor Bulkrax.persistence_adapter over ActiveFedora::Base * Moving methods to adapter pattern * use find_by_source_identifier instead of find_by_bulkrax_identifier (#907) * i903 - move bulkrax identifier custom queries into bulkrax move bulkrax identifier custom queries into bulkrax Issue: - scientist-softserv/hykuup_knapsack#136 * make find_by_source_identifier dynamic Import a csv with child works. The forming of relationships is not working. Part of the problem is the find_by_bulkrax_identifier call. From GBH, this used to be find_by_bulkrax_identifier which not all clients will configure as their source identifier. Instead we need to ask for the source identifier and use that for the sql query. This commit goes along with a PR from Hyku which currently has the find_by_source_identifier.rb files defined. Issue: - scientist-softserv/hykuup_knapsack#128 Co-Authored-By: Kirk Wang <[email protected]> * remove files: they live in Hyku for now Co-Authored-By: Kirk Wang <[email protected]> * 🧹 Place custom queries back in Bulkrax * 🧹 remove misleading comment Co-Authored-By: Kirk Wang <[email protected]> * 🧹 Entry is a required argument when initializing ObjectFactory Fix for broken specs Co-Authored-By: Kirk Wang <[email protected]> * revert changes to pass Entry arg The object factory already has work_identifier: parser.work_identifier. we don't need the entry argument after all. ref: - https://github.com/samvera/bulkrax/blob/main/app/models/concerns/bulkrax/import_behavior.rb#L181 Co-Authored-By: Kirk Wang <[email protected]> --------- Co-authored-by: Kirk Wang <[email protected]> Co-authored-by: Kirk Wang <[email protected]> * 🧹 Make CreateRelationshipJob work for Valkyrie (#908) * 🧹 Make the relationships job work for Valkyrie This will add a relationships path for Valkyrie objects. It also will add a transactions call so set child flag will fire off in IIIF Print. ref: - scientist-softserv/hykuup_knapsack#141 * 💄 rubocop fix Co-Authored-By: Kirk Wang <[email protected]> * ♻️ Adjust rescue logic to move closer to error This also adds some consideration for refactoring the queries to instead use the persistence layer. * Adding notes about transactions --------- Co-authored-by: Shana Moore <[email protected]> Co-authored-by: Jeremy Friesen <[email protected]> * Add todo comment Co-Authored-By: Kirk Wang <[email protected]> * 🎁 Switch transaction to listener This commit will switch the membership transaction to a listener. * ♻️ Migrate persistence layer methods to object factory (#911) * ♻️ Migrate persistence layer methods to object factory In review of the code and in brief discussion with @orangewolf, the methods of the persistence layer could be added to the object factory. We already were configuring the corresponding object factory for each implementation of Bulkrax; so leveraging that configuration made tremendous sense. The methods on the persistence layer remain helpful (perhaps necessary) for documented reasons in the `Bulkrax::ObjectFactoryInterface` module. See: - #895 and its discussion * 🎁 Add Valkyrie object factory interface methods * 🧹 Favor interface based exception Given that we are not directly exposing ActiveFedora nor Hyrax nor Valkyrie objects, we want to translate/transform exceptions into a common exception based on an interface. That way downstream implementers can catch the Bulkrax specific error and not need to do things such as `if defined?(ActiveFedora::RecordInvalid) rescue ActiveFedora::RecordInvalid` It's just funny looking. * 🧹 Get exporters to work This commit contains various changes to get the exporters to work correctly. * make updates work * 🧹 Make DeleteJob work wth new class method .find (#912) * 🧹 Make DeleteJob work wth new class method .find The DeleteJob previously was not working with the old factory#find method because when it is doing a delete action, the parsed_metadata does not get generated like during a regular import. Because of this, the #search_by_identifier method fails to find anything because we don't have a `work_identifier` field which would have came from the parsed_metadata. So instead, we are using the new class method .find which will take an id (which we find on the raw_metadata) to find the object. We make sure to reindex and publish the action to any relevant listeners. * 🎁 Implement a #delete method for the ObjectFactory This commit will add a delete method to the ObjectFactory and the ValkyrieObjectFactory so we can avoid unnecessary conditionals. * 🧹 Rework factories to implement delete method This cuts down on the method chaining. * ♻️ Remove constant This creates hard to parse chatter, and is not needed as we were relying on it for IIIF Print to be able to reference. * ♻️ Reworking structure The Hyrax transactions create a lot of pre-amble and post-amble for performing the save. This commit attempts to consolidate logic to reduce redundancy of that boilerplate. Further, it adds handling for creating collections. We still need to handle form validation. * Adding index to schema * ♻️ Favor asking about model_name over class (#934) Given our effort at lazy migration in Bulkrax we want to do a bit more sniffing regarding the objects. This is not quite adequate for the general case of Collections but it is an improvement. Ideally we should be interrogating the class and asking `klass.collection?` but there are some confounding edge cases around routing that we are in this pickle. ```ruby irb(main):002:0> CollectionResource.model_name => @collection="collections", @element="collection", @Human="Collection", @i18n_key=:collection, @klass=CollectionResource, @name="CollectionResource", @param_key="collection", @plural="collections", @route_key="collections", @Singular="collection", @singular_route_key="collection"> irb(main):003:0> Collection.model_name => @collection="collections", @element="collection", @Human="Collection", @i18n_key=:collection, @klass=Collection, @name="Collection", @param_key="collection", @plural="collections", @route_key="collections", @Singular="collection", @singular_route_key="collection"> irb(main):004:0> ``` * Favor object factory for find * ♻️ Fix return value of transaction create * Correct Hyrax.object_factory -> Bulkrax.object_factory * Download cloud files later (#930) * 🎁 Reschedule ImporterJob if downloads aren't done This commit will add a check in the `ImporterJob` to see if the cloud files finished downloading. If they haven't, the job will be rescheduled until they are. * 🎁 Download Cloud Files later This commit will bring in changes from `5.3.1-british_library` to move the download of cloud files to a background job. --------- Co-authored-by: Jeremy Friesen <[email protected]> * ♻️ Favor configuration over hard-coding and reaching assumptions The main "flip" of logic is that we can remove the `curation_concern?` method because we can instead ask "if Collection || FileSet" and infer when that is false that we have a work. This means removing the very reaching assumption of Hyku and it's implementation foibles for work types. * ♻️ Extract Bulkrax.collection_class_method Instead of relying on the hard-coding, allow for configuration. Co-authored-by: Shana Moore <[email protected]> * ♻️ Favor Bulkrax.collection_model_class Co-authored-by: Shana Moore <[email protected]> * ♻️ Favor Bulkrax.object_factory.find Instead of relying on the direct call to a constant. Co-authored-by: Shana Moore <[email protected]> * ♻️ Extract Bulkrax.object_factory.save! method for We have a place where we try to call save! directly. We do need to pass a user for save event; hence the added method. * ♻️ Favor using object_factory for save! Co-authored-by: Shana Moore <[email protected]> * ♻️ Extract Hyrax.object_factory.search_by_property There is a duplication of this logic elsewhere, but I first wanted to extract common logic then begin extracting full replacement and conforming object interface for Valkyrie. * ♻️ Extract method for Valkyrization We cannot directly query the class. But must instead favor the object_factory. * 🎁 Adding query for find_by_model_and_property_value * ♻️ Remove custom Valkyrie search_by_identifer The super method was refined to use the class object factory; making it redundant and flexible in the same manner as `Bulkrax::ObjectFactory#search_by_identifer`. * ♻️ Favor internal_resource definitions (when available) * ♻️ Extract internal_resources method for curation concerns * ♻️ Favor Bulkrax.object_factory and add fault tolerance * Addressing TODO and minor refactoring * I161 import collection resources (#933) * 🚧 WIP: Import Collection Resource A user should be able to import a collection resource. In this commit, we are able to successfully import and create collection resources. From the console we can see the collection formed relatioships with works, but the frontend's count and display shows 0 relationships. Additionally, we are unable to re run the importer without receiving errors on the collection entry. TODO: specs, refactor, Issue: - scientist-softserv/hykuup_knapsack#161 * remove unused code * refactor #conditionally_destroy_existing_files This refactor was necessary because even though klass == ImageResource, which inherits from Valkyrie::Resouce through it's chain, klass === Valkyrie::Resource was returning false. * exclude CollectionResource class from #destroy_existing_files * WIP - try to import filesets with valkyrie resources * Revert "WIP - try to import filesets with valkyrie resources" This reverts commit 4ab31b6. * 💄 rubocop fix * i162 - import valkyrie works with filesets (#936) * Revert "WIP - try to import filesets with valkyrie resources" This reverts commit 4ab31b6. * WIP * WIP - try to import filesets with valkyrie resources * 🚧 WIP: get filesets to import via bulkrax x valkyrie * 🎉 WIP: filesets to imports via bulkrax x valkyrie There's still a lot to clean up here, but the import is successful in this commit. * 💄 rubocop fixes * uncomment #get_s3_files call and add collections to configuration * Update object_factory.rb * ♻️ Move method and remove single instance definition I'm unclear why we were defining methods on the conf instance; especially given that these exist on the configuration. With this refactor, we're favoring using the Configuration object as the container. * Revert changes due to refactor coming in from main * address errors post big refactor * Refactoring for consistent method signatures Also avoiding setting an unused instance variable * 🐛 remove passing user to work_resource add_file_sets and save merge to strategies Importing a CSV of valkyrie works, collections, files and relationships is working at this point 🎉 * 🎁 Adding a new transaction step to handle different association * ♻️ Extract update_index method to object factory * ♻️ Extract object factory method * ♻️ Extract add_resource_to_collection method * ♻️ XIT out the mockery and stubbery of a spec * ♻️ Extract method publish and add_child_to_parent_work * ♻️ Rename method as it's not conditional Yes, it is conditional but it operates on arrays that could be empty. * Remove add to collection step * 🐛 Fix publish parameter mismatch * Removing custom transaction container. We weren't using it * Favor keyword args instead of hashes * 💄fixing typo * 🎁 Add update_collection to valkyrie object factory * 💄 endless and ever appeasing of the coppers --------- Co-authored-by: Jeremy Friesen <[email protected]> --------- Co-authored-by: Jeremy Friesen <[email protected]> * ♻️ Extract logic for add_user_to_collection_permissions * 📚 Tidying documentation * ♻️ Refactor Object Factories to leverage more inheritance * ♻️ Extract abstract class for ObjectFactory In constructing object inheritance, a more robust strategy is to create an abstract class and then have classes directly extend that abstract class. This helps define and narrow an interface. * ♻️ Move method to interface This is used in both ObjectFactory and ValkyrieObjectFactory * ♻️ Organizing code for Valkyrie Object Factory * Refactoring method names for sorting order * ♻️ Handle Valkyrie::Resource situation * ♻️ Puzzling through implementation details * ♻️ Extract method to enable removal of conditionals * ♻️ Extract FileFactory::InnerWorkings The goal of this extraction is to minimize the exposed interface of what is quite complicated and state dependent logic. * ♻️ Refactor to extract local variable * Adding class attribute for Bulkrax::FileFactory * ♻️ Adding inner methods for file factory interaction * 🐛🏳️ post Big refactor fixes Refactoring caused some bugs. At this point we are able to successfully import CSV works again. * fix typo * 🧹 Add case for `'collectionresource'` In Valkyrie Hyku we're using CollectionResource and this was not being recognized by the CSV parser. * reload the object before calling persisted? on it resolves failure saying that errors is undefined. object.persisted? returned false even though we could see that they got created in the UI. * 💄 rubocop fix * 🐛 Add return in ObjectFactory if valkyrie Adding this early return here so we don't go down to the the #where and trigger a NoMethodError. What it seems like it's doing is checking Postgres for the object but if it doesn't find it then tries in Fedora, however, Valkyrie object don't respond to #where so it throws an error. * save parent object to establish relationships This fixes the reason why works weren't forming relationships with other works * Add FileSet branch to coercer conditional This is in prep to handle Hyrax::FileSets being imports as rows. * Add commit to clarify casecmp in CsvParser * 🎁 Add ability to use tar.gz files This commit will allow users to use tar.gz files as well as zip files for importing. * 🐛 Changing guard to #respond_to?(:where) A spec was failing with the previous way we were checking. * 🎁 Change glyphicon to font awesome Hyrax 4+ applications use font awesome and not glyphicon. This commit will convert all glyphicon to font awesome. * Add require ruby-progressbar (#942) Update bulkrax_tasks.rake Fixes #941 * 🐛 Ensure we include visibility and other keywords for collection Related to: - scientist-softserv/hykuup_knapsack#182 Co-authored-by: LaRita Robinson <[email protected]> * 🐛 Fix visibility check on the object This commit will add a guard for visibility because it is not on a valkyrie resource. * 🐛 Save provided visibility from CSV CSV provided visibility was being clobbered in the ImportCollectionJob. Refs scientist-softserv/hykuup_knapsack#182 * ♻️ Extract methods for better composition * ♻️ Extracting object factory methods I want to avoid having conditionals regarding object factories. This violates the polymorphism and means that other implementors that choose a different `Bulkrax.object_factory` will have unintended consequences. * 💄 endless and ever appeasing of the coppers * ♻️ Favor object factory over hard-coded * Amend the see/refer documentation for parser * 💄 endless and ever appeasing of the coppers * Updating test schema * Remove transactions from initialization * ♻️ Remove explicit calls to AdminSet * 📚 Adding TODO items --------- Co-authored-by: Benjamin Kiah Stroud <[email protected]> Co-authored-by: Rob Kaufman <[email protected]> Co-authored-by: Kirk Wang <[email protected]> Co-authored-by: Jeremy Friesen <[email protected]> Co-authored-by: LaRita Robinson <[email protected]> Co-authored-by: LaRita Robinson <[email protected]> Co-authored-by: Kirk Wang <[email protected]> Co-authored-by: Dan Kerchner <[email protected]>

@object

* Delete _repository_content.html.erb Having this file was causing double rendering of bulkrax links in the sidebar. Issues: - #1850 - scientist-softserv/hykuup_knapsack#111 * use hyrax-4-valkyrie-support bulkrax branch Update bulkrax to point to hyrax-4-valkyrie-support * 🐛 App was not loading after updating Hyrax This commit will adjust for the changes introduced in this commit: samvera/hyrax@8fc0894 * ⚙️ Don't remove backtrace in dev * remove bulkrax_identifier from Hyku bulrax_identifier is not a required property. Each client should set their desired source_identifier in their application. Issue: - scientist-softserv/hykuup_knapsack#136 * 🧹 Make the appropriate link generate This commit will add a decorator to check the `human_readable_type` of the given object is a Valkyrie migration object (which by convention ends with 'resource') and adjust the generated link accordingly. For example, a GenericWorkResource should generate links like `/concern/generic_work_resources/...` instead of `/concern/generic_works/...`. The models were updated to include the Hyrax::NestedWorks so the `Attach Child` button would populate the appropriate work types. Lastly, the `Gemfile.lock` was updated to include the latest version of Hyrax `double_combo` branch. * 🧹 Add Valkyrie test adapter For us to leverage the useful shared specs of Hyrax, we need a test adapter. * 🧹 Always with the coppers * Updating Hyrax to latest version * 🧹 Rework logic for #hydra_model Using 'human_readable_type' was going to be flemsy. Instead we are opting for adding the `valkyrie_bsi` to the index and asking that. * 📚 Add docs regarding knapsack * 🧹 Favor not using valkyrie for the spec * ♻️ Only enable auto-redirect when Valkyrie enabled. * 🧹 Configure Bulkrax We configure Bulkrax to use the `ValkyrieObjectFactory` along with the `ValkyrieMigrationCoercer` so a `GenericWork` will create a `GenericWorkResource`. Also, updated the revision of Bulkrax. * Revert "🧹 Remove files declared in Knapsack" This reverts commit d9fa1a2. * Revert "remove bulkrax_identifier from Hyku" This reverts commit 887fbf2. * ♻️ Bump Hyrax ref * ♻️ Favor Hyrax::SolrService Ideally the code would reference the SolrService in one manner. This is an effort to create an insulating layer around that. * make find_by_bulkrax_identifier.rb more dynamic Not all applications will use bulkrax_identifier as their source_identifier. This code makes it more dynamic. TODO: Make Wings::CustomQueries more dynamic * Leverage updated IIIF Print * 🧹 Appeasing rubocop * find_by_source_identifier files moved to bulkrax ref: - samvera/bulkrax@29f2264 * Revert "find_by_source_identifier files moved to bulkrax" This reverts commit 78d26c5. * update bulkrax and iiif_print versions find_by_source_identifier files were moved into bulkrax. * 🧹 apply conditional to hyku indexing specs refer to object as @object, and thus caused a lot of spec failures without this. * 💄 rubocop fix * 🧹 Fix set child flag This commit will make sure that the set child flag transaction fires when ingesting with Bulkrax. Also fixed a weird bug where the extra space in the query was causing the query to fail. * add guard for ActiveFedora's member_ids AF's member_ids returns an array of ids so mapping (:id) will not work. * Update Bulkrax and IiifPrint Pull in fixes to make bulkrax relationships work for valkyrie. It also fixed the setting of is_child and the linking of relationships Related: - samvera/bulkrax#908 - scientist-softserv/iiif_print#328 * remove HYKU_IIIF_PRINT conditional Per Rob, remove HYKU_IIIF_PRINT conditional. Iiif print will be enabled by default. * update hyrax and iiif print * Valkyrize reindex rake tasks This commit adds reindexing for Valkyrie objects. Issue: - scientist-softserv/hykuup_knapsack#146 Co-Authored-By: Kirk Wang <[email protected]> * 🐛 Ensure underlying change to permissions * Fix for xray-rails not working Issue: - scientist-softserv/hykuup_knapsack#71 Co-Authored-By: C Barton <[email protected]> * Bumping Hyrax version * 💄rubocop fixes Co-Authored-By: C Barton <[email protected]> * update hyrax, bulkrax, iiif_print * update hyrax * Pin Hyrax to functioning build * Delete find_by_source_identifier_spec.rb This spec is no longer needed as its relevant code moved into bulkrax. * Update iiif_print ref: add a reindex after the child work has been saved. - scientist-softserv/iiif_print#332 * 🧹 Remove `form` key from with_pdf_viewer The `show_pdf_viewer` and `show_pdf_download_button` fields were showing up on the form even though in the yaml they were set to false. It looks like just removing the `form` key from the yaml file will prevent them from showing up. Also updated the `Gemfile.lock` file. * 🧹 Fix specs to reflect usage of Double Combo * 🐛 Use registered instead of restricted Restricted is the visibility, registered is the concept. This fixes that misnomer. * 🧹 Amend spec to highlight double combo nuance * 🧹 Rework lease expiry tests See https://github.com/samvera/hyku/wiki/Updating-Hyku-6-with-Hyrax-5-Developer-Notes#access-control-list-acl-considerations * 💄 endless and ever appeasing of the coppers * 🧹 Fix broken spec * 💄 endless and ever appeasing of the cops * Adding documentation * 🧹 Adjust AdminSet spec count to reflect query service * 🧹 Address assumptive redirect for /concern/generic_work/ * 🧹 Fixing redirect assumptions for insitutional visibility * 🧹 Favor up to date double_combo * Fixing a few specs to reflection Valkyriziation * 🧹 Updating Hyrax version Grab the latest sweet sweet double combo * include Bulkrax helpers and update Bulkrax * Rework spec to not rely on reload * 🧹 Fixing tests to use generic_resource_factory The factory is directly copied from Hyrax; and is not fully functional, but it's better than what we've got. * Fixing a spec that needs a #to_a * ♻️ Add Hyku.bulkrax_enabled? Also refactor to set the default work type for Bulkrax later in the application boot cycle. When it was where it was at, I encountered issues with wings model registration. * ♻️ Favor re-using the controller and hopefully routes * Demonstrate how we're using lazy migrations and model naming * ♻️ Remove generated files Favor the Lazy migration strategy * 🧹 endless and ever appeasing of the coppers * ☑️ Tidying up tests * ☑️ Fixing a few broken tests By extending the ability class to include all of the collection models, we open up some additional considerations. * Move ability declarations out of loop, where loop not needed * 💄 endless and ever appeasing of the coppers * ☑️ Fill out require attributes * ☑️ Working through failing tests This does involve fixing a bug that is masking another underlying bug. * Fixing broken specs * 🐛 Refactor collection ability class * Add elsif for FileSet and configure IIIF Print This commit will add an elsif statement to the wings adapter to handle FileSet objects. It will also add the configuration for IIIF Print to use the IiifPrint::PersistenceLayer::ValkyrieAdapter. * Bumping double_combo version * Favor helper method over instance variable * ☑️ We need to create a method to stub * ☑️ Ever working towards tests * 💄 endless and ever appeasing of the coppers * 🎁 Add some FileSet interface friendliness * ☑️ Favor factory over process through actor stack * ☑️ Adding tests for GenericWorkResrouce * 🎁 Favor inheriting from admin_set * ☑️ Removing spec/factories defined Hyrax * 💄 endless and ever appeasing of the coppers * ♻️ Fixing some tests to rely on factories instead of actor * 🚧 Update Hyrax and IIIF Print This commit will update the Hyrax and IIIF Print revisions which should allow us to ingest images and have it render in the UV. Uploading a PDF should also work but PDF splitting current does not work. * ☑️ Fixing a few tests by re-purposing some factories * WIP * Removing unneeded decorator and associated spec * 🧹 Ignore vendor/gems directory * ☑️ Adding documentation regarding Factories * ☑️ Fixing tests and factories for roles_service * ♻️ Favor site configuration for allow downloads Hyrax 5 has a display_media_download_link configuration option that we can repurpose for the allow_downloads functionality that we are looking to back-port from Hyku. * ☑️ Fixing a broken test * Fixing a few broken specs, still some broken * 💄 endless and ever appeasing of the coppers * Removing long-ago removable code * Modify Collection & Admin Set Valkyrization Addresses: ☄️ EPIC: Valkyrize Hyku scientist-softserv/hykuup_knapsack#35 valkyrize collection and admin set resources scientist-softserv/hykuup_knapsack#94 Moves away from directly using Hyrax::PcdmCollection and Hyrax::AdministrativeSet instead favoring inheriting from those (with CollectionResource and AdminSetResource). This allows for inheriting from Hyrax factories and extending them; thus bringing Hyku’s testing ecosystem closer to Hyrax. Note: There are a few of the new specs that still fail. - * Coppers again * ☑️ Favor explicit load of FactoryBot factories * ☑️ Working on getting more tests passing * Fixing ever more specs that keep breaking underneath me * 💄 endless and ever appeasing of the coppers * ♻️ Verify AdminSetResource factory build * Adding specs to demonstrate factories * 🧹 Avoid assuming factory-bot is available. * 🧹 We want to create admin set resources * 🧹 Fix broken spec due to not assuming admin group * ☑️ Fixing specs by re-using factories * ☑️ Adjust spec to use proper collection factory * ☑️ Favor factory over instance double * ☑️ Favor factory over explicit class * ☑️ Favor factory over stub, and correct form class * ☑️ Ensuring we start from a clean slate * Fixing specs * ☑️ Fixing tests squashing bugs that might show up * Fixing specs to use resource classes * Bumping version of IIIF Print * 🐛 Add .each to find_all call Without this, we pass a block to a method that does not anticipate receiving a block. * Fixing feature specs by ensuring admin_group * 🐛 Fixing a couple of different bugs Ever more to do. * 💄 endless and ever appeasing of the coppers * WIP * ♻️ Adding further collection ability testing * ♻️ Put long list of objects into a loop * ♻️ Adding more valkyrie native collection specs * ♻️ Re-arranging collection declarations * ♻️ Working in the spec mines regarding permission * Adding specs for disable * ♻️ Reworking specs and addressing significant typo * 🧹 Fix specs concerning collection management * Ever hacking on the specs * 💄 endless and ever appeasing of the coppers * 🧹 Fix tests regarding collection management * 🧹 Fixing Hyrax::CollectionType#collections query * 🧹 Fixing a few broken specs * 🧹 Reworking factory process * 💄 endless and ever appeasing of the coppers * ♻️ Favor member_ids_ssim over file_set_ids_ssim For several years Hyrax has index `file_set_ids_ssim` as a verbatim copy of `member_ids_ssim`. With Hyrax 5, we're removing the `file_set_ids_ssim` from indexing; And given that it's been a verbatim copy since 2017 or so, it's relatively safe to assume that we can favor, without application impact, the `member_ids_ssim` over the `file_set_ids_ssim` value. It would be nice to have `file_set_ids_ssim` but not as a verbatim copy. Someday, we might have nice things. Related to: - samvera/hyrax#6513 - samvera/hyrax@7108409 * ☑️ Add role trait to reflect tests * 🧹 Set various presenter's with correct file_presenter_class Related to: - samvera/hyrax#6717 * Bumping the double_combo * Bumping version of IIIF Print and Bulkrax * 💄 endless and ever appeasing of the coppers * Adding tests to snare failing specs that work locally * ♻️ Working on factory bot creation of permission templates * ♻️ Fix spec harness for permissions * 📚 Add spec for factories to explain what happens * Update double_combo and add bulkrax migrations * Fixing some underlying specs * ♻️ Fix selector issue * ☑️ Let These Specs Pass! * ☑️ Typo * 💄 endless and ever appeasing of the coppers * Remove redundant declaration * ☑️ Remove flakey tests These tests relied on existing data in solr but did not seek to have clean data, thus received leaking data from other tests. Meaning they would fail. Ugh, tests should (by default) favor starting from a "clean state" instead of inheriting state from other tests. * ☑️ Favor FactoryBot.valkyrie_create To do otherwise, invites a series of weird interactions. * ☑️ Add explicit setting of models before discovery factories * 🧹 Bump to new double_combo * Remove transient as the underlying problem fixed * ♻️ Remove auto-creation of file * ☑️ Favor Hyrax::SolrService.connection to match interface * Bumping to new version of Hyrax double combo * Update Hyrax pulling in Jeremy's changes: custom queries and assigned strategies. * Removing mocked tests that don't prove much * Add embargo and lease to wings We started seeing problems with the embargo (and presumably lease) now allowing us to save the work. This commit adds the necessary fields to the big else statement in the wings initializer. Ref: - scientist-softserv/hykuup_knapsack#157 * Update riiif config to replace deprecated method The `cache_duration_in_days` is deprecated and locally it was causing the web service to freeze. * Adding tenant for request spec * Updating factory spec to catch a failure * Update Hyrax to move binaries This commit will update Hyrax with the ability to move the binaries from Fedora to Disk. There are also some changes to the database schema to make persisting work. Wings initializer was updated to map the Hydra::PCDM::File to the Hyrax::FileMetadata class as well. * start spec to test file set migration * 🐛 Fix Account switch bug * ♻️ Remove method as duplicate of Hyrax The Hyrax::Group.new override handles the "find_by" logic we introduced. * 🎁 Auto-magically migrate file sets and binaries * 💄 endless and ever appeasing of the coppers * ♻️ Remove stray puts statement * Attempting to squash a flakey set of specs * Update #parent_path to account for various case statements The previous assumption was that we'd always have a SolrDocument, which is not true. This method has been updated to be more flexible. * 💄 endless and ever appeasing of the coppers * I had to bail and copy the factories from Hyrax * 💄 endless and ever appeasing of the coppers * ♻️ Disable some consistently failing specs Note, this likely means I can revert a previous commit about adjusting the copies of factories from Hyrax. * Update hyrax version pulls in thumbnail loading fix * Revert "I had to bail and copy the factories from Hyrax" This reverts commit b0cbbac. * Update Hyrax pulls in WIP for af to valkyrie thumbnails + fix for serving thumbnails * Bumping Hyrax version * Updating Hyrax to latest build with FS fixes * Merge branch 'i35-valkyrize-hyku' of https://github.com/samvera/hyku into i35-valkyrize-hyku * ♻️ Update Hyku to new Double Combo and config file_set_model * 💄 endless and ever appeasing of the coppers * Addressing potential routing errors Co-authored-by: Shana Moore <[email protected]> * Remove indexing of collection membership Looking at the Hyrax code, it appears that it should already be done. * ♻️ Adding Hyku::Application.work_types (see docs) * ♻️ Configure `Bulkrax.*_model_class` methods * copy Rob's reprocessor script from GBH into Hyku Co-authored-by: Rob Kaufman <[email protected]> * resolve most rubocop warnings * update rubocop_todo Since we're mostly bringing over the reprocessor script as-is, refactoring the method length is out of scope. * Load HykuKnapsack decorators This commit will allow the HykuKnapsack decorators to be loaded by the application rb as well. * temp: debug build issues * Revert "temp: debug build issues" This reverts commit 06350fc. * Update Bulkrax * Update Bulkrax * Update Bulkrax and Hyrax * 🧹 Update bulkrax gem * Update Bulkrax * 🐛 Fix undefined local variable iiif_print There was a change in this version of IIIF Print that needed an update to routes.rb. Ref: - scientist-softserv/iiif_print#302 * 🐛 Fix collection membership bug (#2169) * 🐛 Fix collection membership bug A hyrax bug made it into Hyku via the collections controller decorator. This removes the unnecessary `collection_params method`. Refs - scientist-softserv/hykuup_knapsack#183 - samvera/hyrax@7add213 * Ignore rubocop error * update Bulkrax * Show PDF.js viewer when PDF.js FlipFlop is true This is a workaround to unblock Mobius ingests. If the feature flipper to display PDF.js is true, we will display that viewer regardless of the property values of :show_pdf_viewer and :show_pdf_download_button. We will revisit valkyrizing this feature fully in the future, when we start the iiif x valkyrie epic. Issue: - scientist-softserv/hykuup_knapsack#185 Co-Authored-By: Kirk Wang <[email protected]> * revert previous commit and update logic of #show_pdf_viewer instead This fixes a failing spec while still rendering PDF.js in the UI Co-Authored-By: Kirk Wang <[email protected]> * Update valkyrie gem * update valkyrie gem * Update bulkrax to fix bug Refs scientist-softserv/hykuup_knapsack#182 * 🧹 Clean up featured works and collections The featured works and collections had some funky css problems where it made sorting them impossible. This commit will override some Hyrax scss and fix some css classes on partials to make the featured works/collections a better user experience. * Update IiifPrint (#2172) Fixes bug deleting works & file sets. Refs scientist-softserv/hykuup_knapsack#187 * ♻️ Favor method_missing over delegation In the `double_combo` branch, we replaced the need for delegation with method_missing. Thus we can remove the delegate declarations and instead rely on method missing behavior. * ♻️ Remove config that was there for flakey tests * Update app/forms/hyrax/forms/admin/appearance_decorator.rb * Favor published valkyrie gem version * 🎁 Valkyrize Reindex jobs (#2173) Addresses all reindex jobs to ensure that they will work for either valkyrie or active fedora objects. Ensures that works and collections are appropriately reindexed as default work and collection images are added OR removed (previously, reindexing didn't occur with removal of default images, resulting in works displaying with a missing image.) * update gemfile to add support for sentry * 🗡 Stab in the dark at fixing an intermittent bug * add conditional to support HykuUp Knapsack specs * add conditional to support HykuUp Knapsack specs * 💄 rubocop fix * 💄 rubocop fix * 🧹 Add core to hash in spec Since we changed the SolrEndpoint, this spec was failing. This change should fix it. * 🧹 Add storage/files as a persisted volumn This commit will add storage/files as a persisted volume for rancher set ups. Hyrax uses this directory to store the binaries. * update bulkrax, hyrax, iiif_print * bump to rc3 --------- Co-authored-by: Kirk Wang <[email protected]> Co-authored-by: Jeremy Friesen <[email protected]> Co-authored-by: Kirk Wang <[email protected]> Co-authored-by: C Barton <[email protected]> Co-authored-by: LaRita Robinson <[email protected]> Co-authored-by: Benjamin Kiah Stroud <[email protected]> Co-authored-by: Rob Kaufman <[email protected]> Co-authored-by: LaRita Robinson <[email protected]>

🧹 Make the relationships job work for Valkyrie

08fd122

This will add a relationships path for Valkyrie objects. It also will add a transactions call so set child flag will fire off in IIIF Print. ref: - scientist-softserv/hykuup_knapsack#141

kirkkwang added the patch-ver for release notes label Feb 2, 2024

ShanaLMoore mentioned this pull request Feb 2, 2024

🧹 Fix relationship links samvera/hyku#2161

Merged

2 tasks

💄 rubocop fix

30021c9

Co-Authored-By: Kirk Wang <[email protected]>

ShanaLMoore changed the title ~~🧹 Make the relationships job work for Valkyrie~~ 🧹 Make CreateRelationshipJob work for Valkyrie Feb 2, 2024

ShanaLMoore requested review from jeremyf and laritakr February 2, 2024 16:00

jeremyf reviewed Feb 2, 2024

View reviewed changes

jeremyf added 2 commits February 2, 2024 11:54

♻️ Adjust rescue logic to move closer to error

60aff98

This also adds some consideration for refactoring the queries to instead use the persistence layer.

Adding notes about transactions

f889c31

jeremyf approved these changes Feb 2, 2024

View reviewed changes

ShanaLMoore reviewed Feb 2, 2024

View reviewed changes

ShanaLMoore merged commit 4844893 into hyrax-4-valkyrie-support Feb 2, 2024
6 checks passed

ShanaLMoore deleted the hyrax-4-valkyrie-support-transactions-in-relationships-job branch February 2, 2024 17:21

no-reply reviewed Feb 5, 2024

View reviewed changes

kirkkwang mentioned this pull request Feb 5, 2024

Change IIIF Print set child flag Transaction to a Listener scientist-softserv/iiif_print#330

Open

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🧹 Make CreateRelationshipJob work for Valkyrie #908

🧹 Make CreateRelationshipJob work for Valkyrie #908

kirkkwang commented Feb 2, 2024 •

edited by ShanaLMoore

Loading

jeremyf Feb 2, 2024

ShanaLMoore Feb 2, 2024

no-reply Feb 5, 2024

kirkkwang Feb 5, 2024

kirkkwang Feb 5, 2024 •

edited

Loading

no-reply Feb 5, 2024

kirkkwang Feb 5, 2024

kirkkwang Feb 5, 2024

kirkkwang Feb 6, 2024 •

edited

Loading

🧹 Make CreateRelationshipJob work for Valkyrie #908

🧹 Make CreateRelationshipJob work for Valkyrie #908

Conversation

kirkkwang commented Feb 2, 2024 • edited by ShanaLMoore Loading

jeremyf Feb 2, 2024

Choose a reason for hiding this comment

ShanaLMoore Feb 2, 2024

Choose a reason for hiding this comment

no-reply Feb 5, 2024

Choose a reason for hiding this comment

kirkkwang Feb 5, 2024

Choose a reason for hiding this comment

kirkkwang Feb 5, 2024 • edited Loading

Choose a reason for hiding this comment

no-reply Feb 5, 2024

Choose a reason for hiding this comment

kirkkwang Feb 5, 2024

Choose a reason for hiding this comment

kirkkwang Feb 5, 2024

Choose a reason for hiding this comment

kirkkwang Feb 6, 2024 • edited Loading

Choose a reason for hiding this comment

kirkkwang commented Feb 2, 2024 •

edited by ShanaLMoore

Loading

kirkkwang Feb 5, 2024 •

edited

Loading

kirkkwang Feb 6, 2024 •

edited

Loading