From ba055343080b0cf73b04cb6581dfb871a752f7aa Mon Sep 17 00:00:00 2001 From: Amandee Ellawala Date: Fri, 8 Nov 2024 01:24:10 +0000 Subject: [PATCH] Resolve review comments --- .../src/app/portfolio/portfolio.controller.ts | 48 +++++++++---------- .../src/app/user/update-user-setting.dto.ts | 10 ++-- .../app/components/header/header.component.ts | 16 +++---- .../src/app/services/user/user.service.ts | 4 +- .../lib/interfaces/user-settings.interface.ts | 4 +- .../src/lib/assistant/assistant.component.ts | 41 ++++++++-------- libs/ui/src/lib/assistant/assistant.html | 2 +- 7 files changed, 60 insertions(+), 65 deletions(-) diff --git a/apps/api/src/app/portfolio/portfolio.controller.ts b/apps/api/src/app/portfolio/portfolio.controller.ts index 5a2b65eed..ab5ef0500 100644 --- a/apps/api/src/app/portfolio/portfolio.controller.ts +++ b/apps/api/src/app/portfolio/portfolio.controller.ts @@ -79,10 +79,10 @@ export class PortfolioController { @Headers(HEADER_KEY_IMPERSONATION.toLowerCase()) impersonationId: string, @Query('accounts') filterByAccounts?: string, @Query('assetClasses') filterByAssetClasses?: string, - @Query('range') dateRange: DateRange = 'max', - @Query('tags') filterByTags?: string, @Query('dataSource') filterByDataSource?: string, + @Query('range') dateRange: DateRange = 'max', @Query('symbol') filterBySymbol?: string, + @Query('tags') filterByTags?: string, @Query('withMarkets') withMarketsParam = 'false' ): Promise { const withMarkets = withMarketsParam === 'true'; @@ -97,9 +97,9 @@ export class PortfolioController { const filters = this.apiService.buildFiltersFromQueryParams({ filterByAccounts, filterByAssetClasses, - filterByTags, filterByDataSource, - filterBySymbol + filterBySymbol, + filterByTags }); const { @@ -297,18 +297,18 @@ export class PortfolioController { @Headers(HEADER_KEY_IMPERSONATION.toLowerCase()) impersonationId: string, @Query('accounts') filterByAccounts?: string, @Query('assetClasses') filterByAssetClasses?: string, + @Query('dataSource') filterByDataSource?: string, @Query('groupBy') groupBy?: GroupBy, @Query('range') dateRange: DateRange = 'max', - @Query('tags') filterByTags?: string, - @Query('dataSource') filterByDataSource?: string, - @Query('symbol') filterBySymbol?: string + @Query('symbol') filterBySymbol?: string, + @Query('tags') filterByTags?: string ): Promise { const filters = this.apiService.buildFiltersFromQueryParams({ filterByAccounts, filterByAssetClasses, - filterByTags, filterByDataSource, - filterBySymbol + filterBySymbol, + filterByTags }); const impersonationUserId = @@ -369,21 +369,21 @@ export class PortfolioController { @Headers(HEADER_KEY_IMPERSONATION.toLowerCase()) impersonationId: string, @Query('accounts') filterByAccounts?: string, @Query('assetClasses') filterByAssetClasses?: string, + @Query('dataSource') filterByDataSource?: string, @Query('holdingType') filterByHoldingType?: string, @Query('query') filterBySearchQuery?: string, @Query('range') dateRange: DateRange = 'max', - @Query('tags') filterByTags?: string, - @Query('dataSource') filterByDataSource?: string, - @Query('symbol') filterBySymbol?: string + @Query('symbol') filterBySymbol?: string, + @Query('tags') filterByTags?: string ): Promise { const filters = this.apiService.buildFiltersFromQueryParams({ filterByAccounts, filterByAssetClasses, + filterByDataSource, filterByHoldingType, filterBySearchQuery, - filterByTags, - filterByDataSource, - filterBySymbol + filterBySymbol, + filterByTags }); const { holdings } = await this.portfolioService.getDetails({ @@ -402,18 +402,18 @@ export class PortfolioController { @Headers(HEADER_KEY_IMPERSONATION.toLowerCase()) impersonationId: string, @Query('accounts') filterByAccounts?: string, @Query('assetClasses') filterByAssetClasses?: string, + @Query('dataSource') filterByDataSource?: string, @Query('groupBy') groupBy?: GroupBy, @Query('range') dateRange: DateRange = 'max', - @Query('tags') filterByTags?: string, - @Query('dataSource') filterByDataSource?: string, - @Query('symbol') filterBySymbol?: string + @Query('symbol') filterBySymbol?: string, + @Query('tags') filterByTags?: string ): Promise { const filters = this.apiService.buildFiltersFromQueryParams({ filterByAccounts, filterByAssetClasses, - filterByTags, filterByDataSource, - filterBySymbol + filterBySymbol, + filterByTags }); let { investments, streaks } = await this.portfolioService.getInvestments({ @@ -473,10 +473,10 @@ export class PortfolioController { @Headers(HEADER_KEY_IMPERSONATION.toLowerCase()) impersonationId: string, @Query('accounts') filterByAccounts?: string, @Query('assetClasses') filterByAssetClasses?: string, - @Query('range') dateRange: DateRange = 'max', - @Query('tags') filterByTags?: string, @Query('dataSource') filterByDataSource?: string, + @Query('range') dateRange: DateRange = 'max', @Query('symbol') filterBySymbol?: string, + @Query('tags') filterByTags?: string, @Query('withExcludedAccounts') withExcludedAccountsParam = 'false' ): Promise { const withExcludedAccounts = withExcludedAccountsParam === 'true'; @@ -484,9 +484,9 @@ export class PortfolioController { const filters = this.apiService.buildFiltersFromQueryParams({ filterByAccounts, filterByAssetClasses, - filterByTags, filterByDataSource, - filterBySymbol + filterBySymbol, + filterByTags }); const performanceInformation = await this.portfolioService.getPerformance({ diff --git a/apps/api/src/app/user/update-user-setting.dto.ts b/apps/api/src/app/user/update-user-setting.dto.ts index d9047de72..b34b6fae2 100644 --- a/apps/api/src/app/user/update-user-setting.dto.ts +++ b/apps/api/src/app/user/update-user-setting.dto.ts @@ -64,17 +64,17 @@ export class UpdateUserSettingDto { @IsOptional() 'filters.assetClasses'?: string[]; - @IsArray() + @IsString() @IsOptional() - 'filters.tags'?: string[]; + 'filters.dataSource'?: string; - @IsArray() + @IsString() @IsOptional() - 'filters.dataSource'?: string[]; + 'filters.symbol'?: string; @IsArray() @IsOptional() - 'filters.symbol'?: string[]; + 'filters.tags'?: string[]; @IsIn(['CHART', 'TABLE'] as HoldingsViewMode[]) @IsOptional() diff --git a/apps/client/src/app/components/header/header.component.ts b/apps/client/src/app/components/header/header.component.ts index 6ee27be68..95435aa6e 100644 --- a/apps/client/src/app/components/header/header.component.ts +++ b/apps/client/src/app/components/header/header.component.ts @@ -175,21 +175,17 @@ export class HeaderComponent implements OnChanges { const userSetting: UpdateUserSettingDto = {}; for (const filter of filters) { - let filtersType: string; - if (filter.type === 'ACCOUNT') { - filtersType = 'accounts'; + userSetting[`filters.accounts`] = filter.id ? [filter.id] : null; } else if (filter.type === 'ASSET_CLASS') { - filtersType = 'assetClasses'; - } else if (filter.type === 'TAG') { - filtersType = 'tags'; + userSetting[`filters.assetClasses`] = filter.id ? [filter.id] : null; } else if (filter.type === 'DATA_SOURCE') { - filtersType = 'dataSource'; + userSetting[`filters.dataSource`] = filter.id ? filter.id : null; } else if (filter.type === 'SYMBOL') { - filtersType = 'symbol'; + userSetting[`filters.symbol`] = filter.id ? filter.id : null; + } else if (filter.type === 'TAG') { + userSetting[`filters.tags`] = filter.id ? [filter.id] : null; } - - userSetting[`filters.${filtersType}`] = filter.id ? [filter.id] : null; } this.dataService diff --git a/apps/client/src/app/services/user/user.service.ts b/apps/client/src/app/services/user/user.service.ts index 3f63841a6..008d3a87f 100644 --- a/apps/client/src/app/services/user/user.service.ts +++ b/apps/client/src/app/services/user/user.service.ts @@ -74,14 +74,14 @@ export class UserService extends ObservableStore { if (user?.settings['filters.dataSource']) { filters.push({ - id: user.settings['filters.dataSource'][0], + id: user.settings['filters.dataSource'], type: 'DATA_SOURCE' }); } if (user?.settings['filters.symbol']) { filters.push({ - id: user.settings['filters.symbol'][0], + id: user.settings['filters.symbol'], type: 'SYMBOL' }); } diff --git a/libs/common/src/lib/interfaces/user-settings.interface.ts b/libs/common/src/lib/interfaces/user-settings.interface.ts index 01e3a605f..ebdba28f2 100644 --- a/libs/common/src/lib/interfaces/user-settings.interface.ts +++ b/libs/common/src/lib/interfaces/user-settings.interface.ts @@ -15,8 +15,8 @@ export interface UserSettings { emergencyFund?: number; 'filters.accounts'?: string[]; 'filters.tags'?: string[]; - 'filters.dataSource'?: string[]; - 'filters.symbol'?: string[]; + 'filters.dataSource'?: string; + 'filters.symbol'?: string; holdingsViewMode?: HoldingsViewMode; isExperimentalFeatures?: boolean; isRestrictedView?: boolean; diff --git a/libs/ui/src/lib/assistant/assistant.component.ts b/libs/ui/src/lib/assistant/assistant.component.ts index 276ca156a..6cf0e75ad 100644 --- a/libs/ui/src/lib/assistant/assistant.component.ts +++ b/libs/ui/src/lib/assistant/assistant.component.ts @@ -63,13 +63,13 @@ import { FormsModule, GfAssetProfileIconComponent, GfAssistantListItemComponent, + GfSymbolModule, MatButtonModule, MatFormFieldModule, MatSelectModule, NgxSkeletonLoaderModule, ReactiveFormsModule, - RouterModule, - GfSymbolModule + RouterModule ], schemas: [CUSTOM_ELEMENTS_SCHEMA], selector: 'gf-assistant', @@ -135,8 +135,8 @@ export class GfAssistantComponent implements OnChanges, OnDestroy, OnInit { public filterForm = this.formBuilder.group({ account: new FormControl(undefined), assetClass: new FormControl(undefined), - tag: new FormControl(undefined), - holdings: new FormControl(undefined) + holdings: new FormControl(undefined), + tag: new FormControl(undefined) }); public isLoading = false; public isOpen = false; @@ -147,14 +147,14 @@ export class GfAssistantComponent implements OnChanges, OnDestroy, OnInit { holdings: [] }; public tags: Filter[] = []; - public allPortfolioHoldings: PortfolioPosition[] = []; + public holdings: PortfolioPosition[] = []; private filterTypes: Filter['type'][] = [ 'ACCOUNT', 'ASSET_CLASS', - 'TAG', 'DATA_SOURCE', - 'SYMBOL' + 'SYMBOL', + 'TAG' ]; private keyManager: FocusKeyManager; private unsubscribeSubject = new Subject(); @@ -335,10 +335,6 @@ export class GfAssistantComponent implements OnChanges, OnDestroy, OnInit { id: this.filterForm.get('assetClass').value, type: 'ASSET_CLASS' }, - { - id: this.filterForm.get('tag').value, - type: 'TAG' - }, { id: this.filterForm.get('holdings').value?.dataSource, type: 'DATA_SOURCE' @@ -346,6 +342,10 @@ export class GfAssistantComponent implements OnChanges, OnDestroy, OnInit { { id: this.filterForm.get('holdings').value?.symbol, type: 'SYMBOL' + }, + { + id: this.filterForm.get('tag').value, + type: 'TAG' } ]); @@ -499,7 +499,7 @@ export class GfAssistantComponent implements OnChanges, OnDestroy, OnInit { }) .pipe(takeUntil(this.unsubscribeSubject)) .subscribe(({ holdings }) => { - this.allPortfolioHoldings = sortBy(holdings, ({ name }) => { + this.holdings = sortBy(holdings, ({ name }) => { return name.toLowerCase(); }); this.setFormValues(); @@ -507,21 +507,20 @@ export class GfAssistantComponent implements OnChanges, OnDestroy, OnInit { } private setFormValues() { - const dataSource = this.user?.settings?.['filters.dataSource']?.[0] ?? null; - const symbol = this.user?.settings?.['filters.symbol']?.[0] ?? null; - const selectedHolding = this.allPortfolioHoldings.filter( - (h) => - (h.dataSource ?? null) === dataSource && (h.symbol ?? null) === symbol + const dataSource = this.user?.settings?.['filters.dataSource'] ?? null; + const symbol = this.user?.settings?.['filters.symbol'] ?? null; + const selectedHolding = this.holdings.find( + (holding) => + (holding.dataSource ?? null) === dataSource && + (holding.symbol ?? null) === symbol ); - const holding = selectedHolding[0] ?? null; - this.filterForm.setValue( { account: this.user?.settings?.['filters.accounts']?.[0] ?? null, assetClass: this.user?.settings?.['filters.assetClasses']?.[0] ?? null, - tag: this.user?.settings?.['filters.tags']?.[0] ?? null, - holdings: holding + holdings: selectedHolding ?? null, + tag: this.user?.settings?.['filters.tags']?.[0] ?? null }, { emitEvent: false diff --git a/libs/ui/src/lib/assistant/assistant.html b/libs/ui/src/lib/assistant/assistant.html index 8c14d8c43..0123aacba 100644 --- a/libs/ui/src/lib/assistant/assistant.html +++ b/libs/ui/src/lib/assistant/assistant.html @@ -133,7 +133,7 @@ filterForm.get('holdings')?.value?.name }} - @for (holding of allPortfolioHoldings; track holding.name) { + @for (holding of holdings; track holding.name) { {{ holding.name }}