From 39d944351ce0ffecc871fc1c49ac8a9e2c4803b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Fromme?= Date: Thu, 23 Oct 2025 10:29:37 +0200 Subject: [PATCH] chore: cleanup, update documentation --- docs/PROJECT_OVERVIEW.md | 52 +++++++++++++++---- src/Form/Model/BookingDto.php | 1 - ...articipantAssignedRoomFieldHandlerTest.php | 10 ++-- 3 files changed, 49 insertions(+), 14 deletions(-) diff --git a/docs/PROJECT_OVERVIEW.md b/docs/PROJECT_OVERVIEW.md index c4c66bf..272d745 100644 --- a/docs/PROJECT_OVERVIEW.md +++ b/docs/PROJECT_OVERVIEW.md @@ -23,11 +23,12 @@ - `Edit\IndexController` - edit flow with validation before API submission - **Shared Logic**: - `ParticipantCardFlowTrait` - Card rendering and form handling - - `ParticipantValidationTrait` - Validation error extraction for card indicators + - `ParticipantCardDataService` - Card data generation with optional validation state - **Validation Pattern**: - Both controllers use validation-only forms (`BookingCreateStep2Type`, `BookingEditType`) - Standard Symfony form flow: `handleRequest()` → `isSubmitted()` → `isValid()` - - On invalid: Extract error indices, display error banner, highlight cards, disable submit button + - On invalid: Cards show red border with "unvollständige oder fehlerhafte Daten" badge + - Validation state determined via `ParticipantCardDataService::getAllCardsDataWithValidation()` - On valid: Proceed to next step (create) or submit to API (edit) ## Key Architectural Layers @@ -39,7 +40,9 @@ - `DataProcessor/` - Transform API data to DTOs ### Form System (`src/Form/`) -- **DTOs**: `BookingCreateDto`, `ParticipantDto` (session-stored) +- **DTOs**: `BookingCreateDto`, `ParticipantDto`, `ParticipantEditDto` (session-stored) + - `ParticipantEditDto` wrapper enables cross-participant validation (e.g., email uniqueness) + - Wrapper contains participant being edited + full booking context for validation - **Field Handlers**: 15+ specialized handlers in `src/Form/Service/` - Registered via service tags with dependency resolution - Process in dependency order via `ParticipantFieldHandlerRegistry` @@ -48,6 +51,11 @@ - Age-based, field-dependent, service-specific conditions - Applied via `CreateFieldStateProvider` / `EditFieldStateProvider` - **HTMX Integration**: Real-time updates for dynamic fields +- **Email Uniqueness Validation**: Adult participants (16+) must have unique email addresses + - Children (under 16) exempt from validation, can share emails with adults + - Implemented via `ParticipantEditDto::validateEmailUniqueness()` callback validator + - Case-insensitive comparison with whitespace normalization + - Validation groups: `booking_create`, `booking_edit` - **Date of Birth Field**: Uses `BirthdayType` with three text inputs (day, month, year) - Widget: `widget: 'text'` renders separate text fields instead of dropdowns - Input format: `input: 'datetime_immutable'` for immutable date handling @@ -66,7 +74,9 @@ - Powers sidebar summary and Step 4 confirmation pricing display - `BookingFingerprintService` - Dirty state detection for edit mode - `TravelDataService` - API integration and caching -- `ParticipantCardDataService` - Card display data +- `ParticipantCardDataService` - Card display data with optional validation state + - `getAllCardsData()` - Basic card data without validation + - `getAllCardsDataWithValidation()` - Card data enriched with validation state for error display - `InsuranceService` - Consolidated insurance operations (eligibility, type filtering, reassignment) with request-scoped caching - `RoomAssignmentService` - Automatic room assignment with intelligent conflict resolution @@ -132,6 +142,15 @@ Services from BPN API may lack complete data (especially prices). Always enrich ### Notification System Field handlers generate notifications (auto-changes) → collected by controller → sent via HX-Trigger → displayed as toasts. +**Implementation:** +- `ParticipantDto::$notifications` - Array of notification messages (keyed by MD5 hash for deduplication) +- `ParticipantDto::addNotification($type, $message, $id = null)` - Add notification with auto-generated or custom ID +- Field handlers call `addNotification()` when making automatic changes (e.g., insurance reassignment, rental clearing) +- Controllers collect notifications via `collectParticipantNotifications()` after field processing +- Notifications sent to frontend via `HX-Trigger` response header with `showNotifications` event +- Frontend `toast_controller.js` displays notifications using toastify-js library +- Notification types: `info`, `warning`, `success` (mapped to Tailwind color classes) + ### Floating-Point Precision in Price Calculations **Critical**: All monetary comparisons must account for floating-point arithmetic accumulation errors. @@ -273,12 +292,20 @@ Critical for correct pricing and auto-reassignment: - Ensure dependencies declared correctly - Check sync pattern: only sync fields in original submission +### Adding Cross-Participant Validation +1. Add validation method to `ParticipantEditDto` (wrapper DTO) +2. Use `#[Assert\Callback]` attribute with appropriate validation groups +3. Access booking context via `$this->bookingContext` for cross-participant checks +4. Access current participant via `$this->participant` +5. Validation errors automatically displayed on participant cards via red border and badge + ### Writing Tests - **Insurance IDs**: Always use strings, not integers (e.g., `'100'` not `100`) - **Room properties**: Use `$label` property, not `$name` - **Participant names**: Index 0 expects "Anmelder:in", others expect "Teilnehmer:in N" (1-based) - **Mock dependencies**: Ensure all constructor dependencies have mocks. Note: `InsuranceService` is stateless with no dependencies (no mocking required) - **Insurance mutability**: In edit mode, insurances are always readonly (API limitation) +- **Notification arrays**: Use associative keys (MD5 hashes), not numeric indices - use `reset($notifications)` to get first notification ## File Locations @@ -352,7 +379,8 @@ Edit mode session requires proper cleanup to prevent dirty state persistence: **Shared Traits** (`src/Controller/Booking/Traits/`): - `ParticipantCardFlowTrait` - Card rendering, form creation, summary calculation -- `ParticipantValidationTrait` - Validation error extraction for card indicators + - Creates `ParticipantEditDto` wrapper for email uniqueness validation + - Wraps participant form type with booking context for cross-participant validation - `BookingCreateTrait` - Create flow helpers - `BookingDataTrait` - API data fetching - `BookingExceptionHandlerTrait` - Error handling @@ -376,6 +404,8 @@ Edit mode session requires proper cleanup to prevent dirty state persistence: **Shared Components**: - `templates/booking/_participant_card.html.twig` - Individual participant card + - Displays validation state with red border and "unvollständige oder fehlerhafte Daten" badge + - Requires `isValid` and `errorMessages` in cardData for validation display - `templates/booking/_participant_form.html.twig` - Participant edit form - `templates/booking/_summary.html.twig` - Pricing summary sidebar @@ -395,7 +425,7 @@ Edit mode session requires proper cleanup to prevent dirty state persistence: ## Testing ```bash -./vendor/bin/phpunit # All tests (182 tests, 465 assertions) +./vendor/bin/phpunit # All tests (234 tests, 588 assertions) ./vendor/bin/phpunit tests/Service/ # Service layer ./vendor/bin/phpunit tests/BusProNet/ # API integration ./vendor/bin/phpunit tests/Form/ # Form processing and field handlers @@ -405,9 +435,10 @@ Edit mode session requires proper cleanup to prevent dirty state persistence: **Test Coverage Areas:** - BusProNet data loaders and processors - XML parsers (travels, hotels, bookings, insurances) -- Form DTOs and field handlers -- Service layer (pricing, insurance matching, room assignment) +- Form DTOs and field handlers (including `ParticipantEditDto` wrapper) +- Service layer (pricing, insurance matching, room assignment, card data generation) - Conditional field system +- Cross-participant validation (email uniqueness) - Utility classes ## Development Environment @@ -432,10 +463,13 @@ ddev exec "php -r 'opcache_reset()';" # Clear opcache after code cha - **Clear opcache after code changes** affecting hydration or serialization - **HTMX targeting consistency**: All swaps target `#main-content` with `innerHTML`, sidebar via OOB swap - **Validation pattern**: Both create and edit flows use validation-only forms that wrap card UI for standard Symfony form handling -- **Card error indicators**: `ParticipantValidationTrait::extractParticipantErrorIndices()` parses form errors to highlight invalid participant cards +- **Card validation display**: Uses `ParticipantCardDataService::getAllCardsDataWithValidation()` to enrich cards with validation state +- **Cross-participant validation**: Implemented via `ParticipantEditDto` wrapper with callback validators +- **Email uniqueness**: Adults (16+) require unique emails; children exempt from validation - **Edit mode service availability**: Services with `available <= 0` remain visible and editable for participants who already have them (prevents fingerprint false positives) - **Session cleanup on exit**: All exit paths from edit mode (save, discard, cancel) properly clear session to reset dirty state - **Participant naming convention**: Index 0 is "Anmelder:in", others are "Teilnehmer:in N" (1-based, not 0-based) +- **Notification array structure**: Uses associative keys (MD5 hashes) for deduplication, not numeric indices - **Floating-point price comparisons**: ALWAYS use `round($price, 2)` before comparing monetary values to avoid precision errors (see "Floating-Point Precision in Price Calculations" section) ## References diff --git a/src/Form/Model/BookingDto.php b/src/Form/Model/BookingDto.php index a8e9189..6c32365 100644 --- a/src/Form/Model/BookingDto.php +++ b/src/Form/Model/BookingDto.php @@ -195,5 +195,4 @@ class BookingDto ->addViolation(); } } - } diff --git a/tests/Form/Service/ParticipantAssignedRoomFieldHandlerTest.php b/tests/Form/Service/ParticipantAssignedRoomFieldHandlerTest.php index b7b8305..edf4da0 100644 --- a/tests/Form/Service/ParticipantAssignedRoomFieldHandlerTest.php +++ b/tests/Form/Service/ParticipantAssignedRoomFieldHandlerTest.php @@ -152,10 +152,12 @@ class ParticipantAssignedRoomFieldHandlerTest extends TestCase // Participant 1 should have a warning notification $notifications = $participant1->notifications; $this->assertCount(1, $notifications); - $this->assertSame('warning', $notifications[0]['type']); - $this->assertStringContainsString('Doppelzimmer', $notifications[0]['message']); - $this->assertStringContainsString('wurde von', $notifications[0]['message']); - $this->assertStringContainsString('entfernt', $notifications[0]['message']); + // Get first notification (notifications are keyed by hash, not numeric index) + $firstNotification = reset($notifications); + $this->assertSame('warning', $firstNotification['type']); + $this->assertStringContainsString('Doppelzimmer', $firstNotification['message']); + $this->assertStringContainsString('wurde von', $firstNotification['message']); + $this->assertStringContainsString('entfernt', $firstNotification['message']); } public function testProcessFieldDoesNotGenerateNotificationForAssignedParticipant(): void