diff --git a/.planning/phases/07-dkv-fleet-module/07-REVIEW-FIX.md b/.planning/phases/07-dkv-fleet-module/07-REVIEW-FIX.md new file mode 100644 index 0000000..9ef6da7 --- /dev/null +++ b/.planning/phases/07-dkv-fleet-module/07-REVIEW-FIX.md @@ -0,0 +1,91 @@ +--- +phase: 07-dkv-fleet-module +fixed_at: 2026-06-27T15:45:00Z +review_path: .planning/phases/07-dkv-fleet-module/07-REVIEW.md +iteration: 1 +findings_in_scope: 8 +fixed: 8 +skipped: 0 +status: all_fixed +--- + +# Phase 07: Code Review Fix Report + +**Fixed at:** 2026-06-27T15:45:00Z +**Source review:** .planning/phases/07-dkv-fleet-module/07-REVIEW.md +**Iteration:** 1 + +**Summary:** +- Findings in scope: 8 (3 Critical, 5 Warning) +- Fixed: 8 +- Skipped: 0 + +## Fixed Issues + +### CR-01: CSV import count field mismatch — import count always `undefined` + +**Files modified:** `apps/api/src/dkv/dkv.service.ts`, `apps/web/src/lib/dkv-api.ts` +**Commit:** f3f610f +**Applied fix:** Renamed the backend return field from `imported` to `count` in `dkv.service.ts` line 501. Updated the TypeScript return type in `dkv-api.ts` to `Promise<{ count: number; mode: string }>` for completeness. The frontend already read `result.count` and `onImported(result.count)`, so both call sites now resolve correctly without further changes. + +--- + +### CR-02: Cron expression silently incorrect for `pollIntervalMin > 59` + +**Files modified:** `apps/api/src/dkv/dkv-scheduler.service.ts`, `apps/api/src/dkv/dto/dkv-config.dto.ts` +**Commit:** 955a946 +**Applied fix:** Replaced the unconditional `*/${intervalMin} * * * *` expression with a conditional: intervals below 60 use the minute field (`*/${intervalMin} * * * *`), intervals 60 and above use the hours field (`0 */${hours} * * *` where `hours = Math.floor(intervalMin / 60)`). Also added `@Max(1440)` (24 h) to `DkvConfigDto.pollIntervalMin` to prevent unbounded values. + +--- + +### CR-03: IMAP connection not closed when mailbox lock fails + +**Files modified:** `apps/api/src/dkv/providers/imap.provider.ts` +**Commit:** cb0d378 +**Applied fix:** Moved `client.getMailboxLock()` inside the `try` block. Declared `lock` as `let lock: ... | null = null` before the `try` so the type is available in `finally`. Changed `lock.release()` to `lock?.release()` so the finally block handles the case where lock acquisition never succeeded. `client.logout()` is now always called regardless of whether `getMailboxLock` threw. + +--- + +### WR-01: nodemailer transport not closed — connection pool leak + +**Files modified:** `apps/api/src/dkv/dkv-mail.service.ts` +**Commit:** ded6523 +**Applied fix:** Added `finally { transport.close(); }` to the existing try/catch block in `sendExportEmail`. The transport is now always closed after each send attempt (success or failure), preventing SMTP connection pool accumulation under repeated sends with retry backoff. + +--- + +### WR-02: Pagination query params not coerced to numbers — `@IsInt()` silently fails + +**Files modified:** `apps/api/src/dkv/dto/dkv-history.dto.ts` +**Commit:** 49eab55 +**Applied fix:** Added `import { Type } from 'class-transformer'` and decorated both `page` and `limit` fields with `@Type(() => Number)`. Also added `@Max(100)` to `limit` to bound the result-set size. HTTP query string values are now coerced from string to number before `@IsInt()` / `@Min()` validators run. + +--- + +### WR-03: `formatDateTime` never catches — invalid dates produce "NaN.NaN.NaN, NaN:NaN Uhr" + +**Files modified:** `apps/web/src/app/(portal)/modules/dkv-fleet/components/InvoiceHistoryTable.tsx` +**Commit:** 338655c +**Applied fix:** Replaced the dead `try/catch` wrapper with an explicit `isNaN(d.getTime())` guard. `new Date()` never throws — it returns an Invalid Date object — so the catch branch was unreachable. The fix returns the raw `isoString` as fallback when the date is invalid, preventing the "NaN.NaN.NaN, NaN:NaN Uhr" display. + +--- + +### WR-04: Exchange UniqueId can contain path characters in generated export filename + +**Files modified:** `apps/api/src/dkv/dkv.service.ts` +**Commit:** 9de16ba +**Applied fix:** Added `.replace(/[^a-zA-Z0-9\-]/g, '_')` sanitisation to the `_extractInvoiceNumber` fallback return value. Exchange EWS UniqueIds are base64 and can contain `+`, `/`, and `=`; a `/` would cause `path.join(userFilesDir, filename)` to resolve into a subdirectory, making `writeFileSync` fail. The sanitisation strips all non-alphanumeric/hyphen characters to `_` before the value reaches the filesystem path. + +--- + +### WR-05: No file size limit on CSV multipart upload + +**Files modified:** `apps/api/src/dkv/dkv.controller.ts` +**Commit:** d2d224c +**Applied fix:** Added `limits: { fileSize: 5 * 1024 * 1024 }` (5 MB) to the `FileInterceptor` options for the `POST /dkv/vehicles/import` endpoint. multer now rejects any upload exceeding 5 MB before Node.js buffers the content, preventing a heap exhaustion via oversized file upload. + +--- + +_Fixed: 2026-06-27T15:45:00Z_ +_Fixer: Claude (gsd-code-fixer)_ +_Iteration: 1_