Add Daily Puzzle Notification#218
Open
amorris13 wants to merge 1 commit into
Open
Conversation
- Implement DailyNotificationReceiver to show reminders if today's puzzle is incomplete. - Add NotificationUtils for scheduling alarms via AlarmManager. - Add notification settings (toggle and time picker) to both the Settings page and Daily Statistics page. - Ensure reboot persistence with OnBootReceiver and RECEIVE_BOOT_COMPLETED permission. - Handle POST_NOTIFICATIONS permission for Android 13+. - Schedule notifications on app launch in MainActivity. - Fix race conditions and ensure lifecycle safety in SettingsFragment. Co-authored-by: amorris13 <4523811+amorris13@users.noreply.github.com>
Contributor
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
📝 WalkthroughWalkthroughThe changes implement a daily puzzle notification system allowing users to opt-in and configure notification time. The system uses AlarmManager for scheduling repeating notifications, broadcasts receivers for posting notifications and rescheduling on device boot, and preference/UI controls in both the daily statistics page and settings. Notifications only trigger if the daily puzzle remains incomplete. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Settings as DailyStatsFragment
participant Utils as NotificationUtils
participant AM as AlarmManager
participant SharedPrefs as SharedPreferences
User->>Settings: Toggle notification switch
activate Settings
Settings->>SharedPrefs: Save enabled=true + hour/minute
activate SharedPrefs
SharedPrefs-->>Settings:
deactivate SharedPrefs
Settings->>Utils: scheduleDailyNotification(context)
activate Utils
Utils->>SharedPrefs: Read enabled, hour, minute
activate SharedPrefs
SharedPrefs-->>Utils: preferences
deactivate SharedPrefs
Utils->>AM: setInexactRepeating(RTC_WAKEUP, ..., intent)
activate AM
AM-->>Utils:
deactivate AM
Utils-->>Settings:
deactivate Utils
Settings-->>User: Alarm scheduled
deactivate Settings
sequenceDiagram
participant AM as AlarmManager
participant Receiver as DailyNotificationReceiver
participant App as Application
participant Game as DailyGame
participant NM as NotificationManager
participant User as User
AM->>Receiver: onReceive(intent)
activate Receiver
Receiver->>App: Retrieve singleton
activate App
App-->>Receiver: Application instance
deactivate App
Receiver->>Game: Get today's game
activate Game
Game-->>Receiver: Game data
deactivate Game
alt Game incomplete
Receiver->>NM: createNotificationChannel (API 26+)
Receiver->>NM: notify(NOTIFICATION_ID, notification)
activate NM
NM-->>User: Display notification
deactivate NM
else Game completed
Receiver->>Receiver: Skip notification
end
deactivate Receiver
sequenceDiagram
participant System as System
participant BootReceiver as OnBootReceiver
participant Utils as NotificationUtils
participant AM as AlarmManager
System->>BootReceiver: ACTION_BOOT_COMPLETED
activate BootReceiver
BootReceiver->>Utils: scheduleDailyNotification(context)
activate Utils
Utils->>AM: setInexactRepeating(...)
activate AM
AM-->>Utils: Alarm scheduled
deactivate AM
Utils-->>BootReceiver:
deactivate Utils
BootReceiver-->>System:
deactivate BootReceiver
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 4
🧹 Nitpick comments (5)
app/src/main/res/xml/preferences.xml (1)
49-63: Inconsistent preference key format.Other preferences in this file use string resources for keys (e.g.,
@string/pref_screen_lock), but the new notification preferences use hardcoded strings. This inconsistency could complicate future maintenance and i18n.Additionally, the category title "Daily Puzzle" should use a string resource.
♻️ Suggested consistency improvement
Add to
strings.xml:<string name="pref_daily_notification_enabled">pref_daily_notification_enabled</string> <string name="pref_daily_notification_time">pref_daily_notification_time</string> <string name="daily_puzzle_category_title">Daily Puzzle</string>Then update
preferences.xml:<androidx.preference.PreferenceCategory - android:title="Daily Puzzle"> + android:title="@string/daily_puzzle_category_title"> <androidx.preference.SwitchPreferenceCompat android:defaultValue="false" - android:key="pref_daily_notification_enabled" + android:key="@string/pref_daily_notification_enabled" android:summary="@string/pref_daily_notification_enabled_summary" android:title="@string/pref_daily_notification_enabled_title" /> <androidx.preference.Preference - android:dependency="pref_daily_notification_enabled" - android:key="pref_daily_notification_time" + android:dependency="@string/pref_daily_notification_enabled" + android:key="@string/pref_daily_notification_time" android:title="@string/pref_daily_notification_time_title" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/res/xml/preferences.xml` around lines 49 - 63, Replace the hardcoded preference keys and category title with string resources: add string entries for "pref_daily_notification_enabled", "pref_daily_notification_time", and "daily_puzzle_category_title" in strings.xml, then update the PreferenceCategory title to use `@string/daily_puzzle_category_title` and change the SwitchPreferenceCompat key and the Preference key to use `@string/pref_daily_notification_enabled` and `@string/pref_daily_notification_time` respectively; modify the Preference elements referenced (PreferenceCategory, SwitchPreferenceCompat, Preference) to use these `@string/`... keys to match the existing preference key format.app/src/main/res/layout/daily_stats_fragment.xml (1)
254-279: Consider addingcontentDescriptionfor accessibility.The
notification_time_containeris clickable and focusable but lacks acontentDescription. Screen reader users may not understand the purpose of this clickable row.♿ Suggested accessibility improvement
<LinearLayout android:id="@+id/notification_time_container" android:layout_width="match_parent" android:layout_height="wrap_content" android:orientation="horizontal" android:gravity="center_vertical" android:layout_marginTop="8dp" android:clickable="true" android:focusable="true" - android:background="?attr/selectableItemBackground"> + android:background="?attr/selectableItemBackground" + android:contentDescription="@string/pref_daily_notification_time_title">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/res/layout/daily_stats_fragment.xml` around lines 254 - 279, The clickable/focusable LinearLayout notification_time_container lacks an accessibility label; add a contentDescription (or use android:labelFor pointing to notification_time_tv) so screen readers announce its purpose—update the LinearLayout with android:contentDescription="@string/..." (or set it programmatically in the fragment/activity using findViewById(R.id.notification_time_container).setContentDescription(...)) and ensure the string resource describes the row (e.g., "Daily notification time, opens time picker").app/src/main/java/com/antsapps/triples/NotificationUtils.java (2)
34-34: Add null check forAlarmManagerservice.While extremely rare on real devices,
getSystemService()can returnnull. A defensive null check prevents potential NPE crashes.Proposed fix
AlarmManager alarmManager = (AlarmManager) context.getSystemService(Context.ALARM_SERVICE); + if (alarmManager == null) { + return; + } Intent intent = new Intent(context, DailyNotificationReceiver.class);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/antsapps/triples/NotificationUtils.java` at line 34, Add a defensive null check after calling context.getSystemService for the AlarmManager in NotificationUtils: verify that AlarmManager alarmManager is not null before using it; if it is null, log or report the condition (e.g., via Log.w or a logger) and return/skip scheduling rather than proceeding, so any subsequent calls that use alarmManager (in NotificationUtils methods) cannot throw an NPE.
56-63: Same null check recommendation forAlarmManager.For consistency and defensive coding, apply the same null check here as suggested for the scheduling method.
Proposed fix
public static void cancelDailyNotification(Context context) { AlarmManager alarmManager = (AlarmManager) context.getSystemService(Context.ALARM_SERVICE); + if (alarmManager == null) { + return; + } Intent intent = new Intent(context, DailyNotificationReceiver.class);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/antsapps/triples/NotificationUtils.java` around lines 56 - 63, The cancelDailyNotification method currently assumes AlarmManager is non-null; add the same defensive null check used in the scheduling method: retrieve AlarmManager via (AlarmManager) context.getSystemService(Context.ALARM_SERVICE), and only call alarmManager.cancel(pendingIntent) if alarmManager != null (leave creating the Intent, PendingIntent (PendingIntent.getBroadcast with FLAG_UPDATE_CURRENT|FLAG_IMMUTABLE) unchanged); this prevents a possible NPE when AlarmManager is unavailable while keeping the rest of cancelDailyNotification and the DailyNotificationReceiver/pendingIntent logic intact.app/src/main/java/com/antsapps/triples/DailyNotificationReceiver.java (1)
53-57: Empty catch block silently swallows notification failures.Consider logging the
SecurityExceptionfor diagnostic purposes, especially useful during development or when troubleshooting user-reported issues.Proposed fix
try { notificationManager.notify(NOTIFICATION_ID, builder.build()); } catch (SecurityException e) { - // Handle missing permission + // Missing POST_NOTIFICATIONS permission; user has denied or not yet granted + android.util.Log.w("DailyNotificationReceiver", "Notification permission not granted", e); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/antsapps/triples/DailyNotificationReceiver.java` around lines 53 - 57, The catch block around notificationManager.notify(NOTIFICATION_ID, builder.build()) in DailyNotificationReceiver currently swallows SecurityException; change it to log the exception (e.g., Log.e or your app's logger) including a clear message and the caught exception object so failures are visible during development/troubleshooting; keep the catch handling minimal (log the error and optionally record analytics) and do not rethrow to preserve behavior for missing permission.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/java/com/antsapps/triples/DailyNotificationReceiver.java`:
- Around line 34-37: The BroadcastReceiver calls
Application.getDailyGameForDate() which can trigger synchronous DB writes (via
DailyGame.createFromDay() -> addDailyGame() -> DBAdapter.addDailyGame()),
risking ANR; modify DailyNotificationReceiver.onReceive() to call
getDailyGameByGameDay() (which returns null if absent) and pass that DailyGame
instance into showNotification(), and update showNotification(DailyGame) to
handle a null game gracefully (e.g., show a fallback notification or schedule
background work to create the game), so no synchronous DB writes occur inside
the receiver.
In `@app/src/main/java/com/antsapps/triples/DailyStatisticsFragment.java`:
- Around line 99-109: The permission callback may call getContext() after the
fragment is detached causing an NPE; update the callback in
DailyStatisticsFragment to guard against a detached fragment by checking
isAdded() (and/or getContext() != null) before calling
PreferenceManager.getDefaultSharedPreferences(getContext()) and
NotificationUtils.scheduleDailyNotification(getContext(), true), and only
perform those actions when the fragment is still attached; if the fragment is
detached or context is null ensure you still update mNotificationSwitch safely
(e.g., setChecked(false) or skip UI work) to avoid touching views when not
attached.
- Around line 265-281: The TimePickerDialog callback currently calls
getContext(), prefs, updateNotificationTimeTv(), and
NotificationUtils.scheduleDailyNotification(...) without guarding against the
Fragment being detached; capture the fragment state and context defensively
inside the lambda: inside the callback first obtain Context ctx = getContext()
and/or check isAdded(), return early if ctx == null or !isAdded(), then use ctx
when editing prefs (or re-resolve SharedPreferences from ctx if prefs can be
null) and when calling NotificationUtils.scheduleDailyNotification(ctx); this
prevents a NullPointerException if the fragment is detached while the dialog is
shown.
In `@app/src/main/java/com/antsapps/triples/SettingsFragment.java`:
- Around line 125-148: timePref.setOnPreferenceClickListener's TimePickerDialog
callback calls getContext() directly which can be null during configuration
changes; capture a non-null Context/Fragment reference before creating the
dialog (e.g., Context ctx = getContext() or Activity act = getActivity()) and
use that captured reference inside the onTimeSet lambda for prefs edits,
updateTimePrefSummary and NotificationUtils.scheduleDailyNotification to avoid
NPEs, and bail out if the captured reference is null before showing the dialog.
---
Nitpick comments:
In `@app/src/main/java/com/antsapps/triples/DailyNotificationReceiver.java`:
- Around line 53-57: The catch block around
notificationManager.notify(NOTIFICATION_ID, builder.build()) in
DailyNotificationReceiver currently swallows SecurityException; change it to log
the exception (e.g., Log.e or your app's logger) including a clear message and
the caught exception object so failures are visible during
development/troubleshooting; keep the catch handling minimal (log the error and
optionally record analytics) and do not rethrow to preserve behavior for missing
permission.
In `@app/src/main/java/com/antsapps/triples/NotificationUtils.java`:
- Line 34: Add a defensive null check after calling context.getSystemService for
the AlarmManager in NotificationUtils: verify that AlarmManager alarmManager is
not null before using it; if it is null, log or report the condition (e.g., via
Log.w or a logger) and return/skip scheduling rather than proceeding, so any
subsequent calls that use alarmManager (in NotificationUtils methods) cannot
throw an NPE.
- Around line 56-63: The cancelDailyNotification method currently assumes
AlarmManager is non-null; add the same defensive null check used in the
scheduling method: retrieve AlarmManager via (AlarmManager)
context.getSystemService(Context.ALARM_SERVICE), and only call
alarmManager.cancel(pendingIntent) if alarmManager != null (leave creating the
Intent, PendingIntent (PendingIntent.getBroadcast with
FLAG_UPDATE_CURRENT|FLAG_IMMUTABLE) unchanged); this prevents a possible NPE
when AlarmManager is unavailable while keeping the rest of
cancelDailyNotification and the DailyNotificationReceiver/pendingIntent logic
intact.
In `@app/src/main/res/layout/daily_stats_fragment.xml`:
- Around line 254-279: The clickable/focusable LinearLayout
notification_time_container lacks an accessibility label; add a
contentDescription (or use android:labelFor pointing to notification_time_tv) so
screen readers announce its purpose—update the LinearLayout with
android:contentDescription="@string/..." (or set it programmatically in the
fragment/activity using
findViewById(R.id.notification_time_container).setContentDescription(...)) and
ensure the string resource describes the row (e.g., "Daily notification time,
opens time picker").
In `@app/src/main/res/xml/preferences.xml`:
- Around line 49-63: Replace the hardcoded preference keys and category title
with string resources: add string entries for "pref_daily_notification_enabled",
"pref_daily_notification_time", and "daily_puzzle_category_title" in
strings.xml, then update the PreferenceCategory title to use
`@string/daily_puzzle_category_title` and change the SwitchPreferenceCompat key
and the Preference key to use `@string/pref_daily_notification_enabled` and
`@string/pref_daily_notification_time` respectively; modify the Preference
elements referenced (PreferenceCategory, SwitchPreferenceCompat, Preference) to
use these `@string/`... keys to match the existing preference key format.
🪄 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
Run ID: 9f0f9d4a-1358-4170-b3c5-3964102bc40d
⛔ Files ignored due to path filters (4)
app/src/test/screenshots/settings_dark.pngis excluded by!**/*.pngapp/src/test/screenshots/settings_light.pngis excluded by!**/*.pngapp/src/test/screenshots/statistics_daily_dark.pngis excluded by!**/*.pngapp/src/test/screenshots/statistics_daily_light.pngis excluded by!**/*.png
📒 Files selected for processing (10)
app/src/main/AndroidManifest.xmlapp/src/main/java/com/antsapps/triples/DailyNotificationReceiver.javaapp/src/main/java/com/antsapps/triples/DailyStatisticsFragment.javaapp/src/main/java/com/antsapps/triples/MainActivity.javaapp/src/main/java/com/antsapps/triples/NotificationUtils.javaapp/src/main/java/com/antsapps/triples/OnBootReceiver.javaapp/src/main/java/com/antsapps/triples/SettingsFragment.javaapp/src/main/res/layout/daily_stats_fragment.xmlapp/src/main/res/values/strings.xmlapp/src/main/res/xml/preferences.xml
Comment on lines
+34
to
+37
| Intent intent = new Intent(context, DailyGameActivity.class); | ||
| DailyGame todayGame = | ||
| Application.getInstance(context).getDailyGameForDate(DailyGame.Day.forToday()); | ||
| intent.putExtra(Game.ID_TAG, todayGame.getId()); |
There was a problem hiding this comment.
Potential ANR: getDailyGameForDate() may perform synchronous database writes in a BroadcastReceiver.
Per the context snippet for Application.getDailyGameForDate(), if the game doesn't exist in memory, it calls DailyGame.createFromDay() and addDailyGame() which triggers synchronous database writes via DBAdapter.addDailyGame(). BroadcastReceivers have a ~10 second execution limit, and disk I/O could cause ANR on slower devices.
Since onReceive() already calls getDailyGameByGameDay() which returns null if the game doesn't exist, consider passing the game instance to showNotification() or using a different approach when the game is null.
Proposed fix: Pass the game from onReceive() and handle null case
`@Override`
public void onReceive(Context context, Intent intent) {
Application application = Application.getInstance(context);
DailyGame todayGame = application.getDailyGameByGameDay(DailyGame.Day.forToday());
if (todayGame == null || todayGame.getGameState() != Game.GameState.COMPLETED) {
- showNotification(context);
+ showNotification(context, todayGame);
}
}
-private void showNotification(Context context) {
+private void showNotification(Context context, DailyGame todayGame) {
createNotificationChannel(context);
Intent intent = new Intent(context, DailyGameActivity.class);
- DailyGame todayGame =
- Application.getInstance(context).getDailyGameForDate(DailyGame.Day.forToday());
- intent.putExtra(Game.ID_TAG, todayGame.getId());
+ // Only include game ID if available; DailyGameActivity handles missing ID
+ if (todayGame != null) {
+ intent.putExtra(Game.ID_TAG, todayGame.getId());
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/com/antsapps/triples/DailyNotificationReceiver.java` around
lines 34 - 37, The BroadcastReceiver calls Application.getDailyGameForDate()
which can trigger synchronous DB writes (via DailyGame.createFromDay() ->
addDailyGame() -> DBAdapter.addDailyGame()), risking ANR; modify
DailyNotificationReceiver.onReceive() to call getDailyGameByGameDay() (which
returns null if absent) and pass that DailyGame instance into
showNotification(), and update showNotification(DailyGame) to handle a null game
gracefully (e.g., show a fallback notification or schedule background work to
create the game), so no synchronous DB writes occur inside the receiver.
Comment on lines
+99
to
+109
| isGranted -> { | ||
| if (isGranted) { | ||
| PreferenceManager.getDefaultSharedPreferences(getContext()) | ||
| .edit() | ||
| .putBoolean(NotificationUtils.PREF_DAILY_NOTIFICATION_ENABLED, true) | ||
| .apply(); | ||
| NotificationUtils.scheduleDailyNotification(getContext(), true); | ||
| } else { | ||
| mNotificationSwitch.setChecked(false); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Potential NullPointerException if fragment is detached during permission callback.
The permission result callback uses getContext() without null checks. If the fragment is detached before the callback executes, this will cause a crash.
🛡️ Suggested defensive fix
mRequestPermissionLauncher =
registerForActivityResult(
new ActivityResultContracts.RequestPermission(),
isGranted -> {
+ Context context = getContext();
+ if (context == null) return;
if (isGranted) {
- PreferenceManager.getDefaultSharedPreferences(getContext())
+ PreferenceManager.getDefaultSharedPreferences(context)
.edit()
.putBoolean(NotificationUtils.PREF_DAILY_NOTIFICATION_ENABLED, true)
.apply();
- NotificationUtils.scheduleDailyNotification(getContext(), true);
+ NotificationUtils.scheduleDailyNotification(context, true);
} else {
mNotificationSwitch.setChecked(false);
}
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/com/antsapps/triples/DailyStatisticsFragment.java` around
lines 99 - 109, The permission callback may call getContext() after the fragment
is detached causing an NPE; update the callback in DailyStatisticsFragment to
guard against a detached fragment by checking isAdded() (and/or getContext() !=
null) before calling PreferenceManager.getDefaultSharedPreferences(getContext())
and NotificationUtils.scheduleDailyNotification(getContext(), true), and only
perform those actions when the fragment is still attached; if the fragment is
detached or context is null ensure you still update mNotificationSwitch safely
(e.g., setChecked(false) or skip UI work) to avoid touching views when not
attached.
Comment on lines
+265
to
+281
| TimePickerDialog timePickerDialog = | ||
| new TimePickerDialog( | ||
| getContext(), | ||
| (view, hourOfDay, minuteOfHour) -> { | ||
| prefs | ||
| .edit() | ||
| .putInt(NotificationUtils.PREF_DAILY_NOTIFICATION_HOUR, hourOfDay) | ||
| .putInt(NotificationUtils.PREF_DAILY_NOTIFICATION_MINUTE, minuteOfHour) | ||
| .apply(); | ||
| updateNotificationTimeTv(); | ||
| NotificationUtils.scheduleDailyNotification(getContext()); | ||
| }, | ||
| hour, | ||
| minute, | ||
| android.text.format.DateFormat.is24HourFormat(getContext())); | ||
| timePickerDialog.show(); | ||
| }); |
There was a problem hiding this comment.
Potential NullPointerException in TimePickerDialog callback.
The TimePickerDialog callback uses getContext() without null checks. If the fragment is detached while the dialog is showing (e.g., configuration change), the callback will crash when accessing preferences or scheduling notifications.
🛡️ Suggested defensive fix
TimePickerDialog timePickerDialog =
new TimePickerDialog(
getContext(),
(view, hourOfDay, minuteOfHour) -> {
+ Context context = getContext();
+ if (context == null) return;
+ SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(context);
prefs
.edit()
.putInt(NotificationUtils.PREF_DAILY_NOTIFICATION_HOUR, hourOfDay)
.putInt(NotificationUtils.PREF_DAILY_NOTIFICATION_MINUTE, minuteOfHour)
.apply();
updateNotificationTimeTv();
- NotificationUtils.scheduleDailyNotification(getContext());
+ NotificationUtils.scheduleDailyNotification(context);
},🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/com/antsapps/triples/DailyStatisticsFragment.java` around
lines 265 - 281, The TimePickerDialog callback currently calls getContext(),
prefs, updateNotificationTimeTv(), and
NotificationUtils.scheduleDailyNotification(...) without guarding against the
Fragment being detached; capture the fragment state and context defensively
inside the lambda: inside the callback first obtain Context ctx = getContext()
and/or check isAdded(), return early if ctx == null or !isAdded(), then use ctx
when editing prefs (or re-resolve SharedPreferences from ctx if prefs can be
null) and when calling NotificationUtils.scheduleDailyNotification(ctx); this
prevents a NullPointerException if the fragment is detached while the dialog is
shown.
Comment on lines
+125
to
+148
| timePref.setOnPreferenceClickListener( | ||
| preference -> { | ||
| SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(getContext()); | ||
| int hour = prefs.getInt(NotificationUtils.PREF_DAILY_NOTIFICATION_HOUR, 20); | ||
| int minute = prefs.getInt(NotificationUtils.PREF_DAILY_NOTIFICATION_MINUTE, 0); | ||
|
|
||
| TimePickerDialog timePickerDialog = | ||
| new TimePickerDialog( | ||
| getContext(), | ||
| (view, hourOfDay, minuteOfHour) -> { | ||
| prefs | ||
| .edit() | ||
| .putInt(NotificationUtils.PREF_DAILY_NOTIFICATION_HOUR, hourOfDay) | ||
| .putInt(NotificationUtils.PREF_DAILY_NOTIFICATION_MINUTE, minuteOfHour) | ||
| .apply(); | ||
| updateTimePrefSummary(timePref); | ||
| NotificationUtils.scheduleDailyNotification(getContext()); | ||
| }, | ||
| hour, | ||
| minute, | ||
| android.text.format.DateFormat.is24HourFormat(getContext())); | ||
| timePickerDialog.show(); | ||
| return true; | ||
| }); |
There was a problem hiding this comment.
Potential NullPointerException in TimePickerDialog callback.
Similar to DailyStatisticsFragment, the time picker callback uses getContext() without null checks. Configuration changes while the dialog is showing could cause a crash.
🛡️ Suggested defensive fix
timePref.setOnPreferenceClickListener(
preference -> {
- SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(getContext());
+ Context context = getContext();
+ if (context == null) return true;
+ SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(context);
int hour = prefs.getInt(NotificationUtils.PREF_DAILY_NOTIFICATION_HOUR, 20);
int minute = prefs.getInt(NotificationUtils.PREF_DAILY_NOTIFICATION_MINUTE, 0);
TimePickerDialog timePickerDialog =
new TimePickerDialog(
- getContext(),
+ context,
(view, hourOfDay, minuteOfHour) -> {
+ Context ctx = getContext();
+ if (ctx == null) return;
prefs
.edit()
.putInt(NotificationUtils.PREF_DAILY_NOTIFICATION_HOUR, hourOfDay)
.putInt(NotificationUtils.PREF_DAILY_NOTIFICATION_MINUTE, minuteOfHour)
.apply();
updateTimePrefSummary(timePref);
- NotificationUtils.scheduleDailyNotification(getContext());
+ NotificationUtils.scheduleDailyNotification(ctx);
},🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/com/antsapps/triples/SettingsFragment.java` around lines
125 - 148, timePref.setOnPreferenceClickListener's TimePickerDialog callback
calls getContext() directly which can be null during configuration changes;
capture a non-null Context/Fragment reference before creating the dialog (e.g.,
Context ctx = getContext() or Activity act = getActivity()) and use that
captured reference inside the onTimeSet lambda for prefs edits,
updateTimePrefSummary and NotificationUtils.scheduleDailyNotification to avoid
NPEs, and bail out if the captured reference is null before showing the dialog.
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.
This change adds a daily reminder notification for the daily puzzle. Users can opt-in and configure the reminder time (default 8 PM) from either the Daily Statistics page or the main Settings page. The notification is only shown if the user hasn't completed the puzzle for the current day. The implementation includes reboot persistence and handles modern Android notification permissions.
Fixes #217
PR created automatically by Jules for task 2702830574255099670 started by @amorris13
Summary
This pull request implements an opt-in daily reminder notification for the daily puzzle. Users can enable the feature and configure the reminder time (default 8 PM) from the Daily Statistics page or Settings page. Notifications are only shown if the user hasn't completed that day's puzzle.
CC: @jules
Implementation Details
Core Notification System
User Interface
DailyStatisticsFragment: Adds a new settings section with:
SettingsFragment: Integrates notification settings into the main Settings page with:
Manifest & Permissions
App Integration
Key Features
String Resources
Added 9 new string resources for UI labels, summaries, notification title/content, and channel metadata.