docs: enhance activeDid migration plan with implementation details

- Add master settings functions implementation strategy - Correct IdentitySection.vue analysis (prop-based, no changes required) - Simplify ContactAmountsView.vue (phased-out method, separate refactoring) - Add new getMasterSettings() function with active_identity integration - Include helper methods _getSettingsWithoutActiveDid() and _getActiveIdentity() - Enhance evidence section with master settings architecture support - Update risk assessment for phased-out methods - Clean up migration timeline formatting This commit focuses the migration plan on components requiring immediate active_identity table changes, separating concerns from broader API refactoring.
2025-08-28 12:32:39 +00:00
parent 649786ae01
commit acbc276ef6
1 changed files with 227 additions and 103 deletions
--- a/doc/activeDid-migration-plan.md
+++ b/doc/activeDid-migration-plan.md
@@ -124,6 +124,134 @@ async $accountSettings(did?: string, defaults: Settings = {}): Promise<Settings>

  return { ...settings, activeDid: activeIdentity.activeDid };
 }
+
+// New method to get settings without activeDid
+async _getSettingsWithoutActiveDid(): Promise<Settings> {
+  const result = await this.$dbQuery(
+    "SELECT id, accountDid, apiServer, filterFeedByNearby, filterFeedByVisible, " +
+    "finishedOnboarding, firstName, hideRegisterPromptOnNewContact, isRegistered, " +
+    "lastName, lastAckedOfferToUserJwtId, lastAckedOfferToUserProjectsJwtId, " +
+    "lastNotifiedClaimId, lastViewedClaimId, notifyingNewActivityTime, " +
+    "notifyingReminderMessage, notifyingReminderTime, partnerApiServer, " +
+    "passkeyExpirationMinutes, profileImageUrl, searchBoxes, showContactGivesInline, " +
+    "showGeneralAdvanced, showShortcutBvc, vapid, warnIfProdServer, warnIfTestServer, " +
+    "webPushServer FROM settings WHERE id = ?",
+    [MASTER_SETTINGS_KEY]
+  );
+
+  if (!result?.values?.length) {
+    return DEFAULT_SETTINGS;
+  }
+
+  return this._mapColumnsToValues(result.columns, result.values)[0] as Settings;
+}
+
+// New method to get active identity
+async _getActiveIdentity(): Promise<{ activeDid: string | null }> {
+  const result = await this.$dbQuery(
+    "SELECT activeDid FROM active_identity WHERE id = 1"
+  );
+
+  if (!result?.values?.length) {
+    return { activeDid: null };
+  }
+
+  return { activeDid: result.values[0][0] as string };
+}
+```
+
+### **Master Settings Functions Implementation Strategy**
+
+#### **1. Update `retrieveSettingsForDefaultAccount()`**
+
+```typescript
+// Current implementation in src/db/databaseUtil.ts:148
+export async function retrieveSettingsForDefaultAccount(): Promise<Settings> {
+  const platform = PlatformServiceFactory.getInstance();
+  const sql = "SELECT * FROM settings WHERE id = ?";
+  const result = await platform.dbQuery(sql, [MASTER_SETTINGS_KEY]);
+  // ... rest of implementation
+}
+
+// Updated implementation
+export async function retrieveSettingsForDefaultAccount(): Promise<Settings> {
+  const platform = PlatformServiceFactory.getInstance();
+
+  // Get settings without activeDid
+  const sql = "SELECT id, accountDid, apiServer, filterFeedByNearby, filterFeedByVisible, " +
+              "finishedOnboarding, firstName, hideRegisterPromptOnNewContact, isRegistered, " +
+              "lastName, lastAckedOfferToUserJwtId, lastAckedOfferToUserProjectsJwtId, " +
+              "lastNotifiedClaimId, lastViewedClaimId, notifyingNewActivityTime, " +
+              "notifyingReminderMessage, notifyingReminderTime, partnerApiServer, " +
+              "passkeyExpirationMinutes, profileImageUrl, searchBoxes, showContactGivesInline, " +
+              "showGeneralAdvanced, showShortcutBvc, vapid, warnIfProdServer, warnIfTestServer, " +
+              "webPushServer FROM settings WHERE id = ?";
+
+  const result = await platform.dbQuery(sql, [MASTER_SETTINGS_KEY]);
+
+  if (!result) {
+    return DEFAULT_SETTINGS;
+  } else {
+    const settings = mapColumnsToValues(result.columns, result.values)[0] as Settings;
+
+    // Handle JSON parsing
+    if (settings.searchBoxes) {
+      settings.searchBoxes = JSON.parse(settings.searchBoxes);
+    }
+
+    // Get activeDid from separate table
+    const activeIdentityResult = await platform.dbQuery(
+      "SELECT activeDid FROM active_identity WHERE id = 1"
+    );
+
+    if (activeIdentityResult?.values?.length) {
+      settings.activeDid = activeIdentityResult.values[0][0] as string;
+    }
+
+    return settings;
+  }
+}
+```
+
+
+
+#### **2. Update `$getMergedSettings()` Method**
+
+```typescript
+// Current implementation in PlatformServiceMixin.ts:485
+async $getMergedSettings(defaultKey: string, accountDid?: string, defaultFallback: Settings = {}): Promise<Settings> {
+  // Get default settings
+  const defaultSettings = await this.$getSettings(defaultKey, defaultFallback);
+  // ... rest of implementation
+}
+
+// Updated implementation
+async $getMergedSettings(defaultKey: string, accountDid?: string, defaultFallback: Settings = {}): Promise<Settings> {
+  try {
+    // Get default settings (now without activeDid)
+    const defaultSettings = await this.$getSettings(defaultKey, defaultFallback);
+
+    // If no account DID, return defaults with activeDid from separate table
+    if (!accountDid) {
+      if (defaultSettings) {
+        // Get activeDid from separate table
+        const activeIdentityResult = await this.$dbQuery(
+          "SELECT activeDid FROM active_identity WHERE id = 1"
+        );
+
+        if (activeIdentityResult?.values?.length) {
+          defaultSettings.activeDid = activeIdentityResult.values[0][0] as string;
+        }
+      }
+      return defaultSettings || defaultFallback;
+    }
+
+    // ... rest of existing implementation for account-specific settings
+  } catch (error) {
+    logger.error(`[Settings Trace] ❌ Failed to get merged settings:`, { defaultKey, accountDid, error });
+    return defaultFallback;
+  }
+}
 ```

 ## What Works (Evidence)
@@ -143,6 +271,11 @@ async $accountSettings(did?: string, defaults: Settings = {}): Promise<Settings>
  - **Evidence**: `src/db-sql/migration.ts:31` - migration system in place
  - **Verify at**: Existing migration scripts and database versioning

+- ✅ **Master settings functions architecture** supports migration
+  - **Time**: 2025-01-27T18:30Z
+  - **Evidence**: Functions use explicit field selection, not `SELECT *`
+  - **Verify at**: `src/db/databaseUtil.ts:148` and `src/utils/PlatformServiceMixin.ts:442`
+
 ## What Doesn't (Evidence & Hypotheses)

 - ❌ **No separate active_identity table** exists
@@ -217,10 +350,10 @@ async $accountSettings(did?: string, defaults: Settings = {}): Promise<Settings>

 ### **High Impact Components**

-1. **`IdentitySection.vue`** - Direct dependency on `activeDid`
-   - **Current**: Uses `activeDid` from component data
-   - **Impact**: Will need to update data binding and refresh logic
-   - **Risk**: **HIGH** - Core identity display component
+1. **`IdentitySection.vue`** - Receives `activeDid` as prop
+   - **Current**: Uses `activeDid` from parent component via prop
+   - **Impact**: **NO CHANGES REQUIRED** - Parent component handles migration
+   - **Risk**: **LOW** - No direct database access

   **Current Implementation:**

@@ -235,12 +368,8 @@ async $accountSettings(did?: string, defaults: Settings = {}): Promise<Settings>

   <script lang="ts">
   export default class IdentitySection extends Vue {
-     activeDid = ""; // Direct component data
-
-     async mounted() {
-       const settings = await this.$accountSettings();
-       this.activeDid = settings.activeDid || "";
-     }
+     @Prop({ required: true }) activeDid!: string; // Received as prop from parent
+     // ... other props and methods
   }
   </script>
   ```
