You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
3.5 KiB
3.5 KiB
$updateSettings to $saveSettings Consolidation Plan
Overview
Consolidate $updateSettings
method into $saveSettings
to eliminate code duplication and improve maintainability. The $updateSettings
method is currently just a thin wrapper around $saveSettings
and $saveUserSettings
, providing no additional functionality.
Current State Analysis
Current Implementation
// Current $updateSettings - just a wrapper
async $updateSettings(changes: Partial<Settings>, did?: string): Promise<boolean> {
try {
if (did) {
return await this.$saveUserSettings(did, changes);
} else {
return await this.$saveSettings(changes);
}
} catch (error) {
logger.error("[PlatformServiceMixin] Error updating settings:", error);
return false;
}
}
Usage Statistics
- $updateSettings: 42 references across codebase
- $saveSettings: 38 references across codebase
- $saveUserSettings: 12 references across codebase
Migration Strategy
Phase 1: Documentation and Planning ✅
- Document current usage patterns
- Identify all call sites
- Create migration plan
Phase 2: Implementation
- Update
$saveSettings
to accept optionaldid
parameter - Add error handling to
$saveSettings
(currently missing) - Deprecate
$updateSettings
with migration notice - Update all call sites to use
$saveSettings
directly
Phase 3: Cleanup
- Remove
$updateSettings
method - Update documentation
- Update tests
Implementation Details
Enhanced $saveSettings Method
async $saveSettings(changes: Partial<Settings>, did?: string): Promise<boolean> {
try {
// Convert settings for database storage
const convertedChanges = this._convertSettingsForStorage(changes);
if (did) {
// User-specific settings
return await this.$saveUserSettings(did, convertedChanges);
} else {
// Default settings
return await this.$saveSettings(convertedChanges);
}
} catch (error) {
logger.error("[PlatformServiceMixin] Error saving settings:", error);
return false;
}
}
Migration Benefits
- Reduced Code Duplication: Single method handles both use cases
- Improved Maintainability: One place to fix issues
- Consistent Error Handling: Unified error handling approach
- Better Type Safety: Single method signature to maintain
Risk Assessment
- Low Risk:
$updateSettings
is just a wrapper, no complex logic - Backward Compatible: Can maintain both methods during transition
- Testable: Existing tests can be updated incrementally
Call Site Migration Examples
Before (using $updateSettings)
await this.$updateSettings({ searchBoxes: [newSearchBox] });
await this.$updateSettings({ filterFeedByNearby: false }, userDid);
After (using $saveSettings)
await this.$saveSettings({ searchBoxes: [newSearchBox] });
await this.$saveSettings({ filterFeedByNearby: false }, userDid);
Testing Strategy
- Unit Tests: Update existing tests to use
$saveSettings
- Integration Tests: Verify both default and user-specific settings work
- Migration Tests: Ensure searchBoxes conversion still works
- Performance Tests: Verify no performance regression
Timeline
- Phase 1: ✅ Complete
- Phase 2: 1-2 days
- Phase 3: 1 day
- Total: 2-3 days
Success Criteria
- All existing functionality preserved
- No performance regression
- All tests passing
- Reduced code duplication
- Improved maintainability