diff --git a/projects/common/src/preference/preference.service.ts b/projects/common/src/preference/preference.service.ts index d498d341d..890feb61c 100644 --- a/projects/common/src/preference/preference.service.ts +++ b/projects/common/src/preference/preference.service.ts @@ -55,6 +55,24 @@ export class PreferenceService { ); } + /** + * Returns the current storage value if defined, else the default value. + */ + public getOnce( + key: PreferenceKey, + defaultValue?: T, + type: StorageType = PreferenceService.DEFAULT_STORAGE_TYPE + ): T { + const storedValue = this.preferenceStorage(type).get(this.asStorageKey(key)); + const value = this.fromStorageValue(storedValue) ?? defaultValue; + + if (value === undefined) { + throw Error(`No value found or default provided for preferenceKey: ${key}`); + } + + return value; + } + public set( key: PreferenceKey, value: PreferenceValue, diff --git a/projects/components/src/table/table.component.test.ts b/projects/components/src/table/table.component.test.ts index a99b7a62c..bfe6ca13a 100644 --- a/projects/components/src/table/table.component.test.ts +++ b/projects/components/src/table/table.component.test.ts @@ -1,7 +1,13 @@ /* eslint-disable max-lines */ import { fakeAsync, flush } from '@angular/core/testing'; import { ActivatedRoute, convertToParamMap } from '@angular/router'; -import { DomElementMeasurerService, NavigationService } from '@hypertrace/common'; +import { + DomElementMeasurerService, + NavigationService, + PreferenceService, + PreferenceValue, + StorageType +} from '@hypertrace/common'; import { runFakeRxjs } from '@hypertrace/test-utils'; import { createHostFactory, mockProvider } from '@ngneat/spectator/jest'; import { MockComponent } from 'ng-mocks'; @@ -20,6 +26,7 @@ import { TableColumnConfigExtended, TableService } from './table.service'; import { ModalService } from '../modal/modal.service'; describe('Table component', () => { + let localStorage: PreferenceValue = { columns: [] }; // TODO remove builders once table stops mutating inputs const buildData = () => [ { @@ -56,7 +63,6 @@ describe('Table component', () => { imports: [LetAsyncModule], providers: [ mockProvider(NavigationService), - mockProvider(ActivatedRoute), mockProvider(ActivatedRoute, { queryParamMap: EMPTY }), @@ -75,6 +81,10 @@ describe('Table component', () => { }), mockProvider(ModalService, { createModal: jest.fn().mockReturnValue({ closed$: of([]) }) + }), + mockProvider(PreferenceService, { + getOnce: jest.fn().mockReturnValue(localStorage), + set: (_: unknown, value: PreferenceValue) => (localStorage = value) }) ], declarations: [MockComponent(PaginatorComponent), MockComponent(SearchBoxComponent)], @@ -256,12 +266,15 @@ describe('Table component', () => { }); spectator.tick(); - expect(spectator.component.columnConfigs![0]).toEqual( - expect.objectContaining({ - sort: TableSortDirection.Ascending, - id: 'firstId' - }) - ); + runFakeRxjs(({ expectObservable }) => { + expectObservable(spectator.component.columnConfigs$.pipe(map(columns => columns[0]))).toBe('x', { + x: expect.objectContaining({ + sort: TableSortDirection.Ascending, + id: 'firstId' + }) + }); + }); + flush(); })); test('does not alter the URL on sorting if syncWithUrl false', fakeAsync(() => { @@ -580,5 +593,157 @@ describe('Table component', () => { ] }); }); + flush(); + })); + + test('saved preferences should be accounted for while building column configs', fakeAsync(() => { + const columns = buildColumns(); + let localStorageOverride: PreferenceValue = { + columns: [ + { + id: 'firstId', + visible: false + }, + { id: 'secondId', visible: true } + ] + }; + const spectator = createHost( + '', + { + hostProps: { + columnConfigs: columns, + data: buildData(), + selectionMode: TableSelectionMode.Single, + mode: TableMode.Flat + }, + providers: [ + mockProvider(PreferenceService, { + getOnce: jest.fn().mockReturnValue(localStorageOverride), + set: jest.fn().mockImplementation((_key: string, value: PreferenceValue, _storageType: StorageType) => { + localStorageOverride = value; + }) + }) + ] + } + ); + spectator.tick(); + + runFakeRxjs(({ expectObservable }) => { + expectObservable(spectator.component.columnConfigs$).toBe('x', { + x: [ + expect.objectContaining({ + id: 'firstId', + visible: false + }), + expect.objectContaining({ + id: 'secondId', + visible: true + }) + ] + }); + }); + + expect(spectator.inject(PreferenceService).getOnce).toHaveBeenLastCalledWith( + 'test-table', + { columns: [] }, + StorageType.Local + ); + flush(); + })); + + test('should save user preferences for columns when a column is hidden', fakeAsync(() => { + const columns = buildColumns(); + let localStorageOverride: PreferenceValue = { + columns: [] + }; + const spectator = createHost( + '', + { + hostProps: { + columnConfigs: columns, + data: buildData(), + selectionMode: TableSelectionMode.Single, + mode: TableMode.Flat + }, + providers: [ + mockProvider(PreferenceService, { + getOnce: jest.fn().mockReturnValue(localStorageOverride), + set: jest.fn().mockImplementation((_key: string, value: PreferenceValue, _storageType: StorageType) => { + localStorageOverride = value; + }) + }) + ] + } + ); + spectator.component.onHideColumn({ ...columns[0] }); + spectator.tick(); + expect(spectator.inject(PreferenceService).set).toHaveBeenCalledWith( + 'test-table', + { + columns: expect.arrayContaining([ + { + id: 'firstId', + visible: false + }, + { + id: 'secondId', + visible: true + } + ]) + }, + StorageType.Local + ); + + flush(); + })); + + test('should save user preferences for columns when columns are edited', fakeAsync(() => { + const columns = buildColumns(); + let localStorageOverride: PreferenceValue = { + columns: [] + }; + const spectator = createHost( + '', + { + hostProps: { + columnConfigs: columns, + data: buildData(), + selectionMode: TableSelectionMode.Single, + mode: TableMode.Flat + }, + providers: [ + mockProvider(PreferenceService, { + getOnce: jest.fn().mockReturnValue(localStorageOverride), + set: jest.fn().mockImplementation((_key: string, value: PreferenceValue, _storageType: StorageType) => { + localStorageOverride = value; + }) + }), + mockProvider(ModalService, { + createModal: jest.fn().mockReturnValue({ closed$: of([columns[0], { ...columns[1], visible: false }]) }) + }) + ] + } + ); + spectator.tick(); + spectator.component.showEditColumnsModal(); + spectator.tick(); + expect(spectator.inject(PreferenceService).set).toHaveBeenCalledWith( + 'test-table', + { + columns: expect.arrayContaining([ + { + id: 'firstId', + visible: true + }, + { + id: 'secondId', + visible: false + } + ]) + }, + StorageType.Local + ); + + flush(); })); }); diff --git a/projects/components/src/table/table.component.ts b/projects/components/src/table/table.component.ts index 8c4a6e592..12482360f 100644 --- a/projects/components/src/table/table.component.ts +++ b/projects/components/src/table/table.component.ts @@ -2,8 +2,8 @@ /* eslint-disable @angular-eslint/component-max-inline-declarations */ import { ModalService } from '../modal/modal.service'; import { - TableEditColumnsModalConfig, - TableEditColumnsModalComponent + TableEditColumnsModalComponent, + TableEditColumnsModalConfig } from './columns/table-edit-columns-modal.component'; import { CdkHeaderRow } from '@angular/cdk/table'; import { @@ -29,13 +29,16 @@ import { Dictionary, DomElementMeasurerService, isEqualIgnoreFunctions, + isNonEmptyString, NavigationService, NumberCoercer, + PreferenceService, + StorageType, TypedSimpleChanges } from '@hypertrace/common'; import { isNil, without } from 'lodash-es'; import { BehaviorSubject, combineLatest, merge, Observable, Subject } from 'rxjs'; -import { switchMap, take, filter, map } from 'rxjs/operators'; +import { filter, map, switchMap, take } from 'rxjs/operators'; import { FilterAttribute } from '../filtering/filter/filter-attribute'; import { LoadAsyncConfig } from '../load-async/load-async.service'; import { PageEvent } from '../paginator/page.event'; @@ -308,6 +311,9 @@ export class TableComponent id: '$$detail' }; + @Input() + public id?: string; + @Input() public columnConfigs?: TableColumnConfig[]; @@ -505,7 +511,8 @@ export class TableComponent private readonly activatedRoute: ActivatedRoute, private readonly domElementMeasurerService: DomElementMeasurerService, private readonly tableService: TableService, - private readonly modalService: ModalService + private readonly modalService: ModalService, + private readonly preferenceService: PreferenceService ) { combineLatest([this.activatedRoute.queryParamMap, this.columnConfigs$]) .pipe( @@ -686,6 +693,49 @@ export class TableComponent } } + private dehydratePersistedColumnConfig(column: TableColumnConfig): PersistedTableColumnConfig { + /* + * Note: The table columns have nested methods, so those are lost here when persistService uses JSON.stringify + * to convert and store. We want to just pluck the relevant properties that are required to be saved. + */ + return { id: column.id, visible: column.visible }; + } + + private saveTablePreferences(columns: TableColumnConfig[]): void { + if (isNonEmptyString(this.id)) { + this.setLocalPreferences({ + ...this.getLocalPreferences(), + columns: columns.map(column => this.dehydratePersistedColumnConfig(column)) + }); + } + } + + private getLocalPreferences(): TableLocalPreferences { + return isNonEmptyString(this.id) + ? this.preferenceService.getOnce(this.id, { columns: [] }, StorageType.Local) + : { columns: [] }; + } + + private setLocalPreferences(preferences: TableLocalPreferences): void { + if (isNonEmptyString(this.id)) { + this.preferenceService.set(this.id, preferences, StorageType.Local); + } + } + + private hydratePersistedColumnConfigs( + columns: TableColumnConfigExtended[], + persistedColumns: PersistedTableColumnConfig[] + ): TableColumnConfigExtended[] { + return columns.map(column => { + const found = persistedColumns.find(persistedColumn => persistedColumn.id === column.id); + + return { + ...column, // Apply default column config + ...(found ? { visible: found.visible } : {}) // Override with any saved properties + }; + }); + } + private setColumnResizeDefaults(columnResizeHandler: HTMLDivElement): void { columnResizeHandler.style.backgroundColor = Color.Transparent; columnResizeHandler.style.right = '2px'; @@ -714,15 +764,21 @@ export class TableComponent this.checkColumnWidthCompatibilityOrThrow(column.width); this.checkColumnWidthCompatibilityOrThrow(column.minWidth); }); - const columnConfigurations = this.buildColumnConfigExtendeds(columnConfigs ?? this.columnConfigs ?? []); + const columnConfigurations = this.buildColumnConfigExtended(columnConfigs ?? this.columnConfigs ?? []); if (isNil(this.columnDefaultConfigs)) { this.columnDefaultConfigs = columnConfigurations; } - const visibleColumns = columnConfigurations.filter(column => column.visible); + + const preferences = this.getLocalPreferences(); + const columnConfigsWithUserPref = this.hydratePersistedColumnConfigs( + columnConfigurations, + preferences.columns ?? [] + ); + + const visibleColumns = columnConfigsWithUserPref.filter(column => column.visible); this.initialColumnConfigIdWidthMap = new Map(visibleColumns.map(column => [column.id, column.width ?? -1])); this.updateVisibleColumns(visibleColumns); - - this.columnConfigsSubject.next(columnConfigurations); + this.columnConfigsSubject.next(columnConfigsWithUserPref); } private checkColumnWidthCompatibilityOrThrow(width?: TableColumnWidth): void { @@ -827,6 +883,7 @@ export class TableComponent const updatedColumns = this.columnConfigsSubject.value; this.updateVisibleColumns(updatedColumns.filter(c => c.visible)); this.distributeWidthToColumns(TableColumnWidthUtil.getColWidthInPx(column.width)); + this.saveTablePreferences(updatedColumns.map(col => (column.id === col.id ? { ...col, visible: false } : col))); this.columnConfigsSubject.next(updatedColumns); } @@ -849,6 +906,7 @@ export class TableComponent ) ) .subscribe(editedColumnConfigs => { + this.saveTablePreferences(editedColumnConfigs); this.initializeColumns(editedColumnConfigs); }); } @@ -913,7 +971,7 @@ export class TableComponent return this.hasExpandableRows() ? index - 1 : index; } - private buildColumnConfigExtendeds(columnConfigs: TableColumnConfig[]): TableColumnConfigExtended[] { + private buildColumnConfigExtended(columnConfigs: TableColumnConfig[]): TableColumnConfigExtended[] { const stateColumns = []; if (this.hasMultiSelect()) { @@ -1183,3 +1241,9 @@ interface ColumnInfo { element: HTMLElement; bounds: ColumnBounds; } + +interface TableLocalPreferences { + columns?: PersistedTableColumnConfig[]; +} + +type PersistedTableColumnConfig = Pick; diff --git a/projects/observability/src/pages/explorer/explorer.component.test.ts b/projects/observability/src/pages/explorer/explorer.component.test.ts index 90c92b25b..9922ca470 100644 --- a/projects/observability/src/pages/explorer/explorer.component.test.ts +++ b/projects/observability/src/pages/explorer/explorer.component.test.ts @@ -11,7 +11,9 @@ import { LayoutChangeService, NavigationService, PreferenceService, + PreferenceValue, RelativeTimeRange, + StorageType, TimeDuration, TimeRangeService, TimeUnit @@ -71,6 +73,7 @@ describe('Explorer component', () => { const testTimeRange = new RelativeTimeRange(new TimeDuration(15, TimeUnit.Minute)); const createComponent = createComponentFactory({ component: ExplorerComponent, + shallow: true, imports: [ ExplorerModule.withDashboardBuilderFactory({ useFactory: (metadataService: MetadataService, filterBuilderLookupService: FilterBuilderLookupService) => @@ -111,7 +114,10 @@ describe('Explorer component', () => { } }, mockProvider(PreferenceService, { - get: jest.fn().mockReturnValue(of(true)) + get: jest.fn().mockReturnValue(of(true)), + getOnce: jest + .fn() + .mockImplementation((_key: string, defaultValue: PreferenceValue, _storageType: StorageType) => defaultValue) }), ...getMockFlexLayoutProviders() ] diff --git a/projects/observability/src/shared/dashboard/widgets/table/table-widget-renderer.component.ts b/projects/observability/src/shared/dashboard/widgets/table/table-widget-renderer.component.ts index 6f5a137b6..793dc2c37 100644 --- a/projects/observability/src/shared/dashboard/widgets/table/table-widget-renderer.component.ts +++ b/projects/observability/src/shared/dashboard/widgets/table/table-widget-renderer.component.ts @@ -33,8 +33,8 @@ import { } from '@hypertrace/components'; import { WidgetRenderer } from '@hypertrace/dashboards'; import { Renderer } from '@hypertrace/hyperdash'; -import { RendererApi, RENDERER_API } from '@hypertrace/hyperdash-angular'; -import { capitalize, isEmpty, isEqual, pick } from 'lodash-es'; +import { RENDERER_API, RendererApi } from '@hypertrace/hyperdash-angular'; +import { capitalize, isEmpty, isEqual } from 'lodash-es'; import { BehaviorSubject, combineLatest, Observable, of, Subject } from 'rxjs'; import { filter, @@ -58,7 +58,6 @@ import { MetadataService } from '../../../services/metadata/metadata.service'; import { InteractionHandler } from '../../interaction/interaction-handler'; import { TableWidgetRowInteractionModel } from './selections/table-widget-row-interaction.model'; import { TableWidgetBaseModel } from './table-widget-base.model'; -import { SpecificationBackedTableColumnDef } from './table-widget-column.model'; import { TableWidgetControlSelectOptionModel } from './table-widget-control-select-option.model'; import { TableWidgetViewToggleModel } from './table-widget-view-toggle.model'; import { TableWidgetModel } from './table-widget.model'; @@ -96,6 +95,7 @@ import { TableWidgetModel } from './table-widget.model'; @@ -316,21 +315,14 @@ export class TableWidgetRendererComponent } private getColumnConfigs(): Observable { - return this.getLocalPreferences().pipe( - switchMap(preferences => - combineLatest([this.getScope(), this.api.change$.pipe(mapTo(true), startWith(true))]).pipe( - switchMap(([scope]) => this.model.getColumns(scope)), - startWith([]), - map((columns: SpecificationBackedTableColumnDef[]) => - this.hydratePersistedColumnConfigs(columns, preferences.columns ?? []) - ), - pairwise(), - filter(([previous, current]) => !isEqualIgnoreFunctions(previous, current)), - map(([_, current]) => current), - share(), - tap(() => this.onDashboardRefresh()) - ) - ) + return combineLatest([this.getScope(), this.api.change$.pipe(mapTo(true), startWith(true))]).pipe( + switchMap(([scope]) => this.model.getColumns(scope)), + startWith([]), + pairwise(), + filter(([previous, current]) => !isEqualIgnoreFunctions(previous, current)), + map(([_, current]) => current), + share(), + tap(() => this.onDashboardRefresh()) ); } @@ -512,17 +504,6 @@ export class TableWidgetRendererComponent }); } - public onColumnsChange(columns: TableColumnConfig[]): void { - if (isNonEmptyString(this.model.getId())) { - this.getLocalPreferences().subscribe(preferences => - this.setLocalPreferences({ - ...preferences, - columns: columns.map(column => this.dehydratePersistedColumnConfig(column)) - }) - ); - } - } - public onRowClicked(row: StatefulTableRow): void { this.getRowClickInteractionHandler(row)?.execute(row); } @@ -558,28 +539,6 @@ export class TableWidgetRendererComponent : viewItems[TableWidgetRendererComponent.DEFAULT_TAB_INDEX]; } - private hydratePersistedColumnConfigs( - columns: SpecificationBackedTableColumnDef[], - persistedColumns: TableColumnConfig[] - ): SpecificationBackedTableColumnDef[] { - return columns.map(column => { - const found = persistedColumns.find(persistedColumn => persistedColumn.id === column.id); - - return { - ...column, // Apply default column config - ...(found ? found : {}) // Override with any saved properties - }; - }); - } - - private dehydratePersistedColumnConfig(column: TableColumnConfig): PersistedTableColumnConfig { - /* - * Note: The table columns have nested methods, so those are lost here when persistService uses JSON.stringify - * to convert and store. We want to just pluck the relevant properties that are required to be saved. - */ - return pick(column, ['id', 'visible']); - } - private getViewPreferences(): Observable { return isNonEmptyString(this.model.viewId) ? this.preferenceService.get(this.model.viewId, {}, StorageType.Local).pipe(first()) @@ -592,20 +551,6 @@ export class TableWidgetRendererComponent } } - private getLocalPreferences(): Observable { - return isNonEmptyString(this.model.getId()) - ? this.preferenceService - .get(this.model.getId()!, {}, StorageType.Local) - .pipe(first()) - : of({}); - } - - private setLocalPreferences(preferences: TableWidgetLocalPreferences): void { - if (isNonEmptyString(this.model.getId())) { - this.preferenceService.set(this.model.getId()!, preferences, StorageType.Local); - } - } - private getSessionPreferences(): Observable { return isNonEmptyString(this.model.getId()) ? this.preferenceService @@ -649,13 +594,7 @@ interface TableWidgetViewPreferences { activeView?: string; } -interface TableWidgetLocalPreferences { - columns?: PersistedTableColumnConfig[]; -} - interface TableWidgetSessionPreferences { checkboxes?: TableCheckboxControl[]; selections?: TableSelectControl[]; } - -type PersistedTableColumnConfig = Pick;