92 lines
4.6 KiB
Markdown
92 lines
4.6 KiB
Markdown
---
|
|
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_
|