From 21f9bcfcc9e2c56d805f9398a3b7f8a7785f613f Mon Sep 17 00:00:00 2001 From: Varun Jain Date: Sun, 2 Aug 2026 13:09:54 +0530 Subject: [PATCH 1/2] Fix duplicate asset profile creation for repeated new manual symbols on import existingAssetProfiles was snapshotted once before the asset-profile creation loop and never refreshed, so two entries in the same import payload for the same new symbol both saw "no existing profile" and both called symbolProfileService.add(), causing a unique constraint violation on the second insert. Track identifiers already created during the loop and skip the redundant create. Closes #7459 --- CHANGELOG.md | 1 + .../api/src/app/import/import.service.spec.ts | 232 ++++++++++++++++++ apps/api/src/app/import/import.service.ts | 18 +- 3 files changed, 249 insertions(+), 2 deletions(-) create mode 100644 apps/api/src/app/import/import.service.spec.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 6c90fdf30..f00f06609 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Fixed the activity import so it no longer fails when multiple rows create the same new manual asset profile - Fixed the handling of the _Exclude from Analysis_ tag in the activities table - Fixed the persistence of an empty comment in the create or update account dialog - Resolved a validation error caused by empty strings in the asset profile details dialog of the admin control panel diff --git a/apps/api/src/app/import/import.service.spec.ts b/apps/api/src/app/import/import.service.spec.ts new file mode 100644 index 000000000..1d939c582 --- /dev/null +++ b/apps/api/src/app/import/import.service.spec.ts @@ -0,0 +1,232 @@ +import { AccountService } from '@ghostfolio/api/app/account/account.service'; +import { ActivitiesService } from '@ghostfolio/api/app/activities/activities.service'; +import { DataProviderService } from '@ghostfolio/api/services/data-provider/data-provider.service'; +import { ExchangeRateDataService } from '@ghostfolio/api/services/exchange-rate-data/exchange-rate-data.service'; +import { MarketDataService } from '@ghostfolio/api/services/market-data/market-data.service'; +import { DataGatheringService } from '@ghostfolio/api/services/queues/data-gathering/data-gathering.service'; +import { SymbolProfileService } from '@ghostfolio/api/services/symbol-profile/symbol-profile.service'; +import { TagService } from '@ghostfolio/api/services/tag/tag.service'; +import { UserWithSettings } from '@ghostfolio/common/types'; + +import { DataSource } from '@prisma/client'; + +import { ImportService } from './import.service'; + +jest.mock('@ghostfolio/api/app/account/account.service', () => { + return { + AccountService: jest.fn().mockImplementation(() => { + return { + getAccounts: () => Promise.resolve([]) + }; + }) + }; +}); + +jest.mock('@ghostfolio/api/app/activities/activities.service', () => { + return { + ActivitiesService: jest.fn().mockImplementation(() => { + return { + getActivities: () => Promise.resolve({ activities: [] }), + createActivity: () => { + return Promise.resolve({ + id: 'ee3949fa-9df5-4b4e-9856-14dd1cfe9c86', + SymbolProfile: { symbol: 'Repeated Fee' } + }); + } + }; + }) + }; +}); + +jest.mock( + '@ghostfolio/api/services/data-provider/data-provider.service', + () => { + return { + DataProviderService: jest.fn().mockImplementation(() => { + return { + getDataSourceForImport: () => DataSource.MANUAL, + validateActivities: () => Promise.resolve({}) + }; + }) + }; + } +); + +jest.mock( + '@ghostfolio/api/services/exchange-rate-data/exchange-rate-data.service', + () => { + return { + ExchangeRateDataService: jest.fn().mockImplementation(() => { + return { + toCurrencyAtDate: () => Promise.resolve(0) + }; + }) + }; + } +); + +jest.mock('@ghostfolio/api/services/market-data/market-data.service', () => { + return { + MarketDataService: jest.fn().mockImplementation(() => { + return { + updateMany: () => Promise.resolve() + }; + }) + }; +}); + +jest.mock( + '@ghostfolio/api/services/queues/data-gathering/data-gathering.service', + () => { + return { + DataGatheringService: jest.fn().mockImplementation(() => { + return { + gatherSymbols: () => undefined + }; + }) + }; + } +); + +jest.mock( + '@ghostfolio/api/services/symbol-profile/symbol-profile.service', + () => { + return { + SymbolProfileService: jest.fn().mockImplementation(() => { + return { + add: jest.fn().mockResolvedValue(undefined), + getSymbolProfiles: () => Promise.resolve([]) + }; + }) + }; + } +); + +jest.mock('@ghostfolio/api/services/tag/tag.service', () => { + return { + TagService: jest.fn().mockImplementation(() => { + return { + getTagsForUser: () => Promise.resolve([]) + }; + }) + }; +}); + +describe('ImportService', () => { + let accountService: AccountService; + let activitiesService: ActivitiesService; + let dataGatheringService: DataGatheringService; + let dataProviderService: DataProviderService; + let exchangeRateDataService: ExchangeRateDataService; + let importService: ImportService; + let marketDataService: MarketDataService; + let symbolProfileService: SymbolProfileService; + let tagService: TagService; + + beforeEach(() => { + accountService = new AccountService(null, null, null, null, null); + activitiesService = new ActivitiesService( + null, + null, + null, + null, + null, + null, + null, + null, + null, + null, + null + ); + dataGatheringService = new DataGatheringService( + null, + null, + null, + null, + null, + null, + null, + null + ); + dataProviderService = new DataProviderService( + null, + [], + null, + null, + null, + null + ); + exchangeRateDataService = new ExchangeRateDataService( + null, + null, + null, + null + ); + marketDataService = new MarketDataService(null); + symbolProfileService = new SymbolProfileService(null); + tagService = new TagService(null); + + importService = new ImportService( + accountService, + activitiesService, + null, + dataGatheringService, + dataProviderService, + exchangeRateDataService, + marketDataService, + null, + null, + symbolProfileService, + tagService + ); + }); + + it('creates only one asset profile when the import payload contains duplicate new manual asset profiles', async () => { + const user = { + id: 'da8a5786-1223-4a51-9a86-2b60433c9f3f', + permissions: [], + settings: { settings: { baseCurrency: 'USD' } } + } as unknown as UserWithSettings; + + const assetProfile = { + currency: 'USD', + dataSource: DataSource.MANUAL, + isActive: true, + marketData: [], + name: 'Repeated Fee', + symbol: 'Repeated Fee' + }; + + await importService.import({ + accountsWithBalancesDto: [], + activitiesDto: [ + { + currency: 'USD', + dataSource: DataSource.MANUAL, + date: '2024-01-01T00:00:00.000Z', + fee: 1, + quantity: 0, + symbol: 'Repeated Fee', + type: 'FEE', + unitPrice: 0 + }, + { + currency: 'USD', + dataSource: DataSource.MANUAL, + date: '2024-01-02T00:00:00.000Z', + fee: 1, + quantity: 0, + symbol: 'Repeated Fee', + type: 'FEE', + unitPrice: 0 + } + ], + assetProfilesWithMarketDataDto: [assetProfile, assetProfile], + maxActivitiesToImport: 10, + tagsDto: [], + user + }); + + expect(symbolProfileService.add).toHaveBeenCalledTimes(1); + }); +}); diff --git a/apps/api/src/app/import/import.service.ts b/apps/api/src/app/import/import.service.ts index 52b0662d6..961031712 100644 --- a/apps/api/src/app/import/import.service.ts +++ b/apps/api/src/app/import/import.service.ts @@ -392,7 +392,14 @@ export class ImportService { }) ); + const createdAssetProfileIdentifiers = new Set(); + for (const assetProfileWithMarketData of assetProfilesWithMarketDataDto) { + const assetProfileIdentifier = getAssetProfileIdentifier({ + dataSource: assetProfileWithMarketData.dataSource, + symbol: assetProfileWithMarketData.symbol + }); + // Check if there is any existing asset profile const existingAssetProfile = existingAssetProfiles.find( ({ dataSource, symbol }) => { @@ -403,8 +410,14 @@ export class ImportService { } ); - // If there is no asset profile or if the asset profile belongs to a different user, then create a new asset profile - if (!existingAssetProfile || existingAssetProfile.userId !== user.id) { + // If there is no asset profile or if the asset profile belongs to a + // different user, then create a new asset profile, unless it has + // already been created earlier in this loop (e.g. multiple imported + // activities referencing the same new manual asset profile) + if ( + (!existingAssetProfile || existingAssetProfile.userId !== user.id) && + !createdAssetProfileIdentifiers.has(assetProfileIdentifier) + ) { const assetProfile: CreateAssetProfileDto = omit( assetProfileWithMarketData, 'marketData' @@ -424,6 +437,7 @@ export class ImportService { }; await this.symbolProfileService.add(assetProfileObject); + createdAssetProfileIdentifiers.add(assetProfileIdentifier); } // Insert or update market data From d00690b2c043cc532d7ed52de4e1b303c71ad0e0 Mon Sep 17 00:00:00 2001 From: Varun Jain Date: Tue, 4 Aug 2026 10:24:53 +0530 Subject: [PATCH 2/2] Reuse existing shared manual asset profile on activity import An asset profile created via the admin control panel has no user (admin.service.ts addAssetProfile), so the ownership check in ImportService.import() treated it as belonging to a different user and cloned it under a random UUID. Activities were attached to the clone while the original asset profile stayed empty. Only clone an existing asset profile when it belongs to a different user, which aligns the asset profile loop with the activity creation that connects on dataSource and symbol without an ownership check. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 8 +- .../api/src/app/import/import.service.spec.ts | 114 +++++++++++++++++- apps/api/src/app/import/import.service.ts | 8 +- 3 files changed, 122 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f00f06609..d9d0b207f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,13 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## Unreleased + +### Fixed + +- Fixed the activity import so it no longer fails when multiple rows create the same new manual asset profile +- Fixed the activity import to reuse an existing manual asset profile of the admin control panel instead of duplicating it + ## 3.41.0 - 2026-08-03 ### Added @@ -36,7 +43,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed -- Fixed the activity import so it no longer fails when multiple rows create the same new manual asset profile - Fixed the handling of the _Exclude from Analysis_ tag in the activities table - Fixed the persistence of an empty comment in the create or update account dialog - Resolved a validation error caused by empty strings in the asset profile details dialog of the admin control panel diff --git a/apps/api/src/app/import/import.service.spec.ts b/apps/api/src/app/import/import.service.spec.ts index 1d939c582..5bf077045 100644 --- a/apps/api/src/app/import/import.service.spec.ts +++ b/apps/api/src/app/import/import.service.spec.ts @@ -8,10 +8,12 @@ import { SymbolProfileService } from '@ghostfolio/api/services/symbol-profile/sy import { TagService } from '@ghostfolio/api/services/tag/tag.service'; import { UserWithSettings } from '@ghostfolio/common/types'; -import { DataSource } from '@prisma/client'; +import { DataSource, SymbolProfile } from '@prisma/client'; import { ImportService } from './import.service'; +let mockExistingAssetProfiles: Partial[] = []; + jest.mock('@ghostfolio/api/app/account/account.service', () => { return { AccountService: jest.fn().mockImplementation(() => { @@ -27,12 +29,14 @@ jest.mock('@ghostfolio/api/app/activities/activities.service', () => { ActivitiesService: jest.fn().mockImplementation(() => { return { getActivities: () => Promise.resolve({ activities: [] }), - createActivity: () => { + createActivity: jest.fn().mockImplementation((data) => { return Promise.resolve({ id: 'ee3949fa-9df5-4b4e-9856-14dd1cfe9c86', - SymbolProfile: { symbol: 'Repeated Fee' } + SymbolProfile: { + symbol: data.SymbolProfile.connectOrCreate.create.symbol + } }); - } + }) }; }) }; @@ -95,7 +99,7 @@ jest.mock( SymbolProfileService: jest.fn().mockImplementation(() => { return { add: jest.fn().mockResolvedValue(undefined), - getSymbolProfiles: () => Promise.resolve([]) + getSymbolProfiles: () => Promise.resolve(mockExistingAssetProfiles) }; }) }; @@ -124,6 +128,8 @@ describe('ImportService', () => { let tagService: TagService; beforeEach(() => { + mockExistingAssetProfiles = []; + accountService = new AccountService(null, null, null, null, null); activitiesService = new ActivitiesService( null, @@ -223,10 +229,108 @@ describe('ImportService', () => { ], assetProfilesWithMarketDataDto: [assetProfile, assetProfile], maxActivitiesToImport: 10, + platformsDto: [], tagsDto: [], user }); expect(symbolProfileService.add).toHaveBeenCalledTimes(1); }); + + it('reuses an existing manual asset profile without a user', async () => { + mockExistingAssetProfiles = [ + { + currency: 'USD', + dataSource: DataSource.MANUAL, + name: 'Manual Asset Profile', + symbol: 'GF_MANUAL', + userId: null + } + ]; + + await importActivitiesWithExistingAssetProfile(); + + expect(symbolProfileService.add).not.toHaveBeenCalled(); + + for (const [{ SymbolProfile }] of ( + activitiesService.createActivity as jest.Mock + ).mock.calls) { + expect(SymbolProfile.connectOrCreate.create.symbol).toEqual('GF_MANUAL'); + } + }); + + it('creates a new asset profile when the existing manual asset profile belongs to a different user', async () => { + mockExistingAssetProfiles = [ + { + currency: 'USD', + dataSource: DataSource.MANUAL, + name: 'Manual Asset Profile', + symbol: 'GF_MANUAL', + userId: '5b7a1b3a-1f1c-4c7a-9a1a-3a1b5b7a1b3a' + } + ]; + + await importActivitiesWithExistingAssetProfile(); + + expect(symbolProfileService.add).toHaveBeenCalledTimes(1); + + const [[{ symbol }]] = (symbolProfileService.add as jest.Mock).mock.calls; + + expect(symbol).not.toEqual('GF_MANUAL'); + + for (const [{ SymbolProfile }] of ( + activitiesService.createActivity as jest.Mock + ).mock.calls) { + expect(SymbolProfile.connectOrCreate.create.symbol).toEqual(symbol); + } + }); + + function importActivitiesWithExistingAssetProfile() { + const user = { + id: 'da8a5786-1223-4a51-9a86-2b60433c9f3f', + permissions: [], + settings: { settings: { baseCurrency: 'USD' } } + } as unknown as UserWithSettings; + + // The client creates a synthetic asset profile per activity + const assetProfile = { + currency: 'USD', + dataSource: DataSource.MANUAL, + isActive: true, + marketData: [], + name: 'GF_MANUAL', + symbol: 'GF_MANUAL' + }; + + return importService.import({ + accountsWithBalancesDto: [], + activitiesDto: [ + { + currency: 'USD', + dataSource: DataSource.MANUAL, + date: '2024-01-01T00:00:00.000Z', + fee: 0, + quantity: 1, + symbol: 'GF_MANUAL', + type: 'BUY', + unitPrice: 1 + }, + { + currency: 'USD', + dataSource: DataSource.MANUAL, + date: '2024-01-02T00:00:00.000Z', + fee: 0, + quantity: 2, + symbol: 'GF_MANUAL', + type: 'BUY', + unitPrice: 1 + } + ], + assetProfilesWithMarketDataDto: [assetProfile, assetProfile], + maxActivitiesToImport: 10, + platformsDto: [], + tagsDto: [], + user + }); + } }); diff --git a/apps/api/src/app/import/import.service.ts b/apps/api/src/app/import/import.service.ts index 961031712..58cbd1bf7 100644 --- a/apps/api/src/app/import/import.service.ts +++ b/apps/api/src/app/import/import.service.ts @@ -413,9 +413,13 @@ export class ImportService { // If there is no asset profile or if the asset profile belongs to a // different user, then create a new asset profile, unless it has // already been created earlier in this loop (e.g. multiple imported - // activities referencing the same new manual asset profile) + // activities referencing the same new manual asset profile). An asset + // profile without a user is shared, for example created via the admin + // control panel, and is reused as is if ( - (!existingAssetProfile || existingAssetProfile.userId !== user.id) && + (!existingAssetProfile || + (existingAssetProfile.userId && + existingAssetProfile.userId !== user.id)) && !createdAssetProfileIdentifiers.has(assetProfileIdentifier) ) { const assetProfile: CreateAssetProfileDto = omit(