OpenSearch and GraphQL for multiple source filters #534
Conversation
There was a problem hiding this comment.
I think a comment needs to be updated in one of the tests, but otherwise this is looking great and seems to be working as expected. 
Good call-out about how this might affect V1 consumers. I think we can add that to the checklist before we deploy to prod. Fwiw, I seem to be able to filter by source using the V1 endpoint on this PR, but I'd want to do a little more testing to confirm that.
| test 'graphqlv2 filter multiple sources' do | ||
| VCR.use_cassette('graphql v2 filter multiple sources') do | ||
|
|
||
| # no filters return 4 sources |
There was a problem hiding this comment.
Is this asserted anywhere? I don't see that count confirmed in the test.
There was a problem hiding this comment.
doh, left that comment in and refactored the test to not be based on a specific number and instead just "there should be more than the filtered count" to make this less fragile. I'll adjust or remove the comments.
There was a problem hiding this comment.
Ah, that explains it. Yeah, I agree that this testing approach is more robust.
| json = JSON.parse(response.body) | ||
| initial_source_array = json['data']['search']['aggregations']['source'] | ||
|
|
||
| # filtering to 2 sources returns 2 sources |
There was a problem hiding this comment.
Should this be filtering to 1 source returns 1 source?
Why are these changes being introduced: * Being able to search multiple source at once is desired Relevant ticket(s): * https://mitlibraries.atlassian.net/browse/RDI-170 How does this address that need: * Creates a method to add "OR" filtering in OpenSearch * Updates GraphQL to expect source as an array of keywords instead of a single string Document any side effects to this change: * GraphQL previously expected Source as a single string and now expects an array of strings. As V2 is not yet launched, this isn't a concern for V2, but may require changes for V1 consumers including Bento which filters to just ArchivesSpace.
JPrevost
force-pushed
the
rdi-170-searching-multiple-sources
branch
from
1bef1e7 to
00a3f15
Compare
Why are these changes being introduced:
Relevant ticket(s):
How does this address that need:
a single string
Document any side effects to this change:
an array of strings. As V2 is not yet launched, this isn't a concern
for V2, but may require changes for V1 consumers including Bento which
filters to just ArchivesSpace.
Developer
our guide and
all issues introduced by these changes have been resolved or opened as new
issues (link to those issues in the Pull Request details above)
Code Reviewer
(not just this pull request message)
Changes GraphQL Schema
YES, in a way that might cause issues when we enable v2 (haven't confirmed fully yet)
Requires database migrations?
NO
Includes new or updated dependencies?
NO