@@ -250,24 +379,16 @@ async $accountSettings(did?: string, defaults: Settings = {}): Promise<Settings>
   ```vue
   <script lang="ts">
   export default class IdentitySection extends Vue {
-     activeDid = ""; // Still needed for template binding
-
-     async mounted() {
-       // This will automatically work if $accountSettings() is updated
-       const settings = await this.$accountSettings();
-       this.activeDid = settings.activeDid || "";
-     }
-
-     // Add watcher for activeDid changes
-     async onActiveDidChanged(newDid: string | null) {
-       if (newDid) {
-         this.activeDid = newDid;
-       }
-     }
+     @Prop({ required: true }) activeDid!: string; // No changes needed
+     // ... other props and methods
   }
   </script>
   ```

+   **Key Insight**: This component requires **zero changes** since it receives
+   `activeDid` as a prop. The parent component that provides this prop will
+   handle the migration automatically through the API layer updates.
+
 2. **`DIDView.vue`** - Heavy activeDid usage
   - **Current**: Initializes `activeDid` in `mounted()` lifecycle
   - **Impact**: Must update initialization logic to use new table
@@ -524,81 +645,15 @@ async $accountSettings(did?: string, defaults: Settings = {}): Promise<Settings>
   **Key Insight**: This component requires zero changes since it already
   uses the proper API method. It's the lowest risk component.

