Skip to content

fix: correct Map vs bool comparison in _updateTaskStatus - #251

Open
Priyanshu-byte-coder wants to merge 1 commit into
AOSSIE-Org:mainfrom
Priyanshu-byte-coder:fix/task-status-type-comparison
Open

fix: correct Map vs bool comparison in _updateTaskStatus#251
Priyanshu-byte-coder wants to merge 1 commit into
AOSSIE-Org:mainfrom
Priyanshu-byte-coder:fix/task-status-type-comparison

Conversation

@Priyanshu-byte-coder

@Priyanshu-byte-coder Priyanshu-byte-coder commented May 14, 2026

Copy link
Copy Markdown

Summary

Closes #222

_updateTaskStatus() stored the return value of updateTaskStatus() in success, then compared success == true. But updateTaskStatus() returns Future<Map<String, dynamic>>, not bool — so Map == bool is always false in Dart. The success SnackBar was unreachable and the failure SnackBar showed on every status update.

Root cause

// Before — wrong type comparison
final success = await _supabaseService.updateTaskStatus(...);
if (success == true) { // ❌ Map<String,dynamic> == bool → always false

Fix

// After — read 'success' key from returned Map
final result = await _supabaseService.updateTaskStatus(...);
if (result['success'] == true) { // ✅ correct

Changes

  • lib/screens/tasks/task_detail_screen.dart:75 — renamed successresult
  • lib/screens/tasks/task_detail_screen.dart:81 — changed success == trueresult['success'] == true

This matches the pattern already used on line 186 in the same file (result['success']).

Summary by CodeRabbit

  • Bug Fixes
    • Enhanced task status update feedback with improved success and error notifications.

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 14, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

The PR fixes a type mismatch bug in task status updates where the return value comparison was unreachable. The method now correctly accesses the success field from the returned map to determine update success and reload task details.

Changes

Task Status Update Result Handling

Layer / File(s) Summary
Type-safe task status update result handling
lib/screens/tasks/task_detail_screen.dart
The _updateTaskStatus method now extracts result['success'] from the returned map instead of comparing the entire map to a boolean, enabling the success branch to execute, reload task details, and display a success snackbar.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

Dart/Flutter, bug-fix

Poem

🐰 A map once compared to a bool so true,
Always returning false, what a terrible view!
Now we extract the success with methodical care,
The snackbar shows green—hooray, it's fair! ✨

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: correct Map vs bool comparison in _updateTaskStatus' directly and accurately describes the main change, which is fixing the type comparison bug identified in issue #222.
Linked Issues check ✅ Passed The PR correctly fixes issue #222 by treating updateTaskStatus() return value as a Map and checking result['success'] == true, and aligns with issue #39's error handling objectives by properly validating success and showing appropriate SnackBar messages.
Out of Scope Changes check ✅ Passed All changes in the PR are scoped to fixing the type comparison bug in _updateTaskStatus(), with no unrelated modifications to other parts of the codebase.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
lib/screens/tasks/task_detail_screen.dart (1)

80-85: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Re-check mounted after awaiting _loadTaskDetails() before using context.

At line 83, await _loadTaskDetails() can unmount this widget. The ScaffoldMessenger.of(context) call on line 84 then uses context after an async gap without a fresh mounted guard. Additionally, line 93 has the same unguarded pattern in the else branch.

Suggested fix
      if (mounted) {
        if (result['success'] == true) {
          // Reload task details
          await _loadTaskDetails();
+         if (!mounted) return;
          ScaffoldMessenger.of(context).showSnackBar(
            SnackBar(
              content: Text(
                'Task status updated to ${_getStatusLabel(status)}',
              ),

Apply the same check after line 93 in the else branch as well.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/screens/tasks/task_detail_screen.dart` around lines 80 - 85, After
awaiting _loadTaskDetails() you must re-check the widget's mounted property
before using the BuildContext; update both the success branch (after await
_loadTaskDetails() and before ScaffoldMessenger.of(context).showSnackBar(...))
and the else branch (the place at line ~93 where ScaffoldMessenger.of(context)
is used) to return or skip the context-dependent calls if mounted is false,
i.e., re-evaluate mounted and only call
ScaffoldMessenger.of(context).showSnackBar(...) when mounted is true to avoid
using context after an async gap.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@lib/screens/tasks/task_detail_screen.dart`:
- Around line 80-85: After awaiting _loadTaskDetails() you must re-check the
widget's mounted property before using the BuildContext; update both the success
branch (after await _loadTaskDetails() and before
ScaffoldMessenger.of(context).showSnackBar(...)) and the else branch (the place
at line ~93 where ScaffoldMessenger.of(context) is used) to return or skip the
context-dependent calls if mounted is false, i.e., re-evaluate mounted and only
call ScaffoldMessenger.of(context).showSnackBar(...) when mounted is true to
avoid using context after an async gap.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 30842278-8bcd-4667-8d72-072652bb9aed

📥 Commits

Reviewing files that changed from the base of the PR and between 1cea2a6 and d77ea28.

📒 Files selected for processing (1)
  • lib/screens/tasks/task_detail_screen.dart

@Priyanshu-byte-coder

Copy link
Copy Markdown
Author

@M4dhav @Swayanshuu this one is approved and mergeable — could you merge when you get a chance? It fixes the Map-vs-bool comparison in _updateTaskStatus. Happy to rebase if needed. PRs #252, #253, #254 in the same batch are also ready for a look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BUG: Wrong type comparison in _updateTaskStatus() — success snackbar never shows

1 participant