Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
📝 WalkthroughWalkthroughThe application adds Android event reminder notifications with persisted per-event settings. It adds notification permission handling, a dedicated settings page, route wiring, shared permission widgets, and timer event integration. ChangesEvent Reminder Notifications
Estimated code review effort: 4 (Complex) | ~45 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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: 2
🧹 Nitpick comments (1)
lib/service/notification_service.dart (1)
26-44: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider guarding
initwith a platform check.
_createEventsChannelandshowEventNotificationboth return early on non-Android platforms, butinitializeruns everywhere.InitializationSettingssupplies onlyandroid, and the plugin raises an error when the settings for the target platform are missing. Thecatchabsorbs that error, so the failure surfaces only in the log. An explicit guard makes the Android-only scope of this service consistent.♻️ Proposed platform guard
Future<NotificationService> init() async { _storage = Get.find<AppStorageService>().mmkv; + if (!Platform.isAndroid) { + Get.log('NotificationService: 非 Android 平台, 跳过初始化'); + return this; + } + try {🤖 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/service/notification_service.dart` around lines 26 - 44, Update NotificationService.init to return early when the current platform is not Android, before creating InitializationSettings or calling _plugin.initialize. Keep the existing Android initialization flow, event-channel setup, and success state unchanged.
🤖 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.
Inline comments:
In `@lib/service/notification_service.dart`:
- Around line 85-99: Wrap the entire body of showEventNotification, including
the permission check and StorageKeys.notificationIds lookup, in the existing try
block so all failures are contained and skipped silently. Keep the current
early-return behavior for uninitialized, non-Android, disabled,
ungranted-permission, and unknown-event cases.
In `@pubspec.yaml`:
- Line 40: Update the project SDK constraints around the
flutter_local_notifications dependency to require Flutter >=3.27.0, and ensure
workflow Flutter versions satisfy that floor; alternatively, pin all workflow
Flutter versions to a compatible release. Keep the 22.1.0 dependency unchanged.
---
Nitpick comments:
In `@lib/service/notification_service.dart`:
- Around line 26-44: Update NotificationService.init to return early when the
current platform is not Android, before creating InitializationSettings or
calling _plugin.initialize. Keep the existing Android initialization flow,
event-channel setup, and success state unchanged.
🪄 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 Plus
Run ID: ce21e97a-e100-4e99-847a-47d226bd9f57
⛔ Files ignored due to path filters (1)
pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (15)
.gitignorelib/config/storage_keys.dartlib/main.dartlib/page/home/controller.dartlib/page/notification_settings/controller.dartlib/page/notification_settings/state.dartlib/page/notification_settings/view.dartlib/page/setting/view.dartlib/page/setting/widgets/permission_row.dartlib/page/setting/widgets/permission_settings.dartlib/page/setting/widgets/widgets.dartlib/route/route_name.dartlib/route/route_page.dartlib/service/notification_service.dartpubspec.yaml
Comment on lines
+85
to
+99
| Future<void> showEventNotification(String eventId) async { | ||
| if (!_initialized || !Platform.isAndroid) return; | ||
| if (!isEventEnabled(eventId)) return; | ||
|
|
||
| // 用户可能在设置里开了开关但系统层面没给权限, 这里兜一层 | ||
| if (!await Permission.notification.isGranted) { | ||
| Get.log('通知权限未授予, 跳过事件通知: $eventId'); | ||
| return; | ||
| } | ||
|
|
||
| final id = StorageKeys.notificationIds[eventId]; | ||
| if (id == null) { | ||
| Get.log('未知的通知事件: $eventId'); | ||
| return; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Move the permission check inside the try block.
The doc comment states that failures are skipped silently. Line 90 awaits Permission.notification.isGranted outside the try, and the map lookups also sit outside it. The caller in lib/page/home/controller.dart at line 520 does not await this future and attaches no error handler. If the permission check throws, the result is an unhandled async error rather than a silent skip. Wrap the whole body so every failure path is contained.
🛡️ Proposed fix
Future<void> showEventNotification(String eventId) async {
if (!_initialized || !Platform.isAndroid) return;
if (!isEventEnabled(eventId)) return;
- // 用户可能在设置里开了开关但系统层面没给权限, 这里兜一层
- if (!await Permission.notification.isGranted) {
- Get.log('通知权限未授予, 跳过事件通知: $eventId');
- return;
- }
-
- final id = StorageKeys.notificationIds[eventId];
- if (id == null) {
- Get.log('未知的通知事件: $eventId');
- return;
- }
-
const androidDetails = AndroidNotificationDetails(
StorageKeys.notificationChannelEvents,
StorageKeys.notificationChannelEventsName,
channelDescription: StorageKeys.notificationChannelEventsDesc,
importance: Importance.high,
priority: Priority.high,
// 点击后自动消失, 用户不需要手动清
autoCancel: true,
);
try {
+ // 用户可能在设置里开了开关但系统层面没给权限, 这里兜一层
+ if (!await Permission.notification.isGranted) {
+ Get.log('通知权限未授予, 跳过事件通知: $eventId');
+ return;
+ }
+
+ final id = StorageKeys.notificationIds[eventId];
+ if (id == null) {
+ Get.log('未知的通知事件: $eventId');
+ return;
+ }
+
await _plugin.show(🤖 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/service/notification_service.dart` around lines 85 - 99, Wrap the entire
body of showEventNotification, including the permission check and
StorageKeys.notificationIds lookup, in the existing try block so all failures
are contained and skipped silently. Keep the current early-return behavior for
uninitialized, non-Android, disabled, ungranted-permission, and unknown-event
cases.
| path: ^1.9.1 | ||
| flutter_background_service: ^5.1.0 | ||
| flutter_local_notifications: ^21.0.0 | ||
| flutter_local_notifications: ^22.1.0 |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Declared SDK constraints and the resolved plugin version.
fd -t f 'pubspec.yaml' -x rg -n 'sdk:|flutter:|flutter_local_notifications' {}
fd -t f 'pubspec.lock' -x rg -n -A3 'flutter_local_notifications' {}
# Android compileSdk / minSdk settings.
fd -t f -e gradle -e kts . android 2>/dev/null | while IFS= read -r f; do
rg -n 'compileSdk|minSdk|targetSdk' "$f" || true
done
# Published versions of the plugin.
curl -s https://pub.dev/api/packages/flutter_local_notifications | jq -r '.latest.version, (.versions[-8:][].version)'Repository: Merack/time_machine
Length of output: 1999
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## pubspec.yaml relevant sections"
sed -n '1,55p' pubspec.yaml
echo
echo "## pubspec.lock flutter_local_notifications full block"
python3 - <<'PY'
from pathlib import Path
text=Path('pubspec.lock').read_text()
start=text.index('flutter_local_notifications:')
end=text.index('\n flutter_local_notifications_linux:', start)+1
print(path)
PY
sed -n '285,298p' pubspec.lock
echo
echo "## android build.gradle relevant sections"
fd -t f -e gradle -e kts . android 2>/dev/null | while IFS= read -r f; do
echo "--- $f"
rg -n -C 3 'compileSdk|minSdk|targetSdk|compileOptions|kotlinOptions|compileSdkVersion|minSdkVersion|targetSdkVersion' "$f" || true
done
echo
echo "## CI Flutter SDK/config references"
git ls-files | rg '(^\.github/workflows/.*\.ya?ml$|(^|/)(pubspec.yaml|analysis_options.yaml)$)' | while IFS= read -r f; do
echo "--- $f"
rg -n 'flutter:|sdk:|uses:.*flutter|fvm|flutter-build|flutter test|flutter pub|environment\.sdk|flutter_local_notifications' "$f" || true
doneRepository: Merack/time_machine
Length of output: 2692
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## pubspec.lock flutter_local_notifications lock block"
sed -n '285,298p' pubspec.lock
echo
echo "## android gradle files compile/min/target settings"
for f in android/app/build.gradle android/build.gradle $(fd -t f -e gradle . android); do
[ -f "$f" ] || continue
echo "--- $f"
rg -n -C 3 'compileSdk|minSdk|targetSdk|compileSdkVersion|minSdkVersion|targetSdkVersion|compileOptions|kotlinOptions' "$f" || true
done
echo
echo "## CI/flutter config references"
for f in $(git ls-files | rg '(^(\.github/workflows/.*\.ya?ml|pubspec.yaml|analysis_options.yaml)$)'); do
echo "--- $f"
rg -n 'flutter:|sdk:|uses:.*flutter|fvm|pubspec.yaml|flutter test|flutter build|environment\.sdk|flutter_local_notifications' "$f" || true
done
echo
echo "## package manifest floor for resolved version from pub"
python3 - <<'PY'
import json, urllib.request
url='https://pub.flutter-io.cn/api/packages/flutter_local_notifications/versions/22.1.0'
try:
with urllib.request.urlopen(url, timeout=20) as r:
data=json.load(r)
except Exception as e:
print("FETCH_ERROR", repr(e))
raise SystemExit
print("version:", data.get('version'))
print("sdk:", data.get('environment', {}).get('sdk'))
print("flutter:", data.get('environment', {}).get('flutter'))
print("dependency_platforms:", data.get('dependency', {}).get('dependency_platforms'))
PYRepository: Merack/time_machine
Length of output: 1471
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## android/build.gradle"
sed -n '1,120p' android/build.gradle
echo
echo "## android/app/build.gradle"
sed -n '1,140p' android/app/build.gradle
echo
echo "## workflow versions"
sed -n '1,90p' .github/workflows/dev.yml
sed -n '1,90p' .github/workflows/release.yml
echo
echo "## try fetching platform manifest with SSL bypass"
python3 - <<'PY'
import json, urllib.request, ssl
url='https://pub.dev/api/packages/flutter_local_notifications/versions/22.1.0'
ctx=ssl._create_unverified_context() if hasattr(ssl, '_create_unverified_context') else None
req=urllib.request.Request(url, headers={'User-Agent': 'CodeRabbit-verifier'})
try:
with urllib.request.urlopen(req, timeout=20, context=ctx) as r:
data=json.load(r)
print('version:', data.get('version'))
print('sdk:', data.get('environment', {}).get('sdk'))
print('flutter:', data.get('environment', {}).get('flutter'))
except Exception as e:
print('FETCH_ERROR', repr(e))
PY
echo
echo "## check lock hash against dev API"
python3 - <<'PY'
import json, urllib.request
sha='40c6a69189a622bda89ddcf50a139f4ba0f0eb9c0fef6718845b1f8b95452ed6'
url=f'https://pub.dev/api/packages/flutter_local_notifications/versions/22.1.0'
for ctx in [urllib.request.SSLContext(urllib.request.SSLContext.PROTOCOL_TLS), None]:
try:
req=urllib.request.Request(url, headers={'User-Agent': 'CodeRabbit-verifier'})
with urllib.request.urlopen(req, timeout=20, context=ctx or None) as r:
data=json.load(r)
print("fetch_ok", data.get('version'), data.get('sha256'))
print("matched", data.get('sha256') == sha)
break
except Exception as e:
print("try_error", type(e).__name__, e)
PYRepository: Merack/time_machine
Length of output: 245
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## locate gradle files"
find android -maxdepth 4 -type f \( -name '*.gradle' -o -name '*.gradle.kts' \) -print | sort
echo
echo "## read all gradle files"
while IFS= read -r f; do
echo "--- $f"
sed -n '1,160p' "$f"
done < <(find android -maxdepth 4 -type f \( -name '*.gradle' -o -name '*.gradle.kts' \) -print | sort)
echo
echo "## workflow versions"
for f in .github/workflows/dev.yml .github/workflows/release.yml; do
[ -f "$f" ] || continue
echo "--- $f"
sed -n '1,90p' "$f"
done
echo
echo "## fetch 22.1.0 platform metadata"
python3 - <<'PY'
import json, ssl, urllib.request
url='https://pub.dev/api/packages/flutter_local_notifications/versions/22.1.0'
headers={'User-Agent': 'CodeRabbit-verifier'}
ctx=ssl._create_unverified_context() if hasattr(ssl, '_create_unverified_context') else None
for retry_ctx in [ctx, None]:
try:
req=urllib.request.Request(url, headers=headers, method='GET')
with urllib.request.urlopen(req, timeout=20, context=retry_ctx) as r:
data=json.load(r)
print('version:', data.get('version'))
print('sha256:', data.get('sha256'))
print('android compileSdk:', data.get('dependency', {}).get('dependency_platforms', {}).get('android', {}).get('min_sdk', {}).get('compile_sdk'))
print('environment:', data.get('environment', {}))
break
except Exception as e:
print('try:', type(e).__name__, str(e))
PYRepository: Merack/time_machine
Length of output: 341
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## android gradle files"
for f in android/build.gradle.kts android/app/build.gradle.kts android/settings.gradle.kts; do
[ -f "$f" ] || continue
echo "--- $f"
sed -n '1,180p' "$f"
done
echo
echo "## workflow versions"
for f in .github/workflows/dev.yml .github/workflows/release.yml; do
[ -f "$f" ] || continue
echo "--- $f"
sed -n '1,90p' "$f"
done
echo
echo "## fetch 22.1.0 platform metadata"
python3 - <<'PY'
import json, ssl, urllib.request
url='https://pub.dev/api/packages/flutter_local_notifications/versions/22.1.0'
headers={'User-Agent': 'CodeRabbit-verifier'}
ctx=ssl._create_unverified_context() if hasattr(ssl, '_create_unverified_context') else None
for retry_ctx in [ctx, None]:
try:
req=urllib.request.Request(url, headers=headers, method='GET')
with urllib.request.urlopen(req, timeout=20, context=retry_ctx) as r:
data=json.load(r)
print('version:', data.get('version'))
print('sha256:', data.get('sha256'))
print('android compileSdk:', data.get('dependency', {}).get('dependency_platforms', {}).get('android', {}).get('min_sdk', {}).get('compile_sdk'))
print('environment:', data.get('environment', {}))
break
except Exception as e:
print('try:', type(e).__name__, str(e))
PYRepository: Merack/time_machine
Length of output: 10258
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## try pub.dev package summary without custom context"
python3 - <<'PY'
import json, urllib.request
url='https://pub.dev/api/packages/flutter_local_notifications'
req=urllib.request.Request(url, headers={'User-Agent': 'CodeRabbit-verifier'})
with urllib.request.urlopen(req, timeout=20) as r:
data=json.load(r)
print("latest", data.get("latest", {}).get("version"))
print("latest_sha256", data.get("latest", {}).get("sha256"))
versions=data.get("versions", [])
for v in versions[-6:]:
pv=v.get("version")
try:
uv=urllib.request.urlopen(f'https://pub.dev/api/packages/flutter_local_notifications/versions/{pv}', timeout=20).read().decode()
info=json.loads(uv)
print(pv, "sha256=", info.get("sha256"), "environment=", info.get("environment"))
except Exception as e:
print(pv, "error", type(e).__name__, e)
PY
echo
echo "## local lock version/hash"
sed -n '289,295p' pubspec.lock
echo
echo "## workflow sdk pins"
rg -n "flutter-version|channel|java-version|subosito/flutter-action" .github/workflows/*.ymlRepository: Merack/time_machine
Length of output: 2497
Confirm the Flutter SDK floor before using flutter_local_notifications 22.x.
pubspec.lock resolves to the published 22.1.0 stable release, and Android uses Flutter-managed SDK values. However, pubspec.yaml has no Dart SDK floor, and the workflows only specify flutter-version: '3.x', so require an explicit Flutter >=3.27.0 constraint or pin the workflow Flutter version to satisfy the 22.x platform requirements.
🤖 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 `@pubspec.yaml` at line 40, Update the project SDK constraints around the
flutter_local_notifications dependency to require Flutter >=3.27.0, and ensure
workflow Flutter versions satisfy that floor; alternatively, pin all workflow
Flutter versions to a compatible release. Keep the 22.1.0 dependency unchanged.
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.
Summary by CodeRabbit