Browse Source
- Add _convertSettingsForStorage helper method to handle Settings → SettingsWithJsonStrings conversion - Fix $saveSettings and $saveUserSettings to properly convert searchBoxes arrays to JSON strings before database storage - Update SearchAreaView.vue to use array format instead of manual JSON.stringify conversion - Add comprehensive test UI in PlatformServiceMixinTest.vue with visual feedback and clear demonstration of conversion process - Document migration strategy for consolidating $updateSettings into $saveSettings to reduce code duplication - Add deprecation notices to $updateSettings method with clear migration guidance The fix ensures that searchBoxes arrays are properly converted to JSON strings before database storage, preventing data corruption and maintaining consistency with the SettingsWithJsonStrings type definition. The enhanced test interface provides clear visualization of the conversion process and database storage format. Migration Strategy: - $saveSettings: ✅ KEEP (will be primary method after consolidation) - $updateSettings: ⚠️ DEPRECATED (will be removed in favor of $saveSettings) - Future: Consolidate to single $saveSettings(changes, did?) method Files changed: - src/utils/PlatformServiceMixin.ts: Add conversion helper, fix save methods, add deprecation notices - src/views/SearchAreaView.vue: Remove manual JSON conversion - src/test/PlatformServiceMixinTest.vue: Add comprehensive test UI with highlighting - docs/migration-templates/updateSettings-consolidation-plan.md: Document future consolidation strategypull/151/head
4 changed files with 359 additions and 13 deletions
@ -0,0 +1,113 @@ |
|||
# $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 |
|||
```typescript |
|||
// 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 ✅ |
|||
- [x] Document current usage patterns |
|||
- [x] Identify all call sites |
|||
- [x] Create migration plan |
|||
|
|||
### Phase 2: Implementation |
|||
- [ ] Update `$saveSettings` to accept optional `did` 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 |
|||
```typescript |
|||
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 |
|||
1. **Reduced Code Duplication**: Single method handles both use cases |
|||
2. **Improved Maintainability**: One place to fix issues |
|||
3. **Consistent Error Handling**: Unified error handling approach |
|||
4. **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) |
|||
```typescript |
|||
await this.$updateSettings({ searchBoxes: [newSearchBox] }); |
|||
await this.$updateSettings({ filterFeedByNearby: false }, userDid); |
|||
``` |
|||
|
|||
### After (using $saveSettings) |
|||
```typescript |
|||
await this.$saveSettings({ searchBoxes: [newSearchBox] }); |
|||
await this.$saveSettings({ filterFeedByNearby: false }, userDid); |
|||
``` |
|||
|
|||
## Testing Strategy |
|||
1. **Unit Tests**: Update existing tests to use `$saveSettings` |
|||
2. **Integration Tests**: Verify both default and user-specific settings work |
|||
3. **Migration Tests**: Ensure searchBoxes conversion still works |
|||
4. **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 |
Loading…
Reference in new issue