From 6f01b52194abd8edf5b3a25ad018ee8577ef8d26 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 14 Feb 2026 23:22:14 +0000 Subject: [PATCH] Fix critical and high severity issues from code review - Fix [class] binding overwriting CSS classes (use [ngClass] instead) - Fix activities fetch returning wrong data for older dates (use year range) - Add @HasPermission(readJournalEntry) to GET endpoints - Apply delta adjustment to netPerformanceInPercentage (was cumulative) - Fix realized profit to only include dividend/interest income (exclude SELL gross proceeds) - Fix formatCurrency hiding $0 values (only hide null/undefined) - Localize monthLabel, weekday headers, and dateLabel using Intl.DateTimeFormat - Add snackbar feedback on note save/delete success and failure - Remove unused date field from DTO, add @IsNotEmpty to note - Add DEFAULT_CURRENCY fallback for userCurrency - Add month/year range validation - Fix January baseline by fetching previous year performance - Add maxWidth to dialog for mobile - Only reload calendar data when dialog reports changes - Remove unused GfActivitiesTableComponent and user subscription from dialog - Handle empty note save gracefully (skip API call if no existing note) - Add dark mode support to dialog SCSS - Add route title for browser tab https://claude.ai/code/session_01XEieh1hHrXc1fcbnA7oBHn --- .../api/src/app/journal/journal.controller.ts | 57 ++++++++++++---- .../journal-day-dialog.component.html | 8 +-- .../journal-day-dialog.component.scss | 32 +++++++++ .../journal-day-dialog.component.ts | 65 +++++++++++++------ .../journal/journal-page.component.ts | 7 +- .../portfolio/journal/journal-page.routes.ts | 5 +- .../create-or-update-journal-entry.dto.ts | 6 +- libs/common/src/lib/permissions.ts | 3 + .../journal-calendar.component.html | 8 ++- .../journal-calendar.component.ts | 47 ++++++++++---- libs/ui/src/lib/services/data.service.ts | 2 +- 11 files changed, 183 insertions(+), 57 deletions(-) diff --git a/apps/api/src/app/journal/journal.controller.ts b/apps/api/src/app/journal/journal.controller.ts index 36dd5ea12..91e9d5134 100644 --- a/apps/api/src/app/journal/journal.controller.ts +++ b/apps/api/src/app/journal/journal.controller.ts @@ -2,6 +2,7 @@ import { OrderService } from '@ghostfolio/api/app/order/order.service'; import { PortfolioService } from '@ghostfolio/api/app/portfolio/portfolio.service'; import { HasPermission } from '@ghostfolio/api/decorators/has-permission.decorator'; import { HasPermissionGuard } from '@ghostfolio/api/guards/has-permission.guard'; +import { DEFAULT_CURRENCY } from '@ghostfolio/common/config'; import { CreateOrUpdateJournalEntryDto } from '@ghostfolio/common/dtos'; import { DATE_FORMAT, resetHours } from '@ghostfolio/common/helper'; import { @@ -25,7 +26,13 @@ import { } from '@nestjs/common'; import { REQUEST } from '@nestjs/core'; import { AuthGuard } from '@nestjs/passport'; -import { endOfMonth, format, parseISO, startOfMonth } from 'date-fns'; +import { + endOfMonth, + format, + parseISO, + startOfMonth, + subMonths +} from 'date-fns'; import { StatusCodes, getReasonPhrase } from 'http-status-codes'; import { JournalService } from './journal.service'; @@ -40,6 +47,7 @@ export class JournalController { ) {} @Get() + @HasPermission(permissions.readJournalEntry) @UseGuards(AuthGuard('jwt'), HasPermissionGuard) public async getJournal( @Query('month') month: string, @@ -49,16 +57,31 @@ export class JournalController { const monthNum = parseInt(month, 10) - 1; const yearNum = parseInt(year, 10); - if (isNaN(monthNum) || isNaN(yearNum)) { + if ( + isNaN(monthNum) || + isNaN(yearNum) || + monthNum < 0 || + monthNum > 11 || + yearNum < 1970 || + yearNum > 2100 + ) { throw new HttpException( getReasonPhrase(StatusCodes.BAD_REQUEST), StatusCodes.BAD_REQUEST ); } + const userCurrency = + this.request.user.settings?.settings?.baseCurrency ?? DEFAULT_CURRENCY; + const startDate = startOfMonth(new Date(yearNum, monthNum)); const endDate = endOfMonth(new Date(yearNum, monthNum)); + // Fetch performance with a baseline that includes the previous month's + // last day, so January deltas are computed correctly + const performanceFetchStart = subMonths(startDate, 1); + const performanceYear = performanceFetchStart.getFullYear(); + const [journalEntries, performanceResponse, activitiesResponse] = await Promise.all([ this.journalService.getJournalEntries({ @@ -67,17 +90,18 @@ export class JournalController { userId }), this.portfolioService.getPerformance({ - dateRange: `${yearNum}`, + dateRange: + `${performanceYear}` === `${yearNum}` ? `${yearNum}` : 'max', filters: [], impersonationId: undefined, userId }), this.orderService.getOrders({ - startDate, endDate, userId, - userCurrency: this.request.user.settings?.settings?.baseCurrency, + userCurrency, includeDrafts: false, + startDate: startDate, withExcludedAccountsAndActivities: false }) ]); @@ -104,26 +128,35 @@ export class JournalController { } } - // Calculate daily performance deltas + // Calculate daily performance deltas (both absolute and percentage) const sortedDates = Array.from(daysMap.keys()).sort(); let previousNetPerformance = 0; + let previousNetPerformanceInPercentage = 0; // Find the chart item just before our month starts to get the baseline for (const item of chart) { const itemDate = parseISO(item.date); if (itemDate < startDate) { previousNetPerformance = item.netPerformance ?? 0; + previousNetPerformanceInPercentage = + item.netPerformanceInPercentage ?? 0; } } for (const dateKey of sortedDates) { const day = daysMap.get(dateKey); + const currentNetPerformance = day.netPerformance; day.netPerformance = currentNetPerformance - previousNetPerformance; previousNetPerformance = currentNetPerformance; + + const currentNetPerformanceInPercentage = day.netPerformanceInPercentage; + day.netPerformanceInPercentage = + currentNetPerformanceInPercentage - previousNetPerformanceInPercentage; + previousNetPerformanceInPercentage = currentNetPerformanceInPercentage; } - // Count activities per day and calculate realized profit + // Count activities per day and track dividend/interest income for (const activity of activities) { const dateKey = format(activity.date, DATE_FORMAT); let day = daysMap.get(dateKey); @@ -143,11 +176,10 @@ export class JournalController { day.activitiesCount++; - if ( - activity.type === 'SELL' || - activity.type === 'DIVIDEND' || - activity.type === 'INTEREST' - ) { + // Track dividend and interest income as realized profit. + // SELL activities use gross proceeds (not actual realized gain), + // so we exclude them to avoid misleading values. + if (activity.type === 'DIVIDEND' || activity.type === 'INTEREST') { day.realizedProfit += activity.valueInBaseCurrency ?? activity.value ?? 0; } @@ -173,6 +205,7 @@ export class JournalController { } @Get(':date') + @HasPermission(permissions.readJournalEntry) @UseGuards(AuthGuard('jwt'), HasPermissionGuard) public async getJournalEntry(@Param('date') dateString: string) { const userId = this.request.user.id; diff --git a/apps/client/src/app/pages/portfolio/journal/journal-day-dialog/journal-day-dialog.component.html b/apps/client/src/app/pages/portfolio/journal/journal-day-dialog/journal-day-dialog.component.html index 69ed8f622..118aac886 100644 --- a/apps/client/src/app/pages/portfolio/journal/journal-day-dialog/journal-day-dialog.component.html +++ b/apps/client/src/app/pages/portfolio/journal/journal-day-dialog/journal-day-dialog.component.html @@ -1,7 +1,7 @@
@@ -20,7 +20,7 @@ @if (data.realizedProfit !== 0) {
-
Realized P&L
+
Dividend & Interest
{{ activity.type }} @@ -119,5 +119,5 @@ diff --git a/apps/client/src/app/pages/portfolio/journal/journal-day-dialog/journal-day-dialog.component.scss b/apps/client/src/app/pages/portfolio/journal/journal-day-dialog/journal-day-dialog.component.scss index 8068cfa81..366691a88 100644 --- a/apps/client/src/app/pages/portfolio/journal/journal-day-dialog/journal-day-dialog.component.scss +++ b/apps/client/src/app/pages/portfolio/journal/journal-day-dialog/journal-day-dialog.component.scss @@ -63,3 +63,35 @@ color: #f97316; } } + +// Dark mode support +:host-context(.is-dark-theme) { + .activities-list { + table { + --border-color: rgba(255, 255, 255, 0.12); + + td { + --border-color: rgba(255, 255, 255, 0.06); + } + } + } + + .activity-type { + &.buy { + background-color: rgba(34, 197, 94, 0.2); + } + + &.sell { + background-color: rgba(239, 68, 68, 0.2); + } + + &.dividend, + &.interest { + background-color: rgba(59, 130, 246, 0.2); + } + + &.fee { + background-color: rgba(249, 115, 22, 0.2); + } + } +} diff --git a/apps/client/src/app/pages/portfolio/journal/journal-day-dialog/journal-day-dialog.component.ts b/apps/client/src/app/pages/portfolio/journal/journal-day-dialog/journal-day-dialog.component.ts index d83ad83dd..590b024f0 100644 --- a/apps/client/src/app/pages/portfolio/journal/journal-day-dialog/journal-day-dialog.component.ts +++ b/apps/client/src/app/pages/portfolio/journal/journal-day-dialog/journal-day-dialog.component.ts @@ -1,7 +1,5 @@ -import { UserService } from '@ghostfolio/client/services/user/user.service'; import { DATE_FORMAT } from '@ghostfolio/common/helper'; -import { Activity, User } from '@ghostfolio/common/interfaces'; -import { GfActivitiesTableComponent } from '@ghostfolio/ui/activities-table'; +import { Activity } from '@ghostfolio/common/interfaces'; import { GfDialogFooterComponent } from '@ghostfolio/ui/dialog-footer'; import { GfDialogHeaderComponent } from '@ghostfolio/ui/dialog-header'; import { DataService } from '@ghostfolio/ui/services'; @@ -25,6 +23,7 @@ import { } from '@angular/material/dialog'; import { MatFormFieldModule } from '@angular/material/form-field'; import { MatInputModule } from '@angular/material/input'; +import { MatSnackBar, MatSnackBarModule } from '@angular/material/snack-bar'; import { format, parseISO } from 'date-fns'; import { Subject } from 'rxjs'; import { takeUntil } from 'rxjs/operators'; @@ -45,14 +44,14 @@ export interface JournalDayDialogData { imports: [ CommonModule, FormsModule, - GfActivitiesTableComponent, GfDialogFooterComponent, GfDialogHeaderComponent, GfValueComponent, MatButtonModule, MatDialogModule, MatFormFieldModule, - MatInputModule + MatInputModule, + MatSnackBarModule ], selector: 'gf-journal-day-dialog', styleUrls: ['./journal-day-dialog.component.scss'], @@ -62,12 +61,13 @@ export class GfJournalDayDialogComponent implements OnInit, OnDestroy { public activities: Activity[] = []; public date: string; public dateLabel: string; + public hasChanges = false; public isLoadingActivities = true; public isLoadingNote = true; public isSavingNote = false; public note = ''; - public user: User; + private originalNote = ''; private unsubscribeSubject = new Subject(); public constructor( @@ -75,21 +75,17 @@ export class GfJournalDayDialogComponent implements OnInit, OnDestroy { @Inject(MAT_DIALOG_DATA) public data: JournalDayDialogData, private dataService: DataService, public dialogRef: MatDialogRef, - private userService: UserService + private snackBar: MatSnackBar ) {} public ngOnInit() { this.date = this.data.date; - this.dateLabel = format(parseISO(this.date), 'EEEE, MMMM d, yyyy'); - - this.userService.stateChanged - .pipe(takeUntil(this.unsubscribeSubject)) - .subscribe((state) => { - if (state?.user) { - this.user = state.user; - this.changeDetectorRef.markForCheck(); - } - }); + this.dateLabel = new Intl.DateTimeFormat(this.data.locale ?? 'en-US', { + weekday: 'long', + year: 'numeric', + month: 'long', + day: 'numeric' + }).format(parseISO(this.date)); this.loadActivities(); this.loadNote(); @@ -108,31 +104,55 @@ export class GfJournalDayDialogComponent implements OnInit, OnDestroy { .pipe(takeUntil(this.unsubscribeSubject)) .subscribe({ next: () => { + this.hasChanges = true; + this.originalNote = this.note.trim(); this.isSavingNote = false; + this.snackBar.open($localize`Note saved`, undefined, { + duration: 3000 + }); this.changeDetectorRef.markForCheck(); }, error: () => { this.isSavingNote = false; + this.snackBar.open($localize`Failed to save note`, undefined, { + duration: 5000 + }); this.changeDetectorRef.markForCheck(); } }); - } else { + } else if (this.originalNote) { + // Only call delete if there was an existing note this.dataService .deleteJournalEntry({ date: this.date }) .pipe(takeUntil(this.unsubscribeSubject)) .subscribe({ next: () => { + this.hasChanges = true; + this.originalNote = ''; this.isSavingNote = false; + this.snackBar.open($localize`Note deleted`, undefined, { + duration: 3000 + }); this.changeDetectorRef.markForCheck(); }, error: () => { this.isSavingNote = false; + this.snackBar.open($localize`Failed to delete note`, undefined, { + duration: 5000 + }); this.changeDetectorRef.markForCheck(); } }); + } else { + // No existing note and empty input - nothing to do + this.isSavingNote = false; } } + public onClose() { + this.dialogRef.close({ hasChanges: this.hasChanges }); + } + public ngOnDestroy() { this.unsubscribeSubject.next(); this.unsubscribeSubject.complete(); @@ -141,11 +161,16 @@ export class GfJournalDayDialogComponent implements OnInit, OnDestroy { private loadActivities() { this.isLoadingActivities = true; + // Use the year of the selected date as range filter to avoid + // fetching all-time activities and missing older dates + const year = this.date.substring(0, 4); + this.dataService .fetchActivities({ filters: [], + range: year as any, skip: 0, - take: 50, + take: 500, sortColumn: 'date', sortDirection: 'desc' }) @@ -174,11 +199,13 @@ export class GfJournalDayDialogComponent implements OnInit, OnDestroy { .subscribe({ next: (entry) => { this.note = entry?.note ?? ''; + this.originalNote = this.note; this.isLoadingNote = false; this.changeDetectorRef.markForCheck(); }, error: () => { this.note = ''; + this.originalNote = ''; this.isLoadingNote = false; this.changeDetectorRef.markForCheck(); } diff --git a/apps/client/src/app/pages/portfolio/journal/journal-page.component.ts b/apps/client/src/app/pages/portfolio/journal/journal-page.component.ts index d1eb2263e..e62b03856 100644 --- a/apps/client/src/app/pages/portfolio/journal/journal-page.component.ts +++ b/apps/client/src/app/pages/portfolio/journal/journal-page.component.ts @@ -89,14 +89,17 @@ export class JournalPageComponent implements OnInit, OnDestroy { realizedProfit: dayData?.realizedProfit ?? 0 } as JournalDayDialogData, height: this.deviceType === 'mobile' ? '98vh' : '80vh', + maxWidth: this.deviceType === 'mobile' ? '100vw' : '50rem', width: this.deviceType === 'mobile' ? '100vw' : '50rem' }); dialogRef .afterClosed() .pipe(takeUntil(this.unsubscribeSubject)) - .subscribe(() => { - this.loadJournalData(); + .subscribe((result) => { + if (result?.hasChanges) { + this.loadJournalData(); + } }); } diff --git a/apps/client/src/app/pages/portfolio/journal/journal-page.routes.ts b/apps/client/src/app/pages/portfolio/journal/journal-page.routes.ts index 7a97f7256..744c45f42 100644 --- a/apps/client/src/app/pages/portfolio/journal/journal-page.routes.ts +++ b/apps/client/src/app/pages/portfolio/journal/journal-page.routes.ts @@ -1,3 +1,5 @@ +import { internalRoutes } from '@ghostfolio/common/routes/routes'; + import { Routes } from '@angular/router'; import { JournalPageComponent } from './journal-page.component'; @@ -5,6 +7,7 @@ import { JournalPageComponent } from './journal-page.component'; export const routes: Routes = [ { component: JournalPageComponent, - path: '' + path: '', + title: internalRoutes.portfolio.subRoutes.journal.title } ]; diff --git a/libs/common/src/lib/dtos/create-or-update-journal-entry.dto.ts b/libs/common/src/lib/dtos/create-or-update-journal-entry.dto.ts index 627291664..d12c3265b 100644 --- a/libs/common/src/lib/dtos/create-or-update-journal-entry.dto.ts +++ b/libs/common/src/lib/dtos/create-or-update-journal-entry.dto.ts @@ -1,9 +1,7 @@ -import { IsISO8601, IsString, MaxLength } from 'class-validator'; +import { IsNotEmpty, IsString, MaxLength } from 'class-validator'; export class CreateOrUpdateJournalEntryDto { - @IsISO8601() - date: string; - + @IsNotEmpty() @IsString() @MaxLength(10000) note: string; diff --git a/libs/common/src/lib/permissions.ts b/libs/common/src/lib/permissions.ts index 21f5e64f3..41fb1220a 100644 --- a/libs/common/src/lib/permissions.ts +++ b/libs/common/src/lib/permissions.ts @@ -43,6 +43,7 @@ export const permissions = { enableSystemMessage: 'enableSystemMessage', impersonateAllUsers: 'impersonateAllUsers', readAiPrompt: 'readAiPrompt', + readJournalEntry: 'readJournalEntry', readMarketData: 'readMarketData', readMarketDataOfMarkets: 'readMarketDataOfMarkets', readMarketDataOfOwnAssetProfile: 'readMarketDataOfOwnAssetProfile', @@ -96,6 +97,7 @@ export function getPermissions(aRole: Role): string[] { permissions.deleteTag, permissions.deleteUser, permissions.readAiPrompt, + permissions.readJournalEntry, permissions.readMarketData, permissions.readMarketDataOfOwnAssetProfile, permissions.readPlatforms, @@ -144,6 +146,7 @@ export function getPermissions(aRole: Role): string[] { permissions.deleteOrder, permissions.deleteWatchlistItem, permissions.readAiPrompt, + permissions.readJournalEntry, permissions.readMarketDataOfOwnAssetProfile, permissions.readPlatforms, permissions.readWatchlist, diff --git a/libs/ui/src/lib/journal-calendar/journal-calendar.component.html b/libs/ui/src/lib/journal-calendar/journal-calendar.component.html index e96c00765..7b7e52e08 100644 --- a/libs/ui/src/lib/journal-calendar/journal-calendar.component.html +++ b/libs/ui/src/lib/journal-calendar/journal-calendar.component.html @@ -101,7 +101,7 @@ class="calendar-cell day-cell" [class.clickable]="day.isCurrentMonth" [class.today]="day.isToday" - [class]="getDayClass(day)" + [ngClass]="getDayClass(day)" (click)="onDayClick(day)" >
{{ day.dayOfMonth }}
@@ -111,8 +111,10 @@
@if (day.data.activitiesCount > 0) {
- {{ day.data.activitiesCount }} - {{ day.data.activitiesCount === 1 ? 'trade' : 'trades' }} + {day.data.activitiesCount, plural, + =1 {1 activity} + other {{{day.data.activitiesCount}} activities} + }
} @if (day.data.hasNote) { diff --git a/libs/ui/src/lib/journal-calendar/journal-calendar.component.ts b/libs/ui/src/lib/journal-calendar/journal-calendar.component.ts index b30785e3e..24c62b8ae 100644 --- a/libs/ui/src/lib/journal-calendar/journal-calendar.component.ts +++ b/libs/ui/src/lib/journal-calendar/journal-calendar.component.ts @@ -10,7 +10,8 @@ import { EventEmitter, Input, OnChanges, - Output + Output, + SimpleChanges } from '@angular/core'; import { MatButtonModule } from '@angular/material/button'; import { MatCardModule } from '@angular/material/card'; @@ -21,6 +22,7 @@ import { endOfMonth, format, getDay, + isToday, startOfMonth, subMonths } from 'date-fns'; @@ -58,10 +60,13 @@ export class GfJournalCalendarComponent implements OnChanges { @Output() monthChanged = new EventEmitter<{ month: number; year: number }>(); public calendarWeeks: CalendarDay[][] = []; - public weekDays = ['Sun', 'Mon', 'Tue', 'Wed', 'Thu', 'Fri', 'Sat']; + public weekDays: string[] = []; public get monthLabel(): string { - return format(new Date(this.year, this.month - 1), 'MMMM yyyy'); + return new Intl.DateTimeFormat(this.locale ?? 'en-US', { + month: 'long', + year: 'numeric' + }).format(new Date(this.year, this.month - 1)); } public get winRate(): number { @@ -82,8 +87,14 @@ export class GfJournalCalendarComponent implements OnChanges { }); } - public ngOnChanges() { - this.buildCalendar(); + public ngOnChanges(changes: SimpleChanges) { + if (changes['days'] || changes['month'] || changes['year']) { + this.buildCalendar(); + } + + if (changes['locale']) { + this.buildWeekDays(); + } } public onPreviousMonth() { @@ -129,7 +140,7 @@ export class GfJournalCalendarComponent implements OnChanges { } public formatCurrency(value: number): string { - if (value === 0 || value === undefined || value === null) { + if (value === undefined || value === null) { return ''; } @@ -145,11 +156,28 @@ export class GfJournalCalendarComponent implements OnChanges { } } + private buildWeekDays() { + // Generate locale-aware weekday names (Sunday-start) + const baseDate = new Date(2024, 0, 7); // A known Sunday + + this.weekDays = Array.from({ length: 7 }, (_, i) => { + const day = new Date(baseDate); + day.setDate(baseDate.getDate() + i); + + return new Intl.DateTimeFormat(this.locale ?? 'en-US', { + weekday: 'short' + }).format(day); + }); + } + private buildCalendar() { + if (!this.weekDays.length) { + this.buildWeekDays(); + } + const currentDate = new Date(this.year, this.month - 1); const monthStart = startOfMonth(currentDate); const monthEnd = endOfMonth(currentDate); - const today = new Date(); const daysDataMap = new Map(); @@ -181,10 +209,7 @@ export class GfJournalCalendarComponent implements OnChanges { date, dayOfMonth: date.getDate(), isCurrentMonth: true, - isToday: - date.getDate() === today.getDate() && - date.getMonth() === today.getMonth() && - date.getFullYear() === today.getFullYear() + isToday: isToday(date) }); } diff --git a/libs/ui/src/lib/services/data.service.ts b/libs/ui/src/lib/services/data.service.ts index acee1281a..23d5cb686 100644 --- a/libs/ui/src/lib/services/data.service.ts +++ b/libs/ui/src/lib/services/data.service.ts @@ -432,7 +432,7 @@ export class DataService { } public putJournalEntry({ date, note }: { date: string; note: string }) { - return this.http.put(`/api/v1/journal/${date}`, { date, note }); + return this.http.put(`/api/v1/journal/${date}`, { note }); } public deleteJournalEntry({ date }: { date: string }) {