From 60189a5406ce8954f711a2193a3ba88eac86f8d0 Mon Sep 17 00:00:00 2001 From: treyt Date: Mon, 9 Mar 2026 22:23:56 -0500 Subject: [PATCH] fix: issue #17 - upcoming tasks Automated fix by Tony CI v3. Refs #17 Co-Authored-By: Claude --- .../Local/CoreData/CoreDataStack.swift | 10 ++++--- .../Local/FilterPreferencesStorage.swift | 8 ++--- .../ViewModels/CollectionViewModel.swift | 30 ++++++++++++------- .../CareSchedule/CareScheduleViewModel.swift | 4 +-- .../PlantDetail/PlantDetailViewModel.swift | 12 +++++--- .../Scenes/TodayView/TodayViewModel.swift | 4 +-- .../CoreDataCareScheduleStorageTests.swift | 8 +++-- .../MockPlantClassificationService.swift | 14 ++++----- 8 files changed, 54 insertions(+), 36 deletions(-) diff --git a/PlantGuide/Data/DataSources/Local/CoreData/CoreDataStack.swift b/PlantGuide/Data/DataSources/Local/CoreData/CoreDataStack.swift index 8bdb29c..bd4a0d6 100644 --- a/PlantGuide/Data/DataSources/Local/CoreData/CoreDataStack.swift +++ b/PlantGuide/Data/DataSources/Local/CoreData/CoreDataStack.swift @@ -469,13 +469,15 @@ extension CoreDataStack { func resetForTesting() throws { let context = viewContext() - // Delete all entities + // Delete all entities individually (NSBatchDeleteRequest is unsupported on in-memory stores) let entityNames = persistentContainer.managedObjectModel.entities.compactMap { $0.name } for entityName in entityNames { - let fetchRequest = NSFetchRequest(entityName: entityName) - let deleteRequest = NSBatchDeleteRequest(fetchRequest: fetchRequest) - try context.execute(deleteRequest) + let fetchRequest = NSFetchRequest(entityName: entityName) + let objects = try context.fetch(fetchRequest) + for object in objects { + context.delete(object) + } } try save(context: context) diff --git a/PlantGuide/Data/DataSources/Local/FilterPreferencesStorage.swift b/PlantGuide/Data/DataSources/Local/FilterPreferencesStorage.swift index de448a2..7fe53ec 100644 --- a/PlantGuide/Data/DataSources/Local/FilterPreferencesStorage.swift +++ b/PlantGuide/Data/DataSources/Local/FilterPreferencesStorage.swift @@ -84,15 +84,15 @@ final class FilterPreferencesStorage: FilterPreferencesStorageProtocol, @uncheck // Save sortAscending userDefaults.set(filter.sortAscending, forKey: Keys.filterSortAscending) - // Save families (as array of strings) - if let families = filter.families { + // Save families (as array of strings) — empty set treated as nil (no filter) + if let families = filter.families, !families.isEmpty { userDefaults.set(Array(families), forKey: Keys.filterFamilies) } else { userDefaults.removeObject(forKey: Keys.filterFamilies) } - // Save light requirements (as array of raw values) - if let lightRequirements = filter.lightRequirements { + // Save light requirements (as array of raw values) — empty set treated as nil + if let lightRequirements = filter.lightRequirements, !lightRequirements.isEmpty { let rawValues = lightRequirements.map { $0.rawValue } userDefaults.set(rawValues, forKey: Keys.filterLightRequirements) } else { diff --git a/PlantGuide/Presentation/Collection/ViewModels/CollectionViewModel.swift b/PlantGuide/Presentation/Collection/ViewModels/CollectionViewModel.swift index 532f7b5..135cea8 100644 --- a/PlantGuide/Presentation/Collection/ViewModels/CollectionViewModel.swift +++ b/PlantGuide/Presentation/Collection/ViewModels/CollectionViewModel.swift @@ -106,6 +106,9 @@ final class CollectionViewModel { /// Task for debounced search private var searchTask: Task? + /// Active load task to prevent duplicate concurrent loads + private var currentLoadTask: Task? + /// Search debounce interval in nanoseconds (300ms) private let searchDebounceNanoseconds: UInt64 = 300_000_000 @@ -156,21 +159,28 @@ final class CollectionViewModel { /// This method fetches all plants based on the current filter and updates /// the view model's state accordingly. Shows loading state during fetch. func loadPlants() async { - guard !isLoading else { return } + if let currentLoadTask { + await currentLoadTask.value + return + } isLoading = true error = nil - do { - let fetchedPlants = try await fetchCollectionUseCase.execute(filter: currentFilter) - allPlants = fetchedPlants - applySearchFilter() - } catch { - self.error = error.localizedDescription - plants = [] - allPlants = [] + let task = Task { @MainActor in + do { + let fetchedPlants = try await self.fetchCollectionUseCase.execute(filter: self.currentFilter) + self.allPlants = fetchedPlants + self.applySearchFilter() + } catch { + self.error = error.localizedDescription + self.plants = [] + self.allPlants = [] + } } - + currentLoadTask = task + await task.value + currentLoadTask = nil isLoading = false } diff --git a/PlantGuide/Presentation/Scenes/CareSchedule/CareScheduleViewModel.swift b/PlantGuide/Presentation/Scenes/CareSchedule/CareScheduleViewModel.swift index 0abe5f8..ef37972 100644 --- a/PlantGuide/Presentation/Scenes/CareSchedule/CareScheduleViewModel.swift +++ b/PlantGuide/Presentation/Scenes/CareSchedule/CareScheduleViewModel.swift @@ -148,8 +148,8 @@ final class CareScheduleViewModel { do { try await careScheduleRepository.updateTask(completedTask) } catch { - // Log error - in a production app, you might want to show an alert - print("Failed to persist task completion: \(error)") + // Revert in-memory state so UI stays consistent with Core Data + allTasks[index] = task } } diff --git a/PlantGuide/Presentation/Scenes/PlantDetail/PlantDetailViewModel.swift b/PlantGuide/Presentation/Scenes/PlantDetail/PlantDetailViewModel.swift index 5807ee1..cd1db0b 100644 --- a/PlantGuide/Presentation/Scenes/PlantDetail/PlantDetailViewModel.swift +++ b/PlantGuide/Presentation/Scenes/PlantDetail/PlantDetailViewModel.swift @@ -133,13 +133,14 @@ final class PlantDetailViewModel { forceRefresh: forceRefresh ) careInfo = info - - // Check for existing schedule - await loadExistingSchedule() } catch { self.error = error } + // Always load the existing schedule from Core Data, even if care info + // fetch failed. This ensures task completion states are always current. + await loadExistingSchedule() + isLoading = false } @@ -298,7 +299,10 @@ final class PlantDetailViewModel { do { try await careScheduleRepository.updateTask(completedTask) } catch { - print("Failed to persist task completion: \(error)") + // Revert in-memory state so UI stays consistent with Core Data + schedule.tasks[index] = task + careSchedule = schedule + self.error = error } } } diff --git a/PlantGuide/Presentation/Scenes/TodayView/TodayViewModel.swift b/PlantGuide/Presentation/Scenes/TodayView/TodayViewModel.swift index 9848407..4db1a24 100644 --- a/PlantGuide/Presentation/Scenes/TodayView/TodayViewModel.swift +++ b/PlantGuide/Presentation/Scenes/TodayView/TodayViewModel.swift @@ -216,8 +216,8 @@ final class TodayViewModel { do { try await careScheduleRepository.updateTask(completedTask) } catch { - // Log error - in a production app, you might want to show an alert - print("Failed to persist task completion: \(error)") + // Revert in-memory state so UI stays consistent with Core Data + allTasks[index] = task } } diff --git a/PlantGuideTests/CoreDataCareScheduleStorageTests.swift b/PlantGuideTests/CoreDataCareScheduleStorageTests.swift index bbce7d7..bc6adf9 100644 --- a/PlantGuideTests/CoreDataCareScheduleStorageTests.swift +++ b/PlantGuideTests/CoreDataCareScheduleStorageTests.swift @@ -86,9 +86,11 @@ final class MockCoreDataStack: CoreDataStackProtocol, @unchecked Sendable { let entityNames = persistentContainer.managedObjectModel.entities.compactMap { $0.name } for entityName in entityNames { - let fetchRequest = NSFetchRequest(entityName: entityName) - let deleteRequest = NSBatchDeleteRequest(fetchRequest: fetchRequest) - try context.execute(deleteRequest) + let fetchRequest = NSFetchRequest(entityName: entityName) + let objects = try context.fetch(fetchRequest) + for object in objects { + context.delete(object) + } } try save(context: context) diff --git a/PlantGuideTests/Mocks/MockPlantClassificationService.swift b/PlantGuideTests/Mocks/MockPlantClassificationService.swift index 5ca35fe..a01c5ed 100644 --- a/PlantGuideTests/Mocks/MockPlantClassificationService.swift +++ b/PlantGuideTests/Mocks/MockPlantClassificationService.swift @@ -143,7 +143,7 @@ struct MockImagePreprocessor: ImagePreprocessorProtocol, Sendable { // MARK: - MockIdentifyPlantUseCase /// Mock implementation of IdentifyPlantUseCaseProtocol for testing -struct MockIdentifyPlantUseCase: IdentifyPlantUseCaseProtocol, Sendable { +final class MockIdentifyPlantUseCase: IdentifyPlantUseCaseProtocol, @unchecked Sendable { // MARK: - Configuration @@ -167,7 +167,7 @@ struct MockIdentifyPlantUseCase: IdentifyPlantUseCaseProtocol, Sendable { /// Creates a mock that returns high-confidence predictions static func withHighConfidencePredictions() -> MockIdentifyPlantUseCase { - var mock = MockIdentifyPlantUseCase() + let mock = MockIdentifyPlantUseCase() mock.predictionsToReturn = [ ViewPlantPrediction( id: UUID(), @@ -181,7 +181,7 @@ struct MockIdentifyPlantUseCase: IdentifyPlantUseCaseProtocol, Sendable { /// Creates a mock that returns low-confidence predictions static func withLowConfidencePredictions() -> MockIdentifyPlantUseCase { - var mock = MockIdentifyPlantUseCase() + let mock = MockIdentifyPlantUseCase() mock.predictionsToReturn = [ ViewPlantPrediction( id: UUID(), @@ -195,7 +195,7 @@ struct MockIdentifyPlantUseCase: IdentifyPlantUseCaseProtocol, Sendable { /// Creates a mock that throws an error static func withError(_ error: Error = IdentifyPlantOnDeviceUseCaseError.noMatchesFound) -> MockIdentifyPlantUseCase { - var mock = MockIdentifyPlantUseCase() + let mock = MockIdentifyPlantUseCase() mock.shouldThrow = true mock.errorToThrow = error return mock @@ -205,7 +205,7 @@ struct MockIdentifyPlantUseCase: IdentifyPlantUseCaseProtocol, Sendable { // MARK: - MockIdentifyPlantOnlineUseCase /// Mock implementation of IdentifyPlantOnlineUseCaseProtocol for testing -struct MockIdentifyPlantOnlineUseCase: IdentifyPlantOnlineUseCaseProtocol, Sendable { +final class MockIdentifyPlantOnlineUseCase: IdentifyPlantOnlineUseCaseProtocol, @unchecked Sendable { // MARK: - Configuration @@ -229,7 +229,7 @@ struct MockIdentifyPlantOnlineUseCase: IdentifyPlantOnlineUseCaseProtocol, Senda /// Creates a mock that returns API predictions static func withPredictions() -> MockIdentifyPlantOnlineUseCase { - var mock = MockIdentifyPlantOnlineUseCase() + let mock = MockIdentifyPlantOnlineUseCase() mock.predictionsToReturn = [ ViewPlantPrediction( id: UUID(), @@ -243,7 +243,7 @@ struct MockIdentifyPlantOnlineUseCase: IdentifyPlantOnlineUseCaseProtocol, Senda /// Creates a mock that throws an error static func withError(_ error: Error = IdentifyPlantOnlineUseCaseError.noMatchesFound) -> MockIdentifyPlantOnlineUseCase { - var mock = MockIdentifyPlantOnlineUseCase() + let mock = MockIdentifyPlantOnlineUseCase() mock.shouldThrow = true mock.errorToThrow = error return mock