KM-13719 Commit some preferences changes as soon as they are modified

[kp-diego-trevisan]

Summary

• Commits showReconnectNotifications, debugLogging and shareServiceQualityData as soon as they are toggled
• Fix a wrong translation on automations

[claude]

Claude finished @kp-diego-trevisan's task —— View job

---

PR Review for KM-13719

Reviewing changes for immediate preference commits and translation fixes.

Review Checklist

• Read and analyze main Swift code changes
• Review translation file changes
• Check for code quality and best practices
• Analyze potential bugs and issues
• Review memory management and retain cycles
• Check threading and concurrency concerns
• Evaluate performance considerations
• Assess security implications
• Review test coverage considerations
• Provide comprehensive feedback and recommendations

---

🔍 Code Review Summary

✅ Positive Aspects

1. Clear Intent: The immediate commit behavior for debug settings is well-documented with inline comments
2. Consistent Implementation: All three properties (showReconnectNotifications, debugLogging, shareServiceQualityData) follow the same pattern
3. Translation Improvements: Localization fixes improve user experience across 13 languages
4. Backwards Compatibility: Changes maintain existing API surface

⚠️ Areas of Concern

Threading & Concurrency Issues

• High Priority: The immediate setters in Editable class (Client+Preferences.swift:607-608, 668-669, 677-678) write directly to target without thread safety
• Risk: Potential race conditions if preferences are modified from multiple threads
• Recommendation: Add proper synchronization or ensure these setters are only called on main queue

Memory Management

• Medium Priority: The target property in Editable class (Client+Preferences.swift:527) is a weak-style reference but not explicitly marked as weak
• Risk: Could create retain cycles if Client.Preferences holds strong reference to Editable
• Recommendation: Consider marking target as weak to prevent potential memory leaks

Performance Considerations

• Medium Priority: Immediate database commits on every toggle (Client+Preferences.swift:180, 484, 494)
• Risk: Frequent disk I/O could impact UI responsiveness during rapid setting changes
• Recommendation: Consider debouncing or batching rapid successive changes

🐛 Potential Bugs

1. Inconsistent Default Values:

   • showReconnectNotifications getter uses ?? defaults.showReconnectNotifications (Client+Preferences.swift:177)
   • But Editable getter uses ?? true hardcoded (Client+Preferences.swift:604)
   • Risk: Inconsistent behavior between main preferences and editable copy

2. Missing Error Handling: Database writes in setters don't handle potential failures

   • Could lead to silent failures in preference persistence

🔒 Security Assessment

✅ No Security Concerns Identified:

• Changes only affect local preference storage
• No sensitive data exposure
• Proper encapsulation maintained

📋 Test Coverage Recommendations

Missing Test Areas:

1. Thread safety of immediate commit properties
2. Behavior during rapid successive toggles
3. Consistency between main preferences and editable defaults
4. Database failure scenarios during immediate commits

🛠️ Specific Recommendations

High Priority Fixes:

// In Editable class, add thread safety:
public var showReconnectNotifications: Bool {
    get {
        return target?.showReconnectNotifications ?? true
    }
    set {
        DispatchQueue.main.async {
            self.target?.showReconnectNotifications = newValue
        }
    }
}

Medium Priority Improvements:

1. Consistent Defaults: Use defaults.showReconnectNotifications instead of hardcoded true
2. Error Handling: Add logging/handling for database write failures
3. Memory Safety: Consider making target weak and handling nil cases

📊 Overall Assessment

Score: 6.5/10

Strengths: Clear implementation, good documentation, meaningful feature
Weaknesses: Thread safety concerns, potential performance impact, inconsistent defaults

The PR successfully implements the requested feature but needs attention to thread safety and consistency issues before merging.

---