-
Notifications
You must be signed in to change notification settings - Fork 124
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
Use a custom Indexer for AdminSets #4048
Conversation
2c951d4
to
9236840
Compare
9236840
to
6bf3ff7
Compare
Indexing for admin sets is under-tested. Adding a test for keys helps ensure the behavior stays fixed under upcoming changes.
Avoid deprecated `index.as` syntax for `AdminSet` data. Admin Sets should be able to index using the AdminSetIndexer, rather than the old-style `index.as` syntax.
6bf3ff7
to
1cfabee
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is looking good, but there are two failing tests due to solr field suffix mismatches. I added suggestions that should fix them.
Co-Authored-By: cjcolvar <cjcolvar@indiana.edu>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
@no-reply Maybe this needs a rebase to fix the valkyrie tests? Or invalidating the cache? |
@cjcolvar seems like the failure is an unrelated random error:
I've seen this one before, and it looks like a test order issue to me. |
Avoid deprecated
index.as
syntax forAdminSet
data.Admin Sets should be able to index using the AdminSetIndexer, rather than the
old-style
index.as
syntax.@samvera/hyrax-code-reviewers