From 9b0cb96638d105b57eb3f1fdba40e002135b129c Mon Sep 17 00:00:00 2001 From: Trey t Date: Tue, 17 Feb 2026 12:00:35 -0600 Subject: [PATCH] =?UTF-8?q?fix:=2010=20audit=20fixes=20=E2=80=94=20memory?= =?UTF-8?q?=20safety,=20performance,=20accessibility,=20architecture?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add a11y label to ProgressMapView reset button and progress bar values - Fix CADisplayLink retain cycle in ItineraryTableViewController via deinit - Add [weak self] to PhotoGalleryViewModel Task closure - Add @MainActor to TripWizardViewModel, remove manual MainActor.run hop - Fix O(n²) rank lookup in PollDetailView/DebugPollPreviewView with enumerated() - Cache itinerarySections via ItinerarySectionBuilder static extraction + @State - Convert CanonicalSyncService/BootstrapService from actor to @MainActor final class - Add .accessibilityHidden(true) to RegionMapSelector Map to prevent duplicate VoiceOver Co-Authored-By: Claude Opus 4.6 --- .../Core/Services/BootstrapService.swift | 14 +- .../Core/Services/CanonicalSyncService.swift | 20 +- .../Core/Services/VisitPhotoService.swift | 14 +- .../Features/Polls/Views/PollDetailView.swift | 4 +- .../Progress/Views/AchievementsListView.swift | 2 + .../Progress/Views/ProgressMapView.swift | 1 + .../Settings/DebugPollPreviewView.swift | 4 +- .../Trip/ViewModels/TripWizardViewModel.swift | 6 +- .../Trip/Views/ItinerarySectionBuilder.swift | 148 ++++++++++++ .../Views/ItineraryTableViewController.swift | 7 + .../Trip/Views/RegionMapSelector.swift | 1 + .../Features/Trip/Views/TripDetailView.swift | 86 ++----- .../Trip/ItinerarySectionBuilderTests.swift | 217 ++++++++++++++++++ 13 files changed, 415 insertions(+), 109 deletions(-) create mode 100644 SportsTime/Features/Trip/Views/ItinerarySectionBuilder.swift create mode 100644 SportsTimeTests/Features/Trip/ItinerarySectionBuilderTests.swift diff --git a/SportsTime/Core/Services/BootstrapService.swift b/SportsTime/Core/Services/BootstrapService.swift index 03ac6a7..8da3819 100644 --- a/SportsTime/Core/Services/BootstrapService.swift +++ b/SportsTime/Core/Services/BootstrapService.swift @@ -10,7 +10,8 @@ import Foundation import SwiftData import CryptoKit -actor BootstrapService { +@MainActor +final class BootstrapService { // MARK: - Errors @@ -117,7 +118,6 @@ actor BootstrapService { /// Bootstrap canonical data from bundled JSON if not already done, /// or re-bootstrap if the bundled data schema version has been bumped. /// This is the main entry point called at app launch. - @MainActor func bootstrapIfNeeded(context: ModelContext) async throws { let syncState = SyncState.current(in: context) let hasCoreCanonicalData = hasRequiredCanonicalData(context: context) @@ -172,7 +172,6 @@ actor BootstrapService { } } - @MainActor private func resetSyncProgress(_ syncState: SyncState) { syncState.lastSuccessfulSync = nil syncState.lastSyncAttempt = nil @@ -198,7 +197,6 @@ actor BootstrapService { // MARK: - Bootstrap Steps - @MainActor private func bootstrapStadiums(context: ModelContext) async throws { guard let url = Bundle.main.url(forResource: "stadiums_canonical", withExtension: "json") else { throw BootstrapError.bundledResourceNotFound("stadiums_canonical.json") @@ -235,7 +233,6 @@ actor BootstrapService { } } - @MainActor private func bootstrapStadiumAliases(context: ModelContext) async throws { guard let url = Bundle.main.url(forResource: "stadium_aliases", withExtension: "json") else { return @@ -278,7 +275,6 @@ actor BootstrapService { } } - @MainActor private func bootstrapLeagueStructure(context: ModelContext) async throws { guard let url = Bundle.main.url(forResource: "league_structure", withExtension: "json") else { return @@ -318,7 +314,6 @@ actor BootstrapService { } } - @MainActor private func bootstrapTeams(context: ModelContext) async throws { guard let url = Bundle.main.url(forResource: "teams_canonical", withExtension: "json") else { throw BootstrapError.bundledResourceNotFound("teams_canonical.json") @@ -352,7 +347,6 @@ actor BootstrapService { } } - @MainActor private func bootstrapTeamAliases(context: ModelContext) async throws { guard let url = Bundle.main.url(forResource: "team_aliases", withExtension: "json") else { return @@ -394,7 +388,6 @@ actor BootstrapService { } } - @MainActor private func bootstrapGames(context: ModelContext) async throws { guard let url = Bundle.main.url(forResource: "games_canonical", withExtension: "json") else { throw BootstrapError.bundledResourceNotFound("games_canonical.json") @@ -464,7 +457,6 @@ actor BootstrapService { } } - @MainActor private func bootstrapSports(context: ModelContext) async throws { guard let url = Bundle.main.url(forResource: "sports_canonical", withExtension: "json") else { return @@ -500,7 +492,6 @@ actor BootstrapService { // MARK: - Helpers - @MainActor private func clearCanonicalData(context: ModelContext) throws { try context.delete(model: CanonicalStadium.self) try context.delete(model: StadiumAlias.self) @@ -511,7 +502,6 @@ actor BootstrapService { try context.delete(model: CanonicalSport.self) } - @MainActor private func hasRequiredCanonicalData(context: ModelContext) -> Bool { let stadiumCount = (try? context.fetchCount( FetchDescriptor( diff --git a/SportsTime/Core/Services/CanonicalSyncService.swift b/SportsTime/Core/Services/CanonicalSyncService.swift index b2bb1dd..e3a14a1 100644 --- a/SportsTime/Core/Services/CanonicalSyncService.swift +++ b/SportsTime/Core/Services/CanonicalSyncService.swift @@ -10,7 +10,8 @@ import Foundation import SwiftData import CloudKit -actor CanonicalSyncService { +@MainActor +final class CanonicalSyncService { // MARK: - Errors @@ -73,7 +74,6 @@ actor CanonicalSyncService { /// - Parameters: /// - context: The ModelContext to use for saving data /// - cancellationToken: Optional token to check for cancellation between entity syncs - @MainActor func syncAll(context: ModelContext, cancellationToken: SyncCancellationToken? = nil) async throws -> SyncResult { let startTime = Date() let syncState = SyncState.current(in: context) @@ -391,7 +391,6 @@ actor CanonicalSyncService { } /// Re-enable sync after it was paused due to failures. - @MainActor func resumeSync(context: ModelContext) { let syncState = SyncState.current(in: context) syncState.syncEnabled = true @@ -418,7 +417,6 @@ actor CanonicalSyncService { // MARK: - Individual Sync Methods - @MainActor private func syncStadiums( context: ModelContext, since lastSync: Date?, @@ -449,7 +447,6 @@ actor CanonicalSyncService { return (updated, skippedIncompatible, skippedOlder) } - @MainActor private func syncTeams( context: ModelContext, since lastSync: Date?, @@ -492,7 +489,6 @@ actor CanonicalSyncService { return (updated, skippedIncompatible, skippedOlder) } - @MainActor private func syncGames( context: ModelContext, since lastSync: Date?, @@ -546,7 +542,6 @@ actor CanonicalSyncService { return (updated, skippedIncompatible, skippedOlder) } - @MainActor private func syncLeagueStructure( context: ModelContext, since lastSync: Date?, @@ -571,7 +566,6 @@ actor CanonicalSyncService { return (updated, skippedIncompatible, skippedOlder) } - @MainActor private func syncTeamAliases( context: ModelContext, since lastSync: Date?, @@ -596,7 +590,6 @@ actor CanonicalSyncService { return (updated, skippedIncompatible, skippedOlder) } - @MainActor private func syncStadiumAliases( context: ModelContext, since lastSync: Date?, @@ -621,7 +614,6 @@ actor CanonicalSyncService { return (updated, skippedIncompatible, skippedOlder) } - @MainActor private func syncSports( context: ModelContext, since lastSync: Date?, @@ -654,7 +646,6 @@ actor CanonicalSyncService { case skippedOlder } - @MainActor private func mergeStadium( _ remote: Stadium, canonicalId: String, @@ -719,7 +710,6 @@ actor CanonicalSyncService { } } - @MainActor private func mergeTeam( _ remote: Team, canonicalId: String, @@ -810,7 +800,6 @@ actor CanonicalSyncService { } } - @MainActor private func mergeGame( _ remote: Game, canonicalId: String, @@ -925,7 +914,6 @@ actor CanonicalSyncService { } } - @MainActor private func mergeLeagueStructure( _ remote: LeagueStructureModel, context: ModelContext @@ -965,7 +953,6 @@ actor CanonicalSyncService { } } - @MainActor private func mergeTeamAlias( _ remote: TeamAlias, context: ModelContext @@ -1004,7 +991,6 @@ actor CanonicalSyncService { } } - @MainActor private func mergeStadiumAlias( _ remote: StadiumAlias, context: ModelContext @@ -1041,7 +1027,6 @@ actor CanonicalSyncService { } } - @MainActor private func mergeSport( _ remote: CanonicalSport, context: ModelContext @@ -1083,7 +1068,6 @@ actor CanonicalSyncService { } } - @MainActor private func ensurePlaceholderStadium( canonicalId: String, sport: Sport, diff --git a/SportsTime/Core/Services/VisitPhotoService.swift b/SportsTime/Core/Services/VisitPhotoService.swift index 5b08559..02d7df0 100644 --- a/SportsTime/Core/Services/VisitPhotoService.swift +++ b/SportsTime/Core/Services/VisitPhotoService.swift @@ -372,20 +372,20 @@ final class PhotoGalleryViewModel { } isLoadingFullImage = true - Task { + Task { [weak self] in do { - let image = try await photoService.fetchFullImage(for: metadata) - fullResolutionImage = image + let image = try await self?.photoService.fetchFullImage(for: metadata) + self?.fullResolutionImage = image } catch let error as PhotoServiceError { - self.error = error + self?.error = error // Fall back to thumbnail if let data = metadata.thumbnailData { - fullResolutionImage = UIImage(data: data) + self?.fullResolutionImage = UIImage(data: data) } } catch { - self.error = .downloadFailed(error.localizedDescription) + self?.error = .downloadFailed(error.localizedDescription) } - isLoadingFullImage = false + self?.isLoadingFullImage = false } } diff --git a/SportsTime/Features/Polls/Views/PollDetailView.swift b/SportsTime/Features/Polls/Views/PollDetailView.swift index 202a086..3d47bf9 100644 --- a/SportsTime/Features/Polls/Views/PollDetailView.swift +++ b/SportsTime/Features/Polls/Views/PollDetailView.swift @@ -276,9 +276,9 @@ struct PollDetailView: View { } VStack(spacing: Theme.Spacing.sm) { - ForEach(results.tripScores, id: \.tripIndex) { item in + ForEach(Array(results.tripScores.enumerated()), id: \.element.tripIndex) { index, item in let trip = results.poll.tripSnapshots[item.tripIndex] - let rank = results.tripScores.firstIndex { $0.tripIndex == item.tripIndex }! + 1 + let rank = index + 1 ResultRow( rank: rank, tripName: trip.displayName, diff --git a/SportsTime/Features/Progress/Views/AchievementsListView.swift b/SportsTime/Features/Progress/Views/AchievementsListView.swift index c5d6471..fdea5a1 100644 --- a/SportsTime/Features/Progress/Views/AchievementsListView.swift +++ b/SportsTime/Features/Progress/Views/AchievementsListView.swift @@ -366,6 +366,7 @@ struct AchievementCard: View { VStack(spacing: 4) { ProgressView(value: achievement.progressPercentage) .progressViewStyle(AchievementProgressStyle(category: achievement.definition.category)) + .accessibilityValue("\(Int(achievement.progressPercentage * 100)) percent, \(achievement.progressText)") Text(achievement.progressText) .font(.caption) @@ -575,6 +576,7 @@ struct AchievementDetailSheet: View { ProgressView(value: achievement.progressPercentage) .progressViewStyle(LargeProgressStyle()) .frame(width: 200) + .accessibilityValue("\(Int(achievement.progressPercentage * 100)) percent, \(achievement.currentProgress) of \(achievement.totalRequired)") Text("\(achievement.currentProgress) / \(achievement.totalRequired)") .font(.headline) diff --git a/SportsTime/Features/Progress/Views/ProgressMapView.swift b/SportsTime/Features/Progress/Views/ProgressMapView.swift index 0542b78..68b918e 100644 --- a/SportsTime/Features/Progress/Views/ProgressMapView.swift +++ b/SportsTime/Features/Progress/Views/ProgressMapView.swift @@ -62,6 +62,7 @@ struct ProgressMapView: View { .padding(12) .background(.regularMaterial, in: Circle()) } + .accessibilityLabel("Reset map view") .padding() } } diff --git a/SportsTime/Features/Settings/DebugPollPreviewView.swift b/SportsTime/Features/Settings/DebugPollPreviewView.swift index ac7ae3b..e3a3b4d 100644 --- a/SportsTime/Features/Settings/DebugPollPreviewView.swift +++ b/SportsTime/Features/Settings/DebugPollPreviewView.swift @@ -189,9 +189,9 @@ struct DebugPollPreviewView: View { } VStack(spacing: Theme.Spacing.sm) { - ForEach(results.tripScores, id: \.tripIndex) { item in + ForEach(Array(results.tripScores.enumerated()), id: \.element.tripIndex) { index, item in let trip = results.poll.tripSnapshots[item.tripIndex] - let rank = results.tripScores.firstIndex { $0.tripIndex == item.tripIndex }! + 1 + let rank = index + 1 DebugResultRow( rank: rank, tripName: trip.displayName, diff --git a/SportsTime/Features/Trip/ViewModels/TripWizardViewModel.swift b/SportsTime/Features/Trip/ViewModels/TripWizardViewModel.swift index 3659ed5..d1f81a8 100644 --- a/SportsTime/Features/Trip/ViewModels/TripWizardViewModel.swift +++ b/SportsTime/Features/Trip/ViewModels/TripWizardViewModel.swift @@ -8,7 +8,7 @@ import Foundation import SwiftUI -@Observable +@MainActor @Observable final class TripWizardViewModel { // MARK: - Planning Mode @@ -186,9 +186,7 @@ final class TripWizardViewModel { } } - await MainActor.run { - self.sportAvailability = availability - } + self.sportAvailability = availability } // MARK: - Reset Logic diff --git a/SportsTime/Features/Trip/Views/ItinerarySectionBuilder.swift b/SportsTime/Features/Trip/Views/ItinerarySectionBuilder.swift new file mode 100644 index 0000000..13cca26 --- /dev/null +++ b/SportsTime/Features/Trip/Views/ItinerarySectionBuilder.swift @@ -0,0 +1,148 @@ +// +// ItinerarySectionBuilder.swift +// SportsTime +// +// Pure static builder for itinerary sections. +// Extracted from TripDetailView to enable caching and testability. +// + +import Foundation + +enum ItinerarySectionBuilder { + + /// Build itinerary sections from trip data. + /// + /// This is a pure function: same inputs always produce the same output. + /// Callers should cache the result and recompute only when inputs change. + static func build( + trip: Trip, + tripDays: [Date], + games: [String: RichGame], + travelOverrides: [String: TravelOverride], + itineraryItems: [ItineraryItem], + allowCustomItems: Bool + ) -> [ItinerarySection] { + var sections: [ItinerarySection] = [] + + // Use TravelPlacement for consistent day calculation (shared with tests). + var travelByDay = TravelPlacement.computeTravelByDay(trip: trip, tripDays: tripDays) + + // Apply user overrides on top of computed defaults. + for (segmentIndex, segment) in trip.travelSegments.enumerated() { + let travelId = stableTravelAnchorId(segment, at: segmentIndex) + guard let override = travelOverrides[travelId] else { continue } + + // Validate override is within valid day range + if let validRange = validDayRange(for: travelId, trip: trip, tripDays: tripDays), + validRange.contains(override.day) { + // Remove from computed position (search all days) + for key in travelByDay.keys { + travelByDay[key]?.removeAll { $0.id == segment.id } + } + travelByDay.keys.forEach { key in + if travelByDay[key]?.isEmpty == true { travelByDay[key] = nil } + } + // Place at overridden position + travelByDay[override.day, default: []].append(segment) + } + } + + // Process ALL days + for (index, dayDate) in tripDays.enumerated() { + let dayNum = index + 1 + let gamesOnDay = gamesOn(date: dayDate, trip: trip, games: games) + + // Travel for this day (if any) - appears before day header + for travelSegment in (travelByDay[dayNum] ?? []) { + let segIdx = trip.travelSegments.firstIndex(where: { $0.id == travelSegment.id }) ?? 0 + sections.append(.travel(travelSegment, segmentIndex: segIdx)) + } + + // Day section - shows games or minimal rest day display + sections.append(.day(dayNumber: dayNum, date: dayDate, games: gamesOnDay)) + + // Custom items for this day (sorted by sortOrder) + if allowCustomItems { + // Add button first - always right after day header + sections.append(.addButton(day: dayNum)) + + let dayItems = itineraryItems + .filter { $0.day == dayNum && $0.isCustom } + .sorted { $0.sortOrder < $1.sortOrder } + for item in dayItems { + sections.append(.customItem(item)) + } + } + } + + return sections + } + + // MARK: - Helpers + + static func stableTravelAnchorId(_ segment: TravelSegment, at index: Int) -> String { + let from = TravelInfo.normalizeCityName(segment.fromLocation.name) + let to = TravelInfo.normalizeCityName(segment.toLocation.name) + return "travel:\(index):\(from)->\(to)" + } + + static func gamesOn(date: Date, trip: Trip, games: [String: RichGame]) -> [RichGame] { + let calendar = Calendar.current + let dayStart = calendar.startOfDay(for: date) + let allGameIds = trip.stops.flatMap { $0.games } + + let foundGames = allGameIds.compactMap { games[$0] } + + return foundGames.filter { richGame in + calendar.startOfDay(for: richGame.game.dateTime) == dayStart + }.sorted { $0.game.dateTime < $1.game.dateTime } + } + + /// Parse the segment index from a travel anchor ID. + /// Format: "travel:INDEX:from->to" → INDEX + private static func parseSegmentIndex(from travelId: String) -> Int? { + let stripped = travelId.replacingOccurrences(of: "travel:", with: "") + let parts = stripped.components(separatedBy: ":") + guard parts.count >= 2, let index = Int(parts[0]) else { return nil } + return index + } + + private static func validDayRange(for travelId: String, trip: Trip, tripDays: [Date]) -> ClosedRange? { + guard let segmentIndex = parseSegmentIndex(from: travelId), + segmentIndex < trip.stops.count - 1 else { + return nil + } + + let fromStop = trip.stops[segmentIndex] + let toStop = trip.stops[segmentIndex + 1] + + let fromDayNum = dayNumber(for: fromStop.departureDate, tripDays: tripDays) + let toDayNum = dayNumber(for: toStop.arrivalDate, tripDays: tripDays) + + let minDay = max(fromDayNum + 1, 1) + let maxDay = min(toDayNum, tripDays.count) + + if minDay > maxDay { + return nil + } + + return minDay...maxDay + } + + private static func dayNumber(for date: Date, tripDays: [Date]) -> Int { + let calendar = Calendar.current + let target = calendar.startOfDay(for: date) + + for (index, tripDay) in tripDays.enumerated() { + if calendar.startOfDay(for: tripDay) == target { + return index + 1 + } + } + + // If date is before trip, return 0; if after, return count + 1 + if let first = tripDays.first, target < calendar.startOfDay(for: first) { + return 0 + } + return tripDays.count + 1 + } +} diff --git a/SportsTime/Features/Trip/Views/ItineraryTableViewController.swift b/SportsTime/Features/Trip/Views/ItineraryTableViewController.swift index 7421ec3..21dc729 100644 --- a/SportsTime/Features/Trip/Views/ItineraryTableViewController.swift +++ b/SportsTime/Features/Trip/Views/ItineraryTableViewController.swift @@ -511,6 +511,13 @@ final class ItineraryTableViewController: UITableViewController { ItineraryReorderingLogic.travelRow(in: flatItems, forDay: day) } + deinit { + #if DEBUG + displayLink?.invalidate() + displayLink = nil + #endif + } + // MARK: - Marketing Video Auto-Scroll #if DEBUG diff --git a/SportsTime/Features/Trip/Views/RegionMapSelector.swift b/SportsTime/Features/Trip/Views/RegionMapSelector.swift index 280efc8..b9ce7f1 100644 --- a/SportsTime/Features/Trip/Views/RegionMapSelector.swift +++ b/SportsTime/Features/Trip/Views/RegionMapSelector.swift @@ -54,6 +54,7 @@ struct RegionMapSelector: View { .stroke(strokeColor(for: .east), lineWidth: strokeWidth(for: .east)) } .mapStyle(.standard(elevation: .flat, pointsOfInterest: .excludingAll)) + .accessibilityHidden(true) .onTapGesture { location in if let coordinate = proxy.convert(location, from: .local) { let tappedRegion = Self.regionForCoordinate(coordinate) diff --git a/SportsTime/Features/Trip/Views/TripDetailView.swift b/SportsTime/Features/Trip/Views/TripDetailView.swift index 137c5fc..fe23bee 100644 --- a/SportsTime/Features/Trip/Views/TripDetailView.swift +++ b/SportsTime/Features/Trip/Views/TripDetailView.swift @@ -45,6 +45,7 @@ struct TripDetailView: View { @State private var draggedTravelId: String? // Track which travel segment is being dragged @State private var dropTargetId: String? // Track which drop zone is being hovered @State private var travelOverrides: [String: TravelOverride] = [:] // Key: travel ID, Value: day + sortOrder + @State private var cachedSections: [ItinerarySection] = [] // Apple Maps state @State private var showMultiRouteAlert = false @@ -134,14 +135,20 @@ struct TripDetailView: View { await loadItineraryItems() await setupSubscription() } + recomputeSections() } .onDisappear { subscriptionCancellable?.cancel() } .onChange(of: itineraryItems) { _, newItems in handleItineraryItemsChange(newItems) + recomputeSections() } .onChange(of: travelOverrides.count) { _, _ in draggedTravelId = nil dropTargetId = nil + recomputeSections() + } + .onChange(of: loadedGames.count) { _, _ in + recomputeSections() } .overlay { if isExporting { exportProgressOverlay } @@ -536,7 +543,7 @@ struct TripDetailView: View { .frame(maxHeight: .infinity) VStack(spacing: Theme.Spacing.md) { - ForEach(Array(itinerarySections.enumerated()), id: \.offset) { index, section in + ForEach(Array(cachedSections.enumerated()), id: \.offset) { index, section in itineraryRow(for: section, at: index) } } @@ -801,11 +808,11 @@ struct TripDetailView: View { private func findDayForTravelSegment(_ segment: TravelSegment) -> Int { // Find which day this travel segment belongs to by looking at sections // Travel appears BEFORE the arrival day, so look FORWARD to find arrival day - for (index, section) in itinerarySections.enumerated() { + for (index, section) in cachedSections.enumerated() { if case .travel(let s, _) = section, s.id == segment.id { // Look forward to find the arrival day - for i in (index + 1).. String { - let from = TravelInfo.normalizeCityName(segment.fromLocation.name) - let to = TravelInfo.normalizeCityName(segment.toLocation.name) - return "travel:\(index):\(from)->\(to)" + ItinerarySectionBuilder.stableTravelAnchorId(segment, at: index) } /// Move item to a new day and sortOrder position @@ -841,63 +846,16 @@ struct TripDetailView: View { print("✅ [Move] Synced \(title) with day: \(day), sortOrder: \(sortOrder)") } - /// Build itinerary sections: shows ALL days with travel, custom items, and add buttons - private var itinerarySections: [ItinerarySection] { - var sections: [ItinerarySection] = [] - let days = tripDays - - // 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, at: segmentIndex) - guard let override = travelOverrides[travelId] else { continue } - - // Validate override is within valid day range - if let validRange = validDayRange(for: travelId), - validRange.contains(override.day) { - // Remove from computed position (search all days) - for key in travelByDay.keys { - travelByDay[key]?.removeAll { $0.id == segment.id } - } - travelByDay.keys.forEach { key in - if travelByDay[key]?.isEmpty == true { travelByDay[key] = nil } - } - // Place at overridden position - travelByDay[override.day, default: []].append(segment) - } - } - - // Process ALL days - for (index, dayDate) in days.enumerated() { - let dayNum = index + 1 - let gamesOnDay = gamesOn(date: dayDate) - - // Travel for this day (if any) - appears before day header - for travelSegment in (travelByDay[dayNum] ?? []) { - let segIdx = trip.travelSegments.firstIndex(where: { $0.id == travelSegment.id }) ?? 0 - sections.append(.travel(travelSegment, segmentIndex: segIdx)) - } - - // Day section - shows games or minimal rest day display - sections.append(.day(dayNumber: dayNum, date: dayDate, games: gamesOnDay)) - - // Custom items for this day (sorted by sortOrder) - if allowCustomItems { - // Add button first - always right after day header - sections.append(.addButton(day: dayNum)) - - let dayItems = itineraryItems - .filter { $0.day == dayNum && $0.isCustom } - .sorted { $0.sortOrder < $1.sortOrder } - for item in dayItems { - sections.append(.customItem(item)) - } - } - } - - return sections + /// Recompute cached itinerary sections from current state. + private func recomputeSections() { + cachedSections = ItinerarySectionBuilder.build( + trip: trip, + tripDays: tripDays, + games: games, + travelOverrides: travelOverrides, + itineraryItems: itineraryItems, + allowCustomItems: allowCustomItems + ) } private var tripDays: [Date] { diff --git a/SportsTimeTests/Features/Trip/ItinerarySectionBuilderTests.swift b/SportsTimeTests/Features/Trip/ItinerarySectionBuilderTests.swift new file mode 100644 index 0000000..39bca88 --- /dev/null +++ b/SportsTimeTests/Features/Trip/ItinerarySectionBuilderTests.swift @@ -0,0 +1,217 @@ +// +// ItinerarySectionBuilderTests.swift +// SportsTimeTests +// +// Tests for ItinerarySectionBuilder pure static function. +// + +import Testing +import CoreLocation +@testable import SportsTime + +@Suite("ItinerarySectionBuilder") +struct ItinerarySectionBuilderTests { + + // MARK: - Helpers + + private func makeTripDays(count: Int, startDate: Date = Date()) -> [Date] { + (0.. (Trip, [Date]) { + let stops = TestFixtures.tripStops(cities: cities, startDate: startDate, daysPerStop: daysPerStop) + var stopsWithGames = stops + for (i, ids) in gameIds.enumerated() where i < stopsWithGames.count { + stopsWithGames[i] = TripStop( + stopNumber: stopsWithGames[i].stopNumber, + city: stopsWithGames[i].city, + state: stopsWithGames[i].state, + coordinate: stopsWithGames[i].coordinate, + arrivalDate: stopsWithGames[i].arrivalDate, + departureDate: stopsWithGames[i].departureDate, + games: ids, + isRestDay: stopsWithGames[i].isRestDay + ) + } + let trip = TestFixtures.trip(stops: stopsWithGames) + let totalDays = cities.count * daysPerStop + let days = makeTripDays(count: totalDays, startDate: startDate) + return (trip, days) + } + + // MARK: - Tests + + @Test("builds one section per day") + func buildsSectionsForEachDay() { + let startDate = Calendar.current.startOfDay(for: Date()) + let (trip, days) = makeTrip(cities: ["New York", "Boston", "Philadelphia"], startDate: startDate) + + let sections = ItinerarySectionBuilder.build( + trip: trip, + tripDays: days, + games: [:], + travelOverrides: [:], + itineraryItems: [], + allowCustomItems: false + ) + + // Should have travel + day sections + let daySections = sections.filter { + if case .day = $0 { return true } + return false + } + #expect(daySections.count == days.count) + } + + @Test("games filtered correctly by date") + func gamesOnFiltersCorrectly() { + let startDate = Calendar.current.startOfDay(for: Date()) + let gameDate = startDate + let game = TestFixtures.game(sport: .mlb, city: "New York", dateTime: gameDate) + let richGame = TestFixtures.richGame(game: game, homeCity: "New York") + + let (trip, days) = makeTrip( + cities: ["New York", "Boston"], + startDate: startDate, + gameIds: [[game.id]] + ) + + let sections = ItinerarySectionBuilder.build( + trip: trip, + tripDays: days, + games: [game.id: richGame], + travelOverrides: [:], + itineraryItems: [], + allowCustomItems: false + ) + + // Find the first day section and verify it has a game + let firstDaySection = sections.first { + if case .day(1, _, _) = $0 { return true } + return false + } + + if case .day(_, _, let gamesOnDay) = firstDaySection { + #expect(gamesOnDay.count == 1) + #expect(gamesOnDay.first?.game.id == game.id) + } else { + Issue.record("Expected day section with games") + } + } + + @Test("travel segments appear in sections") + func travelSegmentsAppear() { + let startDate = Calendar.current.startOfDay(for: Date()) + let travel = TestFixtures.travelSegment(from: "New York", to: "Boston") + let (baseTrip, days) = makeTrip( + cities: ["New York", "Boston"], + startDate: startDate, + daysPerStop: 1 + ) + + // Create trip with travel segment + var trip = baseTrip + trip.travelSegments = [travel] + + // Build without overrides - travel appears at default position + let sectionsDefault = ItinerarySectionBuilder.build( + trip: trip, + tripDays: days, + games: [:], + travelOverrides: [:], + itineraryItems: [], + allowCustomItems: false + ) + + let travelSections = sectionsDefault.filter { + if case .travel = $0 { return true } + return false + } + + // TravelPlacement should place the travel segment + #expect(travelSections.count == 1) + + // Verify travel appears before day 2 (arrival day) + if let travelIdx = sectionsDefault.firstIndex(where: { + if case .travel = $0 { return true } + return false + }) { + let nextIdx = sectionsDefault.index(after: travelIdx) + if nextIdx < sectionsDefault.count, + case .day(let dayNum, _, _) = sectionsDefault[nextIdx] { + #expect(dayNum == 2) + } + } + } + + @Test("custom items included when allowCustomItems is true") + func customItemsIncluded() { + let startDate = Calendar.current.startOfDay(for: Date()) + let (trip, days) = makeTrip(cities: ["New York"], startDate: startDate) + + let customItem = ItineraryItem( + tripId: trip.id, + day: 1, + sortOrder: 1.0, + kind: .custom(CustomInfo(title: "Lunch at Joe's", icon: "fork.knife")) + ) + + let sectionsWithItems = ItinerarySectionBuilder.build( + trip: trip, + tripDays: days, + games: [:], + travelOverrides: [:], + itineraryItems: [customItem], + allowCustomItems: true + ) + + let sectionsWithoutItems = ItinerarySectionBuilder.build( + trip: trip, + tripDays: days, + games: [:], + travelOverrides: [:], + itineraryItems: [customItem], + allowCustomItems: false + ) + + let customSectionsWithItems = sectionsWithItems.filter { $0.isCustomItem } + let addButtonSections = sectionsWithItems.filter { + if case .addButton = $0 { return true } + return false + } + let customSectionsWithoutItems = sectionsWithoutItems.filter { $0.isCustomItem } + + #expect(customSectionsWithItems.count == 1) + #expect(addButtonSections.count >= 1) + #expect(customSectionsWithoutItems.count == 0) + } + + @Test("stableTravelAnchorId format is consistent") + func stableTravelAnchorIdFormat() { + let segment = TestFixtures.travelSegment(from: "New York", to: "Boston") + let id = ItinerarySectionBuilder.stableTravelAnchorId(segment, at: 0) + #expect(id.hasPrefix("travel:0:")) + #expect(id.contains("->")) + } + + @Test("empty trip produces no sections") + func emptyTripProducesNoSections() { + let trip = TestFixtures.trip(stops: []) + let sections = ItinerarySectionBuilder.build( + trip: trip, + tripDays: [], + games: [:], + travelOverrides: [:], + itineraryItems: [], + allowCustomItems: false + ) + #expect(sections.isEmpty) + } +}