fix: address code review issues from daily review

- Apply runtime default for isConfirmBeforeTaskDelete to ensure existing users get confirmation dialog
- Add takeUntil cleanup for dialog subscription in context menu
- Move _isTaskDeleteTriggered flag before delete call to prevent race condition
- Replace alert() with PFLog.warn() for non-blocking notification
- Change any to unknown type for better type safety
- Localize month names using DateAdapter instead of hardcoded English strings
- Remove duplicate monthNames array declaration (DRY)
This commit is contained in:
Johannes Millan 2026-01-09 16:25:41 +01:00
parent 73690c3766
commit a2e1f932f4
7 changed files with 30 additions and 35 deletions

View file

@ -66,6 +66,11 @@ describe('DialogScheduleTaskComponent - Select Due Only Mode', () => {
initialState: {
[CONFIG_FEATURE_NAME]: {
sync: {},
localization: {
lng: undefined,
dateTimeLocale: undefined,
firstDayOfWeek: 0,
},
} as any,
},
}),
@ -170,6 +175,7 @@ describe('DialogScheduleTaskComponent - Select Due Only Mode', () => {
});
it('should set selectedDate to targetDay on initialization', async () => {
fixture.detectChanges();
await component.ngAfterViewInit();
const expectedDate = dateStrToUtcDate('2024-01-15');
@ -191,6 +197,7 @@ describe('DialogScheduleTaskComponent - Select Due Only Mode', () => {
});
it('should not have initial selectedDate when no targetDay provided', async () => {
fixture.detectChanges();
await component.ngAfterViewInit();
// selectedDate should be null since no targetDay was provided

View file

@ -75,6 +75,11 @@ describe('DialogScheduleTaskComponent', () => {
initialState: {
[CONFIG_FEATURE_NAME]: {
sync: {},
localization: {
lng: undefined,
dateTimeLocale: undefined,
firstDayOfWeek: 0,
},
} as any,
},
}),
@ -314,6 +319,7 @@ describe('DialogScheduleTaskComponent', () => {
component.data = { task: mockTask };
component.task = mockTask;
fixture.detectChanges();
await component.ngAfterViewInit();
@ -348,6 +354,7 @@ describe('DialogScheduleTaskComponent', () => {
component.data = { task: mockTask };
component.task = mockTask;
fixture.detectChanges();
await component.ngAfterViewInit();
@ -377,6 +384,7 @@ describe('DialogScheduleTaskComponent', () => {
component.data = { task: mockTask };
component.task = mockTask;
fixture.detectChanges();
await component.ngAfterViewInit();

View file

@ -1,5 +1,5 @@
import { ComponentFixture, fakeAsync, TestBed, tick } from '@angular/core/testing';
import { Component, NO_ERRORS_SCHEMA } from '@angular/core';
import { Component, input, NO_ERRORS_SCHEMA } from '@angular/core';
import { MAT_DIALOG_DATA, MatDialogModule, MatDialogRef } from '@angular/material/dialog';
import { NoopAnimationsModule } from '@angular/platform-browser/animations';
import { TranslateModule } from '@ngx-translate/core';
@ -23,7 +23,9 @@ import { RepeatTaskHeatmapComponent } from '../repeat-task-heatmap/repeat-task-h
template: '',
standalone: true,
})
class MockRepeatTaskHeatmapComponent {}
class MockRepeatTaskHeatmapComponent {
repeatCfgId = input.required<string>();
}
describe('DialogEditTaskRepeatCfgComponent', () => {
let mockDialogRef: jasmine.SpyObj<MatDialogRef<DialogEditTaskRepeatCfgComponent>>;

View file

@ -195,6 +195,7 @@ export class RepeatTaskHeatmapComponent {
): HeatmapData {
const weeks: WeekData[] = [];
const monthLabels: string[] = [];
const monthNames = this._dateAdapter.getMonthNames('short');
let currentMonth = -1;
// Find the first day (based on firstDayOfWeek setting) before or on the start date
@ -219,37 +220,9 @@ export class RepeatTaskHeatmapComponent {
const month = currentDate.getMonth();
if (month !== currentMonth && currentDate.getDate() <= 7 && weekCount > 0) {
const monthNames = [
'Jan',
'Feb',
'Mar',
'Apr',
'May',
'Jun',
'Jul',
'Aug',
'Sep',
'Oct',
'Nov',
'Dec',
];
monthLabels.push(monthNames[month]);
currentMonth = month;
} else if (monthLabels.length === 0 && weekCount === 0) {
const monthNames = [
'Jan',
'Feb',
'Mar',
'Apr',
'May',
'Jun',
'Jul',
'Aug',
'Sep',
'Oct',
'Nov',
'Dec',
];
monthLabels.push(monthNames[month]);
currentMonth = month;
}

View file

@ -290,7 +290,7 @@ export class TaskContextMenuInnerComponent implements AfterViewInit {
}
const isConfirmBeforeTaskDelete =
this._globalConfigService.cfg()?.misc?.isConfirmBeforeTaskDelete;
this._globalConfigService.cfg()?.misc?.isConfirmBeforeTaskDelete ?? true;
if (isConfirmBeforeTaskDelete) {
this._matDialog
@ -302,6 +302,7 @@ export class TaskContextMenuInnerComponent implements AfterViewInit {
},
})
.afterClosed()
.pipe(takeUntil(this._destroy$))
.subscribe(async (isConfirm) => {
if (isConfirm) {
await this._performDelete();
@ -313,9 +314,9 @@ export class TaskContextMenuInnerComponent implements AfterViewInit {
}
private async _performDelete(): Promise<void> {
this._isTaskDeleteTriggered = true;
const taskWithSubTasks = await this._getTaskWithSubtasks();
this._taskService.remove(taskWithSubTasks);
this._isTaskDeleteTriggered = true;
}
startTask(): void {

View file

@ -368,7 +368,7 @@ export class TaskComponent implements OnDestroy, AfterViewInit {
}
const isConfirmBeforeTaskDelete =
this._configService.cfg()?.misc?.isConfirmBeforeTaskDelete;
this._configService.cfg()?.misc?.isConfirmBeforeTaskDelete ?? true;
if (isConfirmBeforeTaskDelete) {
this._matDialog

View file

@ -29,7 +29,7 @@ export const autoFixTypiaErrors = (
} else if (keys[0] === 'globalConfig') {
const defaultValue = getValueByPath(DEFAULT_GLOBAL_CONFIG, keys.slice(1));
setValueByPath(data, keys, defaultValue);
alert(
PFLog.warn(
`Warning: ${path} had an invalid value and was set to default: ${defaultValue}`,
);
} else if (error.expected.includes('undefined') && value === null) {
@ -138,7 +138,11 @@ const parsePath = (path: string): (string | number)[] => {
return keys;
};
const getValueByPath = <T, R = any>(obj: T, path: (string | number)[]): R | undefined =>
const getValueByPath = <T, R = unknown>(
obj: T,
path: (string | number)[],
): R | undefined =>
// eslint-disable-next-line @typescript-eslint/no-explicit-any
path.reduce<any>((acc, key) => acc?.[key], obj);
const setValueByPath = <T extends object>(