From ec3a6ecba0cf424f67894ca3ed66c857cb298b9c Mon Sep 17 00:00:00 2001 From: Asish Kumar Date: Thu, 9 Apr 2026 17:48:48 +0000 Subject: [PATCH] fix(asset-profile): save benchmark toggle through form submit Route the Benchmark / Markets checkbox through the asset profile form instead of persisting it immediately on change. This makes the Save button reflect benchmark edits and keeps the benchmark update in the same submit flow as the rest of the dialog. Add a focused component test for the benchmark save behavior and document the fix in the changelog. --- CHANGELOG.md | 1 + .../asset-profile-dialog.component.spec.ts | 175 ++++++++++++++++++ .../asset-profile-dialog.component.ts | 75 +++++--- .../asset-profile-dialog.html | 13 +- 4 files changed, 224 insertions(+), 40 deletions(-) create mode 100644 apps/client/src/app/components/admin-market-data/asset-profile-dialog/asset-profile-dialog.component.spec.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ebab8184..df24374c3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Fixed the benchmark toggle in the asset profile details dialog to participate in the regular save flow - Improved the style of the activity type component ## 2.253.0 - 2026-03-06 diff --git a/apps/client/src/app/components/admin-market-data/asset-profile-dialog/asset-profile-dialog.component.spec.ts b/apps/client/src/app/components/admin-market-data/asset-profile-dialog/asset-profile-dialog.component.spec.ts new file mode 100644 index 000000000..9c6711ef4 --- /dev/null +++ b/apps/client/src/app/components/admin-market-data/asset-profile-dialog/asset-profile-dialog.component.spec.ts @@ -0,0 +1,175 @@ +import { AdminService, DataService } from '@ghostfolio/ui/services'; + +import { ChangeDetectorRef, DestroyRef } from '@angular/core'; +import { FormBuilder } from '@angular/forms'; +import { MatDialogRef } from '@angular/material/dialog'; +import { MatSnackBar } from '@angular/material/snack-bar'; +import 'reflect-metadata'; +import { of } from 'rxjs'; + +import { GfAssetProfileDialogComponent } from './asset-profile-dialog.component'; + +(globalThis as any).$localize = ( + strings: TemplateStringsArray, + ...expressions: unknown[] +) => { + return strings.reduce((result, string, index) => { + return result + string + String(expressions[index] ?? ''); + }, ''); +}; + +jest.mock('@ghostfolio/common/utils', () => { + const actual = jest.requireActual('@ghostfolio/common/utils'); + + return { + ...actual, + validateObjectForForm: jest.fn().mockResolvedValue(undefined) + }; +}); + +jest.mock('@ghostfolio/client/services/user/user.service', () => ({ + UserService: class {} +})); + +jest.mock( + '@ghostfolio/client/components/admin-market-data/admin-market-data.service', + () => ({ + AdminMarketDataService: class {} + }) +); + +jest.mock('@ionic/angular/standalone', () => ({ + IonIcon: class {} +})); + +jest.mock('@ghostfolio/ui/currency-selector', () => ({ + GfCurrencySelectorComponent: class {} +})); + +jest.mock('@ghostfolio/ui/entity-logo', () => ({ + GfEntityLogoComponent: class {} +})); + +jest.mock('@ghostfolio/ui/historical-market-data-editor', () => ({ + GfHistoricalMarketDataEditorComponent: class {} +})); + +jest.mock('@ghostfolio/ui/i18n', () => ({ + translate: (value: string) => value +})); + +jest.mock('@ghostfolio/ui/line-chart', () => ({ + GfLineChartComponent: class {} +})); + +jest.mock('@ghostfolio/ui/notifications', () => ({ + NotificationService: class {} +})); + +jest.mock('@ghostfolio/ui/portfolio-proportion-chart', () => ({ + GfPortfolioProportionChartComponent: class {} +})); + +jest.mock('@ghostfolio/ui/symbol-autocomplete', () => ({ + GfSymbolAutocompleteComponent: class {} +})); + +jest.mock('@ghostfolio/ui/value', () => ({ + GfValueComponent: class {} +})); + +describe('GfAssetProfileDialogComponent', () => { + let component: GfAssetProfileDialogComponent; + let adminService: jest.Mocked; + let dataService: jest.Mocked; + + beforeEach(() => { + adminService = { + patchAssetProfile: jest.fn().mockReturnValue(of(undefined)) + } as unknown as jest.Mocked; + + dataService = { + deleteBenchmark: jest.fn().mockReturnValue(of(undefined)), + postBenchmark: jest.fn().mockReturnValue(of(undefined)), + updateInfo: jest.fn() + } as unknown as jest.Mocked; + + component = new GfAssetProfileDialogComponent( + {} as never, + adminService, + { markForCheck: jest.fn() } as ChangeDetectorRef, + { dataSource: 'YAHOO', symbol: 'AAPL' } as never, + dataService, + {} as DestroyRef, + { close: jest.fn() } as MatDialogRef, + new FormBuilder(), + {} as never, + { open: jest.fn() } as unknown as MatSnackBar, + {} as never + ); + + component.assetProfile = { + id: 'asset-profile-id', + isActive: true + } as never; + component.benchmarks = []; + + component.assetProfileForm.patchValue({ + countries: '[]', + currency: 'USD', + isActive: true, + name: 'Apple Inc.', + scraperConfiguration: { + defaultMarketPrice: null, + headers: '{}', + locale: '', + mode: '', + selector: '', + url: '' + }, + sectors: '[]', + symbolMapping: '{}', + url: '' + }); + + jest.spyOn(component, 'initialize').mockImplementation(); + }); + + it('only updates the benchmark when the form is submitted', async () => { + component.assetProfileForm.get('isBenchmark')?.setValue(true); + + expect(dataService.postBenchmark).not.toHaveBeenCalled(); + + await component.onSubmitAssetProfileForm(); + + expect(adminService.patchAssetProfile).toHaveBeenCalledTimes(1); + expect(dataService.postBenchmark).toHaveBeenCalledWith({ + dataSource: 'YAHOO', + symbol: 'AAPL' + }); + expect(dataService.updateInfo).toHaveBeenCalledTimes(1); + }); + + it('removes the benchmark when it is unchecked and saved', async () => { + component.benchmarks = [{ id: 'asset-profile-id' }]; + component.isBenchmark = true; + component.assetProfileForm.get('isBenchmark')?.setValue(false); + + await component.onSubmitAssetProfileForm(); + + expect(dataService.deleteBenchmark).toHaveBeenCalledWith({ + dataSource: 'YAHOO', + symbol: 'AAPL' + }); + expect(dataService.postBenchmark).not.toHaveBeenCalled(); + }); + + it('does not call benchmark endpoints when the state is unchanged', async () => { + component.assetProfileForm.get('isBenchmark')?.setValue(false); + + await component.onSubmitAssetProfileForm(); + + expect(dataService.postBenchmark).not.toHaveBeenCalled(); + expect(dataService.deleteBenchmark).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/client/src/app/components/admin-market-data/asset-profile-dialog/asset-profile-dialog.component.ts b/apps/client/src/app/components/admin-market-data/asset-profile-dialog/asset-profile-dialog.component.ts index 6e2dab956..b61edf459 100644 --- a/apps/client/src/app/components/admin-market-data/asset-profile-dialog/asset-profile-dialog.component.ts +++ b/apps/client/src/app/components/admin-market-data/asset-profile-dialog/asset-profile-dialog.component.ts @@ -88,8 +88,8 @@ import { serverOutline } from 'ionicons/icons'; import ms from 'ms'; -import { EMPTY } from 'rxjs'; -import { catchError } from 'rxjs/operators'; +import { EMPTY, of } from 'rxjs'; +import { catchError, switchMap, tap } from 'rxjs/operators'; import { AssetProfileDialogParams } from './interfaces/interfaces'; @@ -155,6 +155,7 @@ export class GfAssetProfileDialogComponent implements OnInit { csvString: '' }), isActive: [true], + isBenchmark: [false], name: ['', Validators.required], scraperConfiguration: this.formBuilder.group({ defaultMarketPrice: null, @@ -382,6 +383,7 @@ export class GfAssetProfileDialogComponent implements OnInit { csvString: GfAssetProfileDialogComponent.HISTORICAL_DATA_TEMPLATE }, isActive: this.assetProfile?.isActive, + isBenchmark: this.isBenchmark, name: this.assetProfile.name ?? this.assetProfile.symbol, scraperConfiguration: { defaultMarketPrice: @@ -459,19 +461,6 @@ export class GfAssetProfileDialogComponent implements OnInit { } } - public onSetBenchmark({ dataSource, symbol }: AssetProfileIdentifier) { - this.dataService - .postBenchmark({ dataSource, symbol }) - .pipe(takeUntilDestroyed(this.destroyRef)) - .subscribe(() => { - this.dataService.updateInfo(); - - this.isBenchmark = true; - - this.changeDetectorRef.markForCheck(); - }); - } - public onSetEditAssetProfileIdentifierMode() { this.isEditAssetProfileIdentifierMode = true; @@ -559,6 +548,7 @@ export class GfAssetProfileDialogComponent implements OnInit { scraperConfiguration as unknown as Prisma.InputJsonObject, url: this.assetProfileForm.get('url').value || null }; + const isBenchmark = !!this.assetProfileForm.get('isBenchmark').value; try { await validateObjectForForm({ @@ -588,6 +578,15 @@ export class GfAssetProfileDialogComponent implements OnInit { }, assetProfile ) + .pipe( + switchMap(() => { + if (isBenchmark === this.isBenchmark) { + return of(undefined); + } + + return this.updateBenchmark(isBenchmark); + }) + ) .subscribe({ next: () => { this.snackBar.open( @@ -742,19 +741,6 @@ export class GfAssetProfileDialogComponent implements OnInit { } } - public onUnsetBenchmark({ dataSource, symbol }: AssetProfileIdentifier) { - this.dataService - .deleteBenchmark({ dataSource, symbol }) - .pipe(takeUntilDestroyed(this.destroyRef)) - .subscribe(() => { - this.dataService.updateInfo(); - - this.isBenchmark = false; - - this.changeDetectorRef.markForCheck(); - }); - } - public onTriggerSubmitAssetProfileForm() { if (this.assetProfileForm.valid) { this.onSubmitAssetProfileForm(); @@ -774,4 +760,37 @@ export class GfAssetProfileDialogComponent implements OnInit { }; } } + + private updateBenchmark(isBenchmark: boolean) { + const benchmark$ = isBenchmark + ? this.dataService.postBenchmark({ + dataSource: this.data.dataSource, + symbol: this.data.symbol + }) + : this.dataService.deleteBenchmark({ + dataSource: this.data.dataSource, + symbol: this.data.symbol + }); + + return benchmark$.pipe( + tap(() => { + this.dataService.updateInfo(); + + this.isBenchmark = isBenchmark; + + if (this.assetProfile?.id) { + this.benchmarks = isBenchmark + ? [ + ...this.benchmarks.filter(({ id }) => { + return id !== this.assetProfile.id; + }), + { id: this.assetProfile.id } + ] + : this.benchmarks.filter(({ id }) => { + return id !== this.assetProfile.id; + }); + } + }) + ); + } } diff --git a/apps/client/src/app/components/admin-market-data/asset-profile-dialog/asset-profile-dialog.html b/apps/client/src/app/components/admin-market-data/asset-profile-dialog/asset-profile-dialog.html index 9ae7f8064..89edaf2fa 100644 --- a/apps/client/src/app/components/admin-market-data/asset-profile-dialog/asset-profile-dialog.html +++ b/apps/client/src/app/components/admin-market-data/asset-profile-dialog/asset-profile-dialog.html @@ -358,21 +358,10 @@
Include in