fix: codebase audit fixes — safety, accessibility, and production hygiene

Address 16 issues from external audit: - Move StoreKit transaction listener ownership to StoreManager singleton with proper deinit - Remove noisy VoiceOver announcements, add missing accessibility on StatPill and BootstrapLoadingView - Replace String @retroactive Identifiable with IdentifiableShareCode wrapper - Add crash guard in AchievementEngine getContributingVisitIds + cache stadium lookups - Pre-compute GamesHistoryViewModel filtered properties to avoid redundant SwiftUI recomputation - Remove force-unwraps in ProgressMapView with safe guard-let fallback - Add diff-based update gating in ItineraryTableViewWrapper to prevent unnecessary reloads - Replace deprecated UIScreen.main with UIWindowScene lookup - Add deinit task cancellation in ScheduleViewModel and SuggestedTripsGenerator - Wrap ~234 unguarded print() calls across 27 files in #if DEBUG Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-22 00:07:53 -06:00
parent 826eadbc0f
commit 91c5eac22d
32 changed files with 434 additions and 67 deletions
--- a/SportsTime/Features/Trip/Views/AddItem/PlaceSearchSheet.swift
+++ b/SportsTime/Features/Trip/Views/AddItem/PlaceSearchSheet.swift
@@ -468,6 +468,8 @@ private struct PlaceRow: View {

 #Preview {
    PlaceSearchSheet { place in
+        #if DEBUG
        print("Selected: \(place.name ?? "unknown")")
+        #endif
    }
 }
--- a/SportsTime/Features/Trip/Views/AddItem/QuickAddItemSheet.swift
+++ b/SportsTime/Features/Trip/Views/AddItem/QuickAddItemSheet.swift
@@ -734,7 +734,9 @@ private struct PressableStyle: ButtonStyle {
        day: 1,
        existingItem: nil
    ) { item in
+        #if DEBUG
        print("Saved: \(item)")
+        #endif
    }
    .preferredColorScheme(.light)
 }
@@ -745,7 +747,9 @@ private struct PressableStyle: ButtonStyle {
        day: 3,
        existingItem: nil
    ) { item in
+        #if DEBUG
        print("Saved: \(item)")
+        #endif
    }
    .preferredColorScheme(.dark)
 }
@@ -770,6 +774,8 @@ private struct PressableStyle: ButtonStyle {
        day: existing.day,
        existingItem: existing
    ) { item in
+        #if DEBUG
        print("Updated: \(item)")
+        #endif
    }
 }
--- a/SportsTime/Features/Trip/Views/ItineraryReorderingLogic.swift
+++ b/SportsTime/Features/Trip/Views/ItineraryReorderingLogic.swift
@@ -83,7 +83,9 @@ enum ItineraryReorderingLogic {
                    // nil is valid during initial display before travel is persisted.
                    let lookedUp = findTravelSortOrder(segment)
                    sortOrder = lookedUp ?? defaultTravelSortOrder
+                    #if DEBUG
                    print("📋 [flattenDays] Travel \(segment.fromLocation.name)->\(segment.toLocation.name) on day \(day.dayNumber): lookedUp=\(String(describing: lookedUp)), using sortOrder=\(sortOrder)")
+                    #endif
                    
                case .games, .dayHeader:
                    // These item types are not movable and handled separately.
@@ -319,6 +321,7 @@ enum ItineraryReorderingLogic {
        }

        // DEBUG: Log the row positions
+        #if DEBUG
        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) {
@@ -326,6 +329,7 @@ enum ItineraryReorderingLogic {
            let gMarker = (gamesRow == i) ? " [GAMES]" : ""
            print("🔢   \(marker) [\(i)] \(items[i])\(gMarker)")
        }
+        #endif

        // Strict region classification:
        // - row < gamesRow => before-games (negative sortOrder)
@@ -333,10 +337,14 @@ enum ItineraryReorderingLogic {
        let isBeforeGames: Bool
        if let gr = gamesRow {
            isBeforeGames = row < gr
+            #if DEBUG
            print("🔢 [calculateSortOrder] row(\(row)) < gamesRow(\(gr)) = \(isBeforeGames) → isBeforeGames=\(isBeforeGames)")
+            #endif
        } else {
            isBeforeGames = false  // No games means everything is "after games"
+            #if DEBUG
            print("🔢 [calculateSortOrder] No games on day \(day) → isBeforeGames=false")
+            #endif
        }
        
        /// Get sortOrder from a movable item (custom item or travel)
@@ -442,7 +450,9 @@ enum ItineraryReorderingLogic {
            }
        }

+        #if DEBUG
        print("🔢 [calculateSortOrder] RESULT: \(result) (isBeforeGames=\(isBeforeGames))")
+        #endif
        return result
    }
    
