# Booking Data Processor Refactoring Plan

## Current Status (2025-10-05)

We discovered a structural inconsistency between the UPDATE and CREATE booking flows while implementing the booking submission feature.

## Problem Statement

The UPDATE and CREATE flows use fundamentally different data structures, leading to code duplication and complexity:

### UPDATE Flow (Current)
- `BookingEditDto` contains a `Booking` object
- `Booking` has centralized service arrays with participant mappings:
  - `booking.additionalServices` - array of Service objects with `mapping` property (0-based indices)
  - `booking.transportationServices` - array of Service objects with `mapping` property
  - `booking.pickupsOutbound` - array of Pickup objects with `mapping` property
- `BookingDataProcessor.createUpdateRequestPayload()`:
  - Resets all service mappings
  - Iterates through participants
  - Rebuilds service mappings by looking up services in booking data
  - Removes unused services
  - Converts 0-based indices to 1-based for API

### CREATE Flow (Current)
- `BookingCreateDto` contains only `participants` array
- Each `ParticipantDto` has direct service references:
  - `courses`, `skiPass`, `additionalServices`, `board`, `rentals`
  - `transportationOutbound`, `transportationInbound`
  - `pickupOutbound`, `pickupInbound`
  - `insurance`
- `BookingDataProcessor.createBookingRequestPayload()`:
  - Collects services directly from participants
  - Groups by service ID
  - Converts to 1-based participant IDs for API

## Root Cause

The UPDATE flow was designed to work with API-sourced `Booking` objects that already have centralized service mappings. The CREATE flow was designed from scratch with a simpler participant-centric approach.

## Proposed Solution

**Align both flows to use the participant-centric structure:**

1. **Both DTOs work the same way:**
   - Both have `participants` array
   - Services are attached directly to participants
   - No centralized service objects with mappings

2. **Unified payload generation:**
   - Use same `collect*Mappings()` methods for both flows
   - Use same `addServicesFromMap()` helper
   - Remove complex service manipulation in update flow

3. **Benefits:**
   - Single source of truth for service mappings
   - Less code duplication
   - Easier to understand and maintain
   - Consistent patterns across all booking operations

## Implementation Steps

### Phase 1: Refactor BookingEditDto.fromBooking()
- [x] Already populates participant services correctly
- [x] Services are already attached to participants
- [ ] Verify all service types are covered

### Phase 2: Refactor BookingDataProcessor.createUpdateRequestPayload()
- [ ] Remove `resetServiceMappings()`
- [ ] Remove `processParticipantServices()` (complex service lookup)
- [ ] Remove `processAdditionalServices()`
- [ ] Remove `processTransportationServices()`
- [ ] Remove `processPickupLocations()`
- [ ] Remove `removeUnusedServices()`
- [ ] Use `collect*Mappings()` methods instead (same as create flow)
- [ ] Update `buildServicesPayload()` to use collected maps
- [ ] Update `buildPickupPayload()` to use collected maps

### Phase 3: Add convertServicesToMap() Helper
- [ ] Create helper to convert service objects with mapping to ID => participant IDs map
- [ ] This bridges the gap between old structure (if needed) and new structure

### Phase 4: Testing
- [ ] Test update flow with all service types
- [ ] Test create flow (should remain unchanged)
- [ ] Verify API payloads are identical before/after refactoring
- [ ] Test edge cases (no services, all services, mixed scenarios)

### Phase 5: Cleanup
- [ ] Remove unused methods from BookingDataProcessor
- [ ] Remove unused properties from Booking model (if any)
- [ ] Update documentation

## Risk Assessment

**MEDIUM RISK** - This refactoring touches critical booking update functionality that is already working in production.

### Risks:
1. Breaking existing update flow
2. Subtle bugs in service mapping
3. Data loss if participant service references are incorrect
4. Payment/bank account handling might break

### Mitigation:
1. Comprehensive testing before deployment
2. Keep git history clean with atomic commits
3. Test with real booking data from sandbox
4. Verify XML payloads match exactly (before/after)
5. Have rollback plan ready

## Decision Point

**Should we refactor NOW or LATER?**

### Arguments for NOW:
- We're already in BookingDataProcessor
- Fresh understanding of both flows
- Prevents further divergence
- Makes current task (booking submission) cleaner

### Arguments for LATER:
- Current task (booking submission) is incomplete
- Refactoring is significant and risky
- Could introduce bugs in working update flow
- Should be separate PR with focused testing
- Current booking submission is more urgent

## Decision

**REFACTOR LATER** - Complete the booking submission task first, then do this refactoring as a separate focused effort.

**AGREED:** The CREATE flow's participant-centric structure is the new standard. The UPDATE flow should adopt this architecture in the future refactoring.

### Reasoning:
1. Booking submission is nearly complete and is the immediate business need
2. Update flow is working and tested - don't break what works
3. Refactoring deserves dedicated focus and testing
4. Can create comprehensive tests for both flows first
5. Allows for proper code review and QA

### Short-term Solution:
- Keep both flows separate for now
- Add the `convertServicesToMap()` helper to bridge structures
- Complete booking submission with current architecture
- Document this technical debt

### Long-term Plan:
- Create separate refactoring task/issue
- Write comprehensive tests for update flow first
- Perform refactoring in dedicated branch
- Extensive testing with sandbox data
- Separate PR with focused review

## Current Task: Booking Submission

We are 60% complete with booking submission implementation:

### Completed:
- [x] BookingResponse, PriceItem, PaymentTerms models
- [x] BookingResponseParser with pricing data
- [x] createBookingRequestPayload() in BookingDataProcessor
- [x] Payment type ID constants
- [x] Helper method addServicesFromMap()

### Remaining:
- [ ] Add TYPE_BOOKING constant to ApiClient
- [ ] Add createBookingInquiry() and createBooking() to ApiClient
- [ ] Update ApiResponseParser to handle BUCHUNG response type
- [ ] Implement two-phase submission in CreateStep4Controller
- [ ] Add clearBookingCreateDto() to BookingService
- [ ] Create BookingSuccessController and template
- [ ] Test with sandbox API

## Next Steps

1. **IMMEDIATE:** Continue with booking submission task
2. **AFTER COMPLETION:** Create refactoring issue/task
3. **FUTURE:** Dedicated refactoring effort with proper testing

---

**Document Created:** 2025-10-05
**Status:** Deferred - Continue with booking submission
**Related:** Booking submission implementation (in progress)