Fix network agg resolution #885
Conversation
| const bucket = new GraphQLObjectType({ | ||
| name: 'bucket', | ||
| fields: { | ||
| doc_count: { | ||
| type: GraphQLInt, | ||
| }, | ||
| key: { | ||
| type: GraphQLString, | ||
| }, | ||
| }, | ||
| }); | ||
|
|
||
| // TODO: Placeholder to expand type | ||
| const networkAggregations = new GraphQLObjectType({ | ||
| name: 'NetworkAggregations', | ||
| fields: { | ||
| bucket_count: { | ||
| type: GraphQLInt, | ||
| }, | ||
| buckets: { | ||
| type: new GraphQLList(bucket), | ||
| }, |
There was a problem hiding this comment.
extend GQL typings for GQL query response
| /** | ||
| * Converts field/types to GQLObjectType definition shape | ||
| * | ||
| * @example | ||
| * { name: "donor_age", type: "NumericAggregations" } => { donor_age: { type: "NetworkNumericAggregations" } } | ||
| */ | ||
| const convertToGQLObjectType = (networkFieldTypes) => { | ||
| return networkFieldTypes.reduce((allFields, currentField) => { | ||
| const field = { | ||
| [currentField.name]: { type: singleToNetworkAggregationMap.get(currentField.type) }, | ||
| }; | ||
| return { ...allFields, ...field }; | ||
| }, {}); | ||
| }; | ||
|
|
There was a problem hiding this comment.
isolate function
| agg.buckets.forEach((bucket) => { | ||
| const { key, doc_count } = bucket; | ||
| const existingBucketIndex = computedBuckets.findIndex((bucket) => bucket.key === key); | ||
| if (existingBucketIndex !== -1) { | ||
| const existingBucket = computedBuckets[existingBucketIndex]; | ||
| if (existingBucket) { | ||
| // update existing bucket | ||
| computedBuckets[existingBucketIndex] = { | ||
| ...existingBucket, | ||
| doc_count: existingBucket.doc_count + doc_count, | ||
| }; | ||
| } | ||
| } else { | ||
| computedBuckets.push(bucket); | ||
| } |
There was a problem hiding this comment.
Size of buckets can be large
Using forEach to either update "found bucket" or add to resolved buckets
Avoids using functional operations on the entire "resolved buckets" and then having a large array of Buckets and needing to flatten and filter on same Bucket key
No way around searching buckets for existing bucket unfortunately as the data structure is an array and a key lookup is not an option.
There was a problem hiding this comment.
the lookup has always been a pain point... perhaps we should make a ticket for future improvement to make the buckets a dictionary available internally in the Arranger server for this and other lookups, even if it's ultimately turned in the UI as an array for display purposes.
ciaranschutte
requested review from
joneubank,
justincorrigible and
demariadaniel
|
CI failing is known issue |
| }; | ||
| } | ||
| } else { | ||
| computedBuckets.push(bucket); |
There was a problem hiding this comment.
lol I don't think I'll ever be happy/comfortable with mutational logic.
Co-authored-by: Anders Richardsson <[email protected]>
Fixes bucket resolution
example:
Server1 buckets: [Cat, Dog]
Server2 buckets: [Cat, Bird]
Output: [Cat, Dog, Bird]