[FOSSOVERFLOW-26] chat and call features have been completed#10
Open
Sri-Varshith wants to merge 5 commits intoOpenLake:mainfrom
Open
[FOSSOVERFLOW-26] chat and call features have been completed#10Sri-Varshith wants to merge 5 commits intoOpenLake:mainfrom
Sri-Varshith wants to merge 5 commits intoOpenLake:mainfrom
Conversation
WalkthroughThis PR introduces meeting type differentiation (chat vs. video) to a Flutter scheduling application. The Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Page as OnlineMeetPage/<br/>ScheduledMeetingsPage
participant Card as MeetingCard
participant Nav as Navigation
participant Chat as DoctorChatPage
participant Jitsi as Jitsi Meet
User->>Page: Tap "Schedule Meeting"
Page->>Page: Open schedule sheet
User->>Page: Enter title, select type (chat/video)
User->>Page: Submit
Page->>Page: Create Meeting with meetingType
Page->>Card: Render MeetingCard with onJoin
User->>Card: Tap "Join" button
Card->>Card: Check meetingType via isChat
alt Meeting Type: Chat
Card->>Nav: onJoin callback
Nav->>Chat: Navigate to DoctorChatPage
Chat->>User: Display chat interface
else Meeting Type: Video
Card->>Nav: onJoin callback
Nav->>Jitsi: Launch Jitsi URL (meeting.jitsiRoom)
Jitsi->>User: Open video conference
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
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)
📝 Coding Plan for PR comments
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: 4
🧹 Nitpick comments (5)
flutter_app/lib/pages/doctor_chat_page.dart (2)
15-20: Consider using a typed model instead ofMap<String, dynamic>.Using a simple data class improves type safety and reduces runtime cast errors. This is a minor improvement that can be deferred.
♻️ Suggested typed model
class ChatMessage { final String text; final bool isUser; const ChatMessage({required this.text, required this.isUser}); }Then use
List<ChatMessage>instead ofList<Map<String, dynamic>>.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@flutter_app/lib/pages/doctor_chat_page.dart` around lines 15 - 20, Replace the untyped _messages List<Map<String, dynamic>> with a typed model: add a ChatMessage class (final String text; final bool isUser; const ChatMessage({required this.text, required this.isUser});) and change the _messages declaration to List<ChatMessage> initialized with ChatMessage(...) instances; update all usages in this file (references to _messages[n]['text'] or ['isUser']) to use the typed fields (.text and .isUser) so the code compiles and gains type safety.
113-128: Consider usingIconButtonfor better accessibility.
GestureDetectorlacks built-in semantics, focus handling, and ink splash feedback. AnIconButton(or wrapping withSemantics) provides better accessibility support for screen readers and keyboard navigation.♻️ Suggested improvement
- GestureDetector( - onTap: _sendMessage, - child: Container( - width: 46, - height: 46, - decoration: const BoxDecoration( - color: Color(0xFF4CAF50), - shape: BoxShape.circle, - ), - child: const Icon( - Icons.send_rounded, - color: Colors.black, - size: 20, - ), - ), - ), + IconButton( + onPressed: _sendMessage, + style: IconButton.styleFrom( + backgroundColor: const Color(0xFF4CAF50), + fixedSize: const Size(46, 46), + ), + icon: const Icon( + Icons.send_rounded, + color: Colors.black, + size: 20, + ), + ),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@flutter_app/lib/pages/doctor_chat_page.dart` around lines 113 - 128, Replace the GestureDetector used for sending messages with an IconButton to gain built-in semantics, focus handling, and ink splash feedback: swap the GestureDetector block (the widget that calls _sendMessage) for an IconButton with onPressed: _sendMessage, provide the same Icon(Icons.send_rounded) and size, add a tooltip string for screen readers, and preserve the circular background and dimensions by wrapping the IconButton in a Container/DecoratedBox with the existing BoxDecoration color and shape if needed so visual layout remains identical.flutter_app/lib/widgets/meeting_card.dart (2)
58-61:_displayStatusduplicates logic fromMeeting.displayStatus.The
Meetingmodel already provides adisplayStatusgetter with the same logic. Consider usingmeeting.displayStatusdirectly to maintain a single source of truth and avoid drift.♻️ Suggested fix
- String get _displayStatus { - if (meeting.isAttended && meeting.status == 'confirmed') return 'completed'; - return meeting.status; - } + String get _displayStatus => meeting.displayStatus;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@flutter_app/lib/widgets/meeting_card.dart` around lines 58 - 61, The _displayStatus getter duplicates logic already implemented on the Meeting model; replace uses of the local getter with meeting.displayStatus and remove the _displayStatus getter entirely so the widget consumes the single source of truth (refer to the _displayStatus getter and the Meeting.displayStatus getter in the Meeting model and change callers to use meeting.displayStatus).
157-159: Nestedifstatements can be combined for clarity.The consecutive
ifconditions create unusual nesting. Consider combining them.♻️ Suggested fix
- if (onCancel != null || onJoin != null) - if (_displayStatus != 'cancelled' && - _displayStatus != 'completed') ...[ + if ((onCancel != null || onJoin != null) && + _displayStatus != 'cancelled' && + _displayStatus != 'completed') ...[🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@flutter_app/lib/widgets/meeting_card.dart` around lines 157 - 159, Combine the nested ifs in the MeetingCard widget by merging the checks for handlers and status into a single conditional: replace the outer "if (onCancel != null || onJoin != null)" plus inner "if (_displayStatus != 'cancelled' && _displayStatus != 'completed')" with one combined condition that tests both (e.g., if ((onCancel != null || onJoin != null) && _displayStatus != 'cancelled' && _displayStatus != 'completed')) so the block that renders the action buttons is executed only when handlers exist and the status is not cancelled/completed.flutter_app/lib/models/meeting_model.dart (1)
8-9: Consider using enums forstatusandmeetingType.String literals for status (
'pending','confirmed', etc.) and meeting type ('video','chat') are scattered across multiple files, making typos easy and refactoring harder. Enums would provide compile-time safety. This can be deferred to a follow-up.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@flutter_app/lib/models/meeting_model.dart` around lines 8 - 9, Replace the String-typed status and meetingType fields in MeetingModel with two enums (e.g., enum MeetingStatus { pending, confirmed, rejected, completed } and enum MeetingType { video, chat }), change the MeetingModel fields from String status and String meetingType to MeetingStatus status and MeetingType meetingType, and update any serialization/deserialization helpers (fromJson/toJson, copyWith, constructors) inside MeetingModel to convert between enum values and their string representations; search for usages of status and meetingType across the codebase and update comparisons (e.g., status == 'pending') to use MeetingStatus.pending (and similarly for MeetingType) or use enum.name/value conversions where string interchange is required (e.g., API payloads).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@flutter_app/lib/models/meeting_model.dart`:
- Around line 33-34: The jitsiRoom getter builds a room name using the raw title
which can include characters invalid in URLs; update the jitsiRoom computed
property to sanitize/encode the title when id is empty (use Uri.encodeComponent
or a stricter slugify routine) so special characters, spaces and non-ASCII are
safely handled; ensure you apply the same replaceAll('-','-')/lowercasing after
encoding or generate a safe slug from title and use that in the returned string
from jitsiRoom.
In `@flutter_app/lib/pages/online_meet_page.dart`:
- Around line 413-425: When marking a meeting cancelled in the onCancel handler,
the new Meeting instance drops the original id (and thus jitsiRoom consistency);
update the onCancel block that mutates _allMeetings (and the Meeting constructor
call) to copy meeting.id (and meeting.jitsiRoom if present) into the new Meeting
so the identifier/room are preserved (mirror the same fix applied in
scheduled_meetings_page.dart); ensure you update the Meeting(...) invocation
inside setState in online_meet_page.dart to include id: meeting.id (and
jitsiRoom: meeting.jitsiRoom) while keeping the other fields and status:
'cancelled'.
- Around line 351-353: The _titleController.addListener currently calls the
outer setState causing full page rebuilds; remove that listener and any
references that call setState from the top-level State, and instead update the
title-related UI inside the sheet's StatefulBuilder by using setSheetState
(inside the sheet builder callback) to update border color or other sheet-local
visuals; ensure _titleController listeners only call setSheetState or are
removed entirely and dispose of the listener properly if you delete it.
In `@flutter_app/lib/pages/scheduled_meetings_page.dart`:
- Around line 77-89: The cancellation handler replaces a Meeting without
preserving its id, which can break jitsiRoom derivation; modify the onCancel
logic to preserve the original meeting.id (either by calling a new
Meeting.copyWith({status: 'cancelled'}) or by including id when constructing the
new Meeting) so _scheduledMeetings[index] keeps the same id; add a copyWith
method to the Meeting class (or update its constructor usage) and update the
onCancel block to use it (or pass meeting.id) to retain all original fields.
---
Nitpick comments:
In `@flutter_app/lib/models/meeting_model.dart`:
- Around line 8-9: Replace the String-typed status and meetingType fields in
MeetingModel with two enums (e.g., enum MeetingStatus { pending, confirmed,
rejected, completed } and enum MeetingType { video, chat }), change the
MeetingModel fields from String status and String meetingType to MeetingStatus
status and MeetingType meetingType, and update any serialization/deserialization
helpers (fromJson/toJson, copyWith, constructors) inside MeetingModel to convert
between enum values and their string representations; search for usages of
status and meetingType across the codebase and update comparisons (e.g., status
== 'pending') to use MeetingStatus.pending (and similarly for MeetingType) or
use enum.name/value conversions where string interchange is required (e.g., API
payloads).
In `@flutter_app/lib/pages/doctor_chat_page.dart`:
- Around line 15-20: Replace the untyped _messages List<Map<String, dynamic>>
with a typed model: add a ChatMessage class (final String text; final bool
isUser; const ChatMessage({required this.text, required this.isUser});) and
change the _messages declaration to List<ChatMessage> initialized with
ChatMessage(...) instances; update all usages in this file (references to
_messages[n]['text'] or ['isUser']) to use the typed fields (.text and .isUser)
so the code compiles and gains type safety.
- Around line 113-128: Replace the GestureDetector used for sending messages
with an IconButton to gain built-in semantics, focus handling, and ink splash
feedback: swap the GestureDetector block (the widget that calls _sendMessage)
for an IconButton with onPressed: _sendMessage, provide the same
Icon(Icons.send_rounded) and size, add a tooltip string for screen readers, and
preserve the circular background and dimensions by wrapping the IconButton in a
Container/DecoratedBox with the existing BoxDecoration color and shape if needed
so visual layout remains identical.
In `@flutter_app/lib/widgets/meeting_card.dart`:
- Around line 58-61: The _displayStatus getter duplicates logic already
implemented on the Meeting model; replace uses of the local getter with
meeting.displayStatus and remove the _displayStatus getter entirely so the
widget consumes the single source of truth (refer to the _displayStatus getter
and the Meeting.displayStatus getter in the Meeting model and change callers to
use meeting.displayStatus).
- Around line 157-159: Combine the nested ifs in the MeetingCard widget by
merging the checks for handlers and status into a single conditional: replace
the outer "if (onCancel != null || onJoin != null)" plus inner "if
(_displayStatus != 'cancelled' && _displayStatus != 'completed')" with one
combined condition that tests both (e.g., if ((onCancel != null || onJoin !=
null) && _displayStatus != 'cancelled' && _displayStatus != 'completed')) so the
block that renders the action buttons is executed only when handlers exist and
the status is not cancelled/completed.
🪄 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: acb26af3-3865-4342-b011-246fdd60324b
📒 Files selected for processing (6)
flutter_app/lib/models/meeting_model.dartflutter_app/lib/pages/doctor_chat_page.dartflutter_app/lib/pages/online_meet_page.dartflutter_app/lib/pages/scheduled_meetings_page.dartflutter_app/lib/widgets/meeting_card.dartflutter_app/lib/widgets/meeting_request_card.dart
Comment on lines
+33
to
+34
| // Unique Jitsi room name derived from meeting id | ||
| String get jitsiRoom => 'getwelplus-${id.isEmpty ? title.replaceAll(' ', '-').toLowerCase() : id}'; |
There was a problem hiding this comment.
jitsiRoom may produce invalid URL paths with special characters.
If title contains characters like &, ?, #, or non-ASCII characters, the generated room name could break the URL or cause unexpected behavior. Consider sanitizing the title more thoroughly or using Uri.encodeComponent.
🛡️ Suggested fix
- String get jitsiRoom => 'getwelplus-${id.isEmpty ? title.replaceAll(' ', '-').toLowerCase() : id}';
+ String get jitsiRoom {
+ if (id.isNotEmpty) return 'getwelplus-$id';
+ final slug = title
+ .toLowerCase()
+ .replaceAll(RegExp(r'[^a-z0-9\s-]'), '')
+ .replaceAll(RegExp(r'\s+'), '-');
+ return 'getwelplus-$slug';
+ }📝 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.
Suggested change
| // Unique Jitsi room name derived from meeting id | |
| String get jitsiRoom => 'getwelplus-${id.isEmpty ? title.replaceAll(' ', '-').toLowerCase() : id}'; | |
| // Unique Jitsi room name derived from meeting id | |
| String get jitsiRoom { | |
| if (id.isNotEmpty) return 'getwelplus-$id'; | |
| final slug = title | |
| .toLowerCase() | |
| .replaceAll(RegExp(r'[^a-z0-9\s-]'), '') | |
| .replaceAll(RegExp(r'\s+'), '-'); | |
| return 'getwelplus-$slug'; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@flutter_app/lib/models/meeting_model.dart` around lines 33 - 34, The
jitsiRoom getter builds a room name using the raw title which can include
characters invalid in URLs; update the jitsiRoom computed property to
sanitize/encode the title when id is empty (use Uri.encodeComponent or a
stricter slugify routine) so special characters, spaces and non-ASCII are safely
handled; ensure you apply the same replaceAll('-','-')/lowercasing after
encoding or generate a safe slug from title and use that in the returned string
from jitsiRoom.
Comment on lines
351
to
+353
| _titleController.addListener(() { | ||
| setState(() {}); // rebuilds when title text changes | ||
| }); | ||
| setState(() {}); | ||
| }); |
There was a problem hiding this comment.
Unnecessary setState triggered by title controller listener.
This listener triggers a full page rebuild whenever the title text changes. However, the title field is inside the StatefulBuilder sheet which uses setSheetState. The outer setState serves no purpose here and causes unnecessary rebuilds of the entire page.
🛡️ Suggested fix: Remove the listener
`@override`
void initState() {
super.initState();
_tabController = TabController(length: 2, vsync: this);
- _titleController.addListener(() {
- setState(() {});
- });
}If the border color update is needed, handle it within setSheetState in the sheet builder.
📝 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.
Suggested change
| _titleController.addListener(() { | |
| setState(() {}); // rebuilds when title text changes | |
| }); | |
| setState(() {}); | |
| }); | |
| `@override` | |
| void initState() { | |
| super.initState(); | |
| _tabController = TabController(length: 2, vsync: this); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@flutter_app/lib/pages/online_meet_page.dart` around lines 351 - 353, The
_titleController.addListener currently calls the outer setState causing full
page rebuilds; remove that listener and any references that call setState from
the top-level State, and instead update the title-related UI inside the sheet's
StatefulBuilder by using setSheetState (inside the sheet builder callback) to
update border color or other sheet-local visuals; ensure _titleController
listeners only call setSheetState or are removed entirely and dispose of the
listener properly if you delete it.
Comment on lines
+413
to
+425
| onCancel: () { | ||
| setState(() { | ||
| final actual = _allMeetings.indexOf(meeting); | ||
| _allMeetings[actual] = Meeting( | ||
| title: meeting.title, | ||
| patientName: meeting.patientName, | ||
| scheduledAt: meeting.scheduledAt, | ||
| createdAt: meeting.createdAt, | ||
| notes: meeting.notes, | ||
| status: 'cancelled', | ||
| meetingType: meeting.meetingType, // preserved | ||
| ); | ||
| }); |
There was a problem hiding this comment.
The id field is lost when recreating Meeting for cancellation.
Same issue as in scheduled_meetings_page.dart — the id field should be preserved to maintain jitsiRoom consistency.
🛡️ Suggested fix
_allMeetings[actual] = Meeting(
+ id: meeting.id,
title: meeting.title,
patientName: meeting.patientName,
scheduledAt: meeting.scheduledAt,
createdAt: meeting.createdAt,
notes: meeting.notes,
status: 'cancelled',
meetingType: meeting.meetingType,
);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@flutter_app/lib/pages/online_meet_page.dart` around lines 413 - 425, When
marking a meeting cancelled in the onCancel handler, the new Meeting instance
drops the original id (and thus jitsiRoom consistency); update the onCancel
block that mutates _allMeetings (and the Meeting constructor call) to copy
meeting.id (and meeting.jitsiRoom if present) into the new Meeting so the
identifier/room are preserved (mirror the same fix applied in
scheduled_meetings_page.dart); ensure you update the Meeting(...) invocation
inside setState in online_meet_page.dart to include id: meeting.id (and
jitsiRoom: meeting.jitsiRoom) while keeping the other fields and status:
'cancelled'.
Comment on lines
+77
to
+89
| onCancel: () { | ||
| setState(() { | ||
| _scheduledMeetings[index] = Meeting( | ||
| title: meeting.title, | ||
| patientName: meeting.patientName, | ||
| scheduledAt: meeting.scheduledAt, | ||
| createdAt: meeting.createdAt, | ||
| notes: meeting.notes, | ||
| status: 'cancelled', | ||
| meetingType: meeting.meetingType, | ||
| ); | ||
| }); | ||
| }, |
There was a problem hiding this comment.
The id field is lost when recreating the Meeting for cancellation.
When cancelling, the new Meeting doesn't preserve meeting.id, which could cause issues if the video call relies on jitsiRoom derived from id. Consider adding a copyWith method to Meeting or explicitly preserving all fields.
🛡️ Suggested fix
onCancel: () {
setState(() {
_scheduledMeetings[index] = Meeting(
+ id: meeting.id,
title: meeting.title,
patientName: meeting.patientName,
scheduledAt: meeting.scheduledAt,
createdAt: meeting.createdAt,
notes: meeting.notes,
status: 'cancelled',
meetingType: meeting.meetingType,
);
});
},📝 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.
Suggested change
| onCancel: () { | |
| setState(() { | |
| _scheduledMeetings[index] = Meeting( | |
| title: meeting.title, | |
| patientName: meeting.patientName, | |
| scheduledAt: meeting.scheduledAt, | |
| createdAt: meeting.createdAt, | |
| notes: meeting.notes, | |
| status: 'cancelled', | |
| meetingType: meeting.meetingType, | |
| ); | |
| }); | |
| }, | |
| onCancel: () { | |
| setState(() { | |
| _scheduledMeetings[index] = Meeting( | |
| id: meeting.id, | |
| title: meeting.title, | |
| patientName: meeting.patientName, | |
| scheduledAt: meeting.scheduledAt, | |
| createdAt: meeting.createdAt, | |
| notes: meeting.notes, | |
| status: 'cancelled', | |
| meetingType: meeting.meetingType, | |
| ); | |
| }); | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@flutter_app/lib/pages/scheduled_meetings_page.dart` around lines 77 - 89, The
cancellation handler replaces a Meeting without preserving its id, which can
break jitsiRoom derivation; modify the onCancel logic to preserve the original
meeting.id (either by calling a new Meeting.copyWith({status: 'cancelled'}) or
by including id when constructing the new Meeting) so _scheduledMeetings[index]
keeps the same id; add a copyWith method to the Meeting class (or update its
constructor usage) and update the onCancel block to use it (or pass meeting.id)
to retain all original fields.
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
1 participant
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.
ui for the chat and call features is implemented :
we are done with the ui part, integration of these features with supabase is left, so features are still not linked but the structure is ready
Screenshots
Summary by CodeRabbit
New Features
Improvements