diff --git a/apps/api/src/app/activities/activities.controller.ts b/apps/api/src/app/activities/activities.controller.ts index 12696472d..63357e6b4 100644 --- a/apps/api/src/app/activities/activities.controller.ts +++ b/apps/api/src/app/activities/activities.controller.ts @@ -1,4 +1,3 @@ -import { UserService } from '@ghostfolio/api/app/user/user.service'; import { HasPermission } from '@ghostfolio/api/decorators/has-permission.decorator'; import { HasPermissionGuard } from '@ghostfolio/api/guards/has-permission.guard'; import { isActivityInFuture } from '@ghostfolio/api/helper/activity.helper'; @@ -8,16 +7,19 @@ import { TransformDataSourceInResponseInterceptor } from '@ghostfolio/api/interc import { ApiService } from '@ghostfolio/api/services/api/api.service'; import { DataProviderService } from '@ghostfolio/api/services/data-provider/data-provider.service'; import { ImpersonationService } from '@ghostfolio/api/services/impersonation/impersonation.service'; +import { PrismaService } from '@ghostfolio/api/services/prisma/prisma.service'; import { DataGatheringService } from '@ghostfolio/api/services/queues/data-gathering/data-gathering.service'; import { getIntervalFromDateRange } from '@ghostfolio/common/calculation-helper'; import { DATA_GATHERING_QUEUE_PRIORITY_HIGH, + DEFAULT_CURRENCY, HEADER_KEY_IMPERSONATION } from '@ghostfolio/common/config'; import { CreateOrderDto, UpdateOrderDto } from '@ghostfolio/common/dtos'; import { ActivitiesResponse, - ActivityResponse + ActivityResponse, + UserSettings } from '@ghostfolio/common/interfaces'; import { permissions } from '@ghostfolio/common/permissions'; import type { RequestWithUser } from '@ghostfolio/common/types'; @@ -55,8 +57,8 @@ export class ActivitiesController { private readonly dataProviderService: DataProviderService, private readonly dataGatheringService: DataGatheringService, private readonly impersonationService: ImpersonationService, - @Inject(REQUEST) private readonly request: RequestWithUser, - private readonly userService: UserService + private readonly prismaService: PrismaService, + @Inject(REQUEST) private readonly request: RequestWithUser ) {} @Delete() @@ -173,7 +175,7 @@ export class ActivitiesController { await this.impersonationService.validateImpersonationId(impersonationId); const userId = impersonationUserId || this.request.user.id; - const { settings } = await this.userService.user({ id: userId }); + const userCurrency = await this.getUserCurrency(impersonationUserId); const { activities, count } = await this.activitiesService.getActivities({ endDate, @@ -183,10 +185,10 @@ export class ActivitiesController { sortDirection, startDate, take, + userCurrency, userId, includeDrafts: true, types: activityTypes, - userCurrency: settings.settings.baseCurrency, withExcludedAccountsAndActivities: true }); @@ -205,12 +207,12 @@ export class ActivitiesController { await this.impersonationService.validateImpersonationId(impersonationId); const userId = impersonationUserId || this.request.user.id; - const { settings } = await this.userService.user({ id: userId }); + const userCurrency = await this.getUserCurrency(impersonationUserId); const { activities } = await this.activitiesService.getActivities({ + userCurrency, userId, includeDrafts: true, - userCurrency: settings.settings.baseCurrency, withExcludedAccountsAndActivities: true }); @@ -382,4 +384,18 @@ export class ActivitiesController { } }); } + + private async getUserCurrency(impersonationUserId: string) { + if (!impersonationUserId) { + return this.request.user.settings.settings.baseCurrency; + } + + const settings = await this.prismaService.settings.findUnique({ + where: { userId: impersonationUserId } + }); + + return ( + (settings?.settings as UserSettings)?.baseCurrency ?? DEFAULT_CURRENCY + ); + } } diff --git a/apps/api/src/app/activities/activities.module.ts b/apps/api/src/app/activities/activities.module.ts index fb0d3c4b0..34091ba5e 100644 --- a/apps/api/src/app/activities/activities.module.ts +++ b/apps/api/src/app/activities/activities.module.ts @@ -2,7 +2,6 @@ import { AccountBalanceService } from '@ghostfolio/api/app/account-balance/accou import { AccountService } from '@ghostfolio/api/app/account/account.service'; import { CacheModule } from '@ghostfolio/api/app/cache/cache.module'; import { RedisCacheModule } from '@ghostfolio/api/app/redis-cache/redis-cache.module'; -import { UserModule } from '@ghostfolio/api/app/user/user.module'; import { RedactValuesInResponseModule } from '@ghostfolio/api/interceptors/redact-values-in-response/redact-values-in-response.module'; import { TransformDataSourceInRequestModule } from '@ghostfolio/api/interceptors/transform-data-source-in-request/transform-data-source-in-request.module'; import { TransformDataSourceInResponseModule } from '@ghostfolio/api/interceptors/transform-data-source-in-response/transform-data-source-in-response.module'; @@ -40,8 +39,7 @@ import { ActivitiesService } from './activities.service'; SymbolProfileModule, TagModule, TransformDataSourceInRequestModule, - TransformDataSourceInResponseModule, - UserModule + TransformDataSourceInResponseModule ], providers: [AccountBalanceService, AccountService, ActivitiesService] }) diff --git a/apps/api/src/app/user/user.service.ts b/apps/api/src/app/user/user.service.ts index 6fb76cbdc..ada59f460 100644 --- a/apps/api/src/app/user/user.service.ts +++ b/apps/api/src/app/user/user.service.ts @@ -59,7 +59,7 @@ import { PerformanceCalculationType } from '@ghostfolio/common/types/performance import { Injectable, Logger } from '@nestjs/common'; import { EventEmitter2 } from '@nestjs/event-emitter'; import { InjectThrottlerStorage, ThrottlerStorage } from '@nestjs/throttler'; -import { Prisma, Role, Settings, User } from '@prisma/client'; +import { Prisma, Role, User } from '@prisma/client'; import { differenceInDays, subDays } from 'date-fns'; import { isNil, without } from 'lodash'; import { createHmac } from 'node:crypto'; @@ -128,7 +128,7 @@ export class UserService { accounts, activitiesCount, firstActivity, - impersonationUserSettings, + impersonationUser, tagsForUser ] = await Promise.all([ this.prismaService.access.findMany({ @@ -157,16 +157,14 @@ export class UserService { where: { userId: impersonationUserId || user.id } }), impersonationUserId - ? this.prismaService.settings.findUnique({ - where: { userId: impersonationUserId } - }) - : Promise.resolve(null), + ? this.user({ id: impersonationUserId }) + : Promise.resolve(null), this.tagService.getTagsForUser(impersonationUserId || user.id) ]); const resolvedUserSettings = resolveUserSettings({ impersonationUserSettings: impersonationUserId - ? ((impersonationUserSettings?.settings ?? {}) as UserSettings) + ? ((impersonationUser?.settings?.settings ?? {}) as UserSettings) : undefined, userSettings: settings.settings as UserSettings }); diff --git a/apps/client/src/app/pages/portfolio/analysis/analysis-page.component.ts b/apps/client/src/app/pages/portfolio/analysis/analysis-page.component.ts index 7a5fb8578..2c34dbcd7 100644 --- a/apps/client/src/app/pages/portfolio/analysis/analysis-page.component.ts +++ b/apps/client/src/app/pages/portfolio/analysis/analysis-page.component.ts @@ -88,7 +88,6 @@ export class GfAnalysisPageComponent implements OnInit { protected dividendsByGroup: InvestmentItem[]; protected readonly dividendTimelineDataLabel = $localize`Dividend`; protected hasPermissionToReadAiPrompt: boolean; - protected hasPermissionToUpdateUserSettings: boolean; protected impersonationId: string | null; protected investments: InvestmentItem[]; protected readonly investmentTimelineDataLabel = $localize`Invested Capital`; @@ -175,11 +174,6 @@ export class GfAnalysisPageComponent implements OnInit { permissions.readAiPrompt ); - this.hasPermissionToUpdateUserSettings = hasPermission( - this.user.permissions, - permissions.updateUserSettings - ); - this.update(); } diff --git a/apps/client/src/app/pages/portfolio/analysis/analysis-page.html b/apps/client/src/app/pages/portfolio/analysis/analysis-page.html index a3e07a4e2..3d22f0c68 100644 --- a/apps/client/src/app/pages/portfolio/analysis/analysis-page.html +++ b/apps/client/src/app/pages/portfolio/analysis/analysis-page.html @@ -137,9 +137,7 @@ [benchmarkDataItems]="benchmarkDataItems" [benchmarks]="benchmarks" [colorScheme]="user?.settings?.colorScheme" - [hasPermissionToUpdateUserSettings]=" - hasPermissionToUpdateUserSettings && !impersonationId - " + [hasPermissionToUpdateUserSettings]="!impersonationId" [isLoading]="isLoadingBenchmarkComparator || isLoadingInvestmentChart" [locale]="user?.settings?.locale" [performanceDataItems]="performanceDataItemsInPercentage" diff --git a/libs/common/src/lib/helper.spec.ts b/libs/common/src/lib/helper.spec.ts index fbaccb1bd..db3f9677d 100644 --- a/libs/common/src/lib/helper.spec.ts +++ b/libs/common/src/lib/helper.spec.ts @@ -441,6 +441,35 @@ describe('Helper', () => { }); }); + it('Filters stay with the authenticated user', () => { + // The filters are always written back to the authenticated user, so + // reading them from the impersonated user would overwrite them + const { 'filters.accounts': filtersAccounts } = resolveUserSettings({ + impersonationUserSettings: { + 'filters.accounts': ['3b3c2b5d-5a4f-4b0a-9d4f-9b1f5e6a7c8d'] + }, + userSettings: { + 'filters.accounts': ['0a1b2c3d-4e5f-6a7b-8c9d-0e1f2a3b4c5d'] + } + }); + + expect(filtersAccounts).toEqual(['0a1b2c3d-4e5f-6a7b-8c9d-0e1f2a3b4c5d']); + }); + + it('Presentation settings unset for the authenticated user do not leak', () => { + // An unset presentation setting must not fall back to the impersonated + // user, otherwise their appearance and language apply to the + // authenticated user + const { colorScheme, language, locale } = resolveUserSettings({ + impersonationUserSettings, + userSettings: { baseCurrency: 'CHF' } + }); + + expect(colorScheme).toBeUndefined(); + expect(language).toBeUndefined(); + expect(locale).toBeUndefined(); + }); + it('Unknown settings default to the impersonated user', () => { // A setting which is not classified as presentation must not leak from // the authenticated user into the impersonated portfolio diff --git a/libs/common/src/lib/helper.ts b/libs/common/src/lib/helper.ts index d4920bac3..2e4125ea9 100644 --- a/libs/common/src/lib/helper.ts +++ b/libs/common/src/lib/helper.ts @@ -33,7 +33,7 @@ import { uk, zhCN } from 'date-fns/locale'; -import { get, isNil, isString, pick } from 'lodash'; +import { get, isNil, isString } from 'lodash'; import { DEFAULT_CURRENCY, @@ -63,9 +63,17 @@ export const DATE_FORMAT_YEARLY = 'yyyy'; // Settings which describe the person looking at the screen rather than the // portfolio being looked at. They stay with the authenticated user while // impersonating. Every other setting follows the impersonated user. +// The filters are included because they are always written back to the +// authenticated user, so reading them from the impersonated user would +// overwrite the filters of the authenticated user. const PRESENTATION_USER_SETTINGS_KEYS: (keyof UserSettings)[] = [ 'colorScheme', 'dateRange', + 'filters.accounts', + 'filters.assetClasses', + 'filters.dataSource', + 'filters.symbol', + 'filters.tags', 'holdingsViewMode', 'isExperimentalFeatures', 'isRestrictedView', @@ -699,6 +707,10 @@ export function resolveUserSettings({ return { ...impersonationUserSettings, - ...pick(userSettings ?? {}, PRESENTATION_USER_SETTINGS_KEYS) + ...Object.fromEntries( + PRESENTATION_USER_SETTINGS_KEYS.map((key) => { + return [key, userSettings?.[key]]; + }) + ) }; }