Task/improve type safety in holding detail dialog component (#7050)

Improve type safety
6 days ago · 5bb8bf834b
5 changed files with 137 additions and 114 deletions
--- a/apps/client/src/app/components/holding-detail-dialog/holding-detail-dialog.component.ts
+++ b/apps/client/src/app/components/holding-detail-dialog/holding-detail-dialog.component.ts
@ -16,6 +16,7 @@ import {
  EnhancedSymbolProfile,
  Filter,
  LineChartItem,
+  NullableLineChartItem,
  User
 } from '@ghostfolio/common/interfaces';
 import { hasPermission, permissions } from '@ghostfolio/common/permissions';
@ -39,11 +40,16 @@ import {
  ChangeDetectorRef,
  Component,
  DestroyRef,
-  Inject,
-  OnInit
+  OnInit,
+  inject
 } from '@angular/core';
 import { takeUntilDestroyed } from '@angular/core/rxjs-interop';
-import { FormBuilder, FormGroup, ReactiveFormsModule } from '@angular/forms';
+import {
+  FormBuilder,
+  FormControl,
+  FormGroup,
+  ReactiveFormsModule
+} from '@angular/forms';
 import { MatButtonModule } from '@angular/material/button';
 import { MatChipsModule } from '@angular/material/chips';
 import {
@ -71,6 +77,7 @@ import {
  swapVerticalOutline,
  walletOutline
 } from 'ionicons/icons';
+import { isNumber } from 'lodash';
 import { NgxSkeletonLoaderModule } from 'ngx-skeleton-loader';
 import { switchMap } from 'rxjs/operators';

@ -106,75 +113,78 @@ import { HoldingDetailDialogParams } from './interfaces/interfaces';
  templateUrl: 'holding-detail-dialog.html'
 })
 export class GfHoldingDetailDialogComponent implements OnInit {
-  public activitiesCount: number;
-  public accounts: Account[];
-  public assetClass: string;
-  public assetSubClass: string;
-  public averagePrice: number;
-  public averagePricePrecision = 2;
-  public benchmarkDataItems: LineChartItem[];
-  public benchmarkLabel = $localize`Average Unit Price`;
-  public countries: {
+  protected accounts: Account[];
+  protected activitiesCount: number;
+  protected assetClass: string;
+  protected assetSubClass: string;
+  protected averagePrice: number;
+  protected averagePricePrecision = 2;
+  protected benchmarkDataItems: NullableLineChartItem[];
+  protected readonly benchmarkLabel = $localize`Average Unit Price`;
+  protected countries: {
    [code: string]: { name: string; value: number };
  };
-  public dataProviderInfo: DataProviderInfo;
-  public dataSource: MatTableDataSource<Activity>;
-  public dateOfFirstActivity: string;
-  public dividendInBaseCurrency: number;
-  public dividendInBaseCurrencyPrecision = 2;
-  public dividendYieldPercentWithCurrencyEffect: number;
-  public feeInBaseCurrency: number;
-  public getCountryName = getCountryName;
-  public hasPermissionToCreateOwnTag: boolean;
-  public hasPermissionToReadMarketDataOfOwnAssetProfile: boolean;
-  public historicalDataItems: LineChartItem[];
-  public holdingForm: FormGroup;
-  public investmentInBaseCurrencyWithCurrencyEffect: number;
-  public investmentInBaseCurrencyWithCurrencyEffectPrecision = 2;
-  public isUUID = isUUID;
-  public marketDataItems: MarketData[] = [];
-  public marketPrice: number;
-  public marketPriceMax: number;
-  public marketPriceMaxPrecision = 2;
-  public marketPriceMin: number;
-  public marketPriceMinPrecision = 2;
-  public marketPricePrecision = 2;
-  public netPerformance: number;
-  public netPerformancePrecision = 2;
-  public netPerformancePercent: number;
-  public netPerformancePercentWithCurrencyEffect: number;
-  public netPerformancePercentWithCurrencyEffectPrecision = 2;
-  public netPerformanceWithCurrencyEffect: number;
-  public netPerformanceWithCurrencyEffectPrecision = 2;
-  public pageIndex = 0;
-  public pageSize = DEFAULT_PAGE_SIZE;
-  public quantity: number;
-  public quantityPrecision = 2;
-  public reportDataGlitchMail: string;
-  public routerLinkAdminControlMarketData =
+  protected dataProviderInfo: DataProviderInfo;
+  protected dataSource: MatTableDataSource<Activity>;
+  protected dateOfFirstActivity: Date;
+  protected dividendInBaseCurrency: number;
+  protected dividendInBaseCurrencyPrecision = 2;
+  protected dividendYieldPercentWithCurrencyEffect: number;
+  protected feeInBaseCurrency: number;
+  protected readonly getCountryName = getCountryName;
+  protected hasPermissionToCreateOwnTag: boolean;
+  protected hasPermissionToReadMarketDataOfOwnAssetProfile: boolean;
+  protected historicalDataItems: LineChartItem[];
+  protected holdingForm: FormGroup<{
+    tags: FormControl<Tag[] | null>;
+  }>;
+  protected investmentInBaseCurrencyWithCurrencyEffect: number;
+  protected investmentInBaseCurrencyWithCurrencyEffectPrecision = 2;
+  protected readonly isUUID = isUUID;
+  protected marketDataItems: MarketData[] = [];
+  protected marketPrice: number;
+  protected marketPriceMax: number;
+  protected marketPriceMaxPrecision = 2;
+  protected marketPriceMin: number;
+  protected marketPriceMinPrecision = 2;
+  protected marketPricePrecision = 2;
+  protected netPerformancePercentWithCurrencyEffect: number;
+  protected netPerformancePercentWithCurrencyEffectPrecision = 2;
+  protected netPerformanceWithCurrencyEffect: number;
+  protected netPerformanceWithCurrencyEffectPrecision = 2;
+  protected pageIndex = 0;
+  protected readonly pageSize = DEFAULT_PAGE_SIZE;
+  protected quantity: number;
+  protected quantityPrecision = 2;
+  protected reportDataGlitchMail: string;
+  protected readonly routerLinkAdminControlMarketData =
    internalRoutes.adminControl.subRoutes.marketData.routerLink;
-  public sectors: {
+  protected sectors: {
    [name: string]: { name: string; value: number };
  };
-  public sortColumn = 'date';
-  public sortDirection: SortDirection = 'desc';
-  public SymbolProfile: EnhancedSymbolProfile;
-  public tags: Tag[];
-  public tagsAvailable: Tag[];
-  public translate = translate;
-  public user: User;
-  public value: number;
-
-  public constructor(
-    private changeDetectorRef: ChangeDetectorRef,
-    private dataService: DataService,
-    private destroyRef: DestroyRef,
-    public dialogRef: MatDialogRef<GfHoldingDetailDialogComponent>,
-    @Inject(MAT_DIALOG_DATA) public data: HoldingDetailDialogParams,
-    private formBuilder: FormBuilder,
-    private router: Router,
-    private userService: UserService
-  ) {
+  protected sortColumn = 'date';
+  protected sortDirection: SortDirection = 'desc';
+  protected SymbolProfile: EnhancedSymbolProfile;
+  protected tagsAvailable: Tag[];
+  protected readonly translate = translate;
+  protected user: User;
+  protected value: number;
+
+  protected readonly data = inject<HoldingDetailDialogParams>(MAT_DIALOG_DATA);
+  protected readonly dialogRef = inject(
+    MatDialogRef<GfHoldingDetailDialogComponent>
+  );
+
+  private tags: Tag[];
+
+  private readonly changeDetectorRef = inject(ChangeDetectorRef);
+  private readonly dataService = inject(DataService);
+  private readonly destroyRef = inject(DestroyRef);
+  private readonly formBuilder = inject(FormBuilder);
+  private readonly router = inject(Router);
+  private readonly userService = inject(UserService);
+
+  public constructor() {
    addIcons({
      arrowDownCircleOutline,
      createOutline,
@ -190,12 +200,11 @@ export class GfHoldingDetailDialogComponent implements OnInit {
    const filters = this.getActivityFilters();

    this.holdingForm = this.formBuilder.group({
-      tags: [] as string[]
+      tags: new FormControl<Tag[]>([])
    });

-    this.holdingForm
-      .get('tags')
-      .valueChanges.pipe(takeUntilDestroyed(this.destroyRef))
+    this.holdingForm.controls.tags.valueChanges
+      .pipe(takeUntilDestroyed(this.destroyRef))
      .subscribe((tags: Tag[]) => {
        const newTag = tags.find(({ id }) => {
          return id === undefined;
@ -268,8 +277,6 @@ export class GfHoldingDetailDialogComponent implements OnInit {
          marketPrice,
          marketPriceMax,
          marketPriceMin,
-          netPerformance,
-          netPerformancePercent,
          netPerformancePercentWithCurrencyEffect,
          netPerformanceWithCurrencyEffect,
          quantity,
@ -290,7 +297,11 @@ export class GfHoldingDetailDialogComponent implements OnInit {
          this.benchmarkDataItems = [];
          this.countries = {};
          this.dataProviderInfo = dataProviderInfo;
-          this.dateOfFirstActivity = dateOfFirstActivity;
+
+          if (dateOfFirstActivity) {
+            this.dateOfFirstActivity = dateOfFirstActivity;
+          }
+
          this.dividendInBaseCurrency = dividendInBaseCurrency;

          if (
@ -318,12 +329,12 @@ export class GfHoldingDetailDialogComponent implements OnInit {
            ({ averagePrice, date, marketPrice }) => {
              this.benchmarkDataItems.push({
                date,
-                value: averagePrice
+                value: isNumber(averagePrice) ? averagePrice : null
              });

              return {
                date,
-                value: marketPrice
+                value: marketPrice ?? 0
              };
            }
          );
@ -365,17 +376,6 @@ export class GfHoldingDetailDialogComponent implements OnInit {
            this.marketPricePrecision = 0;
          }

-          this.netPerformance = netPerformance;
-
-          if (
-            this.data.deviceType === 'mobile' &&
-            this.netPerformance >= NUMERICAL_PRECISION_THRESHOLD_5_FIGURES
-          ) {
-            this.netPerformancePrecision = 0;
-          }
-
-          this.netPerformancePercent = netPerformancePercent;
-
          this.netPerformancePercentWithCurrencyEffect =
            netPerformancePercentWithCurrencyEffect;

@ -456,16 +456,16 @@ export class GfHoldingDetailDialogComponent implements OnInit {
            }
          }

-          if (isToday(parseISO(this.dateOfFirstActivity))) {
+          if (isToday(this.dateOfFirstActivity)) {
            // Add average price
            this.historicalDataItems.push({
-              date: this.dateOfFirstActivity,
+              date: this.dateOfFirstActivity.toISOString(),
              value: this.averagePrice
            });

            // Add benchmark 1
            this.benchmarkDataItems.push({
-              date: this.dateOfFirstActivity,
+              date: this.dateOfFirstActivity.toISOString(),
              value: averagePrice
            });

@ -496,7 +496,7 @@ export class GfHoldingDetailDialogComponent implements OnInit {

          if (
            this.benchmarkDataItems[0]?.value === undefined &&
-            isSameMonth(parseISO(this.dateOfFirstActivity), new Date())
+            isSameMonth(this.dateOfFirstActivity, new Date())
          ) {
            this.benchmarkDataItems[0].value = this.averagePrice;
          }
@ -526,7 +526,7 @@ export class GfHoldingDetailDialogComponent implements OnInit {

          this.hasPermissionToCreateOwnTag =
            hasPermission(this.user.permissions, permissions.createOwnTag) &&
-            this.user?.settings?.isExperimentalFeatures;
+            (this.user?.settings?.isExperimentalFeatures ?? false);

          this.tagsAvailable =
            this.user?.tags?.map((tag) => {
@ -541,13 +541,13 @@ export class GfHoldingDetailDialogComponent implements OnInit {
      });
  }

-  public onChangePage(page: PageEvent) {
+  protected onChangePage(page: PageEvent) {
    this.pageIndex = page.pageIndex;

    this.fetchActivities();
  }

-  public onCloneActivity(aActivity: Activity) {
+  protected onCloneActivity(aActivity: Activity) {
    this.router.navigate(
      internalRoutes.portfolio.subRoutes.activities.routerLink,
      {
@ -558,22 +558,22 @@ export class GfHoldingDetailDialogComponent implements OnInit {
    this.dialogRef.close();
  }

-  public onClose() {
+  protected onClose() {
    this.dialogRef.close();
  }

-  public onCloseHolding() {
+  protected onCloseHolding() {
    const today = new Date();

    const activity: CreateOrderDto = {
-      accountId: this.accounts.length === 1 ? this.accounts[0].id : null,
-      comment: null,
-      currency: this.SymbolProfile.currency,
-      dataSource: this.SymbolProfile.dataSource,
+      accountId: this.accounts.length === 1 ? this.accounts[0].id : undefined,
+      comment: undefined,
+      currency: this.SymbolProfile?.currency ?? '',
+      dataSource: this.SymbolProfile?.dataSource,
      date: today.toISOString(),
      fee: 0,
      quantity: this.quantity,
-      symbol: this.SymbolProfile.symbol,
+      symbol: this.SymbolProfile?.symbol ?? '',
      tags: this.tags.map(({ id }) => {
        return id;
      }),
@ -593,7 +593,7 @@ export class GfHoldingDetailDialogComponent implements OnInit {
      });
  }

-  public onExport() {
+  protected onExport() {
    const activityIds = this.dataSource.data.map(({ id }) => {
      return id;
    });
@ -613,13 +613,13 @@ export class GfHoldingDetailDialogComponent implements OnInit {
      });
  }

-  public onMarketDataChanged(withRefresh = false) {
+  protected onMarketDataChanged(withRefresh = false) {
    if (withRefresh) {
      this.fetchMarketData();
    }
  }

-  public onUpdateActivity(aActivity: Activity) {
+  protected onUpdateActivity(aActivity: Activity) {
    this.router.navigate(
      internalRoutes.portfolio.subRoutes.activities.routerLink,
      {
--- a/libs/common/src/lib/interfaces/index.ts
+++ b/libs/common/src/lib/interfaces/index.ts
@ -25,7 +25,10 @@ import type { HoldingWithParents } from './holding-with-parents.interface';
 import type { Holding } from './holding.interface';
 import type { InfoItem } from './info-item.interface';
 import type { InvestmentItem } from './investment-item.interface';
-import type { LineChartItem } from './line-chart-item.interface';
+import type {
+  LineChartItem,
+  NullableLineChartItem
+} from './line-chart-item.interface';
 import type { LookupItem } from './lookup-item.interface';
 import type { MarketData } from './market-data.interface';
 import type { PortfolioChart } from './portfolio-chart.interface';
@ -158,6 +161,7 @@ export {
  MarketData,
  MarketDataDetailsResponse,
  MarketDataOfMarketsResponse,
+  NullableLineChartItem,
  OAuthResponse,
  PlatformsResponse,
  PortfolioChart,
--- a/libs/common/src/lib/interfaces/line-chart-item.interface.ts
+++ b/libs/common/src/lib/interfaces/line-chart-item.interface.ts
@ -1,4 +1,6 @@
-export interface LineChartItem {
+export interface LineChartItem<T = number> {
  date: string;
-  value: number;
+  value: T;
 }
+
+export type NullableLineChartItem = LineChartItem<number | null>;
--- a/libs/ui/src/lib/historical-market-data-editor/historical-market-data-editor.component.ts
+++ b/libs/ui/src/lib/historical-market-data-editor/historical-market-data-editor.component.ts
@ -68,7 +68,7 @@ export class GfHistoricalMarketDataEditorComponent

  @Input() currency: string;
  @Input() dataSource: DataSource;
-  @Input() dateOfFirstActivity: string;
+  @Input() dateOfFirstActivity: Date;
  @Input() symbol: string;
  @Input() user: User;

@ -124,7 +124,7 @@ export class GfHistoricalMarketDataEditorComponent

  public ngOnChanges() {
    if (this.dateOfFirstActivity) {
-      let date = parseISO(this.dateOfFirstActivity);
+      let date = this.dateOfFirstActivity;

      const missingMarketData: { date: Date; marketPrice?: number }[] = [];

@ -174,7 +174,7 @@ export class GfHistoricalMarketDataEditorComponent
      const dates = Object.keys(this.marketDataByMonth).sort();
      const startDateString = first(dates);
      const startDate = min([
-        parseISO(this.dateOfFirstActivity),
+        this.dateOfFirstActivity,
        ...(startDateString ? [parseISO(startDateString)] : [])
      ]);
      const endDateString = last(dates);
--- a/libs/ui/src/lib/services/data.service.ts
+++ b/libs/ui/src/lib/services/data.service.ts
@ -445,10 +445,27 @@ export class DataService {
  }: {
    dataSource: DataSource;
    symbol: string;
-  }) {
-    return this.http.get<PortfolioHoldingResponse>(
-      `/api/v1/portfolio/holding/${dataSource}/${symbol}`
-    );
+  }): Observable<
+    Omit<PortfolioHoldingResponse, 'dateOfFirstActivity'> & {
+      dateOfFirstActivity: Date | undefined;
+    }
+  > {
+    return this.http
+      .get<PortfolioHoldingResponse>(
+        `/api/v1/portfolio/holding/${dataSource}/${symbol}`
+      )
+      .pipe(
+        map((response) => {
+          const dateOfFirstActivity = response.dateOfFirstActivity
+            ? parseISO(response.dateOfFirstActivity)
+            : undefined;
+
+          return {
+            ...response,
+            dateOfFirstActivity
+          };
+        })
+      );
  }

  public fetchInfo(): InfoItem {