docs: add itinerary reorder refactor design

Unified data model approach: - Single ItineraryItem type (travel becomes a category) - Constraint validation layer for travel rules - Red zone visual feedback for invalid drops - Simplified flattening logic Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2026-01-17 17:20:34 -06:00
parent 828059a12a
commit c82029abe7
1 changed files with 632 additions and 0 deletions
--- a/docs/plans/2026-01-17-itinerary-reorder-design.md
+++ b/docs/plans/2026-01-17-itinerary-reorder-design.md
@@ -0,0 +1,632 @@
+# Itinerary Reorder Refactor Design
+
+## Overview
+
+Refactor the itinerary drag-and-drop system to use a unified data model, fix current bugs, and implement proper constraint validation with visual feedback.
+
+## Current Problems
+
+1. Can't move custom item below a game
+2. Can't move custom item to empty/rest days
+3. Travel positioning is inconsistent - always appears above custom items regardless of drop position
+4. General weirdness with drag targeting and sort order calculation
+
+## Goals
+
+- **Custom items can go anywhere** - any position on any day, including empty rest days
+- **Travel has hard constraints** - day range based on game cities, position constraints on edge days
+- **Games are fixed** - no drag handle, sorted by game time
+- **Red zone feedback** - invalid drop zones are visually highlighted
+- **Unified data model** - single `ItineraryItem` type for all positionable items
+
+---
+
+## Data Model
+
+### Unified ItineraryItem
+
+Replace `CustomItineraryItem` and `TravelDayOverride` with a single model:
+
+```swift
+struct ItineraryItem: Identifiable, Codable, Hashable {
+    let id: UUID
+    let tripId: UUID
+    var category: ItemCategory
+    var day: Int           // 1-indexed
+    var sortOrder: Double
+    var title: String
+    let createdAt: Date
+    var modifiedAt: Date
+
+    // Location (for mappable items)
+    var latitude: Double?
+    var longitude: Double?
+    var address: String?
+
+    // Travel-specific (only when category == .travel)
+    var travelFromCity: String?
+    var travelToCity: String?
+    var travelDistanceMeters: Double?
+    var travelDurationSeconds: Double?
+
+    var isMappable: Bool {
+        latitude != nil && longitude != nil
+    }
+
+    var isTravel: Bool {
+        category == .travel
+    }
+}
+
+enum ItemCategory: String, Codable, CaseIterable {
+    case restaurant
+    case hotel
+    case activity
+    case note
+    case travel
+
+    var icon: String {
+        switch self {
+        case .restaurant: return "🍽️"
+        case .hotel: return "🏨"
+        case .activity: return "🎯"
+        case .note: return "📝"
+        case .travel: return "🚗"
+        }
+    }
+
+    var label: String {
+        switch self {
+        case .restaurant: return "Restaurant"
+        case .hotel: return "Hotel"
+        case .activity: return "Activity"
+        case .note: return "Note"
+        case .travel: return "Travel"
+        }
+    }
+}
+```
+
+### Games Remain Derived
+
+Games are NOT stored as `ItineraryItem`. They're computed from trip data and rendered with assigned sortOrder values:
+
+| SortOrder Range | Usage |
+|-----------------|-------|
+| 0 - 99 | Items BEFORE games |
+| 100 - 199 | Games (100 + index by game time) |
+| 200+ | Items AFTER games |
+
+---
+
+## Constraint Rules
+
+### Custom Items (non-travel)
+
+- **Can go anywhere** within trip date range
+- Any day (1 to tripDayCount), including empty rest days
+- Any sortOrder position
+
+### Travel Items
+
+**Day constraint:**
+- Earliest: Day of last game in departure city (can leave after game)
+- Latest: Day of first game in arrival city (must arrive before game)
+
+**Position constraint on edge days:**
+- On departure day: Must be AFTER all games (sortOrder > max game sortOrder)
+- On arrival day: Must be BEFORE all games (sortOrder < min game sortOrder)
+- On rest days: Can be anywhere
+
+**Example:**
+```
+Day 1: Game in Detroit (sortOrder 100)
+Day 2: Rest day
+Day 3: Rest day
+Day 4: Game in Milwaukee (sortOrder 100)
+
+Travel "Detroit → Milwaukee" valid positions:
+- Day 1: sortOrder > 100 (after the game)
+- Day 2: any sortOrder
+- Day 3: any sortOrder
+- Day 4: sortOrder < 100 (before the game)
+```
+
+### Games
+
+- Fixed position, no drag handle
+- Sorted by game time within a day
+- Multiple games on same day: travel must be after ALL on departure, before ALL on arrival
+
+---
+
+## Constraint Validation Layer
+
+```swift
+struct ItineraryConstraints {
+    let trip: Trip
+    let games: [RichGame]
+
+    private var tripDayCount: Int {
+        // Calculate from trip.startDate to trip.endDate
+    }
+
+    /// Check if position is valid for an item
+    func isValidPosition(for item: ItineraryItem, day: Int, sortOrder: Double) -> Bool {
+        // Day must be within trip range
+        guard day >= 1 && day <= tripDayCount else { return false }
+
+        switch item.category {
+        case .travel:
+            return isValidTravelPosition(item: item, day: day, sortOrder: sortOrder)
+        default:
+            return true  // Custom items can go anywhere
+        }
+    }
+
+    /// Get valid day range for travel
+    func validDayRange(for item: ItineraryItem) -> ClosedRange<Int>? {
+        guard item.category == .travel,
+              let fromCity = item.travelFromCity,
+              let toCity = item.travelToCity else { return nil }
+
+        let departureDays = gameDays(in: fromCity)
+        let arrivalDays = gameDays(in: toCity)
+
+        let minDay = departureDays.max() ?? 1  // Day of last game in departure city
+        let maxDay = arrivalDays.min() ?? tripDayCount  // Day of first game in arrival city
+
+        return minDay...maxDay
+    }
+
+    /// Get valid sortOrder range for travel on a specific day
+    func validSortOrderRange(for item: ItineraryItem, on day: Int) -> ClosedRange<Double> {
+        guard item.category == .travel,
+              let fromCity = item.travelFromCity,
+              let toCity = item.travelToCity else {
+            return 0...Double.greatestFiniteMagnitude
+        }
+
+        let lastDepartureGameDay = gameDays(in: fromCity).max() ?? 0
+        let firstArrivalGameDay = gameDays(in: toCity).min() ?? tripDayCount + 1
+
+        if day == lastDepartureGameDay {
+            // Must be AFTER all games
+            let maxGameSortOrder = maxGameSortOrder(on: day)
+            return (maxGameSortOrder + 0.001)...Double.greatestFiniteMagnitude
+        } else if day == firstArrivalGameDay {
+            // Must be BEFORE all games
+            let minGameSortOrder = minGameSortOrder(on: day)
+            return 0...(minGameSortOrder - 0.001)
+        } else {
+            // Rest day - anywhere
+            return 0...Double.greatestFiniteMagnitude
+        }
+    }
+
+    private func isValidTravelPosition(item: ItineraryItem, day: Int, sortOrder: Double) -> Bool {
+        guard let dayRange = validDayRange(for: item) else { return false }
+        guard dayRange.contains(day) else { return false }
+
+        let sortOrderRange = validSortOrderRange(for: item, on: day)
+        return sortOrderRange.contains(sortOrder)
+    }
+
+    private func gameDays(in city: String) -> [Int] {
+        // Return day numbers that have games in this city
+    }
+
+    private func maxGameSortOrder(on day: Int) -> Double {
+        let dayGames = games.filter { /* game is on this day */ }
+        return 100.0 + Double(dayGames.count - 1)
+    }
+
+    private func minGameSortOrder(on day: Int) -> Double {
+        return 100.0
+    }
+}
+```
+
+---
+
+## Flattening Logic
+
+Simplified approach - group by day, sort by sortOrder:
+
+```swift
+enum ItineraryRowItem {
+    case dayHeader(dayNumber: Int, date: Date)
+    case game(RichGame, sortOrder: Double)
+    case item(ItineraryItem)
+
+    var sortOrder: Double? {
+        switch self {
+        case .dayHeader: return nil
+        case .game(_, let order): return order
+        case .item(let item): return item.sortOrder
+        }
+    }
+
+    var isReorderable: Bool {
+        switch self {
+        case .dayHeader, .game: return false
+        case .item: return true
+        }
+    }
+
+    var id: String {
+        switch self {
+        case .dayHeader(let day, _): return "day:\(day)"
+        case .game(let game, _): return "game:\(game.game.id)"
+        case .item(let item): return "item:\(item.id)"
+        }
+    }
+}
+
+func buildFlatList(trip: Trip, games: [RichGame], items: [ItineraryItem]) -> [ItineraryRowItem] {
+    var result: [ItineraryRowItem] = []
+
+    for dayNumber in 1...tripDayCount {
+        let dayDate = dateFor(dayNumber)
+
+        // 1. Day header (always first)
+        result.append(.dayHeader(dayNumber: dayNumber, date: dayDate))
+
+        // 2. Collect all orderable content
+        var dayContent: [ItineraryRowItem] = []
+
+        // Games with assigned sortOrder (100, 101, 102...)
+        let dayGames = games
+            .filter { Calendar.current.isDate($0.game.dateTime, inSameDayAs: dayDate) }
+            .sorted { $0.game.dateTime < $1.game.dateTime }
+
+        for (index, game) in dayGames.enumerated() {
+            dayContent.append(.game(game, sortOrder: 100.0 + Double(index)))
+        }
+
+        // Items (travel + custom)
+        for item in items.filter({ $0.day == dayNumber }) {
+            dayContent.append(.item(item))
+        }
+
+        // 3. Sort by sortOrder
+        dayContent.sort { ($0.sortOrder ?? 0) < ($1.sortOrder ?? 0) }
+
+        result.append(contentsOf: dayContent)
+    }
+
+    return result
+}
+```
+
+---
+
+## Drag Handling with Red Zone Feedback
+
+### Pre-calculate Invalid Zones on Drag Start
+
+```swift
+final class ItineraryTableViewController: UITableViewController {
+
+    private var constraints: ItineraryConstraints!
+    private var draggingItem: ItineraryItem?
+    private var invalidRows: Set<Int> = []
+
+    override func tableView(_ tableView: UITableView,
+        dragSessionWillBegin session: UIDragSession) {
+
+        guard let indexPath = tableView.indexPathForRow(at: session.location(in: tableView)),
+              case .item(let item) = flatItems[indexPath.row] else { return }
+
+        draggingItem = item
+        invalidRows = calculateInvalidRows(for: item)
+
+        // Refresh cells to show red zones
+        tableView.reloadData()
+    }
+
+    private func calculateInvalidRows(for item: ItineraryItem) -> Set<Int> {
+        var invalid = Set<Int>()
+
+        for (index, row) in flatItems.enumerated() {
+            // Day headers are always invalid drop targets
+            if case .dayHeader = row {
+                invalid.insert(index)
+                continue
+            }
+
+            // For travel items, check constraints
+            if item.category == .travel {
+                let day = dayNumber(forRow: index)
+                let sortOrder = calculateSortOrder(at: index)
+
+                if !constraints.isValidPosition(for: item, day: day, sortOrder: sortOrder) {
+                    invalid.insert(index)
+                }
+            }
+        }
+
+        return invalid
+    }
+
+    override func tableView(_ tableView: UITableView,
+        dragSessionDidEnd session: UIDragSession) {
+
+        draggingItem = nil
+        invalidRows.removeAll()
+        tableView.reloadData()
+    }
+}
+```
+
+### Apply Red Zone Styling
+
+```swift
+override func tableView(_ tableView: UITableView,
+    cellForRowAt indexPath: IndexPath) -> UITableViewCell {
+
+    let cell = // ... configure normally
+
+    // Apply red zone styling during drag
+    if draggingItem != nil && invalidRows.contains(indexPath.row) {
+        cell.contentView.alpha = 0.3
+        cell.backgroundColor = UIColor.systemRed.withAlphaComponent(0.1)
+    } else {
+        cell.contentView.alpha = 1.0
+        cell.backgroundColor = .clear
+    }
+
+    return cell
+}
+```
+
+### Block Invalid Drops
+
+```swift
+override func tableView(_ tableView: UITableView,
+    targetIndexPathForMoveFromRowAt source: IndexPath,
+    toProposedIndexPath proposed: IndexPath) -> IndexPath {
+
+    guard case .item(let item) = flatItems[source.row] else { return source }
+
+    // Can't drop on position 0
+    var targetRow = max(1, proposed.row)
+    targetRow = min(targetRow, flatItems.count - 1)
+
+    // Can't drop ON a day header - go after it
+    if case .dayHeader = flatItems[targetRow] {
+        targetRow += 1
+    }
+
+    let targetDay = dayNumber(forRow: targetRow)
+    let targetSortOrder = calculateSortOrder(at: targetRow)
+
+    // If valid, allow it
+    if constraints.isValidPosition(for: item, day: targetDay, sortOrder: targetSortOrder) {
+        return IndexPath(row: targetRow, section: 0)
+    }
+
+    // Otherwise, find nearest valid position
+    return findNearestValidPosition(for: item, from: targetRow)
+}
+
+private func findNearestValidPosition(for item: ItineraryItem, from row: Int) -> IndexPath {
+    // Search outward from proposed position to find nearest valid
+    var searchRadius = 1
+    while searchRadius < flatItems.count {
+        // Check row + radius
+        let upRow = row + searchRadius
+        if upRow < flatItems.count {
+            let day = dayNumber(forRow: upRow)
+            let sortOrder = calculateSortOrder(at: upRow)
+            if constraints.isValidPosition(for: item, day: day, sortOrder: sortOrder) {
+                return IndexPath(row: upRow, section: 0)
+            }
+        }
+
+        // Check row - radius
+        let downRow = row - searchRadius
+        if downRow >= 1 {
+            let day = dayNumber(forRow: downRow)
+            let sortOrder = calculateSortOrder(at: downRow)
+            if constraints.isValidPosition(for: item, day: day, sortOrder: sortOrder) {
+                return IndexPath(row: downRow, section: 0)
+            }
+        }
+
+        searchRadius += 1
+    }
+
+    // Fallback to source (shouldn't happen)
+    return IndexPath(row: row, section: 0)
+}
+```
+
+---
+
+## Travel Auto-Generation
+
+When a trip is created, generate travel items for each segment:
+
+```swift
+func generateTravelItems(for trip: Trip, games: [RichGame]) -> [ItineraryItem] {
+    let constraints = ItineraryConstraints(trip: trip, games: games)
+    var items: [ItineraryItem] = []
+
+    for segment in trip.travelSegments {
+        let fromCity = segment.fromLocation.name
+        let toCity = segment.toLocation.name
+
+        // Calculate default day (prefer first valid day)
+        let validRange = constraints.validDayRange(for: tempTravelItem) ?? 1...trip.dayCount
+        let defaultDay = validRange.lowerBound
+
+        // Calculate default sortOrder based on day
+        let defaultSortOrder: Double
+        if defaultDay == validRange.lowerBound && gamesExist(on: defaultDay, in: fromCity) {
+            defaultSortOrder = 200.0  // After games on departure day
+        } else if defaultDay == validRange.upperBound && gamesExist(on: defaultDay, in: toCity) {
+            defaultSortOrder = 50.0   // Before games on arrival day
+        } else {
+            defaultSortOrder = 50.0   // Rest day, default to morning
+        }
+
+        let item = ItineraryItem(
+            id: UUID(),
+            tripId: trip.id,
+            category: .travel,
+            day: defaultDay,
+            sortOrder: defaultSortOrder,
+            title: "\(fromCity) → \(toCity)",
+            createdAt: Date(),
+            modifiedAt: Date(),
+            travelFromCity: fromCity,
+            travelToCity: toCity,
+            travelDistanceMeters: segment.distanceMeters,
+            travelDurationSeconds: segment.durationSeconds
+        )
+        items.append(item)
+    }
+
+    return items
+}
+```
+
+### Smart Merge on Re-plan
+
+```swift
+func mergeTravelItems(
+    existing: [ItineraryItem],
+    newSegments: [TravelSegment],
+    trip: Trip,
+    games: [RichGame]
+) -> [ItineraryItem] {
+
+    let constraints = ItineraryConstraints(trip: trip, games: games)
+    var result: [ItineraryItem] = []
+
+    // Index existing travel by route
+    let existingByRoute = Dictionary(
+        grouping: existing.filter { $0.category == .travel }
+    ) { "\($0.travelFromCity ?? "")->\($0.travelToCity ?? "")" }
+
+    for segment in newSegments {
+        let routeKey = "\(segment.fromLocation.name)->\(segment.toLocation.name)"
+
+        if var existingItem = existingByRoute[routeKey]?.first {
+            // Route still exists - preserve position if valid
+            if constraints.isValidPosition(for: existingItem, day: existingItem.day, sortOrder: existingItem.sortOrder) {
+                existingItem.modifiedAt = Date()
+                result.append(existingItem)
+            } else {
+                // Position no longer valid - reset to default
+                let newItems = generateTravelItems(for: trip, games: games)
+                if let newItem = newItems.first(where: { $0.travelFromCity == segment.fromLocation.name && $0.travelToCity == segment.toLocation.name }) {
+                    result.append(newItem)
+                }
+            }
+        } else {
+            // New segment - generate fresh
+            let newItems = generateTravelItems(for: trip, games: games)
+            if let newItem = newItems.first(where: { $0.travelFromCity == segment.fromLocation.name && $0.travelToCity == segment.toLocation.name }) {
+                result.append(newItem)
+            }
+        }
+    }
+
+    // Orphaned travel items (route removed) are NOT included
+    return result
+}
+```
+
+---
+
+## Persistence
+
+### CloudKit Record: `ItineraryItem`
+
+```
+Fields:
+- itemId: String (UUID)
+- tripId: String (UUID)
+- category: String
+- day: Int64
+- sortOrder: Double
+- title: String
+- createdAt: Date
+- modifiedAt: Date
+- latitude: Double?
+- longitude: Double?
+- address: String?
+- travelFromCity: String?
+- travelToCity: String?
+- travelDistanceMeters: Double?
+- travelDurationSeconds: Double?
+```
+
+### SwiftData Model
+
+```swift
+@Model
+final class ItineraryItemModel {
+    @Attribute(.unique) var id: UUID
+    var tripId: UUID
+    var category: String
+    var day: Int
+    var sortOrder: Double
+    var title: String
+    var createdAt: Date
+    var modifiedAt: Date
+
+    var latitude: Double?
+    var longitude: Double?
+    var address: String?
+    var travelFromCity: String?
+    var travelToCity: String?
+    var travelDistanceMeters: Double?
+    var travelDurationSeconds: Double?
+
+    var ckRecordName: String?
+    var ckModifiedAt: Date?
+}
+```
+
+### Service Consolidation
+
+- Delete `TravelOverrideService`
+- Rename `CustomItemService` → `ItineraryItemService`
+- Single service handles all item types
+
+---
+
+## Files to Change
+
+### Create
+- `SportsTime/Core/Models/Domain/ItineraryItem.swift`
+- `SportsTime/Core/Models/Domain/ItineraryConstraints.swift`
+
+### Modify
+- `SportsTime/Features/Trip/Views/ItineraryTableViewController.swift` - Simplified flattening, red zone feedback
+- `SportsTime/Features/Trip/Views/ItineraryTableViewWrapper.swift` - Use unified items
+- `SportsTime/Features/Trip/Views/TripDetailView.swift` - Single items state, unified service
+- `SportsTime/Core/Models/CloudKit/CKModels.swift` - Add `CKItineraryItem`
+- `SportsTime/Core/Services/CustomItemService.swift` → rename to `ItineraryItemService.swift`
+
+### Delete
+- `SportsTime/Core/Models/Domain/TravelDayOverride.swift`
+- `SportsTime/Core/Models/Domain/CustomItineraryItem.swift`
+- `SportsTime/Core/Services/TravelOverrideService.swift`
+
+---
+
+## Summary
+
+| Aspect | Current | New |
+|--------|---------|-----|
+| Data models | `CustomItineraryItem` + `TravelDayOverride` | Single `ItineraryItem` |
+| Travel storage | Separate override model | `.travel` category |
+| Custom item placement | Only after games | Anywhere |
+| Travel constraints | Day range only | Day range + position on edge days |
+| Invalid drop feedback | Snap (buggy) | Red zone highlighting |
+| Services | Two services | One `ItineraryItemService` |
+| Flattening | Complex special-casing | Simple group + sort |