fix: guard rating calculation when ratingCount is zero (fixes #780)#781
fix: guard rating calculation when ratingCount is zero (fixes #780)#781dolliecoder wants to merge 2 commits intoAOSSIE-Org:masterfrom
Conversation
|
🎉 Welcome @dolliecoder!
We appreciate your contribution! 🚀 |
📝 WalkthroughWalkthroughAdded safety guards and tightened control flow across several controllers: rating division now checks for zero counts, realtime listeners short-circuit when events are empty, like deletion only occurs when a like document exists, a participant lookup can return null, and a subscriber removal now throws if no documents found. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
…ealtime listeners and query results
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/controllers/single_room_controller.dart`:
- Around line 248-250: Change getParticipantDocId's signature from
Future<String> to Future<String?> and have it return null when
participantDocsRef.documents.isEmpty; then update every callsite (makeModerator,
removeModerator, makeSpeaker, makeListener, kickOutParticipant) to await
getParticipantDocId into a nullable variable, check for null and early-return if
null (e.g. final participantDocId = await getParticipantDocId(...); if
(participantDocId == null) return;), and then call
updateParticipantDoc(participantDocId, ...) only when non-null. Ensure all
signatures and usages reflect the nullable return type.
In `@lib/controllers/upcomming_rooms_controller.dart`:
- Around line 119-124: Replace the throw in the query result handler so empty
results are treated as a no-op instead of throwing a generic uncaught Exception:
in the .then((value) { ... }) block that currently checks
value.documents.isEmpty and throws Exception('No documents found'), remove the
throw and return a sensible empty sentinel (e.g. null or an empty optional/empty
result) so callers can handle "no record" normally; update any downstream
handling of the returned value from this query chain to treat the sentinel as an
early return/unsubscribed case instead of relying on an exception.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
lib/controllers/explore_story_controller.dartlib/controllers/friend_calling_controller.dartlib/controllers/live_chapter_controller.dartlib/controllers/pair_chat_controller.dartlib/controllers/room_chat_controller.dartlib/controllers/single_room_controller.dartlib/controllers/upcomming_rooms_controller.dart
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/controllers/explore_story_controller.dart
| if (participantDocsRef.documents.isEmpty) { | ||
| return null; // or handle appropriately | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's locate and read the file to understand the current state
git ls-files lib/controllers/single_room_controller.dartRepository: AOSSIE-Org/Resonate
Length of output: 106
🏁 Script executed:
# Get the file size to understand how to read it
wc -l lib/controllers/single_room_controller.dartRepository: AOSSIE-Org/Resonate
Length of output: 110
🏁 Script executed:
# Search for the getParticipantDocId method declaration and implementation
rg -n "getParticipantDocId" lib/controllers/single_room_controller.dart -A 20Repository: AOSSIE-Org/Resonate
Length of output: 3190
🏁 Script executed:
# Search for where the method returns null or is called
rg -n "return null" lib/controllers/single_room_controller.dart -B 3 -A 1Repository: AOSSIE-Org/Resonate
Length of output: 198
🏁 Script executed:
# Check all callsites of getParticipantDocId to see how they handle the result
rg -n "await getParticipantDocId\|getParticipantDocId(" lib/controllers/single_room_controller.dart -B 2 -A 3Repository: AOSSIE-Org/Resonate
Length of output: 207
🏁 Script executed:
# Check for analysis configuration
fd -name "analysis_options.yaml" -o -name "pubspec.yaml" | head -5 | xargs grep -l "enable-null-safety\|strict-null-safety" 2>/dev/null || echo "Checking analysis_options.yaml..."Repository: AOSSIE-Org/Resonate
Length of output: 344
🏁 Script executed:
# Check the analysis configuration file
cat analysis_options.yaml 2>/dev/null | head -30 || echo "No analysis_options.yaml found"Repository: AOSSIE-Org/Resonate
Length of output: 1470
🏁 Script executed:
# Check pubspec.yaml for null-safety configuration
cat pubspec.yaml 2>/dev/null | grep -A 5 "environment\|null-safety" || echo "No environment config found"Repository: AOSSIE-Org/Resonate
Length of output: 328
Null-safety violation: non-nullable method returns null.
getParticipantDocId is declared Future<String> but returns null at line 249, which breaks Dart's null-safety constraints and will fail static analysis. Make the return type Future<String?> and add null checks at all callsites (makeModerator, removeModerator, makeSpeaker, makeListener, kickOutParticipant):
final participantDocId = await getParticipantDocId(participant);
if (participantDocId == null) return;
await updateParticipantDoc(participantDocId, {...});Also update the method signature to Future<String?>.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/controllers/single_room_controller.dart` around lines 248 - 250, Change
getParticipantDocId's signature from Future<String> to Future<String?> and have
it return null when participantDocsRef.documents.isEmpty; then update every
callsite (makeModerator, removeModerator, makeSpeaker, makeListener,
kickOutParticipant) to await getParticipantDocId into a nullable variable, check
for null and early-return if null (e.g. final participantDocId = await
getParticipantDocId(...); if (participantDocId == null) return;), and then call
updateParticipantDoc(participantDocId, ...) only when non-null. Ensure all
signatures and usages reflect the nullable return type.
| .then((value) { | ||
| if (value.documents.isEmpty) { | ||
| throw Exception('No documents found'); | ||
| } | ||
| return value.documents.first; | ||
| }); |
There was a problem hiding this comment.
Avoid throwing an uncaught generic exception on empty query results.
This new Exception('No documents found') is not caught by the current on AppwriteException block, so normal “already unsubscribed / no record” cases can fail unexpectedly. Prefer a no-op early return.
💡 Suggested fix
- var subscribeDocument = await databases
+ final docs = await databases
.listDocuments(
databaseId: upcomingRoomsDatabaseId,
collectionId: subscribedUserCollectionId,
queries: [
Query.and([
Query.equal('userID', authStateController.uid),
Query.equal('upcomingRoomId', upcomingRoomId),
]),
],
)
- .then((value) {
- if (value.documents.isEmpty) {
- throw Exception('No documents found');
- }
- return value.documents.first;
- });
+ .then((value) => value.documents);
+
+ if (docs.isEmpty) {
+ return;
+ }
+ final subscribeDocument = docs.first;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/controllers/upcomming_rooms_controller.dart` around lines 119 - 124,
Replace the throw in the query result handler so empty results are treated as a
no-op instead of throwing a generic uncaught Exception: in the .then((value) {
... }) block that currently checks value.documents.isEmpty and throws
Exception('No documents found'), remove the throw and return a sensible empty
sentinel (e.g. null or an empty optional/empty result) so callers can handle "no
record" normally; update any downstream handling of the returned value from this
query chain to treat the sentinel as an early return/unsubscribed case instead
of relying on an exception.
|
✅ PR Closed - Thank You, @dolliecoder!
We appreciate your effort and look forward to more contributions from you! 🤝 |
1 participant
Description
Fixes #780
Guards the
userRatingcalculation inexplore_story_controller.dartto preventinvalid values (NaN/Infinity) when
ratingCountis zero or null.Previously,
ratingTotal / ratingCountcould produce invalid numeric results.The calculation now safely returns
0.0whenratingCountis not greater than zero.Type of change
How Has This Been Tested?
Checklist:
Summary by CodeRabbit
Bug Fixes
Chores