Feature/threshold calc

[ciaranschutte]

• add second conditional hits resolution which uses the aggregations calculation
• apply data masking of <threshold value> to doc_count
• add property belowThreshold property to aggregations.buckets

[justincorrigible]

given the meaningful ternaries within createStateTypeDefs relying on negatives, it may be good to keep the same original name here (enableDocumentHits) to facilitate keeping the larger mental context. i.e. changing the name to this synonym doesn't add clarity while it does add complexity, now having to keep in mind "this means that".

...or better yet, make this hideHits if you seek to add clarity through simplicity, with a bonus of "not asking negatives" within that other scope.

[justincorrigible]

guess this is another spot where hideHits would be semantically beneficial.
good comment for context! 👍

[ciaranschutte]

common file for TS for now, there will be cleanup needed

[joneubank]

Are these new enums or moved from somewhere else? I would prefer us not to use TS enums, and if we do we should have constant values (ie. first = 'first' or first = 0) to prevent their literal values changing by re-ordering the enum list in the (likely to cause no error, but can cause real tricky bugs especially if enums are used between client and server). Generally speaking, prefer not to use TS enums at all. Would you consider something like:

const Modes = {
  avg: 'avg',
  max: 'max',
  min: 'min',
  sum: 'sum',
} as const satisfies Record<string, string>; // as const makes the object store literal types, not string type. satisfies provides type system checking to ensure all values are strings.
type Mode = typeof Modes[keyof typeof Modes]; // gets values as union type.

[joneubank]

Generally speaking, I don't create a dedicated "types" files, in favour of keeping types near where they need to be declared. Treat them more akin to exported functions. Theres several different concepts stored in this file, including enums used for config/input to queries, the data structure returned by our es queries, whatever Root and Context is.

I think my advice here is similar to not having "utils" files, instead name the file based on its content and export similar ideas. At the moment, this file is not too large to manage, but I always fear these become dumping grounds of any "type" information.

[ciaranschutte]

this is a point where we hit a wall with TS versus legacy JS. There's a lot of typing to be done because it starts to cascade into lots of JS functions.

[joneubank]

Well organized code overall, thanks for this work. Comments are minor suggestions, no changes required.

[joneubank]

Does this mean that if thresholding is enabled but no limit is set we default to everything being masked since we are using max int?

[joneubank]

Very helpful to see the return type declaration. This one is quite large though and may benefit from using some type aliases. No immediate suggestions from me, but the content of aggregations is probably used in multiple places and a good candidate.

Somethign like (naming only very quickly considered):

type AggregationData = {
    bucket_count: number;
    buckets: Array<{
    doc_count: number;
    key: string;
  }>;
}

const applyAggregationMasking(...): { aggregations: Record<string, AggregationData>, thresholdMin: number } => {...}

[joneubank]

Helpful comment, thank you.

[joneubank]

Are these new enums or moved from somewhere else? I would prefer us not to use TS enums, and if we do we should have constant values (ie. first = 'first' or first = 0) to prevent their literal values changing by re-ordering the enum list in the (likely to cause no error, but can cause real tricky bugs especially if enums are used between client and server). Generally speaking, prefer not to use TS enums at all. Would you consider something like:

const Modes = {
  avg: 'avg',
  max: 'max',
  min: 'min',
  sum: 'sum',
} as const satisfies Record<string, string>; // as const makes the object store literal types, not string type. satisfies provides type system checking to ensure all values are strings.
type Mode = typeof Modes[keyof typeof Modes]; // gets values as union type.

[joneubank]

Generally speaking, I don't create a dedicated "types" files, in favour of keeping types near where they need to be declared. Treat them more akin to exported functions. Theres several different concepts stored in this file, including enums used for config/input to queries, the data structure returned by our es queries, whatever Root and Context is.

I think my advice here is similar to not having "utils" files, instead name the file based on its content and export similar ideas. At the moment, this file is not too large to manage, but I always fear these become dumping grounds of any "type" information.