From 633f7d883f9923045b87b79bdce4f85dc62e23a8 Mon Sep 17 00:00:00 2001 From: Trey t Date: Wed, 11 Feb 2026 09:53:25 -0600 Subject: [PATCH] fix: correct travel segment placement for next-day departures Travel segments appeared one day too late on featured/suggested trips when stops had next-morning departures. The placement formula double- counted by using fromDayNum+1 as both minDay and defaultDay, then the invalid-range fallback used minDay (the overshooting value) instead of the arrival day. Also replaced TripDetailView's inline copy of the placement logic with TravelPlacement.computeTravelByDay() so the UI uses the same tested algorithm. Co-Authored-By: Claude Opus 4.6 --- .../Features/Trip/Views/TravelPlacement.swift | 8 +- .../Features/Trip/Views/TripDetailView.swift | 45 ++---- .../Features/Trip/TravelPlacementTests.swift | 128 ++++++++++++++++++ 3 files changed, 144 insertions(+), 37 deletions(-) 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") + } + } }