--- a/SportsTime/Features/Trip/Views/ItineraryTableViewController.swift
+++ b/SportsTime/Features/Trip/Views/ItineraryTableViewController.swift
@@ -869,6 +869,7 @@ final class ItineraryTableViewController: UITableViewController {
            let sortOrder = calculateSortOrder(at: destinationIndexPath.row)
            
            // DEBUG: Log the final state after insertion
+            #if DEBUG
            print("🎯 [Drop] source=\(sourceIndexPath.row) → dest=\(destinationIndexPath.row)")
            print("🎯 [Drop] flatItems around dest:")
            for i in max(0, destinationIndexPath.row - 2)...min(flatItems.count - 1, destinationIndexPath.row + 2) {
@@ -876,6 +877,7 @@ final class ItineraryTableViewController: UITableViewController {
                print("🎯 [Drop] \(marker) [\(i)] \(flatItems[i])")
            }
            print("🎯 [Drop] Calculated day=\(destinationDay), sortOrder=\(sortOrder)")
+            #endif
            
            onCustomItemMoved?(customItem.id, destinationDay, sortOrder)
            
--- a/SportsTime/Features/Trip/Views/ItineraryTableViewWrapper.swift
+++ b/SportsTime/Features/Trip/Views/ItineraryTableViewWrapper.swift
@@ -53,6 +53,10 @@ struct ItineraryTableViewWrapper<HeaderContent: View>: UIViewControllerRepresent
    
    class Coordinator {
        var headerHostingController: UIHostingController<HeaderContent>?
+        var lastStopCount: Int = 0
+        var lastGameIDsHash: Int = 0
+        var lastItemCount: Int = 0
+        var lastOverrideCount: Int = 0
    }
    
    func makeUIViewController(context: Context) -> ItineraryTableViewController {
@@ -73,7 +77,9 @@ struct ItineraryTableViewWrapper<HeaderContent: View>: UIViewControllerRepresent
        
        // Pre-size the header view
        hostingController.view.translatesAutoresizingMaskIntoConstraints = false
-        let targetWidth = UIScreen.main.bounds.width
+        let targetWidth = UIApplication.shared.connectedScenes
+            .compactMap { ($0 as? UIWindowScene)?.screen.bounds.width }
+            .first ?? 390
        let targetSize = CGSize(width: targetWidth, height: UIView.layoutFittingCompressedSize.height)
        let size = hostingController.view.systemLayoutSizeFitting(targetSize, withHorizontalFittingPriority: .required, verticalFittingPriority: .fittingSizeLevel)
        hostingController.view.frame = CGRect(origin: .zero, size: CGSize(width: targetWidth, height: max(size.height, 450)))
@@ -109,6 +115,25 @@ struct ItineraryTableViewWrapper<HeaderContent: View>: UIViewControllerRepresent
        // This avoids recreating the view hierarchy and prevents infinite loops
        context.coordinator.headerHostingController?.rootView = headerContent

+        // Diff inputs before rebuilding to avoid unnecessary reloads
+        let currentStopCount = trip.stops.count
+        let currentGameIDsHash = games.map(\.game.id).hashValue
+        let currentItemCount = itineraryItems.count
+        let currentOverrideCount = travelOverrides.count
+
+        let coord = context.coordinator
+        guard currentStopCount != coord.lastStopCount ||
+              currentGameIDsHash != coord.lastGameIDsHash ||
+              currentItemCount != coord.lastItemCount ||
+              currentOverrideCount != coord.lastOverrideCount else {
+            return
+        }
+
+        coord.lastStopCount = currentStopCount
+        coord.lastGameIDsHash = currentGameIDsHash
+        coord.lastItemCount = currentItemCount
+        coord.lastOverrideCount = currentOverrideCount
+
        let (days, validRanges, allItemsForConstraints, travelSegmentIndices) = buildItineraryData()
        controller.reloadData(
            days: days,
--- a/SportsTime/Features/Trip/Views/TripDetailView.swift
+++ b/SportsTime/Features/Trip/Views/TripDetailView.swift
@@ -197,12 +197,14 @@ struct TripDetailView: View {
    private func handleItineraryItemsChange(_ newItems: [ItineraryItem]) {
        draggedItem = nil
        dropTargetId = nil
+        #if DEBUG
        print("🗺️ [MapUpdate] itineraryItems changed, count: \(newItems.count)")
        for item in newItems {
            if item.isCustom, let info = item.customInfo, info.isMappable {
                print("🗺️ [MapUpdate] Mappable: \(info.title) on day \(item.day), sortOrder: \(item.sortOrder)")
            }
        }
+        #endif
        mapUpdateTask?.cancel()
        mapUpdateTask = Task {
            updateMapRegion()
@@ -851,7 +853,9 @@ struct TripDetailView: View {
        updated.modifiedAt = Date()

        let title = item.customInfo?.title ?? "item"
+        #if DEBUG
        print("📍 [Move] Moving \(title) to day \(day), sortOrder: \(sortOrder)")
+        #endif

        // Update local state
        if let idx = itineraryItems.firstIndex(where: { $0.id == item.id }) {
@@ -863,7 +867,9 @@ struct TripDetailView: View {

        // Sync to CloudKit (debounced)
        await ItineraryItemService.shared.updateItem(updated)
+        #if DEBUG
        print("✅ [Move] Synced \(title) with day: \(day), sortOrder: \(sortOrder)")
+        #endif
    }

    /// Recompute cached itinerary sections from current state.
@@ -1389,7 +1395,9 @@ struct TripDetailView: View {
    // MARK: - Itinerary Items (Local-first persistence with CloudKit sync)

    private func loadItineraryItems() async {
+        #if DEBUG
        print("🔍 [ItineraryItems] Loading items for trip: \(trip.id)")
+        #endif

        // 1. Load from local SwiftData first (instant, works offline)
        let tripId = trip.id
@@ -1398,7 +1406,9 @@ struct TripDetailView: View {
        )
        let localItems = (try? modelContext.fetch(descriptor))?.compactMap(\.toItem) ?? []
        if !localItems.isEmpty {
+            #if DEBUG
            print("✅ [ItineraryItems] Loaded \(localItems.count) items from local cache")
+            #endif
            itineraryItems = localItems
            extractTravelOverrides(from: localItems)
        }
@@ -1406,7 +1416,9 @@ struct TripDetailView: View {
        // 2. Try CloudKit for latest data (background sync)
        do {
            let cloudItems = try await ItineraryItemService.shared.fetchItems(forTripId: trip.id)
+            #if DEBUG
            print("✅ [ItineraryItems] Loaded \(cloudItems.count) items from CloudKit")
+            #endif

            // Merge: use CloudKit as source of truth when available
            if !cloudItems.isEmpty || localItems.isEmpty {
@@ -1415,7 +1427,9 @@ struct TripDetailView: View {
                syncLocalCache(with: cloudItems)
            }
        } catch {
+            #if DEBUG
            print("⚠️ [ItineraryItems] CloudKit fetch failed (using local cache): \(error)")
+            #endif
        }
    }

@@ -1429,7 +1443,9 @@ struct TripDetailView: View {
               segIdx < trip.travelSegments.count {
                let segment = trip.travelSegments[segIdx]
                if !travelInfo.matches(segment: segment) {
+                    #if DEBUG
                    print("⚠️ [TravelOverrides] Mismatched travel cities for segment \(segIdx); using canonical segment cities")
+                    #endif
                }
                let travelId = stableTravelAnchorId(segment, at: segIdx)
                overrides[travelId] = TravelOverride(day: item.day, sortOrder: item.sortOrder)
@@ -1441,7 +1457,9 @@ struct TripDetailView: View {
            }

            guard matches.count == 1, let match = matches.first else {
+                #if DEBUG
                print("⚠️ [TravelOverrides] Ignoring ambiguous legacy travel override \(travelInfo.fromCity)->\(travelInfo.toCity)")
+                #endif
                continue
            }

@@ -1452,7 +1470,9 @@ struct TripDetailView: View {
        }

        travelOverrides = overrides
+        #if DEBUG
        print("✅ [TravelOverrides] Extracted \(overrides.count) travel overrides (day + sortOrder)")
+        #endif
    }

    private func syncLocalCache(with items: [ItineraryItem]) {
@@ -1473,7 +1493,9 @@ struct TripDetailView: View {
    private func saveItineraryItem(_ item: ItineraryItem) async {
        let isUpdate = itineraryItems.contains(where: { $0.id == item.id })
        let title = item.customInfo?.title ?? "item"
+        #if DEBUG
        print("💾 [ItineraryItems] Saving item: '\(title)' (isUpdate: \(isUpdate))")
+        #endif

        // Update in-memory state immediately
        if isUpdate {
@@ -1491,14 +1513,20 @@ struct TripDetailView: View {
        do {
            if isUpdate {
                await ItineraryItemService.shared.updateItem(item)
+                #if DEBUG
                print("✅ [ItineraryItems] Updated in CloudKit: \(title)")
+                #endif
            } else {
                _ = try await ItineraryItemService.shared.createItem(item)
+                #if DEBUG
                print("✅ [ItineraryItems] Created in CloudKit: \(title)")
+                #endif
                markLocalItemSynced(item.id)
            }
        } catch {
+            #if DEBUG
            print("⚠️ [ItineraryItems] CloudKit save failed (saved locally): \(error)")
+            #endif
        }
    }

@@ -1535,7 +1563,9 @@ struct TripDetailView: View {

    private func deleteItineraryItem(_ item: ItineraryItem) async {
        let title = item.customInfo?.title ?? "item"
+        #if DEBUG
        print("🗑️ [ItineraryItems] Deleting item: '\(title)'")
+        #endif

        // Remove from in-memory state
        itineraryItems.removeAll { $0.id == item.id }
@@ -1553,9 +1583,13 @@ struct TripDetailView: View {
        // Delete from CloudKit
        do {
            try await ItineraryItemService.shared.deleteItem(item.id)
+            #if DEBUG
            print("✅ [ItineraryItems] Deleted from CloudKit")
+            #endif
        } catch {
+            #if DEBUG
            print("⚠️ [ItineraryItems] CloudKit delete failed (removed locally): \(error)")
+            #endif
        }
    }

@@ -1632,12 +1666,16 @@ struct TripDetailView: View {
    }

    private func saveTravelDayOverride(travelAnchorId: String, displayDay: Int, sortOrder: Double) async {
+        #if DEBUG
        print("💾 [TravelOverrides] Saving override: \(travelAnchorId) -> day \(displayDay), sortOrder \(sortOrder)")
+        #endif

        guard let segmentIndex = Self.parseSegmentIndex(from: travelAnchorId),
              segmentIndex >= 0,
              segmentIndex < trip.travelSegments.count else {
+            #if DEBUG
            print("❌ [TravelOverrides] Invalid travel segment index in ID: \(travelAnchorId)")
+            #endif
            return
        }

@@ -1676,14 +1714,18 @@ struct TripDetailView: View {
                _ = try await ItineraryItemService.shared.createItem(item)
                markLocalItemSynced(item.id)
            } catch {
+                #if DEBUG
                print("❌ [TravelOverrides] CloudKit save failed: \(error)")
+                #endif
                return
            }
        }
+        #if DEBUG
        if canonicalTravelId != travelAnchorId {
            print("ℹ️ [TravelOverrides] Canonicalized travel ID to \(canonicalTravelId)")
        }
        print("✅ [TravelOverrides] Saved to CloudKit")
+        #endif
    }
 }

--- a/SportsTime/Features/Trip/Views/Wizard/TripWizardView.swift
+++ b/SportsTime/Features/Trip/Views/Wizard/TripWizardView.swift
@@ -203,7 +203,9 @@ struct TripWizardView: View {
                // Team-First mode: fetch ALL games for the season
                // ScenarioEPlanner will generate sliding windows across the full season
                games = try await AppDataProvider.shared.allGames(for: preferences.sports)
+                #if DEBUG
                print("🔍 TripWizard: Team-First mode - fetched \(games.count) games for \(preferences.sports)")
+                #endif
            } else if viewModel.planningMode == .gameFirst && !viewModel.selectedGameIds.isEmpty {
                // Fetch all games for the selected sports within the UI date range
                // GamePickerStep already set viewModel.startDate/endDate to a 7-day span