Add member to an existing group #743
Conversation
- Added UserProfile.fromUser factory method to convert User objects to UserProfile - Added new "Add Members" screen with search and multi-select capabilities - Implemented group member addition functionality with loading states and error handling - Added new localization strings for member management in 8 languages - Added new route and navigation methods for the add members screen - Updated router configuration to support the new add members screen
WalkthroughAdds an "add member to group" feature: new AddGroupMembersScreen with search/selection and add flow, routing and navigation helpers, an "Add Members" button in group info, and localized strings added across multiple locale files; CHANGELOG updated. Changes
Sequence DiagramsequenceDiagram
actor Admin as Admin User
participant GCI as GroupChatInfo
participant Router as Router
participant AGM as AddGroupMembersScreen
participant API as API Service
participant Toast as Toast UI
Admin->>GCI: Tap "Add Members"
GCI->>Router: goToAddGroupMembers(groupId, existingPubkeys)
Router->>AGM: Open AddGroupMembersScreen(groupId, existingMemberPubkeys)
AGM->>AGM: Show search + follows list (existing members disabled)
loop Select members
Admin->>AGM: Search / toggle selections
AGM-->>AGM: Update selection set
end
Admin->>AGM: Tap "Add" button
AGM->>AGM: Validate account, set loading
AGM->>API: addMembersToGroup(groupId, selectedPubkeys)
alt Success
API-->>AGM: Success
AGM->>Toast: Show success message
AGM->>API: Reload group info
AGM->>Router: Close screen / pop
else Failure
API-->>AGM: Error
AGM->>Toast: Show error message
AGM->>AGM: Clear loading
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (2)**/*.dart📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
Files:
lib/**/*.dart📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
Files:
🧠 Learnings (3)📚 Learning: 2025-09-07T13:10:16.542ZApplied to files:
📚 Learning: 2025-08-12T11:21:53.640ZApplied to files:
📚 Learning: 2025-09-04T17:54:09.162ZApplied to files:
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}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
lib/ui/chat/chat_info/add_group_members_screen.dart (2)
132-162: Optimize pubkey comparison.Lines 137-141: The current user's pubkey is converted to lowercase in every iteration. Consider converting it once before the filter for better performance.
Apply this diff to optimize:
List<User> _getFilteredFollows(List<User>? follows, String? currentUserPubkey) { if (follows == null) return []; + final normalizedCurrentUserPubkey = currentUserPubkey?.trim().toLowerCase(); + final availableFollows = follows.where((user) { final userPubkey = user.publicKey.trim().toLowerCase(); - if (currentUserPubkey != null && userPubkey == currentUserPubkey.trim().toLowerCase()) { + if (normalizedCurrentUserPubkey != null && userPubkey == normalizedCurrentUserPubkey) { return false; } return true; }).toList();
164-310: Consider optimizing data transformation.Lines 169-172: The code maps all follows to User objects before filtering. Consider filtering the follows first if possible, then mapping only the needed items to reduce unnecessary object creation. However, if the filtering logic requires User properties, the current approach is acceptable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
ios/Podfile.lockis excluded by!**/*.lock
📒 Files selected for processing (14)
CHANGELOG.md(1 hunks)lib/domain/models/user_profile.dart(2 hunks)lib/locales/de.json(1 hunks)lib/locales/en.json(2 hunks)lib/locales/es.json(1 hunks)lib/locales/fr.json(1 hunks)lib/locales/it.json(1 hunks)lib/locales/pt.json(1 hunks)lib/locales/ru.json(1 hunks)lib/locales/tr.json(1 hunks)lib/routing/router_provider.dart(2 hunks)lib/routing/routes.dart(2 hunks)lib/ui/chat/chat_info/add_group_members_screen.dart(1 hunks)lib/ui/chat/chat_info/group_chat_info.dart(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
**/*.dart: Always declare the type of each variable and function (parameters and return value)
Avoid using dynamic and Object without justification
Create necessary types instead of overusing primitives or dynamic
One export per file
Use PascalCase for classes
Use camelCase for variables, functions, and methods
Avoid magic numbers and define constants
Start each function name with a verb
Use verbs for boolean variables (e.g., isLoading, hasError, canDelete)
Write short functions with a single purpose (under ~20 instructions)
Name functions with a verb plus context; for booleans use isX/hasX/canX; for void use executeX/saveX
Avoid deep nesting via early returns and extraction to utility functions
Use higher-order functions (map, where/filter, reduce) to avoid nesting
Use arrow functions for simple functions (under ~3 statements); use named functions otherwise
Use default parameter values instead of null checks
Reduce function parameters using RO-RO: pass/return parameter objects with declared types
Maintain a single level of abstraction within functions
Encapsulate data in composite types; avoid overusing primitives
Prefer validating data within classes rather than in functions
Prefer immutability; use final for runtime constants and const for compile-time constants
Use const constructors and const literals where possible
Follow SOLID principles
Prefer composition over inheritance
Declare interfaces (abstract classes) to define contracts
Write small classes with a single purpose (under ~200 instructions, <10 public methods, <10 properties)
Use exceptions for unexpected errors
Only catch exceptions to fix expected problems or add context; otherwise use a global handler
Files:
lib/domain/models/user_profile.dartlib/ui/chat/chat_info/add_group_members_screen.dartlib/routing/routes.dartlib/routing/router_provider.dartlib/ui/chat/chat_info/group_chat_info.dart
lib/**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
lib/**/*.dart: Use flutter_rust_bridge to access core app functionality
Use Riverpod for state management; prefer StreamProviders for Rust API streams; use keepAlive if needed
Use freezed to model/manage UI states
Controllers should expose methods as inputs and update UI state that drives the UI
Use AutoRoute for navigation and use extras to pass data between pages
Use Dart extensions to manage reusable code
Use ThemeData to manage themes
Use AppLocalizations for translations
Use constants to manage constant values
Avoid deeply nested widget trees; aim for a flatter widget structure for performance and readability
Break down large widgets into smaller, focused, reusable components
Keep the widget tree shallow to simplify state management and data flow
Utilize const constructors and const widgets wherever possible to reduce rebuilds
Files:
lib/domain/models/user_profile.dartlib/ui/chat/chat_info/add_group_members_screen.dartlib/routing/routes.dartlib/routing/router_provider.dartlib/ui/chat/chat_info/group_chat_info.dart
**/*.md
📄 CodeRabbit inference engine (.cursor/rules/whitenoise.mdc)
**/*.md: NIPs (Nostr Implementation Possibilities) are numbered likeNIP-XXwhereXXare two capitalized hexadecimal digits, e.g.,NIP-01andNIP-C7.
To read a specific NIP, construct the NIP URL following this template:https://raw.githubusercontent.com/nostr-protocol/nips/refs/heads/master/{nip}.md(replace{nip}in the URL template with the relevant NIP name, e.g.,07for NIP-07, orC7for NIP-C7).
To read the definition of a specific kind, construct a URL following this template:https://nostrbook.dev/kinds/{kind}.md(replace{kind}in the template with the kind number, e.g.,https://nostrbook.dev/kinds/0.mdfor kind 0).
Files:
CHANGELOG.md
🧠 Learnings (1)
📚 Learning: 2025-09-07T13:10:16.542Z
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#597
File: lib/config/providers/group_provider.dart:311-314
Timestamp: 2025-09-07T13:10:16.542Z
Learning: In the whitenoise_flutter codebase, the User class used in group_provider.dart (and similar contexts) is presentational only, not the actual user class from the Rust API. There are plans to remove this User class and replace it with UserProfileData, similar to the planned consolidation with ContactModel.
Applied to files:
lib/domain/models/user_profile.dart
🔇 Additional comments (21)
CHANGELOG.md (1)
16-16: LGTM!The changelog entry accurately documents the new feature in a clear and concise manner.
lib/locales/tr.json (1)
297-302: LGTM!The Turkish localization strings for the add members feature are properly formatted and align with the UI flow requirements.
lib/locales/en.json (2)
17-17: Note the addition of unrelated key.The "clearAll" key appears unrelated to the PR's stated objective of adding members to groups. If this is part of the new AddGroupMembersScreen UI, consider documenting its usage; otherwise, it may belong in a separate PR.
298-303: LGTM!The English localization strings for the add members feature are clear and properly formatted.
lib/locales/es.json (1)
297-302: LGTM!The Spanish localization strings are well-translated and consistent with the feature requirements.
lib/locales/de.json (1)
297-302: LGTM!The German localization strings are properly translated and follow the established pattern.
lib/domain/models/user_profile.dart (1)
2-2: LGTM!The import is necessary for the new factory constructor.
lib/locales/it.json (1)
297-302: LGTM!The Italian localization strings are properly translated and consistent with the feature.
lib/locales/ru.json (1)
297-302: LGTM!The Russian localization strings are well-translated and align with the feature requirements.
lib/locales/pt.json (1)
297-302: LGTM! Portuguese translations added correctly.The new translation keys for the add members feature are properly formatted and consistent with the existing locale structure.
lib/routing/routes.dart (2)
17-17: LGTM! Route constant follows established pattern.The route path is consistent with the existing routing structure.
56-62: LGTM! Navigation helper properly implemented.The helper correctly uses
pushfor the modal-like flow and passes the existing member pubkeys as extra data. The type will be properly handled in the route builder.lib/ui/chat/chat_info/group_chat_info.dart (2)
145-153: LGTM! Navigation helper method properly implemented.The method correctly extracts existing member public keys and navigates to the add members screen.
218-239: LGTM! Admin buttons properly structured.The UI correctly displays both Edit Group and Add Members buttons for admins, with appropriate visual hierarchy and spacing.
lib/routing/router_provider.dart (2)
12-12: LGTM! Import properly added.The import for the new screen follows the established ordering pattern.
173-183: LGTM! Route properly configured.The route correctly extracts parameters with appropriate type safety, using null coalescing to default to an empty list when extra data is not provided.
lib/locales/fr.json (1)
297-302: LGTM! French translations added correctly.The new translation keys are properly formatted and consistent with the locale structure.
lib/ui/chat/chat_info/add_group_members_screen.dart (4)
24-36: LGTM! Screen class properly structured.The public API is well-defined with explicit types, immutable fields, and follows Riverpod conventions.
38-56: LGTM! Proper state management and lifecycle handling.The state class correctly manages resources with proper initialization and disposal of the text controller and listener.
80-90: LGTM! Selection toggle properly implemented.The method correctly prevents toggling existing members and efficiently manages the selected users set.
312-458: LGTM! Child widgets properly structured and factory method verified.The
_SelectedChipsand_FollowsListwidgets are well-organized with clear responsibilities. The pubkey normalization logic correctly handles format conversions with appropriate error handling. TheUserProfile.fromUserfactory method at line 445 exists and properly converts User objects to UserProfile instances.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…/parres-hq/whitenoise_flutter into add-and-remove-member-from-group
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
lib/ui/chat/chat_info/add_group_members_screen.dart (2)
62-63: Comment doesn't match implementation.The comment claims whitespace removal applies when the input "starts with npub or is hex-like," but the code only checks for the 'npub' prefix on line 63. Update the comment to match the actual implementation.
Apply this diff to fix the comment:
- // Only remove whitespace if it looks like a public key (starts with npub or is hex-like) + // Only remove whitespace if it looks like a public key (starts with npub)
102-102: Use the correct locale key.Line 102 references a non-existent locale key
'errors.noActiveAccount'. As identified in the previous review, the correct key is'settings.noActiveAccountFound', which exists in all locale files and is used consistently throughout the codebase.Apply this diff to use the correct key:
- safeShowErrorToast('errors.noActiveAccount'.tr()); + safeShowErrorToast('settings.noActiveAccountFound'.tr());
🧹 Nitpick comments (1)
lib/ui/chat/chat_info/add_group_members_screen.dart (1)
164-310: Consider extracting UI sections into separate widgets.The build method spans 146 lines and could benefit from further decomposition. While delegating to
_SelectedChipsand_FollowsListis good, extracting the header (lines 179-206), search field section (lines 207-224), and bottom action bar (lines 288-305) into separate private widgets would improve readability and align with the guideline to "break down large widgets into smaller, focused, reusable components."
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/ui/chat/chat_info/add_group_members_screen.dart(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
**/*.dart: Always declare the type of each variable and function (parameters and return value)
Avoid using dynamic and Object without justification
Create necessary types instead of overusing primitives or dynamic
One export per file
Use PascalCase for classes
Use camelCase for variables, functions, and methods
Avoid magic numbers and define constants
Start each function name with a verb
Use verbs for boolean variables (e.g., isLoading, hasError, canDelete)
Write short functions with a single purpose (under ~20 instructions)
Name functions with a verb plus context; for booleans use isX/hasX/canX; for void use executeX/saveX
Avoid deep nesting via early returns and extraction to utility functions
Use higher-order functions (map, where/filter, reduce) to avoid nesting
Use arrow functions for simple functions (under ~3 statements); use named functions otherwise
Use default parameter values instead of null checks
Reduce function parameters using RO-RO: pass/return parameter objects with declared types
Maintain a single level of abstraction within functions
Encapsulate data in composite types; avoid overusing primitives
Prefer validating data within classes rather than in functions
Prefer immutability; use final for runtime constants and const for compile-time constants
Use const constructors and const literals where possible
Follow SOLID principles
Prefer composition over inheritance
Declare interfaces (abstract classes) to define contracts
Write small classes with a single purpose (under ~200 instructions, <10 public methods, <10 properties)
Use exceptions for unexpected errors
Only catch exceptions to fix expected problems or add context; otherwise use a global handler
Files:
lib/ui/chat/chat_info/add_group_members_screen.dart
lib/**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
lib/**/*.dart: Use flutter_rust_bridge to access core app functionality
Use Riverpod for state management; prefer StreamProviders for Rust API streams; use keepAlive if needed
Use freezed to model/manage UI states
Controllers should expose methods as inputs and update UI state that drives the UI
Use AutoRoute for navigation and use extras to pass data between pages
Use Dart extensions to manage reusable code
Use ThemeData to manage themes
Use AppLocalizations for translations
Use constants to manage constant values
Avoid deeply nested widget trees; aim for a flatter widget structure for performance and readability
Break down large widgets into smaller, focused, reusable components
Keep the widget tree shallow to simplify state management and data flow
Utilize const constructors and const widgets wherever possible to reduce rebuilds
Files:
lib/ui/chat/chat_info/add_group_members_screen.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 (6)
lib/ui/chat/chat_info/add_group_members_screen.dart (6)
24-36: LGTM!Class declaration follows Flutter and Dart best practices: const constructor, properly typed final fields, and clear naming. The API contract is well-defined with required parameters.
38-56: LGTM!State initialization and cleanup follow Flutter best practices. Proper listener management and controller disposal prevent memory leaks.
80-90: LGTM!Selection toggle logic is clean with proper guard for existing members.
132-162: LGTM!Filtering logic is correct and efficient, with proper null handling and early returns. Searching across displayName, nip05, and publicKey provides good UX.
312-375: LGTM!Well-structured private widget with proper separation of concerns. The early return optimization and flexible Wrap layout demonstrate good Flutter practices.
377-458: LGTM!Excellent implementation with robust pubkey normalization and comparison logic. The try-catch blocks in
_isExistingMemberprevent crashes from malformed pubkeys, and the visual feedback for existing members (opacity + disabled interaction) provides clear UX.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
lib/domain/models/user_profile.dart (1)
129-143: Pre-computeformattedPublicKeyfor consistency withfromMetadata.The
fromMetadatafactory pre-computesformattedPublicKey(lines 67-72), butfromUsersets it to an empty string, causing on-demand computation via the getter on first access. This creates inconsistent performance characteristics between instances created by different factories.Apply this diff to pre-compute the formatted key:
factory UserProfile.fromUser(User user) { final npub = PubkeyFormatter(pubkey: user.publicKey).toNpub() ?? ''; + + String formattedKey; + try { + formattedKey = npub.formatPublicKey(); + } catch (e) { + formattedKey = user.publicKey.formatPublicKey(); + } return UserProfile( publicKey: npub, displayName: user.displayName, imagePath: user.imagePath, about: '', website: '', nip05: user.nip05, lud16: '', - formattedPublicKey: '', + formattedPublicKey: formattedKey, ); }lib/ui/chat/chat_info/add_group_members_screen.dart (3)
106-106: Use a descriptive variable name instead ofc.The single-letter variable name
cin the map operation reduces code clarity. According to coding guidelines, variables should have meaningful names.Apply this diff:
- final memberPubkeys = _selectedFollows.map((c) => c.publicKey).toList(); + final memberPubkeys = _selectedFollows.map((user) => user.publicKey).toList();
441-441: Define a named constant for the opacity value.The magic number
0.4should be defined as a named constant to improve code maintainability and make the intent clearer. As per coding guidelines: "Avoid magic numbers and define constants."Add a constant at the class level:
class _FollowsList extends StatelessWidget { static const double _existingMemberOpacity = 0.4; // ... rest of the classThen use it:
- return Opacity( - opacity: isExistingMember ? 0.4 : 1.0, + return Opacity( + opacity: isExistingMember ? _existingMemberOpacity : 1.0,
392-409: Consider pre-computing existing member hex pubkeys for better performance.The
_isExistingMembermethod is called for every item in the list on every build, resulting in O(n×m) complexity where n is the number of follows and m is the number of existing members. Each call also performs pubkey normalization viaPubkeyFormatter.Consider pre-computing a
Set<String>of existing member hex pubkeys once, then using O(1) lookups:class _FollowsList extends StatelessWidget { final List<User> follows; final Set<User> selectedFollows; final List<String> existingMemberPubkeys; final Function(User, bool) onToggleSelection; final String searchQuery; // Cache the hex pubkeys for O(1) lookups late final Set<String> _existingMemberHexPubkeys = _computeExistingMemberHexPubkeys(); const _FollowsList({ required this.follows, required this.selectedFollows, required this.existingMemberPubkeys, required this.onToggleSelection, required this.searchQuery, }); Set<String> _computeExistingMemberHexPubkeys() { return existingMemberPubkeys .map((pubkey) { try { return PubkeyFormatter(pubkey: pubkey).toHex()?.toLowerCase(); } catch (e) { return null; } }) .whereType<String>() .toSet(); } bool _isExistingMember(User user) { try { final userHexPubkey = PubkeyFormatter(pubkey: user.publicKey).toHex()?.toLowerCase(); return userHexPubkey != null && _existingMemberHexPubkeys.contains(userHexPubkey); } catch (e) { return false; } } // ... rest of the class
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
lib/domain/models/user_profile.dart(2 hunks)lib/ui/chat/chat_info/add_group_members_screen.dart(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
**/*.dart: Always declare the type of each variable and function (parameters and return value)
Avoid using dynamic and Object without justification
Create necessary types instead of overusing primitives or dynamic
One export per file
Use PascalCase for classes
Use camelCase for variables, functions, and methods
Avoid magic numbers and define constants
Start each function name with a verb
Use verbs for boolean variables (e.g., isLoading, hasError, canDelete)
Write short functions with a single purpose (under ~20 instructions)
Name functions with a verb plus context; for booleans use isX/hasX/canX; for void use executeX/saveX
Avoid deep nesting via early returns and extraction to utility functions
Use higher-order functions (map, where/filter, reduce) to avoid nesting
Use arrow functions for simple functions (under ~3 statements); use named functions otherwise
Use default parameter values instead of null checks
Reduce function parameters using RO-RO: pass/return parameter objects with declared types
Maintain a single level of abstraction within functions
Encapsulate data in composite types; avoid overusing primitives
Prefer validating data within classes rather than in functions
Prefer immutability; use final for runtime constants and const for compile-time constants
Use const constructors and const literals where possible
Follow SOLID principles
Prefer composition over inheritance
Declare interfaces (abstract classes) to define contracts
Write small classes with a single purpose (under ~200 instructions, <10 public methods, <10 properties)
Use exceptions for unexpected errors
Only catch exceptions to fix expected problems or add context; otherwise use a global handler
Files:
lib/ui/chat/chat_info/add_group_members_screen.dartlib/domain/models/user_profile.dart
lib/**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
lib/**/*.dart: Use flutter_rust_bridge to access core app functionality
Use Riverpod for state management; prefer StreamProviders for Rust API streams; use keepAlive if needed
Use freezed to model/manage UI states
Controllers should expose methods as inputs and update UI state that drives the UI
Use AutoRoute for navigation and use extras to pass data between pages
Use Dart extensions to manage reusable code
Use ThemeData to manage themes
Use AppLocalizations for translations
Use constants to manage constant values
Avoid deeply nested widget trees; aim for a flatter widget structure for performance and readability
Break down large widgets into smaller, focused, reusable components
Keep the widget tree shallow to simplify state management and data flow
Utilize const constructors and const widgets wherever possible to reduce rebuilds
Files:
lib/ui/chat/chat_info/add_group_members_screen.dartlib/domain/models/user_profile.dart
🧠 Learnings (3)
📚 Learning: 2025-09-07T13:10:16.542Z
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#597
File: lib/config/providers/group_provider.dart:311-314
Timestamp: 2025-09-07T13:10:16.542Z
Learning: In the whitenoise_flutter codebase, the User class used in group_provider.dart (and similar contexts) is presentational only, not the actual user class from the Rust API. There are plans to remove this User class and replace it with UserProfileData, similar to the planned consolidation with ContactModel.
Applied to files:
lib/domain/models/user_profile.dart
📚 Learning: 2025-09-03T20:57:53.202Z
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#594
File: lib/config/providers/user_profile_data_provider.dart:0-0
Timestamp: 2025-09-03T20:57:53.202Z
Learning: In the whitenoise_flutter codebase, pubkey normalization (npub/hex format handling) is implemented in a pubkey validations extension rather than being imported directly into individual providers like user_profile_data_provider.dart.
Applied to files:
lib/domain/models/user_profile.dart
📚 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/domain/models/user_profile.dart
🔇 Additional comments (4)
lib/domain/models/user_profile.dart (1)
131-131: Declarenpubasfinal.Per coding guidelines, prefer immutability by using
finalfor runtime constants.Apply this diff:
- final npub = PubkeyFormatter(pubkey: user.publicKey).toNpub() ?? ''; + final npub = PubkeyFormatter(pubkey: user.publicKey).toNpub() ?? '';Wait, that's already
final. Let me check again... Actually, looking at line 131, there's nofinalkeyword shown. Apply this diff:- final npub = PubkeyFormatter(pubkey: user.publicKey).toNpub() ?? ''; + final npub = PubkeyFormatter(pubkey: user.publicKey).toNpub() ?? '';Actually, I need to look at the code more carefully. The line shows:
final npub = PubkeyFormatter(pubkey: user.publicKey).toNpub() ?? '';There IS a
finalkeyword. So this is already correct. I should skip this comment.lib/ui/chat/chat_info/add_group_members_screen.dart (3)
58-78: LGTM: Search normalization correctly handles npub inputs.The search handler properly removes whitespace for npub keys and updates the controller to maintain cursor position. The comment now accurately reflects the implementation (checking only for 'npub' prefix).
92-130: LGTM: Member addition logic is robust with proper error handling.The function correctly validates the active account, invokes the Rust API, reloads group data, and handles errors gracefully. The loading state is properly managed via the
finallyblock, which executes even on early returns. The locale key issue from the previous review has been corrected.
42-42: ****The
Userclass inlib/domain/models/user_model.dartproperly implements bothoperator ==andhashCode, comparing all fields and usingObject.hash()respectively. This meansSet<User>operations will work correctly—twoUserinstances created from identical metadata will be equal and recognized bycontains(), even across rebuilds. The concern about selection logic breaking due to identity equality is unfounded.Likely an incorrect or invalid review comment.
…/parres-hq/whitenoise_flutter into add-and-remove-member-from-group
josefinalliende
left a comment
josefinalliende
left a comment
There was a problem hiding this comment.
Cool! Tried it locally in my iPhone and works perfect! ✨ Left a few comments
| final activeAccount = ref.watch(activePubkeyProvider); | ||
| final follows = followsState.follows; | ||
| final followsModel = follows.map( | ||
| (follow) => User.fromMetadata(follow.metadata, follow.pubkey), |
There was a problem hiding this comment.
Follows in follows provider are already User objects, so this conversion shouldn't me necessary... 🤔
There was a problem hiding this comment.
There are two user classes unfortunately. One in domain/models (used in group impls) and one in rust/api/ (used in follows). So yes, the conversion is necessary. We definitely need to look into possibly merging those two classes though.
Let me check and stick to one
There was a problem hiding this comment.
There are two user classes unfortunately. One in domain/models (used in group impls) and one in rust/api/ (used in follows). So yes, the conversion is necessary. We definitely need to look into possibly merging those two classes though.
Let me check and stick to one
Ah ok So I get it now, I got confused.
There was a problem hiding this comment.
I created this issue for the 2 User classes #749
lib/domain/models/user_profile.dart
Outdated
| } | ||
|
|
||
| /// Convert from User to UserProfile | ||
| factory UserProfile.fromUser(User user) { |
There was a problem hiding this comment.
I think UserProfile.fromMetadata is safer, as it sanitizes text and uses the user metadata. I guess method could be a wrapper for the fromMetadata but just receiving the user. Then passing user.npub and user.metadata ... WDYT?
There was a problem hiding this comment.
Would be nice but this user obj doesn't have metadata.
There was a problem hiding this comment.
ah ok! Maybe I got confused between the 2 kinds of users you mentioned above? I aggree that yes, we need to stop having the same name for both.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
CHANGELOG.md(1 hunks)lib/domain/models/user_profile.dart(2 hunks)lib/locales/en.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- CHANGELOG.md
🧰 Additional context used
📓 Path-based instructions (2)
**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
**/*.dart: Always declare the type of each variable and function (parameters and return value)
Avoid using dynamic and Object without justification
Create necessary types instead of overusing primitives or dynamic
One export per file
Use PascalCase for classes
Use camelCase for variables, functions, and methods
Avoid magic numbers and define constants
Start each function name with a verb
Use verbs for boolean variables (e.g., isLoading, hasError, canDelete)
Write short functions with a single purpose (under ~20 instructions)
Name functions with a verb plus context; for booleans use isX/hasX/canX; for void use executeX/saveX
Avoid deep nesting via early returns and extraction to utility functions
Use higher-order functions (map, where/filter, reduce) to avoid nesting
Use arrow functions for simple functions (under ~3 statements); use named functions otherwise
Use default parameter values instead of null checks
Reduce function parameters using RO-RO: pass/return parameter objects with declared types
Maintain a single level of abstraction within functions
Encapsulate data in composite types; avoid overusing primitives
Prefer validating data within classes rather than in functions
Prefer immutability; use final for runtime constants and const for compile-time constants
Use const constructors and const literals where possible
Follow SOLID principles
Prefer composition over inheritance
Declare interfaces (abstract classes) to define contracts
Write small classes with a single purpose (under ~200 instructions, <10 public methods, <10 properties)
Use exceptions for unexpected errors
Only catch exceptions to fix expected problems or add context; otherwise use a global handler
Files:
lib/domain/models/user_profile.dart
lib/**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
lib/**/*.dart: Use flutter_rust_bridge to access core app functionality
Use Riverpod for state management; prefer StreamProviders for Rust API streams; use keepAlive if needed
Use freezed to model/manage UI states
Controllers should expose methods as inputs and update UI state that drives the UI
Use AutoRoute for navigation and use extras to pass data between pages
Use Dart extensions to manage reusable code
Use ThemeData to manage themes
Use AppLocalizations for translations
Use constants to manage constant values
Avoid deeply nested widget trees; aim for a flatter widget structure for performance and readability
Break down large widgets into smaller, focused, reusable components
Keep the widget tree shallow to simplify state management and data flow
Utilize const constructors and const widgets wherever possible to reduce rebuilds
Files:
lib/domain/models/user_profile.dart
🧠 Learnings (4)
📚 Learning: 2025-09-07T13:10:16.542Z
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#597
File: lib/config/providers/group_provider.dart:311-314
Timestamp: 2025-09-07T13:10:16.542Z
Learning: In the whitenoise_flutter codebase, the User class used in group_provider.dart (and similar contexts) is presentational only, not the actual user class from the Rust API. There are plans to remove this User class and replace it with UserProfileData, similar to the planned consolidation with ContactModel.
Applied to files:
lib/domain/models/user_profile.dart
📚 Learning: 2025-09-03T20:57:53.202Z
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#594
File: lib/config/providers/user_profile_data_provider.dart:0-0
Timestamp: 2025-09-03T20:57:53.202Z
Learning: In the whitenoise_flutter codebase, pubkey normalization (npub/hex format handling) is implemented in a pubkey validations extension rather than being imported directly into individual providers like user_profile_data_provider.dart.
Applied to files:
lib/domain/models/user_profile.dart
📚 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/domain/models/user_profile.dart
📚 Learning: 2025-09-07T02:08:09.199Z
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#594
File: lib/domain/models/contact_model.dart:36-41
Timestamp: 2025-09-07T02:08:09.199Z
Learning: The issue of ContactModel.publicKey potentially being set to an empty string when pubkey.toNpub() conversion fails (causing collision/lookup issues) is being addressed in PR #597 in the whitenoise_flutter codebase.
Applied to files:
lib/domain/models/user_profile.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 (1)
lib/locales/en.json (1)
297-302: Locale keys properly formatted and consistent with existing patterns.The trailing comma addition on line 297 correctly separates properties, and the five new keys (
addMembers,membersAddedSuccess,failedToAddMembers,addingMembers,selectedCount) follow the established camelCase naming convention and placeholder syntax ({count}). JSON structure is valid.
lib/domain/models/user_profile.dart
Outdated
| /// Convert from User to UserProfile | ||
| factory UserProfile.fromUser(User user) { | ||
| final npub = PubkeyFormatter(pubkey: user.publicKey).toNpub() ?? ''; | ||
|
|
||
| return UserProfile( | ||
| publicKey: npub, | ||
| displayName: user.displayName, | ||
| imagePath: user.imagePath, | ||
| about: '', | ||
| website: '', | ||
| nip05: user.nip05, | ||
| lud16: '', | ||
| formattedPublicKey: '', | ||
| ); | ||
| } |
There was a problem hiding this comment.
Apply input sanitization and add displayName fallback.
The fromUser factory lacks the sanitization and validation logic present in fromMetadata. This creates inconsistent data quality guarantees depending on which factory is used:
- No input sanitization:
displayName,imagePath, andnip05are assigned directly without sanitization (no trimming, whitespace normalization, or URL validation). This could lead to UI rendering issues with malformed data. - No fallback for empty displayName: If
user.displayNameis empty, it's used as-is, unlikefromMetadatawhich falls back to"shared.unknownUser".tr(). - Empty formattedPublicKey: Setting this to
''forces recomputation on every getter access (lines 30-41), whereasfromMetadatapre-computes it once.
Based on learnings and josefinalliende's suggestion, consider refactoring fromUser to leverage fromMetadata as a wrapper. However, since User lacks a metadata field, apply similar sanitization logic directly:
factory UserProfile.fromUser(User user) {
final npub = PubkeyFormatter(pubkey: user.publicKey).toNpub() ?? '';
+
+ // Sanitize inputs using the same helpers as fromMetadata
+ final sanitizedDisplayName = _sanitizeString(user.displayName);
+ final sanitizedImagePath = _sanitizeUrl(user.imagePath);
+ final sanitizedNip05 = _sanitizeString(user.nip05);
+
+ // Fallback for empty displayName
+ final finalDisplayName = sanitizedDisplayName.isNotEmpty
+ ? sanitizedDisplayName
+ : 'shared.unknownUser'.tr();
+
+ // Pre-compute formatted key
+ String formattedKey;
+ try {
+ formattedKey = npub.formatPublicKey();
+ } catch (e) {
+ formattedKey = user.publicKey.formatPublicKey();
+ }
return UserProfile(
publicKey: npub,
- displayName: user.displayName,
- imagePath: user.imagePath,
+ displayName: finalDisplayName,
+ imagePath: sanitizedImagePath,
about: '',
website: '',
- nip05: user.nip05,
+ nip05: sanitizedNip05,
lud16: '',
- formattedPublicKey: '',
+ formattedPublicKey: formattedKey,
);
}🤖 Prompt for AI Agents
In lib/domain/models/user_profile.dart around lines 129 to 143, the fromUser
factory assigns fields directly without the sanitization and fallbacks used in
fromMetadata: trim and normalize displayName (and fallback to
"shared.unknownUser".tr() when empty), trim and validate imagePath (ensure valid
URL or leave empty), trim nip05 and validate format, compute and store
formattedPublicKey once using PubkeyFormatter(pubkey:
user.publicKey).toFormatted() (or equivalent) instead of setting it to '', and
set about, website, lud16 to safe defaults (empty strings) — apply the same
sanitization logic as fromMetadata (whitespace trimming, basic URL/nip05
validation) and return a UserProfile constructed with these sanitized values so
behavior matches fromMetadata.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/ui/chat/chat_info/add_group_members_screen.dart(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
**/*.dart: Always declare the type of each variable and function (parameters and return value)
Avoid using dynamic and Object without justification
Create necessary types instead of overusing primitives or dynamic
One export per file
Use PascalCase for classes
Use camelCase for variables, functions, and methods
Avoid magic numbers and define constants
Start each function name with a verb
Use verbs for boolean variables (e.g., isLoading, hasError, canDelete)
Write short functions with a single purpose (under ~20 instructions)
Name functions with a verb plus context; for booleans use isX/hasX/canX; for void use executeX/saveX
Avoid deep nesting via early returns and extraction to utility functions
Use higher-order functions (map, where/filter, reduce) to avoid nesting
Use arrow functions for simple functions (under ~3 statements); use named functions otherwise
Use default parameter values instead of null checks
Reduce function parameters using RO-RO: pass/return parameter objects with declared types
Maintain a single level of abstraction within functions
Encapsulate data in composite types; avoid overusing primitives
Prefer validating data within classes rather than in functions
Prefer immutability; use final for runtime constants and const for compile-time constants
Use const constructors and const literals where possible
Follow SOLID principles
Prefer composition over inheritance
Declare interfaces (abstract classes) to define contracts
Write small classes with a single purpose (under ~200 instructions, <10 public methods, <10 properties)
Use exceptions for unexpected errors
Only catch exceptions to fix expected problems or add context; otherwise use a global handler
Files:
lib/ui/chat/chat_info/add_group_members_screen.dart
lib/**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
lib/**/*.dart: Use flutter_rust_bridge to access core app functionality
Use Riverpod for state management; prefer StreamProviders for Rust API streams; use keepAlive if needed
Use freezed to model/manage UI states
Controllers should expose methods as inputs and update UI state that drives the UI
Use AutoRoute for navigation and use extras to pass data between pages
Use Dart extensions to manage reusable code
Use ThemeData to manage themes
Use AppLocalizations for translations
Use constants to manage constant values
Avoid deeply nested widget trees; aim for a flatter widget structure for performance and readability
Break down large widgets into smaller, focused, reusable components
Keep the widget tree shallow to simplify state management and data flow
Utilize const constructors and const widgets wherever possible to reduce rebuilds
Files:
lib/ui/chat/chat_info/add_group_members_screen.dart
🧠 Learnings (1)
📚 Learning: 2025-09-07T13:10:16.542Z
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#597
File: lib/config/providers/group_provider.dart:311-314
Timestamp: 2025-09-07T13:10:16.542Z
Learning: In the whitenoise_flutter codebase, the User class used in group_provider.dart (and similar contexts) is presentational only, not the actual user class from the Rust API. There are plans to remove this User class and replace it with UserProfileData, similar to the planned consolidation with ContactModel.
Applied to files:
lib/ui/chat/chat_info/add_group_members_screen.dart
🪛 GitHub Actions: CI
lib/ui/chat/chat_info/add_group_members_screen.dart
[error] 1-1: Dart formatter reported changes during 'dart format --set-exit-if-changed lib/ integration_test/'. Exit code 1 indicates formatting changes were required (formatted 230 files, 1 changed).
| return availableFollows | ||
| .where( | ||
| (user) { | ||
| final displayName = user.metadata.displayName ?? user.metadata.name ?? ''; | ||
| final nip05 = user.metadata.nip05 ?? ''; | ||
| return displayName.toLowerCase().contains( | ||
| _searchQuery.toLowerCase(), | ||
| ) || | ||
| nip05.toLowerCase().contains( | ||
| _searchQuery.toLowerCase(), | ||
| ) || | ||
| user.pubkey.toLowerCase().contains( | ||
| _searchQuery.toLowerCase(), | ||
| ); | ||
| }, | ||
| ) | ||
| .toList(); |
There was a problem hiding this comment.
Convert npub queries before filtering.
We strip whitespace from npub input but still compare the raw npub… string against hex pubkeys, so searching by a pasted NIP‐19 key always yields zero matches. Normalize the query once (trim/lowercase), convert npub to hex, and include that hex in the comparisons so both formats work.
@@
- if (_searchQuery.isEmpty) return availableFollows;
+ final normalizedQuery = _searchQuery.trim();
+ if (normalizedQuery.isEmpty) return availableFollows;
+
+ final lowerQuery = normalizedQuery.toLowerCase();
+ String? hexQuery;
+ if (normalizedQuery.startsWith('npub')) {
+ try {
+ hexQuery = PubkeyFormatter(pubkey: normalizedQuery).toHex()?.toLowerCase();
+ } catch (_) {
+ hexQuery = null;
+ }
+ }
return availableFollows
.where(
(user) {
final displayName = user.metadata.displayName ?? user.metadata.name ?? '';
final nip05 = user.metadata.nip05 ?? '';
- return displayName.toLowerCase().contains(
- _searchQuery.toLowerCase(),
- ) ||
- nip05.toLowerCase().contains(
- _searchQuery.toLowerCase(),
- ) ||
- user.pubkey.toLowerCase().contains(
- _searchQuery.toLowerCase(),
- );
+ final pubkeyLower = user.pubkey.toLowerCase();
+ return displayName.toLowerCase().contains(lowerQuery) ||
+ nip05.toLowerCase().contains(lowerQuery) ||
+ pubkeyLower.contains(lowerQuery) ||
+ (hexQuery != null && pubkeyLower.contains(hexQuery));
},
)
.toList();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return availableFollows | |
| .where( | |
| (user) { | |
| final displayName = user.metadata.displayName ?? user.metadata.name ?? ''; | |
| final nip05 = user.metadata.nip05 ?? ''; | |
| return displayName.toLowerCase().contains( | |
| _searchQuery.toLowerCase(), | |
| ) || | |
| nip05.toLowerCase().contains( | |
| _searchQuery.toLowerCase(), | |
| ) || | |
| user.pubkey.toLowerCase().contains( | |
| _searchQuery.toLowerCase(), | |
| ); | |
| }, | |
| ) | |
| .toList(); | |
| // Normalize the search query and early-return if empty | |
| final normalizedQuery = _searchQuery.trim(); | |
| if (normalizedQuery.isEmpty) return availableFollows; | |
| // Prepare lowercase query and optional hex conversion for npub… | |
| final lowerQuery = normalizedQuery.toLowerCase(); | |
| String? hexQuery; | |
| if (normalizedQuery.startsWith('npub')) { | |
| try { | |
| hexQuery = PubkeyFormatter(pubkey: normalizedQuery) | |
| .toHex() | |
| ?.toLowerCase(); | |
| } catch (_) { | |
| hexQuery = null; | |
| } | |
| } | |
| return availableFollows | |
| .where( | |
| (user) { | |
| final displayName = | |
| user.metadata.displayName ?? user.metadata.name ?? ''; | |
| final nip05 = user.metadata.nip05 ?? ''; | |
| final pubkeyLower = user.pubkey.toLowerCase(); | |
| return displayName.toLowerCase().contains(lowerQuery) || | |
| nip05.toLowerCase().contains(lowerQuery) || | |
| pubkeyLower.contains(lowerQuery) || | |
| (hexQuery != null && pubkeyLower.contains(hexQuery)); | |
| }, | |
| ) | |
| .toList(); |
🤖 Prompt for AI Agents
In lib/ui/chat/chat_info/add_group_members_screen.dart around lines 148–164, the
search compares the raw _searchQuery against hex pubkeys but doesn’t convert
pasted NIP-19 npub values, so npub searches fail; normalize the query once
(trim() and toLowerCase()), detect/convert an npub/NIP-19 input to its hex
pubkey (using the existing nip19/npub utility or add a small helper), and
include that converted hexPub in the per-user comparisons (check displayName,
nip05, user.pubkey, and the converted hexPub). Cache the converted hex so you
don’t decode per user and handle conversion errors by ignoring the hex fallback
if conversion fails.
|
When I go to a chat from the chat list, it scrolls to the bottom, when I go to a chat from Member > Send Message, I see the top of the chat. Screen.Recording.2025-10-22.at.23.24.28.mov |
josefinalliende
left a comment
josefinalliende
left a comment
There was a problem hiding this comment.
LGTM! ✅ I think the scroll issue that @untreu2 noticed (good catch) can be fixed in a separate issue. I checked and that happens in master too.
Description
Fixes #744
I'll check the recent commits to generate a PR description.
Add Group Member Management Functionality
This PR implements the ability to add members to existing groups with a dedicated multi-select interface.
Features Added
Add Members Screen
add_group_members_screen.dartFiles Changed
lib/ui/chat/chat_info/add_group_members_screen.dartlib/domain/models/user_profile.dart- Added factory methodlib/ui/chat/chat_info/group_chat_info.dart- Added navigation to add members screenlib/routing/router_provider.dart&routes.dart- Added new routeTesting
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)Summary by CodeRabbit
New Features
Localization