-
Notifications
You must be signed in to change notification settings - Fork 13
Patch group profile image #734
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughAdds cached group image path support: state gains a Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant Provider as GroupsNotifier
participant State as GroupsState
participant UI as UI Components
App->>Provider: setGroups(groups)
activate Provider
Provider->>Provider: _loadGroupImagePaths(groups) (parallel)
Note over Provider: fetch image path per group<br/>catch/log errors, continue
Provider->>State: update groupImagePaths map
deactivate Provider
UI->>Provider: getCachedGroupImagePath(groupId)
activate Provider
Provider-->>UI: return path or null
deactivate Provider
UI->>UI: render avatar with path (or fallback empty)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
lib/ui/chat/chat_info/group_chat_info.dart (1)
130-130: Consider usingref.readinstead ofref.watchfor notifier access.Since you only need the notifier's methods and not reactive updates (the
ref.listenon line 131 handles updates), you could optimize this line to useref.read(groupsProvider.notifier)instead ofref.watch(groupsProvider.notifier).- final groupsNotifier = ref.watch(groupsProvider.notifier); + final groupsNotifier = ref.read(groupsProvider.notifier);
📜 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 (6)
lib/config/providers/group_provider.dart(6 hunks)lib/config/states/group_state.dart(1 hunks)lib/config/states/group_state.freezed.dart(14 hunks)lib/ui/chat/chat_info/group_chat_info.dart(3 hunks)lib/ui/chat/chat_screen.dart(1 hunks)lib/ui/chat/widgets/chat_header_widget.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/config/states/group_state.freezed.dartlib/config/states/group_state.dartlib/config/providers/group_provider.dartlib/ui/chat/widgets/chat_header_widget.dartlib/ui/chat/chat_info/group_chat_info.dartlib/ui/chat/chat_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/config/states/group_state.freezed.dartlib/config/states/group_state.dartlib/config/providers/group_provider.dartlib/ui/chat/widgets/chat_header_widget.dartlib/ui/chat/chat_info/group_chat_info.dartlib/ui/chat/chat_screen.dart
🧠 Learnings (1)
📚 Learning: 2025-09-14T21:22:00.962Z
Learnt from: Quwaysim
PR: parres-hq/whitenoise_flutter#634
File: lib/config/providers/group_provider.dart:1130-1132
Timestamp: 2025-09-14T21:22:00.962Z
Learning: In the whitenoise_flutter codebase, the _updateGroupInfo method in GroupsNotifier performs optimistic updates that directly modify Group objects in the provider state (groups list and groupsMap). For non-DM groups, the display name comes from group.name, so updating the Group object directly is sufficient to reflect name changes in the UI without needing to refresh the separate display name cache.
Applied to files:
lib/ui/chat/chat_info/group_chat_info.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 (11)
lib/config/states/group_state.dart (1)
16-16: LGTM!The new
groupImagePathsfield follows the established pattern for caching group-related data. The type annotation and inline comment are clear and consistent with other similar fields in the state.lib/ui/chat/chat_screen.dart (1)
385-388: LGTM!The change correctly retrieves the cached group image path for non-DM groups with an appropriate fallback to an empty string when the path is null.
lib/ui/chat/widgets/chat_header_widget.dart (1)
73-73: LGTM!The changes correctly retrieve and use the cached group image path. The
groupsNotifieris properly obtained viaref.watch, and the avatar uses the cached path with an appropriate fallback to an empty string.Also applies to: 80-84
lib/ui/chat/chat_info/group_chat_info.dart (2)
18-18: LGTM!The
groupImagePathfield is properly declared as a nullable String to hold the cached image path.
151-151: LGTM!The avatar correctly uses
groupImagePathwith an appropriate fallback. Once thesetStateissue on line 132 is fixed, this will properly display the cached group image.lib/config/providers/group_provider.dart (5)
167-168: LGTM!The call to
_loadGroupImagePathsis properly placed in the initialization sequence, parallel to other group data loading operations.
628-676: LGTM!The
_loadGroupImagePathsmethod follows the established pattern from_loadGroupTypesForAllGroups. It correctly:
- Loads image paths in parallel for better performance
- Handles errors gracefully without blocking the UI
- Updates state only after all loads complete
- Logs appropriate info and error messages
861-865: LGTM!The
getCachedGroupImagePathgetter is well-implemented with clear documentation. It follows the same pattern asgetCachedGroupTypeand properly handles null cases.
957-958: LGTM!The call to
_loadGroupImagePathsfor newly detected groups is properly placed and consistent with the initial loading pattern.
1313-1335: LGTM!The
getGroupDisplayImagemethod is correctly updated to return the cached group image path for regular groups. The comment on line 1313 accurately describes the new behavior.lib/config/states/group_state.freezed.dart (1)
30-31: LGTM!This is generated code from freezed that properly includes the new
groupImagePathsfield in all necessary locations: mixin getter, copyWith interfaces/implementations, constructor, private storage, public accessor, toString, equality, and hashCode.Also applies to: 55-55, 82-82, 118-122, 154-154, 180-180, 216-220, 246-246, 254-255, 326-335, 346-346, 375-378, 392-392, 414-414, 432-432
erskingardner
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
lib/ui/chat/chat_info/chat_info_screen.dart(1 hunks)lib/ui/chat/chat_info/group_chat_info.dart(3 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/group_chat_info.dartlib/ui/chat/chat_info/chat_info_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/group_chat_info.dartlib/ui/chat/chat_info/chat_info_screen.dart
🧠 Learnings (2)
📚 Learning: 2025-09-14T21:22:00.962Z
Learnt from: Quwaysim
PR: parres-hq/whitenoise_flutter#634
File: lib/config/providers/group_provider.dart:1130-1132
Timestamp: 2025-09-14T21:22:00.962Z
Learning: In the whitenoise_flutter codebase, the _updateGroupInfo method in GroupsNotifier performs optimistic updates that directly modify Group objects in the provider state (groups list and groupsMap). For non-DM groups, the display name comes from group.name, so updating the Group object directly is sufficient to reflect name changes in the UI without needing to refresh the separate display name cache.
Applied to files:
lib/ui/chat/chat_info/group_chat_info.dartlib/ui/chat/chat_info/chat_info_screen.dart
📚 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/chat_info_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 (5)
lib/ui/chat/chat_info/chat_info_screen.dart (1)
12-12: LGTM!The import is necessary for the part file
group_chat_info.dartto access theGroupsStatetype used in theProviderSubscription<GroupsState>declaration.lib/ui/chat/chat_info/group_chat_info.dart (4)
18-19: LGTM!The state field declarations follow proper naming conventions and use appropriate nullable types.
29-31: LGTM!The initial load of
groupImagePathis correctly wrapped insetStateand usesref.readappropriately withininitState.
41-45: LGTM!The
disposemethod properly closes the subscription before callingsuper.dispose(), ensuring correct cleanup of the manual listener.
164-164: LGTM!The avatar correctly uses the dynamic
groupImagePathwith a safe fallback to an empty string when the path is null.
| _groupsSubscription = ref.listenManual(groupsProvider, (previous, next) { | ||
| if (mounted) { | ||
| _loadMembers(); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update groupImagePath when the provider changes.
The listener only calls _loadMembers() but doesn't update groupImagePath when the groups provider changes. Since group images are loaded in parallel (per the PR description), the cached image path may become available after initialization, and the UI won't reflect the updated image until another rebuild occurs.
Apply this diff to update the image path when the provider changes:
_groupsSubscription = ref.listenManual(groupsProvider, (previous, next) {
if (mounted) {
+ setState(() {
+ groupImagePath = ref.read(groupsProvider.notifier).getCachedGroupImagePath(widget.groupId);
+ });
_loadMembers();
}
});🤖 Prompt for AI Agents
In lib/ui/chat/chat_info/group_chat_info.dart around lines 33 to 37, the
groupsProvider listener only calls _loadMembers() and does not refresh
groupImagePath when provider data changes; update the listener to also refresh
the cached image path by retrieving the current group's image path from the
provider and assigning it to groupImagePath inside a mounted setState (or call
an existing helper that updates the image), ensuring the UI re-renders when the
provider supplies a newly available image path.
Description
Add group profile image support
Implements automatic loading and display of group profile images across the app.
What's Changed
Backend Integration
groupImagePathsmap toGroupsStateto cache image file paths for all groups_loadGroupImagePaths()method that fetches images usinggetGroupImagePathAPIgetCachedGroupImagePath()getter for synchronous access to cached image pathsUI Updates
Technical Details
getGroupDisplayImage()helper now returns cached group images for regular groupsFiles Changed
lib/config/providers/group_provider.dart- Added image loading logiclib/config/states/group_state.dart- AddedgroupImagePathsfieldlib/ui/chat/chat_screen.dart- Display group images in chat headerlib/ui/chat/chat_info/group_chat_info.dart- Display group images on info pagelib/ui/chat/widgets/chat_header_widget.dart- Updated to support group imagesType 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
Bug Fixes / Improvements