From 21f9bcfcc9e2c56d805f9398a3b7f8a7785f613f Mon Sep 17 00:00:00 2001 From: Varun Jain Date: Sun, 2 Aug 2026 13:09:54 +0530 Subject: [PATCH] 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