fix: resolve travel anchor ID collision for repeat city pairs
Include segment index in travel anchor IDs ("travel:INDEX:from->to")
so Follow Team trips visiting the same city pair multiple times get
unique, independently addressable travel segments. Prevents override
dictionary collisions and incorrect validDayRange lookups.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -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,
|
||||
|
||||
@@ -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() {
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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)
|
||||
|
||||
|
||||
@@ -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")
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user