2026 봄 축제 탭 추가 및 데이터 반영#519
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a 'Festival' filter for partnerships, updating the data models, UI components, and MapViewModel. Feedback suggests using a configuration flag or remote config instead of commenting out code to force the default filter, leveraging automatic serialization for enum mapping, and addressing potential performance and race condition issues in the ViewModel. Additionally, unit tests for the new mapping and filtering logic should be added.
| // val initialFilter = if (newDepartmentId == -1L) FilterType.All else FilterType.Mine // TODO 축제기간 종료 시 주석 해제 | ||
| val initialFilter = FilterType.Festival // TODO 축제기간 한정 Festival 강제. 축제기간 끝나면 주석 |
There was a problem hiding this comment.
축제 기간을 위해 초기 필터를 Festival로 강제하고 기존 로직을 주석 처리하는 방식은 유지보수 시 실수할 위험이 큽니다. 주석 처리 대신, 현재가 축제 기간인지 판단하는 로직(예: 원격 구성이나 상수 플래그)을 도입하여 조건부로 initialFilter를 결정하는 것이 더 안전합니다. 또한, 이 변경으로 인해 기존에 학과가 설정된 사용자의 기본 경험이 변경되므로 의도된 사항인지 확인이 필요합니다.
| periodType = when (it.periodType) { | ||
| "FESTIVAL" -> PeriodType.FESTIVAL | ||
| else -> PeriodType.NORMAL | ||
| } |
There was a problem hiding this comment.
PeriodType enum에 @Serializable을 적용하고 DTO(PartnershipInfo)에서 직접 PeriodType 타입을 사용하면, when 문을 통한 수동 매핑 없이 kotlinx.serialization이 자동으로 처리하게 할 수 있습니다. 이는 코드 가독성을 높이고 오타로 인한 실수를 방지하는 데 도움이 됩니다. 또한, 새로 추가된 이 매핑 로직에 대한 단위 테스트(PartnershipResponseMapperBehaviorSpec)가 누락되었으니 추가가 필요합니다.
| private fun loadFestivalPartnerships() { | ||
| viewModelScope.launch { | ||
| val current = uiState.value | ||
| val currentData = if (current is UiState.Success) current.data else MapState() | ||
|
|
||
| _uiState.value = UiState.Loading | ||
|
|
||
| val partnerships = partnershipRepository.getAllPartnerships().mapNotNull { | ||
| val festivalInfos = | ||
| it.partnershipInfos.filter { info -> info.periodType == PeriodType.FESTIVAL } | ||
| if (festivalInfos.isEmpty()) return@mapNotNull null | ||
|
|
||
| it.copy( | ||
| partnershipInfos = festivalInfos | ||
| ) | ||
| } | ||
|
|
||
| _uiState.value = UiState.Success( | ||
| currentData.copy( | ||
| partnerships = partnerships, | ||
| filterChangeResult = null | ||
| ) | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
loadFestivalPartnerships 함수에서 전체 데이터를 가져와 필터링하는 작업은 데이터 양이 많아질 경우 UI 스레드에 부담을 줄 수 있으므로, 계산 집약적인 작업은 백그라운드 스레드에서 수행하는 것이 좋습니다. 또한, 사용자가 필터를 빠르게 전환할 때 이전 요청이 현재 상태를 덮어쓰는 레이스 컨디션이 발생할 수 있으므로 Job 관리를 통한 요청 취소 로직 도입을 권장합니다. 마지막으로, Festival 필터링 로직에 대한 ViewModel 단위 테스트(MapViewModelBehaviorSpec)도 추가해 주세요.
| val initialFilter = if (newDepartmentId == -1L) FilterType.All else FilterType.Mine | ||
|
|
||
| // val initialFilter = if (newDepartmentId == -1L) FilterType.All else FilterType.Mine // TODO 축제기간 종료 시 주석 해제 | ||
| val initialFilter = FilterType.Festival // TODO 축제기간 한정 Festival 강제. 축제기간 끝나면 주석 |
There was a problem hiding this comment.
이 부분 무조건 Festival이 아니라 Festival인 애가 존재하는 경우에만 이렇게 initialFilter를 Festival로 설정해야해요!!
| val endDate: String? = null | ||
| val endDate: String? = null, | ||
| @SerialName("periodType") | ||
| val periodType: String? = null |
There was a problem hiding this comment.
kotlinx.serialization이 enum 직렬화 처리를 해줘서 String을 받고 아래에서 파싱하는 방법이 아니어도 됩니다! 대신 PeriodType를 @serializable로 달아야 해요.
Festival 정보가 존재하는지 확인하기 위해 fetch한 데이터는 첫 탭에서 그대로 활용
PeraSite
left a comment
PeraSite
left a comment
There was a problem hiding this comment.
private fun List.festivalPartnerships() 은 mapNotNull + return null보단 filter를 쓰는 것이 적합하지 않나용?!
다만 시간이 없으므로 우선 승인합니당
|
List 인 상태에서 그 내부에 또 partnershipInfos: List 가 중첩인 구조인데 partnershipInfos.periodType이 FESTIVAL 인게 하나도 없을땐 그 partnership 자체를 패스시켜야 하는데 뭔가 딱 떠오르는 방법이 없었어요 ㅠㅠ |
private fun List<Partnership>.festivalPartnerships(): List<Partnership> =
filter { partnership ->
partnership.partnershipInfos.any { it.periodType == PeriodType.FESTIVAL }
}이렇게 적으면 될 것 같아요! 페스티벌이 하나도 없으면 패스시킨다가 아니라 페스티벌이 하나라도 있는 애를 골라내는거죵 |
|
근데 [축제] 탭인 상태에서 가게 눌렀을때, 축제에 해당하는 할인 항목만 보여줘야 하지 않나요?? 그건 상관 없나요? |
|
아 UI에서 거르면 되는구나 확인했습니다 |
|
굿굿입니당 |
2 participants
Summary
기존 PartnerShipInfo에 periodType을 추가하고 지도에 [축제] 탭을 추가해서 PeriodType.FESTIVAL 인 데이터만 모아볼 수 있도록 했습니다
Describe your changes
Android.Studio.2026.05.11.180457.mp4
Issue
To reviewers