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 <noreply@anthropic.com>
This commit is contained in:
@@ -45,7 +45,9 @@ enum TravelPlacement {
|
|||||||
|
|
||||||
minDay = max(fromDayNum + 1, 1)
|
minDay = max(fromDayNum + 1, 1)
|
||||||
maxDay = min(toDayNum, tripDays.count)
|
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 {
|
} else {
|
||||||
minDay = 1
|
minDay = 1
|
||||||
maxDay = tripDays.count
|
maxDay = tripDays.count
|
||||||
@@ -56,7 +58,9 @@ enum TravelPlacement {
|
|||||||
if minDay <= maxDay {
|
if minDay <= maxDay {
|
||||||
clampedDefault = max(minDay, min(defaultDay, maxDay))
|
clampedDefault = max(minDay, min(defaultDay, maxDay))
|
||||||
} else {
|
} 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
|
travelByDay[clampedDefault] = segment
|
||||||
|
|||||||
@@ -827,46 +827,21 @@ struct TripDetailView: View {
|
|||||||
var sections: [ItinerarySection] = []
|
var sections: [ItinerarySection] = []
|
||||||
let days = tripDays
|
let days = tripDays
|
||||||
|
|
||||||
// Pre-calculate which day each travel segment belongs to.
|
// Use TravelPlacement for consistent day calculation (shared with tests).
|
||||||
// Uses stop indices (not city name matching) so repeat cities work correctly.
|
var travelByDay = TravelPlacement.computeTravelByDay(trip: trip, tripDays: days)
|
||||||
// trip.travelSegments[i] connects trip.stops[i] → trip.stops[i+1].
|
|
||||||
var travelByDay: [Int: TravelSegment] = [:]
|
// Apply user overrides on top of computed defaults.
|
||||||
for (segmentIndex, segment) in trip.travelSegments.enumerated() {
|
for (segmentIndex, segment) in trip.travelSegments.enumerated() {
|
||||||
let travelId = stableTravelAnchorId(segment)
|
let travelId = stableTravelAnchorId(segment)
|
||||||
|
guard let override = travelOverrides[travelId] else { continue }
|
||||||
|
|
||||||
// Use stop dates for precise placement (handles repeat cities)
|
// Validate override is within valid day range
|
||||||
let minDay: Int
|
if let validRange = validDayRange(for: travelId),
|
||||||
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],
|
|
||||||
validRange.contains(override.day) {
|
validRange.contains(override.day) {
|
||||||
|
// Remove from computed position
|
||||||
|
travelByDay = travelByDay.filter { $0.value.id != segment.id }
|
||||||
|
// Place at overridden position
|
||||||
travelByDay[override.day] = segment
|
travelByDay[override.day] = segment
|
||||||
} else {
|
|
||||||
// Use default (clamped to valid range)
|
|
||||||
let clampedDefault = max(validRange.lowerBound, min(defaultDay, validRange.upperBound))
|
|
||||||
travelByDay[clampedDefault] = segment
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -25,6 +25,15 @@ final class TravelPlacementTests: XCTestCase {
|
|||||||
return calendar.startOfDay(for: calendar.date(from: components)!)
|
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).
|
/// Build an array of trip days from start to end (inclusive).
|
||||||
private func tripDays(from start: Date, to end: Date) -> [Date] {
|
private func tripDays(from start: Date, to end: Date) -> [Date] {
|
||||||
var days: [Date] = []
|
var days: [Date] = []
|
||||||
@@ -230,4 +239,123 @@ final class TravelPlacementTests: XCTestCase {
|
|||||||
XCTAssertEqual(result.count, 1, "Travel segment should be placed")
|
XCTAssertEqual(result.count, 1, "Travel segment should be placed")
|
||||||
XCTAssertNotNil(result[2], "Travel should be on Day 2 (arrival day)")
|
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")
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user