Rework and refactor delete and edit logic

* Ensure only entries that consist of fat and protein carry an fpuID
* Rework and refactor update and delete logic use case based (complex meal, fpu-only, carb-only)
* Various small stuff
* Still WIP

Co-Authored-By: polscm32 <polscm32@users.noreply.github.com>

FreeAPS/Sources/APS/Storage/CarbsStorage.swift

@@ -11,12 +11,11 @@ protocol CarbsObserver {
 protocol CarbsStorage {
     var updatePublisher: AnyPublisher<Void, Never> { get }
     func storeCarbs(_ carbs: [CarbsEntry], areFetchedFromRemote: Bool) async
-    func deleteCarbs(_ treatmentObjectID: NSManagedObjectID) async
+    func deleteCarbsEntryStored(_ treatmentObjectID: NSManagedObjectID) async
     func syncDate() -> Date
     func recent() -> [CarbsEntry]
     func getCarbsNotYetUploadedToNightscout() async -> [NightscoutTreatment]
     func getFPUsNotYetUploadedToNightscout() async -> [NightscoutTreatment]
-    func deleteCarbs(at uniqueID: String, fpuID: String, complex: Bool)
     func getCarbsNotYetUploadedToHealth() async -> [CarbsEntry]
     func getCarbsNotYetUploadedToTidepool() async -> [CarbsEntry]
 }
@@ -286,22 +285,26 @@ final class BaseCarbsStorage: CarbsStorage, Injectable {
         storage.retrieve(OpenAPS.Monitor.carbHistory, as: [CarbsEntry].self)?.reversed() ?? []
     }
 
-    func deleteCarbs(_ treatmentObjectID: NSManagedObjectID) async {
+    func deleteCarbsEntryStored(_ treatmentObjectID: NSManagedObjectID) async {
         let taskContext = CoreDataStack.shared.newTaskContext()
         taskContext.name = "deleteContext"
         taskContext.transactionAuthor = "deleteCarbs"
 
-        var carbEntry: CarbEntryStored?
+        var carbEntryFromCoreData: CarbEntryStored?
 
         await taskContext.perform {
             do {
-                carbEntry = try taskContext.existingObject(with: treatmentObjectID) as? CarbEntryStored
-                guard let carbEntry = carbEntry else {
+                carbEntryFromCoreData = try taskContext.existingObject(with: treatmentObjectID) as? CarbEntryStored
+                guard let carbEntry = carbEntryFromCoreData else {
                     debugPrint("Carb entry for batch delete not found. \(DebuggingIdentifiers.failed)")
                     return
                 }
 
-                if carbEntry.isFPU, let fpuID = carbEntry.fpuID {
+                // entry has fpuID
+                // case 1: carb equivalent entry
+                // case 2: "parent" entry, but containing fat and/or protein, and possibly carbs
+                // => use fpuID ID to delete all corresponding entries via batch delete
+                if let fpuID = carbEntry.fpuID {
                     // fetch request for all carb entries with the same id
                     let fetchRequest: NSFetchRequest<NSFetchRequestResult> = CarbEntryStored.fetchRequest()
                     fetchRequest.predicate = NSPredicate(format: "fpuID == %@", fpuID as CVarArg)
@@ -316,14 +319,17 @@ final class BaseCarbsStorage: CarbsStorage, Injectable {
 
                     // Notifiy subscribers of the batch delete
                     self.updateSubject.send(())
-                } else {
+                }
+                // entry has no fpuID
+                // => it's a carb-only entry. use its ID to for deletion
+                else {
                     taskContext.delete(carbEntry)
 
                     guard taskContext.hasChanges else { return }
                     try taskContext.save()
 
                     debugPrint(
-                        "Data Table State: \(#function) \(DebuggingIdentifiers.succeeded) deleted carb entry from core data"
+                        "CarbsStorage: \(#function) \(DebuggingIdentifiers.succeeded) deleted carb entry from core data"
                     )
                 }
 
@@ -333,36 +339,6 @@ final class BaseCarbsStorage: CarbsStorage, Injectable {
         }
     }
 
-    func deleteCarbs(at uniqueID: String, fpuID: String, complex: Bool) {
-        processQueue.sync {
-            var allValues = storage.retrieve(OpenAPS.Monitor.carbHistory, as: [CarbsEntry].self) ?? []
-
-            if fpuID != "" {
-                if allValues.firstIndex(where: { $0.fpuID == fpuID }) == nil {
-                    debug(.default, "Didn't find any carb equivalents to delete. ID to search for: " + fpuID.description)
-                } else {
-                    allValues.removeAll(where: { $0.fpuID == fpuID })
-                    storage.save(allValues, as: OpenAPS.Monitor.carbHistory)
-                    broadcaster.notify(CarbsObserver.self, on: processQueue) {
-                        $0.carbsDidUpdate(allValues)
-                    }
-                }
-            }
-
-            if fpuID == "" || complex {
-                if allValues.firstIndex(where: { $0.id == uniqueID }) == nil {
-                    debug(.default, "Didn't find any carb entries to delete. ID to search for: " + uniqueID.description)
-                } else {
-                    allValues.removeAll(where: { $0.id == uniqueID })
-                    storage.save(allValues, as: OpenAPS.Monitor.carbHistory)
-                    broadcaster.notify(CarbsObserver.self, on: processQueue) {
-                        $0.carbsDidUpdate(allValues)
-                    }
-                }
-            }
-        }
-    }
-
     func getCarbsNotYetUploadedToNightscout() async -> [NightscoutTreatment] {
         let results = await CoreDataStack.shared.fetchEntitiesAsync(
             ofType: CarbEntryStored.self,

FreeAPS/Sources/Modules/DataTable/DataTableStateModel.swift

@@ -103,9 +103,9 @@ extension DataTable {
 
         // Carb and FPU deletion from history
         /// - **Parameter**: NSManagedObjectID to be able to transfer the object safely from one thread to another thread
-        func invokeCarbDeletionTask(_ treatmentObjectID: NSManagedObjectID) {
+        func invokeCarbDeletionTask(_ treatmentObjectID: NSManagedObjectID, isFpuOrComplexMeal: Bool = false) {
             Task {
-                await deleteCarbs(treatmentObjectID)
+                await deleteCarbs(treatmentObjectID, isFpuOrComplexMeal: isFpuOrComplexMeal)
 
                 await MainActor.run {
                     carbEntryDeleted = true
@@ -114,12 +114,16 @@ extension DataTable {
             }
         }
 
-        func deleteCarbs(_ treatmentObjectID: NSManagedObjectID) async {
-            // Delete from Apple Health/Tidepool
-            await deleteCarbsFromServices(treatmentObjectID)
+        func deleteCarbs(_ treatmentObjectID: NSManagedObjectID, isFpuOrComplexMeal: Bool = false) async {
+            // Delete from Nightscout/Apple Health/Tidepool
+            if isFpuOrComplexMeal {
+                await deleteCarbsAndFPUsFromServices(treatmentObjectID)
+            } else {
+                await deleteCarbsFromServices(treatmentObjectID)
+            }
 
             // Delete carbs from Core Data
-            await carbsStorage.deleteCarbs(treatmentObjectID)
+            await carbsStorage.deleteCarbsEntryStored(treatmentObjectID)
 
             // Perform a determine basal sync to update cob
             await apsManager.determineBasalSync()
@@ -183,6 +187,80 @@ extension DataTable {
             }
         }
 
+        func deleteCarbsAndFPUsFromServices(_ treatmentObjectID: NSManagedObjectID) async {
+            let taskContext = CoreDataStack.shared.newTaskContext()
+            taskContext.name = "deleteContext"
+            taskContext.transactionAuthor = "deleteCarbsFromServices"
+
+            var carbEntryFromCoreData: CarbEntryStored?
+
+            guard let correspondingCarbEntry: (
+                entryValues: (carbs: Decimal, fat: Decimal, protein: Decimal, note: String)?,
+                entryID: NSManagedObjectID?
+            ) = await handleFPUEntry(treatmentObjectID),
+                let nsManagedObjectID: NSManagedObjectID = correspondingCarbEntry.entryID else { return }
+
+            // Delete carbs or FPUs from Nightscout
+            await taskContext.perform {
+                do {
+                    carbEntryFromCoreData = try taskContext.existingObject(with: nsManagedObjectID) as? CarbEntryStored
+                    guard let carbEntry = carbEntryFromCoreData else {
+                        debugPrint("FPU entry or corresponding carb entry for deletion not found. \(DebuggingIdentifiers.failed)")
+                        return
+                    }
+
+                    if let fpuID = carbEntry.fpuID {
+                        // Delete Fat and Protein entries from Nightscout
+                        self.provider.deleteCarbsFromNightscout(withID: fpuID.uuidString)
+
+                        // Delete Fat and Protein entries from Apple Health
+                        let healthObjectsToDelete: [HKSampleType?] = [
+                            AppleHealthConfig.healthFatObject,
+                            AppleHealthConfig.healthProteinObject
+                        ]
+
+                        for sampleType in healthObjectsToDelete {
+                            if let validSampleType = sampleType {
+                                self.provider.deleteMealDataFromHealth(byID: fpuID.uuidString, sampleType: validSampleType)
+                            }
+                        }
+
+                        // if entry has carbs AND fat and/or protein, we also need to remove carbs here
+                        if carbEntry.carbs > 0, let id = carbEntry.id {
+                            self.provider.deleteCarbsFromNightscout(withID: id.uuidString)
+
+                            // Delete carbs from Apple Health
+                            if let sampleType = AppleHealthConfig.healthCarbObject {
+                                self.provider.deleteMealDataFromHealth(byID: id.uuidString, sampleType: sampleType)
+                            }
+                        }
+                    }
+                    // entry has no fpuID
+                    // => it's a carb-only entry. use its ID to for all deletion operations
+                    else if let id = carbEntry.id, let entryDate = carbEntry.date {
+                        self.provider.deleteCarbsFromNightscout(withID: id.uuidString)
+
+                        // Delete carbs from Apple Health
+                        if let sampleType = AppleHealthConfig.healthCarbObject {
+                            self.provider.deleteMealDataFromHealth(byID: id.uuidString, sampleType: sampleType)
+                        }
+
+                        self.provider.deleteCarbsFromTidepool(
+                            withSyncId: id,
+                            carbs: Decimal(carbEntry.carbs),
+                            at: entryDate,
+                            enteredBy: CarbsEntry.local
+                        )
+                    }
+
+                } catch {
+                    debugPrint(
+                        "\(DebuggingIdentifiers.failed) Error deleting carb entry (and associated fpu entries) from remote service(s) (Nightscout, Apple Health, Tidepool) with error: \(error.localizedDescription)"
+                    )
+                }
+            }
+        }
+
         // Insulin deletion from history
         /// - **Parameter**: NSManagedObjectID to be able to transfer the object safely from one thread to another thread
         func invokeInsulinDeletionTask(_ treatmentObjectID: NSManagedObjectID) {
@@ -268,90 +346,41 @@ extension DataTable {
             newNote: String
         ) {
             Task {
-                let originalDate = await getOriginalEntryDate(treatmentObjectID)
-                await updateEntryInCoreData(treatmentObjectID, newCarbs: newCarbs, newNote: newNote)
-                await deleteOldAndCreateNewFPUEntry(
-                    treatmentObjectID: treatmentObjectID,
-                    originalDate: originalDate,
+                // Get original date from entry to re-create the entry later with the updated values and the same date
+                guard let originalEntry = await getOriginalEntryValues(treatmentObjectID) else { return }
+
+                // TODO: theoretically we now already have the original entry here => we maybe don't need to perform all the fetching in the deleteServices method 2 levels down 👀
+
+                // Deletion logic for carb and FPU entries
+                await deleteOldEntries(
+                    treatmentObjectID,
+                    originalEntry: originalEntry,
                     newCarbs: newCarbs,
                     newFat: newFat,
                     newProtein: newProtein,
                     newNote: newNote
                 )
-                await syncWithServices()
-            }
-        }
-
-        /// Retrieves the original date of an entry and sets the isFPU flag
-        /// - Parameter objectID: The ID of the entry
-        /// - Returns: The original date or current date if not found
-        private func getOriginalEntryDate(_ objectID: NSManagedObjectID) async -> Date {
-            let context = CoreDataStack.shared.newTaskContext()
-            context.name = "updateContext"
-            context.transactionAuthor = "updateEntry"
-
-            return await context.perform {
-                do {
-                    guard let entry = try context.existingObject(with: objectID) as? CarbEntryStored
-                    else { return Date() }
 
-                    /// Hacky workaround: Set isFPU flag to true before deletion
-                    /// This is necessary because the deleteCarbs function in the CarbsStorage will fail if the isFPU flag is false and the entry won't get deleted.
-                    entry.isFPU = true
-                    try context.save()
-
-                    return entry.date ?? Date()
-                } catch {
-                    return Date()
-                }
-            }
-        }
-
-        /// Updates a carb entry in Core Data
-        /// The FPU entries are deleted and recreated. We don't need to do this for the carb entries as we can simply update the carb entry in Core Data.
-        /// - Parameters:
-        ///   - objectID: The ID of the entry to update
-        ///   - newCarbs: The new carbs value
-        ///   - newNote: The new note text
-        private func updateEntryInCoreData(
-            _ objectID: NSManagedObjectID,
-            newCarbs: Decimal,
-            newNote: String
-        ) async {
-            let context = CoreDataStack.shared.newTaskContext()
+                await createNewEntries(
+                    originalDate: originalEntry.entryValues?.date ?? Date(),
+                    // TODO: should we add this to the guard or is nullish coalesce safe enough?
+                    newCarbs: newCarbs,
+                    newFat: newFat,
+                    newProtein: newProtein,
+                    newNote: newNote
+                )
 
-            await context.perform {
-                do {
-                    let entry = try context.existingObject(with: objectID) as? CarbEntryStored
-                    entry?.carbs = Double(newCarbs)
-                    entry?.note = newNote
-                    try context.save()
-                } catch {
-                    debugPrint("\(DebuggingIdentifiers.failed) Failed to update entry: \(error.localizedDescription)")
-                }
+                await syncWithServices()
             }
         }
 
-        /// Deletes the old FPU entry and creates a new one with updated values
-        /// - Parameters:
-        ///   - treatmentObjectID: The ID of the entry to delete
-        ///   - originalDate: The original date to preserve
-        ///   - newCarbs: The new carbs value
-        ///   - newFat: The new fat value
-        ///   - newProtein: The new protein value
-        ///   - newNote: The new note text
-        private func deleteOldAndCreateNewFPUEntry(
-            treatmentObjectID: NSManagedObjectID,
+        private func createNewEntries(
             originalDate: Date,
             newCarbs: Decimal,
             newFat: Decimal,
             newProtein: Decimal,
             newNote: String
         ) async {
-            // Delete old FPU entry from Core Data and Remote Services and await this
-            await deleteCarbs(treatmentObjectID)
-
-            // Create new FPU entry
             let newEntry = CarbsEntry(
                 id: UUID().uuidString,
                 createdAt: Date(),
@@ -361,13 +390,84 @@ extension DataTable {
                 protein: newProtein,
                 note: newNote,
                 enteredBy: CarbsEntry.local,
-                isFPU: true,
-                fpuID: UUID().uuidString
+                isFPU: false,
+                fpuID: newFat > 0 || newProtein > 0 ? UUID().uuidString : nil
             )
 
+            // Handles internally whether to create fake carbs or not based on whether fat > 0 or protein > 0
             await carbsStorage.storeCarbs([newEntry], areFetchedFromRemote: false)
         }
 
+        /// Deletes the old carb/ FPU entries and creates new ones with updated values
+        /// - Parameters:
+        ///   - treatmentObjectID: The ID of the entry to delete
+        ///   - originalDate: The original date to preserve
+        ///   - newCarbs: The new carbs value
+        ///   - newFat: The new fat value
+        ///   - newProtein: The new protein value
+        ///   - newNote: The new note text
+        private func deleteOldEntries(
+            _ treatmentObjectID: NSManagedObjectID,
+            originalEntry: (
+                entryValues: (date: Date, oldCarbs: Double, oldFat: Double, oldProtein: Double)?,
+                entryId: NSManagedObjectID
+            ),
+            newCarbs _: Decimal,
+            newFat _: Decimal,
+            newProtein _: Decimal,
+            newNote _: String
+        ) async {
+            // TODO: cleanup
+            // TODO: maybe pass originalEntry instead of treatmentObjectId down?
+
+            if ((originalEntry.entryValues?.oldCarbs ?? 0) == 0 && (originalEntry.entryValues?.oldFat ?? 0) > 0) ||
+                ((originalEntry.entryValues?.oldCarbs ?? 0) == 0 && (originalEntry.entryValues?.oldProtein ?? 0) > 0)
+            {
+                // Delete the zero-carb-entry and all its carb equivalents connected by the same fpuID from remote services and Core Data
+                // Use fpuID
+                await deleteCarbs(treatmentObjectID, isFpuOrComplexMeal: true)
+            } else if ((originalEntry.entryValues?.oldCarbs ?? 0) > 0 && (originalEntry.entryValues?.oldFat ?? 0) > 0) ||
+                ((originalEntry.entryValues?.oldCarbs ?? 0) > 0 && (originalEntry.entryValues?.oldProtein ?? 0) > 0)
+            {
+                // Delete carb entry and carb equivalents that are all connected by the same fpuID from remote services and Core Data
+                // Use fpuID
+                await deleteCarbs(treatmentObjectID, isFpuOrComplexMeal: true)
+
+            } else {
+                // Delete just the carb entry since there are no carb equivalents
+                // Use NSManagedObjectID
+                await deleteCarbs(treatmentObjectID)
+            }
+        }
+
+        /// Retrieves the original entry values
+        /// - Parameter objectID: The ID of the entry
+        /// - Returns: A tuple of the old entry values and its original date and the objectID or nil
+        private func getOriginalEntryValues(_ objectID: NSManagedObjectID) async
+            -> (entryValues: (date: Date, oldCarbs: Double, oldFat: Double, oldProtein: Double)?, entryId: NSManagedObjectID)?
+        {
+            let context = CoreDataStack.shared.newTaskContext()
+            context.name = "updateContext"
+            context.transactionAuthor = "updateEntry"
+            
+            // TODO: possibly extend this by id and fpuID to not having to fetch again later on
+
+            return await context.perform {
+                do {
+                    guard let entry = try context.existingObject(with: objectID) as? CarbEntryStored, let entryDate = entry.date
+                    else { return nil }
+
+                    return (
+                        entryValues: (date: entryDate, oldCarbs: entry.carbs, oldFat: entry.fat, oldProtein: entry.protein),
+                        entryId: entry.objectID
+                    )
+                } catch let error as NSError {
+                    debugPrint("\(DebuggingIdentifiers.failed) Failed to get original date with error: \(error.userInfo)")
+                    return nil
+                }
+            }
+        }
+
         /// Synchronizes the FPU/ Carb entry with all remote services in parallel
         private func syncWithServices() async {
             async let nightscoutUpload: () = provider.nightscoutManager.uploadCarbs()

FreeAPS/Sources/Modules/DataTable/View/DataTableRootView.swift

@@ -626,7 +626,10 @@ extension DataTable {
                     }
                     let treatmentObjectID = carbEntryToDelete.objectID
 
-                    state.invokeCarbDeletionTask(treatmentObjectID)
+                    state.invokeCarbDeletionTask(
+                        treatmentObjectID,
+                        isFpuOrComplexMeal: carbEntryToDelete.isFPU || carbEntryToDelete.fat > 0 || carbEntryToDelete.protein > 0
+                    )
                 }
             } message: {
                 Text("\n" + NSLocalizedString(alertMessage, comment: ""))

FreeAPS/Sources/Modules/Treatments/TreatmentsStateModel.swift

@@ -568,7 +568,7 @@ extension Treatments {
                 note: note,
                 enteredBy: CarbsEntry.local,
                 isFPU: false,
-                fpuID: UUID().uuidString
+                fpuID: fat > 0 || protein > 0 ? UUID().uuidString : nil
             )]
             await carbsStorage.storeCarbs(carbsToStore, areFetchedFromRemote: false)
 

FreeAPS/Sources/Services/HealthKit/HealthKitManager.swift

@@ -245,26 +245,27 @@ final class BaseHealthKitManager: HealthKitManager, Injectable {
             for allSamples in carbs {
                 guard let id = allSamples.id else { continue }
                 let fpuID = allSamples.fpuID ?? id
-
                 let startDate = allSamples.actualDate ?? Date()
 
-                // Carbs Sample
+                // Carbs Sample (only if value is greater than 0)
                 let carbValue = allSamples.carbs
-                let carbSample = HKQuantitySample(
-                    type: carbSampleType,
-                    quantity: HKQuantity(unit: .gram(), doubleValue: Double(carbValue)),
-                    start: startDate,
-                    end: startDate,
-                    metadata: [
-                        HKMetadataKeyExternalUUID: id,
-                        HKMetadataKeySyncIdentifier: id,
-                        HKMetadataKeySyncVersion: 1
-                    ]
-                )
-                samples.append(carbSample)
+                if carbValue > 0 {
+                    let carbSample = HKQuantitySample(
+                        type: carbSampleType,
+                        quantity: HKQuantity(unit: .gram(), doubleValue: Double(carbValue)),
+                        start: startDate,
+                        end: startDate,
+                        metadata: [
+                            HKMetadataKeyExternalUUID: id,
+                            HKMetadataKeySyncIdentifier: id,
+                            HKMetadataKeySyncVersion: 1
+                        ]
+                    )
+                    samples.append(carbSample)
+                }
 
-                // Fat Sample (if available)
-                if let fatValue = allSamples.fat {
+                // Fat Sample (only if value is greater than 0)
+                if let fatValue = allSamples.fat, fatValue > 0 {
                     let fatSample = HKQuantitySample(
                         type: fatSampleType,
                         quantity: HKQuantity(unit: .gram(), doubleValue: Double(fatValue)),
@@ -279,8 +280,8 @@ final class BaseHealthKitManager: HealthKitManager, Injectable {
                     samples.append(fatSample)
                 }
 
-                // Protein Sample (if available)
-                if let proteinValue = allSamples.protein {
+                // Protein Sample (only if value is greater than 0)
+                if let proteinValue = allSamples.protein, proteinValue > 0 {
                     let proteinSample = HKQuantitySample(
                         type: proteinSampleType,
                         quantity: HKQuantity(unit: .gram(), doubleValue: Double(proteinValue)),

FreeAPS/Sources/Services/RemoteControl/TrioRemoteControl+Meal.swift

@@ -69,7 +69,7 @@ extension TrioRemoteControl {
             note: "Remote meal command",
             enteredBy: CarbsEntry.local,
             isFPU: false,
-            fpuID: nil
+            fpuID: fatDecimal ?? 0 > 0 || proteinDecimal ?? 0 > 0 ? UUID().uuidString : nil
         )
 
         await carbsStorage.storeCarbs([mealEntry], areFetchedFromRemote: false)

Model/Helper/CarbEntryStored+helper.swift

@@ -25,7 +25,7 @@ extension NSPredicate {
     static var carbsNotYetUploadedToHealth: NSPredicate {
         let date = Date.oneDayAgo
         return NSPredicate(
-            format: "date >= %@ AND isUploadedToHealth == %@ AND carbs > 0",
+            format: "date >= %@ AND isUploadedToHealth == %@",
             date as NSDate,
             false as NSNumber
         )
@@ -34,7 +34,7 @@ extension NSPredicate {
     static var carbsNotYetUploadedToTidepool: NSPredicate {
         let date = Date.oneDayAgo
         return NSPredicate(
-            format: "date >= %@ AND isUploadedToTidepool == %@ AND carbs > 0",
+            format: "date >= %@ AND isUploadedToTidepool == %@",
             date as NSDate,
             false as NSNumber
         )