-
Notifications
You must be signed in to change notification settings - Fork 7
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
Details endpoint #286
Details endpoint #286
Conversation
15e5fa2
to
30d0a7f
Compare
|
||
private const val DEFAULT_MIN_PROPORTION = 0.05 | ||
const val AGGREGATED_GROUP_BY_FIELDS_DESCRIPTION = | ||
"The fields to stratify by. If empty, only the overall count is returned" |
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.
I had to look up the word stratify but it seems the right one :)
requestContext.filter = request.sequenceFilters | ||
|
||
return siloQueryModel.getDetails( | ||
request.sequenceFilters, |
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.
Shouldn't this be request.sequenceFilters.filterKeys { !nonSequenceFilterFields.contains(it) },
as for aggregrated? It seems that fields
is also passed as a filter?
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.
Or is it not necessary above?
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.
I checked it: It's actually not necessary, since the deserializer of the SequenceFiltersRequestWithFields
takes care of that.
30d0a7f
to
4171a80
Compare
resolves #283