Browse Source

Feature/Premium data source of holdings in import (#8196)

* Support asset profiles of premium data provider which user already has

* Update changelog
pull/8295/head
Thomas Kaul 4 days ago
committed by GitHub
parent
commit
6ef4988d86
No known key found for this signature in database GPG Key ID: B5690EEEBB952194
  1. 1
      CHANGELOG.md
  2. 1
      apps/api/src/app/activities/activities.controller.ts
  3. 57
      apps/api/src/app/endpoints/ai/ai.service.spec.ts
  4. 53
      apps/api/src/app/endpoints/ai/ai.service.ts
  5. 140
      apps/api/src/services/data-provider/data-provider.service.spec.ts
  6. 55
      apps/api/src/services/data-provider/data-provider.service.ts
  7. 3
      apps/api/src/services/import/import.service.ts

1
CHANGELOG.md

@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Changed ### 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 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 - 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 - Hardened the `POST api/v1/import` endpoint by hiding internal error messages

1
apps/api/src/app/activities/activities.controller.ts

@ -233,6 +233,7 @@ export class ActivitiesController {
try { try {
await this.dataProviderService.validateActivities({ await this.dataProviderService.validateActivities({
subscription, subscription,
userId,
activitiesDto: [ activitiesDto: [
{ {
currency: data.currency, currency: data.currency,

57
apps/api/src/app/endpoints/ai/ai.service.spec.ts

@ -12,7 +12,6 @@ import {
PROPERTY_OPENROUTER_MODEL_AGENT, PROPERTY_OPENROUTER_MODEL_AGENT,
PROPERTY_OPENROUTER_MODEL_VISION PROPERTY_OPENROUTER_MODEL_VISION
} from '@ghostfolio/common/config'; } from '@ghostfolio/common/config';
import { SubscriptionType } from '@ghostfolio/common/enums';
import { isValidCustomAssetProfileSymbol } from '@ghostfolio/common/helper'; import { isValidCustomAssetProfileSymbol } from '@ghostfolio/common/helper';
import type { UserWithSettings } from '@ghostfolio/common/types'; import type { UserWithSettings } from '@ghostfolio/common/types';
@ -100,8 +99,7 @@ describe('AiService', () => {
dataProviderService = { dataProviderService = {
getDataSources: jest getDataSources: jest
.fn() .fn()
.mockResolvedValue([DataSource.MANUAL, DataSource.YAHOO]), .mockResolvedValue([DataSource.MANUAL, DataSource.YAHOO])
isPremiumRequired: jest.fn().mockReturnValue(false)
} as unknown as DataProviderService; } as unknown as DataProviderService;
mcpService = { mcpService = {
@ -357,8 +355,7 @@ describe('AiService', () => {
const user = { const user = {
accounts: [{ currency: 'CHF', id: 'account-id', name: 'Brokerage' }], accounts: [{ currency: 'CHF', id: 'account-id', name: 'Brokerage' }],
id: 'user-id', id: 'user-id',
settings: { settings: { baseCurrency: 'EUR', locale: 'de-CH' } }, settings: { settings: { baseCurrency: 'EUR', locale: 'de-CH' } }
subscription: { type: SubscriptionType.Basic }
} as unknown as UserWithSettings; } 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 () => { 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, rows,
accounts: user.accounts, accounts: user.accounts,
broker: 'Swissquote', broker: 'Swissquote',
subscription: user.subscription,
userCurrency: 'EUR', userCurrency: 'EUR',
userId: 'user-id' userId: 'user-id'
}); });
@ -975,55 +971,6 @@ describe('AiService', () => {
], ],
rowsWithoutMatch: [] 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 () => { 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 () => {

53
apps/api/src/app/endpoints/ai/ai.service.ts

@ -124,7 +124,6 @@ export class AiService {
broker, broker,
rows, rows,
accounts: user.accounts, accounts: user.accounts,
subscription: user.subscription,
userCurrency: user.settings.settings.baseCurrency, userCurrency: user.settings.settings.baseCurrency,
userId: user.id userId: user.id
}); });
@ -152,7 +151,6 @@ export class AiService {
accounts, accounts,
broker, broker,
rows, rows,
subscription,
userCurrency, userCurrency,
userId userId
}: { }: {
@ -160,7 +158,6 @@ export class AiService {
accounts: AccountToMatch[]; accounts: AccountToMatch[];
broker?: string; broker?: string;
rows: ActivityRow[]; rows: ActivityRow[];
subscription?: UserWithSettings['subscription'];
userCurrency: string; userCurrency: string;
userId: string; userId: string;
}) { }) {
@ -211,7 +208,6 @@ export class AiService {
abortSignal, abortSignal,
accountHints, accountHints,
assets, assets,
subscription,
userId userId
}) })
: { accounts: [], assets: [] }; : { accounts: [], assets: [] };
@ -524,15 +520,9 @@ export class AiService {
/** /**
* Gives the holdings of the user with the tool get-portfolio of the server * 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 * of the Model Context Protocol, without the holdings whose data source the
* import refuses for the user * import refuses
*/ */
private async getHoldingsToMatch({ private async getHoldingsToMatch({ userId }: { userId: string }) {
subscription,
userId
}: {
subscription?: UserWithSettings['subscription'];
userId: string;
}) {
const [{ content }, dataSources] = await Promise.all([ const [{ content }, dataSources] = await Promise.all([
this.mcpService.getPortfolio({ userId, withValues: false }), this.mcpService.getPortfolio({ userId, withValues: false }),
this.dataProviderService.getDataSources() this.dataProviderService.getDataSources()
@ -540,11 +530,7 @@ export class AiService {
return this.getHoldingsFromTable(content[0].text).filter( return this.getHoldingsFromTable(content[0].text).filter(
({ dataSource }) => { ({ dataSource }) => {
return this.isValidDataSourceForImport({ return this.isValidDataSourceForImport({ dataSource, dataSources });
dataSource,
dataSources,
subscription
});
} }
); );
} }
@ -608,8 +594,8 @@ export class AiService {
* symbol only. The holdings let the model select an asset profile which the * 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 * 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 * 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 * is not given to the model, so that the model selects an asset profile of
* asset profile of the search instead and the whole import does not fail. * the search instead and the whole import does not fail.
* If get-accounts fails, the model gets no accounts, so that only the * 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, * 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 * the model gets no holdings. The reasoning is disabled, as it can add
@ -625,13 +611,11 @@ export class AiService {
abortSignal, abortSignal,
accountHints, accountHints,
assets, assets,
subscription,
userId userId
}: { }: {
abortSignal?: AbortSignal; abortSignal?: AbortSignal;
accountHints: AccountHint[]; accountHints: AccountHint[];
assets: AssetToMatch[]; assets: AssetToMatch[];
subscription?: UserWithSettings['subscription'];
userId: string; userId: string;
}) { }) {
const startTime = performance.now(); const startTime = performance.now();
@ -650,7 +634,7 @@ export class AiService {
}) })
: undefined, : undefined,
assets.length > 0 assets.length > 0
? this.getHoldingsToMatch({ subscription, userId }).catch( ? this.getHoldingsToMatch({ userId }).catch(
(error: Error): undefined => { (error: Error): undefined => {
this.logger.warn(`Could not get the holdings: ${error.message}`); 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 * 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 * source of a holding: it is configured. The data source is first
* Premium, for example after the Premium subscription has expired. The data * transformed as POST api/v1/import transforms it. A data source which
* source is first transformed as POST api/v1/import transforms it. A symbol * requires Ghostfolio Premium is taken, as the import takes it for an asset
* which the data source cannot resolve, for example a delisted one, is not * profile which the user already has. A symbol which the data source cannot
* found, as this needs a request to the data provider. * resolve, for example a delisted one, is not found, as this needs a
* request to the data provider.
*/ */
private isValidDataSourceForImport({ private isValidDataSourceForImport({
dataSource, dataSource,
dataSources, dataSources
subscription
}: { }: {
dataSource: string; dataSource: string;
dataSources: DataSource[]; dataSources: DataSource[];
subscription?: UserWithSettings['subscription'];
}) { }) {
const dataSourceOfImport = transformDataSourceInRequest({ return dataSources.includes(
transformDataSourceInRequest({
dataSource, dataSource,
configurationService: this.configurationService configurationService: this.configurationService
});
return (
dataSources.includes(dataSourceOfImport) &&
!this.dataProviderService.isPremiumRequired({
subscription,
dataSource: dataSourceOfImport
}) })
); );
} }

140
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 { PrismaService } from '@ghostfolio/api/services/prisma/prisma.service';
import { PropertyService } from '@ghostfolio/api/services/property/property.service'; import { PropertyService } from '@ghostfolio/api/services/property/property.service';
import { RedisCacheService } from '@ghostfolio/api/services/redis-cache/redis-cache.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'; import { DataProviderService } from './data-provider.service';
@ -109,4 +112,139 @@ describe('DataProviderService', () => {
}).toThrow('No data provider has been found.'); }).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'
});
});
});
}); });

55
apps/api/src/services/data-provider/data-provider.service.ts

@ -222,7 +222,8 @@ export class DataProviderService implements OnModuleInit {
activitiesDto, activitiesDto,
assetProfilesWithMarketDataDto, assetProfilesWithMarketDataDto,
maxActivitiesToImport, maxActivitiesToImport,
subscription subscription,
userId
}: { }: {
activitiesDto: Pick< activitiesDto: Pick<
Partial<CreateOrderDto>, Partial<CreateOrderDto>,
@ -231,6 +232,7 @@ export class DataProviderService implements OnModuleInit {
assetProfilesWithMarketDataDto?: ImportDataDto['assetProfiles']; assetProfilesWithMarketDataDto?: ImportDataDto['assetProfiles'];
maxActivitiesToImport: number; maxActivitiesToImport: number;
subscription: UserWithSettings['subscription']; subscription: UserWithSettings['subscription'];
userId: string;
}) { }) {
if (activitiesDto?.length > maxActivitiesToImport) { if (activitiesDto?.length > maxActivitiesToImport) {
throw new ImportValidationError( throw new ImportValidationError(
@ -273,16 +275,30 @@ export class DataProviderService implements OnModuleInit {
); );
} }
const assetProfileIdentifier = getAssetProfileIdentifier({
dataSource,
symbol
});
if (this.isPremiumRequired({ dataSource, subscription })) { 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( throw new ImportValidationError(
`${activityPath}.dataSource ("${maskedDataSource}") requires Ghostfolio Premium` `${activityPath}.dataSource ("${maskedDataSource}") requires Ghostfolio Premium`
); );
} }
const assetProfileIdentifier = getAssetProfileIdentifier({ assetProfiles[assetProfileIdentifier] = assetProfileOfUser;
dataSource,
symbol continue;
}); }
if (!assetProfiles[assetProfileIdentifier]) { if (!assetProfiles[assetProfileIdentifier]) {
const assetProfileInImport = assetProfilesWithMarketDataDto?.find( const assetProfileInImport = assetProfilesWithMarketDataDto?.find(
@ -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<SymbolProfile>
> {
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({ private hasCurrency({
assetProfileIdentifiers, assetProfileIdentifiers,
currency currency

3
apps/api/src/services/import/import.service.ts

@ -761,7 +761,8 @@ export class ImportService {
activitiesDto, activitiesDto,
assetProfilesWithMarketDataDto, assetProfilesWithMarketDataDto,
maxActivitiesToImport, maxActivitiesToImport,
subscription: user.subscription subscription: user.subscription,
userId: user.id
}); });
const activitiesExtendedWithErrors = await this.extendActivitiesWithErrors({ const activitiesExtendedWithErrors = await this.extendActivitiesWithErrors({

Loading…
Cancel
Save