Fix grocery sync deletions not persisting and Reminders race condition

Stop cascading syncs by adding an isReconciling flag so that
reconcileFromServer no longer triggers scheduleSync via addItem/deleteItem.
Make Reminders write-only by removing the diff/sync logic from the
onDataChanged callback. Fetch fresh server state in RecipeView reconcile
instead of using stale local cache. Track pending removal recipe IDs via
DataStore so performInitialSync can push deletions for recipes whose
grocery keys have already been removed from groceryDict.

Fix a race condition in RemindersGroceryStore where EKEventStoreChanged
notifications triggered load() before saveMappings() finished writing to
disk, causing the correct in-memory state to be overwritten with stale
data. Add ignoreNextExternalChange flag to skip self-triggered reloads.

Restyle the add/remove all grocery button to match the Plan recipe button
style using Label, subheadline font, and rounded rectangle background.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit is contained in:
2026-02-15 06:04:41 +01:00
parent 8b23652f10
commit c38d4075be
6 changed files with 123 additions and 35 deletions

View File

@@ -28,20 +28,7 @@ class GroceryListManager: ObservableObject {
remindersStore.onDataChanged = { [weak self] in remindersStore.onDataChanged = { [weak self] in
guard let self else { return } guard let self else { return }
if self.mode == .appleReminders { if self.mode == .appleReminders {
let previousDict = self.groceryDict
self.groceryDict = self.remindersStore.groceryDict self.groceryDict = self.remindersStore.groceryDict
// Only sync recipes that changed externally (e.g. checked off in Reminders app),
// not ones we just modified ourselves.
for recipeId in self.remindersStore.groceryDict.keys {
guard !self.recentlyModifiedByUs.contains(recipeId) else { continue }
// Detect if item count changed (external add/remove/complete)
let oldCount = previousDict[recipeId]?.items.count ?? 0
let newCount = self.remindersStore.groceryDict[recipeId]?.items.count ?? 0
if oldCount != newCount {
self.syncManager?.scheduleSync(forRecipeId: recipeId)
}
}
self.recentlyModifiedByUs.removeAll() self.recentlyModifiedByUs.removeAll()
} }
} }
@@ -63,6 +50,7 @@ class GroceryListManager: ObservableObject {
remindersStore.addItem(itemName, toRecipe: recipeId, recipeName: recipeName) remindersStore.addItem(itemName, toRecipe: recipeId, recipeName: recipeName)
groceryDict = remindersStore.groceryDict groceryDict = remindersStore.groceryDict
} }
syncManager?.clearPendingRemoval(recipeId: recipeId)
syncManager?.scheduleSync(forRecipeId: recipeId) syncManager?.scheduleSync(forRecipeId: recipeId)
} }
@@ -76,6 +64,7 @@ class GroceryListManager: ObservableObject {
remindersStore.addItems(items, toRecipe: recipeId, recipeName: recipeName) remindersStore.addItems(items, toRecipe: recipeId, recipeName: recipeName)
groceryDict = remindersStore.groceryDict groceryDict = remindersStore.groceryDict
} }
syncManager?.clearPendingRemoval(recipeId: recipeId)
syncManager?.scheduleSync(forRecipeId: recipeId) syncManager?.scheduleSync(forRecipeId: recipeId)
} }
@@ -88,6 +77,9 @@ class GroceryListManager: ObservableObject {
recentlyModifiedByUs.insert(recipeId) recentlyModifiedByUs.insert(recipeId)
remindersStore.deleteItem(itemName, fromRecipe: recipeId) remindersStore.deleteItem(itemName, fromRecipe: recipeId)
} }
if groceryDict[recipeId] == nil {
syncManager?.trackPendingRemoval(recipeId: recipeId)
}
syncManager?.scheduleSync(forRecipeId: recipeId) syncManager?.scheduleSync(forRecipeId: recipeId)
} }
@@ -100,6 +92,7 @@ class GroceryListManager: ObservableObject {
recentlyModifiedByUs.insert(recipeId) recentlyModifiedByUs.insert(recipeId)
remindersStore.deleteGroceryRecipe(recipeId) remindersStore.deleteGroceryRecipe(recipeId)
} }
syncManager?.trackPendingRemoval(recipeId: recipeId)
syncManager?.scheduleSync(forRecipeId: recipeId) syncManager?.scheduleSync(forRecipeId: recipeId)
} }
@@ -114,6 +107,7 @@ class GroceryListManager: ObservableObject {
remindersStore.deleteAll() remindersStore.deleteAll()
} }
for recipeId in recipeIds { for recipeId in recipeIds {
syncManager?.trackPendingRemoval(recipeId: recipeId)
syncManager?.scheduleSync(forRecipeId: recipeId) syncManager?.scheduleSync(forRecipeId: recipeId)
} }
} }

