Task/improve type safety in allocations page component (#7076)

Improve type safety

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<Account, 'name'> & {
       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;
+  protected readonly deviceType = computed(
-  public hasImpersonationId: boolean;
+    () => this.deviceDetectorService.deviceInfo().deviceType
-  public holdings: {
+  );
+  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;
+  protected isLoading = false;
-  public markets: {
+  protected markets: PortfolioDetails['markets'];
-    [key in Market]: { id: Market; valueInPercentage: number };
+  protected marketsAdvanced: {
-  };
-  public marketsAdvanced: {
     [key in MarketAdvanced]: {
       id: MarketAdvanced;
       name: string;
       value: number;
     };
   };
-  public platforms: {
+  protected platforms: {
     [id: string]: Pick<Platform, 'name'> & {
       id: string;
       value: number;
     };
   };
-  public portfolioDetails: PortfolioDetails;
+  protected portfolioDetails: PortfolioDetails;
-  public sectors: {
+  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[];
+  protected topHoldings: HoldingWithParents[];
-  public topHoldingsMap: {
+  protected readonly UNKNOWN_KEY = UNKNOWN_KEY;
+  protected user: User;
+
+  private topHoldingsMap: {
     [name: string]: { name: string; value: number };
   };
-  public totalValueInEtf = 0;
+  private totalValueInEtf = 0;
-  public UNKNOWN_KEY = UNKNOWN_KEY;
+
-  public user: User;
+  private readonly changeDetectorRef = inject(ChangeDetectorRef);
-  public worldMapChartFormat: string;
+  private readonly dataService = inject(DataService);
-
+  private readonly destroyRef = inject(DestroyRef);
-  public constructor(
+  private readonly deviceDetectorService = inject(DeviceDetectorService);
-    private changeDetectorRef: ChangeDetectorRef,
+  private readonly dialog = inject(MatDialog);
-    private dataService: DataService,
+  private readonly impersonationStorageService = inject(
-    private destroyRef: DestroyRef,
+    ImpersonationStorageService
-    private deviceDetectorService: DeviceDetectorService,
+  );
-    private dialog: MatDialog,
+  private readonly route = inject(ActivatedRoute);
-    private impersonationStorageService: ImpersonationStorageService,
+  private readonly router = inject(Router);
-    private route: ActivatedRoute,
+  private readonly userService = inject(UserService);
-    private router: Router,
+
-    private userService: UserService
+  public constructor() {
-  ) {
     this.route.queryParams
       .pipe(takeUntilDestroyed(this.destroyRef))
-      .subscribe((params) => {
+      .subscribe(
-        if (params['accountId'] && params['accountDetailDialog']) {
+        ({ accountId, accountDetailDialog }: AllocationsPageParams) => {
-          this.openAccountDetailDialog(params['accountId']);
+          if (accountId && accountDetailDialog) {
+            this.openAccountDetailDialog(accountId);
+          }
         }
-      });
+      );
   }
 
-  public ngOnInit() {
+  protected get worldMapChartFormat(): string {
-    this.deviceType = this.deviceDetectorService.getDeviceInfo().deviceType;
+    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))
+      .pipe(
-      .subscribe((state) => {
+        filter((state) => !!state?.user),
-        if (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()
+          this.changeDetectorRef.markForCheck();
-            .pipe(takeUntilDestroyed(this.destroyRef))
+        }),
-            .subscribe((portfolioDetails) => {
+        switchMap(() => this.fetchPortfolioDetails()),
-              this.initialize();
+        takeUntilDestroyed(this.destroyRef)
-
+      )
-              this.portfolioDetails = portfolioDetails;
+      .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) {
+          if (continentData) {
-            this.continents[continent].value +=
+            continentData.value += weight * value;
-              weight *
-              (isNumber(position.valueInBaseCurrency)
-                ? position.valueInBaseCurrency
-                : position.valueInPercentage);
           } else {
             this.continents[continent] = {
               name: translate(continent),
-              value:
+              value: weight * value
-                weight *
-                (isNumber(position.valueInBaseCurrency)
-                  ? this.portfolioDetails.holdings[symbol].valueInBaseCurrency
-                  : this.portfolioDetails.holdings[symbol].valueInPercentage)
             };
           }
 
-          if (this.countries[code]?.value) {
+          const countryData = this.countries[code];
-            this.countries[code].value +=
+
-              weight *
+          if (countryData) {
-              (isNumber(position.valueInBaseCurrency)
+            countryData.value += weight * value;
-                ? position.valueInBaseCurrency
-                : position.valueInPercentage);
           } else {
             this.countries[code] = {
               name: getCountryName({ code }),
-              value:
+              value: weight * value
-                weight *
-                (isNumber(position.valueInBaseCurrency)
-                  ? this.portfolioDetails.holdings[symbol].valueInBaseCurrency
-                  : this.portfolioDetails.holdings[symbol].valueInPercentage)
             };
           }
         }
       } else {
-        this.continents[UNKNOWN_KEY].value += isNumber(
+        const value =
-          position.valueInBaseCurrency
+          (isNumber(position.valueInBaseCurrency)
-        )
+            ? position.valueInBaseCurrency
-          ? this.portfolioDetails.holdings[symbol].valueInBaseCurrency
+            : position.valueInPercentage) ?? 0;
-          : this.portfolioDetails.holdings[symbol].valueInPercentage;
+
-
+        const continentData = this.continents[UNKNOWN_KEY];
-        this.countries[UNKNOWN_KEY].value += isNumber(
+
-          position.valueInBaseCurrency
+        if (continentData) {
-        )
+          continentData.value += value;
-          ? this.portfolioDetails.holdings[symbol].valueInBaseCurrency
+        }
-          : this.portfolioDetails.holdings[symbol].valueInPercentage;
+
+        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) {
+          if (holdingData) {
-            this.topHoldingsMap[normalizedAssetName].value += isNumber(
+            holdingData.value += value;
-              valueInBaseCurrency
-            )
-              ? valueInBaseCurrency
-              : allocationInPercentage *
-                this.portfolioDetails.holdings[symbol].valueInPercentage;
           } else {
             this.topHoldingsMap[normalizedAssetName] = {
               name,
-              value: isNumber(valueInBaseCurrency)
+              value
-                ? valueInBaseCurrency
-                : allocationInPercentage *
-                  this.portfolioDetails.holdings[symbol].valueInPercentage
             };
           }
         }
@@ -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) {
+          const sectorData = this.sectors[name];
-            this.sectors[name].value +=
+
-              weight *
+          if (sectorData) {
-              (isNumber(position.valueInBaseCurrency)
+            sectorData.value += weight * value;
-                ? position.valueInBaseCurrency
-                : position.valueInPercentage);
           } else {
             this.sectors[name] = {
               name: translate(name),
-              value:
+              value: weight * value
-                weight *
-                (isNumber(position.valueInBaseCurrency)
-                  ? this.portfolioDetails.holdings[symbol].valueInBaseCurrency
-                  : this.portfolioDetails.holdings[symbol].valueInPercentage)
             };
           }
         }
       } else {
-        this.sectors[UNKNOWN_KEY].value += isNumber(
+        const value =
-          position.valueInBaseCurrency
+          (isNumber(position.valueInBaseCurrency)
-        )
+            ? position.valueInBaseCurrency
-          ? this.portfolioDetails.holdings[symbol].valueInBaseCurrency
+            : position.valueInPercentage) ?? 0;
-          : this.portfolioDetails.holdings[symbol].valueInPercentage;
+
+        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)
+        value:
-          ? position.valueInBaseCurrency
+          (isNumber(position.valueInBaseCurrency)
-          : position.valueInPercentage
+            ? position.valueInBaseCurrency
+            : position.valueInPercentage) ?? 0
       };
     }
 
     this.markets = this.portfolioDetails.markets;
 
-    Object.values(this.portfolioDetails.marketsAdvanced).forEach(
+    if (this.portfolioDetails.marketsAdvanced) {
-      ({ id, valueInBaseCurrency, valueInPercentage }) => {
+      Object.values(this.portfolioDetails.marketsAdvanced).forEach(
-        this.marketsAdvanced[id].value = isNumber(valueInBaseCurrency)
+        ({ id, valueInBaseCurrency, valueInPercentage }) => {
-          ? valueInBaseCurrency
+          this.marketsAdvanced[id].value = isNumber(valueInBaseCurrency)
-          : valueInPercentage;
+            ? 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,
+            allocationInPercentage: value
-            valueInBaseCurrency: null
           };
         }
 
@@ -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',
+      height: this.deviceType() === 'mobile' ? '98vh' : '80vh',
-      width: this.deviceType === 'mobile' ? '100vw' : '50rem'
+      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;
-  }
 }

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)"
           />
         </mat-card-content>

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;
+}

libs/common/src/lib/interfaces/holding.interface.ts

@@ -1,5 +1,5 @@
 export interface Holding {
   allocationInPercentage: number;
   name: string;
-  valueInBaseCurrency: number;
+  valueInBaseCurrency?: number;
 }