From 125292a5f22e34f689800a823432b12a015c91c1 Mon Sep 17 00:00:00 2001 From: Thomas Kaul <4159106+dtslvr@users.noreply.github.com> Date: Wed, 26 Aug 2026 16:49:11 +0200 Subject: [PATCH] Fix creation of asset profiles with symbol in wrong letter case by using original symbol --- .../app/activities/activities.controller.ts | 2 +- apps/api/src/app/import/import.service.ts | 5 +- .../data-provider.service.spec.ts | 97 +++++++++++++++++++ .../data-provider/data-provider.service.ts | 8 +- .../data-gathering.service.spec.ts | 59 +++++++++-- .../data-gathering/data-gathering.service.ts | 4 +- .../admin-market-data.component.ts | 11 ++- libs/common/src/lib/helper.spec.ts | 33 +++++++ libs/common/src/lib/helper.ts | 17 ++++ libs/ui/src/lib/services/admin.service.ts | 9 +- 10 files changed, 228 insertions(+), 17 deletions(-) create mode 100644 apps/api/src/services/data-provider/data-provider.service.spec.ts diff --git a/apps/api/src/app/activities/activities.controller.ts b/apps/api/src/app/activities/activities.controller.ts index 3ea0f8a99..5d0d932ee 100644 --- a/apps/api/src/app/activities/activities.controller.ts +++ b/apps/api/src/app/activities/activities.controller.ts @@ -227,7 +227,7 @@ export class ActivitiesController { let assetProfiles: { [assetProfileIdentifier: string]: Partial; - }; + } = {}; try { assetProfiles = await this.dataProviderService.validateActivities({ diff --git a/apps/api/src/app/import/import.service.ts b/apps/api/src/app/import/import.service.ts index 313325f04..0a0a9550b 100644 --- a/apps/api/src/app/import/import.service.ts +++ b/apps/api/src/app/import/import.service.ts @@ -847,14 +847,17 @@ export class ImportService { name, scraperConfiguration, sectors, - symbol, symbolMapping, url, updatedAt } = assetProfile; + + const symbol = activity.assetProfile.symbol; + const validatedAccount = accounts.find(({ id }) => { return id === accountId; }); + const validatedTags = tags.filter(({ id: tagId }) => { return tagIds.some((activityTagId) => { return activityTagId === tagId; diff --git a/apps/api/src/services/data-provider/data-provider.service.spec.ts b/apps/api/src/services/data-provider/data-provider.service.spec.ts new file mode 100644 index 000000000..9bcb0ff13 --- /dev/null +++ b/apps/api/src/services/data-provider/data-provider.service.spec.ts @@ -0,0 +1,97 @@ +import { getAssetProfileIdentifier } from '@ghostfolio/common/helper'; + +import { DataSource } from '@prisma/client'; + +import { DataProviderService } from './data-provider.service'; + +describe('DataProviderService', () => { + let dataProviderService: DataProviderService; + let getAssetProfile: jest.Mock; + + beforeEach(() => { + getAssetProfile = jest.fn(); + + const dataProviderInterface = { + getAssetProfile, + getName: () => { + return DataSource.YAHOO; + } + }; + + dataProviderService = new DataProviderService( + null, + [dataProviderInterface] as any, + null, + null, + null, + null + ); + }); + + describe('getAssetProfiles', () => { + it('Corrects the letter case of the symbol', async () => { + getAssetProfile.mockResolvedValue({ + currency: 'USD', + dataSource: DataSource.YAHOO, + name: 'Apple Inc.', + symbol: 'AAPL' + }); + + const assetProfiles = await dataProviderService.getAssetProfiles([ + { dataSource: DataSource.YAHOO, symbol: 'aapl' } + ]); + + expect( + assetProfiles[ + getAssetProfileIdentifier({ + dataSource: DataSource.YAHOO, + symbol: 'aapl' + }) + ].symbol + ).toEqual('AAPL'); + }); + + it('Keeps the requested symbol if the data provider resolves it to a different symbol', async () => { + getAssetProfile.mockResolvedValue({ + currency: 'USD', + dataSource: DataSource.YAHOO, + name: 'Meta Platforms, Inc.', + symbol: 'META' + }); + + const assetProfiles = await dataProviderService.getAssetProfiles([ + { dataSource: DataSource.YAHOO, symbol: 'FB' } + ]); + + expect( + assetProfiles[ + getAssetProfileIdentifier({ + dataSource: DataSource.YAHOO, + symbol: 'FB' + }) + ].symbol + ).toEqual('FB'); + }); + + it('Keeps the requested symbol if the data provider reports no symbol', async () => { + getAssetProfile.mockResolvedValue({ + currency: 'USD', + dataSource: DataSource.YAHOO, + name: 'Apple Inc.' + }); + + const assetProfiles = await dataProviderService.getAssetProfiles([ + { dataSource: DataSource.YAHOO, symbol: 'aapl' } + ]); + + expect( + assetProfiles[ + getAssetProfileIdentifier({ + dataSource: DataSource.YAHOO, + symbol: 'aapl' + }) + ].symbol + ).toEqual('aapl'); + }); + }); +}); diff --git a/apps/api/src/services/data-provider/data-provider.service.ts b/apps/api/src/services/data-provider/data-provider.service.ts index 15ec0b150..3a62a480a 100644 --- a/apps/api/src/services/data-provider/data-provider.service.ts +++ b/apps/api/src/services/data-provider/data-provider.service.ts @@ -23,6 +23,7 @@ import { getStartOfUtcDate, isCurrency, isDerivedCurrency, + isSameSymbol, isValidCustomAssetProfileSymbol, isValidSearchQuery } from '@ghostfolio/common/helper'; @@ -131,7 +132,12 @@ export class DataProviderService implements OnModuleInit { ] = { ...assetProfile, name: formatAssetProfileName(assetProfile), - symbol: assetProfile.symbol ?? symbol + symbol: isSameSymbol({ + symbol1: symbol, + symbol2: assetProfile.symbol + }) + ? assetProfile.symbol + : symbol }; } }) diff --git a/apps/api/src/services/queues/data-gathering/data-gathering.service.spec.ts b/apps/api/src/services/queues/data-gathering/data-gathering.service.spec.ts index 4fc80d3d6..270acbcff 100644 --- a/apps/api/src/services/queues/data-gathering/data-gathering.service.spec.ts +++ b/apps/api/src/services/queues/data-gathering/data-gathering.service.spec.ts @@ -4,36 +4,55 @@ import { } from '@ghostfolio/common/config'; import { parseDate } from '@ghostfolio/common/helper'; +import { DataSource } from '@prisma/client'; + import { DataGatheringService } from './data-gathering.service'; describe('DataGatheringService', () => { let dataGatheringQueue: { addBulk: jest.Mock; clean: jest.Mock }; let dataGatheringService: DataGatheringService; - let dataProviderService: { getHistoricalRaw: jest.Mock }; - let prismaService: { marketData: { groupBy: jest.Mock; upsert: jest.Mock } }; + + let dataProviderService: { + getAssetProfiles: jest.Mock; + getHistoricalRaw: jest.Mock; + }; + + let prismaService: { + marketData: { groupBy: jest.Mock; upsert: jest.Mock }; + symbolProfile: { upsert: jest.Mock }; + }; + + let symbolProfileService: { getSymbolProfiles: jest.Mock }; beforeEach(() => { dataGatheringQueue = { addBulk: jest.fn().mockResolvedValue([]), clean: jest.fn().mockResolvedValue([]) }; - dataProviderService = { getHistoricalRaw: jest.fn() }; + dataProviderService = { + getAssetProfiles: jest.fn().mockResolvedValue({}), + getHistoricalRaw: jest.fn() + }; prismaService = { marketData: { groupBy: jest.fn().mockResolvedValue([]), upsert: jest.fn().mockResolvedValue({}) - } + }, + symbolProfile: { upsert: jest.fn().mockResolvedValue({}) } + }; + symbolProfileService = { + getSymbolProfiles: jest.fn().mockResolvedValue([]) }; dataGatheringService = new DataGatheringService( - null, + [], dataGatheringQueue as any, dataProviderService as any, null, null, prismaService as any, null, - null + symbolProfileService as any ); }); @@ -110,6 +129,34 @@ describe('DataGatheringService', () => { }); }); + describe('gatherAssetProfiles', () => { + it('Keeps the requested symbol, so that no duplicate asset profile is created', async () => { + dataProviderService.getAssetProfiles.mockResolvedValue({ + 'YAHOO-aapl': { + currency: 'USD', + dataSource: DataSource.YAHOO, + name: 'Apple Inc.', + symbol: 'AAPL' + } + }); + + await dataGatheringService.gatherAssetProfiles([ + { dataSource: DataSource.YAHOO, symbol: 'aapl' } + ]); + + expect(prismaService.symbolProfile.upsert).toHaveBeenCalledWith( + expect.objectContaining({ + where: { + dataSource_symbol: { + dataSource: DataSource.YAHOO, + symbol: 'aapl' + } + } + }) + ); + }); + }); + describe('gatherRecentMarketData', () => { it('queries the asset profiles with recent market data once and reuses them', async () => { const assetProfileIdentifiersWithRecentMarketData = [ diff --git a/apps/api/src/services/queues/data-gathering/data-gathering.service.ts b/apps/api/src/services/queues/data-gathering/data-gathering.service.ts index 691d3a0e1..fc7efbdec 100644 --- a/apps/api/src/services/queues/data-gathering/data-gathering.service.ts +++ b/apps/api/src/services/queues/data-gathering/data-gathering.service.ts @@ -128,9 +128,7 @@ export class DataGatheringService { }); } catch (error) { this.logger.error( - `Failed to enhance data for ${symbol} (${ - assetProfile.dataSource - }) by ${dataEnhancer.getName()}`, + `Failed to enhance data for ${symbol} (${dataSource}) by ${dataEnhancer.getName()}`, error ); } diff --git a/apps/client/src/app/components/admin-market-data/admin-market-data.component.ts b/apps/client/src/app/components/admin-market-data/admin-market-data.component.ts index 87389f957..9c3c0c791 100644 --- a/apps/client/src/app/components/admin-market-data/admin-market-data.component.ts +++ b/apps/client/src/app/components/admin-market-data/admin-market-data.component.ts @@ -496,14 +496,19 @@ export class GfAdminMarketDataComponent implements AfterViewInit, OnInit { this.adminService .addAssetProfile({ dataSource, symbol }) .pipe(takeUntilDestroyed(this.destroyRef)) - .subscribe(() => { + .subscribe((assetProfile) => { this.loadData(); + + this.onOpenAssetProfileDialog({ + dataSource, + symbol: assetProfile?.symbol ?? symbol + }); }); } else { this.loadData(); - } - this.onOpenAssetProfileDialog({ dataSource, symbol }); + this.onOpenAssetProfileDialog({ dataSource, symbol }); + } }); }); } diff --git a/libs/common/src/lib/helper.spec.ts b/libs/common/src/lib/helper.spec.ts index 9fd4e8f38..a69c5eaf9 100644 --- a/libs/common/src/lib/helper.spec.ts +++ b/libs/common/src/lib/helper.spec.ts @@ -11,6 +11,7 @@ import { isAccountExcluded, isCurrency, isCurrencySymbol, + isSameSymbol, isSplitRatio, isValidCustomAssetProfileSymbol, isValidGranteeOfAccess, @@ -311,6 +312,38 @@ describe('Helper', () => { }); }); + describe('Is same symbol', () => { + it('Same symbol', () => { + expect(isSameSymbol({ symbol1: 'AAPL', symbol2: 'AAPL' })).toEqual(true); + }); + + it('Same symbol in a different letter case', () => { + expect(isSameSymbol({ symbol1: 'aapl', symbol2: 'AAPL' })).toEqual(true); + expect(isSameSymbol({ symbol1: 'AaPl', symbol2: 'AAPL' })).toEqual(true); + expect( + isSameSymbol({ symbol1: 'usd-coin', symbol2: 'USD-Coin' }) + ).toEqual(true); + }); + + it('Different symbol', () => { + expect(isSameSymbol({ symbol1: 'FB', symbol2: 'META' })).toEqual(false); + expect( + isSameSymbol({ symbol1: 'US0378331005', symbol2: 'AAPL' }) + ).toEqual(false); + expect(isSameSymbol({ symbol1: 'BRK.B', symbol2: 'BRK-B' })).toEqual( + false + ); + }); + + it('Missing symbol', () => { + expect(isSameSymbol({ symbol1: undefined, symbol2: 'AAPL' })).toEqual( + false + ); + expect(isSameSymbol({ symbol1: 'AAPL', symbol2: null })).toEqual(false); + expect(isSameSymbol({ symbol1: '', symbol2: '' })).toEqual(false); + }); + }); + describe('Is split ratio', () => { it('Forward split', () => { expect(isSplitRatio({ denominator: 1, numerator: 2 })).toEqual(true); diff --git a/libs/common/src/lib/helper.ts b/libs/common/src/lib/helper.ts index a51a74808..b2d4c5295 100644 --- a/libs/common/src/lib/helper.ts +++ b/libs/common/src/lib/helper.ts @@ -606,6 +606,23 @@ export function isRootCurrency(aCurrency: string) { }); } +/** + * Checks whether two symbols are the same, ignoring the letter case. Data + * providers can report a symbol in a different letter case than requested, for + * example "AAPL" for "aapl". + */ +export function isSameSymbol({ + symbol1, + symbol2 +}: { + symbol1: string; + symbol2: string; +}) { + return ( + !!symbol1 && !!symbol2 && symbol1.toLowerCase() === symbol2.toLowerCase() + ); +} + /** * Validates the ratio of a stock split, expressed as the number of shares held * after the split (numerator) per number of shares held before (denominator), diff --git a/libs/ui/src/lib/services/admin.service.ts b/libs/ui/src/lib/services/admin.service.ts index aa048faaf..df200a521 100644 --- a/libs/ui/src/lib/services/admin.service.ts +++ b/libs/ui/src/lib/services/admin.service.ts @@ -25,7 +25,12 @@ import { GF_ENVIRONMENT } from '@ghostfolio/ui/environment'; import { HttpClient, HttpHeaders, HttpParams } from '@angular/common/http'; import { inject, Service } from '@angular/core'; -import { AssetProfileSplit, MarketData, Platform } from '@prisma/client'; +import { + AssetProfileSplit, + MarketData, + Platform, + SymbolProfile +} from '@prisma/client'; import { JobStatus } from 'bull'; import { isNumber } from 'lodash'; @@ -35,7 +40,7 @@ export class AdminService { private readonly http = inject(HttpClient); public addAssetProfile({ dataSource, symbol }: AssetProfileIdentifier) { - return this.http.post( + return this.http.post( `/api/v1/admin/profile-data/${dataSource}/${encodeURIComponent(symbol)}`, null );