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

Replace calls to ActiveFedora::Base.where (removes 7 refs to ActiveFedora) #3820

Closed
elrayle opened this issue Jun 17, 2019 · 4 comments
Closed

Comments

@elrayle
Copy link
Contributor

elrayle commented Jun 17, 2019

Prerequisites

This issue cannot be completed until the following issues are completed first...


Descriptive summary

The ActiveFedora::Base.where is used to apply a solr query and return instances of Active Fedora objects.

It is used to accomplish a few tasks in Hyrax...

  • get a count of objects searching solr and counting results
  • get a count of objects that the current user can see by searching solr, limiting results based on current_ability, and counting the filtered results
  • get a set of objects

This issue involves removing references to ActiveFedora::Base.where.

ActiveFedora code

Active Fedora class being replaced...

Example from app/models/hyrax/collection_type.rb

      ActiveFedora::Base.where(collection_type_gid_ssim: gid.to_s)

To locate, search code for ActiveFedora::Base.where in app. You will also need to search for .where, but some of these will be for non ActiveFedora::Base objects (e.g. Users) and should not be updated as they are using ActiveRecord #where. The ones for ActiveFedora::Base objects (e.g. AdminSet, Collection, Work, FileSet) will need to be updated.

Replacement code

Pseudocode replacement code...

Perhaps create a service that performs all these steps that can be used as a direct replacement for #where.

Considerations

  • Can some uses of where to find related objects be replaced with calls to one of the custom navigation queries or the standard find_references_by query method?
  • Do NOT replace this call if found in lib/wings directories.

Impact on Testing

Not expected to require test changes. All tests should pass with the replacement code.

@elrayle
Copy link
Contributor Author

elrayle commented Jun 17, 2019

Checkout PR #3804 which as a first step replaces ActiveFedora::SolrService with Hyrax::SolrService which continues to call ActiveFedora::SolrService.

Something similar might make a good first PR for this issue.

@lsitu lsitu self-assigned this Aug 29, 2019
@lsitu
Copy link
Contributor

lsitu commented Aug 29, 2019

@no-reply The ActiveFedora::Base.where call is used in lib/wings several times:
https://github.com/samvera/hyrax/blob/master/lib/wings/models/concerns/collection_behavior.rb#L25
https://github.com/samvera/hyrax/blob/master/lib/wings/valkyrie/query_service.rb#L137
https://github.com/samvera/hyrax/blob/master/lib/wings/valkyrie/query_service.rb#L150

It seems like we we shouldn't replace it (see the Considerations section in the description text). Is there anything can do for the ticket?

@elrayle
Copy link
Contributor Author

elrayle commented Jul 13, 2021

Previous Work

PR #4910 replace ActiveFedora where with Valkyrized where

ActiveFedora::Base.where finds objects in the solr index and then returns ActiveFedora::Base objects. The objects themselves are not always needed, but in some cases they are. This PR addresses the need to get objects back.

This adds service FindObjectsViaSolrService which returns ActiveFedora::Base objects or Valkyrie::Resource objects based on the value of use_valkyrie.

PR #4929 replace AF.where in dashboard_helper_behavior

Uses Hyrax::SolrQueryBuilderService.construct_query to generate a query that can be submitted to Hyrax::SolrService.

NOTE: Hyrax::SolrQueryBuilderService is deprecated and uses should be replaced with calls to Hyrax::SolrQueryService instead. See PR #4938.

PR #4938 Refactor hyrax search query builder to build and execute queries

Replaces Hyrax::SolrQueryBuilderService, which implemented methods to that cannot be chained, with Hyrax::SolrQueryService, which allows methods to be chained to create more complex queries.

See Issue #4946 which addresses the deprecation.

NOTE: Remaining work for this issue should use Hyrax::SolrQueryService.

PR #5024 replace calls to deprecated SolrQueryBuilderService with SolrQueryService

Updates the same file that was changed in PR #4929, but using SolrQuery Service instead.

PR #5023 deprecate FindObjectsViaSolrService replaced by SolrQueryService#get_objects

Add #get_objects method to SolrQueryService. This allows for the construction of the query by chaining methods and then requesting the solr_docs with #get OR requesting the ActiveFedora::Base or Valkyrie::Resource objects with #get_objects.

elrayle added a commit that referenced this issue Jul 13, 2021
Partially addresses Issue #3820 by removing use of `ActiveFedora::Base #where` method when using `#accessible_by` method.

