Browse Source

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
pull/7513/head
Varun Jain 4 weeks ago
parent
commit
21f9bcfcc9
  1. 1
      CHANGELOG.md
  2. 232
      apps/api/src/app/import/import.service.spec.ts
  3. 18
      apps/api/src/app/import/import.service.ts

1
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

232
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);
});
});

18
apps/api/src/app/import/import.service.ts

@ -392,7 +392,14 @@ export class ImportService {
})
);
const createdAssetProfileIdentifiers = new Set<string>();
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

Loading…
Cancel
Save