Merged
Conversation
#1027 Refactors the CreateRelationshipsJob to lock child records when necessary, to ensure that concurrent jobs do not interfere with each other. Fixes the save! method in the valkyrie_object_factory because the resources were not being saved correctly since they respond to save... they were being saved directly by calling save rather than by using the persister.
Valkyrie works were going through the save path in the valkyrie object factory. This means the event notifications weren't being sent. When this was fixed, it became clear the error handling in the create_relationships_job was not correct, as the event jobs needed to include a user. The relationship update was failing because it was defined incorrectly.
laritakr
added a commit
to notch8/adventist_knapsack
that referenced
this pull request
Apr 21, 2025
# Story Creating child works with slugs did not correctly build and display the works and relationships. Creating through the UI did not show the relationships, but creating through Bulkrax did not work at all. Member relationships use the object's id. However the solr document is indexed with the slug as the id. We have to ensure that we search the appropriate terms in the solr document when we are searching by id. This commit fixes two aspects of child works with slugs: 1. It ensures that a work can find its child works with slugs. 2. It ensures that a work with slugs can find its parent work. This preparation is needed before [bulkrax relationship fixes](samvera/bulkrax#1028) are incorporated into the application, as they would break the UI if they used slugs. Note: The AdlSolrDocumentDecorator required refactoring in order to use `prepend` to have the instance method override Hyrax's SolrDocument. Previously methods were added via `include`, and nothing was overridden. # Expected Behavior Before Changes When creating a child work with a slug in the UI, the relationship was not shown. Child works and parent works could not form relationships through Bulkrax. # Expected Behavior After Changes Parent works include child works both with and without slugs in the items list. Child works with slugs show their parent work in the relationships section. Filesets still show correctly as part of the work in the items list. # Screenshots / Video <details> <summary>Work with 2 child works</summary> This is a work that has a slugs. It has two child works, one with a slug and one with a UUID. It also has a fileset. It correctly finds its parent work which also has a slug.  </details> # Notes
Ever since valkyrization, the CreateRelationshipsJob specs have been commented out. This commit rewrites them and turns them on.
laritakr
force-pushed
the
i-1027-relationship-race-condition
branch
from
6afd3cc to
70419ec
Compare
Find based on identifier AND the correct importer run
The record locking does not seem to be working as expected. This commit reloads the record immediately prior to updating it, to ensure that the most recent version is used. This should help prevent lost data.
Prior work tried to carry along the updated parent after a relationship was created, but as other jobs could also be updating the parent, we could still lose other changes while still keeping the relationship. To be safer, we now re-fetch the parent immediately before applying the relationship. This should ensure that we always have the latest version of the parent.
orangewolf
approved these changes
May 8, 2025
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary
#1027
Bug fixes:
save!method in thevalkyrie_object_factory- the resources respond to save so they were being saved directly by calling save rather than by using the persister. This should behave the same way, since we includeHyrax::ArResource, but ideallysaveis a fallback, not the default.Notes
Remaining tasks:
Screenshots
Import with mixed types of parents
import1.csv
Three relationship jobs get submitted for the above CSV: COL1 as parent, COL2 as parent, and TAQ1 as parent.
Prior to these changes, Work TAQ1 would always be missing at least one of its three relationships (2 as child of the collections, and 1 as parent to TAQ2).
With these proposed changes, two collections and two works were added. Work TAQ1 was added to COL1 and COL2, and work TAQ2 was added to work TAQ1 and COL1.
Adding relationships to two collections at the same time
120 works were created in one import.
In a second import, two collections were created and associated to all 120 works.