Update profile name immediately after editing #559
Conversation
WalkthroughThe metadata cache provider now listens to activeAccountProvider and updates the in-memory metadata cache for the active account when its metadata changes. A helper standardizes the pubkey to an npub, builds a ContactModel, upserts the cache with a timestamp, and logs results. One whitespace-only edit added a blank line in the settings UI. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant ProfileSettings
participant ActiveAccountProvider
participant MetadataCacheProvider
participant InMemoryCache
ProfileSettings->>ActiveAccountProvider: update active account metadata
Note over ActiveAccountProvider: emits new metadata
ActiveAccountProvider->>MetadataCacheProvider: notify metadata change (ref.listen)
rect rgba(200,230,255,0.6)
MetadataCacheProvider->>MetadataCacheProvider: _getStandardizedNpub(pubkey)
MetadataCacheProvider->>MetadataCacheProvider: compare with cached entry
alt changed or missing
MetadataCacheProvider->>MetadataCacheProvider: ContactModel.fromMetadata(...)
MetadataCacheProvider->>InMemoryCache: upsert {npub: contact, ts: now}
InMemoryCache-->>MetadataCacheProvider: ack
MetadataCacheProvider-->>MetadataCacheProvider: update state & log
else unchanged
MetadataCacheProvider-->>MetadataCacheProvider: return early (no-op)
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/config/providers/metadata_cache_provider.dart (1)
163-168: Remove activeAccount check from metadata fetch
Blocking on activeAccountProvider.future in _fetchMetadataForKey prevents unauthenticated metadata retrieval (e.g. examples, pre-login flows). Since wn_users_api.userMetadata doesn’t require auth, delete the guard:lib/config/providers/metadata_cache_provider.dart @@ -166,10 +166,3 @@ Future<ContactModel> _fetchMetadataForKey(String publicKey) async { - final activeAccountState = await ref.read(activeAccountProvider.future); - final activeAccount = activeAccountState.account; - if (activeAccount == null) { - throw StateError('No active account found'); - }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
lib/config/providers/metadata_cache_provider.dart(1 hunks)lib/ui/settings/general_settings_screen.dart(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
**/*.dart: Always declare the type of each variable and function (parameters and return value). Avoid using 'any'. Create necessary types.
Don't leave blank lines within a function.
One export per file.
Use PascalCase for classes.
Use camelCase for variables, functions, and methods.
Use underscores_case for file and directory names.
Use UPPERCASE for environment variables. Avoid magic numbers and define constants.
Start each function with a verb.
Use verbs for boolean variables. Example: isLoading, hasError, canDelete, etc.
Use complete words instead of abbreviations and correct spelling, except for standard and well-known abbreviations (API, URL, i, j, err, ctx, req, res, next).
Write short functions with a single purpose. Less than 20 instructions.
Name functions with a verb and something else. If it returns a boolean, use isX or hasX, canX, etc. If it doesn't return anything, use executeX or saveX, etc.
Avoid nesting blocks by early checks and returns, or extraction to utility functions.
Use higher-order functions (map, filter, reduce, etc.) to avoid function nesting. Use arrow functions for simple functions (less than 3 instructions). Use named functions for non-simple functions.
Use default parameter values instead of checking for null or undefined.
Reduce function parameters using RO-RO: use an object to pass multiple parameters and to return results. Declare necessary types for input arguments and output.
Use a single level of abstraction in functions.
Don't abuse primitive types and encapsulate data in composite types.
Avoid data validations in functions and use classes with internal validation.
Prefer immutability for data. Use readonly for data that doesn't change. Use 'as const' for literals that don't change.
Declare interfaces to define contracts.
Write small classes with a single purpose. Less than 200 instructions, less than 10 public methods, less than 10 properties.
Use exceptions to handle errors you don't expect. If you catch an exception, it sh...
Files:
lib/ui/settings/general_settings_screen.dartlib/config/providers/metadata_cache_provider.dart
🧠 Learnings (2)
📓 Common learnings
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#455
File: lib/ui/settings/profile/switch_profile_bottom_sheet.dart:0-0
Timestamp: 2025-08-12T11:21:53.640Z
Learning: In the whitenoise_flutter codebase, ContactModel.publicKey can be stored in either npub format (from metadata cache) or hex format (from account storage). The activeAccountProvider may return either format depending on how the account was originally stored, so normalization to hex format is required when comparing with other hex-normalized keys in sorting logic.
📚 Learning: 2025-08-12T11:21:53.640Z
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#455
File: lib/ui/settings/profile/switch_profile_bottom_sheet.dart:0-0
Timestamp: 2025-08-12T11:21:53.640Z
Learning: In the whitenoise_flutter codebase, ContactModel.publicKey can be stored in either npub format (from metadata cache) or hex format (from account storage). The activeAccountProvider may return either format depending on how the account was originally stored, so normalization to hex format is required when comparing with other hex-normalized keys in sorting logic.
Applied to files:
lib/config/providers/metadata_cache_provider.dart
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Flutter CI
🔇 Additional comments (3)
lib/ui/settings/general_settings_screen.dart (1)
42-42: No-op change acknowledged.Whitespace-only; no behavioral impact. Nothing to do.
lib/config/providers/metadata_cache_provider.dart (2)
88-112: LGTM on active-account cache write path.Standardizing to npub and forcing a fresh CachedMetadata write is correct and aligns with how getContactModel keys the cache.
338-342: Ensure cache keys use a consistent NPUB format.
- Verify
_normalizePublicKeyreturns the same NPUB key used elsewhere; if it currently lowercases hex, introduce an NPUB‐normalizing helper (e.g._getStandardizedNpub) and apply it.- Migrate
isContactCached/updateCachedMetadataand their call sites inlib/examples/metadata_cache_examples.dartandlib/utils/metadata_cache_utils.dartto use the standardized variants.
josefinalliende
left a comment
josefinalliende
left a comment
There was a problem hiding this comment.
Thanks for this fix! 🙌
I do have one (potentially annoying) comment: we eventually want to remove the metadata cache completely (#541). The problem is still pretty widespread, and in my previous attempts to remove it (first when taking this issue #418, then again during the Big Plans refactor), I ended up breaking a lot of things 😭.
That’s why I think the best approach here might be to move this fix into the active account provider instead, to make it easier (not harder) to remove later. WDYT?
If that feels too annoying or out of scope for this PR, no worries, I can take it on in another one. I was planning to assign myself to #514 anyway.
|
I think that should be done in another PR, because without merging this, users will see the bug in v0.1.4. So no matter how we do it, the user shouldn't see this bug. We can work together on removing the metadata cache. |
| MetadataCacheState build() => const MetadataCacheState(); | ||
| MetadataCacheState build() { | ||
| // Listen for active account changes to keep cache in sync with latest metadata | ||
| ref.listen(activeAccountProvider, (previous, next) { |
There was a problem hiding this comment.
This is nice. I have a concern though: are we sure activeAccountProvider only rebuilds on metadata change? If yes, cool but if not, I'm not sure we want to run the operation everytime. The upside is that this operation is not expensive so I'll just leave this as a question/comment/discussion.
There was a problem hiding this comment.
Good catch. I didn't care about this since it's just a map write and the metadata will be removed soon. But you're right, so I added a comparison.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
lib/config/providers/metadata_cache_provider.dart (4)
168-175: Avoid hard dependency on activeAccountProvider during fetch.Fetching metadata for an arbitrary key should not fail when no active account is set. Remove this guard to prevent false negatives during app bootstrap.
Apply:
- final activeAccountState = await ref.read(activeAccountProvider.future); - final activeAccount = activeAccountState.account; - if (activeAccount == null) { - throw StateError('No active account found'); - }
284-299: Null metadata in bulk populate leads to runtime/type issues.queryResults values are FlutterMetadata?. Guard nulls to avoid passing null into fromMetadata.
Apply:
- final metadata = entry.value; + final FlutterMetadata? metadata = entry.value; + if (metadata == null) { + skipped++; + continue; + } @@ - final contactModel = ContactModel.fromMetadata( + final ContactModel contactModel = ContactModel.fromMetadata( publicKey: standardNpub, metadata: metadata, );
371-383: Duplicate/fragmented entries: updateCachedMetadata writes under non-standardized key.Other paths store under standardized npub, but this method uses a plain normalized key, causing duplicates and misses on reads.
Apply:
- void updateCachedMetadata(String publicKey, ContactModel contactModel) { - final normalizedKey = _normalizePublicKey(publicKey); - - final newCache = Map<String, CachedMetadata>.from(state.cache); - newCache[normalizedKey] = CachedMetadata( - contactModel: contactModel, - cachedAt: DateTime.now(), - ); - - state = state.copyWith(cache: newCache); - _logger.info('Updated cached metadata for $normalizedKey'); - } + Future<void> updateCachedMetadata(String publicKey, ContactModel contactModel) async { + final String standardNpub = await _getStandardizedNpub(publicKey); + final Map<String, CachedMetadata> newCache = Map<String, CachedMetadata>.from(state.cache); + newCache[standardNpub] = CachedMetadata( + contactModel: contactModel, + cachedAt: DateTime.now(), + ); + state = state.copyWith(cache: newCache); + _logger.info('Updated cached metadata for $standardNpub'); + }Please update call sites accordingly.
343-347: Convert isContactCached to async standardized-npub lookup
isContactCached currently calls _normalizePublicKey and looks up state.cache by that key, but most writes use the async _getStandardizedNpub (npub) key—so hex inputs always miss. Replace or complement the sync method with:- bool isContactCached(String publicKey) { - final normalizedKey = _normalizePublicKey(publicKey); - final cached = state.cache[normalizedKey]; - return cached != null && !cached.isExpired; - } + Future<bool> isContactCachedAsync(String publicKey) async { + final standardNpub = await _getStandardizedNpub(publicKey); + final cached = state.cache[standardNpub]; + return cached != null && !cached.isExpired; + }Deprecate the sync variant or add a fallback lookup for normalized keys to avoid false negatives.
♻️ Duplicate comments (1)
lib/config/providers/metadata_cache_provider.dart (1)
62-80: LGTM: reactive sync on activeAccountProvider removes hex/npub false negatives.Unconditional update on data emissions is correct and aligns with prior guidance. Keep as is.
🧹 Nitpick comments (4)
lib/config/providers/metadata_cache_provider.dart (4)
83-117: Type annotations and equality guard improvements.
- Add explicit types per our Dart guidelines.
- The equality guard only checks displayName/about/nip05; if UI depends on other metadata (e.g., picture/banner/website), this could miss updates. Verify field coverage.
Apply explicit types:
Future<void> _updateCacheForActiveAccount(String pubkey, FlutterMetadata metadata) async { @@ - final standardNpub = await _getStandardizedNpub(pubkey); + final String standardNpub = await _getStandardizedNpub(pubkey); @@ - final existingCached = state.cache[standardNpub]; + final CachedMetadata? existingCached = state.cache[standardNpub]; @@ - final existing = existingCached.contactModel; + final ContactModel existing = existingCached.contactModel; @@ - final contactModel = ContactModel.fromMetadata( + final ContactModel contactModel = ContactModel.fromMetadata( @@ - final newCache = Map<String, CachedMetadata>.from(state.cache); + final Map<String, CachedMetadata> newCache = Map<String, CachedMetadata>.from(state.cache);
321-340: Parallelize getContactModels to reduce latency.Fetches are currently sequential. Use Future.wait with per-item error handling.
Apply:
- Future<List<ContactModel>> getContactModels(List<String> publicKeys) async { - final results = <ContactModel>[]; - - for (final publicKey in publicKeys) { - try { - final contactModel = await getContactModel(publicKey); - results.add(contactModel); - } catch (e) { - _logger.warning('Failed to get contact model for $publicKey: $e'); - // Add fallback model to maintain list integrity - results.add( - ContactModel( - displayName: 'Unknown User', - publicKey: _normalizePublicKey(publicKey), - ), - ); - } - } - - return results; - } + Future<List<ContactModel>> getContactModels(List<String> publicKeys) async { + final futures = publicKeys.map((publicKey) async { + try { + return await getContactModel(publicKey); + } catch (e) { + _logger.warning('Failed to get contact model for $publicKey: $e'); + return ContactModel( + displayName: kUnknownUserName, + publicKey: _normalizePublicKey(publicKey), + ); + } + }).toList(growable: false); + return await Future.wait(futures); + }Add near top-level:
const String kUnknownUserName = 'Unknown User';
185-205: Consolidate “Unknown User” into a constant.Use a single constant to avoid typos and simplify localization later.
Example:
const String kUnknownUserName = 'Unknown User';Then replace both occurrences.
Also applies to: 330-336
61-80: Style nits per guidelines.
- Add explicit local variable types (applied above to key spots).
- Avoid blank lines inside functions where not needed.
Also applies to: 83-117, 158-205, 270-317, 321-340, 343-383
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
lib/config/providers/metadata_cache_provider.dart(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
**/*.dart: Always declare the type of each variable and function (parameters and return value). Avoid using 'any'. Create necessary types.
Don't leave blank lines within a function.
One export per file.
Use PascalCase for classes.
Use camelCase for variables, functions, and methods.
Use underscores_case for file and directory names.
Use UPPERCASE for environment variables. Avoid magic numbers and define constants.
Start each function with a verb.
Use verbs for boolean variables. Example: isLoading, hasError, canDelete, etc.
Use complete words instead of abbreviations and correct spelling, except for standard and well-known abbreviations (API, URL, i, j, err, ctx, req, res, next).
Write short functions with a single purpose. Less than 20 instructions.
Name functions with a verb and something else. If it returns a boolean, use isX or hasX, canX, etc. If it doesn't return anything, use executeX or saveX, etc.
Avoid nesting blocks by early checks and returns, or extraction to utility functions.
Use higher-order functions (map, filter, reduce, etc.) to avoid function nesting. Use arrow functions for simple functions (less than 3 instructions). Use named functions for non-simple functions.
Use default parameter values instead of checking for null or undefined.
Reduce function parameters using RO-RO: use an object to pass multiple parameters and to return results. Declare necessary types for input arguments and output.
Use a single level of abstraction in functions.
Don't abuse primitive types and encapsulate data in composite types.
Avoid data validations in functions and use classes with internal validation.
Prefer immutability for data. Use readonly for data that doesn't change. Use 'as const' for literals that don't change.
Declare interfaces to define contracts.
Write small classes with a single purpose. Less than 200 instructions, less than 10 public methods, less than 10 properties.
Use exceptions to handle errors you don't expect. If you catch an exception, it sh...
Files:
lib/config/providers/metadata_cache_provider.dart
🧠 Learnings (2)
📓 Common learnings
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#455
File: lib/ui/settings/profile/switch_profile_bottom_sheet.dart:0-0
Timestamp: 2025-08-12T11:21:53.640Z
Learning: In the whitenoise_flutter codebase, ContactModel.publicKey can be stored in either npub format (from metadata cache) or hex format (from account storage). The activeAccountProvider may return either format depending on how the account was originally stored, so normalization to hex format is required when comparing with other hex-normalized keys in sorting logic.
📚 Learning: 2025-08-12T11:21:53.640Z
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#455
File: lib/ui/settings/profile/switch_profile_bottom_sheet.dart:0-0
Timestamp: 2025-08-12T11:21:53.640Z
Learning: In the whitenoise_flutter codebase, ContactModel.publicKey can be stored in either npub format (from metadata cache) or hex format (from account storage). The activeAccountProvider may return either format depending on how the account was originally stored, so normalization to hex format is required when comparing with other hex-normalized keys in sorting logic.
Applied to files:
lib/config/providers/metadata_cache_provider.dart
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Flutter CI
| ); | ||
|
|
||
| // Update the cache | ||
| final newCache = Map<String, CachedMetadata>.from(state.cache); |
There was a problem hiding this comment.
We're trying to completely remove the CachedMetadata from the app altogether. Can you maybe sync with Josefina about this?
The new userMetadata method on the bridge is only hitting a sqlite db so it's going to be SUPER fast. e.g. it never hits relays. So I think that you can probably vastly simplify this and just call that method whenever you want the user's metadata anywhere in the app.
|
I'm closing this PR. Because #560 fixes the same bug without using metadata cache. |
Description
The issue was caused by stale data in the metadataCacheProvider. When profile metadata was updated through the activeAccountProvider, the metadata cache wasn't being notified of these changes, causing the GeneralSettingsScreen to continue displaying outdated cached information.
Implemented a reactive cache invalidation system by enhancing the MetadataCacheProvider to automatically listen for active account changes and update the cache accordingly.
Screen.Recording.2025-08-31.at.04.07.29.mov
Type of Change
Checklist
just precommitto ensure that formatting and linting are correctjust check-flutter-coverageto ensure that flutter coverage rules are passingCHANGELOG.mdfile with your changes (if they affect the user experience)Fixes #549
Summary by CodeRabbit
New Features
Improvements