Backport #5850 #5858
Backport #5850 #5858
Conversation
no-reply
force-pushed
the
nested-graph-backport
branch
from
cef05a3
to
02f5a4c
Compare
|
This backport PR was created before the upstream PR got merged and I think it is missing some of the work that was incorporated later. Specifically this commit: 78a6098 |
|
Tamsin confirmed that max depth was not left out on purpose. Rob did a line by line comparison. Rob will follow up by adding it and we will ping Chris for a final review. |
|
@cjcolvar I went back and compared the final merged version w/ this code. the maxdepth was missing, but I don't see any other missing changes. Does it look like it has parity to you now? |
drop `CollectionNesting` depending on configuration and emit deprecation warnings to encourage adopters to use the new approach.
orangewolf
force-pushed
the
nested-graph-backport
branch
from
78c3449
to
962142b
Compare
There was a problem hiding this comment.
What about adding a loop over all of the form tests (spec/forms/hyrax/forms/dashboard/nest_collection_form_spec.rb) to check they work with nested indexing enabled and disabled?
I think that along with the tests already added should cover testing the exact query and the user-facing functionality.
app/search_builders/hyrax/dashboard/nested_collections_search_builder.rb
Outdated
Show resolved
Hide resolved
…builder.rb Co-authored-by: Chris Colvard <chris.colvard@gmail.com>
that makes sense. I likely cant do it today, but I'll see if can get to it tomorrow or more likely Friday. |
Prior to this commit, we had disabled the logic for indexing the parent child relationships of objects. This results in broken behavior regarding nesting collections and works. Were we to enable the "classic" strategy for indexing, we created significant (and known) performance issues. With this commit, we are taking code that was backported to the 3.x branch of Hyrax via [samvera/hyrax#5858][5858] and [samvera/hyrax#5916][5916] and projecting that onto the 2.9.x implementation. This projection means that we are not directly backporting but instead structuring the code such that if you include it, it assumes you using SOLR's native graph indexing. The aforementioned PRs put those changes behind a configuration variable; which is not something that makes sense given this "hack". This PR aims to resolve three issues, which by their nature, are inter-related. The issues are: - [x] Adding works to a collection [#59][59] - [x] Backporting graph indexing [#68][68] - [ ] Adding a collection to another collection [#70][70] [59]:https://github.com/scientist-softserv/adventist-dl/issues/59 [68]:https://github.com/scientist-softserv/adventist-dl/issues/68 [70]:https://github.com/scientist-softserv/adventist-dl/issues/70 [5858]:samvera/hyrax#5858 [5916]:samvera/hyrax#5916
Prior to this commit, we had disabled the logic for indexing the parent child relationships of objects. This results in broken behavior regarding nesting collections and works. Were we to enable the "classic" strategy for indexing, we created significant (and known) performance issues. With this commit, we are taking code that was backported to the 3.x branch of Hyrax via [samvera/hyrax#5858][5858] and [samvera/hyrax#5916][5916] and projecting that onto the 2.9.x implementation. This projection means that we are not directly backporting but instead structuring the code such that if you include it, it assumes you using SOLR's native graph indexing. The aforementioned PRs put those changes behind a configuration variable; which is not something that makes sense given this "hack." This PR aims to resolve three issues, which by their nature, are inter-related. The issues are: - [x] Adding works to a collection [#59][59] - [x] Backporting graph indexing [#68][68] - [x] Adding a collection to another collection [#70][70] [59]: https://github.com/scientist-softserv/adventist-dl/issues/59 [68]: https://github.com/scientist-softserv/adventist-dl/issues/68 [70]: https://github.com/scientist-softserv/adventist-dl/issues/70 [5858]: samvera/hyrax#5858 [5916]: samvera/hyrax#5916
This code introduces the concept of a PR for Hyrax v3: - https://github.com/samvera/hyrax/blob/88c068433c1585400f7a7017e0d8dd6b8f77b33c/lib/hyrax/configuration.rb#L882C1-L889 Related to: - samvera/hyrax#5858
limited/guarded backport of query portion; following this up with deprecations, test updates, etc...
@samvera/hyrax-code-reviewers