From 92f0832368d14646fca22da4d8ac341f64a3cf72 Mon Sep 17 00:00:00 2001 From: anandtiwary <52081890+anandtiwary@users.noreply.github.com> Date: Thu, 11 Mar 2021 23:38:06 -0800 Subject: [PATCH 1/6] refactor: wip --- .../multi-select/multi-select.component.ts | 4 +- projects/observability/src/public-api.ts | 4 + .../field-value/field-value.service.ts | 75 +++++++++++++++++++ 3 files changed, 81 insertions(+), 2 deletions(-) create mode 100644 projects/observability/src/shared/services/field-value/field-value.service.ts diff --git a/projects/components/src/multi-select/multi-select.component.ts b/projects/components/src/multi-select/multi-select.component.ts index b842283de..9bdce2b41 100644 --- a/projects/components/src/multi-select/multi-select.component.ts +++ b/projects/components/src/multi-select/multi-select.component.ts @@ -12,7 +12,7 @@ import { import { IconType } from '@hypertrace/assets-library'; import { LoggerService, queryListAndChanges$, TypedSimpleChanges } from '@hypertrace/common'; import { EMPTY, merge, Observable, of } from 'rxjs'; -import { map, switchMap } from 'rxjs/operators'; +import { map, switchMap, tap } from 'rxjs/operators'; import { IconSize } from '../icon/icon-size'; import { SearchBoxDisplayMode } from '../search-box/search-box.component'; import { SelectOption } from '../select/select-option'; @@ -140,7 +140,6 @@ export class MultiSelectComponent implements AfterContentInit, OnChanges { public ngAfterContentInit(): void { this.selected$ = this.buildObservableOfSelected(); this.setTriggerLabel(); - this.filteredItems = this.items?.toArray(); } public ngOnChanges(changes: TypedSimpleChanges): void { @@ -209,6 +208,7 @@ export class MultiSelectComponent implements AfterContentInit, OnChanges { return queryListAndChanges$(this.items).pipe( switchMap(items => merge(of(undefined), ...items.map(option => option.optionChange$))), + tap(() => this.filteredItems = this.items?.toArray()), map(() => this.findItems(this.selected) ?? []) ); } diff --git a/projects/observability/src/public-api.ts b/projects/observability/src/public-api.ts index d720965dd..d9fd04392 100644 --- a/projects/observability/src/public-api.ts +++ b/projects/observability/src/public-api.ts @@ -2,6 +2,7 @@ * Public API Surface of observability */ + // Schema export * from './shared/graphql/model/schema/entity'; @@ -17,6 +18,9 @@ export * from './shared/graphql/request/builders/selections/observability-specif export * from './shared/graphql/request/builders/specification/entity/entity-specification-builder'; export * from './shared/graphql/request/builders/specification/explore/explore-specification-builder'; +// Field Value +export * from './shared/services/field-value/field-value.service'; + // Handlers export * from './shared/graphql/request/handlers/entities/query/entities-graphql-query-builder.service'; export * from './shared/graphql/request/handlers/entities/query/entities-graphql-query-handler.service'; diff --git a/projects/observability/src/shared/services/field-value/field-value.service.ts b/projects/observability/src/shared/services/field-value/field-value.service.ts new file mode 100644 index 000000000..d4b0d39c1 --- /dev/null +++ b/projects/observability/src/shared/services/field-value/field-value.service.ts @@ -0,0 +1,75 @@ +import { TimeRange } from './../../../../../common/src/time/time-range'; +import { + ExploreGraphQlQueryHandlerService, + EXPLORE_GQL_REQUEST, + GraphQlExploreRequest, + GraphQlExploreResponse +} from './../../graphql/request/handlers/explore/explore-graphql-query-handler.service'; +import { ExploreSpecificationBuilder } from './../../graphql/request/builders/specification/explore/explore-specification-builder'; +import { switchMap, shareReplay, map } from 'rxjs/operators'; +import { Injectable } from '@angular/core'; +import { GraphQlRequestService } from '@hypertrace/graphql-client'; +import { Observable } from 'rxjs'; +import { ExploreSpecification } from '../../graphql/model/schema/specifications/explore-specification'; +import { GraphQlFilter, GraphQlTimeRange, MetricAggregationType } from '@hypertrace/distributed-tracing'; +import { TimeRangeService } from '@hypertrace/common'; + +@Injectable({ providedIn: 'root' }) +export class FieldValueService { + private readonly exploreSpecificationBuilder: ExploreSpecificationBuilder = new ExploreSpecificationBuilder(); + + public constructor( + private readonly graphQlRequestService: GraphQlRequestService, + private readonly timeRangeService: TimeRangeService + ) {} + + public getUniqueValues( + scope: string, + field: string, + filters: GraphQlFilter[] = [], + limit: number = 10 + ): Observable { + return this.timeRangeService.getTimeRangeAndChanges().pipe( + switchMap(timeRange => { + const selections = this.buildExploreSelections(field); + const exploreRequest = this.buildUniqueValueExploreRequest(scope, selections, timeRange, filters, limit); + + return this.graphQlRequestService.query(exploreRequest).pipe( + map((response: GraphQlExploreResponse) => + response.results.map(result => result[selections[0].resultAlias()].value as string) + ), + shareReplay(1) + ); + }) + ); + } + + protected buildUniqueValueExploreRequest( + scope: string, + selections: ExploreSpecification[], + timeRange: TimeRange, + filters: GraphQlFilter[] = [], + limit: number = 10 + ): GraphQlExploreRequest { + return { + requestType: EXPLORE_GQL_REQUEST, + selections: selections, + context: scope, + limit: limit, + includeTotal: true, + timeRange: new GraphQlTimeRange(timeRange.startTime, timeRange.endTime), + filters: filters, + groupBy: { + keys: [selections[0].name], + includeRest: false + } + }; + } + + private buildExploreSelections(field: string): ExploreSpecification[] { + return [ + this.exploreSpecificationBuilder.exploreSpecificationForKey(field), + this.exploreSpecificationBuilder.exploreSpecificationForKey(field, MetricAggregationType.Count) + ]; + } +} From 1cda1c7d2e407e8652933fd09ccce020dce6c6f5 Mon Sep 17 00:00:00 2001 From: anandtiwary <52081890+anandtiwary@users.noreply.github.com> Date: Mon, 15 Mar 2021 16:28:20 -0700 Subject: [PATCH 2/6] refactor: adding more tests --- .../multi-select/multi-select.component.scss | 6 + .../multi-select.component.test.ts | 100 +++++++++++---- .../multi-select/multi-select.component.ts | 118 +++++++++--------- .../src/multi-select/multi-select.module.ts | 4 +- projects/observability/src/public-api.ts | 3 - .../field-value/field-value.service.ts | 75 ----------- 6 files changed, 142 insertions(+), 164 deletions(-) delete mode 100644 projects/observability/src/shared/services/field-value/field-value.service.ts diff --git a/projects/components/src/multi-select/multi-select.component.scss b/projects/components/src/multi-select/multi-select.component.scss index 1d5ec38f3..f7bd7912f 100644 --- a/projects/components/src/multi-select/multi-select.component.scss +++ b/projects/components/src/multi-select/multi-select.component.scss @@ -63,6 +63,12 @@ .multi-select-content { @include dropdown(6px); min-width: 120px; + display: flex; + flex-direction: column; + + .divider { + padding: 0px 16px 4px 16px;; + } .multi-select-option { display: flex; diff --git a/projects/components/src/multi-select/multi-select.component.test.ts b/projects/components/src/multi-select/multi-select.component.test.ts index 0c9aa3f00..be1a4d83e 100644 --- a/projects/components/src/multi-select/multi-select.component.test.ts +++ b/projects/components/src/multi-select/multi-select.component.test.ts @@ -1,11 +1,18 @@ +import { CommonModule } from '@angular/common'; import { fakeAsync, flush } from '@angular/core/testing'; import { IconType } from '@hypertrace/assets-library'; -import { SearchBoxComponent } from '@hypertrace/components'; +import { NavigationService } from '@hypertrace/common'; +import { mockProvider } from '@ngneat/spectator'; import { createHostFactory, SpectatorHost } from '@ngneat/spectator/jest'; import { MockComponent } from 'ng-mocks'; +import { NEVER } from 'rxjs'; +import { ButtonComponent } from '../button/button.component'; import { DividerComponent } from '../divider/divider.component'; import { LabelComponent } from '../label/label.component'; import { LetAsyncModule } from '../let-async/let-async.module'; +import { PopoverComponent } from '../popover/popover.component'; +import { PopoverModule } from '../popover/popover.module'; +import { SearchBoxComponent } from '../search-box/search-box.component'; import { SelectOptionComponent } from '../select/select-option.component'; import { MultiSelectJustify } from './multi-select-justify'; import { MultiSelectComponent, TriggerLabelDisplayMode } from './multi-select.component'; @@ -13,9 +20,19 @@ import { MultiSelectComponent, TriggerLabelDisplayMode } from './multi-select.co describe('Multi Select Component', () => { const hostFactory = createHostFactory>({ component: MultiSelectComponent, - imports: [LetAsyncModule], - entryComponents: [SelectOptionComponent], - declarations: [MockComponent(LabelComponent), MockComponent(DividerComponent), MockComponent(SearchBoxComponent)], + imports: [LetAsyncModule, PopoverModule, CommonModule], + providers: [ + mockProvider(NavigationService, { + navigation$: NEVER + }) + ], + declarations: [ + SelectOptionComponent, + MockComponent(LabelComponent), + MockComponent(DividerComponent), + MockComponent(SearchBoxComponent), + MockComponent(ButtonComponent) + ], shallow: true }); @@ -30,21 +47,41 @@ describe('Multi Select Component', () => { test('should display initial selections', fakeAsync(() => { spectator = hostFactory( ` - + `, { hostProps: { options: selectionOptions, - selected: [selectionOptions[1].value] + selected: [selectionOptions[1].value], + triggerLabelDisplayMode: TriggerLabelDisplayMode.Selection } } ); + spectator.tick(); + expect(spectator.component.triggerLabel).toEqual(selectionOptions[1].label); + expect(spectator.query('.trigger-content')).toExist(); + expect(spectator.query('.trigger-label-container')).toExist(); + expect(spectator.query('.trigger-label')).toExist(); + expect(spectator.query('.trigger-icon')).toExist(); + + const popoverComponent = spectator.query(PopoverComponent); + expect(popoverComponent?.closeOnClick).toEqual(false); + expect(popoverComponent?.closeOnNavigate).toEqual(true); + spectator.click('.trigger-content'); - expect(spectator.element).toHaveText(selectionOptions[1].label); + + expect(spectator.query('.multi-select-content', { root: true })).toExist(); + expect(spectator.query('.multi-select-content .search-bar', { root: true })).toExist(); + expect(spectator.query('.multi-select-content .multi-select-option', { root: true })).toExist(); + + expect(spectator.query('.multi-select-content', { root: true })).toExist(); + const optionElements = spectator.queryAll('.multi-select-option', { root: true }); + + expect(optionElements.length).toEqual(3); spectator.setHostInput({ selected: [selectionOptions[1].value, selectionOptions[2].value] @@ -144,12 +181,12 @@ describe('Multi Select Component', () => { flush(); })); - test('should notify and update selection when all checkbox is selected', fakeAsync(() => { + test('should show select all and clear selected buttons', fakeAsync(() => { const onChange = jest.fn(); spectator = hostFactory( ` - + `, @@ -158,7 +195,6 @@ describe('Multi Select Component', () => { options: selectionOptions, selected: [selectionOptions[1].value], placeholder: 'Select options', - showAllOptionControl: true, onChange: onChange } } @@ -167,20 +203,27 @@ describe('Multi Select Component', () => { spectator.tick(); spectator.click('.trigger-content'); - const allOptionElement = spectator.query('.all-options', { root: true }); - expect(allOptionElement).toExist(); - spectator.click(allOptionElement!); + expect(spectator.component.isAnyOptionSelected()).toEqual(true); + const clearSelectedButton = spectator.query('.clear-selected', { root: true }); + expect(clearSelectedButton).toExist(); + spectator.click(clearSelectedButton!); + spectator.tick(); + expect(spectator.queryAll('input:checked', { root: true }).length).toBe(0); expect(onChange).toHaveBeenCalledTimes(1); - expect(onChange).toHaveBeenCalledWith(selectionOptions.map(option => option.value)); - expect(spectator.query(LabelComponent)?.label).toEqual('first and 2 more'); - - // De select all - spectator.click(allOptionElement!); - expect(onChange).toHaveBeenCalledTimes(2); expect(onChange).toHaveBeenLastCalledWith([]); expect(spectator.query(LabelComponent)?.label).toEqual('Select options'); + const allOptionElement = spectator.query('.select-all', { root: true }); + expect(allOptionElement).toExist(); + spectator.click(allOptionElement!); + + spectator.tick(); + const selectedElements = spectator.queryAll('input:checked', { root: true }); + expect(selectedElements.length).toBe(3); + + expect(onChange).toHaveBeenCalledWith(selectionOptions.map(option => option.value)); + expect(spectator.query(LabelComponent)?.label).toEqual('first and 2 more'); flush(); })); @@ -234,21 +277,23 @@ describe('Multi Select Component', () => { ); spectator.tick(); - expect(spectator.element).toHaveText(selectionOptions[1].label); + expect(spectator.component.triggerLabel).toEqual(selectionOptions[1].label); + expect(spectator.query('.trigger-content')).toExist(); + expect(spectator.query('.trigger-label-container')).toExist(); + expect(spectator.query('.trigger-label')).toExist(); + expect(spectator.query('.trigger-icon')).toExist(); expect(spectator.query('.trigger-content')!.getAttribute('style')).toBe('justify-content: flex-start;'); spectator.setInput({ justify: MultiSelectJustify.Center }); - expect(spectator.element).toHaveText(selectionOptions[1].label); expect(spectator.query('.trigger-content')!.getAttribute('style')).toBe('justify-content: center;'); spectator.setInput({ justify: MultiSelectJustify.Right }); - expect(spectator.element).toHaveText(selectionOptions[1].label); expect(spectator.query('.trigger-content')!.getAttribute('style')).toBe('justify-content: flex-end;'); })); @@ -267,18 +312,19 @@ describe('Multi Select Component', () => { } ); - spectator.tick(); - expect(spectator.query('.search-bar')).toExist(); - spectator.click('.search-bar'); + spectator.click('.trigger-content'); + + const searchBar = spectator.query('.search-bar', { root: true }); + expect(searchBar).toExist(); - spectator.triggerEventHandler(SearchBoxComponent, 'valueChange', 'fi'); + spectator.component.searchOptions('fi'); spectator.tick(); let options = spectator.queryAll('.multi-select-option', { root: true }); expect(options.length).toBe(1); expect(options[0]).toContainText('first'); - spectator.triggerEventHandler(SearchBoxComponent, 'valueChange', 'i'); + spectator.component.searchOptions('i'); spectator.tick(); options = spectator.queryAll('.multi-select-option', { root: true }); diff --git a/projects/components/src/multi-select/multi-select.component.ts b/projects/components/src/multi-select/multi-select.component.ts index 9bdce2b41..b82828fb0 100644 --- a/projects/components/src/multi-select/multi-select.component.ts +++ b/projects/components/src/multi-select/multi-select.component.ts @@ -10,12 +10,12 @@ import { QueryList } from '@angular/core'; import { IconType } from '@hypertrace/assets-library'; -import { LoggerService, queryListAndChanges$, TypedSimpleChanges } from '@hypertrace/common'; -import { EMPTY, merge, Observable, of } from 'rxjs'; -import { map, switchMap, tap } from 'rxjs/operators'; +import { LoggerService, queryListAndChanges$ } from '@hypertrace/common'; +import { EMPTY, Observable, Subject, combineLatest, BehaviorSubject } from 'rxjs'; +import { map } from 'rxjs/operators'; +import { ButtonRole, ButtonStyle } from '../button/button'; import { IconSize } from '../icon/icon-size'; import { SearchBoxDisplayMode } from '../search-box/search-box.component'; -import { SelectOption } from '../select/select-option'; import { SelectOptionComponent } from '../select/select-option.component'; import { SelectSize } from '../select/select-size'; import { MultiSelectJustify } from './multi-select-justify'; @@ -33,7 +33,6 @@ import { MultiSelectJustify } from './multi-select-justify'; this.disabled ? 'disabled' : '', this.popoverOpen ? 'open' : '' ]" - *htLetAsync="this.selected$ as selected" > - -
- - Select All -
- - - - -
+ + + + + + +
implements AfterContentInit, OnChanges { public justify: MultiSelectJustify = MultiSelectJustify.Left; @Input() + /** @deprecated */ public showAllOptionControl?: boolean = false; @Input() @@ -128,59 +144,66 @@ export class MultiSelectComponent implements AfterContentInit, OnChanges { public readonly selectedChange: EventEmitter = new EventEmitter(); @ContentChildren(SelectOptionComponent) - public items?: QueryList>; + private readonly allOptionsList?: QueryList>; + private allOptions$!: Observable>>; + + public filteredOptions$!: Observable[]>; + private readonly searchSubject: Subject = new BehaviorSubject(''); public popoverOpen: boolean = false; - public selected$?: Observable[]>; public triggerLabel?: string; - public filteredItems?: SelectOptionComponent[]; - public constructor(private readonly loggerService: LoggerService) {} + public constructor(_loggerService: LoggerService) {} public ngAfterContentInit(): void { - this.selected$ = this.buildObservableOfSelected(); + this.allOptions$ = this.allOptionsList !== undefined ? queryListAndChanges$(this.allOptionsList) : EMPTY; + this.filteredOptions$ = combineLatest([this.allOptions$, this.searchSubject]).pipe( + map(([options, searchText]) => + options.filter(option => option.label.toLowerCase().includes(searchText.toLowerCase())) + ) + ); this.setTriggerLabel(); } - public ngOnChanges(changes: TypedSimpleChanges): void { - if (this.items !== undefined && changes.selected !== undefined) { - this.selected$ = this.buildObservableOfSelected(); - } + public ngOnChanges(): void { this.setTriggerLabel(); } public searchOptions(searchText: string): void { - this.filteredItems = this.items?.filter(item => item.label.toLowerCase().includes(searchText.toLowerCase())); + this.searchSubject.next(searchText); + } + + public onSelectAll(): void { + this.setSelection(this.allOptionsList!.map(item => item.value)); } - public onAllSelectionChange(): void { - this.selected = this.areAllOptionsSelected() ? [] : this.items!.map(item => item.value); // Select All or none - this.setSelection(); + public onClearSelected(): void { + this.setSelection([]); } public isIconOnlyMode(): boolean { return this.triggerLabelDisplayMode === TriggerLabelDisplayMode.Icon; } - public areAllOptionsSelected(): boolean { - return this.selected !== undefined && this.items !== undefined && this.selected.length === this.items.length; + public isAnyOptionSelected(): boolean { + return this.selected !== undefined && this.allOptionsList !== undefined && this.selected.length > 0; } public onSelectionChange(item: SelectOptionComponent): void { - this.selected = this.isSelectedItem(item) + const selected = this.isSelectedItem(item) ? this.selected?.filter(value => value !== item.value) : (this.selected ?? []).concat(item.value); - this.setSelection(); + this.setSelection(selected ?? []); } public isSelectedItem(item: SelectOptionComponent): boolean { return this.selected !== undefined && this.selected.filter(value => value === item.value).length > 0; } - private setSelection(): void { + private setSelection(selected: V[]): void { + this.selected = selected; this.setTriggerLabel(); - this.selected$ = this.buildObservableOfSelected(); this.selectedChange.emit(this.selected); } @@ -191,7 +214,9 @@ export class MultiSelectComponent implements AfterContentInit, OnChanges { return; } - const selectedItems: SelectOptionComponent[] | undefined = this.items?.filter(item => this.isSelectedItem(item)); + const selectedItems: SelectOptionComponent[] | undefined = this.allOptionsList?.filter(item => + this.isSelectedItem(item) + ); if (selectedItems === undefined || selectedItems.length === 0) { this.triggerLabel = this.placeholder; } else if (selectedItems.length === 1) { @@ -200,29 +225,6 @@ export class MultiSelectComponent implements AfterContentInit, OnChanges { this.triggerLabel = `${selectedItems[0].label} and ${selectedItems.length - 1} more`; } } - - private buildObservableOfSelected(): Observable[]> { - if (!this.items) { - return EMPTY; - } - - return queryListAndChanges$(this.items).pipe( - switchMap(items => merge(of(undefined), ...items.map(option => option.optionChange$))), - tap(() => this.filteredItems = this.items?.toArray()), - map(() => this.findItems(this.selected) ?? []) - ); - } - - // Find the select option object for a value - private findItems(value: V[] | undefined): SelectOption[] | undefined { - if (this.items === undefined) { - this.loggerService.warn(`Invalid items for select option '${String(value)}'`); - - return undefined; - } - - return this.items.filter(item => this.isSelectedItem(item)); - } } export const enum TriggerLabelDisplayMode { diff --git a/projects/components/src/multi-select/multi-select.module.ts b/projects/components/src/multi-select/multi-select.module.ts index 25786a4ed..b7131d135 100644 --- a/projects/components/src/multi-select/multi-select.module.ts +++ b/projects/components/src/multi-select/multi-select.module.ts @@ -1,6 +1,7 @@ import { CommonModule } from '@angular/common'; import { NgModule } from '@angular/core'; import { FormsModule } from '@angular/forms'; +import { ButtonModule } from '../button/button.module'; import { DividerModule } from '../divider/divider.module'; import { IconModule } from '../icon/icon.module'; import { LabelModule } from '../label/label.module'; @@ -18,7 +19,8 @@ import { MultiSelectComponent } from './multi-select.component'; LetAsyncModule, PopoverModule, DividerModule, - TraceSearchBoxModule + TraceSearchBoxModule, + ButtonModule ], declarations: [MultiSelectComponent], exports: [MultiSelectComponent] diff --git a/projects/observability/src/public-api.ts b/projects/observability/src/public-api.ts index d9fd04392..5088bb5d5 100644 --- a/projects/observability/src/public-api.ts +++ b/projects/observability/src/public-api.ts @@ -18,9 +18,6 @@ export * from './shared/graphql/request/builders/selections/observability-specif export * from './shared/graphql/request/builders/specification/entity/entity-specification-builder'; export * from './shared/graphql/request/builders/specification/explore/explore-specification-builder'; -// Field Value -export * from './shared/services/field-value/field-value.service'; - // Handlers export * from './shared/graphql/request/handlers/entities/query/entities-graphql-query-builder.service'; export * from './shared/graphql/request/handlers/entities/query/entities-graphql-query-handler.service'; diff --git a/projects/observability/src/shared/services/field-value/field-value.service.ts b/projects/observability/src/shared/services/field-value/field-value.service.ts deleted file mode 100644 index d4b0d39c1..000000000 --- a/projects/observability/src/shared/services/field-value/field-value.service.ts +++ /dev/null @@ -1,75 +0,0 @@ -import { TimeRange } from './../../../../../common/src/time/time-range'; -import { - ExploreGraphQlQueryHandlerService, - EXPLORE_GQL_REQUEST, - GraphQlExploreRequest, - GraphQlExploreResponse -} from './../../graphql/request/handlers/explore/explore-graphql-query-handler.service'; -import { ExploreSpecificationBuilder } from './../../graphql/request/builders/specification/explore/explore-specification-builder'; -import { switchMap, shareReplay, map } from 'rxjs/operators'; -import { Injectable } from '@angular/core'; -import { GraphQlRequestService } from '@hypertrace/graphql-client'; -import { Observable } from 'rxjs'; -import { ExploreSpecification } from '../../graphql/model/schema/specifications/explore-specification'; -import { GraphQlFilter, GraphQlTimeRange, MetricAggregationType } from '@hypertrace/distributed-tracing'; -import { TimeRangeService } from '@hypertrace/common'; - -@Injectable({ providedIn: 'root' }) -export class FieldValueService { - private readonly exploreSpecificationBuilder: ExploreSpecificationBuilder = new ExploreSpecificationBuilder(); - - public constructor( - private readonly graphQlRequestService: GraphQlRequestService, - private readonly timeRangeService: TimeRangeService - ) {} - - public getUniqueValues( - scope: string, - field: string, - filters: GraphQlFilter[] = [], - limit: number = 10 - ): Observable { - return this.timeRangeService.getTimeRangeAndChanges().pipe( - switchMap(timeRange => { - const selections = this.buildExploreSelections(field); - const exploreRequest = this.buildUniqueValueExploreRequest(scope, selections, timeRange, filters, limit); - - return this.graphQlRequestService.query(exploreRequest).pipe( - map((response: GraphQlExploreResponse) => - response.results.map(result => result[selections[0].resultAlias()].value as string) - ), - shareReplay(1) - ); - }) - ); - } - - protected buildUniqueValueExploreRequest( - scope: string, - selections: ExploreSpecification[], - timeRange: TimeRange, - filters: GraphQlFilter[] = [], - limit: number = 10 - ): GraphQlExploreRequest { - return { - requestType: EXPLORE_GQL_REQUEST, - selections: selections, - context: scope, - limit: limit, - includeTotal: true, - timeRange: new GraphQlTimeRange(timeRange.startTime, timeRange.endTime), - filters: filters, - groupBy: { - keys: [selections[0].name], - includeRest: false - } - }; - } - - private buildExploreSelections(field: string): ExploreSpecification[] { - return [ - this.exploreSpecificationBuilder.exploreSpecificationForKey(field), - this.exploreSpecificationBuilder.exploreSpecificationForKey(field, MetricAggregationType.Count) - ]; - } -} From 134c991b506a6c88b146f6b5f9e6deaff6e9a5ac Mon Sep 17 00:00:00 2001 From: anandtiwary <52081890+anandtiwary@users.noreply.github.com> Date: Mon, 15 Mar 2021 17:07:14 -0700 Subject: [PATCH 3/6] chore: downgrading husky --- package-lock.json | 122 +++++++++++++++++- package.json | 2 +- .../multi-select.component.test.ts | 6 +- .../multi-select/multi-select.component.ts | 6 +- .../src/multi-select/multi-select.module.ts | 14 +- 5 files changed, 124 insertions(+), 26 deletions(-) diff --git a/package-lock.json b/package-lock.json index 4af6a717b..b99b08306 100644 --- a/package-lock.json +++ b/package-lock.json @@ -7023,6 +7023,12 @@ "dot-prop": "^5.1.0" } }, + "compare-versions": { + "version": "3.6.0", + "resolved": "https://registry.npmjs.org/compare-versions/-/compare-versions-3.6.0.tgz", + "integrity": "sha512-W6Af2Iw1z4CB7q4uU4hv646dW9GQuBM+YpC0UvUCWSD8w90SJjp+ujJuXaEMtAXBtSqGfMPuFOVn4/+FlaqfBA==", + "dev": true + }, "component-emitter": { "version": "1.3.0", "resolved": "https://registry.npmjs.org/component-emitter/-/component-emitter-1.3.0.tgz", @@ -9769,6 +9775,15 @@ "path-exists": "^4.0.0" } }, + "find-versions": { + "version": "4.0.0", + "resolved": "https://registry.npmjs.org/find-versions/-/find-versions-4.0.0.tgz", + "integrity": "sha512-wgpWy002tA+wgmO27buH/9KzyEOQnKsG/R0yrcjPT9BOFm0zRBVQbZ95nRGXWMywS8YR5knRbpohio0bcJABxQ==", + "dev": true, + "requires": { + "semver-regex": "^3.1.2" + } + }, "findit2": { "version": "2.2.3", "resolved": "https://registry.npmjs.org/findit2/-/findit2-2.2.3.tgz", @@ -10971,10 +10986,82 @@ } }, "husky": { - "version": "5.1.2", - "resolved": "https://registry.npmjs.org/husky/-/husky-5.1.2.tgz", - "integrity": "sha512-lilaRYeDXcAOj8DuRnN9IxUyEMVbYg9rK7yVNkPB5V4hCvxIUxpMeiv9K2h77CE0HzjCnk1Br0oWe1IghXngDQ==", - "dev": true + "version": "4.3.8", + "resolved": "https://registry.npmjs.org/husky/-/husky-4.3.8.tgz", + "integrity": "sha512-LCqqsB0PzJQ/AlCgfrfzRe3e3+NvmefAdKQhRYpxS4u6clblBoDdzzvHi8fmxKRzvMxPY/1WZWzomPZww0Anow==", + "dev": true, + "requires": { + "chalk": "^4.0.0", + "ci-info": "^2.0.0", + "compare-versions": "^3.6.0", + "cosmiconfig": "^7.0.0", + "find-versions": "^4.0.0", + "opencollective-postinstall": "^2.0.2", + "pkg-dir": "^5.0.0", + "please-upgrade-node": "^3.2.0", + "slash": "^3.0.0", + "which-pm-runs": "^1.0.0" + }, + "dependencies": { + "ansi-styles": { + "version": "4.3.0", + "resolved": "https://registry.npmjs.org/ansi-styles/-/ansi-styles-4.3.0.tgz", + "integrity": "sha512-zbB9rCJAT1rbjiVDb2hqKFHNYLxgtk8NURxZ3IZwD3F6NtxbXZQCnnSi1Lkx+IDohdPlFp222wVALIheZJQSEg==", + "dev": true, + "requires": { + "color-convert": "^2.0.1" + } + }, + "chalk": { + "version": "4.1.0", + "resolved": "https://registry.npmjs.org/chalk/-/chalk-4.1.0.tgz", + "integrity": "sha512-qwx12AxXe2Q5xQ43Ac//I6v5aXTipYrSESdOgzrN+9XjgEpyjpKuvSGaN4qE93f7TQTlerQQ8S+EQ0EyDoVL1A==", + "dev": true, + "requires": { + "ansi-styles": "^4.1.0", + "supports-color": "^7.1.0" + } + }, + "color-convert": { + "version": "2.0.1", + "resolved": "https://registry.npmjs.org/color-convert/-/color-convert-2.0.1.tgz", + "integrity": "sha512-RRECPsj7iu/xb5oKYcsFHSppFNnsj/52OVTRKb4zP5onXwVF3zVmmToNcOfGC+CRDpfK/U584fMg38ZHCaElKQ==", + "dev": true, + "requires": { + "color-name": "~1.1.4" + } + }, + "color-name": { + "version": "1.1.4", + "resolved": "https://registry.npmjs.org/color-name/-/color-name-1.1.4.tgz", + "integrity": "sha512-dOy+3AuW3a2wNbZHIuMZpTcgjGuLU/uBL/ubcZF9OXbDo8ff4O8yVp5Bf0efS8uEoYo5q4Fx7dY9OgQGXgAsQA==", + "dev": true + }, + "has-flag": { + "version": "4.0.0", + "resolved": "https://registry.npmjs.org/has-flag/-/has-flag-4.0.0.tgz", + "integrity": "sha512-EykJT/Q1KjTWctppgIAgfSO0tKVuZUjhgMr17kqTumMl6Afv3EISleU7qZUzoXDFTAHTDC4NOoG/ZxU3EvlMPQ==", + "dev": true + }, + "pkg-dir": { + "version": "5.0.0", + "resolved": "https://registry.npmjs.org/pkg-dir/-/pkg-dir-5.0.0.tgz", + "integrity": "sha512-NPE8TDbzl/3YQYY7CSS228s3g2ollTFnc+Qi3tqmqJp9Vg2ovUpixcJEo2HJScN2Ez+kEaal6y70c0ehqJBJeA==", + "dev": true, + "requires": { + "find-up": "^5.0.0" + } + }, + "supports-color": { + "version": "7.2.0", + "resolved": "https://registry.npmjs.org/supports-color/-/supports-color-7.2.0.tgz", + "integrity": "sha512-qpCAvRl9stuOHveKsn7HncJRvv501qIacKzQlO/+Lwxc9+0q2wLyv4Dfvt80/DPn2pqOBsJdDiogXGR9+OvwRw==", + "dev": true, + "requires": { + "has-flag": "^4.0.0" + } + } + } }, "i18next": { "version": "17.3.1", @@ -16618,6 +16705,15 @@ } } }, + "please-upgrade-node": { + "version": "3.2.0", + "resolved": "https://registry.npmjs.org/please-upgrade-node/-/please-upgrade-node-3.2.0.tgz", + "integrity": "sha512-gQR3WpIgNIKwBMVLkpMUeR3e1/E1y42bqDQZfql+kDeXd8COYfM8PQA4X6y7a8u9Ua9FHmsrrmirW2vHs45hWg==", + "dev": true, + "requires": { + "semver-compare": "^1.0.0" + } + }, "png-js": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/png-js/-/png-js-1.0.0.tgz", @@ -18966,6 +19062,12 @@ "resolved": "https://registry.npmjs.org/semver/-/semver-7.3.2.tgz", "integrity": "sha512-OrOb32TeeambH6UrhtShmF7CRDqhL6/5XpPNp2DuRH6+9QLw/orhp72j87v8Qa1ScDkvrrBNpZcDejAirJmfXQ==" }, + "semver-compare": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/semver-compare/-/semver-compare-1.0.0.tgz", + "integrity": "sha1-De4hahyUGrN+nvsXiPavxf9VN/w=", + "dev": true + }, "semver-dsl": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/semver-dsl/-/semver-dsl-1.0.1.tgz", @@ -19000,6 +19102,12 @@ } } }, + "semver-regex": { + "version": "3.1.2", + "resolved": "https://registry.npmjs.org/semver-regex/-/semver-regex-3.1.2.tgz", + "integrity": "sha512-bXWyL6EAKOJa81XG1OZ/Yyuq+oT0b2YLlxx7c+mrdYPaPbnj6WgVULXhinMIeZGufuUBu/eVRqXEhiv4imfwxA==", + "dev": true + }, "send": { "version": "0.17.1", "resolved": "https://registry.npmjs.org/send/-/send-0.17.1.tgz", @@ -23242,6 +23350,12 @@ "integrity": "sha1-2e8H3Od7mQK4o6j6SzHD4/fm6Ho=", "dev": true }, + "which-pm-runs": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/which-pm-runs/-/which-pm-runs-1.0.0.tgz", + "integrity": "sha1-Zws6+8VS4LVd9rd4DKdGFfI60cs=", + "dev": true + }, "wide-align": { "version": "1.1.3", "resolved": "https://registry.npmjs.org/wide-align/-/wide-align-1.1.3.tgz", diff --git a/package.json b/package.json index b4fb2c9ff..05aa7a4b2 100644 --- a/package.json +++ b/package.json @@ -95,7 +95,7 @@ "codelyzer": "^6.0.1", "commitizen": "^4.2.3", "cz-conventional-changelog": "^3.3.0", - "husky": "^5.1.2", + "husky": "^4.3.8", "jest": "^26.6.3", "jest-config": "^26.6.3", "jest-html-reporter": "^3.2.0", diff --git a/projects/components/src/multi-select/multi-select.component.test.ts b/projects/components/src/multi-select/multi-select.component.test.ts index be1a4d83e..9cdcb4318 100644 --- a/projects/components/src/multi-select/multi-select.component.test.ts +++ b/projects/components/src/multi-select/multi-select.component.test.ts @@ -2,14 +2,12 @@ import { CommonModule } from '@angular/common'; import { fakeAsync, flush } from '@angular/core/testing'; import { IconType } from '@hypertrace/assets-library'; import { NavigationService } from '@hypertrace/common'; -import { mockProvider } from '@ngneat/spectator'; -import { createHostFactory, SpectatorHost } from '@ngneat/spectator/jest'; +import { createHostFactory, mockProvider, SpectatorHost } from '@ngneat/spectator/jest'; import { MockComponent } from 'ng-mocks'; import { NEVER } from 'rxjs'; import { ButtonComponent } from '../button/button.component'; import { DividerComponent } from '../divider/divider.component'; import { LabelComponent } from '../label/label.component'; -import { LetAsyncModule } from '../let-async/let-async.module'; import { PopoverComponent } from '../popover/popover.component'; import { PopoverModule } from '../popover/popover.module'; import { SearchBoxComponent } from '../search-box/search-box.component'; @@ -20,7 +18,7 @@ import { MultiSelectComponent, TriggerLabelDisplayMode } from './multi-select.co describe('Multi Select Component', () => { const hostFactory = createHostFactory>({ component: MultiSelectComponent, - imports: [LetAsyncModule, PopoverModule, CommonModule], + imports: [PopoverModule, CommonModule], providers: [ mockProvider(NavigationService, { navigation$: NEVER diff --git a/projects/components/src/multi-select/multi-select.component.ts b/projects/components/src/multi-select/multi-select.component.ts index b82828fb0..faf16bbc3 100644 --- a/projects/components/src/multi-select/multi-select.component.ts +++ b/projects/components/src/multi-select/multi-select.component.ts @@ -10,8 +10,8 @@ import { QueryList } from '@angular/core'; import { IconType } from '@hypertrace/assets-library'; -import { LoggerService, queryListAndChanges$ } from '@hypertrace/common'; -import { EMPTY, Observable, Subject, combineLatest, BehaviorSubject } from 'rxjs'; +import { queryListAndChanges$ } from '@hypertrace/common'; +import { BehaviorSubject, combineLatest, EMPTY, Observable, Subject } from 'rxjs'; import { map } from 'rxjs/operators'; import { ButtonRole, ButtonStyle } from '../button/button'; import { IconSize } from '../icon/icon-size'; @@ -153,8 +153,6 @@ export class MultiSelectComponent implements AfterContentInit, OnChanges { public popoverOpen: boolean = false; public triggerLabel?: string; - public constructor(_loggerService: LoggerService) {} - public ngAfterContentInit(): void { this.allOptions$ = this.allOptionsList !== undefined ? queryListAndChanges$(this.allOptionsList) : EMPTY; this.filteredOptions$ = combineLatest([this.allOptions$, this.searchSubject]).pipe( diff --git a/projects/components/src/multi-select/multi-select.module.ts b/projects/components/src/multi-select/multi-select.module.ts index b7131d135..ea1edf790 100644 --- a/projects/components/src/multi-select/multi-select.module.ts +++ b/projects/components/src/multi-select/multi-select.module.ts @@ -1,27 +1,15 @@ import { CommonModule } from '@angular/common'; import { NgModule } from '@angular/core'; -import { FormsModule } from '@angular/forms'; import { ButtonModule } from '../button/button.module'; import { DividerModule } from '../divider/divider.module'; import { IconModule } from '../icon/icon.module'; import { LabelModule } from '../label/label.module'; -import { LetAsyncModule } from '../let-async/let-async.module'; import { PopoverModule } from '../popover/popover.module'; import { TraceSearchBoxModule } from '../search-box/search-box.module'; import { MultiSelectComponent } from './multi-select.component'; @NgModule({ - imports: [ - FormsModule, - CommonModule, - IconModule, - LabelModule, - LetAsyncModule, - PopoverModule, - DividerModule, - TraceSearchBoxModule, - ButtonModule - ], + imports: [CommonModule, IconModule, LabelModule, PopoverModule, DividerModule, TraceSearchBoxModule, ButtonModule], declarations: [MultiSelectComponent], exports: [MultiSelectComponent] }) From 1b1d758102911445673ea90b7d8215ba9fb8f8a9 Mon Sep 17 00:00:00 2001 From: anandtiwary <52081890+anandtiwary@users.noreply.github.com> Date: Mon, 15 Mar 2021 17:32:13 -0700 Subject: [PATCH 4/6] refactor: fixing formatting --- .../components/src/multi-select/multi-select.component.scss | 2 +- projects/observability/src/public-api.ts | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/projects/components/src/multi-select/multi-select.component.scss b/projects/components/src/multi-select/multi-select.component.scss index f7bd7912f..d576ee82f 100644 --- a/projects/components/src/multi-select/multi-select.component.scss +++ b/projects/components/src/multi-select/multi-select.component.scss @@ -67,7 +67,7 @@ flex-direction: column; .divider { - padding: 0px 16px 4px 16px;; + padding: 0px 16px 4px 16px; } .multi-select-option { diff --git a/projects/observability/src/public-api.ts b/projects/observability/src/public-api.ts index 5088bb5d5..d720965dd 100644 --- a/projects/observability/src/public-api.ts +++ b/projects/observability/src/public-api.ts @@ -2,7 +2,6 @@ * Public API Surface of observability */ - // Schema export * from './shared/graphql/model/schema/entity'; From 4789f70bb01e3d5c28071b99eed64f68041ece5c Mon Sep 17 00:00:00 2001 From: anandtiwary <52081890+anandtiwary@users.noreply.github.com> Date: Mon, 15 Mar 2021 23:36:03 -0700 Subject: [PATCH 5/6] refactor: addressing review comments --- .../multi-select.component.test.ts | 16 ++++++- .../multi-select/multi-select.component.ts | 42 +++++++++---------- 2 files changed, 34 insertions(+), 24 deletions(-) diff --git a/projects/components/src/multi-select/multi-select.component.test.ts b/projects/components/src/multi-select/multi-select.component.test.ts index 9cdcb4318..2d1847767 100644 --- a/projects/components/src/multi-select/multi-select.component.test.ts +++ b/projects/components/src/multi-select/multi-select.component.test.ts @@ -184,7 +184,7 @@ describe('Multi Select Component', () => { spectator = hostFactory( ` - + `, @@ -193,6 +193,7 @@ describe('Multi Select Component', () => { options: selectionOptions, selected: [selectionOptions[1].value], placeholder: 'Select options', + enableSearch: true, onChange: onChange } } @@ -201,6 +202,9 @@ describe('Multi Select Component', () => { spectator.tick(); spectator.click('.trigger-content'); + expect(spectator.query('.search-bar', { root: true })).toExist(); + expect(spectator.query('.divider', { root: true })).toExist(); + expect(spectator.component.isAnyOptionSelected()).toEqual(true); const clearSelectedButton = spectator.query('.clear-selected', { root: true }); expect(clearSelectedButton).toExist(); @@ -222,6 +226,16 @@ describe('Multi Select Component', () => { expect(onChange).toHaveBeenCalledWith(selectionOptions.map(option => option.value)); expect(spectator.query(LabelComponent)?.label).toEqual('first and 2 more'); + + spectator.setHostInput({ + enableSearch: false + }); + + expect(spectator.query('.search-bar', { root: true })).not.toExist(); + expect(spectator.query('.divider', { root: true })).not.toExist(); + expect(spectator.query('.clear-selected', { root: true })).not.toExist(); + expect(spectator.query('.select-all', { root: true })).not.toExist(); + flush(); })); diff --git a/projects/components/src/multi-select/multi-select.component.ts b/projects/components/src/multi-select/multi-select.component.ts index faf16bbc3..1967b8a43 100644 --- a/projects/components/src/multi-select/multi-select.component.ts +++ b/projects/components/src/multi-select/multi-select.component.ts @@ -63,26 +63,26 @@ import { MultiSelectJustify } from './multi-select-justify'; [debounceTime]="200" displayMode="${SearchBoxDisplayMode.NoBorder}" > + + + + + - - - - -
implements AfterContentInit, OnChanges { @Input() public justify: MultiSelectJustify = MultiSelectJustify.Left; - @Input() - /** @deprecated */ - public showAllOptionControl?: boolean = false; - @Input() public triggerLabelDisplayMode: TriggerLabelDisplayMode = TriggerLabelDisplayMode.Selection; From 7c75ed508186f1e4c5ee6ae5c652a000aa0c5ef9 Mon Sep 17 00:00:00 2001 From: anandtiwary <52081890+anandtiwary@users.noreply.github.com> Date: Tue, 16 Mar 2021 11:57:08 -0700 Subject: [PATCH 6/6] refactor: displaying search box and buttons for large options list --- .../multi-select.component.test.ts | 31 +++++++--- .../multi-select/multi-select.component.ts | 56 ++++++++++--------- 2 files changed, 54 insertions(+), 33 deletions(-) diff --git a/projects/components/src/multi-select/multi-select.component.test.ts b/projects/components/src/multi-select/multi-select.component.test.ts index 2d1847767..7a4d30658 100644 --- a/projects/components/src/multi-select/multi-select.component.test.ts +++ b/projects/components/src/multi-select/multi-select.component.test.ts @@ -39,7 +39,10 @@ describe('Multi Select Component', () => { const selectionOptions = [ { label: 'first', value: 'first-value' }, { label: 'second', value: 'second-value' }, - { label: 'third', value: 'third-value' } + { label: 'third', value: 'third-value' }, + { label: 'fourth', value: 'fourth-value' }, + { label: 'fifth', value: 'fifth-value' }, + { label: 'sixth', value: 'sixth-value' } ]; test('should display initial selections', fakeAsync(() => { @@ -79,7 +82,7 @@ describe('Multi Select Component', () => { expect(spectator.query('.multi-select-content', { root: true })).toExist(); const optionElements = spectator.queryAll('.multi-select-option', { root: true }); - expect(optionElements.length).toEqual(3); + expect(optionElements.length).toEqual(6); spectator.setHostInput({ selected: [selectionOptions[1].value, selectionOptions[2].value] @@ -114,7 +117,7 @@ describe('Multi Select Component', () => { spectator.click('.trigger-content'); const optionElements = spectator.queryAll('.multi-select-option:not(.all-options)', { root: true }); expect(spectator.query('.multi-select-content', { root: true })).toExist(); - expect(optionElements.length).toBe(3); + expect(optionElements.length).toBe(6); const selectedElements = spectator.queryAll('input:checked', { root: true }); expect(selectedElements.length).toBe(2); @@ -222,10 +225,10 @@ describe('Multi Select Component', () => { spectator.tick(); const selectedElements = spectator.queryAll('input:checked', { root: true }); - expect(selectedElements.length).toBe(3); + expect(selectedElements.length).toBe(6); expect(onChange).toHaveBeenCalledWith(selectionOptions.map(option => option.value)); - expect(spectator.query(LabelComponent)?.label).toEqual('first and 2 more'); + expect(spectator.query(LabelComponent)?.label).toEqual('first and 5 more'); spectator.setHostInput({ enableSearch: false @@ -333,16 +336,30 @@ describe('Multi Select Component', () => { spectator.tick(); let options = spectator.queryAll('.multi-select-option', { root: true }); - expect(options.length).toBe(1); + expect(options.length).toBe(2); expect(options[0]).toContainText('first'); spectator.component.searchOptions('i'); spectator.tick(); options = spectator.queryAll('.multi-select-option', { root: true }); - expect(options.length).toBe(2); + expect(options.length).toBe(4); expect(options[0]).toContainText('first'); expect(options[1]).toContainText('third'); + + expect(spectator.query('.divider', { root: true })).toExist(); + expect(spectator.query('.clear-selected', { root: true })).not.toExist(); // Due to initial selection + expect(spectator.query('.select-all', { root: true })).toExist(); + + // Set selected options to less than 5 and search box and buttons should hide + spectator.setHostInput({ + options: selectionOptions.slice(0, 3) + }); + + expect(spectator.query('.search-bar', { root: true })).not.toExist(); + expect(spectator.query('.divider', { root: true })).not.toExist(); + expect(spectator.query('.clear-selected', { root: true })).not.toExist(); + expect(spectator.query('.select-all', { root: true })).not.toExist(); flush(); })); }); diff --git a/projects/components/src/multi-select/multi-select.component.ts b/projects/components/src/multi-select/multi-select.component.ts index 1967b8a43..4b55900c7 100644 --- a/projects/components/src/multi-select/multi-select.component.ts +++ b/projects/components/src/multi-select/multi-select.component.ts @@ -57,31 +57,35 @@ import { MultiSelectJustify } from './multi-select-justify';
- - - - - - + + + + + + + + + +
implements AfterContentInit, OnChanges { @ContentChildren(SelectOptionComponent) private readonly allOptionsList?: QueryList>; - private allOptions$!: Observable>>; + public allOptions$!: Observable>>; public filteredOptions$!: Observable[]>; private readonly searchSubject: Subject = new BehaviorSubject('');