See original [`#accessible_by`\(https://github.com/samvera/hydra-head/blob/main/hydra-access-controls/lib/active_fedora/accessible_by.rb) method defined in hydra-access-controls.

See Hyrax override of [`#gated_discovery_filters`](https://github.com/samvera/hyrax/blob/b034218b89dde7df534e32d1e5ade9161e129a1d/app/search_builders/hyrax/search_filters.rb#L16-L21).
elrayle added a commit that referenced this issue Jul 13, 2021
Partially addresses Issue #3820 by removing use of `ActiveFedora::Base #where` method when using `#accessible_by` method.

See original [`#accessible_by`\(https://github.com/samvera/hydra-head/blob/main/hydra-access-controls/lib/active_fedora/accessible_by.rb) method defined in hydra-access-controls.

See Hyrax override of [`#gated_discovery_filters`](https://github.com/samvera/hyrax/blob/b034218b89dde7df534e32d1e5ade9161e129a1d/app/search_builders/hyrax/search_filters.rb#L16-L21).
elrayle added a commit that referenced this issue Jul 14, 2021
Partially addresses Issue #3820 by removing use of `ActiveFedora::Base #where` method when using `#accessible_by` method.

See original [`#accessible_by`\(https://github.com/samvera/hydra-head/blob/main/hydra-access-controls/lib/active_fedora/accessible_by.rb) method defined in hydra-access-controls.

See Hyrax override of [`#gated_discovery_filters`](https://github.com/samvera/hyrax/blob/b034218b89dde7df534e32d1e5ade9161e129a1d/app/search_builders/hyrax/search_filters.rb#L16-L21).
elrayle added a commit that referenced this issue Jul 14, 2021
Partially addresses Issue #3820 by removing use of `ActiveFedora::Base #where` method when using `#accessible_by` method.

See original [`#accessible_by`\(https://github.com/samvera/hydra-head/blob/main/hydra-access-controls/lib/active_fedora/accessible_by.rb) method defined in hydra-access-controls.

See Hyrax override of [`#gated_discovery_filters`](https://github.com/samvera/hyrax/blob/b034218b89dde7df534e32d1e5ade9161e129a1d/app/search_builders/hyrax/search_filters.rb#L16-L21).
elrayle added a commit that referenced this issue Jul 14, 2021
Partially addresses Issue #3820 by removing use of `ActiveFedora::Base #where` method when using `#accessible_by` method.

See original [`#accessible_by`\(https://github.com/samvera/hydra-head/blob/main/hydra-access-controls/lib/active_fedora/accessible_by.rb) method defined in hydra-access-controls.

See Hyrax override of [`#gated_discovery_filters`](https://github.com/samvera/hyrax/blob/b034218b89dde7df534e32d1e5ade9161e129a1d/app/search_builders/hyrax/search_filters.rb#L16-L21).
@elrayle elrayle removed their assignment Jul 14, 2021
@elrayle
Copy link
Contributor Author

elrayle commented Jul 14, 2021

Remaining Work after PRs #4910, #4929, and #4938

searching for ActiveFedora::Base.where return 6 matches in 4 files

4 need the equivalent of #accessible_by added as a method in Hyrax::SolrQueryService

2 others should follow the pattern established in #4929 except using Hyrax::SolrQueryService as in PR #5024

.where applied to ActiveFedora::Relation

3 uses of relation.where with relation being an ActiveFedora::Relation that should also follow the pattern established in #4929 except using Hyrax::SolrQueryService as in PR #5024

The three above were worked around for the postgres case in #6088. This won't work for non-postgres, but that's future scope.

a few others that might also be using ActiveFedora where

3 that need further investigation

These three got marked off by #6088 as well. This won't work for non-postgres, but that's future scope.

Examples

PR #5024 uses a single SolrQueryService method to build the query and uses the #count method to get the count in DashboardHelperBehavior.

Hyrax::SolrQueryService.new.with_field_pairs(field_pairs: field_pairs(user)).count

PR #5023 uses chained SolrQueryService methods to build the query and uses #get_objects to get Valkyrie::Resource objects in MultipleMembershipChecker

      Hyrax::SolrQueryService.new
                             .with_model(model: ::Collection)
                             .with_field_pairs(field_pairs: field_pairs, join_with: ' OR ')
                             .get_objects(use_valkyrie: true)

elrayle added a commit that referenced this issue Jul 14, 2021
Partially addresses Issue #3820 by removing use of `ActiveFedora::Base #where` method when using `#accessible_by` method.

See original [`#accessible_by`\(https://github.com/samvera/hydra-head/blob/main/hydra-access-controls/lib/active_fedora/accessible_by.rb) method defined in hydra-access-controls.

See Hyrax override of [`#gated_discovery_filters`](https://github.com/samvera/hyrax/blob/b034218b89dde7df534e32d1e5ade9161e129a1d/app/search_builders/hyrax/search_filters.rb#L16-L21).
elrayle added a commit that referenced this issue Jul 14, 2021
Partially addresses Issue #3820 by removing use of `ActiveFedora::Base #where` method when using `#accessible_by` method.

See original [`#accessible_by`\(https://github.com/samvera/hydra-head/blob/main/hydra-access-controls/lib/active_fedora/accessible_by.rb) method defined in hydra-access-controls.

See Hyrax override of [`#gated_discovery_filters`](https://github.com/samvera/hyrax/blob/b034218b89dde7df534e32d1e5ade9161e129a1d/app/search_builders/hyrax/search_filters.rb#L16-L21).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

No branches or pull requests

6 participants