fix: 10 audit fixes — memory safety, performance, accessibility, architecture
- 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 <noreply@anthropic.com>
This commit is contained in:
@@ -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<CanonicalStadium>(
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -62,6 +62,7 @@ struct ProgressMapView: View {
|
||||
.padding(12)
|
||||
.background(.regularMaterial, in: Circle())
|
||||
}
|
||||
.accessibilityLabel("Reset map view")
|
||||
.padding()
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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
|
||||
|
||||
148
SportsTime/Features/Trip/Views/ItinerarySectionBuilder.swift
Normal file
148
SportsTime/Features/Trip/Views/ItinerarySectionBuilder.swift
Normal file
@@ -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<Int>? {
|
||||
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
|
||||
}
|
||||
}
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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)..<itinerarySections.count {
|
||||
if case .day(let dayNumber, _, _) = itinerarySections[i] {
|
||||
for i in (index + 1)..<cachedSections.count {
|
||||
if case .day(let dayNumber, _, _) = cachedSections[i] {
|
||||
return dayNumber
|
||||
}
|
||||
}
|
||||
@@ -816,9 +823,7 @@ struct TripDetailView: View {
|
||||
|
||||
/// Create a stable anchor ID for a travel segment (UUIDs regenerate on reload)
|
||||
private 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)"
|
||||
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] {
|
||||
|
||||
217
SportsTimeTests/Features/Trip/ItinerarySectionBuilderTests.swift
Normal file
217
SportsTimeTests/Features/Trip/ItinerarySectionBuilderTests.swift
Normal file
@@ -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..<count).map {
|
||||
Calendar.current.date(byAdding: .day, value: $0, to: Calendar.current.startOfDay(for: startDate))!
|
||||
}
|
||||
}
|
||||
|
||||
private func makeTrip(
|
||||
cities: [String] = ["New York", "Boston"],
|
||||
startDate: Date = Date(),
|
||||
daysPerStop: Int = 1,
|
||||
gameIds: [[String]] = []
|
||||
) -> (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)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user