-
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
Replace calls to ActiveFedora::Base.search_by_id #3935
Conversation
27f082f
to
d12f3fa
Compare
app/models/hyrax/base.rb
Outdated
@@ -0,0 +1,11 @@ | |||
module Hyrax | |||
class Base |
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.
does it make sense for this to live in a Base
class? What about pushing this into a search service?
I think it would be worth looking about in services/hyrax
to see if there's already a natural home for this functionality.
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.
Alternatively, both existing calls seem like they're weird optimizations, designed to avoid loading data from fedora. I wonder if we couldn't refactor them away?
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 I've moved it to Hyrax::SolrService class since the behavior of the method is related to SOLR search. And yes, it seems like something that we could refactor at some point later.
6fabf64
to
78c858e
Compare
Hyrax::Base.search_by_id.
Hyrax::ObjectNotFoundError.
78c858e
to
ec5cf2e
Compare
Fixes #3822
Replace calls to ActiveFedora::Base.search_by_id with Hyrax::Base.search_by_id.
@samvera/hyrax-code-reviewers