From 26f0c358111806b12633f6ac1e140095922e0c37 Mon Sep 17 00:00:00 2001 From: Kenrick Tandrian <60643640+KenTandrian@users.noreply.github.com> Date: Sun, 21 Jun 2026 22:43:15 +0700 Subject: [PATCH] Task/improve type safety in allocations page component (#7076) Improve type safety --- .../allocations/allocations-page.component.ts | 333 +++++++++--------- .../allocations/allocations-page.html | 2 +- .../allocations/interfaces/interfaces.ts | 6 + .../src/lib/interfaces/holding.interface.ts | 2 +- 4 files changed, 177 insertions(+), 166 deletions(-) create mode 100644 apps/client/src/app/pages/portfolio/allocations/interfaces/interfaces.ts diff --git a/apps/client/src/app/pages/portfolio/allocations/allocations-page.component.ts b/apps/client/src/app/pages/portfolio/allocations/allocations-page.component.ts index 9aab27dc0..1ed8bfa66 100644 --- a/apps/client/src/app/pages/portfolio/allocations/allocations-page.component.ts +++ b/apps/client/src/app/pages/portfolio/allocations/allocations-page.component.ts @@ -12,7 +12,7 @@ import { User } from '@ghostfolio/common/interfaces'; import { hasPermission, permissions } from '@ghostfolio/common/permissions'; -import { Market, MarketAdvanced } from '@ghostfolio/common/types'; +import { MarketAdvanced } from '@ghostfolio/common/types'; import { translate } from '@ghostfolio/ui/i18n'; import { GfPortfolioProportionChartComponent } from '@ghostfolio/ui/portfolio-proportion-chart'; import { GfPremiumIndicatorComponent } from '@ghostfolio/ui/premium-indicator'; @@ -24,7 +24,9 @@ import { GfWorldMapChartComponent } from '@ghostfolio/ui/world-map-chart'; import { ChangeDetectorRef, Component, + computed, DestroyRef, + inject, OnInit } from '@angular/core'; import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; @@ -41,6 +43,9 @@ import { } from '@prisma/client'; import { isNumber } from 'lodash'; import { DeviceDetectorService } from 'ngx-device-detector'; +import { filter, switchMap, tap } from 'rxjs'; + +import { AllocationsPageParams } from './interfaces/interfaces'; @Component({ imports: [ @@ -57,21 +62,23 @@ import { DeviceDetectorService } from 'ngx-device-detector'; templateUrl: './allocations-page.html' }) export class GfAllocationsPageComponent implements OnInit { - public accounts: { + protected accounts: { [id: string]: Pick & { id: string; value: number; }; }; - public continents: { + protected continents: { [code: string]: { name: string; value: number }; }; - public countries: { + protected countries: { [code: string]: { name: string; value: number }; }; - public deviceType: string; - public hasImpersonationId: boolean; - public holdings: { + protected readonly deviceType = computed( + () => this.deviceDetectorService.deviceInfo().deviceType + ); + protected hasImpersonationId: boolean; + protected holdings: { [symbol: string]: Pick< PortfolioPosition['assetProfile'], | 'assetClass' @@ -82,28 +89,26 @@ export class GfAllocationsPageComponent implements OnInit { | 'name' > & { etfProvider: string; exchange?: string; value: number }; }; - public isLoading = false; - public markets: { - [key in Market]: { id: Market; valueInPercentage: number }; - }; - public marketsAdvanced: { + protected isLoading = false; + protected markets: PortfolioDetails['markets']; + protected marketsAdvanced: { [key in MarketAdvanced]: { id: MarketAdvanced; name: string; value: number; }; }; - public platforms: { + protected platforms: { [id: string]: Pick & { id: string; value: number; }; }; - public portfolioDetails: PortfolioDetails; - public sectors: { + protected portfolioDetails: PortfolioDetails; + protected sectors: { [name: string]: { name: string; value: number }; }; - public symbols: { + protected symbols: { [name: string]: { dataSource?: DataSource; name: string; @@ -111,38 +116,46 @@ export class GfAllocationsPageComponent implements OnInit { value: number; }; }; - public topHoldings: HoldingWithParents[]; - public topHoldingsMap: { + protected topHoldings: HoldingWithParents[]; + protected readonly UNKNOWN_KEY = UNKNOWN_KEY; + protected user: User; + + private topHoldingsMap: { [name: string]: { name: string; value: number }; }; - public totalValueInEtf = 0; - public UNKNOWN_KEY = UNKNOWN_KEY; - public user: User; - public worldMapChartFormat: string; - - public constructor( - private changeDetectorRef: ChangeDetectorRef, - private dataService: DataService, - private destroyRef: DestroyRef, - private deviceDetectorService: DeviceDetectorService, - private dialog: MatDialog, - private impersonationStorageService: ImpersonationStorageService, - private route: ActivatedRoute, - private router: Router, - private userService: UserService - ) { + private totalValueInEtf = 0; + + private readonly changeDetectorRef = inject(ChangeDetectorRef); + private readonly dataService = inject(DataService); + private readonly destroyRef = inject(DestroyRef); + private readonly deviceDetectorService = inject(DeviceDetectorService); + private readonly dialog = inject(MatDialog); + private readonly impersonationStorageService = inject( + ImpersonationStorageService + ); + private readonly route = inject(ActivatedRoute); + private readonly router = inject(Router); + private readonly userService = inject(UserService); + + public constructor() { this.route.queryParams .pipe(takeUntilDestroyed(this.destroyRef)) - .subscribe((params) => { - if (params['accountId'] && params['accountDetailDialog']) { - this.openAccountDetailDialog(params['accountId']); + .subscribe( + ({ accountId, accountDetailDialog }: AllocationsPageParams) => { + if (accountId && accountDetailDialog) { + this.openAccountDetailDialog(accountId); + } } - }); + ); } - public ngOnInit() { - this.deviceType = this.deviceDetectorService.getDeviceInfo().deviceType; + protected get worldMapChartFormat(): string { + return this.showValuesInPercentage() + ? '{0}%' + : `{0} ${this.user?.settings?.baseCurrency}`; + } + public ngOnInit() { this.impersonationStorageService .onChangeHasImpersonation() .pipe(takeUntilDestroyed(this.destroyRef)) @@ -151,56 +164,58 @@ export class GfAllocationsPageComponent implements OnInit { }); this.userService.stateChanged - .pipe(takeUntilDestroyed(this.destroyRef)) - .subscribe((state) => { - if (state?.user) { + .pipe( + filter((state) => !!state?.user), + tap((state) => { this.user = state.user; - this.worldMapChartFormat = this.showValuesInPercentage() - ? `{0}%` - : `{0} ${this.user?.settings?.baseCurrency}`; - this.isLoading = true; this.initialize(); - this.fetchPortfolioDetails() - .pipe(takeUntilDestroyed(this.destroyRef)) - .subscribe((portfolioDetails) => { - this.initialize(); - - this.portfolioDetails = portfolioDetails; + this.changeDetectorRef.markForCheck(); + }), + switchMap(() => this.fetchPortfolioDetails()), + takeUntilDestroyed(this.destroyRef) + ) + .subscribe((portfolioDetails) => { + this.initialize(); - this.initializeAllocationsData(); + this.portfolioDetails = portfolioDetails; - this.isLoading = false; + this.initializeAllocationsData(); - this.changeDetectorRef.markForCheck(); - }); + this.isLoading = false; - this.changeDetectorRef.markForCheck(); - } + this.changeDetectorRef.markForCheck(); }); this.initialize(); } - public onAccountChartClicked({ accountId }: { accountId: string }) { + protected onAccountChartClicked({ accountId }: { accountId: string }) { if (accountId && accountId !== UNKNOWN_KEY) { - this.router.navigate([], { + void this.router.navigate([], { queryParams: { accountId, accountDetailDialog: true } }); } } - public onSymbolChartClicked({ dataSource, symbol }: AssetProfileIdentifier) { + protected onSymbolChartClicked({ + dataSource, + symbol + }: AssetProfileIdentifier) { if (dataSource && symbol) { - this.router.navigate([], { + void this.router.navigate([], { queryParams: { dataSource, symbol, holdingDetailDialog: true } }); } } + protected showValuesInPercentage() { + return this.hasImpersonationId || this.user?.settings?.isRestrictedView; + } + private extractCurrency({ assetClass, assetSubClass, @@ -226,9 +241,9 @@ export class GfAllocationsPageComponent implements OnInit { name }: { assetSubClass: PortfolioPosition['assetProfile']['assetSubClass']; - name: string; + name?: string; }) { - if (assetSubClass === 'ETF') { + if (assetSubClass === 'ETF' && name) { const [firstWord] = name.split(' '); return firstWord; } @@ -298,7 +313,7 @@ export class GfAllocationsPageComponent implements OnInit { this.platforms = {}; this.portfolioDetails = { accounts: {}, - createdAt: undefined, + createdAt: new Date(), holdings: {}, platforms: {}, summary: undefined @@ -327,7 +342,7 @@ export class GfAllocationsPageComponent implements OnInit { let value = 0; if (this.showValuesInPercentage()) { - value = valueInPercentage; + value = valueInPercentage ?? 0; } else { value = valueInBaseCurrency; } @@ -342,30 +357,24 @@ export class GfAllocationsPageComponent implements OnInit { for (const [symbol, position] of Object.entries( this.portfolioDetails.holdings )) { - let value = 0; - - if (this.showValuesInPercentage()) { - value = position.allocationInPercentage; - } else { - value = position.valueInBaseCurrency; - } - this.holdings[symbol] = { - value, assetClass: position.assetProfile.assetClass || (UNKNOWN_KEY as AssetClass), - assetClassLabel: position.assetProfile.assetClassLabel || UNKNOWN_KEY, + assetClassLabel: position.assetProfile.assetClassLabel ?? UNKNOWN_KEY, assetSubClass: position.assetProfile.assetSubClass || (UNKNOWN_KEY as AssetSubClass), assetSubClassLabel: - position.assetProfile.assetSubClassLabel || UNKNOWN_KEY, + position.assetProfile.assetSubClassLabel ?? UNKNOWN_KEY, currency: this.extractCurrency(position.assetProfile), etfProvider: this.extractEtfProvider({ assetSubClass: position.assetProfile.assetSubClass, name: position.assetProfile.name }), exchange: position.exchange, - name: position.assetProfile.name + name: position.assetProfile.name, + value: this.showValuesInPercentage() + ? position.allocationInPercentage + : (position.valueInBaseCurrency ?? 0) }; // Prepare analysis data by continents, countries, holdings and sectors @@ -373,53 +382,50 @@ export class GfAllocationsPageComponent implements OnInit { if (position.assetProfile.countries.length > 0) { for (const country of position.assetProfile.countries) { const { code, continent, weight } = country; + const value = + (isNumber(position.valueInBaseCurrency) + ? position.valueInBaseCurrency + : position.valueInPercentage) ?? 0; + + const continentData = this.continents[continent]; - if (this.continents[continent]?.value) { - this.continents[continent].value += - weight * - (isNumber(position.valueInBaseCurrency) - ? position.valueInBaseCurrency - : position.valueInPercentage); + if (continentData) { + continentData.value += weight * value; } else { this.continents[continent] = { name: translate(continent), - value: - weight * - (isNumber(position.valueInBaseCurrency) - ? this.portfolioDetails.holdings[symbol].valueInBaseCurrency - : this.portfolioDetails.holdings[symbol].valueInPercentage) + value: weight * value }; } - if (this.countries[code]?.value) { - this.countries[code].value += - weight * - (isNumber(position.valueInBaseCurrency) - ? position.valueInBaseCurrency - : position.valueInPercentage); + const countryData = this.countries[code]; + + if (countryData) { + countryData.value += weight * value; } else { this.countries[code] = { name: getCountryName({ code }), - value: - weight * - (isNumber(position.valueInBaseCurrency) - ? this.portfolioDetails.holdings[symbol].valueInBaseCurrency - : this.portfolioDetails.holdings[symbol].valueInPercentage) + value: weight * value }; } } } else { - this.continents[UNKNOWN_KEY].value += isNumber( - position.valueInBaseCurrency - ) - ? this.portfolioDetails.holdings[symbol].valueInBaseCurrency - : this.portfolioDetails.holdings[symbol].valueInPercentage; - - this.countries[UNKNOWN_KEY].value += isNumber( - position.valueInBaseCurrency - ) - ? this.portfolioDetails.holdings[symbol].valueInBaseCurrency - : this.portfolioDetails.holdings[symbol].valueInPercentage; + const value = + (isNumber(position.valueInBaseCurrency) + ? position.valueInBaseCurrency + : position.valueInPercentage) ?? 0; + + const continentData = this.continents[UNKNOWN_KEY]; + + if (continentData) { + continentData.value += value; + } + + const countryData = this.countries[UNKNOWN_KEY]; + + if (countryData) { + countryData.value += value; + } } if (position.assetProfile.holdings.length > 0) { @@ -429,21 +435,18 @@ export class GfAllocationsPageComponent implements OnInit { valueInBaseCurrency } of position.assetProfile.holdings) { const normalizedAssetName = this.normalizeAssetName(name); + const value = isNumber(valueInBaseCurrency) + ? valueInBaseCurrency + : allocationInPercentage * (position.valueInPercentage ?? 0); + + const holdingData = this.topHoldingsMap[normalizedAssetName]; - if (this.topHoldingsMap[normalizedAssetName]?.value) { - this.topHoldingsMap[normalizedAssetName].value += isNumber( - valueInBaseCurrency - ) - ? valueInBaseCurrency - : allocationInPercentage * - this.portfolioDetails.holdings[symbol].valueInPercentage; + if (holdingData) { + holdingData.value += value; } else { this.topHoldingsMap[normalizedAssetName] = { name, - value: isNumber(valueInBaseCurrency) - ? valueInBaseCurrency - : allocationInPercentage * - this.portfolioDetails.holdings[symbol].valueInPercentage + value }; } } @@ -452,30 +455,33 @@ export class GfAllocationsPageComponent implements OnInit { if (position.assetProfile.sectors.length > 0) { for (const sector of position.assetProfile.sectors) { const { name, weight } = sector; + const value = + (isNumber(position.valueInBaseCurrency) + ? position.valueInBaseCurrency + : position.valueInPercentage) ?? 0; - if (this.sectors[name]?.value) { - this.sectors[name].value += - weight * - (isNumber(position.valueInBaseCurrency) - ? position.valueInBaseCurrency - : position.valueInPercentage); + const sectorData = this.sectors[name]; + + if (sectorData) { + sectorData.value += weight * value; } else { this.sectors[name] = { name: translate(name), - value: - weight * - (isNumber(position.valueInBaseCurrency) - ? this.portfolioDetails.holdings[symbol].valueInBaseCurrency - : this.portfolioDetails.holdings[symbol].valueInPercentage) + value: weight * value }; } } } else { - this.sectors[UNKNOWN_KEY].value += isNumber( - position.valueInBaseCurrency - ) - ? this.portfolioDetails.holdings[symbol].valueInBaseCurrency - : this.portfolioDetails.holdings[symbol].valueInPercentage; + const value = + (isNumber(position.valueInBaseCurrency) + ? position.valueInBaseCurrency + : position.valueInPercentage) ?? 0; + + const sectorData = this.sectors[UNKNOWN_KEY]; + + if (sectorData) { + sectorData.value += value; + } } if (this.holdings[symbol].assetSubClass === 'ETF') { @@ -484,23 +490,26 @@ export class GfAllocationsPageComponent implements OnInit { this.symbols[prettifySymbol(symbol)] = { dataSource: position.assetProfile.dataSource, - name: position.assetProfile.name, + name: position.assetProfile.name ?? '', symbol: prettifySymbol(symbol), - value: isNumber(position.valueInBaseCurrency) - ? position.valueInBaseCurrency - : position.valueInPercentage + value: + (isNumber(position.valueInBaseCurrency) + ? position.valueInBaseCurrency + : position.valueInPercentage) ?? 0 }; } this.markets = this.portfolioDetails.markets; - Object.values(this.portfolioDetails.marketsAdvanced).forEach( - ({ id, valueInBaseCurrency, valueInPercentage }) => { - this.marketsAdvanced[id].value = isNumber(valueInBaseCurrency) - ? valueInBaseCurrency - : valueInPercentage; - } - ); + if (this.portfolioDetails.marketsAdvanced) { + Object.values(this.portfolioDetails.marketsAdvanced).forEach( + ({ id, valueInBaseCurrency, valueInPercentage }) => { + this.marketsAdvanced[id].value = isNumber(valueInBaseCurrency) + ? valueInBaseCurrency + : valueInPercentage; + } + ); + } for (const [ id, @@ -509,7 +518,7 @@ export class GfAllocationsPageComponent implements OnInit { let value = 0; if (this.showValuesInPercentage()) { - value = valueInPercentage; + value = valueInPercentage ?? 0; } else { value = valueInBaseCurrency; } @@ -522,12 +531,11 @@ export class GfAllocationsPageComponent implements OnInit { } this.topHoldings = Object.values(this.topHoldingsMap) - .map(({ name, value }) => { + .map(({ name, value }): HoldingWithParents => { if (this.showValuesInPercentage()) { return { name, - allocationInPercentage: value, - valueInBaseCurrency: null + allocationInPercentage: value }; } @@ -547,11 +555,12 @@ export class GfAllocationsPageComponent implements OnInit { } ); - return currentParentHolding + return currentParentHolding && + isNumber(currentParentHolding.valueInBaseCurrency) ? { allocationInPercentage: currentParentHolding.valueInBaseCurrency / value, - name: holding.assetProfile.name, + name: holding.assetProfile.name ?? '', position: holding, symbol: prettifySymbol(symbol), valueInBaseCurrency: @@ -596,26 +605,22 @@ export class GfAllocationsPageComponent implements OnInit { autoFocus: false, data: { accountId: aAccountId, - deviceType: this.deviceType, + deviceType: this.deviceType(), hasImpersonationId: this.hasImpersonationId, hasPermissionToCreateActivity: !this.hasImpersonationId && hasPermission(this.user?.permissions, permissions.createActivity) && !this.user?.settings?.isRestrictedView }, - height: this.deviceType === 'mobile' ? '98vh' : '80vh', - width: this.deviceType === 'mobile' ? '100vw' : '50rem' + height: this.deviceType() === 'mobile' ? '98vh' : '80vh', + width: this.deviceType() === 'mobile' ? '100vw' : '50rem' }); dialogRef .afterClosed() .pipe(takeUntilDestroyed(this.destroyRef)) .subscribe(() => { - this.router.navigate(['.'], { relativeTo: this.route }); + void this.router.navigate(['.'], { relativeTo: this.route }); }); } - - public showValuesInPercentage() { - return this.hasImpersonationId || this.user?.settings?.isRestrictedView; - } } diff --git a/apps/client/src/app/pages/portfolio/allocations/allocations-page.html b/apps/client/src/app/pages/portfolio/allocations/allocations-page.html index a1000189b..d5c765958 100644 --- a/apps/client/src/app/pages/portfolio/allocations/allocations-page.html +++ b/apps/client/src/app/pages/portfolio/allocations/allocations-page.html @@ -115,7 +115,7 @@ [isInPercentage]="showValuesInPercentage()" [keys]="['symbol']" [locale]="user?.settings?.locale" - [showLabels]="deviceType !== 'mobile'" + [showLabels]="deviceType() !== 'mobile'" (proportionChartClicked)="onSymbolChartClicked($event)" /> diff --git a/apps/client/src/app/pages/portfolio/allocations/interfaces/interfaces.ts b/apps/client/src/app/pages/portfolio/allocations/interfaces/interfaces.ts new file mode 100644 index 000000000..dc90f5f95 --- /dev/null +++ b/apps/client/src/app/pages/portfolio/allocations/interfaces/interfaces.ts @@ -0,0 +1,6 @@ +import { Params } from '@angular/router'; + +export interface AllocationsPageParams extends Params { + accountDetailDialog?: string; + accountId?: string; +} diff --git a/libs/common/src/lib/interfaces/holding.interface.ts b/libs/common/src/lib/interfaces/holding.interface.ts index e963bc5a7..c63729a34 100644 --- a/libs/common/src/lib/interfaces/holding.interface.ts +++ b/libs/common/src/lib/interfaces/holding.interface.ts @@ -1,5 +1,5 @@ export interface Holding { allocationInPercentage: number; name: string; - valueInBaseCurrency: number; + valueInBaseCurrency?: number; }