diff --git a/CHANGELOG.md b/CHANGELOG.md index 7391a2e2b8..cc53dee0e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- Extended the creation and the import of activities to support the asset profiles of a premium data provider which the user already has - Improved the performance of the historical market data table of the admin control panel by counting the market data per asset profile - Improved the performance of the historical market data table of the admin control panel by querying subscription usage in a single database query - Hardened the `POST api/v1/import` endpoint by hiding internal error messages diff --git a/apps/api/src/app/activities/activities.controller.ts b/apps/api/src/app/activities/activities.controller.ts index 98c3f674bc..84f62395a7 100644 --- a/apps/api/src/app/activities/activities.controller.ts +++ b/apps/api/src/app/activities/activities.controller.ts @@ -233,6 +233,7 @@ export class ActivitiesController { try { await this.dataProviderService.validateActivities({ subscription, + userId, activitiesDto: [ { currency: data.currency, diff --git a/apps/api/src/app/endpoints/ai/ai.service.spec.ts b/apps/api/src/app/endpoints/ai/ai.service.spec.ts index 69636a19fb..bc9f6cd6ce 100644 --- a/apps/api/src/app/endpoints/ai/ai.service.spec.ts +++ b/apps/api/src/app/endpoints/ai/ai.service.spec.ts @@ -12,7 +12,6 @@ import { PROPERTY_OPENROUTER_MODEL_AGENT, PROPERTY_OPENROUTER_MODEL_VISION } from '@ghostfolio/common/config'; -import { SubscriptionType } from '@ghostfolio/common/enums'; import { isValidCustomAssetProfileSymbol } from '@ghostfolio/common/helper'; import type { UserWithSettings } from '@ghostfolio/common/types'; @@ -100,8 +99,7 @@ describe('AiService', () => { dataProviderService = { getDataSources: jest .fn() - .mockResolvedValue([DataSource.MANUAL, DataSource.YAHOO]), - isPremiumRequired: jest.fn().mockReturnValue(false) + .mockResolvedValue([DataSource.MANUAL, DataSource.YAHOO]) } as unknown as DataProviderService; mcpService = { @@ -357,8 +355,7 @@ describe('AiService', () => { const user = { accounts: [{ currency: 'CHF', id: 'account-id', name: 'Brokerage' }], id: 'user-id', - settings: { settings: { baseCurrency: 'EUR', locale: 'de-CH' } }, - subscription: { type: SubscriptionType.Basic } + settings: { settings: { baseCurrency: 'EUR', locale: 'de-CH' } } } as unknown as UserWithSettings; it('Reads the rows of the file and matches them with the accounts, the base currency and the id of the user', async () => { @@ -402,7 +399,6 @@ describe('AiService', () => { rows, accounts: user.accounts, broker: 'Swissquote', - subscription: user.subscription, userCurrency: 'EUR', userId: 'user-id' }); @@ -975,55 +971,6 @@ describe('AiService', () => { ], rowsWithoutMatch: [] }); - - expect(dataProviderService.isPremiumRequired).toHaveBeenCalledWith({ - dataSource: DataSource.YAHOO, - subscription: undefined - }); - }); - - it('Passes no holding with a data source which requires Premium to the model and gives a row whose asset match is such a holding as a row without a match', async () => { - jest.spyOn(Logger.prototype, 'warn').mockImplementation(); - - jest.mocked(dataProviderService.isPremiumRequired).mockReturnValue(true); - - const row = createActivityRow({ - accountName: null, - isin: 'IE00BK5BQT80', - symbol: null - }); - - const subscription = { - type: SubscriptionType.Basic - } as UserWithSettings['subscription']; - - mockRowMatches({ - assetProfilesOfSearch: [], - assets: [ - { currency: 'USD', dataSource: 'YAHOO', id: 0, symbol: 'VWRA.SW' } - ] - }); - - mockHoldings(); - - await expect( - aiService.getActivitiesFromRows({ - accounts, - subscription, - userCurrency, - userId, - rows: [row] - }) - ).resolves.toEqual({ activities: [], rowsWithoutMatch: [row] }); - - expect(dataProviderService.isPremiumRequired).toHaveBeenCalledWith({ - subscription, - dataSource: DataSource.YAHOO - }); - - const { prompt } = jest.mocked(generateText).mock.calls[0][0]; - - expect(JSON.parse(prompt as string)).toHaveProperty('holdings', []); }); it('Passes no holding with a data source which is not configured to the model and gives a row whose asset match is such a holding as a row without a match', async () => { diff --git a/apps/api/src/app/endpoints/ai/ai.service.ts b/apps/api/src/app/endpoints/ai/ai.service.ts index 516e93ad81..02af5d8d0a 100644 --- a/apps/api/src/app/endpoints/ai/ai.service.ts +++ b/apps/api/src/app/endpoints/ai/ai.service.ts @@ -124,7 +124,6 @@ export class AiService { broker, rows, accounts: user.accounts, - subscription: user.subscription, userCurrency: user.settings.settings.baseCurrency, userId: user.id }); @@ -152,7 +151,6 @@ export class AiService { accounts, broker, rows, - subscription, userCurrency, userId }: { @@ -160,7 +158,6 @@ export class AiService { accounts: AccountToMatch[]; broker?: string; rows: ActivityRow[]; - subscription?: UserWithSettings['subscription']; userCurrency: string; userId: string; }) { @@ -211,7 +208,6 @@ export class AiService { abortSignal, accountHints, assets, - subscription, userId }) : { accounts: [], assets: [] }; @@ -524,15 +520,9 @@ export class AiService { /** * Gives the holdings of the user with the tool get-portfolio of the server * of the Model Context Protocol, without the holdings whose data source the - * import refuses for the user + * import refuses */ - private async getHoldingsToMatch({ - subscription, - userId - }: { - subscription?: UserWithSettings['subscription']; - userId: string; - }) { + private async getHoldingsToMatch({ userId }: { userId: string }) { const [{ content }, dataSources] = await Promise.all([ this.mcpService.getPortfolio({ userId, withValues: false }), this.dataProviderService.getDataSources() @@ -540,11 +530,7 @@ export class AiService { return this.getHoldingsFromTable(content[0].text).filter( ({ dataSource }) => { - return this.isValidDataSourceForImport({ - dataSource, - dataSources, - subscription - }); + return this.isValidDataSourceForImport({ dataSource, dataSources }); } ); } @@ -608,8 +594,8 @@ export class AiService { * symbol only. The holdings let the model select an asset profile which the * user already has, also if no search gives it, for example as no data * provider finds its ISIN. A holding whose data source the import refuses - * for the user is not given to the model, so that the model selects an - * asset profile of the search instead and the whole import does not fail. + * is not given to the model, so that the model selects an asset profile of + * the search instead and the whole import does not fail. * If get-accounts fails, the model gets no accounts, so that only the * account hints have no match. If get-portfolio or the data sources fail, * the model gets no holdings. The reasoning is disabled, as it can add @@ -625,13 +611,11 @@ export class AiService { abortSignal, accountHints, assets, - subscription, userId }: { abortSignal?: AbortSignal; accountHints: AccountHint[]; assets: AssetToMatch[]; - subscription?: UserWithSettings['subscription']; userId: string; }) { const startTime = performance.now(); @@ -650,7 +634,7 @@ export class AiService { }) : undefined, assets.length > 0 - ? this.getHoldingsToMatch({ subscription, userId }).catch( + ? this.getHoldingsToMatch({ userId }).catch( (error: Error): undefined => { this.logger.warn(`Could not get the holdings: ${error.message}`); @@ -751,31 +735,24 @@ export class AiService { /** * Checks without a request to a data provider if the import takes a data - * source for the user: it is configured and does not require Ghostfolio - * Premium, for example after the Premium subscription has expired. The data - * source is first transformed as POST api/v1/import transforms it. A symbol - * which the data source cannot resolve, for example a delisted one, is not - * found, as this needs a request to the data provider. + * source of a holding: it is configured. The data source is first + * transformed as POST api/v1/import transforms it. A data source which + * requires Ghostfolio Premium is taken, as the import takes it for an asset + * profile which the user already has. A symbol which the data source cannot + * resolve, for example a delisted one, is not found, as this needs a + * request to the data provider. */ private isValidDataSourceForImport({ dataSource, - dataSources, - subscription + dataSources }: { dataSource: string; dataSources: DataSource[]; - subscription?: UserWithSettings['subscription']; }) { - const dataSourceOfImport = transformDataSourceInRequest({ - dataSource, - configurationService: this.configurationService - }); - - return ( - dataSources.includes(dataSourceOfImport) && - !this.dataProviderService.isPremiumRequired({ - subscription, - dataSource: dataSourceOfImport + return dataSources.includes( + transformDataSourceInRequest({ + dataSource, + configurationService: this.configurationService }) ); } 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 index 815ab2743e..d2e6b513f5 100644 --- a/apps/api/src/services/data-provider/data-provider.service.spec.ts +++ b/apps/api/src/services/data-provider/data-provider.service.spec.ts @@ -4,8 +4,11 @@ import { MarketDataService } from '@ghostfolio/api/services/market-data/market-d import { PrismaService } from '@ghostfolio/api/services/prisma/prisma.service'; import { PropertyService } from '@ghostfolio/api/services/property/property.service'; import { RedisCacheService } from '@ghostfolio/api/services/redis-cache/redis-cache.service'; +import { SubscriptionType } from '@ghostfolio/common/enums'; +import { getAssetProfileIdentifier } from '@ghostfolio/common/helper'; +import type { UserWithSettings } from '@ghostfolio/common/types'; -import { DataSource } from '@prisma/client'; +import { AssetClass, AssetSubClass, DataSource } from '@prisma/client'; import { DataProviderService } from './data-provider.service'; @@ -109,4 +112,139 @@ describe('DataProviderService', () => { }).toThrow('No data provider has been found.'); }); }); + + describe('validateActivities', () => { + const assetProfileOfUser = { + assetClass: AssetClass.EQUITY, + assetSubClass: AssetSubClass.ETF, + currency: 'USD', + dataSource: DataSource.EOD_HISTORICAL_DATA, + isin: 'IE00BK5BQT80', + name: 'Vanguard FTSE All-World UCITS ETF', + symbol: 'VWRA.LSE' + }; + + const activityOfAssetProfileOfUser = { + currency: 'USD', + dataSource: DataSource.EOD_HISTORICAL_DATA, + symbol: 'VWRA.LSE', + type: 'BUY' as const + }; + + const basicSubscription = { + type: SubscriptionType.Basic + } as UserWithSettings['subscription']; + + const userId = 'user-id'; + + let dataProviderService: DataProviderService; + let eodHistoricalDataService: DataProviderInterface; + let findFirstSymbolProfile: jest.Mock; + + beforeEach(() => { + const configuration = { + DATA_SOURCES: [DataSource.EOD_HISTORICAL_DATA, DataSource.YAHOO], + DATA_SOURCES_GHOSTFOLIO_DATA_PROVIDER: [], + ENABLE_FEATURE_SUBSCRIPTION: true + }; + + eodHistoricalDataService = { + getAssetProfile: jest.fn().mockResolvedValue(assetProfileOfUser), + getDataProviderInfo: jest.fn().mockReturnValue({ isPremium: true }), + getName: jest.fn().mockReturnValue(DataSource.EOD_HISTORICAL_DATA) + } as unknown as DataProviderInterface; + + findFirstSymbolProfile = jest.fn().mockResolvedValue(assetProfileOfUser); + + dataProviderService = new DataProviderService( + { + get: (key: keyof typeof configuration) => { + return configuration[key]; + } + } as unknown as ConfigurationService, + [eodHistoricalDataService], + {} as MarketDataService, + { + symbolProfile: { findFirst: findFirstSymbolProfile } + } as unknown as PrismaService, + { + getByKey: jest.fn().mockResolvedValue(undefined) + } as unknown as PropertyService, + {} as RedisCacheService + ); + }); + + it('takes the stored asset profile of a premium data provider without a request to the data provider for the Basic subscription if the user has an activity with it', async () => { + await expect( + dataProviderService.validateActivities({ + userId, + activitiesDto: [activityOfAssetProfileOfUser], + maxActivitiesToImport: 1, + subscription: basicSubscription + }) + ).resolves.toEqual({ + [getAssetProfileIdentifier(assetProfileOfUser)]: assetProfileOfUser + }); + + expect(findFirstSymbolProfile).toHaveBeenCalledWith( + expect.objectContaining({ + where: { + activities: { some: { userId } }, + dataSource: DataSource.EOD_HISTORICAL_DATA, + symbol: 'VWRA.LSE' + } + }) + ); + + expect(eodHistoricalDataService.getAssetProfile).not.toHaveBeenCalled(); + }); + + it('gets the stored asset profile once for several activities with it', async () => { + await dataProviderService.validateActivities({ + userId, + activitiesDto: [ + activityOfAssetProfileOfUser, + { ...activityOfAssetProfileOfUser, type: 'SELL' } + ], + maxActivitiesToImport: 2, + subscription: basicSubscription + }); + + expect(findFirstSymbolProfile).toHaveBeenCalledTimes(1); + }); + + it('throws an error for a premium data provider for the Basic subscription if the user has no activity with the asset profile', async () => { + findFirstSymbolProfile.mockResolvedValue(null); + + await expect( + dataProviderService.validateActivities({ + userId, + activitiesDto: [activityOfAssetProfileOfUser], + maxActivitiesToImport: 1, + subscription: basicSubscription + }) + ).rejects.toThrow( + 'activity.dataSource ("EOD_HISTORICAL_DATA") requires Ghostfolio Premium' + ); + + expect(eodHistoricalDataService.getAssetProfile).not.toHaveBeenCalled(); + }); + + it('resolves the asset profile of a premium data provider with the data provider for the Premium subscription', async () => { + await dataProviderService.validateActivities({ + userId, + activitiesDto: [activityOfAssetProfileOfUser], + maxActivitiesToImport: 1, + subscription: { + type: SubscriptionType.Premium + } as UserWithSettings['subscription'] + }); + + expect(findFirstSymbolProfile).not.toHaveBeenCalled(); + + expect(eodHistoricalDataService.getAssetProfile).toHaveBeenCalledWith({ + symbol: 'VWRA.LSE' + }); + }); + }); }); 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 526637891e..2fe65c2678 100644 --- a/apps/api/src/services/data-provider/data-provider.service.ts +++ b/apps/api/src/services/data-provider/data-provider.service.ts @@ -222,7 +222,8 @@ export class DataProviderService implements OnModuleInit { activitiesDto, assetProfilesWithMarketDataDto, maxActivitiesToImport, - subscription + subscription, + userId }: { activitiesDto: Pick< Partial, @@ -231,6 +232,7 @@ export class DataProviderService implements OnModuleInit { assetProfilesWithMarketDataDto?: ImportDataDto['assetProfiles']; maxActivitiesToImport: number; subscription: UserWithSettings['subscription']; + userId: string; }) { if (activitiesDto?.length > maxActivitiesToImport) { throw new ImportValidationError( @@ -273,17 +275,31 @@ export class DataProviderService implements OnModuleInit { ); } - if (this.isPremiumRequired({ dataSource, subscription })) { - throw new ImportValidationError( - `${activityPath}.dataSource ("${maskedDataSource}") requires Ghostfolio Premium` - ); - } - const assetProfileIdentifier = getAssetProfileIdentifier({ dataSource, symbol }); + if (this.isPremiumRequired({ dataSource, subscription })) { + // The user can add activities to an asset profile of a premium data + // provider which the user already has, for example after the Premium + // subscription has expired. The stored asset profile is taken, so + // that the validation sends no request to the data provider. + const assetProfileOfUser = + assetProfiles[assetProfileIdentifier] ?? + (await this.getAssetProfileOfUser({ dataSource, symbol, userId })); + + if (!assetProfileOfUser) { + throw new ImportValidationError( + `${activityPath}.dataSource ("${maskedDataSource}") requires Ghostfolio Premium` + ); + } + + assetProfiles[assetProfileIdentifier] = assetProfileOfUser; + + continue; + } + if (!assetProfiles[assetProfileIdentifier]) { const assetProfileInImport = assetProfilesWithMarketDataDto?.find( (assetProfileWithMarketData) => { @@ -948,6 +964,35 @@ export class DataProviderService implements OnModuleInit { }; } + /** + * Gives the stored asset profile if the user has an activity with it, with + * the fields which a data provider gives + */ + private async getAssetProfileOfUser({ + dataSource, + symbol, + userId + }: AssetProfileIdentifier & { userId: string }): Promise< + Partial + > { + return this.prismaService.symbolProfile.findFirst({ + select: { + assetClass: true, + assetSubClass: true, + currency: true, + dataSource: true, + isin: true, + name: true, + symbol: true + }, + where: { + dataSource, + symbol, + activities: { some: { userId } } + } + }); + } + private hasCurrency({ assetProfileIdentifiers, currency diff --git a/apps/api/src/services/import/import.service.ts b/apps/api/src/services/import/import.service.ts index 2d09cb78df..57132691bd 100644 --- a/apps/api/src/services/import/import.service.ts +++ b/apps/api/src/services/import/import.service.ts @@ -761,7 +761,8 @@ export class ImportService { activitiesDto, assetProfilesWithMarketDataDto, maxActivitiesToImport, - subscription: user.subscription + subscription: user.subscription, + userId: user.id }); const activitiesExtendedWithErrors = await this.extendActivitiesWithErrors({