Skip to content
This repository was archived by the owner on Nov 14, 2023. It is now read-only.
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
382 changes: 137 additions & 245 deletions package-lock.json

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions projects/assets-library/src/icons/icon-type.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ export const enum IconType {
Remove = 'remove',
RemoveCircle = 'remove_circle',
Restore = 'restore',
Save = 'save',
Search = 'svg:search',
Settings = 'settings',
Share = 'share',
Expand Down
3 changes: 2 additions & 1 deletion projects/common/src/constants/application-constants.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
export const enum ApplicationFeature {
PageTimeRange = 'ui.page-time-range'
PageTimeRange = 'ui.page-time-range',
SavedQueries = 'ui.saved-queries'
}
10 changes: 10 additions & 0 deletions projects/observability/src/pages/explorer/explorer.component.scss
Original file line number Diff line number Diff line change
Expand Up @@ -53,4 +53,14 @@
@include body-1-medium;
}
}

.explorer-save-button {
align-self: flex-end;
margin: 24px 24px 16px;
}

.explorer-toggle-container {
display: flex;
justify-content: space-between;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { RouterTestingModule } from '@angular/router/testing';
import { IconLibraryTestingModule } from '@hypertrace/assets-library';
import {
DEFAULT_COLOR_PALETTE,
FeatureState,
FeatureStateResolver,
LayoutChangeService,
NavigationService,
Expand All @@ -20,6 +21,7 @@ import {
FilterBarComponent,
FilterBuilderLookupService,
FilterOperator,
NotificationService,
ToggleGroupComponent
} from '@hypertrace/components';
import { GraphQlRequestService } from '@hypertrace/graphql-client';
Expand Down Expand Up @@ -87,7 +89,7 @@ describe('Explorer component', () => {
query: jest.fn().mockReturnValueOnce(of(mockAttributes)).mockReturnValue(EMPTY)
}),
mockProvider(FeatureStateResolver, {
getFeatureState: jest.fn().mockReturnValue(of(true))
getFeatureState: jest.fn().mockReturnValue(of(FeatureState.Enabled))
}),
mockProvider(TimeRangeService, {
getCurrentTimeRange: () => testTimeRange,
Expand All @@ -108,7 +110,10 @@ describe('Explorer component', () => {
}
},
mockProvider(PreferenceService, {
get: jest.fn().mockReturnValue(of(true))
get: jest.fn().mockReturnValue(of([]))
}),
mockProvider(NotificationService, {
createSuccessToast: jest.fn()
}),
...getMockFlexLayoutProviders()
]
Expand Down Expand Up @@ -412,4 +417,15 @@ describe('Explorer component', () => {
new TimeDuration(30, TimeUnit.Second)
);
}));

test('shows notification when a query is saved successfully', fakeAsync(() => {
init();
const notificationServiceSpy = spyOn(spectator.inject(NotificationService), 'createSuccessToast');

const saveQueryButton = spectator.query('.explorer-save-button');
expect(saveQueryButton).toExist();

spectator.click(saveQueryButton as HTMLElement);
expect(notificationServiceSpy).toHaveBeenCalledWith('Query Saved Successfully!');
}));
});
61 changes: 53 additions & 8 deletions projects/observability/src/pages/explorer/explorer.component.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,18 @@
import { ChangeDetectionStrategy, Component, Inject } from '@angular/core';
import { ActivatedRoute, ParamMap } from '@angular/router';
import { IconType } from '@hypertrace/assets-library';
import {
ApplicationFeature,
assertUnreachable,
FeatureState,
FeatureStateResolver,
NavigationService,
PreferenceService,
QueryParamObject,
TimeDuration,
TimeDurationService
} from '@hypertrace/common';
import { Filter, ToggleItem } from '@hypertrace/components';
import { ButtonSize, Filter, NotificationService, ToggleItem } from '@hypertrace/components';
import { isEmpty, isNil } from 'lodash-es';
import { concat, EMPTY, Observable, Subject } from 'rxjs';
import { map, take } from 'rxjs/operators';
Expand Down Expand Up @@ -41,19 +45,34 @@ import {
template: `
<div class="explorer" *htLetAsync="this.initialState$ as initialState">
<ht-page-header class="explorer-header"></ht-page-header>
<ht-toggle-group
class="explorer-data-toggle"
[items]="this.contextItems"
[activeItem]="initialState.contextToggle"
(activeItemChange)="this.onContextUpdated($event.value)"
></ht-toggle-group>

<div class="explorer-toggle-container">
<ht-toggle-group
class="explorer-data-toggle"
[items]="this.contextItems"
[activeItem]="initialState.contextToggle"
(activeItemChange)="this.onContextUpdated($event.value)"
></ht-toggle-group>

<ht-button
class="explorer-save-button"
icon="${IconType.Save}"
label="Save Query"
role="tertiary"
size="${ButtonSize.Small}"
[disabled]="filters.length < 1"
(click)="onClickSaveQuery()"
*ngIf="enableSavedQueries"
></ht-button>
</div>

<ht-filter-bar
class="explorer-filter-bar"
[attributes]="this.attributes$ | async"
[syncWithUrl]="true"
(filtersChange)="this.onFiltersUpdated($event)"
></ht-filter-bar>

<div class="explorer-content">
<ht-panel
*htLetAsync="this.visualizationExpanded$ as visualizationExpanded"
Expand Down Expand Up @@ -118,11 +137,14 @@ export class ExplorerComponent {
private static readonly VISUALIZATION_EXPANDED_PREFERENCE: string = 'explorer.visualizationExpanded';
private static readonly RESULTS_EXPANDED_PREFERENCE: string = 'explorer.resultsExpanded';
private readonly explorerDashboardBuilder: ExplorerDashboardBuilder;
private savedQueries: SavedQuery[] = [];
private currentContext: ExplorerGeneratedDashboardContext = ObservabilityTraceType.Api;

Choose a reason for hiding this comment

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

I think we can make a do without having this currentContext variable here.
As to how, we can change the contextChangeSubject (not currentContext$) from Subject to BehaviorSubject.
BehaviorSubject are Subject which directly give us their current value without being subscribed to. Given both of them are referring to type ExplorerGeneratedDashboardContext, we can do contextChangeSubject.getValue(). That will also facilitate setting the initial value :)

Choose a reason for hiding this comment

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

Also, it's more of a nitpick comment, so you can ignore it if you want.

Copy link
Author

Choose a reason for hiding this comment

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

I tried changing the Subject to BehaviourSubject by changing

  private readonly contextChangeSubject: Subject<ExplorerGeneratedDashboardContext> = new Subject();

to

  private readonly contextChangeSubject: BehaviorSubject<ExplorerGeneratedDashboardContext> = new BehaviorSubject(
    ObservabilityTraceType.Api
  );

But TypeScript is going crazy with errors:

Screenshot 2022-05-16 at 2 02 40 PM

Am I doing something wrong?

Copy link

@jaywalker21 jaywalker21 May 16, 2022

Choose a reason for hiding this comment

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

@cseas

Ohh, there are two ways to handle this, that would resolve everything. You can bake it in future PRs if it makes sense.

private readonly contextChangeSubject: BehaviorSubject<ExplorerGeneratedDashboardContext> = new BehaviorSubject(ObservabilityTraceType.Api as ExplorerGeneratedDashboardContext);

OR

private readonly contextChangeSubject: BehaviorSubject<ExplorerGeneratedDashboardContext> = new BehaviorSubject<ExplorerGeneratedDashboardContext>(ObservabilityTraceType.Api);

Copy link
Author

Choose a reason for hiding this comment

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

That worked. Thanks!

Change added to the next PR in the below commit:

ae6eea6

But changing Subject to BehaviorSubject started failing an existing test, couldn't figure out why so I reverted the change.

Here's the failing test:

https://github.com/razorpay/hypertrace-ui/runs/6450485969?check_suite_focus=true#step:4:1235

public readonly resultsDashboard$: Observable<ExplorerGeneratedDashboard>;
public readonly vizDashboard$: Observable<ExplorerGeneratedDashboard>;
public readonly initialState$: Observable<InitialExplorerState>;
public readonly currentContext$: Observable<ExplorerGeneratedDashboardContext>;
public attributes$: Observable<AttributeMetadata[]> = EMPTY;
public enableSavedQueries: boolean = false;

public readonly contextItems: ContextToggleItem[] = [
{
Expand All @@ -148,8 +170,10 @@ export class ExplorerComponent {
private readonly contextChangeSubject: Subject<ExplorerGeneratedDashboardContext> = new Subject();

public constructor(
private readonly featureStateResolver: FeatureStateResolver,
private readonly metadataService: MetadataService,
private readonly navigationService: NavigationService,
private readonly notificationService: NotificationService,
private readonly timeDurationService: TimeDurationService,
private readonly preferenceService: PreferenceService,
@Inject(EXPLORER_DASHBOARD_BUILDER_FACTORY) explorerDashboardBuilderFactory: ExplorerDashboardBuilderFactory,
Expand All @@ -168,6 +192,22 @@ export class ExplorerComponent {
this.initialState$.pipe(map(value => value.contextToggle.value.dashboardContext)),
this.contextChangeSubject
);
this.featureStateResolver.getFeatureState(ApplicationFeature.SavedQueries).subscribe(featureState => {
this.enableSavedQueries = featureState === FeatureState.Enabled ? true : false;
});
this.preferenceService.get('savedQueries', []).subscribe(queries => {
this.savedQueries = queries as SavedQuery[];
});
this.currentContext$.subscribe(value => (this.currentContext = value));
}

public onClickSaveQuery(): void {
const currentScope = this.getQueryParamFromContext(this.currentContext);
const currentFilterUrlStrings = this.filters.map(filter => filter.urlString);

const newSavedQueries = [...this.savedQueries, { scope: currentScope, filterUrlStrings: currentFilterUrlStrings }];
this.preferenceService.set('savedQueries', newSavedQueries);
this.notificationService.createSuccessToast('Query Saved Successfully!');
}

public onVisualizationRequestUpdated(newRequest: ExploreVisualizationRequest): void {
Expand All @@ -187,7 +227,7 @@ export class ExplorerComponent {
switch (context) {
case ObservabilityTraceType.Api:
return ScopeQueryParam.EndpointTraces;
case 'SPAN':
case SPAN_SCOPE:
return ScopeQueryParam.Spans;
default:
return assertUnreachable(context);
Expand Down Expand Up @@ -339,3 +379,8 @@ const enum ExplorerQueryParam {
GroupLimit = 'limit',
Series = 'series'
}

export interface SavedQuery {
scope: ScopeQueryParam;
filterUrlStrings: string[];
}
4 changes: 4 additions & 0 deletions projects/observability/src/pages/explorer/explorer.module.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import { CommonModule } from '@angular/common';
import { FactorySansProvider, ModuleWithProviders, NgModule } from '@angular/core';
import {
ButtonModule,
FilterBarModule,
LetAsyncModule,
NotificationModule,
PageHeaderModule,
PanelModule,
ToggleGroupModule
Expand All @@ -14,7 +16,9 @@ import { ExplorerComponent } from './explorer.component';

@NgModule({
imports: [
ButtonModule,
CommonModule,
NotificationModule,
ObservabilityDashboardModule,
FilterBarModule,
ExploreQueryEditorModule,
Expand Down
2 changes: 2 additions & 0 deletions src/app/shared/feature-resolver/feature-resolver.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ export class FeatureResolverService extends FeatureStateResolver {
switch (flag) {
case ApplicationFeature.PageTimeRange:
return of(FeatureState.Disabled);
case ApplicationFeature.SavedQueries:
return of(FeatureState.Disabled);
default:
return of(FeatureState.Enabled);
}
Expand Down