feat: add community discovery and node selector on first launch#564
Merged
feat: add community discovery and node selector on first launch#564
Conversation
Add a community selector screen shown after the walkthrough for new users, allowing them to choose a trusted Mostro community before reaching the home screen. Fetches kind 0 and kind 38385 metadata from relay.mostro.network via standalone WebSocket connection. Includes trusted communities for Cuba, Spain, Colombia and Bolivia, search filtering, custom node support, loading skeleton, error handling, and full i18n (en/es/it). Existing users are not interrupted thanks to backward-compatible auto-migration.
- Remove duplicate skipForNow keys from all ARB files - Fix race condition: treat loading state as unselected in redirect - Add mounted checks before state mutations in CommunitySelectedNotifier - Remove manual string truncation, rely on TextOverflow.ellipsis - Add try-catch to launchUrl for social links
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a first-run community/node selector: compile-time trusted community configs, a Community model, a WebSocket-based CommunityRepository to fetch Nostr metadata, providers and persistence for selection, UI widgets/screens, GoRouter redirect integration, and removal of Riverpod codegen in several providers. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as CommunitySelectorScreen
participant Provider as communityListProvider
participant Repo as CommunityRepository
participant Relay as Nostr Relay (wss://...)
participant Prefs as SharedPreferences
participant Router
User->>UI: open /community_selector
UI->>Provider: request community list
Provider->>Provider: build baseline from trustedCommunities
Provider->>Repo: fetchCommunityMetadata(pubkeys)
Repo->>Relay: open WebSocket, send REQ (kinds 0 & 38385)
Relay-->>Repo: EVENT / EOSE messages
Repo-->>Provider: Map<pubkey, CommunityMetadata>
Provider-->>UI: enriched List<Community>
User->>UI: select pubkey / skip / add custom node
UI->>Prefs: persist community_selected = true
UI->>Router: navigate to home (`context.go('/')`)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
lib/core/config.dart (1)
21-40: Avoid keeping the trusted community registry in two places.These pubkeys are now maintained here and again in
lib/core/config/communities.dart:30-71. If the two lists drift, the community selector can stop resolving the expected trusted node. Prefer a single shared source of truth and derive the trusted-node list from it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/core/config.dart` around lines 21 - 40, The file currently duplicates the trusted community registry as inline maps containing 'pubkey' and 'name' (the list shown) which also exists elsewhere causing drift; consolidate by extracting that canonical list into a single exported constant (the authoritative array of {'pubkey','name'} maps), remove the duplicate entries from this file, and update the consumer that builds the trusted-node list (the community selector) to import and derive its trusted-node list from that single exported constant so all code references the same source of truth.lib/features/community/providers/community_selector_provider.dart (1)
15-20: Avoiddynamictype for_prefs– useSharedPreferences.The
dynamictype loses compile-time safety. IfsharedPreferencesProviderreturns a different type or the API changes, errors would only surface at runtime.♻️ Suggested fix
+import 'package:shared_preferences/shared_preferences.dart'; + class CommunitySelectedNotifier extends StateNotifier<AsyncValue<bool>> { - final dynamic _prefs; + final SharedPreferences _prefs; CommunitySelectedNotifier(this._prefs) : super(const AsyncValue.loading()) { _init(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/features/community/providers/community_selector_provider.dart` around lines 15 - 20, The field `_prefs` in CommunitySelectedNotifier is declared as dynamic which loses type safety; change its type to SharedPreferences (from package:shared_preferences/shared_preferences.dart), update the constructor and any usages inside CommunitySelectedNotifier (including the _init method) to expect a SharedPreferences instance provided by sharedPreferencesProvider, and add the appropriate import for SharedPreferences so the compiler enforces correct API usage.lib/features/community/widgets/community_card.dart (1)
195-209: Consider adding a loading placeholder for network images.The avatar implementation correctly falls back to
NymAvataron error, but there's no visual feedback during image loading. This could cause a brief visual jump when the image loads.💡 Optional: Add loadingBuilder for smoother UX
Widget _buildAvatar() { if (community.picture != null) { return ClipOval( child: Image.network( community.picture!, width: 44, height: 44, fit: BoxFit.cover, + loadingBuilder: (_, child, loadingProgress) { + if (loadingProgress == null) return child; + return Container( + width: 44, + height: 44, + decoration: BoxDecoration( + color: Colors.white.withValues(alpha: 0.1), + shape: BoxShape.circle, + ), + ); + }, errorBuilder: (_, __, ___) => NymAvatar(pubkeyHex: community.pubkey, size: 44), ), ); } return NymAvatar(pubkeyHex: community.pubkey, size: 44); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/features/community/widgets/community_card.dart` around lines 195 - 209, The avatar currently uses Image.network inside _buildAvatar with only an errorBuilder fallback, so add a loadingBuilder to Image.network to show a placeholder while the network image is loading (for example reuse NymAvatar(pubkeyHex: community.pubkey, size: 44) or a SizedBox with spinner) to avoid visual jumps; locate the Image.network in _buildAvatar and add loadingBuilder that returns the placeholder while loadingProgress != null and the actual image once loadingProgress is null, preserving the existing errorBuilder fallback.lib/features/community/community.dart (1)
48-71:copyWithcannot reset nullable fields tonull.The current implementation uses
??for all nullable fields, which means once a field has a non-null value, it cannot be reset tonullviacopyWith. This is likely intentional for this use case (metadata only gets enriched, never cleared), but worth noting for future maintainability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/features/community/community.dart` around lines 48 - 71, Community.copyWith currently uses the "??" fallback so callers cannot explicitly reset nullable metadata fields to null; change the API to distinguish "not provided" from "explicitly null" (e.g. introduce a private sentinel like const _undefined = Object() and change parameters to Object? name = _undefined, Object? about = _undefined, Object? picture = _undefined, Object? currencies = _undefined, etc.), then inside copyWith check for the sentinel to decide whether to keep the existing value, set to the provided non-null value, or set to null (e.g. name == _undefined ? this.name : name as String?); apply this pattern for name, about, picture, currencies and any other nullable fields so callers can explicitly pass null to clear a field.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/features/community/community_fetcher.dart`:
- Around line 7-15: CommunityFetcher (class) and its static fetch method, which
opens the WebSocket to relay.mostro.network and returns Map<String,
CommunityMetadata>, must be moved out of the feature layer and wrapped behind a
repository in lib/data; create a new CommunityRepository with a method like
fetchCommunityMetadata(pubkeys) that encapsulates the WebSocket logic
(previously in CommunityFetcher) and returns the same Map<String,
CommunityMetadata>, update the feature code that calls CommunityFetcher (the
provider that invokes fetch) to depend on and call
CommunityRepository.fetchCommunityMetadata instead, and ensure dependency
injection or provider wiring supplies the repository to the feature rather than
referencing the transport directly.
- Around line 84-100: The code swallows fetch failures (timeout/socket errors)
by only logging and returning results, preventing communityListProvider from
seeing errors; change the timeout and catch handling so failures propagate:
remove or change the onTimeout callback on completer.future.timeout(_timeout,
...) so it throws a TimeoutException (or simply let timeout throw) instead of
just logging, and in the catch block (where logger.e('Failed to fetch community
data: $e')) rethrow the caught error (or throw a new exception) after logging;
keep the finally block to still await ws?.close() and cancel subscription as
currently done so resources are cleaned up while errors propagate to
communityListProvider.
In `@lib/features/community/screens/community_selector_screen.dart`:
- Around line 351-358: The helper _ensureNodeExists currently calls
notifier.addCustomNode(pubkey) without awaiting, which can race with subsequent
selectNode; change _ensureNodeExists to be async and await
notifier.addCustomNode(pubkey) (MostroNodesNotifier.addCustomNode) so the node
is persisted and state updated before any following selectNode call, and update
any callers of _ensureNodeExists to await its Future so they don't proceed
before the node exists (see selectNode usage).
- Around line 315-331: The fragile length-delta detection in _onUseCustomNode
should be replaced with a deterministic set-diff based on node identity: before
calling AddCustomNodeDialog.show(context, ref) snapshot the current pubkeys from
ref.read(mostroNodesProvider), then after the dialog read the provider again and
compute the difference (e.g., newPubkeys =
afterPubkeys.difference(beforePubkeys)); if exactly one new pubkey exists use
that to call _selectAndProceed(lastNewPubkey) (or if multiple were added choose
the intended one or handle accordingly); keep the existing mounted checks and
setState(_isSelecting) wrapping the await _selectAndProceed call.
---
Nitpick comments:
In `@lib/core/config.dart`:
- Around line 21-40: The file currently duplicates the trusted community
registry as inline maps containing 'pubkey' and 'name' (the list shown) which
also exists elsewhere causing drift; consolidate by extracting that canonical
list into a single exported constant (the authoritative array of
{'pubkey','name'} maps), remove the duplicate entries from this file, and update
the consumer that builds the trusted-node list (the community selector) to
import and derive its trusted-node list from that single exported constant so
all code references the same source of truth.
In `@lib/features/community/community.dart`:
- Around line 48-71: Community.copyWith currently uses the "??" fallback so
callers cannot explicitly reset nullable metadata fields to null; change the API
to distinguish "not provided" from "explicitly null" (e.g. introduce a private
sentinel like const _undefined = Object() and change parameters to Object? name
= _undefined, Object? about = _undefined, Object? picture = _undefined, Object?
currencies = _undefined, etc.), then inside copyWith check for the sentinel to
decide whether to keep the existing value, set to the provided non-null value,
or set to null (e.g. name == _undefined ? this.name : name as String?); apply
this pattern for name, about, picture, currencies and any other nullable fields
so callers can explicitly pass null to clear a field.
In `@lib/features/community/providers/community_selector_provider.dart`:
- Around line 15-20: The field `_prefs` in CommunitySelectedNotifier is declared
as dynamic which loses type safety; change its type to SharedPreferences (from
package:shared_preferences/shared_preferences.dart), update the constructor and
any usages inside CommunitySelectedNotifier (including the _init method) to
expect a SharedPreferences instance provided by sharedPreferencesProvider, and
add the appropriate import for SharedPreferences so the compiler enforces
correct API usage.
In `@lib/features/community/widgets/community_card.dart`:
- Around line 195-209: The avatar currently uses Image.network inside
_buildAvatar with only an errorBuilder fallback, so add a loadingBuilder to
Image.network to show a placeholder while the network image is loading (for
example reuse NymAvatar(pubkeyHex: community.pubkey, size: 44) or a SizedBox
with spinner) to avoid visual jumps; locate the Image.network in _buildAvatar
and add loadingBuilder that returns the placeholder while loadingProgress !=
null and the actual image once loadingProgress is null, preserving the existing
errorBuilder fallback.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 03aa088d-667c-4f4d-b67b-0716ea0529ae
📒 Files selected for processing (13)
lib/core/app_routes.dartlib/core/config.dartlib/core/config/communities.dartlib/data/models/enums/storage_keys.dartlib/features/community/community.dartlib/features/community/community_fetcher.dartlib/features/community/providers/community_selector_provider.dartlib/features/community/screens/community_selector_screen.dartlib/features/community/widgets/community_card.dartlib/features/walkthrough/screens/walkthrough_screen.dartlib/l10n/intl_en.arblib/l10n/intl_es.arblib/l10n/intl_it.arb
- Move WebSocket fetcher to CommunityRepository in data/repositories - Propagate fetch errors so communityListProvider shows error state - Fix dynamic _prefs type to SharedPreferencesAsync for type safety - Await _ensureNodeExists to prevent race with selectNode - Use set-diff for robust custom node detection after dialog - Derive Config.trustedMostroNodes from trustedCommunities (single source) - Add loadingBuilder to avatar for smooth image loading
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
lib/core/config.dart (1)
14-15: Consider usingdefaultMostroPubkeyfromcommunities.dartinstead of maintaining a duplicate constant.
_defaultMostroPubKeyduplicatesdefaultMostroPubkeyalready defined incommunities.dart. Since the PR aims to establishtrustedCommunitiesas the single source of truth, this constant could be removed in favor of importing the existing one.♻️ Proposed refactor
import 'package:flutter/foundation.dart'; import 'package:mostro_mobile/core/config/communities.dart'; class Config { // Nostr configuration static const List<String> nostrRelays = [ 'wss://relay.mostro.network', //'ws://127.0.0.1:7000', //'ws://192.168.1.103:7000', //'ws://10.0.2.2:7000', // mobile emulator ]; - // Trusted Mostro nodes registry - static const String _defaultMostroPubKey = - '82fa8cb978b43c79b2156585bac2c011176a21d2aead6d9f7c575c005be88390'; - // Derived from trustedCommunities to maintain single source of truth static final List<Map<String, String>> trustedMostroNodes = trustedCommunities .map((c) => { 'pubkey': c.pubkey, 'name': c.region, }) .toList(); // Mostro hexkey (backward compatible, overridable via env variable) static const String mostroPubKey = String.fromEnvironment( 'MOSTRO_PUB_KEY', - defaultValue: _defaultMostroPubKey, + defaultValue: defaultMostroPubkey, );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/core/config.dart` around lines 14 - 15, Remove the duplicated constant _defaultMostroPubKey from the config class and import and use the existing defaultMostroPubkey from communities.dart instead; update any references in this file (e.g., usage within the Config class and any places that reference _defaultMostroPubKey) to reference defaultMostroPubkey and ensure trustedCommunities remains the single source of truth for community keys; add the import for the symbol and delete the local constant to avoid duplication.lib/data/repositories/community_repository.dart (2)
53-88: Consider handling WebSocket closure before EOSE arrival.The
onDonecallback at line 84 completes the completer without error when the WebSocket closes. If the relay closes the connection before sending both EOSE messages,resultsmay contain incomplete data (only partial metadata fetched). This is acceptable for a "best effort" fetch, but the caller should be aware that the returned map may have emptyCommunityMetadataentries.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/data/repositories/community_repository.dart` around lines 53 - 88, The WebSocket may close before both EOSE messages arrive; update the ws.listen onDone handler to detect if eoseCount < 2 and, when that happens, complete the completer with an error (e.g., completeError(StateError('WebSocket closed before EOSE'))) so callers receive an explicit failure instead of silently returning potentially incomplete results; reference eoseCount, completer, onDone, ws.listen, _processEvent and _timeout when making this change so the logic around waiting for EOSEs and timeout remains consistent.
106-136: Consider adding signature verification for kind 0 events to align with established patterns.The
_processEventmethod correctly deduplicates events by keeping the latestcreated_atand sanitizes image URLs to HTTPS. However, it does not verify event signatures. The MostroNodesNotifier usesevent.isVerified()for similar kind 0 metadata processing and logs a warning when verification fails (with fallback logic due to a dart_nostr limitation).For a community selector that fetches from the trusted relay
wss://relay.mostro.networkon first launch only, this omission may be acceptable, but applying the same verification pattern used in MostroNodesNotifier would provide consistency across the codebase.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/data/repositories/community_repository.dart` around lines 106 - 136, In _processEvent, add signature verification for kind 0 events similar to MostroNodesNotifier: when kind == 0 call event.isVerified() (or the equivalent verification method on the event object) and if it returns false log a warning via logger.w('Failed to verify kind 0 event for $pubkey') then continue with the existing fallback logic (i.e., still attempt to parse and store the content but ensure the warning is emitted), updating references to CommunityMetadata (kind0, kind0CreatedAt) only after handling verification; this keeps behavior consistent while preserving the current fallback semantics used elsewhere.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@lib/core/config.dart`:
- Around line 14-15: Remove the duplicated constant _defaultMostroPubKey from
the config class and import and use the existing defaultMostroPubkey from
communities.dart instead; update any references in this file (e.g., usage within
the Config class and any places that reference _defaultMostroPubKey) to
reference defaultMostroPubkey and ensure trustedCommunities remains the single
source of truth for community keys; add the import for the symbol and delete the
local constant to avoid duplication.
In `@lib/data/repositories/community_repository.dart`:
- Around line 53-88: The WebSocket may close before both EOSE messages arrive;
update the ws.listen onDone handler to detect if eoseCount < 2 and, when that
happens, complete the completer with an error (e.g.,
completeError(StateError('WebSocket closed before EOSE'))) so callers receive an
explicit failure instead of silently returning potentially incomplete results;
reference eoseCount, completer, onDone, ws.listen, _processEvent and _timeout
when making this change so the logic around waiting for EOSEs and timeout
remains consistent.
- Around line 106-136: In _processEvent, add signature verification for kind 0
events similar to MostroNodesNotifier: when kind == 0 call event.isVerified()
(or the equivalent verification method on the event object) and if it returns
false log a warning via logger.w('Failed to verify kind 0 event for $pubkey')
then continue with the existing fallback logic (i.e., still attempt to parse and
store the content but ensure the warning is emitted), updating references to
CommunityMetadata (kind0, kind0CreatedAt) only after handling verification; this
keeps behavior consistent while preserving the current fallback semantics used
elsewhere.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 638597fd-2bd1-45f2-83ae-78b618b4f15d
📒 Files selected for processing (5)
lib/core/config.dartlib/data/repositories/community_repository.dartlib/features/community/providers/community_selector_provider.dartlib/features/community/screens/community_selector_screen.dartlib/features/community/widgets/community_card.dart
Replace @riverpod annotations with manual providers in the 3 files that used code generation (event_bus, mostro_service_provider, order_notifier_provider). This eliminates the riverpod_generator and riverpod_annotation dependencies whose analyzer requirement conflicted with the test package on Flutter 3.41.6.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/services/event_bus.dart`:
- Around line 15-19: The AutoDisposeProvider `eventBusProvider` currently
creates an `EventBus` but leaves its `StreamController` open because the
`ref.onDispose(bus.dispose)` call is commented out; restore proper disposal by
registering the provider's cleanup callback (e.g., call `ref.onDispose` with
`bus.dispose` or a closure that calls `bus.dispose()`) so `EventBus.dispose()`
runs when the provider is disposed, thus preventing the StreamController leak.
In `@lib/shared/providers/mostro_service_provider.dart`:
- Around line 12-25: The provider calls MostroService.init() and
LifecycleManager.onResumed() may call it again, which leaks subscriptions
because MostroService.init() reassigns _ordersSubscription without cancelling;
make init() idempotent by checking and cancelling any existing
_ordersSubscription before creating a new subscription (cancel and null-check
_ordersSubscription inside MostroService.init()), or alternatively remove the
extra call to MostroService.init() from LifecycleManager.onResumed(); ensure you
reference the _ordersSubscription field and the MostroService.init() method (and
keep MostroService.dispose() unchanged to still cancel on provider disposal).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 500e55a3-eeb6-4322-9f46-c8a5dd806021
⛔ Files ignored due to path filters (1)
pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
lib/core/config.dartlib/features/order/providers/order_notifier_provider.dartlib/services/event_bus.dartlib/shared/providers/mostro_service_provider.dartpubspec.yaml
💤 Files with no reviewable changes (1)
- pubspec.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/core/config.dart
…nodes without trade info Add NIP-01 signature verification (event ID + Schnorr) before processing kind 0 and kind 38385 events to prevent relay-injected spoofed metadata. Filter out community nodes that lack valid kind 38385 trade info from the selector.
Replace single hardcoded relay with Config.nostrRelays list, trying each relay in order until one succeeds. Prevents community selector from breaking when the primary relay is down.
…etch Return partial results when timeout fires but events were already received, instead of discarding them. Only rethrow to try the next relay when zero events arrived.
Add SystemUiOverlayStyle.light to AppBarTheme for screens with AppBar, and wrap CommunitySelectorScreen with AnnotatedRegion since it lacks an AppBar.
Fix race condition where the router redirect evaluated communitySelectedProvider while still loading from SharedPreferences, causing the community selector to show on every launch. Skip redirect during loading and refresh the router when the provider resolves.
so the StreamController is properly closed when the provider is disposed.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
test/data/repositories/community_signature_test.dart (1)
8-33: Avoid re-implementing the verifier inside the test helper.
verifyRawEvent()duplicates the same NIP-01 logic asCommunityRepository, so both copies can drift together and still leave the suite green. Extracting the verifier into a shared utility, or drivingCommunityRepositorythrough a fake relay payload, would give this test real regression value.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/data/repositories/community_signature_test.dart` around lines 8 - 33, The test helper verifyRawEvent duplicates CommunityRepository's NIP-01 logic; remove this re-implementation and instead call the single source-of-truth verifier or exercise CommunityRepository directly: replace verifyRawEvent usages with a call to the shared verifier (e.g., NostrKeyPairs.verifyEvent or CommunityRepository.verifyEvent if it exists) or update the test to construct a fake relay payload and pass it through CommunityRepository processing so the repository's actual verification code is exercised; ensure references to NostrKeyPairs.verify, CommunityRepository, and the test helper verifyRawEvent are updated/removed accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/COMMUNITY_DISCOVERY.md`:
- Around line 16-18: The fenced ASCII flow blocks like the one containing "App
install -> Walkthrough (complete or skip) -> Community Selector -> Home" in
COMMUNITY_DISCOVERY.md are missing language tags and trigger markdownlint MD040;
update each of those fenced code blocks (including the blocks around lines with
the shown flow and the other ranges noted: 22-24, 28-30, 50-67, 82-105, 162-173,
236-255) by adding a language specifier (use "text") after the opening triple
backticks so they read ```text and stop failing the linter.
In `@lib/data/repositories/community_repository.dart`:
- Around line 41-44: The relay fetch logic is marking success too early: update
_fetchFromRelay() so it does not set hasEvents (or mark a pubkey as served) on
receipt of any EVENT — only set/apply results for a pubkey after _verifyEvent()
returns true and the verified event is applied; likewise change the
onDone/stream completion logic so the fetch is not considered complete when the
socket closes or onDone fires unless both subscriptions reached EOSE or at least
one verified event was applied; if no verified events were applied, have
_fetchFromRelay() signal failure (e.g., return null/throw) so
fetchCommunityMetadata() will continue to the next relay instead of returning
the pre-seeded empty map.
In `@lib/features/community/screens/community_selector_screen.dart`:
- Around line 297-360: The onboarding selection flow currently allows exceptions
from _selectAndProceed and _ensureNodeExists to bubble up with no user feedback;
update _selectAndProceed to wrap the sequence (ensure node exists,
nodesNotifier.selectNode, and communitySelectedProvider.markCommunitySelected)
in a try/catch that logs the error (using your app logger) and surfaces a
localized error to the user (e.g., ScaffoldMessenger.showSnackBar or a dialog
via context) before returning, ensuring buttons are re-enabled; also ensure
_ensureNodeExists persists the custom node before any in-memory selection (use
notifier.addCustomNode to persist and await it, then proceed) and surface/log
any errors there instead of throwing.
In `@lib/features/community/widgets/community_card.dart`:
- Around line 234-245: The current _formatSats builds strings with hard-coded
"K/M" and passes the unit through S.of(context)!.communityFormatSats; change it
to use locale-aware NumberFormat for the numeric part and append the literal
"sats" (not localized). Specifically, in _formatSats use
Localizations.localeOf(context) to get the locale, format amounts <1000 with
NumberFormat.decimalPattern(locale).format(amount) and format >=1000 with
NumberFormat.compact(locale).format(amount), then return a plain string like
"<formatted> sats" (remove calls to S.of(context)!.communityFormatSats and
ensure the unit is the hardcoded "sats").
---
Nitpick comments:
In `@test/data/repositories/community_signature_test.dart`:
- Around line 8-33: The test helper verifyRawEvent duplicates
CommunityRepository's NIP-01 logic; remove this re-implementation and instead
call the single source-of-truth verifier or exercise CommunityRepository
directly: replace verifyRawEvent usages with a call to the shared verifier
(e.g., NostrKeyPairs.verifyEvent or CommunityRepository.verifyEvent if it
exists) or update the test to construct a fake relay payload and pass it through
CommunityRepository processing so the repository's actual verification code is
exercised; ensure references to NostrKeyPairs.verify, CommunityRepository, and
the test helper verifyRawEvent are updated/removed accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9d103528-fb2b-45e3-84c6-b0714be087c6
📒 Files selected for processing (16)
docs/COMMUNITY_DISCOVERY.mdlib/core/app.dartlib/core/app_routes.dartlib/core/app_theme.dartlib/data/repositories/community_repository.dartlib/features/community/community.dartlib/features/community/providers/community_selector_provider.dartlib/features/community/screens/community_selector_screen.dartlib/features/community/widgets/community_card.dartlib/l10n/intl_de.arblib/l10n/intl_en.arblib/l10n/intl_es.arblib/l10n/intl_fr.arblib/l10n/intl_it.arbtest/data/repositories/community_signature_test.darttest/features/mostro/mostro_nodes_notifier_test.dart
✅ Files skipped from review due to trivial changes (2)
- test/features/mostro/mostro_nodes_notifier_test.dart
- lib/core/app_theme.dart
🚧 Files skipped from review as they are similar to previous changes (6)
- lib/l10n/intl_es.arb
- lib/l10n/intl_en.arb
- lib/l10n/intl_it.arb
- lib/features/community/community.dart
- lib/features/community/providers/community_selector_provider.dart
- lib/core/app_routes.dart
…, and verification tracking - Add error handling in _selectAndProceed with user-facing SnackBar - Track hasEvents only after verified events are applied, so unverified relays fall through to the next relay instead of returning empty results - Use locale-aware NumberFormat.compact for sats formatting in cards - Add language tags to fenced code blocks in COMMUNITY_DISCOVERY.md - Update doc to match current redirect and relay fallback behavior - Remove duplicated test helper in favor of CommunityRepository.verifyEvent
Member
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/data/repositories/community_repository.dart`:
- Around line 38-139: The _fetchFromRelay function currently returns the
pre-seeded empty results when the WebSocket closes early (onDone) without any
verified events, preventing failover; update the logic so that if the connection
finishes (onDone or completer completion) and hasEvents is false (and eoseCount
< 2), the function throws an error instead of returning results: specifically,
in the ws.listen onDone handler (or immediately after awaiting
completer.future.timeout) check if (!hasEvents) and complete the completer with
an error (or throw) so the outer catch path logs and rethrows; reference the
_fetchFromRelay function, the completer, onDone, hasEvents, and
eoseCount/subIdKind* variables when applying this change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 64ccb5d3-d262-46fb-a2d7-a654e8abc7ed
📒 Files selected for processing (17)
docs/COMMUNITY_DISCOVERY.mdlib/core/app.dartlib/core/app_routes.dartlib/core/app_theme.dartlib/data/repositories/community_repository.dartlib/features/community/community.dartlib/features/community/providers/community_selector_provider.dartlib/features/community/screens/community_selector_screen.dartlib/features/community/widgets/community_card.dartlib/l10n/intl_de.arblib/l10n/intl_en.arblib/l10n/intl_es.arblib/l10n/intl_fr.arblib/l10n/intl_it.arblib/services/event_bus.darttest/data/repositories/community_signature_test.darttest/features/mostro/mostro_nodes_notifier_test.dart
✅ Files skipped from review due to trivial changes (6)
- test/features/mostro/mostro_nodes_notifier_test.dart
- lib/core/app_theme.dart
- lib/l10n/intl_es.arb
- lib/l10n/intl_it.arb
- test/data/repositories/community_signature_test.dart
- docs/COMMUNITY_DISCOVERY.md
🚧 Files skipped from review as they are similar to previous changes (5)
- lib/core/app.dart
- lib/services/event_bus.dart
- lib/l10n/intl_de.arb
- lib/l10n/intl_en.arb
- lib/features/community/community.dart
Member
AndreaDiazCorreia
left a comment
AndreaDiazCorreia
left a comment
There was a problem hiding this comment.
tACK, but there are still some comments pending from CodeRabbit. Also, I’m not sure if it’s confusing to call them “communities” in the onboarding and “nodes” in settings. We should decide which term to use, or clarify it as “Community nodes” so it’s clear what we mean when we say “nodes.”
- Add disclaimer modal to community selector screen shown on first load - Add disclaimer warning below Mostro node selector in settings - Add i18n strings for disclaimer (EN, ES, FR, DE, IT) - Disclaimer: Mostro team not responsible for node operator actions
…vents The onDone handler now completes with an error when the connection closes early without any verified events and before both EOSEs arrived, allowing fetchCommunityMetadata to try the next relay instead of returning empty results.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add a community selector screen shown after the walkthrough for new
users, allowing them to choose a trusted Mostro community before
reaching the home screen. Fetches kind 0 and kind 38385 metadata
from relay.mostro.network via standalone WebSocket connection.
Includes trusted communities for Cuba, Spain, Colombia and Bolivia,
search filtering, custom node support, loading skeleton, error
handling, and full i18n (en/es/it). Existing users are not
interrupted thanks to backward-compatible auto-migration.
Summary by CodeRabbit
New Features
Behavior
Documentation
Chores