Wi-Fi接続時のみ転送設定の追加 #305
Wi-Fi接続時のみ転送設定の追加 #305
Conversation
WalkthroughWi‑Fi接続時のみ通知を送信する設定を追加しました。設定画面のトグル、保存用ユースケースとデータストア保存、通知ユースケース内でのWi‑Fi接続チェック（ConnectivityManager）を組み込み、マニフェストにネットワーク状態アクセス権限を追加しました。 Changes
Sequence Diagram(s)sequenceDiagram
participant User as ユーザー
participant UI as SettingsScreen
participant VM as SettingsViewModel
participant UC as SaveWifiOnlyNotificationSettingUseCase
participant Repo as UserSettingsRepository
participant DS as UserSettingsDataStore
User->>UI: Wi‑Fiのみ設定を切替
UI->>VM: updateWifiOnlySetting(flag)
VM->>UC: invoke(flag)
UC->>Repo: getCurrentSettings()
Repo->>DS: get()
DS-->>Repo: UserSettings
Repo-->>UC: UserSettings
UC->>Repo: save(updatedSettings)
Repo->>DS: save(isWifiOnlyNotificationEnabled)
DS-->>Repo: success
Repo-->>UC: Result<Unit>
UC-->>VM: Result<Unit>
VM->>UI: UiState 更新（isWifiOnlyNotificationEnabled）
sequenceDiagram
participant App as 呼び出し元
participant NUC as NotifyTargetAppNotificationUseCaseImpl
participant Repo as UserSettingsRepository
participant CM as ConnectivityManager
participant NotifyUC as NotifyUseCase
App->>NUC: invoke(target)
NUC->>Repo: getCurrentSettings()
Repo-->>NUC: UserSettings(isWifiOnlyNotificationEnabled)
alt Wi‑Fiのみ有効かつ未接続
NUC->>CM: ネットワーク状態を確認
CM-->>NUC: not Wi‑Fi
NUC-->>App: Result.success(Unit)（送信スキップ）
else Wi‑Fi接続中 または Wi‑Fiのみ無効
NUC->>NotifyUC: invoke(target)
NotifyUC-->>NUC: Result<Unit>
NUC-->>App: Result<Unit>
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
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. 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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/screen/settings/SettingsScreen.kt (1)
10-27: Wi-Fi接続時のみ転送する。をローカライズして、Row全体をタップ可能にラベルがハードコードされているためローカライズできません。同時に、Row全体を
toggleableにしてラベルタップでも切り替えられるようにすると、操作性とアクセシビリティが向上します。文字列リソースの追加と、以下の修正を実施してください。修正例
+import androidx.compose.foundation.selection.toggleable import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.Spacer @@ import androidx.compose.material3.Checkbox import androidx.compose.material3.MaterialTheme @@ import androidx.compose.ui.platform.LocalSoftwareKeyboardController import androidx.compose.ui.res.stringResource +import androidx.compose.ui.semantics.Role @@ - Row( - verticalAlignment = Alignment.CenterVertically - ) { - Checkbox( - checked = isWifiOnlyNotificationEnabled, - onCheckedChange = onWifiOnlySettingChanged - ) - Text("Wi-Fi接続時のみ転送する。") - } + Row( + modifier = Modifier + .fillMaxWidth() + .toggleable( + value = isWifiOnlyNotificationEnabled, + onValueChange = onWifiOnlySettingChanged, + role = Role.Checkbox + ), + verticalAlignment = Alignment.CenterVertically + ) { + Checkbox( + checked = isWifiOnlyNotificationEnabled, + onCheckedChange = null + ) + Text(stringResource(id = R.string.wifi_only_notification)) + }
🤖 Fix all issues with AI agents
In
`@AndroidApp/domain/src/test/java/me/nya_n/notificationnotifier/UseCaseTest.kt`:
- Around line 76-81: Test uses the old 3-parameter UserSettings constructor;
update the UserSettings constructor call in UserSettingsRepositoryTest to pass
the new fourth boolean parameter (isWifiOnlyNotificationEnabled) so the
instantiation matches the new 4-parameter signature of the UserSettings class
(update the val data = UserSettings(...) call to include the additional boolean,
e.g., set it to false).
In
`@AndroidApp/model/src/main/kotlin/me/nya_n/notificationnotifier/model/UserSettings.kt`:
- Around line 13-14: The non-nullable field isWifiOnlyNotificationEnabled in
UserSettings lacks a default, causing Gson to throw on deserializing older
backups; update the property declaration (isWifiOnlyNotificationEnabled) to
provide a sensible default value (e.g., false) so missing JSON keys do not cause
JsonSyntaxException during restore and ensure the class deserializes
successfully when the field is absent.
In
`@AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/screen/settings/SettingsViewModel.kt`:
- Around line 64-67: The updateWifiOnlySetting function currently ignores the
Result from saveWifiOnlyNotificationSettingUseCase and updates _uiState
unconditionally; change it to call saveWifiOnlyNotificationSettingUseCase and
check the returned Result (e.g., result.isSuccess) and only update
_uiState.copy(isWifiOnlyNotificationEnabled = ...) on success, and handle
failure by setting an error/failed state or leaving the UI unchanged (mirror the
pattern used in notifyTest(), exportData(), importData()); reference the
saveWifiOnlyNotificationSettingUseCase invocation and the _uiState.update call
when making the change.
🧹 Nitpick comments (1)
AndroidApp/domain/src/main/kotlin/me/nya_n/notificationnotifier/domain/usecase/impl/SaveWifiOnlyNotificationSettingUseCaseImpl.kt (1)
9-18:runCatching内の非局所returnを避けて簡潔化してください現在は
runCatchingブロック内でreturn Result.success(Unit)しており、成功時にrunCatchingの戻り値を使わない形になっています。意図は同じでも読みづらいので、ブロックはUnitを返す形にすると明確です。♻️ 修正案
- override fun invoke(isWifiOnlyNotificationEnabled: Boolean): Result<Unit> { - return runCatching { - val settings = userSettingsRepository.getUserSettings() - userSettingsRepository.saveUserSettings( - settings.copy( - isWifiOnlyNotificationEnabled = isWifiOnlyNotificationEnabled - ) - ) - return Result.success(Unit) - } - } + override fun invoke(isWifiOnlyNotificationEnabled: Boolean): Result<Unit> = + runCatching { + val settings = userSettingsRepository.getUserSettings() + userSettingsRepository.saveUserSettings( + settings.copy( + isWifiOnlyNotificationEnabled = isWifiOnlyNotificationEnabled + ) + ) + }
| every { userSettingsRepository.getUserSettings() } returns UserSettings( | ||
| "192.168.10.18", | ||
| 8484, | ||
| true | ||
| true, | ||
| false | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find all UserSettings constructor usages to identify which need updating
rg -n "UserSettings\s*\(" --type kotlin -B 1 -A 5Repository: ptkNktq/AndroidNotificationNotifier
Length of output: 20933
UserSettingsRepositoryTest.kt の44行目を4パラメータコンストラクタに更新してください
UserSettingsRepositoryTest.kt の46行目でUserSettingsの3パラメータコンストラクタが使用されています。新しい4パラメータシグネチャに対応するため、isWifiOnlyNotificationEnabledパラメータを追加する必要があります。
該当コード
val data = UserSettings("192.168.10.18", 8484, false)
修正例：
val data = UserSettings("192.168.10.18", 8484, false, false)
🤖 Prompt for AI Agents
In `@AndroidApp/domain/src/test/java/me/nya_n/notificationnotifier/UseCaseTest.kt`
around lines 76 - 81, Test uses the old 3-parameter UserSettings constructor;
update the UserSettings constructor call in UserSettingsRepositoryTest to pass
the new fourth boolean parameter (isWifiOnlyNotificationEnabled) so the
instantiation matches the new 4-parameter signature of the UserSettings class
(update the val data = UserSettings(...) call to include the additional boolean,
e.g., set it to false).
| @SerializedName("is_wifi_only_notification_enabled") | ||
| val isWifiOnlyNotificationEnabled: Boolean |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find Backup.kt and examine its structure
find . -name "Backup.kt" -o -name "backup.kt" | head -20Repository: ptkNktq/AndroidNotificationNotifier
Length of output: 159
🏁 Script executed:
# List files in the model directory to understand structure
git ls-files | grep -E "(Backup|UserSettings|backup)" | head -30Repository: ptkNktq/AndroidNotificationNotifier
Length of output: 939
🏁 Script executed:
# Search for UserSettings usage in backup context
rg -n "UserSettings" --type kotlin | grep -E "(Backup|backup)" | head -30Repository: ptkNktq/AndroidNotificationNotifier
Length of output: 370
🏁 Script executed:
# Look for Gson configuration and deserialization logic
rg -n "Gson|fromJson|deserialize" --type kotlin | head -40Repository: ptkNktq/AndroidNotificationNotifier
Length of output: 1216
🏁 Script executed:
# Check for any default value assignments in UserSettings
rg -n "UserSettings" -A 30 --type kotlin | grep -E "(class UserSettings|=|default)" | head -40Repository: ptkNktq/AndroidNotificationNotifier
Length of output: 6710
🏁 Script executed:
# Read Backup.kt to see its structure
cat -n AndroidApp/model/src/main/kotlin/me/nya_n/notificationnotifier/model/Backup.ktRepository: ptkNktq/AndroidNotificationNotifier
Length of output: 445
🏁 Script executed:
# Read UserSettings.kt to see all fields and current defaults
cat -n AndroidApp/model/src/main/kotlin/me/nya_n/notificationnotifier/model/UserSettings.ktRepository: ptkNktq/AndroidNotificationNotifier
Length of output: 713
🏁 Script executed:
# Read ImportDataUseCaseImpl to see deserialization and error handling
cat -n AndroidApp/domain/src/main/kotlin/me/nya_n/notificationnotifier/domain/usecase/impl/ImportDataUseCaseImpl.ktRepository: ptkNktq/AndroidNotificationNotifier
Length of output: 1713
🏁 Script executed:
# Search for Gson configuration, custom TypeAdapter, or JsonDeserializer
rg -n "TypeAdapter|JsonDeserializer|GsonBuilder|setFieldNamingStrategy" --type kotlinRepository: ptkNktq/AndroidNotificationNotifier
Length of output: 61
🏁 Script executed:
# Check for Gson version in build.gradle files
find . -name "build.gradle*" -type f | xargs grep -l "gson"Repository: ptkNktq/AndroidNotificationNotifier
Length of output: 151
🏁 Script executed:
# Search for test cases related to backup import
rg -n "ImportDataUseCase|import.*json|backup" --type kotlin | grep -i "test"Repository: ptkNktq/AndroidNotificationNotifier
Length of output: 1075
🏁 Script executed:
# Check if there's a Gson configuration or dependency setup
git ls-files | grep -i "gson\|gradle" | head -20Repository: ptkNktq/AndroidNotificationNotifier
Length of output: 642
🏁 Script executed:
# Check Gson version and dependencies
cat -n AndroidApp/gradle/libs.versions.toml | grep -i gsonRepository: ptkNktq/AndroidNotificationNotifier
Length of output: 226
🏁 Script executed:
# Check domain build.gradle.kts for Gson config
cat -n AndroidApp/domain/build.gradle.kts | head -50Repository: ptkNktq/AndroidNotificationNotifier
Length of output: 1420
🏁 Script executed:
# Check UseCaseTest.kt to see if there are backward compatibility tests
cat -n AndroidApp/domain/src/androidTest/java/me/nya_n/notificationnotifier/UseCaseTest.kt | head -150Repository: ptkNktq/AndroidNotificationNotifier
Length of output: 7950
🏁 Script executed:
# Search for any test data or old backup JSON files
find . -name "*.json" -type f | head -20Repository: ptkNktq/AndroidNotificationNotifier
Length of output: 294
🏁 Script executed:
# Verify Gson behavior - search for any error handling or migration logic
rg -n "JsonSyntaxException|catch|try" AndroidApp/domain/src/main/kotlin/me/nya_n/notificationnotifier/domain/usecase/impl/ImportDataUseCaseImpl.ktRepository: ptkNktq/AndroidNotificationNotifier
Length of output: 61
🏁 Script executed:
# Check if there's any version migration or schema update logic
rg -n "version|migration|upgrade" --type kotlin | grep -i "backup\|import\|restore" | head -20Repository: ptkNktq/AndroidNotificationNotifier
Length of output: 510
🏁 Script executed:
# Verify field initialization behavior in UserSettings
cat -n AndroidApp/model/src/main/kotlin/me/nya_n/notificationnotifier/model/UserSettings.ktRepository: ptkNktq/AndroidNotificationNotifier
Length of output: 713
古いバックアップファイルの復元に失敗する可能性
新しく追加されたnon-nullableのisWifiOnlyNotificationEnabledフィールドにはデフォルト値が設定されていません。Gsonはデフォルト値のないnon-nullableフィールドが欠落している場合、デシリアライズ時にJsonSyntaxExceptionをスローします。そのため、このフィールドを含まない古いバックアップをリストアする際に失敗します。
推奨される修正
デフォルト値を追加してください：
`@SerializedName`("is_wifi_only_notification_enabled")
- val isWifiOnlyNotificationEnabled: Boolean
+ val isWifiOnlyNotificationEnabled: Boolean = false📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @SerializedName("is_wifi_only_notification_enabled") | |
| val isWifiOnlyNotificationEnabled: Boolean | |
| `@SerializedName`("is_wifi_only_notification_enabled") | |
| val isWifiOnlyNotificationEnabled: Boolean = false |
🤖 Prompt for AI Agents
In
`@AndroidApp/model/src/main/kotlin/me/nya_n/notificationnotifier/model/UserSettings.kt`
around lines 13 - 14, The non-nullable field isWifiOnlyNotificationEnabled in
UserSettings lacks a default, causing Gson to throw on deserializing older
backups; update the property declaration (isWifiOnlyNotificationEnabled) to
provide a sensible default value (e.g., false) so missing JSON keys do not cause
JsonSyntaxException during restore and ensure the class deserializes
successfully when the field is absent.
| fun updateWifiOnlySetting(isWifiOnlyNotificationEnabled: Boolean) { | ||
| saveWifiOnlyNotificationSettingUseCase(isWifiOnlyNotificationEnabled) | ||
| _uiState.update { it.copy(isWifiOnlyNotificationEnabled = isWifiOnlyNotificationEnabled) } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "SettingsViewModel.kt" | head -20Repository: ptkNktq/AndroidNotificationNotifier
Length of output: 180
🏁 Script executed:
cat -n ./AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/screen/settings/SettingsViewModel.ktRepository: ptkNktq/AndroidNotificationNotifier
Length of output: 4573
🏁 Script executed:
find . -type f -name "SaveWifiOnlyNotificationSettingUseCase.kt" | head -5Repository: ptkNktq/AndroidNotificationNotifier
Length of output: 201
🏁 Script executed:
cat -n ./AndroidApp/domain/src/main/kotlin/me/nya_n/notificationnotifier/domain/usecase/SaveWifiOnlyNotificationSettingUseCase.ktRepository: ptkNktq/AndroidNotificationNotifier
Length of output: 298
🏁 Script executed:
# Check if there are any other similar use cases that return Result
rg "SaveWifiOnlyNotificationSettingUseCase" -A 5 -B 5Repository: ptkNktq/AndroidNotificationNotifier
Length of output: 11200
保存失敗時のUI整合性を担保してください
Resultを無視してUI状態を更新しています。保存失敗時に表示と永続値がズレます。成功時のみ更新するか、失敗時の扱いを入れるべきです。同ファイル内のnotifyTest()、exportData()、importData()メソッドなど、他の同様のメソッドと同じく.isSuccessをチェックして条件付きで状態更新することを推奨します。
♻️ 修正案
fun updateWifiOnlySetting(isWifiOnlyNotificationEnabled: Boolean) {
- saveWifiOnlyNotificationSettingUseCase(isWifiOnlyNotificationEnabled)
- _uiState.update { it.copy(isWifiOnlyNotificationEnabled = isWifiOnlyNotificationEnabled) }
+ val result = saveWifiOnlyNotificationSettingUseCase(isWifiOnlyNotificationEnabled)
+ if (result.isSuccess) {
+ _uiState.update { it.copy(isWifiOnlyNotificationEnabled = isWifiOnlyNotificationEnabled) }
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fun updateWifiOnlySetting(isWifiOnlyNotificationEnabled: Boolean) { | |
| saveWifiOnlyNotificationSettingUseCase(isWifiOnlyNotificationEnabled) | |
| _uiState.update { it.copy(isWifiOnlyNotificationEnabled = isWifiOnlyNotificationEnabled) } | |
| } | |
| fun updateWifiOnlySetting(isWifiOnlyNotificationEnabled: Boolean) { | |
| val result = saveWifiOnlyNotificationSettingUseCase(isWifiOnlyNotificationEnabled) | |
| if (result.isSuccess) { | |
| _uiState.update { it.copy(isWifiOnlyNotificationEnabled = isWifiOnlyNotificationEnabled) } | |
| } | |
| } |
🤖 Prompt for AI Agents
In
`@AndroidApp/ui/src/main/kotlin/me/nya_n/notificationnotifier/ui/screen/settings/SettingsViewModel.kt`
around lines 64 - 67, The updateWifiOnlySetting function currently ignores the
Result from saveWifiOnlyNotificationSettingUseCase and updates _uiState
unconditionally; change it to call saveWifiOnlyNotificationSettingUseCase and
check the returned Result (e.g., result.isSuccess) and only update
_uiState.copy(isWifiOnlyNotificationEnabled = ...) on success, and handle
failure by setting an error/failed state or leaving the UI unchanged (mirror the
pattern used in notifyTest(), exportData(), importData()); reference the
saveWifiOnlyNotificationSettingUseCase invocation and the _uiState.update call
when making the change.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.github/workflows/android-build.yml:
- Line 35: The Discord user mention "<@225341405726244865>" is hardcoded;
replace it with a secret or external input: register the ID as a GitHub secret
(e.g. DISCORD_NOTIFY_USER_ID) and update the workflow content to use the secret
reference inside the mention (e.g. "<@${{ secrets.DISCORD_NOTIFY_USER_ID }}>"),
or alternatively expose it as a workflow input or environment variable and
reference that (e.g. "<@${{ inputs.DISCORD_NOTIFY_USER_ID }}>" or "<@${{
env.DISCORD_NOTIFY_USER_ID }}>"); ensure the string quoting is preserved so the
YAML remains valid and tests the notification path after change.
In `@README.md`:
- Around line 52-53: The Japanese phrase "ための必要です。" in the README should be
corrected to the natural "ために必要です。"; locate the sentence containing
"通知対象のアプリを選択するための必要です。" and replace that fragment with "通知対象のアプリを選択するために必要です。"
so the sentence reads naturally.
Summary by CodeRabbit
新機能
権限
ドキュメント
✏️ Tip: You can customize this high-level summary in your review settings.