-3. **`ContactAmountsView.vue`** - Direct settings access
-   - **Current**: Accesses `activeDid` directly from settings
-   - **Impact**: Must update to use new API methods
-   - **Risk**: **MEDIUM** - Financial display component
+3. **`ContactAmountsView.vue`** - Uses phased-out method
+   - **Current**: Uses `$getSettings(MASTER_SETTINGS_KEY)` (being phased out)
+   - **Impact**: **NO CHANGES REQUIRED** - Will be updated when migrating to `getMasterSettings`
+   - **Risk**: **LOW** - Part of planned refactoring, not migration-specific

-   **Current Implementation:**
-
-   ```vue
-   <script lang="ts">
-   export default class ContactAmountsView extends Vue {
-     activeDid = ""; // Component data
-
-     async created() {
-       try {
-         const contactDid = this.$route.query["contactDid"] as string;
-         const contact = await this.$getContact(contactDid);
-         this.contact = contact;
-
-         // Direct access to settings table - this needs to change
-         const settings = await this.$getSettings(MASTER_SETTINGS_KEY);
-         this.activeDid = settings?.activeDid || "";
-
-         if (this.activeDid && this.contact) {
-           this.loadGives(this.activeDid, this.contact);
-         }
-       } catch (err: any) {
-         // ... error handling
-       }
-     }
-
-     async loadGives(activeDid: string, contact: Contact) {
-       // Uses activeDid parameter
-       // ... implementation
-     }
-   }
-   </script>
-   ```
-
-   **Required Changes:**
-
-   ```vue
-   <script lang="ts">
-   export default class ContactAmountsView extends Vue {
-     activeDid = ""; // Component data still needed
-
-     async created() {
-       try {
-         const contactDid = this.$route.query["contactDid"] as string;
-         const contact = await this.$getContact(contactDid);
-         this.contact = contact;
-
-         // Use the proper API method instead of direct settings access
-         const settings = await this.$accountSettings();
-         this.activeDid = settings.activeDid || "";
-
-         if (this.activeDid && this.contact) {
-           this.loadGives(this.activeDid, this.contact);
-         }
-       } catch (err: any) {
-         // ... error handling
-       }
-     }
-
-     // No changes needed - method will work automatically
-     async loadGives(activeDid: string, contact: Contact) {
-       // Uses activeDid parameter
-       // ... implementation
-     }
-   }
-   </script>
-   ```
-
-   **Key Insight**: This component needs one specific change - replace
-   `$getSettings(MASTER_SETTINGS_KEY)` with `$accountSettings()`. The
-   rest of the component works automatically.
+   **Note**: This component will be updated as part of the broader refactoring to
+   replace `$getSettings(MASTER_SETTINGS_KEY)` with `getMasterSettings()`, which
+   is separate from the activeDid migration. The migration plan focuses only on
+   components that require immediate changes for the active_identity table.

 ### **Service Layer Impact**

@@ -629,6 +684,74 @@ async $accountSettings(did?: string, defaults: Settings = {}): Promise<Settings>
   - **New**: Database persistence required
   - **Impact**: Identity switching logic

+### **Master Settings Functions Impact**
+
+1. **`retrieveSettingsForDefaultAccount()` function**
+   - **Current**: Returns settings with `activeDid` field from master settings
+   - **New**: Returns settings without `activeDid` field
+   - **Impact**: **HIGH** - Used by migration scripts and core database utilities
+   - **Location**: `src/db/databaseUtil.ts:148`
+
+2. **`$getMergedSettings()` method**
+   - **Current**: Merges default and account settings, includes `activeDid` from defaults
+   - **New**: Merges settings without `activeDid`, adds from `active_identity` table
+   - **Impact**: **HIGH** - Core method used by `$accountSettings()`
+   - **Location**: `src/utils/PlatformServiceMixin.ts:485`
+
+**Note**: `$getSettings(MASTER_SETTINGS_KEY)` is being phased out in favor of `getMasterSettings`,
+so it doesn't require updates for this migration.
+
+### **New `getMasterSettings()` Function**
+
+Since we're phasing out `$getSettings(MASTER_SETTINGS_KEY)`, this migration
+provides an opportunity to implement the new `getMasterSettings()` function
+that will handle the active_identity table integration from the start:
+
+```typescript
+// New getMasterSettings function to replace phased-out $getSettings
+async getMasterSettings(): Promise<Settings> {
+  try {
+    // Get master settings without activeDid
+    const result = await this.$dbQuery(
+      "SELECT id, accountDid, apiServer, filterFeedByNearby, filterFeedByVisible, " +
+      "finishedOnboarding, firstName, hideRegisterPromptOnNewContact, isRegistered, " +
+      "lastName, lastAckedOfferToUserJwtId, lastAckedOfferToUserProjectsJwtId, " +
+      "lastNotifiedClaimId, lastViewedClaimId, notifyingNewActivityTime, " +
+      "notifyingReminderMessage, notifyingReminderTime, partnerApiServer, " +
+      "passkeyExpirationMinutes, profileImageUrl, searchBoxes, showContactGivesInline, " +
+      "showGeneralAdvanced, showShortcutBvc, vapid, warnIfProdServer, warnIfTestServer, " +
+      "webPushServer FROM settings WHERE id = ?",
+      [MASTER_SETTINGS_KEY]
+    );
+
+    if (!result?.values?.length) {
+      return DEFAULT_SETTINGS;
+    }
+
+    const settings = this._mapColumnsToValues(result.columns, result.values)[0] as Settings;
+
+    // Handle JSON field parsing
+    if (settings.searchBoxes) {
+      settings.searchBoxes = this._parseJsonField(settings.searchBoxes, []);
+    }
+
+    // Get activeDid from separate table
+    const activeIdentityResult = await this.$dbQuery(
+      "SELECT activeDid FROM active_identity WHERE id = 1"
+    );
+
+    if (activeIdentityResult?.values?.length) {
+      settings.activeDid = activeIdentityResult.values[0][0] as string;
+    }
+
+    return settings;
+  } catch (error) {
+    logger.error(`[Settings Trace] ❌ Failed to get master settings:`, { error });
+    return DEFAULT_SETTINGS;
+  }
+}
+```
+
 ### **Testing Impact**

 1. **Unit Tests**
@@ -663,14 +786,15 @@ async $accountSettings(did?: string, defaults: Settings = {}): Promise<Settings>
 - **Critical Risk**: PlatformServiceMixin, Platform Services
 - **High Risk**: Identity-related components, views using `$accountSettings()`
 - **Medium Risk**: Components with direct settings access, identity management
- **Low Risk**: Components using only basic settings, utility components
+- **Low Risk**: Components using only basic settings, utility components,
+prop-based components, components using phased-out methods

 ### **Migration Timeline Impact**

- **Phase 1**: Schema Creation (1-2 days) - No component impact
- **Phase 2**: Data Migration (1 day) - No component impact
- **Phase 3**: API Updates (3-5 days) - All components affected
- **Phase 4**: Cleanup (1-2 days) - No component impact
+- **Phase 1**: Schema Creation  - No component impact
+- **Phase 2**: Data Migration  - No component impact
+- **Phase 3**: API Updates  - All components affected
+- **Phase 4**: Cleanup - No component impact

 ### **Update Priority Order**