diff --git a/apps/api/src/app/endpoints/mcp/mcp.controller.spec.ts b/apps/api/src/app/endpoints/mcp/mcp.controller.spec.ts index aed5e20b96..78e84bf53d 100644 --- a/apps/api/src/app/endpoints/mcp/mcp.controller.spec.ts +++ b/apps/api/src/app/endpoints/mcp/mcp.controller.spec.ts @@ -2,6 +2,7 @@ import { REQUIRES_SCOPE_KEY } from '@ghostfolio/api/decorators/requires-scope.de import { McpToolExceptionFilter } from '@ghostfolio/api/filters/mcp-tool-exception.filter'; import { AccessGuard } from '@ghostfolio/api/guards/access.guard'; import { getMcpUserOfBearerToken } from '@ghostfolio/api/helper/bearer-token.helper'; +import { SubscriptionType } from '@ghostfolio/common/enums'; import { getScopesOfAccess, getScopesOfAccessLevel, @@ -130,6 +131,44 @@ async function getWithValuesOfReadTools(accessLevel: AccessLevel) { }); } +/** + * Gives whether the controller asks the service for the asset performance in + * base currency of an access with the permission "View" + */ +async function getWithAssetPerformanceInBaseCurrency( + subscriptionType?: SubscriptionType +) { + const mcpService = { getPerformance: jest.fn() }; + + const controller = new GhostfolioMcpController( + mcpService as unknown as McpService + ); + + await controller.getPerformance( + { + isActive: true, + scopes: getScopesOfAccess({ + scopes: getScopesOfAccessLevel('READ'), + type: 'MCP' + }), + userId: 'user-id', + userSettings: { baseCurrency: 'CHF' }, + userSubscription: subscriptionType + ? ({ + type: subscriptionType + } as ImpersonationContext['userSubscription']) + : undefined + }, + {} as Parameters[1] + ); + + return ( + mcpService.getPerformance.mock.calls[0][0] as { + withAssetPerformanceInBaseCurrency: boolean; + } + ).withAssetPerformanceInBaseCurrency; +} + describe('GhostfolioMcpController', () => { // A tool without the decorator of the scope would be open to every access, // hence a new tool has to declare its scope @@ -270,6 +309,22 @@ describe('GhostfolioMcpController', () => { ]); }); + it('Gives the asset performance in base currency for a Premium subscription', async () => { + expect( + await getWithAssetPerformanceInBaseCurrency(SubscriptionType.Premium) + ).toBe(true); + }); + + it('Gives the asset performance in base currency if the subscription is disabled', async () => { + expect(await getWithAssetPerformanceInBaseCurrency()).toBe(true); + }); + + it('Gives no asset performance in base currency for a Basic subscription', async () => { + expect( + await getWithAssetPerformanceInBaseCurrency(SubscriptionType.Basic) + ).toBe(false); + }); + it('Lists no tool for an inactive access', () => { expect( getNamesOfListedTools({ diff --git a/apps/api/src/app/endpoints/mcp/mcp.controller.ts b/apps/api/src/app/endpoints/mcp/mcp.controller.ts index c9df178000..2fa4f2e235 100644 --- a/apps/api/src/app/endpoints/mcp/mcp.controller.ts +++ b/apps/api/src/app/endpoints/mcp/mcp.controller.ts @@ -3,6 +3,7 @@ import { RequiresScopeOfAccess } from '@ghostfolio/api/decorators/requires-scope import { McpToolExceptionFilter } from '@ghostfolio/api/filters/mcp-tool-exception.filter'; import { PortfolioTableService } from '@ghostfolio/api/services/portfolio-table/portfolio-table.service'; import { MCP_MAX_ACTIVITIES } from '@ghostfolio/common/config'; +import { SubscriptionType } from '@ghostfolio/common/enums'; import { hasScope, scopes } from '@ghostfolio/common/scopes'; import type { ImpersonationContext } from '@ghostfolio/common/types'; @@ -37,7 +38,7 @@ export class GhostfolioMcpController { ', ' )}. If the access reads the monetary values, these columns are given in addition: ${PortfolioTableService.getAccountsTableValueColumnNames().join( ', ' - )}. The allocation in percentage is relative to the accounts of the result, hence the parameters change it.`, + )}. The allocation in percentage is relative to the accounts of the result, hence the parameters change it. The parameters change the value in base currency as well, with the holding parameter it is the value of the holding in the account without the cash balance. The balance is always the full cash balance of the account.`, name: 'get-accounts', parameters: GET_ACCOUNTS_PARAMETERS }) @@ -98,12 +99,18 @@ export class GhostfolioMcpController { }) public async getPerformance( @Impersonation() - { scopes: impersonationScopes, userId }: ImpersonationContext, + { + scopes: impersonationScopes, + userId, + userSubscription + }: ImpersonationContext, @Payload() parameters: z.infer ) { return this.mcpService.getPerformance({ ...parameters, userId, + withAssetPerformanceInBaseCurrency: + userSubscription?.type !== SubscriptionType.Basic, withValues: hasScope(impersonationScopes, scopes.portfolioReadValues) }); } diff --git a/apps/api/src/app/endpoints/mcp/mcp.module.ts b/apps/api/src/app/endpoints/mcp/mcp.module.ts index 1da7a06823..f9d571d407 100644 --- a/apps/api/src/app/endpoints/mcp/mcp.module.ts +++ b/apps/api/src/app/endpoints/mcp/mcp.module.ts @@ -39,7 +39,7 @@ import { McpService } from './mcp.service'; return new McpStrategy({ instructions: - 'Ghostfolio is a wealth management application. The tools read the portfolio and the watchlist of the user who granted the access and import activities into the portfolio. They give no quantity and no monetary value (except the unit price of an activity). Only the tools which the permission of the access covers are listed. To import activities, the access needs the permission "Restricted view and manage".', + 'Ghostfolio is a wealth management application. The tools read the portfolio and the watchlist of the user who granted the access and import activities into the portfolio. They give the quantities and the monetary values only if the access has the permission "View" or "View and manage", otherwise no quantity and no monetary value (except the unit price of an activity). Only the tools which the permission of the access covers are listed. To import activities, the access needs the permission "Restricted view and manage" or "View and manage".', name: 'ghostfolio', resolveUser: getMcpUserOfBearerToken, title: 'Ghostfolio', diff --git a/apps/api/src/app/endpoints/mcp/mcp.service.spec.ts b/apps/api/src/app/endpoints/mcp/mcp.service.spec.ts index ec2a1eea49..d6a3211144 100644 --- a/apps/api/src/app/endpoints/mcp/mcp.service.spec.ts +++ b/apps/api/src/app/endpoints/mcp/mcp.service.spec.ts @@ -378,6 +378,18 @@ describe('McpService', () => { expect.objectContaining({ withValues: true }) ); }); + + it('Gives the asset performance in base currency if the subscription permits it', async () => { + await mcpService.getPerformance({ + userId, + range: '2024', + withAssetPerformanceInBaseCurrency: true + }); + + expect(portfolioTableService.getPerformanceTable).toHaveBeenCalledWith( + expect.objectContaining({ withAssetPerformanceInBaseCurrency: true }) + ); + }); }); describe('getPortfolio', () => { diff --git a/apps/api/src/app/endpoints/mcp/mcp.service.ts b/apps/api/src/app/endpoints/mcp/mcp.service.ts index f4f5f38bd1..4b4c32f6c5 100644 --- a/apps/api/src/app/endpoints/mcp/mcp.service.ts +++ b/apps/api/src/app/endpoints/mcp/mcp.service.ts @@ -110,9 +110,11 @@ export class McpService { holding, range, userId, + withAssetPerformanceInBaseCurrency, withValues }: z.infer & { userId: string; + withAssetPerformanceInBaseCurrency?: boolean; withValues?: boolean; }) { const filters = this.apiService.buildFiltersFromQueryParams({ @@ -124,6 +126,7 @@ export class McpService { const table = await this.portfolioTableService.getPerformanceTable({ filters, userId, + withAssetPerformanceInBaseCurrency, withValues, dateRange: range }); diff --git a/apps/api/src/services/portfolio-table/portfolio-table.service.spec.ts b/apps/api/src/services/portfolio-table/portfolio-table.service.spec.ts index a9a565c1f2..76a6ea85d1 100644 --- a/apps/api/src/services/portfolio-table/portfolio-table.service.spec.ts +++ b/apps/api/src/services/portfolio-table/portfolio-table.service.spec.ts @@ -323,7 +323,6 @@ describe('PortfolioTableService', () => { it('gives the columns with a quantity or with a monetary value', () => { expect(PortfolioTableService.getHoldingsTableValueColumnNames()).toEqual([ 'Quantity', - 'Market Price', 'Value in Base Currency' ]); }); @@ -631,7 +630,7 @@ describe('PortfolioTableService', () => { expect(row).toMatch(/\| 75\.000% \|$/); }); - it('gives the quantity, the market price and the value with the values', async () => { + it('gives the quantity and the value with the values', async () => { const result = await getHoldingsTable([createHolding()], { withValues: true }); @@ -640,16 +639,23 @@ describe('PortfolioTableService', () => { return line.startsWith('| Name of AAPL'); }); - expect(row).toMatch(/\| 75\.000% \| 5 \| 400 \| 2000\.00 \|$/); + expect(row).toMatch(/\| 75\.000% \| 5 \| 2000\.00 \|$/); }); }); describe('getPerformanceTable', () => { function getPerformanceTable( parameters: Parameters[0] = {}, - withValues?: boolean + { + withAssetPerformanceInBaseCurrency, + withValues + }: { + withAssetPerformanceInBaseCurrency?: boolean; + withValues?: boolean; + } = {} ) { return createPortfolioTableService(parameters).getPerformanceTable({ + withAssetPerformanceInBaseCurrency, withValues, dateRange: 'ytd', userId: 'user-id' @@ -694,7 +700,10 @@ describe('PortfolioTableService', () => { }); it('gives the current value and the performance in base currency with the values', async () => { - const result = await getPerformanceTable({}, true); + const result = await getPerformanceTable( + {}, + { withAssetPerformanceInBaseCurrency: true, withValues: true } + ); const [row] = result.split('\n').filter((line) => { return line.startsWith('| 10.000%'); @@ -705,6 +714,18 @@ describe('PortfolioTableService', () => { ); }); + it('gives no asset performance and no currency performance in base currency without the asset performance in base currency', async () => { + const result = await getPerformanceTable({}, { withValues: true }); + + const [row] = result.split('\n').filter((line) => { + return line.startsWith('| 10.000%'); + }); + + expect(result).not.toContain('Asset Performance in Base Currency'); + expect(result).not.toContain('Currency Performance in Base Currency'); + expect(row).toBe('| 10.000% | 5.000% | 15.000% | 2000.00 | 300.00 |'); + }); + it('gives a currency performance in base currency of zero without a sign', async () => { const result = await getPerformanceTable( { @@ -713,7 +734,7 @@ describe('PortfolioTableService', () => { netPerformanceWithCurrencyEffect: 200 }) }, - true + { withAssetPerformanceInBaseCurrency: true, withValues: true } ); const [row] = result.split('\n').filter((line) => { diff --git a/apps/api/src/services/portfolio-table/portfolio-table.service.ts b/apps/api/src/services/portfolio-table/portfolio-table.service.ts index 33767bb62a..dc9db2926a 100644 --- a/apps/api/src/services/portfolio-table/portfolio-table.service.ts +++ b/apps/api/src/services/portfolio-table/portfolio-table.service.ts @@ -31,6 +31,12 @@ import { format } from 'date-fns'; import { DataSourceTableContext } from './interfaces/data-source-table-context.interface'; import { HoldingsTableColumnDefinition } from './types/holdings-table-column-definition.type'; +const ASSET_PERFORMANCE_IN_BASE_CURRENCY_COLUMN_NAME = + 'Asset Performance in Base Currency'; + +const CURRENCY_PERFORMANCE_IN_BASE_CURRENCY_COLUMN_NAME = + 'Currency Performance in Base Currency'; + const DATA_SOURCE_COLUMN_NAME = 'Data Source'; function getDataSourceColumnDefinition( @@ -281,13 +287,6 @@ export class PortfolioTableService { }, name: 'Quantity' }, - { - align: 'right', - getValue: ({ marketPrice }) => { - return marketPrice.toString(); - }, - name: 'Market Price' - }, { align: 'right', getValue: ({ valueInBaseCurrency }) => { @@ -348,14 +347,14 @@ export class PortfolioTableService { getValue: ({ netPerformance }) => { return getAmount(netPerformance); }, - name: 'Asset Performance in Base Currency' + name: ASSET_PERFORMANCE_IN_BASE_CURRENCY_COLUMN_NAME }, { align: 'right', getValue: ({ netPerformance, netPerformanceWithCurrencyEffect }) => { return getAmount(netPerformanceWithCurrencyEffect - netPerformance); }, - name: 'Currency Performance in Base Currency' + name: CURRENCY_PERFORMANCE_IN_BASE_CURRENCY_COLUMN_NAME }, { align: 'right', @@ -665,11 +664,13 @@ export class PortfolioTableService { dateRange, filters, userId, + withAssetPerformanceInBaseCurrency = false, withValues = false }: { dateRange: DateRange; filters?: Filter[]; userId: string; + withAssetPerformanceInBaseCurrency?: boolean; withValues?: boolean; }) { const { chart, performance } = await this.portfolioService.getPerformance({ @@ -689,6 +690,14 @@ export class PortfolioTableService { PortfolioTableService.PERFORMANCE_TABLE_COLUMN_DEFINITIONS, valueColumnDefinitions: PortfolioTableService.PERFORMANCE_TABLE_VALUE_COLUMN_DEFINITIONS + }).filter(({ name }) => { + return ( + withAssetPerformanceInBaseCurrency || + ![ + ASSET_PERFORMANCE_IN_BASE_CURRENCY_COLUMN_NAME, + CURRENCY_PERFORMANCE_IN_BASE_CURRENCY_COLUMN_NAME + ].includes(name) + ); }), rows: [performance] }) diff --git a/libs/common/src/lib/scopes.ts b/libs/common/src/lib/scopes.ts index f3f78eadf4..39b1954fc2 100644 --- a/libs/common/src/lib/scopes.ts +++ b/libs/common/src/lib/scopes.ts @@ -74,8 +74,8 @@ const SCOPES_OF_TYPE: Record = { }; /** - * Access types which combine a write scope with the restricted read access, - * because their tools change data without exposing the monetary values + * Access types which can combine a write scope with the restricted read access, + * because their tools which change data do not expose the monetary values */ export function canGrantRestrictedWriteAccess({ type }: { type: AccessType }) { return type === 'MCP';