diff --git a/SportsTime/Features/Trip/Views/TravelPlacement.swift b/SportsTime/Features/Trip/Views/TravelPlacement.swift index 8c36585..8539df6 100644 --- a/SportsTime/Features/Trip/Views/TravelPlacement.swift +++ b/SportsTime/Features/Trip/Views/TravelPlacement.swift @@ -45,7 +45,9 @@ enum TravelPlacement { minDay = max(fromDayNum + 1, 1) maxDay = min(toDayNum, tripDays.count) - defaultDay = minDay + // Cap default at the arrival day to prevent overshoot when + // departure is already the next morning (fromDayNum+1 > toDayNum). + defaultDay = min(fromDayNum + 1, toDayNum) } else { minDay = 1 maxDay = tripDays.count @@ -56,7 +58,9 @@ enum TravelPlacement { if minDay <= maxDay { clampedDefault = max(minDay, min(defaultDay, maxDay)) } else { - clampedDefault = minDay + // Invalid range (departure day is at or past arrival day). + // Fall back to arrival day, clamped within trip bounds. + clampedDefault = max(1, min(defaultDay, tripDays.count)) } travelByDay[clampedDefault] = segment diff --git a/SportsTime/Features/Trip/Views/TripDetailView.swift b/SportsTime/Features/Trip/Views/TripDetailView.swift index 5e942d2..090ac07 100644 --- a/SportsTime/Features/Trip/Views/TripDetailView.swift +++ b/SportsTime/Features/Trip/Views/TripDetailView.swift @@ -827,46 +827,21 @@ struct TripDetailView: View { var sections: [ItinerarySection] = [] let days = tripDays - // Pre-calculate which day each travel segment belongs to. - // Uses stop indices (not city name matching) so repeat cities work correctly. - // trip.travelSegments[i] connects trip.stops[i] → trip.stops[i+1]. - var travelByDay: [Int: TravelSegment] = [:] + // Use TravelPlacement for consistent day calculation (shared with tests). + var travelByDay = TravelPlacement.computeTravelByDay(trip: trip, tripDays: days) + + // Apply user overrides on top of computed defaults. for (segmentIndex, segment) in trip.travelSegments.enumerated() { let travelId = stableTravelAnchorId(segment) + guard let override = travelOverrides[travelId] else { continue } - // Use stop dates for precise placement (handles repeat cities) - let minDay: Int - let maxDay: Int - let defaultDay: Int - - if segmentIndex < trip.stops.count - 1 { - let fromStop = trip.stops[segmentIndex] - let toStop = trip.stops[segmentIndex + 1] - - let fromDayNum = dayNumber(for: fromStop.departureDate) - let toDayNum = dayNumber(for: toStop.arrivalDate) - - // Travel goes after the from stop's last day, up to the to stop's first day - minDay = max(fromDayNum + 1, 1) - maxDay = min(toDayNum, days.count) - defaultDay = minDay - } else { - // Fallback: segment doesn't align with stops - minDay = 1 - maxDay = days.count - defaultDay = 1 - } - - let validRange = minDay <= maxDay ? minDay...maxDay : minDay...minDay - - // Check for user override - only use if within valid range - if let override = travelOverrides[travelId], + // 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 } + // Place at overridden position travelByDay[override.day] = segment - } else { - // Use default (clamped to valid range) - let clampedDefault = max(validRange.lowerBound, min(defaultDay, validRange.upperBound)) - travelByDay[clampedDefault] = segment } } diff --git a/SportsTimeTests/Features/Trip/TravelPlacementTests.swift b/SportsTimeTests/Features/Trip/TravelPlacementTests.swift index d2476a7..a2e74e2 100644 --- a/SportsTimeTests/Features/Trip/TravelPlacementTests.swift +++ b/SportsTimeTests/Features/Trip/TravelPlacementTests.swift @@ -25,6 +25,15 @@ final class TravelPlacementTests: XCTestCase { return calendar.startOfDay(for: calendar.date(from: components)!) } + /// Create a date for March 2026 at a given day number. + private func mar(_ day: Int) -> Date { + var components = DateComponents() + components.year = 2026 + components.month = 3 + components.day = day + return calendar.startOfDay(for: calendar.date(from: components)!) + } + /// Build an array of trip days from start to end (inclusive). private func tripDays(from start: Date, to end: Date) -> [Date] { var days: [Date] = [] @@ -230,4 +239,123 @@ final class TravelPlacementTests: XCTestCase { XCTAssertEqual(result.count, 1, "Travel segment should be placed") XCTAssertNotNil(result[2], "Travel should be on Day 2 (arrival day)") } + + // MARK: - Next-Day Departure Bug (Featured/Suggested trips) + + func test_nextDayDeparture_travelPlacedOnArrivalDay() { + // The exact bug scenario from featured trips: + // Clearwater arrives May 1, departs May 2 (next-morning departure) + // New Orleans arrives May 2, departs May 3 + // + // BUG: fromDayNum=2 (May 2), minDay=fromDayNum+1=3, toDayNum=2 (May 2) + // minDay(3) > maxDay(2) → fallback uses minDay=3 → travel on Day 3 (WRONG) + // FIX: Travel should be on Day 2 (the arrival day) + let stops = [ + makeStop(city: "Clearwater", arrival: may(1), departure: may(2)), + makeStop(city: "New Orleans", arrival: may(2), departure: may(3)) + ] + let segments = [makeSegment(from: "Clearwater", to: "New Orleans")] + + let trip = Trip( + name: "Featured 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) + + XCTAssertEqual(result.count, 1, "Should have 1 travel segment placed") + XCTAssertNotNil(result[2], "Travel should be on Day 2 (arrival day, not Day 3)") + XCTAssertNil(result[3], "Travel should NOT be on Day 3 (the overshoot)") + } + + func test_nextDayDeparture_withGap_travelPlacedCorrectly() { + // Next-day departure with a multi-day gap to next stop: + // Stop A arrives May 1, departs May 2 (next morning) + // Stop B arrives May 5, departs May 6 + // + // fromDayNum=2, minDay=3, toDayNum=5 → minDay(3) <= maxDay(5) → travel on Day 3 + // But ideally travel should be on Day 2 (the departure day itself). + // After fix: defaultDay = min(fromDayNum+1, toDayNum) = min(3, 5) = 3 + // This is still valid (day 3 is within range), but the key fix is the + // invalid-range case doesn't apply here. + let stops = [ + makeStop(city: "StopA", arrival: may(1), departure: may(2)), + makeStop(city: "StopB", arrival: may(5), departure: may(6)) + ] + let segments = [makeSegment(from: "StopA", to: "StopB")] + + let trip = Trip( + name: "Gap Trip", + 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 travel segment placed") + // Travel should be within valid range [3...5], defaulting to Day 3 + let placedDay = result.keys.first! + XCTAssertGreaterThanOrEqual(placedDay, 2, "Travel should be on or after departure day") + XCTAssertLessThanOrEqual(placedDay, 5, "Travel should be on or before arrival day") + } + + func test_threeStops_nextDayDeparture_allSegmentsPlaced() { + // Full featured trip pattern with 3 stops, all next-day departures: + // Stop A: arrival Mar 15, departure Mar 16 + // Stop B: arrival Mar 16, departure Mar 17 + // Stop C: arrival Mar 20, departure Mar 21 + // + // Segment 0 (A→B): fromDayNum=2 (Mar 16), toDayNum=2 (Mar 16) + // BUG: minDay=3, maxDay=2 → fallback to minDay=3 (Day 3 = Mar 17) WRONG + // FIX: Travel on Day 2 (Mar 16, arrival day) + // + // Segment 1 (B→C): fromDayNum=3 (Mar 17), toDayNum=6 (Mar 20) + // minDay=4, maxDay=6 → travel on Day 4 (Mar 18) — this works correctly + let stops = [ + makeStop(city: "StopA", arrival: mar(15), departure: mar(16)), + makeStop(city: "StopB", arrival: mar(16), departure: mar(17)), + makeStop(city: "StopC", arrival: mar(20), departure: mar(21)) + ] + let segments = [ + makeSegment(from: "StopA", to: "StopB"), + makeSegment(from: "StopB", to: "StopC") + ] + + let trip = Trip( + name: "Three Stop Featured", + preferences: TripPreferences(), + stops: stops, + travelSegments: segments, + totalGames: 0, + totalDistanceMeters: 0, + totalDrivingSeconds: 0 + ) + + let days = tripDays(from: mar(15), to: mar(21)) + let result = TravelPlacement.computeTravelByDay(trip: trip, tripDays: days) + + XCTAssertEqual(result.count, 2, "Both travel segments should be placed") + + // Segment 0 (A→B): should be on Day 2 (Mar 16, the arrival/departure day) + XCTAssertNotNil(result[2], "A→B travel should be on Day 2 (Mar 16)") + + // Segment 1 (B→C): should be on Day 3 or 4 (Mar 17 or 18, within valid range) + let seg1Day = result.first(where: { $0.key != 2 })?.key + XCTAssertNotNil(seg1Day, "B→C travel should be placed") + if let day = seg1Day { + XCTAssertGreaterThanOrEqual(day, 3, "B→C travel should be on or after Day 3") + XCTAssertLessThanOrEqual(day, 6, "B→C travel should be on or before Day 6") + } + } }