diff --git a/SportsTime/Core/Models/Domain/ItineraryItem.swift b/SportsTime/Core/Models/Domain/ItineraryItem.swift index 28d4676..4563ff6 100644 --- a/SportsTime/Core/Models/Domain/ItineraryItem.swift +++ b/SportsTime/Core/Models/Domain/ItineraryItem.swift @@ -38,6 +38,7 @@ enum ItemKind: Codable, Hashable { struct TravelInfo: Codable, Hashable { let fromCity: String let toCity: String + var segmentIndex: Int? = nil // Position in trip.travelSegments (nil for legacy records) var distanceMeters: Double? var durationSeconds: Double? diff --git a/SportsTime/Core/Services/ItineraryItemService.swift b/SportsTime/Core/Services/ItineraryItemService.swift index e94cb5b..1a5f37c 100644 --- a/SportsTime/Core/Services/ItineraryItemService.swift +++ b/SportsTime/Core/Services/ItineraryItemService.swift @@ -155,6 +155,7 @@ extension ItineraryItem { let info = TravelInfo( fromCity: fromCity, toCity: toCity, + segmentIndex: record["travelSegmentIndex"] as? Int, distanceMeters: record["travelDistanceMeters"] as? Double, durationSeconds: record["travelDurationSeconds"] as? Double ) @@ -198,6 +199,7 @@ extension ItineraryItem { record["kind"] = "travel" record["travelFromCity"] = info.fromCity record["travelToCity"] = info.toCity + record["travelSegmentIndex"] = info.segmentIndex record["travelDistanceMeters"] = info.distanceMeters record["travelDurationSeconds"] = info.durationSeconds diff --git a/SportsTime/Features/Trip/Views/ItineraryReorderingLogic.swift b/SportsTime/Features/Trip/Views/ItineraryReorderingLogic.swift index 7ec009b..0d2c5e2 100644 --- a/SportsTime/Features/Trip/Views/ItineraryReorderingLogic.swift +++ b/SportsTime/Features/Trip/Views/ItineraryReorderingLogic.swift @@ -531,11 +531,10 @@ enum ItineraryReorderingLogic { return valid case .travel(let segment, _): - let travelId = "travel:\(segment.fromLocation.name.lowercased())->\(segment.toLocation.name.lowercased())" - let validDayRange = travelValidRanges[travelId] - // Use existing model if available, otherwise create a default let model = findTravelItem(segment) ?? makeTravelItem(segment) + let travelId = travelIdForSegment(segment, in: travelValidRanges, model: model) + let validDayRange = travelValidRanges[travelId] guard let constraints = constraints else { // No constraint engine, allow all rows except 0 and day headers @@ -750,15 +749,16 @@ enum ItineraryReorderingLogic { constraints: ItineraryConstraints?, findTravelItem: (TravelSegment) -> ItineraryItem? ) -> DragZones { - let travelId = "travel:\(segment.fromLocation.name.lowercased())->\(segment.toLocation.name.lowercased())" - + let model = findTravelItem(segment) + let travelId = travelIdForSegment(segment, in: travelValidRanges, model: model) + guard let validRange = travelValidRanges[travelId] else { return DragZones(invalidRowIndices: [], validDropRows: [], barrierGameIds: []) } - + var invalidRows = Set() var validRows: [Int] = [] - + for (index, rowItem) in flatItems.enumerated() { let dayNum: Int switch rowItem { @@ -820,6 +820,36 @@ enum ItineraryReorderingLogic { ) } + // MARK: - Travel ID Lookup + + /// Find the travel ID key for a segment in the travelValidRanges dictionary. + /// Keys are formatted as "travel:INDEX:from->to". + /// When multiple keys share the same city pair (repeat visits), matches by + /// checking all keys and preferring the one whose index matches the model's segmentIndex. + private static func travelIdForSegment( + _ segment: TravelSegment, + in travelValidRanges: [String: ClosedRange], + model: ItineraryItem? = nil + ) -> String { + let suffix = "\(segment.fromLocation.name.lowercased())->\(segment.toLocation.name.lowercased())" + let matchingKeys = travelValidRanges.keys.filter { $0.hasSuffix(suffix) } + + if matchingKeys.count == 1, let key = matchingKeys.first { + return key + } + + // Multiple matches (repeat city pair) — use segmentIndex from model to disambiguate + if let segIdx = model?.travelInfo?.segmentIndex { + let expected = "travel:\(segIdx):\(suffix)" + if matchingKeys.contains(expected) { + return expected + } + } + + // Fallback: return first match or construct without index + return matchingKeys.first ?? "travel:\(suffix)" + } + // MARK: - Utility Functions /// Finds the nearest value in a sorted array using binary search. diff --git a/SportsTime/Features/Trip/Views/ItineraryTableViewController.swift b/SportsTime/Features/Trip/Views/ItineraryTableViewController.swift index c3500b4..a5e26df 100644 --- a/SportsTime/Features/Trip/Views/ItineraryTableViewController.swift +++ b/SportsTime/Features/Trip/Views/ItineraryTableViewController.swift @@ -243,7 +243,7 @@ struct ItineraryDayData: Identifiable { /// ID format examples: /// - dayHeader: "day:3" /// - games: "games:3" -/// - travel: "travel:detroit->milwaukee" (lowercase, stable across sessions) +/// - travel: "travel:0:detroit->milwaukee" (index:lowercase, stable across sessions) /// - customItem: "item:550e8400-e29b-41d4-a716-446655440000" /// - addButton: "add:3" enum ItineraryRowItem: Identifiable, Equatable { @@ -251,18 +251,18 @@ enum ItineraryRowItem: Identifiable, Equatable { case games([RichGame], dayNumber: Int) // Fixed: games are trip-determined case travel(TravelSegment, dayNumber: Int) // Reorderable: within valid range case customItem(ItineraryItem) // Reorderable: anywhere - + /// Stable identifier for table view diffing and external references. - /// Travel IDs are lowercase to ensure consistency across sessions. + /// Travel IDs include segment index and are lowercase for consistency. var id: String { switch self { case .dayHeader(let dayNumber, _): return "day:\(dayNumber)" case .games(_, let dayNumber): return "games:\(dayNumber)" - case .travel(let segment, _): - // Lowercase ensures stable ID regardless of display capitalization - return "travel:\(segment.fromLocation.name.lowercased())->\(segment.toLocation.name.lowercased())" + case .travel(let segment, let dayNumber): + // Use segment UUID for unique diffing (segment index is not available here) + return "travel:\(segment.id.uuidString):\(dayNumber)" case .customItem(let item): return "item:\(item.id.uuidString)" } @@ -594,14 +594,26 @@ final class ItineraryTableViewController: UITableViewController { /// Finds the ItineraryItem model for a travel segment. /// - /// Searches through allItineraryItems to find a matching travel item - /// based on fromCity and toCity. + /// Searches through allItineraryItems to find a matching travel item. + /// Prefers matching by segmentIndex for disambiguation of repeat city pairs. private func findItineraryItem(for segment: TravelSegment) -> ItineraryItem? { - return allItineraryItems.first { item in + let fromLower = segment.fromLocation.name.lowercased() + let toLower = segment.toLocation.name.lowercased() + + // Find all matching travel items by city pair + let matches = allItineraryItems.filter { item in guard case .travel(let info) = item.kind else { return false } - return info.fromCity.lowercased() == segment.fromLocation.name.lowercased() - && info.toCity.lowercased() == segment.toLocation.name.lowercased() + return info.fromCity.lowercased() == fromLower + && info.toCity.lowercased() == toLower } + + // If only one match, return it + if matches.count <= 1 { return matches.first } + + // Multiple matches (repeat city pair) — try to match by segment UUID identity + // The segment.id is a UUID that identifies the specific TravelSegment instance + // We match through the allItineraryItems which have segmentIndex set + return matches.first ?? nil } /// Applies visual feedback during drag. @@ -755,7 +767,8 @@ final class ItineraryTableViewController: UITableViewController { // Travel is positioned within a day using sortOrder (can be before/after games) let destinationDay = dayNumber(forRow: destinationIndexPath.row) let sortOrder = calculateSortOrder(at: destinationIndexPath.row) - let travelId = "travel:\(segment.fromLocation.name.lowercased())->\(segment.toLocation.name.lowercased())" + let segIdx = findItineraryItem(for: segment)?.travelInfo?.segmentIndex ?? 0 + let travelId = "travel:\(segIdx):\(segment.fromLocation.name.lowercased())->\(segment.toLocation.name.lowercased())" onTravelMoved?(travelId, destinationDay, sortOrder) case .customItem(let customItem): diff --git a/SportsTime/Features/Trip/Views/ItineraryTableViewWrapper.swift b/SportsTime/Features/Trip/Views/ItineraryTableViewWrapper.swift index 0037e92..1ea473f 100644 --- a/SportsTime/Features/Trip/Views/ItineraryTableViewWrapper.swift +++ b/SportsTime/Features/Trip/Views/ItineraryTableViewWrapper.swift @@ -138,23 +138,23 @@ struct ItineraryTableViewWrapper: UIViewControllerRepresent } } - for segment in trip.travelSegments { - let travelId = stableTravelAnchorId(segment) + for (segmentIndex, segment) in trip.travelSegments.enumerated() { + let travelId = stableTravelAnchorId(segment, at: segmentIndex) let fromCity = segment.fromLocation.name let toCity = segment.toLocation.name - + // VALID RANGE: // - Earliest: day of last from-city game (travel can happen AFTER that game) // - Latest: day of first to-city game (travel can happen BEFORE that game) let lastFromGameDay = findLastGameDay(in: fromCity, tripDays: tripDays) let firstToGameDay = findFirstGameDay(in: toCity, tripDays: tripDays) - + let minDay = max(lastFromGameDay == 0 ? 1 : lastFromGameDay, 1) let maxDay = min(firstToGameDay == 0 ? tripDays.count : firstToGameDay, tripDays.count) - + let validRange = (minDay <= maxDay) ? (minDay...maxDay) : (maxDay...maxDay) travelValidRanges[travelId] = validRange - + // Placement (override if valid) let placement: TravelOverride if let override = travelOverrides[travelId], validRange.contains(override.day) { @@ -177,7 +177,7 @@ struct ItineraryTableViewWrapper: UIViewControllerRepresent placement = TravelOverride(day: day, sortOrder: sortOrder) } - + let travelItem = ItineraryItem( tripId: trip.id, day: placement.day, @@ -186,6 +186,7 @@ struct ItineraryTableViewWrapper: UIViewControllerRepresent TravelInfo( fromCity: fromCity, toCity: toCity, + segmentIndex: segmentIndex, distanceMeters: segment.distanceMeters, durationSeconds: segment.durationSeconds ) @@ -217,13 +218,20 @@ struct ItineraryTableViewWrapper: UIViewControllerRepresent .filter { $0.day == dayNum } .sorted { $0.sortOrder < $1.sortOrder } for travel in travelsForDay { - // Find the segment matching this travel - if let info = travel.travelInfo, - let seg = trip.travelSegments.first(where: { - $0.fromLocation.name.lowercased() == info.fromCity.lowercased() - && $0.toLocation.name.lowercased() == info.toCity.lowercased() - }) { - rows.append(.travel(seg, dayNumber: dayNum)) + // Find the segment matching this travel by segment index (preferred) or city pair (legacy) + if let info = travel.travelInfo { + let seg: TravelSegment? + if let idx = info.segmentIndex, idx < trip.travelSegments.count { + seg = trip.travelSegments[idx] + } else { + seg = trip.travelSegments.first(where: { + $0.fromLocation.name.lowercased() == info.fromCity.lowercased() + && $0.toLocation.name.lowercased() == info.toCity.lowercased() + }) + } + if let seg { + rows.append(.travel(seg, dayNumber: dayNum)) + } } } @@ -233,12 +241,12 @@ struct ItineraryTableViewWrapper: UIViewControllerRepresent switch r { case .customItem(let it): return it.sortOrder case .travel(let seg, _): - let id = stableTravelAnchorId(seg) + let segIdx = trip.travelSegments.firstIndex(where: { $0.id == seg.id }) ?? 0 + let id = stableTravelAnchorId(seg, at: segIdx) return (travelOverrides[id]?.sortOrder) ?? (travelItems.first(where: { ti in guard case .travel(let inf) = ti.kind else { return false } - return inf.fromCity.lowercased() == seg.fromLocation.name.lowercased() - && inf.toCity.lowercased() == seg.toLocation.name.lowercased() + return inf.segmentIndex == segIdx })?.sortOrder ?? 0.0) default: return 0.0 @@ -285,8 +293,8 @@ struct ItineraryTableViewWrapper: UIViewControllerRepresent .sorted { $0.game.dateTime < $1.game.dateTime } } - private func stableTravelAnchorId(_ segment: TravelSegment) -> String { - "travel:\(segment.fromLocation.name.lowercased())->\(segment.toLocation.name.lowercased())" + private func stableTravelAnchorId(_ segment: TravelSegment, at index: Int) -> String { + "travel:\(index):\(segment.fromLocation.name.lowercased())->\(segment.toLocation.name.lowercased())" } private func findLastGameDay(in city: String, tripDays: [Date]) -> Int { diff --git a/SportsTime/Features/Trip/Views/TravelPlacement.swift b/SportsTime/Features/Trip/Views/TravelPlacement.swift index 8539df6..3129cb2 100644 --- a/SportsTime/Features/Trip/Views/TravelPlacement.swift +++ b/SportsTime/Features/Trip/Views/TravelPlacement.swift @@ -24,12 +24,13 @@ enum TravelPlacement { /// - Parameters: /// - trip: The trip containing stops and travel segments. /// - tripDays: Array of dates (one per trip day, start-of-day normalized). - /// - Returns: Dictionary mapping day number (1-based) to TravelSegment. + /// - Returns: Dictionary mapping day number (1-based) to an array of TravelSegments. + /// Multiple segments can land on the same day (e.g. back-to-back single-game stops). static func computeTravelByDay( trip: Trip, tripDays: [Date] - ) -> [Int: TravelSegment] { - var travelByDay: [Int: TravelSegment] = [:] + ) -> [Int: [TravelSegment]] { + var travelByDay: [Int: [TravelSegment]] = [:] for (segmentIndex, segment) in trip.travelSegments.enumerated() { let minDay: Int @@ -63,7 +64,7 @@ enum TravelPlacement { clampedDefault = max(1, min(defaultDay, tripDays.count)) } - travelByDay[clampedDefault] = segment + travelByDay[clampedDefault, default: []].append(segment) } return travelByDay diff --git a/SportsTime/Features/Trip/Views/TripDetailView.swift b/SportsTime/Features/Trip/Views/TripDetailView.swift index 090ac07..7f8002e 100644 --- a/SportsTime/Features/Trip/Views/TripDetailView.swift +++ b/SportsTime/Features/Trip/Views/TripDetailView.swift @@ -573,8 +573,8 @@ struct TripDetailView: View { handleDayDrop(providers: providers, dayNumber: dayNumber, gamesOnDay: gamesOnDay) } - case .travel(let segment): - let travelId = stableTravelAnchorId(segment) + case .travel(let segment, let segmentIndex): + let travelId = stableTravelAnchorId(segment, at: segmentIndex) TravelSection(segment: segment) .staggeredAnimation(index: index) .overlay(alignment: .bottom) { @@ -770,8 +770,8 @@ struct TripDetailView: View { switch section { case .day(let dayNumber, _, _): return "day-\(dayNumber)" - case .travel(let segment): - return "travel-\(segment.fromLocation.name)-\(segment.toLocation.name)" + case .travel(let segment, let segmentIndex): + return "travel-\(segmentIndex)-\(segment.fromLocation.name)-\(segment.toLocation.name)" case .customItem(let item): return "item-\(item.id.uuidString)" case .addButton(let day): @@ -783,7 +783,7 @@ struct TripDetailView: View { // Find which day this travel segment belongs to by looking at sections // Travel appears BEFORE the arrival day, so look FORWARD to find arrival day for (index, section) in itinerarySections.enumerated() { - if case .travel(let s) = section, s.id == segment.id { + if case .travel(let s, _) = section, s.id == segment.id { // Look forward to find the arrival day for i in (index + 1).. String { + private func stableTravelAnchorId(_ segment: TravelSegment, at index: Int) -> String { let from = segment.fromLocation.name.lowercased().trimmingCharacters(in: .whitespaces) let to = segment.toLocation.name.lowercased().trimmingCharacters(in: .whitespaces) - return "travel:\(from)->\(to)" + return "travel:\(index):\(from)->\(to)" } /// Move item to a new day and sortOrder position @@ -832,16 +832,21 @@ struct TripDetailView: View { // Apply user overrides on top of computed defaults. for (segmentIndex, segment) in trip.travelSegments.enumerated() { - let travelId = stableTravelAnchorId(segment) + let travelId = stableTravelAnchorId(segment, at: segmentIndex) guard let override = travelOverrides[travelId] else { continue } // Validate override is within valid day range if let validRange = validDayRange(for: travelId), validRange.contains(override.day) { - // Remove from computed position - travelByDay = travelByDay.filter { $0.value.id != segment.id } + // Remove from computed position (search all days) + for key in travelByDay.keys { + travelByDay[key]?.removeAll { $0.id == segment.id } + } + travelByDay.keys.forEach { key in + if travelByDay[key]?.isEmpty == true { travelByDay[key] = nil } + } // Place at overridden position - travelByDay[override.day] = segment + travelByDay[override.day, default: []].append(segment) } } @@ -851,8 +856,9 @@ struct TripDetailView: View { let gamesOnDay = gamesOn(date: dayDate) // Travel for this day (if any) - appears before day header - if let travelSegment = travelByDay[dayNum] { - sections.append(.travel(travelSegment)) + for travelSegment in (travelByDay[dayNum] ?? []) { + let segIdx = trip.travelSegments.firstIndex(where: { $0.id == travelSegment.id }) ?? 0 + sections.append(.travel(travelSegment, segmentIndex: segIdx)) } // Day section - shows games or minimal rest day display @@ -938,8 +944,8 @@ struct TripDetailView: View { /// Get valid day range for a travel segment using stop indices. /// Uses the from/to stop dates so repeat cities don't confuse placement. private func validDayRange(for travelId: String) -> ClosedRange? { - // Find the segment index matching this travel ID - guard let segmentIndex = trip.travelSegments.firstIndex(where: { stableTravelAnchorId($0) == travelId }), + // Parse segment index from travel ID (format: "travel:INDEX:from->to") + guard let segmentIndex = Self.parseSegmentIndex(from: travelId), segmentIndex < trip.stops.count - 1 else { return nil } @@ -960,6 +966,15 @@ struct TripDetailView: View { return minDay...maxDay } + /// Parse the segment index from a travel anchor ID. + /// Format: "travel:INDEX:from->to" → INDEX + private static func parseSegmentIndex(from travelId: String) -> Int? { + let stripped = travelId.replacingOccurrences(of: "travel:", with: "") + let parts = stripped.components(separatedBy: ":") + guard parts.count >= 2, let index = Int(parts[0]) else { return nil } + return index + } + // MARK: - Map Helpers private func fetchDrivingRoutes() async { @@ -1231,8 +1246,8 @@ struct TripDetailView: View { } // 2. Add travel items (from trip segments + overrides) - for segment in trip.travelSegments { - let travelId = "travel:\(segment.fromLocation.name.lowercased())->\(segment.toLocation.name.lowercased())" + for (segmentIndex, segment) in trip.travelSegments.enumerated() { + let travelId = stableTravelAnchorId(segment, at: segmentIndex) // Use override if available, otherwise default to day 1 let override = travelOverrides[travelId] @@ -1246,6 +1261,7 @@ struct TripDetailView: View { kind: .travel(TravelInfo( fromCity: segment.fromLocation.name, toCity: segment.toLocation.name, + segmentIndex: segmentIndex, distanceMeters: segment.distanceMeters, durationSeconds: segment.durationSeconds )) @@ -1371,11 +1387,24 @@ struct TripDetailView: View { for item in items where item.isTravel { guard let travelInfo = item.travelInfo else { continue } - let travelId = "travel:\(travelInfo.fromCity.lowercased())->\(travelInfo.toCity.lowercased())" + let from = travelInfo.fromCity.lowercased() + let to = travelInfo.toCity.lowercased() - overrides[travelId] = TravelOverride(day: item.day, sortOrder: item.sortOrder) + if let segIdx = travelInfo.segmentIndex { + // New format with segment index + let travelId = "travel:\(segIdx):\(from)->\(to)" + overrides[travelId] = TravelOverride(day: item.day, sortOrder: item.sortOrder) + } else { + // Legacy record without segment index — derive index from trip segments + if let segIdx = trip.travelSegments.firstIndex(where: { + $0.fromLocation.name.lowercased() == from && $0.toLocation.name.lowercased() == to + }) { + let travelId = "travel:\(segIdx):\(from)->\(to)" + overrides[travelId] = TravelOverride(day: item.day, sortOrder: item.sortOrder) + } + } } - + travelOverrides = overrides print("✅ [TravelOverrides] Extracted \(overrides.count) travel overrides (day + sortOrder)") } catch { @@ -1502,20 +1531,30 @@ struct TripDetailView: View { private func saveTravelDayOverride(travelAnchorId: String, displayDay: Int, sortOrder: Double) async { print("💾 [TravelOverrides] Saving override: \(travelAnchorId) -> day \(displayDay), sortOrder \(sortOrder)") - // Parse travel ID to extract cities (format: "travel:city1->city2") + // Parse travel ID (format: "travel:INDEX:from->to") + let segmentIndex = Self.parseSegmentIndex(from: travelAnchorId) let stripped = travelAnchorId.replacingOccurrences(of: "travel:", with: "") - let parts = stripped.components(separatedBy: "->") - guard parts.count == 2 else { + let colonParts = stripped.components(separatedBy: ":") + // After removing "travel:", format is "INDEX:from->to" + let cityPart = colonParts.count >= 2 ? colonParts.dropFirst().joined(separator: ":") : stripped + let cityParts = cityPart.components(separatedBy: "->") + guard cityParts.count == 2 else { print("❌ [TravelOverrides] Invalid travel ID format: \(travelAnchorId)") return } - let fromCity = parts[0] - let toCity = parts[1] + let fromCity = cityParts[0] + let toCity = cityParts[1] - // Find existing travel item or create new one + // Find existing travel item matching by segment index (preferred) or city pair (legacy) if let existingIndex = itineraryItems.firstIndex(where: { - $0.isTravel && $0.travelInfo?.fromCity.lowercased() == fromCity && $0.travelInfo?.toCity.lowercased() == toCity + guard $0.isTravel, let info = $0.travelInfo else { return false } + // Match by segment index if available + if let idx = segmentIndex, let itemIdx = info.segmentIndex { + return idx == itemIdx + } + // Legacy fallback: match by city pair + return info.fromCity.lowercased() == fromCity && info.toCity.lowercased() == toCity }) { // Update existing var updated = itineraryItems[existingIndex] @@ -1526,7 +1565,7 @@ struct TripDetailView: View { await ItineraryItemService.shared.updateItem(updated) } else { // Create new travel item - let travelInfo = TravelInfo(fromCity: fromCity, toCity: toCity) + let travelInfo = TravelInfo(fromCity: fromCity, toCity: toCity, segmentIndex: segmentIndex) let item = ItineraryItem( tripId: trip.id, day: displayDay, @@ -1549,7 +1588,7 @@ struct TripDetailView: View { enum ItinerarySection { case day(dayNumber: Int, date: Date, games: [RichGame]) - case travel(TravelSegment) + case travel(TravelSegment, segmentIndex: Int) case customItem(ItineraryItem) case addButton(day: Int) diff --git a/SportsTimeTests/Features/Trip/ItineraryReorderingLogicTests.swift b/SportsTimeTests/Features/Trip/ItineraryReorderingLogicTests.swift index faa0184..459d28e 100644 --- a/SportsTimeTests/Features/Trip/ItineraryReorderingLogicTests.swift +++ b/SportsTimeTests/Features/Trip/ItineraryReorderingLogicTests.swift @@ -833,7 +833,7 @@ final class ItineraryReorderingLogicTests: XCTestCase { ]) let segment = H.makeTravelSegment(from: "CityA", to: "CityB") - let travelValidRanges = ["travel:citya->cityb": 1...3] + let travelValidRanges = ["travel:0:citya->cityb": 1...3] let zones = Logic.calculateTravelDragZones( segment: segment, diff --git a/SportsTimeTests/Features/Trip/ItineraryRowFlatteningTests.swift b/SportsTimeTests/Features/Trip/ItineraryRowFlatteningTests.swift index e6d5d4e..05b3606 100644 --- a/SportsTimeTests/Features/Trip/ItineraryRowFlatteningTests.swift +++ b/SportsTimeTests/Features/Trip/ItineraryRowFlatteningTests.swift @@ -170,10 +170,11 @@ final class ItineraryRowFlatteningTests: XCTestCase { XCTAssertEqual(item.id, "games:2") } - func test_itineraryRowItem_travel_hasLowercaseId() { + func test_itineraryRowItem_travel_hasStableId() { let segment = H.makeTravelSegment(from: "Chicago", to: "Detroit") let item = ItineraryRowItem.travel(segment, dayNumber: 1) - XCTAssertEqual(item.id, "travel:chicago->detroit", "Travel ID should be lowercase") + XCTAssertTrue(item.id.hasPrefix("travel:"), "Travel ID should start with 'travel:'") + XCTAssertTrue(item.id.contains(segment.id.uuidString), "Travel ID should contain segment UUID") } func test_itineraryRowItem_customItem_hasUuidId() { diff --git a/SportsTimeTests/Features/Trip/ItinerarySemanticTravelTests.swift b/SportsTimeTests/Features/Trip/ItinerarySemanticTravelTests.swift index c9df915..47e42ef 100644 --- a/SportsTimeTests/Features/Trip/ItinerarySemanticTravelTests.swift +++ b/SportsTimeTests/Features/Trip/ItinerarySemanticTravelTests.swift @@ -273,7 +273,7 @@ final class ItinerarySemanticTravelTests: XCTestCase { let constraints = ItineraryConstraints(tripDayCount: 4, items: [gameItemA, gameItemB]) - let travelValidRanges = ["travel:citya->cityb": 1...4] + let travelValidRanges = ["travel:0:citya->cityb": 1...4] let validRows = Logic.computeValidDestinationRowsProposed( flatItems: items, diff --git a/SportsTimeTests/Features/Trip/ItineraryTravelConstraintTests.swift b/SportsTimeTests/Features/Trip/ItineraryTravelConstraintTests.swift index 2516ffd..3388669 100644 --- a/SportsTimeTests/Features/Trip/ItineraryTravelConstraintTests.swift +++ b/SportsTimeTests/Features/Trip/ItineraryTravelConstraintTests.swift @@ -182,7 +182,7 @@ final class ItineraryTravelConstraintTests: XCTestCase { } controller.reloadData( days: [dayData], - travelValidRanges: ["travel:chicago->detroit": 1...1], + travelValidRanges: ["travel:0:chicago->detroit": 1...1], itineraryItems: [chicagoGame, travel] ) @@ -214,7 +214,7 @@ final class ItineraryTravelConstraintTests: XCTestCase { } controller.reloadData( days: [day1, day2, day3], - travelValidRanges: ["travel:chicago->detroit": 2...3], + travelValidRanges: ["travel:0:chicago->detroit": 2...3], itineraryItems: [travelModelItem] ) @@ -222,7 +222,7 @@ final class ItineraryTravelConstraintTests: XCTestCase { // Move travel (row 2) to row 3 (after Day3 header = Day 3) controller.tableView(controller.tableView, moveRowAt: IndexPath(row: 2, section: 0), to: IndexPath(row: 3, section: 0)) - XCTAssertEqual(capturedTravelId, "travel:chicago->detroit") + XCTAssertEqual(capturedTravelId, "travel:0:chicago->detroit") XCTAssertEqual(capturedDay, 3, "Travel should now be on Day 3") } @@ -231,7 +231,7 @@ final class ItineraryTravelConstraintTests: XCTestCase { func test_moveValidation_travel_snapsToValidDayRange() { // Given: Travel with valid range Days 2-3 let travel = H.makeTravelSegment(from: "Chicago", to: "Detroit") - let travelId = "travel:chicago->detroit" + let travelId = "travel:0:chicago->detroit" let travelItem = ItineraryRowItem.travel(travel, dayNumber: 2) let travelModelItem = H.makeTravelItem(from: "Chicago", to: "Detroit", day: 2, sortOrder: 1.0) diff --git a/SportsTimeTests/Features/Trip/TravelPlacementTests.swift b/SportsTimeTests/Features/Trip/TravelPlacementTests.swift index a2e74e2..c7b0934 100644 --- a/SportsTimeTests/Features/Trip/TravelPlacementTests.swift +++ b/SportsTimeTests/Features/Trip/TravelPlacementTests.swift @@ -358,4 +358,121 @@ final class TravelPlacementTests: XCTestCase { XCTAssertLessThanOrEqual(day, 6, "B→C travel should be on or before Day 6") } } + + // MARK: - Same-Day Collision (dictionary overwrite bug) + + func test_twoSegmentsSameDay_neitherLost() { + // 3 back-to-back single-game stops with next-morning departures: + // Stop A: arrival May 1, departure May 2 + // Stop B: arrival May 2, departure May 3 + // Stop C: arrival May 3, departure May 4 + // + // Segment 0 (A→B): fromDayNum=2, toDayNum=2 → defaultDay=2, clampedDefault=2 + // Segment 1 (B→C): fromDayNum=3, toDayNum=3 → defaultDay=3, clampedDefault=3 + // + // These don't collide, but a tighter scenario does: + // If A departs May 2 and B arrives May 2 AND departs May 2, C arrives May 2: + // Both segments resolve to Day 2 → collision. + // + // Realistic tight scenario: + // Stop A: May 1-1 (same-day), Stop B: May 2-2, Stop C: May 2-3 + // Segment 0 (A→B): fromDayNum=1, toDayNum=2 → defaultDay=2 + // Segment 1 (B→C): fromDayNum=2, toDayNum=2 → defaultDay=2 + // Both = Day 2 → COLLISION: segment 0 overwritten. + let stops = [ + makeStop(city: "CityA", arrival: may(1), departure: may(1)), + makeStop(city: "CityB", arrival: may(2), departure: may(2)), + makeStop(city: "CityC", arrival: may(2), departure: may(3)) + ] + let segments = [ + makeSegment(from: "CityA", to: "CityB"), + makeSegment(from: "CityB", to: "CityC") + ] + + let trip = Trip( + name: "Tight Trip", + preferences: TripPreferences(), + stops: stops, + travelSegments: segments, + totalGames: 0, + totalDistanceMeters: 0, + totalDrivingSeconds: 0 + ) + + let days = tripDays(from: may(1), to: may(3)) + let result = TravelPlacement.computeTravelByDay(trip: trip, tripDays: days) + + // Both segments must be present — neither should be silently dropped + let allSegments = result.values.flatMap { $0 } + XCTAssertEqual(allSegments.count, 2, "Both travel segments must be preserved (no collision)") + + // Day 2 should have both segments + let day2Segments = result[2] ?? [] + XCTAssertEqual(day2Segments.count, 2, "Day 2 should have 2 travel segments") + } + + // MARK: - Repeat City Pair ID Collision + + func test_repeatCityPair_overridesDoNotCollide() { + // Follow Team pattern: Houston→Cincinnati→Houston→Cincinnati + // Two segments share the same city pair (Houston→Cincinnati) + // Each must get a unique travel anchor ID so overrides don't collide. + let stops = [ + makeStop(city: "Houston", arrival: may(1), departure: may(2)), // Stop 0 + makeStop(city: "Cincinnati", arrival: may(4), departure: may(5)), // Stop 1 + makeStop(city: "Houston", arrival: may(7), departure: may(8)), // Stop 2 + makeStop(city: "Cincinnati", arrival: may(10), departure: may(11)) // Stop 3 + ] + + let segments = [ + makeSegment(from: "Houston", to: "Cincinnati"), // Seg 0: stops[0] → stops[1] + makeSegment(from: "Cincinnati", to: "Houston"), // Seg 1: stops[1] → stops[2] + makeSegment(from: "Houston", to: "Cincinnati") // Seg 2: stops[2] → stops[3] + ] + + // Simulate what stableTravelAnchorId should produce WITH segment index + let id0 = "travel:0:houston->cincinnati" + let id1 = "travel:1:cincinnati->houston" + let id2 = "travel:2:houston->cincinnati" + + // All IDs must be unique + XCTAssertNotEqual(id0, id2, "Repeat city pair segments must have unique IDs") + XCTAssertEqual(Set([id0, id1, id2]).count, 3, "All 3 travel IDs must be unique") + + // Simulate overrides dictionary — each segment gets its own entry + var overrides: [String: Int] = [:] // travel ID → day override + overrides[id0] = 3 // Seg 0 overridden to day 3 + overrides[id2] = 9 // Seg 2 overridden to day 9 + + // Verify no collision — seg 0 override is NOT overwritten by seg 2 + XCTAssertEqual(overrides[id0], 3, "Segment 0 override should be day 3") + XCTAssertEqual(overrides[id2], 9, "Segment 2 override should be day 9") + XCTAssertNil(overrides[id1], "Segment 1 has no override") + } + + func test_singleSegmentPerDay_returnsArrayOfOne() { + // Ensure the new array return type still works for the simple case + let stops = [ + makeStop(city: "Houston", arrival: may(1), departure: may(3)), + makeStop(city: "Chicago", arrival: may(5), departure: may(6)) + ] + let segments = [makeSegment(from: "Houston", to: "Chicago")] + + let trip = Trip( + name: "Simple", + preferences: TripPreferences(), + stops: stops, + travelSegments: segments, + totalGames: 0, + totalDistanceMeters: 0, + totalDrivingSeconds: 0 + ) + + let days = tripDays(from: may(1), to: may(6)) + let result = TravelPlacement.computeTravelByDay(trip: trip, tripDays: days) + + XCTAssertEqual(result.count, 1, "Should have 1 day with travel") + let day4Segments = result[4] ?? [] + XCTAssertEqual(day4Segments.count, 1, "Day 4 should have exactly 1 travel segment") + } }