-
Notifications
You must be signed in to change notification settings - Fork 0
[REFACTOR/#317] 알림 권한 로직 개선 #318
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
710a198
7a956e3
dbfb291
fdd6da5
2dd1d8f
89f5624
f6584eb
53ceff5
b6c5e39
138c5ba
2a0c8e8
d9026f8
3b59e19
7194bfd
8581b98
c70281e
c4d510e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,9 +4,8 @@ import com.sopt.clody.data.remote.datasource.RemoteConfigDataSource | |
| import com.sopt.clody.domain.appupdate.AppUpdateChecker | ||
| import com.sopt.clody.domain.model.AppUpdateState | ||
| import com.sopt.clody.domain.util.VersionComparator | ||
| import java.time.LocalDateTime | ||
| import java.time.format.TextStyle | ||
| import java.util.Locale | ||
| import java.time.ZoneId | ||
| import java.time.ZonedDateTime | ||
| import javax.inject.Inject | ||
|
|
||
| class AppUpdateCheckerImpl @Inject constructor( | ||
|
|
@@ -34,29 +33,35 @@ class AppUpdateCheckerImpl @Inject constructor( | |
| } | ||
| } | ||
|
|
||
| /** | ||
| * Firebase RemoteConfig 로부터 점검 시간에 해당 하는지 검사하는 함수. | ||
| * | ||
| * @return 점검 시간 해당 여부 | ||
| * */ | ||
| override suspend fun isUnderInspection(): Boolean { | ||
| val start = remoteConfigDataSource.getInspectionStart() ?: return false | ||
| val end = remoteConfigDataSource.getInspectionEnd() ?: return false | ||
| val now = LocalDateTime.now() | ||
| return now.isAfter(start) && now.isBefore(end) | ||
| } | ||
| val serverZone = ZoneId.of(SERVER_TIMEZONE) | ||
|
|
||
| override fun getInspectionTimeText(): String? { | ||
| val start = remoteConfigDataSource.getInspectionStart() | ||
| val end = remoteConfigDataSource.getInspectionEnd() | ||
| if (start == null || end == null) return null | ||
| val nowServer = ZonedDateTime.now(serverZone) | ||
| val startZ = start.atZone(serverZone) | ||
| val endZ = end.atZone(serverZone) | ||
|
|
||
| val startText = formatDateTimeWithDayOfWeek(start) | ||
| val endText = formatDateTimeWithDayOfWeek(end) | ||
| return "$startText ~ $endText" | ||
| return nowServer in startZ..endZ | ||
| } | ||
|
|
||
| private fun formatDateTimeWithDayOfWeek(dateTime: LocalDateTime): String { | ||
| val dayOfWeek = dateTime.dayOfWeek.getDisplayName(TextStyle.SHORT, Locale.KOREAN) | ||
| val month = dateTime.monthValue | ||
| val day = dateTime.dayOfMonth | ||
| val hour = dateTime.hour.toString().padStart(2, '0') | ||
| /** | ||
| * Firebase RemoteConfig 로부터 점검 시간을 가져와 반환하는 함수. | ||
| * | ||
| * @return 점검 시작 시간과 종료 시간을 "2025-08-11T18:00:00" 형식으로 반환 | ||
| * */ | ||
| override suspend fun getInspectionTimeText(): Pair<String, String>? { | ||
| val start = remoteConfigDataSource.getInspectionStart() ?: return null | ||
| val end = remoteConfigDataSource.getInspectionEnd() ?: return null | ||
| return start.toString() to end.toString() | ||
| } | ||
|
Comment on lines
+53
to
+62
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Doc/format mismatch: toString() omits seconds; can break downstream parsing KDoc promises "2025-08-11T18:00:00", but Apply explicit formatting and fetch freshness: -override suspend fun getInspectionTimeText(): Pair<String, String>? {
- val start = remoteConfigDataSource.getInspectionStart() ?: return null
- val end = remoteConfigDataSource.getInspectionEnd() ?: return null
- return start.toString() to end.toString()
-}
+override suspend fun getInspectionTimeText(): Pair<String, String>? {
+ // Ensure fresh values
+ remoteConfigDataSource.fetch()
+ val start = remoteConfigDataSource.getInspectionStart() ?: return null
+ val end = remoteConfigDataSource.getInspectionEnd() ?: return null
+ return start.format(ISO_LOCAL_SECOND) to end.format(ISO_LOCAL_SECOND)
+}Add the formatter and import (outside this hunk): import java.time.format.DateTimeFormatter companion object {
private const val SERVER_TIMEZONE = "Asia/Seoul"
+ private val ISO_LOCAL_SECOND: DateTimeFormatter =
+ DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss")
}If you prefer stronger typing, return 🤖 Prompt for AI Agents |
||
|
|
||
| return "$month/$day($dayOfWeek) ${hour}시" | ||
| companion object { | ||
| private const val SERVER_TIMEZONE = "Asia/Seoul" | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -33,7 +33,7 @@ class TimeReminderViewModel @Inject constructor( | |||||||||||||||||||||||||||||||||||
| var selectedTime by mutableStateOf("21:30") | ||||||||||||||||||||||||||||||||||||
| private set | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| fun sendNotification(context: Context, isPermissionGranted: Boolean) { | ||||||||||||||||||||||||||||||||||||
| fun sendNotification(context: Context) { | ||||||||||||||||||||||||||||||||||||
| viewModelScope.launch { | ||||||||||||||||||||||||||||||||||||
| if (networkConnectivityObserver.networkStatus.first() == NetworkStatus.Unavailable) { | ||||||||||||||||||||||||||||||||||||
| _timeReminderState.value = TimeReminderState.Failure(errorMessageProvider.getNetworkError()) | ||||||||||||||||||||||||||||||||||||
|
|
@@ -47,9 +47,9 @@ class TimeReminderViewModel @Inject constructor( | |||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| val requestDto = SendNotificationRequestDto( | ||||||||||||||||||||||||||||||||||||
| isDiaryAlarm = isPermissionGranted, | ||||||||||||||||||||||||||||||||||||
| isDiaryAlarm = true, | ||||||||||||||||||||||||||||||||||||
| isDraftAlarm = false, | ||||||||||||||||||||||||||||||||||||
| isReplyAlarm = isPermissionGranted, | ||||||||||||||||||||||||||||||||||||
| isReplyAlarm = true, | ||||||||||||||||||||||||||||||||||||
| time = selectedTime, | ||||||||||||||||||||||||||||||||||||
| fcmToken = fcmToken, | ||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
49
to
55
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Hardcoded notification alarm settings may not reflect user preferences. Setting Consider retrieving the current notification preferences before building the request: - val requestDto = SendNotificationRequestDto(
- isDiaryAlarm = true,
- isDraftAlarm = false,
- isReplyAlarm = true,
- time = selectedTime,
- fcmToken = fcmToken,
- )
+ val notificationInfo = notificationRepository.getNotificationInfo().getOrNull()
+ val requestDto = SendNotificationRequestDto(
+ isDiaryAlarm = notificationInfo?.isDiaryAlarm ?: true,
+ isDraftAlarm = false,
+ isReplyAlarm = notificationInfo?.isReplyAlarm ?: true,
+ time = selectedTime,
+ fcmToken = fcmToken,
+ )📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remote Config is never fetched here; results may be stale on cold start
Unlike
getAppUpdateState(),isUnderInspection()doesn’t callremoteConfigDataSource.fetch(). On app launch, this can yield stale/default values and skip a valid inspection window.Apply this minimal fix:
override suspend fun isUnderInspection(): Boolean { + // Ensure Remote Config is up-to-date before reading values. + remoteConfigDataSource.fetch() val start = remoteConfigDataSource.getInspectionStart() ?: return false val end = remoteConfigDataSource.getInspectionEnd() ?: return false val serverZone = ZoneId.of(SERVER_TIMEZONE) val nowServer = ZonedDateTime.now(serverZone) val startZ = start.atZone(serverZone) val endZ = end.atZone(serverZone) return nowServer in startZ..endZ }If
fetch()is expensive, consider internal caching or a shared preload in the app’s startup pipeline.📝 Committable suggestion
🤖 Prompt for AI Agents