Skip to content

Fix macOS goal editing updates #5758

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kodjima33 merged 1 commit into from
Mar 17, 2026
Merged

Fix macOS goal editing updates #5758

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 28 additions & 1 deletion desktop/Desktop/Sources/APIClient.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2030,7 +2030,34 @@ extension APIClient {
throw APIError.httpError(statusCode: (response as? HTTPURLResponse)?.statusCode ?? 0)
}

return try decoder.decode(Goal.self, from: data)
let goal = try decoder.decode(Goal.self, from: data)
goalsCache = nil
return goal
}

/// Updates editable goal fields.
func updateGoal(goalId: String, title: String, currentValue: Double, targetValue: Double) async throws -> Goal {
struct UpdateGoalRequest: Encodable {
let title: String
let currentValue: Double
let targetValue: Double

enum CodingKeys: String, CodingKey {
case title
case currentValue = "current_value"
case targetValue = "target_value"
}
}

let request = UpdateGoalRequest(
title: title,
currentValue: currentValue,
targetValue: targetValue
)

let goal: Goal = try await patch("v1/goals/\(goalId)", body: request)
goalsCache = nil
return goal
}

/// Gets completed goals for history
Expand Down
4 changes: 3 additions & 1 deletion desktop/Desktop/Sources/MainWindow/Components/GoalsWidget.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import SwiftUI
struct GoalsWidget: View {
let goals: [Goal]
let onCreateGoal: (String, Double, Double) -> Void // (title, currentValue, targetValue)
let onUpdateGoal: (Goal, String, Double, Double) -> Void
let onUpdateProgress: (Goal, Double) -> Void
let onDeleteGoal: (Goal) -> Void

Expand Down Expand Up @@ -134,7 +135,7 @@ struct GoalsWidget: View {
GoalEditSheet(
goal: goal,
onSave: { title, current, target in
onUpdateProgress(goal, current)
onUpdateGoal(goal, title, current, target)
},
onDelete: {
onDeleteGoal(goal)
Expand Down Expand Up @@ -963,6 +964,7 @@ private struct GoalHeaderButton: View {
GoalsWidget(
goals: [],
onCreateGoal: { _, _, _ in },
onUpdateGoal: { _, _, _, _ in },
onUpdateProgress: { _, _ in },
onDeleteGoal: { _ in }
)
Expand Down
30 changes: 30 additions & 0 deletions desktop/Desktop/Sources/MainWindow/Pages/DashboardPage.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,26 @@ class DashboardViewModel: ObservableObject {
}
}

func updateGoal(_ goal: Goal, title: String, currentValue: Double, targetValue: Double) async {
log("Goals: Updating goal '\(goal.title)' -> title='\(title)', current=\(currentValue), target=\(targetValue)")

do {
let updated = try await APIClient.shared.updateGoal(
goalId: goal.id,
title: title,
currentValue: currentValue,
targetValue: targetValue
)

_ = try? await GoalStorage.shared.syncServerGoal(updated)
goals = try await GoalStorage.shared.getLocalGoals()
Comment on lines +172 to +173
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot Mar 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Stale local data when syncServerGoal fails silently

syncServerGoal is called with try?, so if the local SQLite write fails, the subsequent getLocalGoals() will return the old values — making the UI appear unchanged even though the API update succeeded. The user sees no error and has to re-open the goals list to observe the change is gone.

The same pattern exists in createGoal. If a silent local-sync failure is acceptable here, consider at least applying the server response directly to the in-memory array as a fallback:

_ = try? await GoalStorage.shared.syncServerGoal(updated)
// Refresh from local; fall back to injecting the API response directly
if let refreshed = try? await GoalStorage.shared.getLocalGoals(), !refreshed.isEmpty {
    goals = refreshed
} else if let index = goals.firstIndex(where: { $0.id == updated.id }) {
    goals[index] = updated
}

log("Goals: Updated goal '\(updated.title)' confirmed by API")
} catch {
logError("Failed to update goal", error: error)
goals = (try? await GoalStorage.shared.getLocalGoals()) ?? goals
}
}
Comment on lines +161 to +179
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot Mar 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 No optimistic update before API call

updateGoalProgress immediately updates both goals[index].currentValue and local SQLite before making the API call, giving instant UI feedback. The new updateGoal waits for the full API round-trip before refreshing the UI, which can cause a noticeable lag when editing a goal's title or target.

Consider applying a similar optimistic strategy — update the in-memory goals array immediately, then reconcile with the API response:

func updateGoal(_ goal: Goal, title: String, currentValue: Double, targetValue: Double) async {
    log("Goals: Updating goal '\(goal.title)' -> title='\(title)', current=\(currentValue), target=\(targetValue)")

    // Optimistic update
    if let index = goals.firstIndex(where: { $0.id == goal.id }) {
        goals[index].title = title
        goals[index].currentValue = currentValue
        goals[index].targetValue = targetValue
    }

    do {
        let updated = try await APIClient.shared.updateGoal(
            goalId: goal.id,
            title: title,
            currentValue: currentValue,
            targetValue: targetValue
        )

        _ = try? await GoalStorage.shared.syncServerGoal(updated)
        goals = try await GoalStorage.shared.getLocalGoals()
        log("Goals: Updated goal '\(updated.title)' confirmed by API")
    } catch {
        logError("Failed to update goal", error: error)
        goals = (try? await GoalStorage.shared.getLocalGoals()) ?? goals
    }
}

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!


func deleteGoal(_ goal: Goal) async {
do {
// Soft-delete locally first for instant UI update
Expand Down Expand Up @@ -230,6 +250,16 @@ struct DashboardPage: View {
)
}
},
onUpdateGoal: { goal, title, current, target in
Task {
await viewModel.updateGoal(
goal,
title: title,
currentValue: current,
targetValue: target
)
}
},
onUpdateProgress: { goal, value in
Task {
await viewModel.updateGoalProgress(goal, currentValue: value)
Expand Down