search for legacy collections in statistics OverTime

[no-reply]

even if a different collection class is used, search for legacy models in
Collections::OverTime. Valkyrie collections indexed via ActiveFedora by
Wings will still be indexed as Collection.

@samvera/hyrax-code-reviewers

[elrayle]

May want to make changes here. See comment about code change.

[elrayle]

Situations like this are why I put together PR #5207. I wonder if the first commit sets the stage for addressing several use cases. In the current definition, it uses...

   def collection_classes
      [collection_model.constantize, Hyrax::PcdmCollection].uniq
    end

I know you expressed some concerns about this initial approach. I've been batting this around and wonder if a more flexible approach would be...

   def collection_classes
      klasses = [collection_class, Hyrax::PcdmCollection]
      klasses << ::Collection if defined? ::Collection
      klasses.uniq
    end

This should address some, if not all, of the failing tests in that PR. And the change here would become .new(allowable_types: Hyrax.config.collection_classes

I'll try updating that PR to determine if it addresses the failing tests.

[no-reply]

i still see value in pursuing the ultimate goal of #5207, but i'm not sure your change there addresses my concern. specifically: i still think that the set of collection classes we care about seems to be differing on a case by case basis. from my comment there #5207 (comment):

in some cases (e.g. on the search side) i think we want/need to continue to find indexed data under model ::Collection. in part this is because Wings indexes to the Solr core used by ActiveFedora using that Collection model. but it also seems nicer to downstream apps opting into valkyrie anyway---they may not want to force a large scale re-index as a prerequisite to going live with a config switch.

on the flip side, i think we've encountered some other cases where we want to include Hyrax::PcdmCollection in the config regardless of settings. you've covered this case here, but i think maybe this blanket approach is too blunt (or at least that's a possible interpretation of some of the failing tests?).

i still don't feel clear on when we want to include which classes where so i'm worried that seeking a singular solution is premature.

all this said: it does seem like #5207 will pass if rebased on #5229, and i'm willing to give it a shot as a first step.

[elrayle]

I addressed the specific issue you point out about ::Collection needing to be in the list by including it if it is defined.

This change in Hyrax.collection_classes means that there may be more classes than we need in the list (i.e. [::Collection, Hydra::PcdmCollection, Hyrax.collection_class]). In most situations I am seeing, it is fine for there to be more classes than needed, but would be a problem if there were less classes (e.g. missing ::Collection when it is not configured as the collection_class keeps it out of the filter for search builders).

Seems like it might work to have Hyrax.collection_classes as the default approach and for any where having more classes listed is not ok, handle those as one-off approaches.

Your thoughts or concerns?

[no-reply]

means that there may be more classes than we need in the list (i.e. [::Collection, Hydra::PcdmCollection, Hyrax.collection_class]).

i think there's a good argument to be made that ::Collection always needs to be in this list for search builders, since much of our current code indexes as "Collection" regardless of what collection model class is configured.

until we can guarantee that this isn't the case anymore, it seems preferable to include it. i think what i want to see to move #5207 forward is some specific consideration for the strategy for cutting away from this as the status quo.

[elrayle]

Since we have a configuration that identifies the collection class, it seems like long term, we would want to remove all references to ::Collection. Not doing this requires apps to have a ::Collection class even if they are using a different class for collections.

It sounds like the problem you are trying to solve is the need to have solr docs that have has_model_ssim:Collection return along with other collection results even if the app has migrated to using a different collection class. I wonder if there is a better way to do that other than forcing an app to have a ::Collection class.

The simple solution would be a recommendation that if a site wants to include finding has_model_ssim:Collection with other collections, then define a class named ::Collection in the app. Otherwise, it is not required.

[no-reply]

is there any chance we could move this forward?

i agree that in the long term removing ::Collection references is desirable. however, there are already ::Collection references in the code base, and i don't think we should block adding statistics support for Hyrax::PcdmCollection pending their removal.

[stale]

This issue has been automatically marked as stale because it has not had activity for 30 days. It will be closed if no further activity occurs within 14 days. Thank you for your contributions.