-
Notifications
You must be signed in to change notification settings - Fork 0
[Refactor/#309] 타임존을 고려한 API 수정사항을 반영합니다. #311
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 1 commit
fa26d56
348cfc0
c0df468
8f0e438
49d405f
45cb135
90ba7af
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 |
|---|---|---|
|
|
@@ -22,6 +22,7 @@ import kotlinx.coroutines.flow.MutableStateFlow | |
| import kotlinx.coroutines.flow.StateFlow | ||
| import kotlinx.coroutines.launch | ||
| import java.time.LocalDate | ||
| import java.time.ZoneId | ||
| import javax.inject.Inject | ||
|
|
||
| @HiltViewModel | ||
|
|
@@ -232,9 +233,14 @@ class HomeViewModel @Inject constructor( | |
| } | ||
|
|
||
| fun canWriteDiary(): Boolean { | ||
| val userTimeZone = ZoneId.systemDefault().id | ||
| val today = LocalDate.now() | ||
| val selected = _selectedDate.value | ||
| val isAvailableDay = selected == today || selected == today.minusDays(1) | ||
| val isAvailableDay = if (userTimeZone == "Asia/Seoul") { | ||
| selected == today || selected == today.minusDays(1) | ||
| } else { | ||
| selected == today | ||
| } | ||
|
Comment on lines
+239
to
+243
Contributor
Author
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. 깜빡하고 이 부분도 해당 커밋에 합쳐버렸는데, 전날 일기 작성 불가능하게 분기처리 해놓은 부분입니다! |
||
| return _diaryCount.value == 0 && isAvailableDay | ||
| } | ||
|
|
||
|
|
@@ -276,7 +282,7 @@ class HomeViewModel @Inject constructor( | |
| isDiaryAlarm = info.isDiaryAlarm, | ||
| isDraftAlarm = true, | ||
| isReplyAlarm = info.isReplyAlarm, | ||
| time = info.time.ifEmpty { "21:30" }, | ||
| time = info.time, | ||
| fcmToken = fcmToken, | ||
| ) | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,6 +7,8 @@ import com.sopt.clody.data.remote.dto.request.SendNotificationRequestDto | |
| import com.sopt.clody.data.remote.util.NetworkUtil | ||
| import com.sopt.clody.domain.Notification | ||
| import com.sopt.clody.domain.repository.NotificationRepository | ||
| import com.sopt.clody.presentation.utils.extension.TimePeriod | ||
| import com.sopt.clody.presentation.utils.extension.convertUTZtoKST | ||
| import com.sopt.clody.presentation.utils.network.ErrorMessages.FAILURE_NETWORK_MESSAGE | ||
| import com.sopt.clody.presentation.utils.network.ErrorMessages.FAILURE_TEMPORARY_MESSAGE | ||
| import com.sopt.clody.presentation.utils.network.ErrorMessages.UNKNOWN_ERROR | ||
|
|
@@ -140,7 +142,7 @@ class NotificationSettingViewModel @Inject constructor( | |
| } | ||
| } | ||
|
|
||
| fun changeNotificationTime(context: Context, time: String) { | ||
| fun changeNotificationTime(context: Context, timePeriod: TimePeriod, hour: String, minute: String) { | ||
| _notificationTimeChangeState.value = NotificationTimeChangeState.Loading | ||
| viewModelScope.launch { | ||
| if (!networkUtil.isNetworkAvailable()) { | ||
|
|
@@ -157,7 +159,7 @@ class NotificationSettingViewModel @Inject constructor( | |
| isDiaryAlarm = _diaryAlarm.value, | ||
| isDraftAlarm = _draftAlarm.value, | ||
| isReplyAlarm = _replyAlarm.value, | ||
| time = time, | ||
| time = convertUTZtoKST(timePeriod, hour, minute), | ||
|
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. 💡 Verification agent 🧩 Analysis chainVerify timezone conversion function robustness The conversion from user's timezone to KST using
🏁 Script executed: #!/bin/bash
# Find and examine the convertUTZtoKST function implementation
ast-grep --pattern 'fun convertUTZtoKST($_, $_, $_) { $$$ }'
# Also check for any error handling in the conversion functions
fd . -name "TimeZoneExt.kt" | head -1 | xargs catLength of output: 343 🏁 Script executed: #!/bin/bash
# 1. Locate the implementation of convertUTZtoKST
rg --context 5 'fun convertUTZtoKST' -n
# 2. Find and print the contents of TimeZoneExt.kt (first 200 lines) to inspect any helper functions or error handling
find . -type f -name 'TimeZoneExt.kt' -print -exec sed -n '1,200p' {} \;Length of output: 3182 Fix The current implementation mistakenly concatenates the hour string with • File: fun convertUTZtoKST(timePeriod: TimePeriod, hour: String, minute: String): String {
val userZoneId = ZoneId.systemDefault()
val kstZoneId = ZoneId.of("Asia/Seoul")
- val hour24 = when (timePeriod) {
- TimePeriod.AM -> if (hour == "12") 0 else hour.toInt()
- TimePeriod.PM -> if (hour == "12") 12 else (hour + 12).toInt()
- }
+ val hour24 = when (timePeriod) {
+ TimePeriod.AM -> if (hour == "12") 0 else hour.toInt()
+ TimePeriod.PM -> if (hour == "12") 12 else hour.toInt() + 12
+ }
val userTime = LocalTime.of(hour24, minute.toInt())
val userZoned = ZonedDateTime.of(LocalDate.now(userZoneId), userTime, userZoneId)
val kstZoned = userZoned.withZoneSameInstant(kstZoneId)
val kstHour = kstZoned.hour
val kstMinute = kstZoned.minute
- return String.format("%02s:%02s", kstHour, kstMinute)
+ return String.format("%02d:%02d", kstHour, kstMinute)
}Additional recommendations:
🤖 Prompt for AI Agents |
||
| fcmToken = fcmToken, | ||
| ) | ||
| notificationRepository.sendNotification(requestDto).fold( | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,56 @@ | ||||||||||||
| package com.sopt.clody.presentation.utils.extension | ||||||||||||
|
|
||||||||||||
| import java.time.LocalDate | ||||||||||||
| import java.time.LocalTime | ||||||||||||
| import java.time.ZoneId | ||||||||||||
| import java.time.ZonedDateTime | ||||||||||||
| import java.time.format.DateTimeFormatter | ||||||||||||
|
|
||||||||||||
| /** | ||||||||||||
| * @param time 서버로부터 수신받은 시간으로 "21:30" 와 같은 형태로 전달받는다. | ||||||||||||
| * */ | ||||||||||||
| fun convertKSTtoUTZ(time: String): Triple<TimePeriod, Int, Int> { | ||||||||||||
| val kstZoneId = ZoneId.of("Asia/Seoul") | ||||||||||||
| val userZoneId = ZoneId.systemDefault() | ||||||||||||
|
|
||||||||||||
| val formatter = DateTimeFormatter.ofPattern("HH:mm") | ||||||||||||
| val localTime = LocalTime.parse(time, formatter) | ||||||||||||
|
|
||||||||||||
| val kstDateTime = ZonedDateTime.of(LocalDate.now(kstZoneId), localTime, kstZoneId) | ||||||||||||
| val userDateTime = kstDateTime.withZoneSameInstant(userZoneId) | ||||||||||||
|
|
||||||||||||
| val hour24 = userDateTime.hour | ||||||||||||
| val minute = userDateTime.minute | ||||||||||||
| val timePeriod = if (hour24 < 12) TimePeriod.AM else TimePeriod.PM | ||||||||||||
| val hour12 = when { | ||||||||||||
| hour24 == 0 -> 12 | ||||||||||||
| hour24 > 12 -> hour24 - 12 | ||||||||||||
| else -> hour24 | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| return Triple(timePeriod, hour12, minute) | ||||||||||||
| } | ||||||||||||
coderabbitai[bot] marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||
|
|
||||||||||||
| /** | ||||||||||||
| * @param timePeriod 오전/오후 | ||||||||||||
| * @param hour 시간 | ||||||||||||
| * @param minute 분 | ||||||||||||
| * */ | ||||||||||||
| fun convertUTZtoKST(timePeriod: TimePeriod, hour: String, minute: String): String { | ||||||||||||
| val userZoneId = ZoneId.systemDefault() | ||||||||||||
| val kstZoneId = ZoneId.of("Asia/Seoul") | ||||||||||||
|
|
||||||||||||
| val hour24 = when (timePeriod) { | ||||||||||||
| TimePeriod.AM -> if (hour == "12") 0 else hour.toInt() | ||||||||||||
| TimePeriod.PM -> if (hour == "12") 12 else (hour + 12).toInt() | ||||||||||||
| } | ||||||||||||
SYAAINN marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||
|
|
||||||||||||
| val userTime = LocalTime.of(hour24, minute.toInt()) | ||||||||||||
|
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 Add input validation for minute parameter. The minute parameter is converted to integer without validation, which could cause a Apply this diff to add minute validation: - val userTime = LocalTime.of(hour24, minute.toInt())
+ val minuteInt = minute.toIntOrNull() ?: throw IllegalArgumentException("Invalid minute format: $minute")
+ require(minuteInt in 0..59) { "Invalid minute: $minute. Must be 0-59." }
+ val userTime = LocalTime.of(hour24, minuteInt)📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||
| val userZoned = ZonedDateTime.of(LocalDate.now(userZoneId), userTime, userZoneId) | ||||||||||||
| val kstZoned = userZoned.withZoneSameInstant(kstZoneId) | ||||||||||||
|
|
||||||||||||
| val kstHour = kstZoned.hour | ||||||||||||
| val kstMinute = kstZoned.minute | ||||||||||||
|
|
||||||||||||
| return String.format("%02s:%02s", kstHour, kstMinute) | ||||||||||||
| } | ||||||||||||
coderabbitai[bot] marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||
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.
💡 Verification agent
🧩 Analysis chain
Verify the function name accuracy.
The import
convertUTZtoKSTsuggests converting from "UTZ" (possibly User Time Zone) to KST, but this naming could be confusing. Ensure the function name accurately reflects the conversion direction and consider using more descriptive names likeconvertUserTimeToKST.🏁 Script executed:
Length of output: 57
🏁 Script executed:
Length of output: 5295
Rename convertUTZtoKST for clarity
The
convertUTZtoKSTutility indeed reads from the system default (user) timezone and converts that time to KST. However, “UTZ” is non-standard and can be easily confused with “UTC.” Consider renaming this function to more clearly express its behavior, for example:convertLocalTimeToKSTconvertUserTimeToKSTLocations to update:
app/src/main/java/com/sopt/clody/presentation/utils/extension/TimeZoneExt.kt(line 39)NotificationSettingViewModel.ktandTimeReminderViewModel.kt🤖 Prompt for AI Agents