From 98d4a4901e505d2b026f63808e55e53d17bbf0e5 Mon Sep 17 00:00:00 2001 From: Prakhar Mishra Date: Tue, 20 Oct 2026 01:24:08 +0530 Subject: [PATCH] fix(api): reuse global manual asset profiles during import Admin-created manual asset profiles are global and have userId null. Include them in custom profile lookups and treat them as reusable during import to avoid duplicate SymbolProfile rows. Prefer the user's own profile over a global profile with the same name and never reuse another user's private profile. Fixes #5193 --- .../api/src/app/import/import.service.spec.ts | 206 +++++++++++++++++- apps/api/src/app/import/import.service.ts | 39 +++- .../symbol-profile.service.spec.ts | 49 +++++ .../symbol-profile/symbol-profile.service.ts | 10 +- 4 files changed, 286 insertions(+), 18 deletions(-) create mode 100644 apps/api/src/services/symbol-profile/symbol-profile.service.spec.ts diff --git a/apps/api/src/app/import/import.service.spec.ts b/apps/api/src/app/import/import.service.spec.ts index d6d41134a5..987c07676e 100644 --- a/apps/api/src/app/import/import.service.spec.ts +++ b/apps/api/src/app/import/import.service.spec.ts @@ -33,6 +33,8 @@ describe('ImportService', () => { let addSymbolProfile: jest.Mock; let createActivity: jest.Mock; let gatherSymbols: jest.Mock; + let getCustomSymbolProfilesByNames: jest.Mock; + let getSymbolProfiles: jest.Mock; let importService: ImportService; let updateManyMarketData: jest.Mock; @@ -46,6 +48,8 @@ describe('ImportService', () => { addSymbolProfile = jest.fn(); createActivity = jest.fn(); gatherSymbols = jest.fn(); + getCustomSymbolProfilesByNames = jest.fn().mockResolvedValue([]); + getSymbolProfiles = jest.fn().mockResolvedValue([]); updateManyMarketData = jest.fn(); const validateActivities = jest.fn( @@ -94,8 +98,8 @@ describe('ImportService', () => { null, { add: addSymbolProfile, - getCustomSymbolProfilesByNames: jest.fn().mockResolvedValue([]), - getSymbolProfiles: jest.fn().mockResolvedValue([]) + getCustomSymbolProfilesByNames, + getSymbolProfiles } as unknown as SymbolProfileService, { getTagsForUser: jest.fn().mockResolvedValue([]) @@ -481,6 +485,204 @@ describe('ImportService', () => { ]); }); + it('reuses a global custom asset profile by symbol', async () => { + getSymbolProfiles.mockResolvedValue([ + { + dataSource: DataSource.MANUAL, + name: 'Global X Copper Miners ETF', + symbol: 'GF_COPX', + userId: null + } + ]); + mockCreatedAssetProfiles([ + { dataSource: DataSource.MANUAL, symbol: 'GF_COPX' } + ]); + + await importActivities({ + activitiesDto: [ + createActivityDto({ + dataSource: DataSource.MANUAL, + date: '2024-01-01T00:00:00.000Z', + symbol: 'GF_COPX', + type: 'BUY' + }) + ], + assetProfilesWithMarketDataDto: [ + { + currency: 'USD', + dataSource: DataSource.MANUAL, + marketData: [], + name: 'Global X Copper Miners ETF', + symbol: 'GF_COPX' + } + ] + }); + + expect(addSymbolProfile).not.toHaveBeenCalled(); + + expect( + createActivity.mock.calls[0][0].SymbolProfile.connectOrCreate.where + ).toEqual({ + dataSource_symbol: { + dataSource: DataSource.MANUAL, + symbol: 'GF_COPX' + } + }); + }); + + it('reuses a global custom asset profile by name', async () => { + getCustomSymbolProfilesByNames.mockResolvedValue([ + { + name: 'Global X Copper Miners ETF', + symbol: 'GF_COPX', + userId: null + } + ]); + mockCreatedAssetProfiles([ + { dataSource: DataSource.MANUAL, symbol: 'GF_COPX' } + ]); + + await importActivities({ + activitiesDto: [ + createActivityDto({ + dataSource: DataSource.MANUAL, + date: '2024-01-01T00:00:00.000Z', + symbol: CUSTOM_ASSET_PROFILE_SYMBOL, + type: 'BUY' + }) + ], + assetProfilesWithMarketDataDto: [ + { + currency: 'USD', + dataSource: DataSource.MANUAL, + marketData: [], + name: 'Global X Copper Miners ETF', + symbol: CUSTOM_ASSET_PROFILE_SYMBOL + } + ] + }); + + expect(addSymbolProfile).not.toHaveBeenCalled(); + + expect( + createActivity.mock.calls[0][0].SymbolProfile.connectOrCreate.where + ).toEqual({ + dataSource_symbol: { + dataSource: DataSource.MANUAL, + symbol: 'GF_COPX' + } + }); + }); + + it('prefers the custom asset profile of the user over a global profile with the same name', async () => { + getCustomSymbolProfilesByNames.mockResolvedValue([ + { + name: 'Global X Copper Miners ETF', + symbol: 'GF_GLOBAL', + userId: null + }, + { + name: 'Global X Copper Miners ETF', + symbol: 'GF_USER', + userId: 'user-id' + } + ]); + mockCreatedAssetProfiles([ + { dataSource: DataSource.MANUAL, symbol: 'GF_USER' } + ]); + + await importActivities({ + activitiesDto: [ + createActivityDto({ + dataSource: DataSource.MANUAL, + date: '2024-01-01T00:00:00.000Z', + symbol: CUSTOM_ASSET_PROFILE_SYMBOL, + type: 'BUY' + }) + ], + assetProfilesWithMarketDataDto: [ + { + currency: 'USD', + dataSource: DataSource.MANUAL, + marketData: [], + name: 'Global X Copper Miners ETF', + symbol: CUSTOM_ASSET_PROFILE_SYMBOL + } + ] + }); + + expect(addSymbolProfile).not.toHaveBeenCalled(); + + expect( + createActivity.mock.calls[0][0].SymbolProfile.connectOrCreate.where + ).toEqual({ + dataSource_symbol: { + dataSource: DataSource.MANUAL, + symbol: 'GF_USER' + } + }); + }); + + it('does not reuse a custom asset profile of another user', async () => { + getSymbolProfiles.mockResolvedValue([ + { + dataSource: DataSource.MANUAL, + name: 'Global X Copper Miners ETF', + symbol: 'GF_OTHER', + userId: 'other-user-id' + } + ]); + addSymbolProfile.mockImplementation((assetProfile) => { + return assetProfile; + }); + createActivity.mockImplementationOnce(({ date, SymbolProfile, type }) => { + return { + date, + type, + SymbolProfile: { + dataSource: SymbolProfile.connectOrCreate.create.dataSource, + symbol: SymbolProfile.connectOrCreate.create.symbol + } + }; + }); + + await importActivities({ + activitiesDto: [ + createActivityDto({ + dataSource: DataSource.MANUAL, + date: '2024-01-01T00:00:00.000Z', + symbol: 'GF_OTHER', + type: 'BUY' + }) + ], + assetProfilesWithMarketDataDto: [ + { + currency: 'USD', + dataSource: DataSource.MANUAL, + marketData: [], + name: 'Global X Copper Miners ETF', + symbol: 'GF_OTHER' + } + ] + }); + + expect(addSymbolProfile).toHaveBeenCalledTimes(1); + + const { symbol } = addSymbolProfile.mock.calls[0][0]; + + expect(symbol).not.toBe('GF_OTHER'); + expect(isUUID(symbol)).toBe(true); + + expect( + createActivity.mock.calls[0][0].SymbolProfile.connectOrCreate.where + ).toEqual({ + dataSource_symbol: { + dataSource: DataSource.MANUAL, + symbol + } + }); + }); + it('keeps the asset profiles of the activities in a dry run', async () => { const activities = await importActivities({ activitiesDto: [ diff --git a/apps/api/src/app/import/import.service.ts b/apps/api/src/app/import/import.service.ts index 6f036479c7..e4b8fd3ab0 100644 --- a/apps/api/src/app/import/import.service.ts +++ b/apps/api/src/app/import/import.service.ts @@ -596,7 +596,7 @@ export class ImportService { return name; }); - const [existingAssetProfiles, existingCustomAssetProfilesOfUser] = + const [existingAssetProfiles, existingCustomAssetProfiles] = await Promise.all([ this.symbolProfileService.getSymbolProfiles( assetProfilesWithMarketDataDto.map(({ dataSource, symbol }) => { @@ -624,27 +624,42 @@ export class ImportService { } ); - // If there is no asset profile or if the asset profile belongs to a - // different user, then reuse the custom asset profile of the user or - // create a new asset profile - if (existingAssetProfile?.userId !== user.id) { - // Check if the user has a custom asset profile with the same name. + // An asset profile is reusable if it belongs to the user or if it is a + // global asset profile. Only profiles of other users must not be reused. + const isReusableAssetProfile = + existingAssetProfile !== undefined && + (existingAssetProfile.userId === user.id || + existingAssetProfile.userId === null); + + // If there is no reusable asset profile, then reuse a custom asset + // profile with the same name or create a new asset profile + if (!isReusableAssetProfile) { + // Check if there is a custom asset profile with the same name. Prefer + // the profile of the user over a global asset profile. // Skip asset profiles with a legacy free-text symbol as they would // fail the symbol validation on a future import. - const existingCustomAssetProfileOfUser = + const matchingCustomAssetProfiles = assetProfileWithMarketData.dataSource === DataSource.MANUAL - ? existingCustomAssetProfilesOfUser.find((customAssetProfile) => { + ? existingCustomAssetProfiles.filter((customAssetProfile) => { return ( customAssetProfile.name === assetProfileWithMarketData.name && isValidCustomAssetProfileSymbol(customAssetProfile.symbol) ); }) - : undefined; + : []; + + const existingCustomAssetProfileToReuse = + matchingCustomAssetProfiles.find(({ userId }) => { + return userId === user.id; + }) ?? + matchingCustomAssetProfiles.find(({ userId }) => { + return userId === null; + }); - if (existingCustomAssetProfileOfUser) { - // Reuse the custom asset profile of the user instead of creating a duplicate - symbol = existingCustomAssetProfileOfUser.symbol; + if (existingCustomAssetProfileToReuse) { + // Reuse the custom asset profile instead of creating a duplicate + symbol = existingCustomAssetProfileToReuse.symbol; } else { const assetProfile: CreateAssetProfileDto = omit( assetProfileWithMarketData, diff --git a/apps/api/src/services/symbol-profile/symbol-profile.service.spec.ts b/apps/api/src/services/symbol-profile/symbol-profile.service.spec.ts new file mode 100644 index 0000000000..a177869065 --- /dev/null +++ b/apps/api/src/services/symbol-profile/symbol-profile.service.spec.ts @@ -0,0 +1,49 @@ +import { PrismaService } from '@ghostfolio/api/services/prisma/prisma.service'; + +import { DataSource } from '@prisma/client'; + +import { SymbolProfileService } from './symbol-profile.service'; + +describe('SymbolProfileService', () => { + let findMany: jest.Mock; + let symbolProfileService: SymbolProfileService; + + beforeEach(() => { + findMany = jest.fn().mockResolvedValue([]); + + symbolProfileService = new SymbolProfileService({ + symbolProfile: { + findMany + } + } as unknown as PrismaService); + }); + + describe('getCustomSymbolProfilesByNames', () => { + it('includes the manual asset profiles of the user and global profiles', async () => { + await symbolProfileService.getCustomSymbolProfilesByNames({ + names: ['Global X Copper Miners ETF'], + userId: 'user-id' + }); + + expect(findMany).toHaveBeenCalledWith({ + select: { name: true, symbol: true, userId: true }, + where: { + dataSource: DataSource.MANUAL, + name: { in: ['Global X Copper Miners ETF'] }, + OR: [{ userId: 'user-id' }, { userId: null }] + } + }); + }); + + it('returns an empty list without names', async () => { + await expect( + symbolProfileService.getCustomSymbolProfilesByNames({ + names: [], + userId: 'user-id' + }) + ).resolves.toEqual([]); + + expect(findMany).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/apps/api/src/services/symbol-profile/symbol-profile.service.ts b/apps/api/src/services/symbol-profile/symbol-profile.service.ts index ebc8a94c76..a52edf3897 100644 --- a/apps/api/src/services/symbol-profile/symbol-profile.service.ts +++ b/apps/api/src/services/symbol-profile/symbol-profile.service.ts @@ -111,17 +111,19 @@ export class SymbolProfileService { }: { names: string[]; userId: string; - }): Promise[]> { + }): Promise[]> { if (names.length === 0) { return []; } return this.prismaService.symbolProfile.findMany({ - select: { name: true, symbol: true }, + select: { name: true, symbol: true, userId: true }, where: { - userId, dataSource: DataSource.MANUAL, - name: { in: names } + name: { in: names }, + // Include manual asset profiles created by an admin, which are global + // and therefore not owned by a specific user + OR: [{ userId }, { userId: null }] } }); }