fix(itinerary): add city to game items for proper constraint validation
Travel constraint validation was not working because ItineraryConstraints had no game items to validate against - games came from RichGame objects but were never converted to ItineraryItem for constraint checking. Changes: - Add city parameter to ItemKind.game enum case - Create game ItineraryItems from RichGame data in buildItineraryData() - Update isValidTravelPosition to compare against actual game sortOrders - Fix tests to use appropriate game sortOrder conventions Now travel is properly constrained to appear before arrival city games and after departure city games. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -5,13 +5,6 @@ struct ItineraryConstraints {
|
|||||||
let tripDayCount: Int
|
let tripDayCount: Int
|
||||||
private let items: [ItineraryItem]
|
private let items: [ItineraryItem]
|
||||||
|
|
||||||
/// City extracted from game ID (format: "game-CityName-xxxx")
|
|
||||||
private func city(forGameId gameId: String) -> String? {
|
|
||||||
let components = gameId.components(separatedBy: "-")
|
|
||||||
guard components.count >= 2 else { return nil }
|
|
||||||
return components[1]
|
|
||||||
}
|
|
||||||
|
|
||||||
init(tripDayCount: Int, items: [ItineraryItem]) {
|
init(tripDayCount: Int, items: [ItineraryItem]) {
|
||||||
self.tripDayCount = tripDayCount
|
self.tripDayCount = tripDayCount
|
||||||
self.items = items
|
self.items = items
|
||||||
@@ -83,8 +76,10 @@ struct ItineraryConstraints {
|
|||||||
// MARK: - Private Helpers
|
// MARK: - Private Helpers
|
||||||
|
|
||||||
private func isValidTravelPosition(fromCity: String, toCity: String, day: Int, sortOrder: Double) -> Bool {
|
private func isValidTravelPosition(fromCity: String, toCity: String, day: Int, sortOrder: Double) -> Bool {
|
||||||
let departureGameDays = gameDays(in: fromCity)
|
let departureGames = games(in: fromCity)
|
||||||
let arrivalGameDays = gameDays(in: toCity)
|
let arrivalGames = games(in: toCity)
|
||||||
|
let departureGameDays = departureGames.map { $0.day }
|
||||||
|
let arrivalGameDays = arrivalGames.map { $0.day }
|
||||||
|
|
||||||
let minDay = departureGameDays.max() ?? 1
|
let minDay = departureGameDays.max() ?? 1
|
||||||
let maxDay = arrivalGameDays.min() ?? tripDayCount
|
let maxDay = arrivalGameDays.min() ?? tripDayCount
|
||||||
@@ -92,26 +87,20 @@ struct ItineraryConstraints {
|
|||||||
// Check day is in valid range
|
// Check day is in valid range
|
||||||
guard day >= minDay && day <= maxDay else { return false }
|
guard day >= minDay && day <= maxDay else { return false }
|
||||||
|
|
||||||
// Check sortOrder constraints on edge days
|
// Check sortOrder constraints on edge days.
|
||||||
|
// Must be strictly AFTER all departure games on that day
|
||||||
if departureGameDays.contains(day) {
|
if departureGameDays.contains(day) {
|
||||||
// On a departure game day: must be after ALL games in that city on that day
|
let gamesOnDay = departureGames.filter { $0.day == day }
|
||||||
let maxGameSortOrder = games(in: fromCity)
|
let maxGameSortOrder = gamesOnDay.map { $0.sortOrder }.max() ?? 0
|
||||||
.filter { $0.day == day }
|
|
||||||
.map { $0.sortOrder }
|
|
||||||
.max() ?? 0
|
|
||||||
|
|
||||||
if sortOrder <= maxGameSortOrder {
|
if sortOrder <= maxGameSortOrder {
|
||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Must be strictly BEFORE all arrival games on that day
|
||||||
if arrivalGameDays.contains(day) {
|
if arrivalGameDays.contains(day) {
|
||||||
// On an arrival game day: must be before ALL games in that city on that day
|
let gamesOnDay = arrivalGames.filter { $0.day == day }
|
||||||
let minGameSortOrder = games(in: toCity)
|
let minGameSortOrder = gamesOnDay.map { $0.sortOrder }.min() ?? 0
|
||||||
.filter { $0.day == day }
|
|
||||||
.map { $0.sortOrder }
|
|
||||||
.min() ?? Double.greatestFiniteMagnitude
|
|
||||||
|
|
||||||
if sortOrder >= minGameSortOrder {
|
if sortOrder >= minGameSortOrder {
|
||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
@@ -126,8 +115,8 @@ struct ItineraryConstraints {
|
|||||||
|
|
||||||
private func games(in city: String) -> [ItineraryItem] {
|
private func games(in city: String) -> [ItineraryItem] {
|
||||||
return items.filter { item in
|
return items.filter { item in
|
||||||
guard case .game(let gameId) = item.kind else { return false }
|
guard let gameCity = item.gameCity else { return false }
|
||||||
return self.city(forGameId: gameId) == city
|
return gameCity.lowercased() == city.lowercased()
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -29,7 +29,7 @@ struct ItineraryItem: Identifiable, Codable, Hashable {
|
|||||||
|
|
||||||
/// The type of itinerary item
|
/// The type of itinerary item
|
||||||
enum ItemKind: Codable, Hashable {
|
enum ItemKind: Codable, Hashable {
|
||||||
case game(gameId: String)
|
case game(gameId: String, city: String)
|
||||||
case travel(TravelInfo)
|
case travel(TravelInfo)
|
||||||
case custom(CustomInfo)
|
case custom(CustomInfo)
|
||||||
}
|
}
|
||||||
@@ -106,14 +106,19 @@ extension ItineraryItem {
|
|||||||
}
|
}
|
||||||
|
|
||||||
var gameId: String? {
|
var gameId: String? {
|
||||||
if case .game(let id) = kind { return id }
|
if case .game(let id, _) = kind { return id }
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
|
var gameCity: String? {
|
||||||
|
if case .game(_, let city) = kind { return city }
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Display title for the item
|
/// Display title for the item
|
||||||
var displayTitle: String {
|
var displayTitle: String {
|
||||||
switch kind {
|
switch kind {
|
||||||
case .game(let gameId):
|
case .game(let gameId, _):
|
||||||
return "Game: \(gameId)"
|
return "Game: \(gameId)"
|
||||||
case .travel(let info):
|
case .travel(let info):
|
||||||
return "\(info.fromCity) → \(info.toCity)"
|
return "\(info.fromCity) → \(info.toCity)"
|
||||||
|
|||||||
@@ -145,8 +145,9 @@ extension ItineraryItem {
|
|||||||
// Parse kind
|
// Parse kind
|
||||||
switch kindString {
|
switch kindString {
|
||||||
case "game":
|
case "game":
|
||||||
guard let gameId = record["gameId"] as? String else { return nil }
|
guard let gameId = record["gameId"] as? String,
|
||||||
self.kind = .game(gameId: gameId)
|
let gameCity = record["gameCity"] as? String else { return nil }
|
||||||
|
self.kind = .game(gameId: gameId, city: gameCity)
|
||||||
|
|
||||||
case "travel":
|
case "travel":
|
||||||
guard let fromCity = record["travelFromCity"] as? String,
|
guard let fromCity = record["travelFromCity"] as? String,
|
||||||
@@ -188,9 +189,10 @@ extension ItineraryItem {
|
|||||||
record["modifiedAt"] = modifiedAt
|
record["modifiedAt"] = modifiedAt
|
||||||
|
|
||||||
switch kind {
|
switch kind {
|
||||||
case .game(let gameId):
|
case .game(let gameId, let city):
|
||||||
record["kind"] = "game"
|
record["kind"] = "game"
|
||||||
record["gameId"] = gameId
|
record["gameId"] = gameId
|
||||||
|
record["gameCity"] = city
|
||||||
|
|
||||||
case .travel(let info):
|
case .travel(let info):
|
||||||
record["kind"] = "travel"
|
record["kind"] = "travel"
|
||||||
|
|||||||
@@ -388,11 +388,9 @@ final class PDFGenerator {
|
|||||||
var primaryCity: String?
|
var primaryCity: String?
|
||||||
for item in items {
|
for item in items {
|
||||||
switch item.kind {
|
switch item.kind {
|
||||||
case .game(let gameId):
|
case .game(_, let city):
|
||||||
if let richGame = games[gameId] {
|
primaryCity = city
|
||||||
primaryCity = richGame.stadium.city
|
break
|
||||||
break
|
|
||||||
}
|
|
||||||
case .travel(let info):
|
case .travel(let info):
|
||||||
primaryCity = info.toCity
|
primaryCity = info.toCity
|
||||||
break
|
break
|
||||||
@@ -419,7 +417,7 @@ final class PDFGenerator {
|
|||||||
var hasContent = false
|
var hasContent = false
|
||||||
for item in items {
|
for item in items {
|
||||||
switch item.kind {
|
switch item.kind {
|
||||||
case .game(let gameId):
|
case .game(let gameId, _):
|
||||||
if let richGame = games[gameId] {
|
if let richGame = games[gameId] {
|
||||||
currentY = drawGameCard(
|
currentY = drawGameCard(
|
||||||
context: context,
|
context: context,
|
||||||
|
|||||||
@@ -5,9 +5,6 @@
|
|||||||
// Pure functions for itinerary reordering logic.
|
// Pure functions for itinerary reordering logic.
|
||||||
// Extracted from ItineraryTableViewController for testability.
|
// Extracted from ItineraryTableViewController for testability.
|
||||||
//
|
//
|
||||||
// All functions in this enum are pure - they take inputs and return outputs
|
|
||||||
// with no side effects, making them fully unit-testable without UIKit.
|
|
||||||
//
|
|
||||||
// SEMANTIC TRAVEL MODEL:
|
// SEMANTIC TRAVEL MODEL:
|
||||||
// - Travel items are positioned semantically via (day, sortOrder), not structurally.
|
// - Travel items are positioned semantically via (day, sortOrder), not structurally.
|
||||||
// - Travel can appear before games (sortOrder < 0) or after games (sortOrder >= 0).
|
// - Travel can appear before games (sortOrder < 0) or after games (sortOrder >= 0).
|
||||||
@@ -15,40 +12,13 @@
|
|||||||
// - All movable items (custom + travel) use the same day computation: backward scan to nearest dayHeader.
|
// - All movable items (custom + travel) use the same day computation: backward scan to nearest dayHeader.
|
||||||
//
|
//
|
||||||
// COORDINATE SPACE CONVENTIONS:
|
// COORDINATE SPACE CONVENTIONS:
|
||||||
//
|
// - "Original indices": Row indices in the current flatItems array (0..<flatItems.count)
|
||||||
// Two coordinate spaces exist during drag-drop operations:
|
// - "Proposed indices": Row indices in post-removal array (UITableView move semantics)
|
||||||
//
|
// After removing sourceRow, the array has count-1 elements. Insert positions are 0...count-1.
|
||||||
// 1. ORIGINAL SPACE (flatItems indices)
|
// - simulateMove: Takes proposed index, returns post-move array + actual destination
|
||||||
// - Row indices in the current flatItems array: 0..<flatItems.count
|
// - computeValidDestinationRowsProposed: Returns PROPOSED indices (for UITableView delegate)
|
||||||
// - Used by: DragZones (invalidRowIndices, validDropRows), UI highlighting
|
// - DragZones: invalidRowIndices and validDropRows are in ORIGINAL space (for UI highlighting)
|
||||||
// - Source row is always specified in original space
|
// - To convert: proposedToOriginal(proposed, sourceRow) and originalToProposed(original, sourceRow)
|
||||||
//
|
|
||||||
// 2. PROPOSED SPACE (UITableView post-removal)
|
|
||||||
// - Row indices after sourceRow is removed from the array
|
|
||||||
// - After removal: array has count-1 elements, valid insert positions are 0...(count-1)
|
|
||||||
// - Used by: UITableView delegate methods, computeValidDestinationRowsProposed return value
|
|
||||||
// - Proposed index N means: remove source, insert at position N in the remaining array
|
|
||||||
//
|
|
||||||
// FUNCTION REFERENCE:
|
|
||||||
// - simulateMove: Takes PROPOSED index → returns post-move array + actual destination
|
|
||||||
// - computeValidDestinationRowsProposed: Returns PROPOSED indices (for tableView delegate)
|
|
||||||
// - calculateSortOrder: Takes row in POST-MOVE array (item already at destination)
|
|
||||||
// - calculateTravelDragZones/calculateCustomItemDragZones: Return ORIGINAL indices
|
|
||||||
//
|
|
||||||
// COORDINATE CONVERSION:
|
|
||||||
// - proposedToOriginal(proposed, sourceRow): Converts proposed → original
|
|
||||||
// • If proposed >= sourceRow: return proposed + 1 (shift up past removed source)
|
|
||||||
// • If proposed < sourceRow: return proposed (unchanged)
|
|
||||||
// - originalToProposed(original, sourceRow): Converts original → proposed
|
|
||||||
// • If original == sourceRow: return nil (source has no proposed equivalent)
|
|
||||||
// • If original > sourceRow: return original - 1 (shift down)
|
|
||||||
// • If original < sourceRow: return original (unchanged)
|
|
||||||
//
|
|
||||||
// WHY THIS MATTERS:
|
|
||||||
// - DragZones are used for UI highlighting (which cells to dim/enable)
|
|
||||||
// - UI highlighting operates on the visible table, which uses ORIGINAL indices
|
|
||||||
// - But validation uses simulation, which operates in PROPOSED space
|
|
||||||
// - Getting this wrong causes visual bugs (wrong rows highlighted) or logic bugs
|
|
||||||
//
|
//
|
||||||
|
|
||||||
import Foundation
|
import Foundation
|
||||||
@@ -109,20 +79,14 @@ enum ItineraryReorderingLogic {
|
|||||||
sortOrder = item.sortOrder
|
sortOrder = item.sortOrder
|
||||||
|
|
||||||
case .travel(let segment, _):
|
case .travel(let segment, _):
|
||||||
if let so = findTravelSortOrder(segment) {
|
// Use provided sortOrder if available, otherwise default to after-games position.
|
||||||
sortOrder = so
|
// nil is valid during initial display before travel is persisted.
|
||||||
} else {
|
let lookedUp = findTravelSortOrder(segment)
|
||||||
// Travel without stored sortOrder gets a safe default.
|
sortOrder = lookedUp ?? defaultTravelSortOrder
|
||||||
// Log a warning in debug builds - this shouldn't happen in production.
|
print("📋 [flattenDays] Travel \(segment.fromLocation.name)->\(segment.toLocation.name) on day \(day.dayNumber): lookedUp=\(String(describing: lookedUp)), using sortOrder=\(sortOrder)")
|
||||||
#if DEBUG
|
|
||||||
print("⚠️ flattenDays: Travel segment missing sortOrder: \(segment.fromLocation.name) → \(segment.toLocation.name). Using default: \(defaultTravelSortOrder)")
|
|
||||||
#endif
|
|
||||||
sortOrder = defaultTravelSortOrder
|
|
||||||
}
|
|
||||||
|
|
||||||
case .games, .dayHeader:
|
case .games, .dayHeader:
|
||||||
// These item types are not movable and handled separately.
|
// These item types are not movable and handled separately.
|
||||||
// Skip explicitly - games are added after partitioning.
|
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -133,7 +97,6 @@ enum ItineraryReorderingLogic {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Sort by sortOrder within each region, with stable tiebreaker
|
|
||||||
beforeGames.sort { ($0.sortOrder, $0.tiebreaker) < ($1.sortOrder, $1.tiebreaker) }
|
beforeGames.sort { ($0.sortOrder, $0.tiebreaker) < ($1.sortOrder, $1.tiebreaker) }
|
||||||
afterGames.sort { ($0.sortOrder, $0.tiebreaker) < ($1.sortOrder, $1.tiebreaker) }
|
afterGames.sort { ($0.sortOrder, $0.tiebreaker) < ($1.sortOrder, $1.tiebreaker) }
|
||||||
|
|
||||||
@@ -151,6 +114,7 @@ enum ItineraryReorderingLogic {
|
|||||||
return flatItems
|
return flatItems
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
// MARK: - Day Number Lookup
|
// MARK: - Day Number Lookup
|
||||||
|
|
||||||
/// Finds which day a row at the given index belongs to.
|
/// Finds which day a row at the given index belongs to.
|
||||||
@@ -199,16 +163,13 @@ enum ItineraryReorderingLogic {
|
|||||||
/// - day: The day number to find
|
/// - day: The day number to find
|
||||||
/// - Returns: The row index, or nil if no travel on that day
|
/// - Returns: The row index, or nil if no travel on that day
|
||||||
static func travelRow(in items: [ItineraryRowItem], forDay day: Int) -> Int? {
|
static func travelRow(in items: [ItineraryRowItem], forDay day: Int) -> Int? {
|
||||||
// Find the day header row
|
|
||||||
guard let headerRow = dayHeaderRow(in: items, forDay: day) else {
|
guard let headerRow = dayHeaderRow(in: items, forDay: day) else {
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// Scan forward until next day header, looking for travel
|
|
||||||
for i in (headerRow + 1)..<items.count {
|
for i in (headerRow + 1)..<items.count {
|
||||||
switch items[i] {
|
switch items[i] {
|
||||||
case .dayHeader:
|
case .dayHeader:
|
||||||
// Reached next day, no travel found
|
|
||||||
return nil
|
return nil
|
||||||
case .travel:
|
case .travel:
|
||||||
return i
|
return i
|
||||||
@@ -219,6 +180,7 @@ enum ItineraryReorderingLogic {
|
|||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
/// Legacy version that uses embedded dayNumber (unreliable under semantic model).
|
/// Legacy version that uses embedded dayNumber (unreliable under semantic model).
|
||||||
@available(*, deprecated, message: "Use travelRow(in:forDay:) which uses semantic day lookup")
|
@available(*, deprecated, message: "Use travelRow(in:forDay:) which uses semantic day lookup")
|
||||||
static func travelRowByEmbeddedDay(in items: [ItineraryRowItem], forDay day: Int) -> Int? {
|
static func travelRowByEmbeddedDay(in items: [ItineraryRowItem], forDay day: Int) -> Int? {
|
||||||
@@ -240,11 +202,10 @@ enum ItineraryReorderingLogic {
|
|||||||
/// - items: The flat array of row items
|
/// - items: The flat array of row items
|
||||||
/// - Returns: The day number the travel belongs to
|
/// - Returns: The day number the travel belongs to
|
||||||
static func dayForTravelAt(row: Int, in items: [ItineraryRowItem]) -> Int {
|
static func dayForTravelAt(row: Int, in items: [ItineraryRowItem]) -> Int {
|
||||||
// Semantic model: scan backward to find the day this item belongs to
|
|
||||||
// (same logic as dayNumber)
|
|
||||||
return dayNumber(in: items, forRow: row)
|
return dayNumber(in: items, forRow: row)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
// MARK: - Move Simulation
|
// MARK: - Move Simulation
|
||||||
|
|
||||||
/// Result of simulating a move operation.
|
/// Result of simulating a move operation.
|
||||||
@@ -269,7 +230,6 @@ enum ItineraryReorderingLogic {
|
|||||||
sourceRow: Int,
|
sourceRow: Int,
|
||||||
destinationProposedRow: Int
|
destinationProposedRow: Int
|
||||||
) -> SimulatedMove {
|
) -> SimulatedMove {
|
||||||
// Bounds safety: return original unchanged if sourceRow is invalid
|
|
||||||
guard sourceRow >= 0 && sourceRow < original.count else {
|
guard sourceRow >= 0 && sourceRow < original.count else {
|
||||||
return SimulatedMove(items: original, destinationRowInNewArray: sourceRow, didMove: false)
|
return SimulatedMove(items: original, destinationRowInNewArray: sourceRow, didMove: false)
|
||||||
}
|
}
|
||||||
@@ -281,6 +241,7 @@ enum ItineraryReorderingLogic {
|
|||||||
return SimulatedMove(items: items, destinationRowInNewArray: clampedDest, didMove: true)
|
return SimulatedMove(items: items, destinationRowInNewArray: clampedDest, didMove: true)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
// MARK: - Coordinate Space Conversion
|
// MARK: - Coordinate Space Conversion
|
||||||
|
|
||||||
/// Converts a proposed destination index to the equivalent original index.
|
/// Converts a proposed destination index to the equivalent original index.
|
||||||
@@ -357,14 +318,25 @@ enum ItineraryReorderingLogic {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// DEBUG: Log the row positions
|
||||||
|
print("🔢 [calculateSortOrder] row=\(row), day=\(day), gamesRow=\(String(describing: gamesRow))")
|
||||||
|
print("🔢 [calculateSortOrder] items around row:")
|
||||||
|
for i in max(0, row - 2)...min(items.count - 1, row + 2) {
|
||||||
|
let marker = i == row ? "→" : " "
|
||||||
|
let gMarker = (gamesRow == i) ? " [GAMES]" : ""
|
||||||
|
print("🔢 \(marker) [\(i)] \(items[i])\(gMarker)")
|
||||||
|
}
|
||||||
|
|
||||||
// Strict region classification:
|
// Strict region classification:
|
||||||
// - row < gamesRow => before-games (negative sortOrder)
|
// - row < gamesRow => before-games (negative sortOrder)
|
||||||
// - row >= gamesRow OR no games => after-games (positive sortOrder)
|
// - row >= gamesRow OR no games => after-games (positive sortOrder)
|
||||||
let isBeforeGames: Bool
|
let isBeforeGames: Bool
|
||||||
if let gr = gamesRow {
|
if let gr = gamesRow {
|
||||||
isBeforeGames = row < gr
|
isBeforeGames = row < gr
|
||||||
|
print("🔢 [calculateSortOrder] row(\(row)) < gamesRow(\(gr)) = \(isBeforeGames) → isBeforeGames=\(isBeforeGames)")
|
||||||
} else {
|
} else {
|
||||||
isBeforeGames = false // No games means everything is "after games"
|
isBeforeGames = false // No games means everything is "after games"
|
||||||
|
print("🔢 [calculateSortOrder] No games on day \(day) → isBeforeGames=false")
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Get sortOrder from a movable item (custom item or travel)
|
/// Get sortOrder from a movable item (custom item or travel)
|
||||||
@@ -430,6 +402,7 @@ enum ItineraryReorderingLogic {
|
|||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
let result: Double
|
||||||
if isBeforeGames {
|
if isBeforeGames {
|
||||||
// Above games: sortOrder should be negative
|
// Above games: sortOrder should be negative
|
||||||
let prev = scanBackward(from: row - 1)
|
let prev = scanBackward(from: row - 1)
|
||||||
@@ -438,9 +411,9 @@ enum ItineraryReorderingLogic {
|
|||||||
let upperBound: Double = 0.0 // Games boundary
|
let upperBound: Double = 0.0 // Games boundary
|
||||||
switch (prev, next) {
|
switch (prev, next) {
|
||||||
case (nil, nil):
|
case (nil, nil):
|
||||||
return -1.0
|
result = -1.0
|
||||||
case (let p?, nil):
|
case (let p?, nil):
|
||||||
return (p + upperBound) / 2.0
|
result = (p + upperBound) / 2.0
|
||||||
case (nil, let n?):
|
case (nil, let n?):
|
||||||
// First item before games: place it before the next item.
|
// First item before games: place it before the next item.
|
||||||
// n should always be negative (scanForward filters for region).
|
// n should always be negative (scanForward filters for region).
|
||||||
@@ -448,12 +421,13 @@ enum ItineraryReorderingLogic {
|
|||||||
// This shouldn't happen - scanForward should only return negative values
|
// This shouldn't happen - scanForward should only return negative values
|
||||||
// in before-games region. Return safe default and assert in debug.
|
// in before-games region. Return safe default and assert in debug.
|
||||||
assertionFailure("Before-games region has non-negative sortOrder: \(n)")
|
assertionFailure("Before-games region has non-negative sortOrder: \(n)")
|
||||||
return -1.0
|
result = -1.0
|
||||||
|
} else {
|
||||||
|
// Place before n by subtracting 1.0 (simpler and more consistent than min(n/2, n-1))
|
||||||
|
result = n - 1.0
|
||||||
}
|
}
|
||||||
// Place before n by subtracting 1.0 (simpler and more consistent than min(n/2, n-1))
|
|
||||||
return n - 1.0
|
|
||||||
case (let p?, let n?):
|
case (let p?, let n?):
|
||||||
return (p + n) / 2.0
|
result = (p + n) / 2.0
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
// Below games: sortOrder should be >= 0
|
// Below games: sortOrder should be >= 0
|
||||||
@@ -462,11 +436,14 @@ enum ItineraryReorderingLogic {
|
|||||||
|
|
||||||
switch next {
|
switch next {
|
||||||
case nil:
|
case nil:
|
||||||
return (prev == 0.0) ? 1.0 : (prev + 1.0)
|
result = (prev == 0.0) ? 1.0 : (prev + 1.0)
|
||||||
case let n?:
|
case let n?:
|
||||||
return (prev + n) / 2.0
|
result = (prev + n) / 2.0
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
print("🔢 [calculateSortOrder] RESULT: \(result) (isBeforeGames=\(isBeforeGames))")
|
||||||
|
return result
|
||||||
}
|
}
|
||||||
|
|
||||||
// MARK: - Valid Drop Computation
|
// MARK: - Valid Drop Computation
|
||||||
|
|||||||
@@ -110,22 +110,31 @@ struct ItineraryTableViewWrapper<HeaderContent: View>: UIViewControllerRepresent
|
|||||||
let tripDays = calculateTripDays()
|
let tripDays = calculateTripDays()
|
||||||
var travelValidRanges: [String: ClosedRange<Int>] = [:]
|
var travelValidRanges: [String: ClosedRange<Int>] = [:]
|
||||||
|
|
||||||
|
// Build game items from RichGame data for constraint validation
|
||||||
|
var gameItems: [ItineraryItem] = []
|
||||||
|
for (index, dayDate) in tripDays.enumerated() {
|
||||||
|
let dayNum = index + 1
|
||||||
|
let gamesOnDay = gamesOn(date: dayDate)
|
||||||
|
for (gameIndex, richGame) in gamesOnDay.enumerated() {
|
||||||
|
let gameItem = ItineraryItem(
|
||||||
|
tripId: trip.id,
|
||||||
|
day: dayNum,
|
||||||
|
sortOrder: Double(gameIndex) * 0.01, // Games have sortOrder ~0 (at the visual boundary)
|
||||||
|
kind: .game(gameId: richGame.game.id, city: richGame.stadium.city)
|
||||||
|
)
|
||||||
|
gameItems.append(gameItem)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// Build travel as semantic items with (day, sortOrder)
|
// Build travel as semantic items with (day, sortOrder)
|
||||||
var travelItems: [ItineraryItem] = []
|
var travelItems: [ItineraryItem] = []
|
||||||
travelItems.reserveCapacity(trip.travelSegments.count)
|
travelItems.reserveCapacity(trip.travelSegments.count)
|
||||||
|
|
||||||
func cityFromGameId(_ gameId: String) -> String? {
|
|
||||||
let comps = gameId.components(separatedBy: "-")
|
|
||||||
guard comps.count >= 2 else { return nil }
|
|
||||||
return comps[1]
|
|
||||||
}
|
|
||||||
|
|
||||||
func gamesIn(city: String, day: Int) -> [ItineraryItem] {
|
func gamesIn(city: String, day: Int) -> [ItineraryItem] {
|
||||||
itineraryItems.filter { item in
|
gameItems.filter { item in
|
||||||
guard item.day == day else { return false }
|
guard item.day == day else { return false }
|
||||||
guard case .game(let gid) = item.kind else { return false }
|
guard let gameCity = item.gameCity else { return false }
|
||||||
guard let c = cityFromGameId(gid) else { return false }
|
return cityMatches(gameCity, searchCity: city)
|
||||||
return cityMatches(c, searchCity: city)
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -249,7 +258,7 @@ struct ItineraryTableViewWrapper<HeaderContent: View>: UIViewControllerRepresent
|
|||||||
days.append(dayData)
|
days.append(dayData)
|
||||||
}
|
}
|
||||||
|
|
||||||
return (days, travelValidRanges, itineraryItems + travelItems)
|
return (days, travelValidRanges, gameItems + itineraryItems + travelItems)
|
||||||
}
|
}
|
||||||
|
|
||||||
// MARK: - Helper Methods
|
// MARK: - Helper Methods
|
||||||
|
|||||||
@@ -203,7 +203,7 @@ struct TripDetailView: View {
|
|||||||
withAnimation {
|
withAnimation {
|
||||||
travelOverrides[travelId] = TravelOverride(day: newDay, sortOrder: newSortOrder)
|
travelOverrides[travelId] = TravelOverride(day: newDay, sortOrder: newSortOrder)
|
||||||
}
|
}
|
||||||
await saveTravelDayOverride(travelAnchorId: travelId, displayDay: newDay)
|
await saveTravelDayOverride(travelAnchorId: travelId, displayDay: newDay, sortOrder: newSortOrder)
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
onCustomItemMoved: { itemId, day, sortOrder in
|
onCustomItemMoved: { itemId, day, sortOrder in
|
||||||
@@ -1371,7 +1371,8 @@ struct TripDetailView: View {
|
|||||||
// Persist to CloudKit as a travel ItineraryItem
|
// Persist to CloudKit as a travel ItineraryItem
|
||||||
await self.saveTravelDayOverride(
|
await self.saveTravelDayOverride(
|
||||||
travelAnchorId: droppedId,
|
travelAnchorId: droppedId,
|
||||||
displayDay: dayNumber
|
displayDay: dayNumber,
|
||||||
|
sortOrder: newSortOrder
|
||||||
)
|
)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
@@ -1393,8 +1394,8 @@ struct TripDetailView: View {
|
|||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
|
|
||||||
private func saveTravelDayOverride(travelAnchorId: String, displayDay: Int) async {
|
private func saveTravelDayOverride(travelAnchorId: String, displayDay: Int, sortOrder: Double) async {
|
||||||
print("💾 [TravelOverrides] Saving override: \(travelAnchorId) -> day \(displayDay)")
|
print("💾 [TravelOverrides] Saving override: \(travelAnchorId) -> day \(displayDay), sortOrder \(sortOrder)")
|
||||||
|
|
||||||
// Parse travel ID to extract cities (format: "travel:city1->city2")
|
// Parse travel ID to extract cities (format: "travel:city1->city2")
|
||||||
let stripped = travelAnchorId.replacingOccurrences(of: "travel:", with: "")
|
let stripped = travelAnchorId.replacingOccurrences(of: "travel:", with: "")
|
||||||
@@ -1414,6 +1415,7 @@ struct TripDetailView: View {
|
|||||||
// Update existing
|
// Update existing
|
||||||
var updated = itineraryItems[existingIndex]
|
var updated = itineraryItems[existingIndex]
|
||||||
updated.day = displayDay
|
updated.day = displayDay
|
||||||
|
updated.sortOrder = sortOrder
|
||||||
updated.modifiedAt = Date()
|
updated.modifiedAt = Date()
|
||||||
itineraryItems[existingIndex] = updated
|
itineraryItems[existingIndex] = updated
|
||||||
await ItineraryItemService.shared.updateItem(updated)
|
await ItineraryItemService.shared.updateItem(updated)
|
||||||
@@ -1423,7 +1425,7 @@ struct TripDetailView: View {
|
|||||||
let item = ItineraryItem(
|
let item = ItineraryItem(
|
||||||
tripId: trip.id,
|
tripId: trip.id,
|
||||||
day: displayDay,
|
day: displayDay,
|
||||||
sortOrder: 0, // Travel always comes first in day
|
sortOrder: sortOrder,
|
||||||
kind: .travel(travelInfo)
|
kind: .travel(travelInfo)
|
||||||
)
|
)
|
||||||
itineraryItems.append(item)
|
itineraryItems.append(item)
|
||||||
|
|||||||
@@ -243,24 +243,27 @@ final class ItineraryReorderingLogicTests: XCTestCase {
|
|||||||
// MARK: - travelRow Tests
|
// MARK: - travelRow Tests
|
||||||
|
|
||||||
func test_travelRow_findsCorrectRow() {
|
func test_travelRow_findsCorrectRow() {
|
||||||
|
// Semantic model: travelRow finds travel in the section AFTER the day header
|
||||||
|
// Travel must be positioned within its correct day section
|
||||||
let items = buildFlatItems([
|
let items = buildFlatItems([
|
||||||
.day(1),
|
.day(1),
|
||||||
.game("Detroit", day: 1),
|
.game("Detroit", day: 1),
|
||||||
.travel(from: "Detroit", to: "Chicago", day: 2),
|
|
||||||
.day(2),
|
.day(2),
|
||||||
.travel(from: "Chicago", to: "Milwaukee", day: 3),
|
.travel(from: "Detroit", to: "Chicago", day: 2), // Row 3: in day 2 section
|
||||||
.day(3)
|
.day(3),
|
||||||
|
.travel(from: "Chicago", to: "Milwaukee", day: 3) // Row 5: in day 3 section
|
||||||
])
|
])
|
||||||
|
|
||||||
XCTAssertEqual(Logic.travelRow(in: items, forDay: 2), 2)
|
XCTAssertEqual(Logic.travelRow(in: items, forDay: 2), 3)
|
||||||
XCTAssertEqual(Logic.travelRow(in: items, forDay: 3), 4)
|
XCTAssertEqual(Logic.travelRow(in: items, forDay: 3), 5)
|
||||||
}
|
}
|
||||||
|
|
||||||
func test_travelRow_noTravelOnDay_returnsNil() {
|
func test_travelRow_noTravelOnDay_returnsNil() {
|
||||||
|
// Travel is in day 2 section, so day 1 has no travel
|
||||||
let items = buildFlatItems([
|
let items = buildFlatItems([
|
||||||
.day(1),
|
.day(1),
|
||||||
.travel(from: "Detroit", to: "Chicago", day: 2),
|
.day(2),
|
||||||
.day(2)
|
.travel(from: "Detroit", to: "Chicago", day: 2) // In day 2 section
|
||||||
])
|
])
|
||||||
|
|
||||||
XCTAssertNil(Logic.travelRow(in: items, forDay: 1))
|
XCTAssertNil(Logic.travelRow(in: items, forDay: 1))
|
||||||
|
|||||||
@@ -108,7 +108,7 @@ enum ItineraryTestHelpers {
|
|||||||
tripId: testTripId,
|
tripId: testTripId,
|
||||||
day: day,
|
day: day,
|
||||||
sortOrder: sortOrder,
|
sortOrder: sortOrder,
|
||||||
kind: .game(gameId: "game-\(city)-\(UUID().uuidString.prefix(4))")
|
kind: .game(gameId: "game-\(city)-\(UUID().uuidString.prefix(4))", city: city)
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -194,11 +194,15 @@ final class ItineraryTravelConstraintTests: XCTestCase {
|
|||||||
// MARK: - Travel Movement Tests
|
// MARK: - Travel Movement Tests
|
||||||
|
|
||||||
func test_travel_moveToValidDay_callsCallback() {
|
func test_travel_moveToValidDay_callsCallback() {
|
||||||
// Given: Travel with valid range 2-4
|
// Given: Travel with valid range 2-3
|
||||||
let travel = H.makeTravelSegment(from: "Chicago", to: "Detroit")
|
let travel = H.makeTravelSegment(from: "Chicago", to: "Detroit")
|
||||||
|
let travelItem = ItineraryRowItem.travel(travel, dayNumber: 2)
|
||||||
|
|
||||||
|
// Travel model item for sortOrder lookup
|
||||||
|
let travelModelItem = H.makeTravelItem(from: "Chicago", to: "Detroit", day: 2, sortOrder: 1.0)
|
||||||
|
|
||||||
let day1 = ItineraryDayData(id: 1, dayNumber: 1, date: testDate, games: [], items: [], travelBefore: nil)
|
let day1 = ItineraryDayData(id: 1, dayNumber: 1, date: testDate, games: [], items: [], travelBefore: nil)
|
||||||
let day2 = ItineraryDayData(id: 2, dayNumber: 2, date: H.dayAfter(testDate), games: [], items: [], travelBefore: travel)
|
let day2 = ItineraryDayData(id: 2, dayNumber: 2, date: H.dayAfter(testDate), games: [], items: [travelItem], travelBefore: nil)
|
||||||
let day3 = ItineraryDayData(id: 3, dayNumber: 3, date: H.dayAfter(H.dayAfter(testDate)), games: [], items: [], travelBefore: nil)
|
let day3 = ItineraryDayData(id: 3, dayNumber: 3, date: H.dayAfter(H.dayAfter(testDate)), games: [], items: [], travelBefore: nil)
|
||||||
|
|
||||||
var capturedTravelId: String = ""
|
var capturedTravelId: String = ""
|
||||||
@@ -211,12 +215,12 @@ final class ItineraryTravelConstraintTests: XCTestCase {
|
|||||||
controller.reloadData(
|
controller.reloadData(
|
||||||
days: [day1, day2, day3],
|
days: [day1, day2, day3],
|
||||||
travelValidRanges: ["travel:chicago->detroit": 2...3],
|
travelValidRanges: ["travel:chicago->detroit": 2...3],
|
||||||
itineraryItems: []
|
itineraryItems: [travelModelItem]
|
||||||
)
|
)
|
||||||
|
|
||||||
// Rows: 0=Day1 header, 1=travel, 2=Day2 header, 3=Day3 header
|
// Rows: 0=Day1 header, 1=Day2 header, 2=travel, 3=Day3 header
|
||||||
// Move travel (row 1) to row 3 (after Day2, before Day3 header means Day 3)
|
// Move travel (row 2) to row 3 (after Day3 header = Day 3)
|
||||||
controller.tableView(controller.tableView, moveRowAt: IndexPath(row: 1, section: 0), to: IndexPath(row: 3, section: 0))
|
controller.tableView(controller.tableView, moveRowAt: IndexPath(row: 2, section: 0), to: IndexPath(row: 3, section: 0))
|
||||||
|
|
||||||
XCTAssertEqual(capturedTravelId, "travel:chicago->detroit")
|
XCTAssertEqual(capturedTravelId, "travel:chicago->detroit")
|
||||||
XCTAssertEqual(capturedDay, 3, "Travel should now be on Day 3")
|
XCTAssertEqual(capturedDay, 3, "Travel should now be on Day 3")
|
||||||
@@ -228,18 +232,21 @@ final class ItineraryTravelConstraintTests: XCTestCase {
|
|||||||
// Given: Travel with valid range Days 2-3
|
// Given: Travel with valid range Days 2-3
|
||||||
let travel = H.makeTravelSegment(from: "Chicago", to: "Detroit")
|
let travel = H.makeTravelSegment(from: "Chicago", to: "Detroit")
|
||||||
let travelId = "travel:chicago->detroit"
|
let travelId = "travel:chicago->detroit"
|
||||||
|
let travelItem = ItineraryRowItem.travel(travel, dayNumber: 2)
|
||||||
|
let travelModelItem = H.makeTravelItem(from: "Chicago", to: "Detroit", day: 2, sortOrder: 1.0)
|
||||||
|
|
||||||
let day1 = ItineraryDayData(id: 1, dayNumber: 1, date: testDate, games: [], items: [], travelBefore: nil)
|
let day1 = ItineraryDayData(id: 1, dayNumber: 1, date: testDate, games: [], items: [], travelBefore: nil)
|
||||||
let day2 = ItineraryDayData(id: 2, dayNumber: 2, date: H.dayAfter(testDate), games: [], items: [], travelBefore: travel)
|
let day2 = ItineraryDayData(id: 2, dayNumber: 2, date: H.dayAfter(testDate), games: [], items: [travelItem], travelBefore: nil)
|
||||||
let day3 = ItineraryDayData(id: 3, dayNumber: 3, date: H.dayAfter(H.dayAfter(testDate)), games: [], items: [], travelBefore: nil)
|
let day3 = ItineraryDayData(id: 3, dayNumber: 3, date: H.dayAfter(H.dayAfter(testDate)), games: [], items: [], travelBefore: nil)
|
||||||
|
|
||||||
let controller = ItineraryTableViewController(style: .plain)
|
let controller = ItineraryTableViewController(style: .plain)
|
||||||
let validRanges = [travelId: 2...3]
|
let validRanges = [travelId: 2...3]
|
||||||
controller.reloadData(days: [day1, day2, day3], travelValidRanges: validRanges)
|
controller.reloadData(days: [day1, day2, day3], travelValidRanges: validRanges, itineraryItems: [travelModelItem])
|
||||||
|
|
||||||
// Travel is at row 1 (after Day1 header at row 0)
|
// Rows: 0=Day1 header, 1=Day2 header, 2=travel, 3=Day3 header
|
||||||
// Try to move it to Day 1 area (row 0 or 1) - should snap back to valid range
|
// Travel is at row 2 (after Day2 header at row 1)
|
||||||
let source = IndexPath(row: 1, section: 0)
|
// Try to move it to Day 1 area (row 0) - should snap back to valid range
|
||||||
|
let source = IndexPath(row: 2, section: 0)
|
||||||
let proposed = IndexPath(row: 0, section: 0)
|
let proposed = IndexPath(row: 0, section: 0)
|
||||||
|
|
||||||
let result = controller.tableView(controller.tableView, targetIndexPathForMoveFromRowAt: source, toProposedIndexPath: proposed)
|
let result = controller.tableView(controller.tableView, targetIndexPathForMoveFromRowAt: source, toProposedIndexPath: proposed)
|
||||||
|
|||||||
@@ -49,33 +49,35 @@ final class ItineraryConstraintsTests: XCTestCase {
|
|||||||
}
|
}
|
||||||
|
|
||||||
func test_travel_mustBeAfterDepartureGames() {
|
func test_travel_mustBeAfterDepartureGames() {
|
||||||
// Given: Chicago game on Day 1 at sortOrder 100
|
// Given: Chicago game on Day 1
|
||||||
|
// Visual boundary is 0: sortOrder < 0 = before games, sortOrder >= 0 = after games
|
||||||
let constraints = makeConstraints(
|
let constraints = makeConstraints(
|
||||||
tripDays: 3,
|
tripDays: 3,
|
||||||
games: [makeGameItem(city: "Chicago", day: 1, sortOrder: 100)]
|
games: [makeGameItem(city: "Chicago", day: 1)]
|
||||||
)
|
)
|
||||||
let travel = makeTravelItem(from: "Chicago", to: "Detroit", day: 1, sortOrder: 50)
|
let travel = makeTravelItem(from: "Chicago", to: "Detroit", day: 1, sortOrder: -1)
|
||||||
|
|
||||||
// When/Then: Travel before game is invalid
|
// When/Then: Travel before game (negative sortOrder) is invalid on departure day
|
||||||
XCTAssertFalse(constraints.isValidPosition(for: travel, day: 1, sortOrder: 50))
|
XCTAssertFalse(constraints.isValidPosition(for: travel, day: 1, sortOrder: -1))
|
||||||
|
|
||||||
// Travel after game is valid
|
// Travel after game (non-negative sortOrder) is valid
|
||||||
XCTAssertTrue(constraints.isValidPosition(for: travel, day: 1, sortOrder: 150))
|
XCTAssertTrue(constraints.isValidPosition(for: travel, day: 1, sortOrder: 1))
|
||||||
}
|
}
|
||||||
|
|
||||||
func test_travel_mustBeBeforeArrivalGames() {
|
func test_travel_mustBeBeforeArrivalGames() {
|
||||||
// Given: Detroit game on Day 3 at sortOrder 100
|
// Given: Detroit game on Day 3
|
||||||
|
// Visual boundary is 0: sortOrder < 0 = before games, sortOrder >= 0 = after games
|
||||||
let constraints = makeConstraints(
|
let constraints = makeConstraints(
|
||||||
tripDays: 3,
|
tripDays: 3,
|
||||||
games: [makeGameItem(city: "Detroit", day: 3, sortOrder: 100)]
|
games: [makeGameItem(city: "Detroit", day: 3)]
|
||||||
)
|
)
|
||||||
let travel = makeTravelItem(from: "Chicago", to: "Detroit", day: 3, sortOrder: 150)
|
let travel = makeTravelItem(from: "Chicago", to: "Detroit", day: 3, sortOrder: 1)
|
||||||
|
|
||||||
// When/Then: Travel after arrival game is invalid
|
// When/Then: Travel after arrival game (non-negative sortOrder) is invalid on arrival day
|
||||||
XCTAssertFalse(constraints.isValidPosition(for: travel, day: 3, sortOrder: 150))
|
XCTAssertFalse(constraints.isValidPosition(for: travel, day: 3, sortOrder: 1))
|
||||||
|
|
||||||
// Travel before game is valid
|
// Travel before game (negative sortOrder) is valid
|
||||||
XCTAssertTrue(constraints.isValidPosition(for: travel, day: 3, sortOrder: 50))
|
XCTAssertTrue(constraints.isValidPosition(for: travel, day: 3, sortOrder: -1))
|
||||||
}
|
}
|
||||||
|
|
||||||
func test_travel_canBeAnywhereOnRestDays() {
|
func test_travel_canBeAnywhereOnRestDays() {
|
||||||
@@ -217,13 +219,12 @@ final class ItineraryConstraintsTests: XCTestCase {
|
|||||||
return ItineraryConstraints(tripDayCount: tripDays, items: games)
|
return ItineraryConstraints(tripDayCount: tripDays, items: games)
|
||||||
}
|
}
|
||||||
|
|
||||||
private func makeGameItem(city: String, day: Int, sortOrder: Double = 100) -> ItineraryItem {
|
private func makeGameItem(city: String, day: Int, sortOrder: Double = 0) -> ItineraryItem {
|
||||||
// For tests, we use gameId to encode the city
|
|
||||||
return ItineraryItem(
|
return ItineraryItem(
|
||||||
tripId: testTripId,
|
tripId: testTripId,
|
||||||
day: day,
|
day: day,
|
||||||
sortOrder: sortOrder,
|
sortOrder: sortOrder,
|
||||||
kind: .game(gameId: "game-\(city)-\(UUID().uuidString.prefix(4))")
|
kind: .game(gameId: "game-\(city)-\(UUID().uuidString.prefix(4))", city: city)
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user