Browse Source

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
pull/6340/head
Claude 2 weeks ago
parent
commit
6f01b52194
Failed to extract signature
  1. 57
      apps/api/src/app/journal/journal.controller.ts
  2. 8
      apps/client/src/app/pages/portfolio/journal/journal-day-dialog/journal-day-dialog.component.html
  3. 32
      apps/client/src/app/pages/portfolio/journal/journal-day-dialog/journal-day-dialog.component.scss
  4. 65
      apps/client/src/app/pages/portfolio/journal/journal-day-dialog/journal-day-dialog.component.ts
  5. 7
      apps/client/src/app/pages/portfolio/journal/journal-page.component.ts
  6. 5
      apps/client/src/app/pages/portfolio/journal/journal-page.routes.ts
  7. 6
      libs/common/src/lib/dtos/create-or-update-journal-entry.dto.ts
  8. 3
      libs/common/src/lib/permissions.ts
  9. 8
      libs/ui/src/lib/journal-calendar/journal-calendar.component.html
  10. 47
      libs/ui/src/lib/journal-calendar/journal-calendar.component.ts
  11. 2
      libs/ui/src/lib/services/data.service.ts

57
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;

8
apps/client/src/app/pages/portfolio/journal/journal-day-dialog/journal-day-dialog.component.html

@ -1,7 +1,7 @@
<gf-dialog-header
[deviceType]="data.deviceType"
[title]="dateLabel"
(closeButtonClicked)="dialogRef.close()"
(closeButtonClicked)="onClose()"
/>
<div class="flex-grow-1 overflow-auto px-3" mat-dialog-content>
@ -20,7 +20,7 @@
@if (data.realizedProfit !== 0) {
<div class="performance-metric">
<div class="metric-label" i18n>Realized P&amp;L</div>
<div class="metric-label" i18n>Dividend &amp; Interest</div>
<gf-value
[colorizeSign]="true"
[isCurrency]="true"
@ -57,7 +57,7 @@
<td>
<span
class="activity-type"
[class]="activity.type?.toLowerCase()"
[ngClass]="activity.type?.toLowerCase()"
>
{{ activity.type }}
</span>
@ -119,5 +119,5 @@
<gf-dialog-footer
[deviceType]="data.deviceType"
(closeButtonClicked)="dialogRef.close()"
(closeButtonClicked)="onClose()"
/>

32
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);
}
}
}

65
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<void>();
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<GfJournalDayDialogComponent>,
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();
}

7
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();
}
});
}

5
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
}
];

6
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;

3
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,

8
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)"
>
<div class="day-number">{{ day.dayOfMonth }}</div>
@ -111,8 +111,10 @@
</div>
@if (day.data.activitiesCount > 0) {
<div class="day-activities">
{{ day.data.activitiesCount }}
{{ day.data.activitiesCount === 1 ? 'trade' : 'trades' }}
<span i18n>{day.data.activitiesCount, plural,
=1 {1 activity}
other {{{day.data.activitiesCount}} activities}
}</span>
</div>
}
@if (day.data.hasNote) {

47
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<string, JournalCalendarDataItem>();
@ -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)
});
}

2
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 }) {

Loading…
Cancel
Save