View File

@@ -13,6 +13,11 @@ class GroceryStateSyncManager {
private var debounceTimers: [String: Task<Void, Never>] = [:] private var debounceTimers: [String: Task<Void, Never>] = [:]
private let debounceInterval: TimeInterval = 2.0 private let debounceInterval: TimeInterval = 2.0
private var isReconciling = false
private let dataStore = DataStore()
private let pendingRemovalPath = "grocery_pending_removals.data"
private(set) var pendingRemovalRecipeIds: Set<String> = []
init(appState: AppState, groceryManager: GroceryListManager) { init(appState: AppState, groceryManager: GroceryListManager) {
self.appState = appState self.appState = appState
@@ -24,6 +29,7 @@ class GroceryStateSyncManager {
/// Debounced sync trigger. Waits `debounceInterval` seconds then pushes state for the recipe. /// Debounced sync trigger. Waits `debounceInterval` seconds then pushes state for the recipe.
func scheduleSync(forRecipeId recipeId: String) { func scheduleSync(forRecipeId recipeId: String) {
guard UserSettings.shared.grocerySyncEnabled else { return } guard UserSettings.shared.grocerySyncEnabled else { return }
guard !isReconciling else { return }
debounceTimers[recipeId]?.cancel() debounceTimers[recipeId]?.cancel()
debounceTimers[recipeId] = Task { [weak self] in debounceTimers[recipeId] = Task { [weak self] in
@@ -38,17 +44,18 @@ class GroceryStateSyncManager {
guard let appState, let groceryManager else { return } guard let appState, let groceryManager else { return }
guard let recipeIdInt = Int(recipeId) else { return } guard let recipeIdInt = Int(recipeId) else { return }
// Build local state from current grocery data // Fetch latest recipe from server first so we can detect deletions
let localState = buildLocalState(forRecipeId: recipeId, groceryManager: groceryManager)
// Fetch latest recipe from server
guard let serverRecipe = await appState.getRecipe(id: recipeIdInt, fetchMode: .onlyServer) else { guard let serverRecipe = await appState.getRecipe(id: recipeIdInt, fetchMode: .onlyServer) else {
Logger.data.error("Grocery sync: failed to fetch recipe \(recipeId) from server") Logger.data.error("Grocery sync: failed to fetch recipe \(recipeId) from server")
return return
} }
// Merge local state with server state
let serverState = serverRecipe.groceryState let serverState = serverRecipe.groceryState
// Build local state, passing server state so deleted items can be marked .removed
let localState = buildLocalState(forRecipeId: recipeId, groceryManager: groceryManager, serverState: serverState)
// Merge local state with server state
let merged = mergeStates(local: localState, server: serverState) let merged = mergeStates(local: localState, server: serverState)
// Upload merged state // Upload merged state
@@ -67,6 +74,9 @@ class GroceryStateSyncManager {
guard let groceryManager else { return } guard let groceryManager else { return }
guard let serverState, !serverState.items.isEmpty else { return } guard let serverState, !serverState.items.isEmpty else { return }
isReconciling = true
defer { isReconciling = false }
let localItems = Set( let localItems = Set(
groceryManager.groceryDict[recipeId]?.items.map(\.name) ?? [] groceryManager.groceryDict[recipeId]?.items.map(\.name) ?? []
) )
@@ -94,6 +104,8 @@ class GroceryStateSyncManager {
func performInitialSync() async { func performInitialSync() async {
guard let appState, let groceryManager else { return } guard let appState, let groceryManager else { return }
await loadPendingRemovals()
let recipeIds = Array(groceryManager.groceryDict.keys) let recipeIds = Array(groceryManager.groceryDict.keys)
for recipeId in recipeIds { for recipeId in recipeIds {
guard let recipeIdInt = Int(recipeId) else { continue } guard let recipeIdInt = Int(recipeId) else { continue }
@@ -111,6 +123,18 @@ class GroceryStateSyncManager {
) )
} }
} }
// Push deletion state for recipes whose items were fully removed
for recipeId in pendingRemovalRecipeIds {
guard !recipeIds.contains(recipeId) else {
// Recipe was re-added locally since removal was tracked; clear it
pendingRemovalRecipeIds.remove(recipeId)
continue
}
await pushGroceryState(forRecipeId: recipeId)
pendingRemovalRecipeIds.remove(recipeId)
}
savePendingRemovals()
} }
// MARK: - Merge Logic // MARK: - Merge Logic
@@ -151,18 +175,59 @@ class GroceryStateSyncManager {
// MARK: - Build Local State // MARK: - Build Local State
/// Builds a `GroceryState` from the current local grocery data for a recipe. /// Builds a `GroceryState` from the current local grocery data for a recipe.
private func buildLocalState(forRecipeId recipeId: String, groceryManager: GroceryListManager) -> GroceryState { /// When `serverState` is provided, any server item with `.added` status that is
guard let groceryRecipe = groceryManager.groceryDict[recipeId] else { /// absent locally is emitted as `.removed` so the deletion propagates to the server.
return GroceryState() private func buildLocalState(forRecipeId recipeId: String, groceryManager: GroceryListManager, serverState: GroceryState?) -> GroceryState {
}
var items: [String: GroceryItemState] = [:] var items: [String: GroceryItemState] = [:]
let now = GroceryStateDate.now() let now = GroceryStateDate.now()
// Existing local items
if let groceryRecipe = groceryManager.groceryDict[recipeId] {
for item in groceryRecipe.items { for item in groceryRecipe.items {
let status: GroceryItemState.Status = item.isChecked ? .completed : .added let status: GroceryItemState.Status = item.isChecked ? .completed : .added
items[item.name] = GroceryItemState(status: status, addedAt: now, modifiedAt: now) items[item.name] = GroceryItemState(status: status, addedAt: now, modifiedAt: now)
} }
}
// Mark items that exist on server as .added but are absent locally as .removed
if let serverState {
for (itemName, serverItem) in serverState.items {
if items[itemName] == nil && serverItem.status == .added {
items[itemName] = GroceryItemState(
status: .removed,
addedAt: serverItem.addedAt,
modifiedAt: now
)
}
}
}
return GroceryState(lastModified: now, items: items) return GroceryState(lastModified: now, items: items)
} }
// MARK: - Pending Removal Tracking
/// Records a recipe ID whose grocery items were fully removed, so that
/// `performInitialSync` can push the deletion even after the key disappears
/// from `groceryDict`.
func trackPendingRemoval(recipeId: String) {
pendingRemovalRecipeIds.insert(recipeId)
savePendingRemovals()
}
func clearPendingRemoval(recipeId: String) {
guard pendingRemovalRecipeIds.remove(recipeId) != nil else { return }
savePendingRemovals()
}
private func loadPendingRemovals() async {
guard let loaded: Set<String> = try? await dataStore.load(fromPath: pendingRemovalPath) else { return }
pendingRemovalRecipeIds = loaded
}
private func savePendingRemovals() {
Task {
await dataStore.save(data: pendingRemovalRecipeIds, toPath: pendingRemovalPath)
}
}
} }

View File

@@ -35,6 +35,11 @@ class RemindersGroceryStore {
private let mappingPath = "reminder_mappings.data" private let mappingPath = "reminder_mappings.data"
private var mappingStore = ReminderMappingStore() private var mappingStore = ReminderMappingStore()
/// When true, the next `EKEventStoreChanged` notification is skipped because
/// it was triggered by our own save. Prevents a race where `load()` reads stale
/// mapping data from disk before `saveMappings()` finishes writing.
private var ignoreNextExternalChange = false
init() { init() {
NotificationCenter.default.addObserver( NotificationCenter.default.addObserver(
forName: .EKEventStoreChanged, forName: .EKEventStoreChanged,
@@ -43,6 +48,10 @@ class RemindersGroceryStore {
) { [weak self] _ in ) { [weak self] _ in
Task { @MainActor in Task { @MainActor in
guard let self else { return } guard let self else { return }
if self.ignoreNextExternalChange {
self.ignoreNextExternalChange = false
return
}
await self.load() await self.load()
self.onDataChanged?() self.onDataChanged?()
} }
@@ -104,11 +113,13 @@ class RemindersGroceryStore {
reminder.title = itemName reminder.title = itemName
reminder.calendar = calendar reminder.calendar = calendar
do { do {
ignoreNextExternalChange = true
try eventStore.save(reminder, commit: true) try eventStore.save(reminder, commit: true)
let name = recipeName ?? "-" let name = recipeName ?? "-"
addMapping(reminderIdentifier: reminder.calendarItemIdentifier, recipeId: recipeId, recipeName: name, itemName: itemName) addMapping(reminderIdentifier: reminder.calendarItemIdentifier, recipeId: recipeId, recipeName: name, itemName: itemName)
appendToCache(itemName: itemName, recipeId: recipeId, recipeName: name) appendToCache(itemName: itemName, recipeId: recipeId, recipeName: name)
} catch { } catch {
ignoreNextExternalChange = false
Logger.view.error("Failed to save reminder: \(error.localizedDescription)") Logger.view.error("Failed to save reminder: \(error.localizedDescription)")
} }
} }
@@ -132,8 +143,10 @@ class RemindersGroceryStore {
} }
} }
do { do {
ignoreNextExternalChange = true
try eventStore.commit() try eventStore.commit()
} catch { } catch {
ignoreNextExternalChange = false
Logger.view.error("Failed to commit reminders: \(error.localizedDescription)") Logger.view.error("Failed to commit reminders: \(error.localizedDescription)")
} }
saveMappings() saveMappings()
@@ -153,8 +166,10 @@ class RemindersGroceryStore {
let reminders = await fetchReminders(matching: predicate, in: store) let reminders = await fetchReminders(matching: predicate, in: store)
for reminder in reminders where reminder.calendarItemIdentifier == identifier { for reminder in reminders where reminder.calendarItemIdentifier == identifier {
do { do {
self.ignoreNextExternalChange = true
try self.eventStore.remove(reminder, commit: true) try self.eventStore.remove(reminder, commit: true)
} catch { } catch {
self.ignoreNextExternalChange = false
Logger.view.error("Failed to remove reminder: \(error.localizedDescription)") Logger.view.error("Failed to remove reminder: \(error.localizedDescription)")
} }
break break
@@ -181,8 +196,10 @@ class RemindersGroceryStore {
} }
} }
do { do {
self.ignoreNextExternalChange = true
try self.eventStore.commit() try self.eventStore.commit()
} catch { } catch {
self.ignoreNextExternalChange = false
Logger.view.error("Failed to commit reminder removal: \(error.localizedDescription)") Logger.view.error("Failed to commit reminder removal: \(error.localizedDescription)")
} }
self.mappingStore.recipes.removeValue(forKey: recipeId) self.mappingStore.recipes.removeValue(forKey: recipeId)
@@ -209,8 +226,10 @@ class RemindersGroceryStore {
} }
} }
do { do {
self.ignoreNextExternalChange = true
try self.eventStore.commit() try self.eventStore.commit()
} catch { } catch {
self.ignoreNextExternalChange = false
Logger.view.error("Failed to commit reminder removal: \(error.localizedDescription)") Logger.view.error("Failed to commit reminder removal: \(error.localizedDescription)")
} }
self.mappingStore.recipes = [:] self.mappingStore.recipes = [:]

View File

@@ -3802,12 +3802,12 @@
} }
} }
}, },
"Remove from Grocery List" : { "Remove all from Grocery List" : {
"localizations" : { "localizations" : {
"de" : { "de" : {
"stringUnit" : { "stringUnit" : {
"state" : "translated", "state" : "translated",
"value" : "Von der Einkaufsliste entfernen" "value" : "Alle von Einkaufsliste entfernen"
} }
}, },
"es" : { "es" : {

View File

@@ -88,8 +88,12 @@ struct RecipeView: View {
// Reconcile server grocery state with local data // Reconcile server grocery state with local data
if UserSettings.shared.grocerySyncEnabled { if UserSettings.shared.grocerySyncEnabled {
let serverRecipe = await appState.getRecipe(
id: viewModel.recipe.recipe_id,
fetchMode: .onlyServer
)
groceryList.syncManager?.reconcileFromServer( groceryList.syncManager?.reconcileFromServer(
serverState: viewModel.recipeDetail.groceryState, serverState: serverRecipe?.groceryState,
recipeId: String(viewModel.recipe.recipe_id), recipeId: String(viewModel.recipe.recipe_id),
recipeName: viewModel.recipeDetail.name recipeName: viewModel.recipeDetail.name
) )

View File

@@ -61,14 +61,20 @@ struct RecipeIngredientSection: View {
} }
} }
} label: { } label: {
HStack { Label(
Image(systemName: groceryList.containsRecipe(viewModel.observableRecipeDetail.id) ? "cart.badge.minus" : "cart.badge.plus") groceryList.containsRecipe(viewModel.observableRecipeDetail.id) ? "Remove all from Grocery List" : "Add All to Grocery List",
.foregroundStyle(groceryList.containsRecipe(viewModel.observableRecipeDetail.id) ? .red : .green) systemImage: groceryList.containsRecipe(viewModel.observableRecipeDetail.id) ? "cart.badge.minus" : "cart.badge.plus"
Text(groceryList.containsRecipe(viewModel.observableRecipeDetail.id) ? "Remove from Grocery List" : "Add All to Grocery List") )
} .font(.subheadline)
.fontWeight(.medium)
.frame(maxWidth: .infinity) .frame(maxWidth: .infinity)
.padding(.vertical, 10)
.foregroundStyle(groceryList.containsRecipe(viewModel.observableRecipeDetail.id) ? Color.red : Color.green)
.background(
RoundedRectangle(cornerRadius: 10)
.fill((groceryList.containsRecipe(viewModel.observableRecipeDetail.id) ? Color.red : Color.green).opacity(0.1))
)
} }
.buttonStyle(.bordered)
.padding(.top, 8) .padding(.top, 8)
} }
.padding() .padding()