feat: filter changes by anandtiwary · Pull Request #1422 · hypertrace/hypertrace-ui · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { ChangeDetectionStrategy, Component, EventEmitter, Input, OnChanges, Out
import { IconType } from '@hypertrace/assets-library';
import { IconSize } from '../../icon/icon-size';
import { FilterBuilderLookupService } from '../filter/builder/filter-builder-lookup.service';
import { Filter } from '../filter/filter';
import { Filter, FilterValue } from '../filter/filter';
import { FilterAttribute } from '../filter/filter-attribute';
import { FilterUrlService } from '../filter/filter-url.service';

Expand Down Expand Up @@ -45,7 +45,7 @@ export class FilterButtonComponent implements OnChanges {
public attribute?: FilterAttribute;

@Input()
public value?: unknown;
public value?: FilterValue;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems completely dependent on the filter implementation - what's the point of typing it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is required for Filter to GqlFilter transformation

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the kind of thing I was saying the other day we shouldn't try to make generic, because there is no one GQL filter representation.


@Output()
public readonly popoverOpen: EventEmitter<boolean> = new EventEmitter();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { sortUnknown } from '@hypertrace/common';
import { ButtonRole } from '../../button/button';
import { ModalRef, MODAL_DATA } from '../../modal/modal';
import { FilterBuilderLookupService } from '../filter/builder/filter-builder-lookup.service';
import { IncompleteFilter } from '../filter/filter';
import { FilterValue, IncompleteFilter } from '../filter/filter';
import { FilterAttribute } from '../filter/filter-attribute';
import { FilterOperator } from '../filter/filter-operators';
import { FilterUrlService } from '../filter/filter-url.service';
Expand Down Expand Up @@ -43,7 +43,7 @@ import { FilterUrlService } from '../filter/filter-url.service';
})
export class InFilterModalComponent {
public isSupported: boolean = false;
public selected: Set<unknown> = new Set<unknown>();
public selected: Set<FilterValue> = new Set<FilterValue>();

public constructor(
private readonly modalRef: ModalRef<never>,
Expand Down Expand Up @@ -91,7 +91,7 @@ export class InFilterModalComponent {
this.modalRef.close();
}

public onChecked(checked: boolean, value: unknown): void {
public onChecked(checked: boolean, value: FilterValue): void {
checked ? this.selected.add(value) : this.selected.delete(value);
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Injectable } from '@angular/core';
import { assertUnreachable } from '@hypertrace/common';
import { FilterValue } from '../filter';
import { FilterAttributeType } from '../filter-attribute-type';
import { AbstractFilterBuilder } from './types/abstract-filter-builder';
import { BooleanFilterBuilder } from './types/boolean-filter-builder';
Expand All @@ -11,7 +12,7 @@ import { StringMapFilterBuilder } from './types/string-map-filter-builder';
providedIn: 'root'
})
export class FilterBuilderLookupService {
public lookup(type: FilterAttributeType): AbstractFilterBuilder<unknown> {
public lookup(type: FilterAttributeType): AbstractFilterBuilder<FilterValue> {
switch (type) {
case FilterAttributeType.Boolean:
return new BooleanFilterBuilder();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import { collapseWhitespace } from '@hypertrace/common';
import { isEmpty } from 'lodash-es';
import { Filter } from '../../filter';
import { Filter, FilterValue, IncompleteFilter } from '../../filter';
import { FilterAttribute } from '../../filter-attribute';
import { FilterAttributeType } from '../../filter-attribute-type';
import { MAP_LHS_DELIMITER } from '../../filter-delimiters';
import { FilterOperator, toUrlFilterOperator } from '../../filter-operators';
import { FilterAttributeExpression } from '../../parser/parsed-filter';

export abstract class AbstractFilterBuilder<TValue> {
export abstract class AbstractFilterBuilder<TValue extends FilterValue> {
public abstract supportedAttributeType(): FilterAttributeType;

public abstract supportedSubpathOperators(): FilterOperator[];
Expand Down Expand Up @@ -47,6 +47,30 @@ export abstract class AbstractFilterBuilder<TValue> {
};
}

public buildPartialFilter(
attribute: FilterAttribute,
operator?: FilterOperator,
value?: TValue,
subpath?: string
): IncompleteFilter<TValue> {
if (
operator !== undefined &&
((isEmpty(subpath) && !this.supportedTopLevelOperators().includes(operator)) ||
(!isEmpty(subpath) && !this.supportedSubpathOperators().includes(operator)))
) {
throw Error(`Operator '${operator}' not supported for filter attribute type '${attribute.type}'`);
}

return {
metadata: attribute,
field: attribute.name,
subpath: subpath,
operator: operator,
value: value,
userString: this.buildUserFilterString(attribute, subpath, operator, value)
};
}

public buildUserFilterString(
attribute: FilterAttribute,
subpath?: string,
Expand Down
16 changes: 16 additions & 0 deletions projects/components/src/filtering/filter/filter-url.service.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Injectable } from '@angular/core';
import { NavigationService } from '@hypertrace/common';
import { remove } from 'lodash-es';
import { Observable } from 'rxjs';
import { map } from 'rxjs/operators';
import { FilterBuilderLookupService } from './builder/filter-builder-lookup.service';
Expand All @@ -25,6 +26,21 @@ export class FilterUrlService {
return this.navigationService.navigation$.pipe(map(() => this.getUrlFilters(attributes)));
}

public getUrlFiltersForAttributes(attributes: FilterAttribute[]): (Filter | IncompleteFilter)[] {
const urlFilters = this.getUrlFilters(attributes);

return attributes.map(attribute => {
const match = urlFilters.find(f => f.field === attribute.name);
if (match !== undefined) {
remove(urlFilters, f => f === match);

return match;
}

return this.filterBuilderLookupService.lookup(attribute.type).buildPartialFilter(attribute);
});
}

public getUrlFilters(attributes: FilterAttribute[]): Filter[] {
return this.navigationService
.getAllValuesForQueryParameter(FilterUrlService.FILTER_QUERY_PARAM)
Expand Down
9 changes: 6 additions & 3 deletions projects/components/src/filtering/filter/filter.ts
Original file line number Diff line number Diff line change
@@ -1,23 +1,26 @@
import { Dictionary } from '@hypertrace/common';
import { FilterAttribute } from './filter-attribute';
import { FilterOperator, incompatibleOperators } from './filter-operators';

export interface Filter<TValue = unknown> extends IncompleteFilter {
export interface Filter<TValue extends FilterValue = FilterValue> extends IncompleteFilter {
operator: FilterOperator;
value: TValue;
urlString: string;
}

export interface IncompleteFilter<TValue = unknown> extends FieldFilter<TValue> {
export interface IncompleteFilter<TValue extends FilterValue = FilterValue> extends FieldFilter<TValue> {
metadata: FilterAttribute;
userString: string;
}

export interface FieldFilter<TValue = unknown> {
export interface FieldFilter<TValue extends FilterValue = FilterValue> {
field: string;
subpath?: string;
operator?: FilterOperator;
value?: TValue;
}

export type FilterValue = string | number | boolean | Date | Dictionary<FilterValue> | FilterValue[];

export const areCompatibleFilters = (f1: Filter, f2: Filter) =>
f1.field !== f2.field || f1.subpath !== f2.subpath || !incompatibleOperators(f1.operator).includes(f2.operator);
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Injectable } from '@angular/core';
import { assertUnreachable } from '@hypertrace/common';
import { FilterValue } from '../filter';
import { FilterAttributeType } from '../filter-attribute-type';
import { FilterOperator } from '../filter-operators';
import { AbstractFilterParser } from './types/abstract-filter-parser';
Expand All @@ -14,7 +15,7 @@ export class FilterParserLookupService {
// TODO remove the separate parsers entirely.
// There's next to no logic left in them, and they duplicate (incorrectly) supported operators,
// Which should be based on attribute type (as defined in filter builders)
public lookup(operator: FilterOperator): AbstractFilterParser<unknown> {
public lookup(operator: FilterOperator): AbstractFilterParser<FilterValue> {
switch (operator) {
case FilterOperator.Equals:
case FilterOperator.NotEquals:
Expand Down
4 changes: 2 additions & 2 deletions projects/components/src/table/table-api.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Dictionary } from '@hypertrace/common';
import { Observable } from 'rxjs';
import { FieldFilter } from '../filtering/filter/filter';
import { FieldFilter, FilterValue } from '../filtering/filter/filter';
import { FilterOperator } from '../filtering/filter/filter-operators';
import { TableCellAlignmentType } from './cells/types/table-cell-alignment-type';

Expand Down Expand Up @@ -60,7 +60,7 @@ export interface RowStateChange {

export interface TableFilter extends FieldFilter {
operator: FilterOperator;
value: unknown;
value: FilterValue;
}

export const enum TableSortDirection {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ export class ExploreVisualizationBuilder implements OnDestroy {
selections: state.series.map(series => series.specification),
context: state.context,
interval: this.resolveInterval(state.interval),
filters: state.filters && this.graphQlFilterBuilderService.buildGraphQlFilters(state.filters),
filters: state.filters && this.graphQlFilterBuilderService.buildGraphQlFieldFilters(state.filters),
groupBy: state.groupBy,
limit: state.resultLimit
});
Expand Down Expand Up @@ -178,7 +178,7 @@ export class ExploreVisualizationBuilder implements OnDestroy {
traceType: traceType,
properties: specifications,
limit: 100,
filters: filters && this.graphQlFilterBuilderService.buildGraphQlFilters(filters)
filters: filters && this.graphQlFilterBuilderService.buildGraphQlFieldFilters(filters)
};
}

Expand All @@ -190,7 +190,7 @@ export class ExploreVisualizationBuilder implements OnDestroy {
requestType: SPANS_GQL_REQUEST,
properties: specifications,
limit: 100,
filters: filters && this.graphQlFilterBuilderService.buildGraphQlFilters(filters)
filters: filters && this.graphQlFilterBuilderService.buildGraphQlFieldFilters(filters)
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,10 @@ export class NavigableDashboardComponent implements OnChanges {
const rootDataSource = dashboard.getRootDataSource<GraphQlFilterDataSourceModel>();
rootDataSource
?.clearFilters()
.addFilters(...this.implicitFilters, ...this.graphQlFilterBuilderService.buildGraphQlFilters(explicitFilters));
.addFilters(
...this.implicitFilters,
...this.graphQlFilterBuilderService.buildGraphQlFieldFilters(explicitFilters)
);
dashboard.refresh();
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { FilterOperator, TableControlOptionType, TableSelectControlOption } from '@hypertrace/components';
import { FilterOperator, FilterValue, TableControlOptionType, TableSelectControlOption } from '@hypertrace/components';
import { Model } from '@hypertrace/hyperdash';
import { Observable } from 'rxjs';
import { map } from 'rxjs/operators';
Expand All @@ -17,7 +17,7 @@ export class EntitiesAttributeOptionsDataSourceModel extends EntitiesAttributeDa
metaValue: {
field: this.specification.name,
operator: FilterOperator.Equals,
value: value
value: value as FilterValue
}
}))
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,6 @@ export abstract class TableDataSourceModel extends GraphQlDataSourceModel<TableD
): TableDataResponse<TableRow>;

protected toGraphQlFilters(tableFilters: TableFilter[] = []): GraphQlFilter[] {
return this.graphQlFilterBuilderService.buildGraphQlFilters(tableFilters);
return this.graphQlFilterBuilderService.buildGraphQlFiltersFromTableFilters(tableFilters);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -615,7 +615,7 @@ export class TableWidgetRendererComponent
private mergeFilters(tableFilter: TableFilter): TableFilter[] {
const existingSelectFiltersWithChangedRemoved = this.removeFilters(tableFilter.field);

return [...existingSelectFiltersWithChangedRemoved, tableFilter].filter(f => f.value !== undefined); // Remove filters that are unset
return [...existingSelectFiltersWithChangedRemoved, tableFilter];
}

private removeFilters(field: string): TableFilter[] {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ describe('Graphql filter builder service', () => {
const spectator = serviceFactory();

expect(
spectator.service.buildGraphQlFilters([
spectator.service.buildGraphQlFieldFilters([
buildFilter(attribute2, FilterOperator.Equals, 'foo'),
buildFilter(attribute2, FilterOperator.NotEquals, 'bar'),
buildFilter(attribute1, FilterOperator.GreaterThan, 5),
Expand Down