feat: add therapist dashboard metrics (patients/sessions/therapies)#214
Open
nthsneha wants to merge 3 commits intoAOSSIE-Org:mainfrom
Open
feat: add therapist dashboard metrics (patients/sessions/therapies)#214nthsneha wants to merge 3 commits intoAOSSIE-Org:mainfrom
nthsneha wants to merge 3 commits intoAOSSIE-Org:mainfrom
Conversation
📝 WalkthroughWalkthroughThis PR implements two features: background synchronization of activity completion when users navigate away from the daily activities screen on the patient side, and a stats dashboard displaying aggregate metrics (total patients, sessions, therapies) on the therapist home screen. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Screen as Daily Activities<br/>Screen
participant PopScope
participant TaskProvider
participant Repo as Task Repo
User->>Screen: Navigate away
Screen->>PopScope: Trigger pop
PopScope->>TaskProvider: updateActivityInBackground()
TaskProvider->>TaskProvider: Set syncStatus to loading
TaskProvider->>TaskProvider: Notify listeners
TaskProvider->>Repo: updateActivityCompletion(_allTasks)
Repo-->>TaskProvider: Result (success/failure)
TaskProvider->>TaskProvider: Copy result to syncStatus
TaskProvider->>TaskProvider: Notify listeners
PopScope->>Screen: Complete navigation
sequenceDiagram
participant HomeScreen
participant TherapistProvider
participant SessionProvider
participant Repo as Supabase Repo
HomeScreen->>TherapistProvider: fetchTotals()
TherapistProvider->>Repo: getTotalPatients()
Repo-->>TherapistProvider: Count result
TherapistProvider->>Repo: getTotalSessions()
Repo-->>TherapistProvider: Count result
TherapistProvider->>Repo: getTotalTherapies()
Repo-->>TherapistProvider: Count result
TherapistProvider->>TherapistProvider: Notify listeners
HomeScreen->>SessionProvider: fetchTherapistSessions()
SessionProvider-->>HomeScreen: Sessions loaded
HomeScreen->>HomeScreen: Render stats dashboard
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
therapist/lib/repository/supabase_therapist_repository.dart (1)
134-152: Avoid fetching full row lists just to compute totals.Line 134-Line 152 currently materializes all matching IDs and counts client-side. This does unnecessary data transfer for large therapist datasets; prefer server-side counting.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@therapist/lib/repository/supabase_therapist_repository.dart` around lines 134 - 152, The current implementations (e.g., getTotalSessions and the patient-counting block) fetch full ID lists and count them client-side; change the Supabase query to request a server-side count instead (use the select call's count/fetch options such as FetchOptions(count: CountOption.exact) or the client’s equivalent) on _supabaseClient.from('session') / from('patient') so the query returns the total via response.count, then return that integer in ActionResultSuccess instead of response.length.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@patient/lib/presentation/activities/daily_activities_screen.dart`:
- Around line 60-74: The code currently calls
TaskProvider.updateActivityInBackground() in both the onPopInvokedWithResult
callback and the AppBar leading IconButton onPressed, causing duplicate
background sync; remove the updateActivityInBackground() call from the
IconButton onPressed and leave only Navigator.pop(context) there so the
onPopInvokedWithResult closure remains the single place that triggers
updateActivityInBackground(), referencing updateActivityInBackground(),
onPopInvokedWithResult, the leading IconButton onPressed, TaskProvider, and
Navigator.pop.
In `@patient/lib/provider/task_provider.dart`:
- Around line 82-92: Add a reentrancy guard to updateActivityInBackground to
prevent concurrent runs: introduce a private boolean flag (e.g.
_isUpdatingActivities) and at the top of updateActivityInBackground return
immediately if the flag is true; set the flag true before setting _syncStatus
and calling updateActivityCompletion(_allTasks) and reset it to false in a
finally block so it always clears even on error. This ensures overlapping calls
cannot race on _syncStatus/_apiStatus or trigger duplicate backend updates while
keeping the existing _syncStatus = _apiStatus assignment and notifyListeners
behavior intact.
In `@therapist/lib/presentation/home/home_screen.dart`:
- Around line 255-300: You added a new dynamic Consumer2 block that renders
StatsCard widgets but left the old static metrics block in place, causing
duplicate stat sections; remove the earlier static metrics Container (the
previous group of StatsCard widgets) so only the
Consumer2<TherapistDataProvider, SessionProvider> block renders the stats,
ensuring you keep the StatsCard usages inside Consumer2 and delete the redundant
static cards/widgets.
In `@therapist/lib/provider/therapist_provider.dart`:
- Around line 182-203: The fetchTotals() method can leave _isLoading true if an
exception occurs and uses unsafe casts; wrap the body that calls
_therapistRepository.getTotalPatients/getTotalSessions/getTotalTherapies in a
try/finally so _isLoading is always set to false and notifyListeners() is called
in finally, and replace the `as int` casts by safe type checks on the
ActionResultSuccess.data (e.g., `if (patientsResult is ActionResultSuccess &&
patientsResult.data is int) _totalPatients = patientsResult.data as int` or
assign a fallback like 0 when data is null/invalid) so assignments for
_totalPatients, _totalSessions, and _totalTherapies never throw.
In `@therapist/lib/repository/supabase_therapist_repository.dart`:
- Around line 169-171: The distinct therapy count computation currently treats
null therapy_type_id as a valid distinct value; update the logic that builds
count (where response is List) to filter out nulls before mapping to
therapy_type_id and calling toSet(), e.g., only include entries with a non-null
e['therapy_type_id'] so the resulting set and count exclude nulls; locate the
block using the local variables response and count in
supabase_therapist_repository.dart and apply the filter there.
---
Nitpick comments:
In `@therapist/lib/repository/supabase_therapist_repository.dart`:
- Around line 134-152: The current implementations (e.g., getTotalSessions and
the patient-counting block) fetch full ID lists and count them client-side;
change the Supabase query to request a server-side count instead (use the select
call's count/fetch options such as FetchOptions(count: CountOption.exact) or the
client’s equivalent) on _supabaseClient.from('session') / from('patient') so the
query returns the total via response.count, then return that integer in
ActionResultSuccess instead of response.length.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 44cb7378-a439-42b6-93e3-64a5e474ac5a
📒 Files selected for processing (5)
patient/lib/presentation/activities/daily_activities_screen.dartpatient/lib/provider/task_provider.darttherapist/lib/presentation/home/home_screen.darttherapist/lib/provider/therapist_provider.darttherapist/lib/repository/supabase_therapist_repository.dart
Comment on lines
+60
to
+74
| onPopInvokedWithResult: (didPop, result) { | ||
| if (didPop) { | ||
| context.read<TaskProvider>().updateActivityInBackground(); | ||
| } | ||
| }, | ||
| child: Scaffold( | ||
| appBar: AppBar( | ||
| title: const Text('Daily Activities'), | ||
| centerTitle: true, | ||
| leading: IconButton( | ||
| icon: const Icon(Icons.arrow_back_ios_new_rounded), | ||
| onPressed: () { | ||
| context.read<TaskProvider>().updateActivityInBackground(); | ||
| Navigator.pop(context); | ||
| }, |
There was a problem hiding this comment.
Avoid double-triggering background sync on back navigation.
Line 72 and Lines 60-63 both invoke updateActivityInBackground(), so one back-button tap can dispatch two concurrent sync calls. Keep only one trigger path.
Proposed fix
onPopInvokedWithResult: (didPop, result) {
if (didPop) {
context.read<TaskProvider>().updateActivityInBackground();
}
},
...
onPressed: () {
- context.read<TaskProvider>().updateActivityInBackground();
Navigator.pop(context);
},📝 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
| onPopInvokedWithResult: (didPop, result) { | |
| if (didPop) { | |
| context.read<TaskProvider>().updateActivityInBackground(); | |
| } | |
| }, | |
| child: Scaffold( | |
| appBar: AppBar( | |
| title: const Text('Daily Activities'), | |
| centerTitle: true, | |
| leading: IconButton( | |
| icon: const Icon(Icons.arrow_back_ios_new_rounded), | |
| onPressed: () { | |
| context.read<TaskProvider>().updateActivityInBackground(); | |
| Navigator.pop(context); | |
| }, | |
| onPopInvokedWithResult: (didPop, result) { | |
| if (didPop) { | |
| context.read<TaskProvider>().updateActivityInBackground(); | |
| } | |
| }, | |
| child: Scaffold( | |
| appBar: AppBar( | |
| title: const Text('Daily Activities'), | |
| centerTitle: true, | |
| leading: IconButton( | |
| icon: const Icon(Icons.arrow_back_ios_new_rounded), | |
| onPressed: () { | |
| Navigator.pop(context); | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@patient/lib/presentation/activities/daily_activities_screen.dart` around
lines 60 - 74, The code currently calls
TaskProvider.updateActivityInBackground() in both the onPopInvokedWithResult
callback and the AppBar leading IconButton onPressed, causing duplicate
background sync; remove the updateActivityInBackground() call from the
IconButton onPressed and leave only Navigator.pop(context) there so the
onPopInvokedWithResult closure remains the single place that triggers
updateActivityInBackground(), referencing updateActivityInBackground(),
onPopInvokedWithResult, the leading IconButton onPressed, TaskProvider, and
Navigator.pop.
Comment on lines
+82
to
+92
| Future<void> updateActivityInBackground() async { | ||
| if (_allTasks.isEmpty) { | ||
| return; | ||
| } | ||
| _syncStatus = ApiStatus.loading; | ||
| notifyListeners(); | ||
| await updateActivityCompletion(_allTasks); | ||
| // updateActivityCompletion sets _apiStatus, copy it to _syncStatus | ||
| _syncStatus = _apiStatus; | ||
| notifyListeners(); | ||
| } |
There was a problem hiding this comment.
Guard updateActivityInBackground() against concurrent execution.
Without a guard, overlapping calls can race on _syncStatus/_apiStatus and issue duplicate backend updates.
Proposed fix
Future<void> updateActivityInBackground() async {
- if (_allTasks.isEmpty) {
+ if (_allTasks.isEmpty || _syncStatus == ApiStatus.loading) {
return;
}
_syncStatus = ApiStatus.loading;
notifyListeners();
await updateActivityCompletion(_allTasks);
// updateActivityCompletion sets _apiStatus, copy it to _syncStatus
_syncStatus = _apiStatus;
notifyListeners();
}📝 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
| Future<void> updateActivityInBackground() async { | |
| if (_allTasks.isEmpty) { | |
| return; | |
| } | |
| _syncStatus = ApiStatus.loading; | |
| notifyListeners(); | |
| await updateActivityCompletion(_allTasks); | |
| // updateActivityCompletion sets _apiStatus, copy it to _syncStatus | |
| _syncStatus = _apiStatus; | |
| notifyListeners(); | |
| } | |
| Future<void> updateActivityInBackground() async { | |
| if (_allTasks.isEmpty || _syncStatus == ApiStatus.loading) { | |
| return; | |
| } | |
| _syncStatus = ApiStatus.loading; | |
| notifyListeners(); | |
| await updateActivityCompletion(_allTasks); | |
| // updateActivityCompletion sets _apiStatus, copy it to _syncStatus | |
| _syncStatus = _apiStatus; | |
| notifyListeners(); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@patient/lib/provider/task_provider.dart` around lines 82 - 92, Add a
reentrancy guard to updateActivityInBackground to prevent concurrent runs:
introduce a private boolean flag (e.g. _isUpdatingActivities) and at the top of
updateActivityInBackground return immediately if the flag is true; set the flag
true before setting _syncStatus and calling updateActivityCompletion(_allTasks)
and reset it to false in a finally block so it always clears even on error. This
ensures overlapping calls cannot race on _syncStatus/_apiStatus or trigger
duplicate backend updates while keeping the existing _syncStatus = _apiStatus
assignment and notifyListeners behavior intact.
Comment on lines
+255
to
+300
| Consumer2<TherapistDataProvider, SessionProvider>( | ||
| builder: (context, therapistProvider, sessionProvider, _) { | ||
| if (therapistProvider.isLoading || sessionProvider.isLoading) { | ||
| return const Center(child: CircularProgressIndicator()); | ||
| } | ||
| } | ||
| return const SizedBox.shrink(); | ||
| }, | ||
| ), | ||
|
|
||
| return Container( | ||
| padding: const EdgeInsets.all(20), | ||
| decoration: BoxDecoration( | ||
| color: Colors.white, | ||
| borderRadius: BorderRadius.circular(16), | ||
| boxShadow: [ | ||
| BoxShadow( | ||
| color: Colors.black.withOpacity(0.05), | ||
| blurRadius: 10, | ||
| offset: const Offset(0, 5), | ||
| ), | ||
| ], | ||
| ), | ||
| child: Row( | ||
| mainAxisAlignment: MainAxisAlignment.spaceAround, | ||
| children: [ | ||
| StatsCard( | ||
| imagePath: 'assets/icon1.png', | ||
| backgroundColor: const Color(0xFFFEE8E8), | ||
| label: 'Patients', | ||
| value: therapistProvider.totalPatients.toString(), | ||
| ), | ||
| StatsCard( | ||
| imagePath: 'assets/icon2.png', | ||
| backgroundColor: const Color(0xFFF1E8FE), | ||
| label: 'Sessions', | ||
| value: sessionProvider.totalSessions.toString(), | ||
| ), | ||
| StatsCard( | ||
| imagePath: 'assets/icon3.png', | ||
| backgroundColor: const Color(0xFFE8FEF0), | ||
| label: 'Therapies', | ||
| value: therapistProvider.totalTherapies.toString(), | ||
| ), | ||
| ], | ||
| ), | ||
| ); | ||
| }, | ||
| ), | ||
| const SizedBox(height: 24), |
There was a problem hiding this comment.
Live metrics were added without removing the static metrics block.
The new Consumer2 cards render in addition to the old static cards (Line 217-Line 253), so the dashboard shows duplicate stat sections.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@therapist/lib/presentation/home/home_screen.dart` around lines 255 - 300, You
added a new dynamic Consumer2 block that renders StatsCard widgets but left the
old static metrics block in place, causing duplicate stat sections; remove the
earlier static metrics Container (the previous group of StatsCard widgets) so
only the Consumer2<TherapistDataProvider, SessionProvider> block renders the
stats, ensuring you keep the StatsCard usages inside Consumer2 and delete the
redundant static cards/widgets.
Comment on lines
+182
to
+203
| Future<void> fetchTotals() async { | ||
| _isLoading = true; | ||
| notifyListeners(); | ||
|
|
||
| final patientsResult = await _therapistRepository.getTotalPatients(); | ||
| if (patientsResult is ActionResultSuccess) { | ||
| _totalPatients = patientsResult.data as int; | ||
| } | ||
|
|
||
| final sessionsResult = await _therapistRepository.getTotalSessions(); | ||
| if (sessionsResult is ActionResultSuccess) { | ||
| _totalSessions = sessionsResult.data as int; | ||
| } | ||
|
|
||
| final therapiesResult = await _therapistRepository.getTotalTherapies(); | ||
| if (therapiesResult is ActionResultSuccess) { | ||
| _totalTherapies = therapiesResult.data as int; | ||
| } | ||
|
|
||
| _isLoading = false; | ||
| notifyListeners(); | ||
| } |
There was a problem hiding this comment.
Harden fetchTotals() with try/finally and safe type checks.
If any call/cast fails before Line 201, _isLoading never resets. Also, as int casts at Line 188, Line 193, and Line 198 can throw on unexpected payloads.
Proposed fix
Future<void> fetchTotals() async {
_isLoading = true;
notifyListeners();
-
- final patientsResult = await _therapistRepository.getTotalPatients();
- if (patientsResult is ActionResultSuccess) {
- _totalPatients = patientsResult.data as int;
- }
-
- final sessionsResult = await _therapistRepository.getTotalSessions();
- if (sessionsResult is ActionResultSuccess) {
- _totalSessions = sessionsResult.data as int;
- }
-
- final therapiesResult = await _therapistRepository.getTotalTherapies();
- if (therapiesResult is ActionResultSuccess) {
- _totalTherapies = therapiesResult.data as int;
- }
-
- _isLoading = false;
- notifyListeners();
+ try {
+ final patientsResult = await _therapistRepository.getTotalPatients();
+ if (patientsResult is ActionResultSuccess && patientsResult.data is int) {
+ _totalPatients = patientsResult.data as int;
+ }
+
+ final sessionsResult = await _therapistRepository.getTotalSessions();
+ if (sessionsResult is ActionResultSuccess && sessionsResult.data is int) {
+ _totalSessions = sessionsResult.data as int;
+ }
+
+ final therapiesResult = await _therapistRepository.getTotalTherapies();
+ if (therapiesResult is ActionResultSuccess && therapiesResult.data is int) {
+ _totalTherapies = therapiesResult.data as int;
+ }
+ } finally {
+ _isLoading = false;
+ notifyListeners();
+ }
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@therapist/lib/provider/therapist_provider.dart` around lines 182 - 203, The
fetchTotals() method can leave _isLoading true if an exception occurs and uses
unsafe casts; wrap the body that calls
_therapistRepository.getTotalPatients/getTotalSessions/getTotalTherapies in a
try/finally so _isLoading is always set to false and notifyListeners() is called
in finally, and replace the `as int` casts by safe type checks on the
ActionResultSuccess.data (e.g., `if (patientsResult is ActionResultSuccess &&
patientsResult.data is int) _totalPatients = patientsResult.data as int` or
assign a fallback like 0 when data is null/invalid) so assignments for
_totalPatients, _totalSessions, and _totalTherapies never throw.
Comment on lines
+169
to
+171
| final count = response is List | ||
| ? (response.map((e) => e['therapy_type_id']).toSet().length) | ||
| : 0; |
There was a problem hiding this comment.
Exclude null therapy_type_id values from distinct therapy count.
Line 170 can count null as one distinct therapy, which can overstate totals when rows are incomplete.
Proposed fix
- final count = response is List
- ? (response.map((e) => e['therapy_type_id']).toSet().length)
- : 0;
+ final count = response is List
+ ? response
+ .map((e) => e['therapy_type_id'])
+ .where((id) => id != null)
+ .toSet()
+ .length
+ : 0;📝 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
| final count = response is List | |
| ? (response.map((e) => e['therapy_type_id']).toSet().length) | |
| : 0; | |
| final count = response is List | |
| ? response | |
| .map((e) => e['therapy_type_id']) | |
| .where((id) => id != null) | |
| .toSet() | |
| .length | |
| : 0; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@therapist/lib/repository/supabase_therapist_repository.dart` around lines 169
- 171, The distinct therapy count computation currently treats null
therapy_type_id as a valid distinct value; update the logic that builds count
(where response is List) to filter out nulls before mapping to therapy_type_id
and calling toSet(), e.g., only include entries with a non-null
e['therapy_type_id'] so the resulting set and count exclude nulls; locate the
block using the local variables response and count in
supabase_therapist_repository.dart and apply the filter there.
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.
📝 Description
Adds therapist dashboard summary metrics and backend support to display:
🔧 Changes Made
therapist/lib/repository/supabase_therapist_repository.dartgetTotalPatients(),getTotalSessions(),getTotalTherapies()in Supabase repository.therapist/lib/provider/therapist_provider.darttotalPatients,totalSessions,totalTherapiesfetchTotals()to call the new repository methods and update state.therapist/lib/presentation/home/home_screen.dartfetchTotals()+fetchTherapistSessions()Consumer2<TherapistDataProvider, SessionProvider>✅ Checklist
Summary by CodeRabbit