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

find existing records more often, leading to better round tripping experiance #853

Merged
merged 2 commits into from
Aug 25, 2023

Conversation

orangewolf
Copy link
Member

ActiveFedora use to find objects with {field => value}. Now it only reliably works with {solr_name(field) => value} so we use that instead. Also if find by id fails we should still check source_identifier to see if we can find it that way.

@@ -102,7 +103,8 @@ def find_or_create
end

def search_by_identifier
query = { work_identifier =>
work_index = ::ActiveFedora.index_field_mapper.solr_name(work_identifier, :facetable)
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. Will work_identifier always be :facetable? Is there a chance this will different project-to-project, and thus fail sometimes?
  2. Is there a simple way to achieve this without calling ActiveFedora directly? With Valkyrization on the horizon, using ActiveFedora directly is going the way of the dinosaur. If the answer is "no" that's fine, I don't think it should block this PR; this ticket will rework this line later

Copy link
Member Author

@orangewolf orangewolf Aug 24, 2023

Choose a reason for hiding this comment

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

I think the answer with both is to add a new config variable to bulkrax with a default that just is "#{work_identifier}_ssim" and this should be a new ticket for later work

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm reverting this change because it's breaking my ability to import FileSet's using the row.

ref: #867

@orangewolf orangewolf added the patch-ver for release notes label Aug 24, 2023
@orangewolf orangewolf merged commit da5c83b into main Aug 25, 2023
6 of 7 checks passed
@orangewolf orangewolf deleted the where_waldo branch August 25, 2023 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch-ver for release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants