Reboot of the ActiveDID migration #188
Open
anomalist
wants to merge 0 commits from
active_did_redux into master
pull from: active_did_redux
merge into: :master
:master
:retire-legacy-notification
:notify-api_endpoint-query
:notify-api-sms
:notify-api
:giftopia-app-icon
:fix/ios-share-target-reliability
:thanks-button-rework
:gifted-details-footer
:16kb-pages
:notify-api_android
:edit-proj-parent
:daily-notification-plugin-integration
:2026-01-01-tweaks
:no-locks
:web-share-target-native-implementation
:homeview-gift-recording-improvements
:accountview-contact-management-bundling
:gifted-dialog-recipient-fix
:entitygrid-infinite-scroll-improvements
:meeting-project-dialog
:refactor-initialize
:integrate-notification-plugin
:project-representative-dialog
:entity-selection-list-component
:bulk-members-dialog-refactor
:contact-path
:entity-selection-list-component-infinite-scroll
:meeting-members-admission-dialog
:address-duplicates
:meeting-members-admission-dialog-refactor
:android-file-save
:meeting-members-admission-improvements
:emojis
:meeting-members-set-visibility
:ios-disable-zoom
:view-headings-refresh
:star-projects2
:remove-cannot-upload-images-notification
:star-projects
:notification-system
:load-build-mode-env-file
:notify-initialization-fix
:new-activity-mark-read
:active_did_redux
:ios-qr-code-copy
:master-patch
:registration-prompt-parity
:seed-phrase-backup-prompt
:claimview-fullfills-offer
:wip_new_notifications
:account-import-duplicate-prevention
:electron-copy-paste-keyboard-shortcuts
:switching-identities-change-name
:playwright-test-00-fix
:profile_include_location
:electron-build-config-overwrite
:projectview-hide-offer-link-unregistered
:activedid_migration
:build-web-serve-test
:didview-invalid-did-handling
:electron-build-capacitor-config
:contact-gifting-current-user
:android-safe-area-insets
:deep-link-views-safe-area-inset
:dialog-notification-z-index
:ios-contact-copy
:onboard-alert-component
:dialog-styles-unified
:units-mocking
:performance-optimizations-testing
:playwright-test-60-fix
:notification-section
:fix-deep-link
:platformservicemixin-interface-consolidation
:nearby-filter
:replace-iconrenderer
:imagemagick-anrdoid
:ask-for-contacts-export
:offer-validation-logic
:playwright-test-updates
:logger-level
:remove-image-cache
:claim-view-error-handling
:build-improvement
:get-get-hash
:logging-upgrade
:notification-line-wrapping
:build-dev-to-dist
:fix-contact-import-export
:web-serve-fix
:deep-link
:web-tests
:build-with-env
:onboarding-dialog-fix
:streamline-attempt
:matthew-scratch-2025-06-28
:gifting-periphery-improvements
:gifting-ui-2025-05
:migrate-dexie-to-sqlite
:deep-links-android-update
:android-15-check
:capacitor-local-save
:master-settings-upgrade
:contacts-view-fixes
:ui-fixes-2025-06-w2
:home-icon-enhancements
:search-map-fix
:sql-absurd-sql-further
:sql-absurd-sql
:new-storage
:sql-wa-sqlite
:trent-tweaks
:qrcode-capacitor
:cross-platform-factory-redux
:build-ios
:ai-context
:cross-platform-factory
:registration-gate
:db-backup-cross-platform
:eye-slash
:homeview-cleanup-2025-03
:fix-service-worker
:main
:app_id_fix
:electron_fix_20250317
:homeview-refresh-2025-02
:deep_linking
:ui-fixes-2025-03
:side_step
:split_build_process
:d9085ced6df7dc7bdcd899959cea6489cab7f8b8
:v-onboarding-2024-04
:nostr
:playwright-pwa-install-test
:offer-edit
:passkey-cache
:passkey
:profile-pic
:notify-time
:ui-fixes-2024-03
:photo-reverse
:starred-projects
:vite-version
:design-tweaks-2023-12
:sw-cleanup
:home-view-notification-improvements
:friend-tech-inspired-pwa-dialog
:notification-request-permission-dialog
:plan-loc
:project-gives
:tweaks
:simple-signer
:experimental_plugin
:tmp
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
ActiveDid Migration Plan - Separate Table Architecture
Author: Matthew Raymer
Date: 2025-01-27T18:30Z
Status: 🎯 PLANNING - Active migration planning phase
Objective
Move the
activeDidfield from thesettingstable to a dedicatedactive_identitytable to improve database architecture and separateidentity selection from user preferences.
Result
This document serves as the comprehensive planning and implementation
guide for the ActiveDid migration.
Use/Run
Reference this document during implementation to ensure all migration
steps are followed correctly and all stakeholders are aligned on the
approach.
Context & Scope
Environment & Preconditions
Architecture / Process Overview
The migration follows a phased approach to minimize risk and ensure
data integrity:
Interfaces & Contracts
Database Schema Changes
settingsactiveDid TEXTactive_identityactiveDid TEXTAPI Contract Changes
$accountSettings()$saveSettings()$updateActiveDid()Repro: End-to-End Procedure
Phase 1: Schema Creation
Phase 2: Data Migration
Phase 3: API Updates
What Works (Evidence)
✅ Current activeDid storage in settings table
src/db/tables/settings.ts:25- activeDid field exists✅ PlatformServiceMixin integration with activeDid
src/utils/PlatformServiceMixin.ts:108- activeDid tracking✅ Database migration infrastructure exists
src/db-sql/migration.ts:31- migration system in placeWhat Doesn't (Evidence & Hypotheses)
❌ No separate active_identity table exists
❌ Platform services hardcoded to settings table
src/services/platforms/*.ts- direct settings table accessRisks, Limits, Assumptions
Next Steps
References
Competence Hooks
user preferences, improves database normalization, enables future
identity management features
testing rollback scenarios, missing data validation during migration
planning
phase is necessary
Collaboration Hooks
Assumptions & Limits
Component & View Impact Analysis
High Impact Components
IdentitySection.vue- Direct dependency onactiveDidactiveDidfrom component dataCurrent Implementation:
Required Changes:
DIDView.vue- Heavy activeDid usageactiveDidinmounted()lifecycleCurrent Implementation:
Required Changes:
HomeView.vue- ActiveDid change detectiononActiveDidChanged()watcher methodCurrent Implementation:
Required Changes:
Key Insight: HomeView will require minimal changes since it already uses
the
$accountSettings()method, which will be updated to handle the newtable structure transparently.
Medium Impact Components
InviteOneAcceptView.vue- Identity fallback logicactiveDidexistsClaimView.vue- Settings retrievalactiveDidfrom$accountSettings()ContactAmountsView.vue- Direct settings accessactiveDiddirectly from settingsService Layer Impact
WebPlatformService.tsactive_identitytable queriesCapacitorPlatformService.tsPlatformServiceMixin.ts$accountSettings(),$saveSettings()API Contract Changes
$saveSettings()methodsettings.activeDidactive_identity.activeDid$updateActiveDid()methodTesting Impact
Unit Tests
Integration Tests
Platform Tests
Performance Impact
Additional Table Join
Caching Considerations
Risk Assessment by Component Type
$accountSettings()Migration Timeline Impact
Update Priority Order
Deferred for depth
@@ -127,0 +130,4 @@-- Create new active_identity table with proper constraintsCREATE TABLE IF NOT EXISTS active_identity (id INTEGER PRIMARY KEY CHECK (id = 1),activeDid TEXT NOT NULL,Since the activeDid can actually be a blank ('') value, I personally would prefer that this is nullable. (Saying "NOT NULL" but using '' to represent null just feels odd... like something that's trying to bypass type-checking.)
@@ -127,0 +136,4 @@);-- Add performance indexesCREATE INDEX IF NOT EXISTS idx_active_identity_activeDid ON active_identity(activeDid);Why put an index on activeDid? It'll never be the input to a query.
@@ -127,0 +137,4 @@-- Add performance indexesCREATE INDEX IF NOT EXISTS idx_active_identity_activeDid ON active_identity(activeDid);CREATE UNIQUE INDEX IF NOT EXISTS idx_active_identity_single_record ON active_identity(id);There's no reason to index the 'id' column since it's a primary key. (Even if it weren't, we'll only ever have one entry so an index is overkill.)
Update $getActiveIdentity() method to return { activeDid: string } instead of full ActiveIdentity object. Add validation to ensure activeDid exists in accounts table and clear corrupted values. Update migration plan to reflect completed first step of API layer implementation. - Change return type from Promise<ActiveIdentity> to Promise<{ activeDid: string }> - Add account validation with automatic corruption cleanup - Simplify query to only select activeDid field - Improve error handling to return empty string instead of throwing - Update migration plan documentation with current status@@ -225,0 +225,4 @@logger.info("[GiftedDialog] Settings received:", {activeDid: this.activeDid,apiServer: this.apiServer,});I vote we make this a debug.
@@ -199,0 +268,4 @@);}},I'm not convinced this is the approach we want for data consistency. If the initial migration goes wrong then we have bigger problems (and hopefully we notify the user).
This will actually pick some other value indiscriminately, and it could do it an unexpected times (eg. in the ContactsView explicitly in this PR, or any other page that accesses the getActiveIdentity). I vote we remove it, and if we feel there are migration problems then provide more feedback for the user to choose.
I will have to give this some thought. It did seem to give the whole migration more stability -- but I feel you that it seems a bit "icky"
(Unfortunately the lines have changed, but I assume this is all about the $needsActiveIdentitySelection method.)
I don't see where this is used now. Maybe the usages have been removed, in which case this could be removed. If it's still used or in planning, I'll watch for the usages... I tried to determine the purpose (like why it's good to return true if there is no active identity but there are other settings) but I don't understand.
$needsActiveIdentitySelection is still there as a definition but it unused. I'm removing it.
It's gone! 💥
I like the removal of the MASTER_SETTINGS here! 👏 Let's hope it goes well.
I vote we make all those migrations into a single one, since they're all part of the same merge when we go to master.
@@ -551,0 +681,4 @@await this.$dbExec("UPDATE active_identity SET activeDid = '', lastUpdated = datetime('now') WHERE id = 1",);return { activeDid: "" };I'm still not sure that these checks are necessary but I won't let that hold up things since I don't see how it can hurt. But: instead of erasing the activeDid, I suggest we keep the value and create a blank entry for it in the settings table.
I tweaked this a bit but the checks are still there. I've got it wired up to a foreign key (accounts.did).
I'll try a bit of a clean-up experiment.
Looks good to me.
@@ -861,2 +1048,4 @@// eslint-disable-next-line @typescript-eslint/no-unused-varsvoid id;// eslint-disable-next-line @typescript-eslint/no-unused-varsvoid activeDidField;These voids seem unnecessary. Educate me if I'm wrong.
Really depends on your philosophy of typing. I vote they stay since they only make it explicit and don't survive into the actual build.
@@ -675,0 +814,4 @@elementWillRender:this.numNewOffersToUser + this.numNewOffersToUserProjects > 0,timestamp: new Date().toISOString(),});More logging that seems like they should be "debug" statements.
I'll be doing another pass on these before we merge into master.
@@ -31,0 +55,4 @@return true;}return false;}, { timeout: 5000 });If these timeout, do the tests fail? (I hope so. The Playwright docs don't make it obvious.)
If I remember correctly (and pretty sure I do), there is a global timeout of 45s so "yes".
Updated. Internal documentation to spell this out for you.
@@ -22,0 +24,4 @@if (process.env.NODE_ENV === 'development') {process.env.VITE_DEFAULT_ENDORSER_API_SERVER = 'http://localhost:3000';process.env.VITE_DEFAULT_PARTNER_API_SERVER = 'http://localhost:3000';}If this is necessary then it appears that something is wrong with the ".env.development" environment loading on line 9. Is there something deeper to fix here? (I ask because it's the addition of more environment logic, where it would be preferable to keep these forced settings isolated for maintainability.)
Yeah I saw this as well. I'll clean this up today.
- Fix 50-record-offer.spec.ts multiple alert button conflict * Replace generic alert selector with success-specific selector * Use getByRole('alert').filter({ hasText: 'Success' }) pattern - Fix 20-create-project.spec.ts onboarding dialog timeout * Replace unreliable div > svg.fa-xmark selector * Use established closeOnboardingAndFinish testId pattern * Add waitForFunction to ensure dialog dismissal - Fix 25-create-project-x10.spec.ts onboarding dialog timeout * Apply same onboarding dismissal pattern as other tests * Ensure consistent dialog handling across test suite These fixes use established patterns from working tests to resolve 6 failing tests caused by UI selector conflicts and timing issues.## Changes Made ### Code Cleanup - Remove unused $needsActiveIdentitySelection() method (36 lines) - Remove method signature from IPlatformServiceMixin interface - Remove method signature from Vue module declaration ### Database Consistency Fix - Change activeDid clearing from empty string ('') to NULL for consistency - Ensures proper foreign key constraint compatibility - Maintains API compatibility by still returning empty string to components ## Impact - Reduces codebase complexity by removing unused functionality - Improves database integrity with consistent NULL usage - No breaking changes to component APIs - Migration and auto-selection now handle all identity management ## Files Changed - src/utils/PlatformServiceMixin.ts: -42 lines, +1 line- Remove explicit transaction wrapping in migration service that caused "cannot start a transaction within a transaction" errors - Fix executeSet method call format to include both statement and values properties as required by Capacitor SQLite plugin - Update CapacitorPlatformService to properly handle multi-statement SQL using executeSet for migration SQL blocks - Ensure migration 004 (active_identity_management) executes atomically without nested transaction conflicts - Remove unnecessary try/catch wrapper Fixes iOS simulator migration failures where: - Migration 004 would fail with transaction errors - executeSet would fail with "Must provide a set as Array of {statement,values}" - Database initialization would fail after migration errors Tested on iOS simulator with successful migration completion and active_identity table creation with proper data migration.View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.