Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
package `in`.koreatech.koin.feature.callvan.ui.create

import `in`.koreatech.koin.feature.callvan.model.CallvanLocationOption
import java.util.Calendar

data class CallvanCreateState(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

State 클래스에 @Immutable 애노테이션이 누락되었습니다.

코딩 가이드라인에 따르면 feature 모듈의 State 클래스는 Compose 최적화를 위해 @Immutable 애노테이션 사용이 필수입니다.

🛠️ 수정 제안
 package `in`.koreatech.koin.feature.callvan.ui.create
 
 import `in`.koreatech.koin.feature.callvan.model.CallvanLocationOption
+import androidx.compose.runtime.Immutable
 import java.util.Calendar
 
+@Immutable
 data class CallvanCreateState(

As per coding guidelines: "feature/**" - "Compose 최적화를 위해 State 클래스에 @Immutable 및 ImmutableList 사용 필수"

📝 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.

Suggested change
data class CallvanCreateState(
package `in`.koreatech.koin.feature.callvan.ui.create
import `in`.koreatech.koin.feature.callvan.model.CallvanLocationOption
import androidx.compose.runtime.Immutable
import java.util.Calendar
`@Immutable`
data class CallvanCreateState(
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/create/CallvanCreateState.kt`
at line 6, The CallvanCreateState data class is missing the `@Immutable`
annotation required by the feature module coding guideline; add the
androidx.compose.runtime.Immutable annotation above the CallvanCreateState
declaration and ensure any list properties inside CallvanCreateState use an
ImmutableList (or other immutable collection) instead of mutable lists so
Compose can optimize correctly.

val departureLocation: CallvanLocationOption? = null,
val arrivalLocation: CallvanLocationOption? = null,
val departureCustomText: String? = null,
val arrivalCustomText: String? = null,
val selectedYear: Int = Calendar.getInstance().get(Calendar.YEAR),
val selectedMonth: Int = Calendar.getInstance().get(Calendar.MONTH) + 1,
val selectedDay: Int = Calendar.getInstance().get(Calendar.DAY_OF_MONTH),
val selectedHour: Int = 12,
val selectedMinute: Int = 0,
val isAm: Boolean = true,
Comment on lines +14 to +16
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

기본 시간이 자정(12:00 AM)으로 설정되어 있습니다.

selectedHour = 12isAm = true 조합은 자정(00:00)을 의미합니다. 콜밴 예약의 기본값으로는 정오(12:00 PM)가 더 적합할 수 있습니다.

💡 정오를 기본값으로 하려면
     val selectedHour: Int = 12,
     val selectedMinute: Int = 0,
-    val isAm: Boolean = true,
+    val isAm: 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.

Suggested change
val selectedHour: Int = 12,
val selectedMinute: Int = 0,
val isAm: Boolean = true,
val selectedHour: Int = 12,
val selectedMinute: Int = 0,
val isAm: Boolean = false,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/create/CallvanCreateState.kt`
around lines 14 - 16, 기본 시간이 현재 자정으로 설정되어 있으므로 CallvanCreateState의 기본값을 정오로
바꾸세요: selectedHour를 12로 유지하고 isAm를 true에서 false로 변경(또는 AM/PM 플래그를 반전)하여 12:00
PM(정오)을 기본값으로 만들고 selectedMinute는 0으로 유지하도록 CallvanCreateState의 필드(selectedHour,
selectedMinute, isAm)를 업데이트하세요.

val maxParticipants: Int = 1,
val isDatePickerVisible: Boolean = false,
val isTimePickerVisible: Boolean = false,
val isLocationPickerVisible: Boolean = false,
val isPickingDeparture: Boolean = true,
val isSubmitting: Boolean = false
) {
val isFormComplete: Boolean
get() {
val departureValid = departureLocation != null &&
(departureLocation != CallvanLocationOption.OTHER || !departureCustomText.isNullOrBlank())
val arrivalValid = arrivalLocation != null &&
(arrivalLocation != CallvanLocationOption.OTHER || !arrivalCustomText.isNullOrBlank())
return departureValid && arrivalValid
}

val formattedDate: String
get() = "${selectedYear}년 ${selectedMonth}월 ${selectedDay}일"

val formattedTime: String
get() = "%02d:%02d".format(selectedHour, selectedMinute)

val amPmText: String
get() = if (isAm) "오전" else "오후"

val participantsText: String
get() = "$maxParticipants 명"

val apiDepartureDate: String
get() = "%04d-%02d-%02d".format(selectedYear, selectedMonth, selectedDay)
Comment on lines +39 to +46
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이런건 UI에서 처리하는게 낫지 않나요

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@KYM-P @skdud0629 @TTRR1007 어떻게 생각하시나요

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

포맷팅 로직이라 여기 넣었는데 ui단이 나을까요? 재사용성이나 유닛 테스트 시에 더 좋지 않을까 생각되는데 어떤가요


val apiDepartureTime: String
get() {
val hour24 = when {
isAm && selectedHour == 12 -> 0
isAm -> selectedHour
!isAm && selectedHour == 12 -> 12
else -> selectedHour + 12
}
return "%02d:%02d".format(hour24, selectedMinute)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,201 @@
package `in`.koreatech.koin.feature.callvan.ui.create

import androidx.lifecycle.ViewModel
import dagger.hilt.android.lifecycle.HiltViewModel
import `in`.koreatech.koin.domain.usecase.callvan.CreateCallvanPostUseCase
import `in`.koreatech.koin.feature.callvan.model.CallvanLocationOption
import java.util.Calendar
import javax.inject.Inject
import org.orbitmvi.orbit.Container
import org.orbitmvi.orbit.ContainerHost
import org.orbitmvi.orbit.syntax.simple.blockingIntent
import org.orbitmvi.orbit.syntax.simple.intent
import org.orbitmvi.orbit.syntax.simple.postSideEffect
import org.orbitmvi.orbit.syntax.simple.reduce
import org.orbitmvi.orbit.viewmodel.container

@Suppress("TooManyFunctions")
@HiltViewModel
class CallvanCreateViewModel @Inject constructor(
private val createCallvanPostUseCase: CreateCallvanPostUseCase
) : ViewModel(), ContainerHost<CallvanCreateState, CallvanCreateSideEffect> {

override val container: Container<CallvanCreateState, CallvanCreateSideEffect> = container(
CallvanCreateState()
)

fun openDepartureLocationPicker() = blockingIntent {
reduce { state.copy(isLocationPickerVisible = true, isPickingDeparture = true) }
}

fun openArrivalLocationPicker() = blockingIntent {
reduce { state.copy(isLocationPickerVisible = true, isPickingDeparture = false) }
}

fun closeLocationPicker() = blockingIntent {
reduce { state.copy(isLocationPickerVisible = false) }
}

fun selectLocation(location: CallvanLocationOption, customText: String? = null) = blockingIntent {
reduce {
if (state.isPickingDeparture) {
state.copy(
departureLocation = location,
departureCustomText = customText,
isLocationPickerVisible = false
)
} else {
state.copy(
arrivalLocation = location,
arrivalCustomText = customText,
isLocationPickerVisible = false
)
}
}
}

fun swapLocations() = blockingIntent {
reduce {
state.copy(
departureLocation = state.arrivalLocation,
arrivalLocation = state.departureLocation,
departureCustomText = state.arrivalCustomText,
arrivalCustomText = state.departureCustomText
)
}
}

fun toggleDatePicker() = blockingIntent {
reduce {
state.copy(
isDatePickerVisible = !state.isDatePickerVisible,
isTimePickerVisible = false
)
}
}

fun updateYear(yearIndex: Int) = blockingIntent {
val currentYear = Calendar.getInstance().get(Calendar.YEAR)
val newYear = currentYear + yearIndex
reduce {
val maxDay = getDaysInMonth(newYear, state.selectedMonth)
state.copy(
selectedYear = newYear,
selectedDay = state.selectedDay.coerceAtMost(maxDay)
)
}
}

fun updateMonth(monthIndex: Int) = blockingIntent {
val newMonth = monthIndex + 1
reduce {
val maxDay = getDaysInMonth(state.selectedYear, newMonth)
state.copy(
selectedMonth = newMonth,
selectedDay = state.selectedDay.coerceAtMost(maxDay)
)
}
}

fun updateDay(dayIndex: Int) = blockingIntent {
reduce { state.copy(selectedDay = dayIndex + 1) }
}

fun resetDate() = blockingIntent {
val today = Calendar.getInstance()
reduce {
state.copy(
selectedYear = today.get(Calendar.YEAR),
selectedMonth = today.get(Calendar.MONTH) + 1,
selectedDay = today.get(Calendar.DAY_OF_MONTH)
)
}
}

fun confirmDate() = blockingIntent {
reduce { state.copy(isDatePickerVisible = false) }
}

fun toggleTimePicker() = blockingIntent {
reduce {
state.copy(
isTimePickerVisible = !state.isTimePickerVisible,
isDatePickerVisible = false
)
}
}

fun updateAmPm(amPmIndex: Int) = blockingIntent {
reduce { state.copy(isAm = amPmIndex == 0) }
}

fun updateHour(hourIndex: Int) = blockingIntent {
reduce { state.copy(selectedHour = hourIndex + 1) }
}

fun updateMinute(minuteIndex: Int) = blockingIntent {
reduce { state.copy(selectedMinute = minuteIndex) }
}

fun resetTime() = blockingIntent {
reduce { state.copy(selectedHour = 12, selectedMinute = 0, isAm = true) }
}

fun confirmTime() = blockingIntent {
reduce { state.copy(isTimePickerVisible = false) }
}

fun decrementParticipants() = blockingIntent {
reduce {
if (state.maxParticipants > 1) {
state.copy(maxParticipants = state.maxParticipants - 1)
} else {
state
}
}
}

fun incrementParticipants() = blockingIntent {
reduce {
if (state.maxParticipants < 8) {
state.copy(maxParticipants = state.maxParticipants + 1)
} else {
state
}
}
}

fun submit() = intent {
val currentState = state
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

currentState를 만든 이유가 있나요

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

intent의 경우는 비동기이기 때문에 로직이 실행중에도 다른 입력으로 변경될 수 있고 이에 따라 값이 변할 수 있는데 이때 빠르게 조작하면서 생기는 오류를 해결하고자 submit()이 실행될 때의 상태를 저장하도록 했습니다!

예를 들어 사용자가 submit을 누른 후 조작 실수로 다른 장소를 클릭했을 때 저 변수가 없다면 그 장소가 그대로 저장되는 문제가 있습니다

if (!currentState.isFormComplete || currentState.isSubmitting) return@intent
reduce { state.copy(isSubmitting = true) }
createCallvanPostUseCase(
departureType = if (currentState.departureLocation == CallvanLocationOption.OTHER) {
currentState.departureCustomText ?: ""
} else {
currentState.departureLocation!!.type
},
arrivalType = if (currentState.arrivalLocation == CallvanLocationOption.OTHER) {
currentState.arrivalCustomText ?: ""
} else {
currentState.arrivalLocation!!.type
},
departureDate = currentState.apiDepartureDate,
departureTime = currentState.apiDepartureTime,
maxParticipants = currentState.maxParticipants
).onSuccess { post ->
Comment on lines +172 to +186
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

UseCase 호출 시 파라미터 매핑이 잘못되었습니다.

CreateCallvanPostUseCase의 시그니처를 확인하면 departureCustomNamearrivalCustomName이 별도의 파라미터로 존재합니다. 현재 코드는 OTHER 위치 타입일 때 customTextdepartureType에 직접 전달하고 있어 API 호출이 올바르지 않습니다.

domain/src/main/java/in/koreatech/koin/domain/usecase/callvan/CreateCallvanPostUseCase.kt의 시그니처:

  • departureType: 위치 타입 (예: "KOREATECH", "OTHER" 등)
  • departureCustomName: OTHER일 때의 커스텀 이름
🐛 수정 제안
         createCallvanPostUseCase(
-            departureType = if (currentState.departureLocation == CallvanLocationOption.OTHER) {
-                currentState.departureCustomText ?: ""
-            } else {
-                currentState.departureLocation!!.type
-            },
-            arrivalType = if (currentState.arrivalLocation == CallvanLocationOption.OTHER) {
-                currentState.arrivalCustomText ?: ""
-            } else {
-                currentState.arrivalLocation!!.type
-            },
+            departureType = currentState.departureLocation!!.type,
+            departureCustomName = if (currentState.departureLocation == CallvanLocationOption.OTHER) {
+                currentState.departureCustomText
+            } else {
+                null
+            },
+            arrivalType = currentState.arrivalLocation!!.type,
+            arrivalCustomName = if (currentState.arrivalLocation == CallvanLocationOption.OTHER) {
+                currentState.arrivalCustomText
+            } else {
+                null
+            },
             departureDate = currentState.apiDepartureDate,
             departureTime = currentState.apiDepartureTime,
             maxParticipants = currentState.maxParticipants
         )
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@feature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/create/CallvanCreateViewModel.kt`
around lines 172 - 186, The call to CreateCallvanPostUseCase is mapping custom
names into departureType/arrivalType; instead pass the actual enum/type into
departureType/arrivalType and supply the custom text to
departureCustomName/arrivalCustomName when the location is OTHER. Update the
call site in CallvanCreateViewModel (the createCallvanPostUseCase invocation)
so: set departureType = currentState.departureLocation!!.type (or
CallvanLocationOption.OTHER.type) and departureCustomName =
currentState.departureCustomText ?: "" when departureLocation == OTHER
(otherwise empty), and likewise set arrivalType =
currentState.arrivalLocation!!.type and arrivalCustomName =
currentState.arrivalCustomText ?: "" when arrivalLocation == OTHER (otherwise
empty), keeping the other params (departureDate, departureTime, maxParticipants)
unchanged.

reduce { state.copy(isSubmitting = false) }
postSideEffect(CallvanCreateSideEffect.NavigateToDetail(post.id))
}.onFailure {
reduce { state.copy(isSubmitting = false) }
postSideEffect(CallvanCreateSideEffect.ShowSubmitError)
}
}

private fun getDaysInMonth(year: Int, month: Int): Int {
return Calendar.getInstance().apply {
clear()
set(year, month - 1, 1)
}.getActualMaximum(Calendar.DAY_OF_MONTH)
}
}
Loading