diff --git a/COMMIT_MESSAGE.txt b/COMMIT_MESSAGE.txt index 6f5ed6f..a8f07d1 100644 --- a/COMMIT_MESSAGE.txt +++ b/COMMIT_MESSAGE.txt @@ -1,25 +1,22 @@ -refactor(android): P2.1 Batch B - delegate validation methods to services +refactor(android): Batch 0 - centralize constants in DailyNotificationConstants -Refactor plugin methods that validate input then delegate to services: -- requestNotificationPermissions() → PermissionManager -- openChannelSettings() → ChannelManager -- createSchedule/updateSchedule/deleteSchedule/enableSchedule() → ScheduleHelper -- scheduleUserNotification() → ScheduleHelper (database operations) -- registerCallback() → CallbackHelper -- injectInvalidTestData() → TestDataHelper -- requestExactAlarmPermission() → PermissionManager -- openExactAlarmSettings() → PermissionManager -- checkExactAlarmPermission() → PermissionManager -- cancelAllNotifications() → ScheduleHelper (database operations, partial) -- testAlarm() → DailyNotificationScheduler +Create DailyNotificationConstants.kt to eliminate magic numbers and string +duplication across the codebase. -Enhanced services: -- PermissionManager: Added checkExactAlarmPermission() and requestExactAlarmPermission() -- ChannelManager: Enhanced openChannelSettings() with channelId parameter and fallback logic -- ScheduleHelper: Added disableAllSchedulesByKind() method -- DailyNotificationScheduler: Added testAlarm() wrapper method +Centralized constants: +- PERMISSION_REQUEST_CODE (1001) +- DEFAULT_CHANNEL_ID, DEFAULT_CHANNEL_NAME, DEFAULT_CHANNEL_DESCRIPTION +- ACTION_NOTIFICATION, EXTRA_NOTIFICATION_ID +- PREFS_NAME (SharedPreferences file name) +- WorkManager tags, schedule IDs, notification IDs -Reduces plugin class complexity by ~200 lines. -Services already exist - this is delegation, not extraction. +Replaced duplicates in: +- DailyNotificationPlugin.kt (PERMISSION_REQUEST_CODE, PREFS_NAME) +- PermissionManager.java (PERMISSION_REQUEST_CODE) +- ChannelManager.java (all channel constants) +- DailyNotificationScheduler.java (ACTION_NOTIFICATION, EXTRA_NOTIFICATION_ID) -Refs: docs/progress/P2.1-BATCH-B-STATE.md +This is the foundation for the remaining refactoring batches. +All files compile and reference the centralized constants. + +Refs: Cursor directive Batch 0 diff --git a/android/src/main/java/com/timesafari/dailynotification/ChannelManager.java b/android/src/main/java/com/timesafari/dailynotification/ChannelManager.java index d4be88c..affbeff 100644 --- a/android/src/main/java/com/timesafari/dailynotification/ChannelManager.java +++ b/android/src/main/java/com/timesafari/dailynotification/ChannelManager.java @@ -21,9 +21,8 @@ import android.util.Log; */ public class ChannelManager { private static final String TAG = "ChannelManager"; - private static final String DEFAULT_CHANNEL_ID = "timesafari.daily"; - private static final String DEFAULT_CHANNEL_NAME = "Daily Notifications"; - private static final String DEFAULT_CHANNEL_DESCRIPTION = "Daily notifications from TimeSafari"; + // Channel constants moved to DailyNotificationConstants + // Use DailyNotificationConstants.DEFAULT_CHANNEL_ID, etc. private final Context context; private final NotificationManager notificationManager; @@ -44,7 +43,7 @@ public class ChannelManager { Log.d(TAG, "Ensuring notification channel exists"); if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { - NotificationChannel channel = notificationManager.getNotificationChannel(DEFAULT_CHANNEL_ID); + NotificationChannel channel = notificationManager.getNotificationChannel(com.timesafari.dailynotification.DailyNotificationConstants.DEFAULT_CHANNEL_ID); if (channel == null) { Log.d(TAG, "Creating notification channel"); @@ -73,7 +72,7 @@ public class ChannelManager { public boolean isChannelEnabled() { try { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { - NotificationChannel channel = notificationManager.getNotificationChannel(DEFAULT_CHANNEL_ID); + NotificationChannel channel = notificationManager.getNotificationChannel(com.timesafari.dailynotification.DailyNotificationConstants.DEFAULT_CHANNEL_ID); if (channel == null) { Log.w(TAG, "Channel does not exist"); return false; @@ -100,7 +99,7 @@ public class ChannelManager { public int getChannelImportance() { try { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { - NotificationChannel channel = notificationManager.getNotificationChannel(DEFAULT_CHANNEL_ID); + NotificationChannel channel = notificationManager.getNotificationChannel(com.timesafari.dailynotification.DailyNotificationConstants.DEFAULT_CHANNEL_ID); if (channel != null) { return channel.getImportance(); } @@ -118,7 +117,7 @@ public class ChannelManager { * @return true if settings intent was launched, false otherwise */ public boolean openChannelSettings() { - return openChannelSettings(DEFAULT_CHANNEL_ID); + return openChannelSettings(com.timesafari.dailynotification.DailyNotificationConstants.DEFAULT_CHANNEL_ID); } /** @@ -143,7 +142,7 @@ public class ChannelManager { try { Intent intent = new Intent(Settings.ACTION_CHANNEL_NOTIFICATION_SETTINGS) .putExtra(Settings.EXTRA_APP_PACKAGE, context.getPackageName()) - .putExtra(Settings.EXTRA_CHANNEL_ID, channelId != null ? channelId : DEFAULT_CHANNEL_ID) + .putExtra(Settings.EXTRA_CHANNEL_ID, channelId != null ? channelId : com.timesafari.dailynotification.DailyNotificationConstants.DEFAULT_CHANNEL_ID) .addFlags(Intent.FLAG_ACTIVITY_NEW_TASK); context.startActivity(intent); @@ -181,11 +180,11 @@ public class ChannelManager { private void createDefaultChannel() { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { NotificationChannel channel = new NotificationChannel( - DEFAULT_CHANNEL_ID, - DEFAULT_CHANNEL_NAME, + com.timesafari.dailynotification.DailyNotificationConstants.DEFAULT_CHANNEL_ID, + com.timesafari.dailynotification.DailyNotificationConstants.DEFAULT_CHANNEL_NAME, NotificationManager.IMPORTANCE_HIGH ); - channel.setDescription(DEFAULT_CHANNEL_DESCRIPTION); + channel.setDescription(com.timesafari.dailynotification.DailyNotificationConstants.DEFAULT_CHANNEL_DESCRIPTION); channel.enableLights(true); channel.enableVibration(true); channel.setShowBadge(true); @@ -201,7 +200,7 @@ public class ChannelManager { * @return the default channel ID */ public String getDefaultChannelId() { - return DEFAULT_CHANNEL_ID; + return com.timesafari.dailynotification.DailyNotificationConstants.DEFAULT_CHANNEL_ID; } /** @@ -210,7 +209,7 @@ public class ChannelManager { public void logChannelStatus() { try { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { - NotificationChannel channel = notificationManager.getNotificationChannel(DEFAULT_CHANNEL_ID); + NotificationChannel channel = notificationManager.getNotificationChannel(com.timesafari.dailynotification.DailyNotificationConstants.DEFAULT_CHANNEL_ID); if (channel != null) { Log.i(TAG, "Channel Status - ID: " + channel.getId() + ", Importance: " + channel.getImportance() + diff --git a/android/src/main/java/com/timesafari/dailynotification/DailyNotificationConstants.kt b/android/src/main/java/com/timesafari/dailynotification/DailyNotificationConstants.kt new file mode 100644 index 0000000..fcfba13 --- /dev/null +++ b/android/src/main/java/com/timesafari/dailynotification/DailyNotificationConstants.kt @@ -0,0 +1,153 @@ +/** + * DailyNotificationConstants.kt + * + * Centralized constants for Daily Notification Plugin + * Eliminates magic numbers and string duplication across the codebase + * + * @author Matthew Raymer + * @version 1.0.0 + */ + +package com.timesafari.dailynotification + +/** + * Centralized constants for Daily Notification Plugin + * + * All request codes, channel IDs, action strings, and extra keys + * should be defined here and imported where needed. + */ +object DailyNotificationConstants { + + // ============================================================ + // Permission Request Codes + // ============================================================ + + /** + * Request code for notification permission requests + * Used by ActivityCompat.requestPermissions() + */ + const val PERMISSION_REQUEST_CODE = 1001 + + // ============================================================ + // Notification Channel Constants + // ============================================================ + + /** + * Default notification channel ID + * Must match across ChannelManager and NotifyReceiver + */ + const val DEFAULT_CHANNEL_ID = "timesafari.daily" + + /** + * Default notification channel name (user-visible) + */ + const val DEFAULT_CHANNEL_NAME = "Daily Notifications" + + /** + * Default notification channel description + */ + const val DEFAULT_CHANNEL_DESCRIPTION = "Daily notifications from TimeSafari" + + // ============================================================ + // Intent Actions + // ============================================================ + + /** + * Action string for notification broadcast intents + * Used by AlarmManager PendingIntents + */ + const val ACTION_NOTIFICATION = "com.timesafari.daily.NOTIFICATION" + + // ============================================================ + // Intent Extras Keys + // ============================================================ + + /** + * Extra key for notification ID in broadcast intents + */ + const val EXTRA_NOTIFICATION_ID = "notification_id" + + /** + * Extra key for schedule ID in broadcast intents + */ + const val EXTRA_SCHEDULE_ID = "schedule_id" + + // ============================================================ + // Notification IDs + // ============================================================ + + /** + * Default notification ID for daily notifications + * Used by NotificationManager.notify() + */ + const val DEFAULT_NOTIFICATION_ID = 1001 + + // ============================================================ + // SharedPreferences Keys + // ============================================================ + + /** + * SharedPreferences file name for plugin storage + */ + const val PREFS_NAME = "daily_notification_timesafari" + + /** + * SharedPreferences key for starred plan IDs + */ + const val PREFS_KEY_STARRED_PLAN_IDS = "starredPlanIds" + + // ============================================================ + // WorkManager Tags + // ============================================================ + + /** + * WorkManager tag for prefetch jobs + */ + const val WORK_TAG_PREFETCH = "prefetch" + + /** + * WorkManager tag for fetch jobs + */ + const val WORK_TAG_FETCH = "daily_notification_fetch" + + /** + * WorkManager tag for maintenance jobs + */ + const val WORK_TAG_MAINTENANCE = "daily_notification_maintenance" + + /** + * WorkManager tag for soft refetch jobs + */ + const val WORK_TAG_SOFT_REFETCH = "soft_refetch" + + /** + * WorkManager tag for display jobs + */ + const val WORK_TAG_DISPLAY = "daily_notification_display" + + /** + * WorkManager tag for dismiss jobs + */ + const val WORK_TAG_DISMISS = "daily_notification_dismiss" + + // ============================================================ + // Schedule IDs + // ============================================================ + + /** + * Default schedule ID for daily notifications + * Used when user doesn't provide a custom ID + */ + const val DEFAULT_SCHEDULE_ID = "daily_notification" + + // ============================================================ + // Request Code Versioning + // ============================================================ + + /** + * Version for request code derivation algorithm + * Increment if request code generation logic changes + */ + const val REQUEST_CODE_VERSION = 1 +} + diff --git a/android/src/main/java/com/timesafari/dailynotification/DailyNotificationPlugin.kt b/android/src/main/java/com/timesafari/dailynotification/DailyNotificationPlugin.kt index 07f8121..f1b7cab 100644 --- a/android/src/main/java/com/timesafari/dailynotification/DailyNotificationPlugin.kt +++ b/android/src/main/java/com/timesafari/dailynotification/DailyNotificationPlugin.kt @@ -86,8 +86,6 @@ open class DailyNotificationPlugin : Plugin() { private var db: DailyNotificationDatabase? = null - private val PERMISSION_REQUEST_CODE = 1001 - // Service instances for delegation private var statusChecker: NotificationStatusChecker? = null private var permissionManager: PermissionManager? = null @@ -345,7 +343,7 @@ open class DailyNotificationPlugin : Plugin() { Log.i(TAG, "Updating starred plans: count=${planIds.size}") // Store in SharedPreferences (matching TestNativeFetcher expectations) - val prefsName = "daily_notification_timesafari" + val prefsName = DailyNotificationConstants.PREFS_NAME val keyStarredPlanIds = "starredPlanIds" val prefs: SharedPreferences = context.getSharedPreferences(prefsName, Context.MODE_PRIVATE) @@ -422,7 +420,7 @@ open class DailyNotificationPlugin : Plugin() { ) { Log.i(TAG, "handleRequestPermissionsResult called: requestCode=$requestCode, permissions=${permissions.contentToString()}") - if (requestCode == PERMISSION_REQUEST_CODE) { + if (requestCode == DailyNotificationConstants.PERMISSION_REQUEST_CODE) { // Retrieve the saved call val call = savedCall if (call != null) { diff --git a/android/src/main/java/com/timesafari/dailynotification/DailyNotificationScheduler.java b/android/src/main/java/com/timesafari/dailynotification/DailyNotificationScheduler.java index fc484b2..476fa83 100644 --- a/android/src/main/java/com/timesafari/dailynotification/DailyNotificationScheduler.java +++ b/android/src/main/java/com/timesafari/dailynotification/DailyNotificationScheduler.java @@ -29,8 +29,7 @@ import java.util.concurrent.ConcurrentHashMap; public class DailyNotificationScheduler { private static final String TAG = "DailyNotificationScheduler"; - private static final String ACTION_NOTIFICATION = "com.timesafari.daily.NOTIFICATION"; - private static final String EXTRA_NOTIFICATION_ID = "notification_id"; + // Intent action and extras moved to DailyNotificationConstants private final Context context; private final AlarmManager alarmManager; @@ -157,8 +156,8 @@ public class DailyNotificationScheduler { // Create intent for the notification Intent intent = new Intent(context, DailyNotificationReceiver.class); - intent.setAction(ACTION_NOTIFICATION); - intent.putExtra(EXTRA_NOTIFICATION_ID, content.getId()); + intent.setAction(com.timesafari.dailynotification.DailyNotificationConstants.ACTION_NOTIFICATION); + intent.putExtra(com.timesafari.dailynotification.DailyNotificationConstants.EXTRA_NOTIFICATION_ID, content.getId()); // Check if this is a static reminder if (content.getId().startsWith("reminder_") || content.getId().contains("_reminder")) { diff --git a/android/src/main/java/com/timesafari/dailynotification/PermissionManager.java b/android/src/main/java/com/timesafari/dailynotification/PermissionManager.java index c9732ef..b0f3128 100644 --- a/android/src/main/java/com/timesafari/dailynotification/PermissionManager.java +++ b/android/src/main/java/com/timesafari/dailynotification/PermissionManager.java @@ -86,7 +86,7 @@ public class PermissionManager { androidx.core.app.ActivityCompat.requestPermissions( activity, new String[]{android.Manifest.permission.POST_NOTIFICATIONS}, - 1001 // PERMISSION_REQUEST_CODE - matches DailyNotificationPlugin.PERMISSION_REQUEST_CODE + com.timesafari.dailynotification.DailyNotificationConstants.PERMISSION_REQUEST_CODE // Centralized constant ); Log.d(TAG, "Permission dialog shown, waiting for user response");