feat: add therapist dashboard metrics (patients/sessions/therapies)#214
feat: add therapist dashboard metrics (patients/sessions/therapies)#214nthsneha wants to merge 6 commits into
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 40 minutes and 52 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughRefactors patient daily-activities back navigation to run a background sync before popping and surface sync failure via SnackBar; adds TaskProvider sync status and background update method. Adds therapist totals fetching (patients, sessions, therapies) and a stats dashboard on the home screen, with Supabase-backed repository implementations. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Screen as Daily Activities Screen
participant PopScope
participant TaskProvider
participant Repo as Task Repo
User->>Screen: press back / navigate away
Screen->>PopScope: onWillPop
PopScope->>TaskProvider: updateActivityInBackground()
TaskProvider->>TaskProvider: set syncStatus = loading / notifyListeners
TaskProvider->>Repo: updateActivityCompletion(_allTasks)
Repo-->>TaskProvider: result (success/failure)
TaskProvider->>TaskProvider: copy _apiStatus -> _syncStatus / notifyListeners
TaskProvider-->>PopScope: completion
PopScope->>Screen: allow pop (Navigator.maybePop)
Screen->>User: show SnackBar if syncStatus == ApiStatus.failure
sequenceDiagram
participant HomeScreen
participant TherapistProvider
participant SessionProvider
participant Repo as Supabase Repo
HomeScreen->>TherapistProvider: fetchTotals()
TherapistProvider->>Repo: getTotalPatients()
TherapistProvider->>Repo: getTotalSessions()
TherapistProvider->>Repo: getTotalTherapies()
Repo-->>TherapistProvider: counts
TherapistProvider->>TherapistProvider: update totals / notifyListeners
HomeScreen->>SessionProvider: fetchTherapistSessions()
SessionProvider-->>HomeScreen: sessions loaded
HomeScreen->>HomeScreen: render stats dashboard or loader
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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
|
Hey @nthsneha, went through the diff. The intent is right but there are several issues across all three commits that block this from merging cleanly. What's working
Blockers
onPressed: () => Navigator.pop(context),
Future<void> updateActivityInBackground() async {
if (_allTasks.isEmpty || _syncStatus == ApiStatus.loading) return;
_syncStatus = ApiStatus.loading;
notifyListeners();
await updateActivityCompletion(_allTasks);
_syncStatus = _apiStatus;
notifyListeners();
}
Future<void> fetchTotals() async {
_isLoading = true;
notifyListeners();
try {
final patientsResult = await _therapistRepository.getTotalPatients();
if (patientsResult is ActionResultSuccess) {
_totalPatients = (patientsResult.data as num?)?.toInt() ?? 0;
}
// same for sessions and therapies
} finally {
_isLoading = false;
notifyListeners();
}
}
final count = response is List
? response.map((e) => e['therapy_type_id']).whereType<String>().toSet().length
: 0;
final results = await Future.wait([
_therapistRepository.getTotalPatients(),
_therapistRepository.getTotalSessions(),
_therapistRepository.getTotalTherapies(),
]);
Minor
Fix the double-sync, the stuck loading state, the duplicate UI block, and the null therapy count and this is close to mergeable. |
|
@Varadraj75 Made the changes |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@patient/lib/presentation/activities/daily_activities_screen.dart`:
- Around line 59-75: The callback passed to PopScope uses
context.read<TaskProvider>() after awaiting updateActivityInBackground(), which
is unsafe if the widget is disposed; capture the provider before the async gap
by obtaining a local variable (e.g., final taskProvider =
context.read<TaskProvider>() or final provider = context.read<TaskProvider>())
prior to calling await taskProvider.updateActivityInBackground(), then use that
local reference to check provider.syncStatus and avoid using
context.read<TaskProvider>() after the await while still respecting mounted
before calling Navigator.pop.
🪄 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: 9860b446-ad6e-446e-bfcd-e6b8c62791f3
📒 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
🚧 Files skipped from review as they are similar to previous changes (3)
- therapist/lib/presentation/home/home_screen.dart
- therapist/lib/repository/supabase_therapist_repository.dart
- therapist/lib/provider/therapist_provider.dart
|
@nthsneha, went through the latest diff. Two blockers still need fixing before this merges, plus one thing worth verifying. Blocker 1:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@patient/lib/presentation/activities/daily_activities_screen.dart`:
- Around line 61-74: The onPopInvokedWithResult callback currently awaits
taskProvider.updateActivityInBackground() and then calls Navigator.pop, but if
updateActivityInBackground() throws the pop will never run; wrap the async call
in a try { await taskProvider.updateActivityInBackground(); } catch (e) { /*
optionally log or handle */ } finally { if (taskProvider.syncStatus ==
ApiStatus.failure && mounted) show the SnackBar; if (mounted)
Navigator.pop(context); } so that updateActivityInBackground (referenced by
TaskProvider.updateActivityInBackground) errors are handled and
Navigator.pop(context) always executes from the finally block while preserving
the existing snackbar logic that checks taskProvider.syncStatus and mounted.
🪄 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: 2661bb83-95a8-4007-92c7-fa2ed4b5bebe
📒 Files selected for processing (1)
patient/lib/presentation/activities/daily_activities_screen.dart
2 participants
📝 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
New Features
Bug Fixes / Improvements