Bugfix/#167 다가오는 세션 상태별 출석 체크 기능 수정#168
Conversation
|
Important Review skippedAuto reviews are limited to specific labels. 🏷️ Labels to auto review (1)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Walkthrough이번 변경에서는 홈 화면의 세션/출석 UI 및 데이터 모델 구조를 대대적으로 리팩터링하고, 출석 코드 입력 및 상태 처리 로직을 개선하였습니다. 주요 변경점은 세션 데이터 모델의 구조화, 출석 상태별 UI/로직 분리, 출석 코드 입력의 상태 관리 외부화, 그리고 저장소/도메인 계층의 메서드 시그니처 및 반환 타입 일치화입니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant HomeScreen
participant HomeViewModel
participant ScheduleRepository
participant API
User->>HomeScreen: 화면 진입/새로고침/출석 요청
HomeScreen->>HomeViewModel: EnterHomeScreen / RefreshUpcomingSession / ClickRequestAttendance
HomeViewModel->>ScheduleRepository: getSessions() / getUpcomingSessionInfo()
ScheduleRepository->>API: 세션/출석 정보 요청
API-->>ScheduleRepository: 세션/출석 데이터 반환
ScheduleRepository-->>HomeViewModel: HomeSessionList / UpcomingSessionInfo 반환
HomeViewModel-->>HomeScreen: 상태 업데이트 (세션리스트, 출석상태 등)
User->>HomeScreen: 출석코드 입력
HomeScreen->>HomeViewModel: ChangeAttendanceCodeDigits
HomeViewModel-->>HomeScreen: 코드 입력 상태/에러 반영
HomeViewModel->>ScheduleRepository: postAttendance() (출석 요청)
ScheduleRepository->>API: 출석 처리 요청
API-->>ScheduleRepository: 출석 결과 반환
ScheduleRepository-->>HomeViewModel: 성공/실패 결과
HomeViewModel-->>HomeScreen: 출석 결과/에러 상태 반영
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
DongChyeon
requested review from
TaeseongYun and
jinukeu
and removed request for
TaeseongYun
DongChyeon
force-pushed
the
bugfix/#167-session-attendance
branch
from
9bd68a2 to
1c67717
Compare
DongChyeon
force-pushed
the
bugfix/#167-session-attendance
branch
from
1c67717 to
f35d24d
Compare
jinukeu
left a comment
jinukeu
left a comment
There was a problem hiding this comment.
HomeState 뷰 로직 데이터 제거
-> 개인적인 의견이지만 ViewModel에서는 도메인 관련 데이터만 들고 있고 UI에서 분기처리하는 것이 좋다고 생각했습니다
Q. ViewModel에서는 도메인 관련 데이터만 들고있고 UI에서 분기처리하는 것이 좋은 이유도 말씀해주실 수 있을까요?
제 의견을 말씀드리면, UI에는 로직을 최소화하는게 좋다고 생각해요.
Compose 공식 문서에서도 나와있는 상태 호이스팅 처럼 UI, 컴포넌트에 로직이 많아지게 되면 재사용성이 떨어지게 됩니다.
MVVM에서도 마찬가지로 View는 아무것도 모르는 바보로 설정하고 보여주기만 하는 것 처럼요.
View를 바보로 만들면 어떤 점이 좋냐면 ...
XML 기반에서 Compose로 넘어간다고 가정하겠습니다.
만약 XML 내에서 LocalDate를 파싱한 로직이 있다면 해당 로직을 Compose 코드로도 이동해야하는 불편함이 있습니다.
하지만 LocalDate 파싱 로직을 전부 ViewModel 단에서 처리했다면 XML -> Compose 마이그레이션 시, LocalDate 파싱 로직은 전혀 손댈 필요가 없어진다는 장점이 있어요.
| val attendanceTitle: String | ||
| get() { | ||
| val today = LocalDate.now() | ||
| val formatter = DateTimeFormatter.ofPattern("yyyy-MM-dd") | ||
|
|
||
| val hasTodaySession = sessions.any { session -> | ||
| runCatching { LocalDate.parse(session.date, formatter) } | ||
| .getOrNull() == today | ||
| } | ||
|
|
||
| return when { | ||
| hasTodaySession -> { | ||
| "세션 당일이에요! 활기찬 하루 되세요 :)" | ||
| } | ||
| sessions.all { it.progressPhase == ScheduleProgressPhase.DONE } -> { | ||
| "다가오는 세션이 없어요." | ||
| } | ||
| else -> "다가오는 세션을 준비해볼까요?" | ||
| } | ||
| } | ||
|
|
||
| val todayOrUpcomingSession: Session? | ||
| get() { | ||
| val today = LocalDate.now() | ||
| val formatter = DateTimeFormatter.ofPattern("yyyy-MM-dd") | ||
|
|
||
| val todaySession = sessions.firstOrNull { session -> | ||
| runCatching { LocalDate.parse(session.date, formatter) } | ||
| .getOrNull() == today | ||
| } | ||
|
|
||
| if (todaySession != null) return todaySession | ||
|
|
||
| return sessions | ||
| .filter { it.progressPhase != ScheduleProgressPhase.DONE } | ||
| .minByOrNull { | ||
| runCatching { LocalDate.parse(it.date, formatter) } | ||
| .getOrDefault(LocalDate.MAX) | ||
| } | ||
| } | ||
|
|
||
| val attendState: AttendState | ||
| get() { | ||
| val session = todayOrUpcomingSession ?: return AttendState.NOT_YET | ||
| if (session.isAttended) return AttendState.COMPLETED | ||
| if (session.progressPhase == ScheduleProgressPhase.DONE) return AttendState.NOT_YET | ||
|
|
||
| val now = LocalTime.now() | ||
| val formatter = DateTimeFormatter.ofPattern("HH:mm") | ||
|
|
||
| val startTime = runCatching { LocalTime.parse(session.startTime, formatter) }.getOrNull() | ||
| val endTime = runCatching { LocalTime.parse(session.endTime, formatter) }.getOrNull() | ||
|
|
||
| val attendOpenTime = startTime?.minusMinutes(20) | ||
|
|
||
| return when { | ||
| attendOpenTime == null || endTime == null -> AttendState.NOT_YET | ||
| now.isBefore(attendOpenTime) -> AttendState.NOT_YET | ||
| else -> AttendState.POSSIBLE | ||
| } | ||
| } |
There was a problem hiding this comment.
Q. 이 로직들이 전부 Compose로 이동했다고 봤는데 맞을까요?
There was a problem hiding this comment.
해당 로직들은 relativeDays, canCheckIn 필드값으로 판단할 수 있기 때문에 클라이언트단에서 굳이 LocalDate를 사용해서
계산할 필요가 없다고 생각해서 지웠습니다.
| val inputFormat = DateTimeFormatter.ofPattern("yyyy-MM-dd") | ||
| val outputFormat = DateTimeFormatter.ofPattern("M월 d일") |
There was a problem hiding this comment.
리컴포지션이 발생할 때마다 inputFormat, outputFormat이 매번 새로 생성될 것 같아요
jinukeu
left a comment
jinukeu
left a comment
There was a problem hiding this comment.
우선 일정을 맞춰야하기 때문에 어프로브 합니다.
제 생각이 무조건 맞다고 생각하지 않기 때문에 편하게 의견 나누면서 더 좋은 방향으로 코드 설계하면 좋을거같아요!
val hasTodaySession: Boolean
get() = sessionList.sessions.any { it.relativeDays == 0 }와 같은 도메인 기반의 상태에서의 파생 정보는 들고 있는게 좋다고 생각하지만 사실 표시할 UI 데이터까지 뷰모델에서 들고 있어도 크게 문제는 없을 것 같지만 |
예를 들어 이 부분은 저도 같은 생각이에요! 확인 했습니다 |
| fun List<DateGroupedSchedule>.toState() = flatMap { session -> | ||
| session.schedules.map { schedule -> | ||
| HomeState.Session( | ||
| id = schedule.id, | ||
| title = schedule.name, | ||
| date = schedule.date, | ||
| place = schedule.place.orEmpty(), | ||
| startTime = schedule.time.orEmpty(), | ||
| endTime = schedule.endTime.orEmpty(), | ||
| startDayOfWeek = session.dayOfTheWeek, | ||
| progressPhase = schedule.scheduleProgressPhase | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
There was a problem hiding this comment.
확인했습니다.
https://github.com/PRNDcompany/android-style-guide/blob/main/Architecture.md
레이어별로 Model convert 하는건 크게 어색하지 않다고 생각하긴해용
There was a problem hiding this comment.
넵넵! 다음번에 작업할 때 참고하겠습니다
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🔭 Outside diff range comments (2)
feature/home/src/main/java/com/yapp/feature/home/component/HomeHeader.kt (2)
54-55:⚠️ Potential issue💡
pageIndex경계값 처리 누락
indexOfFirst가 조건에 맞는 항목을 찾지 못하면-1을 반환합니다. 이후LaunchedEffect에서pageIndex > 0만을 검사하면-1인 경우는 물론, 0번째 항목(리스트 맨 앞)도 스크롤 대상에서 빠집니다.- val pageIndex = sessions.indexOfFirst { it.id == upcomingSessionId } + val pageIndex = sessions.indexOfFirst { it.id == upcomingSessionId }.coerceAtLeast(0)또는
LaunchedEffect조건을>= 0으로 변경해도 됩니다.
151-155: 🛠️ Refactor suggestion
⚠️ 첫 번째 아이템으로 스크롤되지 않음앞서 언급한 문제와 연관되어, 현재 조건은
pageIndex > 0이므로 0번째(가장 앞) 세션이 ‘다가오는 세션’일 때 자동 스크롤이 일어나지 않습니다.- if (pageIndex > 0 && selectedIndex != pageIndex) { + if (pageIndex >= 0 && selectedIndex != pageIndex) {사용자 경험(UX) 측면에서 최우선 노출이 의도라면 위와 같이 수정하는 편이 안전합니다.
♻️ Duplicate comments (1)
feature/home/src/main/java/com/yapp/feature/home/component/HomeAttendaceContents.kt (1)
86-92: 🧹 Nitpick (assertive)DateTimeFormatter를 recomposition마다 새로 생성하고 있습니다
inputFormat/outputFormat두 포맷터가@Composable본문 안에서 매 리컴포지션마다 새로 할당됩니다.DateTimeFormatter는 thread-safe 하므로 전역val이나remember로 한 번만 생성해도 됩니다. 이전 커밋에도 동일 지적이 있었으므로 재검토 부탁드립니다.- val inputFormat = DateTimeFormatter.ofPattern("yyyy-MM-dd") - val outputFormat = DateTimeFormatter.ofPattern("M월 d일") + val inputFormat = remember { DateTimeFormatter.ofPattern("yyyy-MM-dd") } + val outputFormat = remember { DateTimeFormatter.ofPattern("M월 d일") }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (17)
core/data-api/src/main/java/com/yapp/dataapi/ScheduleRepository.kt(1 hunks)core/data/src/main/java/com/yapp/core/data/data/repository/ScheduleRepositoryImpl.kt(2 hunks)core/data/src/main/java/com/yapp/core/data/remote/model/response/SessionResponse.kt(2 hunks)core/data/src/main/java/com/yapp/core/data/remote/model/response/UpcomingSessionAttendanceResponse.kt(1 hunks)core/domain/src/main/java/com/yapp/domain/GetSessionsUseCase.kt(0 hunks)core/model/src/main/java/com/yapp/model/Sessions.kt(1 hunks)core/model/src/main/java/com/yapp/model/exceptions/YappExceptions.kt(1 hunks)feature/home/src/main/java/com/yapp/feature/home/HomeContract.kt(1 hunks)feature/home/src/main/java/com/yapp/feature/home/HomeScreen.kt(3 hunks)feature/home/src/main/java/com/yapp/feature/home/HomeViewModel.kt(4 hunks)feature/home/src/main/java/com/yapp/feature/home/component/HomeAttendaceContents.kt(5 hunks)feature/home/src/main/java/com/yapp/feature/home/component/HomeContentsBody.kt(2 hunks)feature/home/src/main/java/com/yapp/feature/home/component/HomeHeader.kt(5 hunks)feature/home/src/main/java/com/yapp/feature/home/convert/HomeSessionModelToState.kt(0 hunks)feature/home/src/main/java/com/yapp/feature/home/dialog/AttendanceDialog.kt(6 hunks)feature/home/src/main/res/values/strings.xml(2 hunks)feature/schedule/src/main/java/com/yapp/feature/schedule/ScheduleViewModel.kt(2 hunks)
💤 Files with no reviewable changes (2)
- feature/home/src/main/java/com/yapp/feature/home/convert/HomeSessionModelToState.kt
- core/domain/src/main/java/com/yapp/domain/GetSessionsUseCase.kt
🧰 Additional context used
🧬 Code Graph Analysis (2)
feature/schedule/src/main/java/com/yapp/feature/schedule/ScheduleViewModel.kt (1)
core/domain/src/main/java/com/yapp/domain/RunCatchingIgnoreCancelled.kt (1)
runCatchingIgnoreCancelled(5-8)
feature/home/src/main/java/com/yapp/feature/home/dialog/AttendanceDialog.kt (1)
core/designsystem/src/main/java/com/yapp/core/designsystem/component/button/solid/SolidButton.kt (1)
YappSolidPrimaryButtonLarge(113-142)
🔇 Additional comments (24)
core/model/src/main/java/com/yapp/model/exceptions/YappExceptions.kt (3)
22-28: YappServerError에 새로운 오류 코드 추가서버 오류 코드가 확장되어 세션 및 출석 관련 에러 처리를 개선했습니다. 특히
SCH_1005(예정된 세션 없음)와ATD_1001(출석 코드 불일치) 코드가 추가되어 더 구체적인 예외 처리가 가능해졌습니다.
33-35: 예외 클래스 메시지 명시적 정의 개선기존 예외 클래스들의 생성자에서 빈 괄호를 제거하고 명시적인 오류 메시지를 정의하여 코드 일관성과 가독성이 향상되었습니다.
42-43: 세션 및 출석 관련 새로운 예외 클래스 추가출석 코드 불일치와 예정된 세션 없음 상황을 처리하기 위한 새로운 예외 클래스가 추가되었습니다. 이는 PR 목표에 맞게 출석 기능의 오류 처리를 개선합니다.
core/data-api/src/main/java/com/yapp/dataapi/ScheduleRepository.kt (2)
3-4: 도메인 모델 import 변경
HomeSession에서HomeSessionList로 import가 변경되었습니다. 이는 모델 구조 변경에 맞게 적절히 수정되었습니다.
8-9: getSessions() 메서드 반환 타입 변경
getSessions()메서드의 반환 타입이HomeSession에서HomeSessionList로 변경되었습니다. 이는 세션 데이터를 보다 세분화된 모델로 구조화하는 리팩토링에 부합합니다.core/data/src/main/java/com/yapp/core/data/data/repository/ScheduleRepositoryImpl.kt (2)
6-9: 필요한 import 추가 및 변경
toHomeSessionListModel확장 함수 import가 추가되고, 모델 클래스 import가HomeSession에서HomeSessionList로 변경되었습니다. 이는 리포지토리 구현의 변경 사항을 적절히 반영합니다.
22-24: getSessions() 구현 간소화
getSessions()메서드 구현이toHomeSessionListModel()확장 함수를 사용하여 훨씬 간결해졌습니다. 이전 구현에서는 세션 데이터와 예정된 세션 ID를 수동으로 추출하고 변환했지만, 지금은 단일 호출로 처리됩니다. 이러한 간소화는 유지보수성을 향상시키고 코드를 더 읽기 쉽게 만듭니다.core/data/src/main/java/com/yapp/core/data/remote/model/response/UpcomingSessionAttendanceResponse.kt (1)
33-34: remainingDays 계산 로직 수정
remainingDays계산 방식이max(0, relativeDays)에서max(0, -relativeDays)로 변경되었습니다. 이는relativeDays값의 부호를 반전시킨 후 음수 값을 방지하는 논리적 수정입니다. PR의 목표인 날짜 계산 로직 간소화에 부합하며, 세션이 오늘인지 판단하는 로직을 개선합니다.feature/home/src/main/java/com/yapp/feature/home/component/HomeContentsBody.kt (2)
29-31: 함수 매개변수 변경이 잘 이루어졌습니다!이전에는 정적 문자열을 받았지만, 이제 도메인 모델인
UpcomingSessionInfo?를 직접 받아 상태에 따라 동적으로 메시지를 표시하는 방식으로 개선되었습니다. 이는 ViewModel이 UI 로직을 처리하는 대신 도메인 데이터만 전달하고 뷰 레이어에서 표시 로직을 처리하는 원칙에 맞게 리팩토링된 것입니다.
38-44: 문자열 리소스를 활용한 동적 텍스트 처리가 잘 구현되었습니다.세션 상태에 따라 다른 메시지를 표시하는 로직이
when표현식을 통해 깔끔하게 구현되었습니다. 하드코딩된 문자열 대신 문자열 리소스를 사용함으로써 다국어 지원과 중앙 관리가 용이해졌습니다.core/data/src/main/java/com/yapp/core/data/remote/model/response/SessionResponse.kt (2)
62-65: 적절한 데이터 모델 변환 함수 추가
SessionResponse를HomeSessionList도메인 모델로 변환하는 확장 함수가 잘 구현되었습니다. 세션 목록과 예정된 세션 ID를 포함하여 UI 레이어에서 필요한 모든 데이터를 제공합니다.
67-80: Null 안전 처리가 잘 구현되었습니다.
place.orEmpty()와endTime.orEmpty()를 사용하여 null 값을 빈 문자열로 처리하는 방식이 안전합니다. 도메인 모델로의 변환 과정에서 모든 필요한 필드가 올바르게 매핑되었습니다.feature/home/src/main/res/values/strings.xml (3)
26-28: 홈 화면 출석 상태 메시지가 잘 정의되었습니다.세션 출석 관련 상태에 따른 다양한 메시지가 명확하게 정의되었습니다. 세션이 없을 때, 세션 당일일 때, 세션이 다가올 때에 대한 사용자 친화적인 메시지가 추가되었습니다.
30-40: 세션 상태 및 시간 관련 문자열이 체계적으로 추가되었습니다.세션의 다양한 상태(종료, 예정, 진행 중 등)와 시간 표시에 관한 문자열이 일관된 명명 규칙(
session_*)으로 추가되었습니다. 특히%1$s와 같은 포맷 지정자를 사용하여 동적 텍스트를 지원하는 방식이 적절합니다.
45-45: 출석 코드 오류 메시지 추가출석 코드가 일치하지 않을 때 표시할 오류 메시지가 추가되었습니다. 이는 PR 내용 중 "출석 인증 코드 오류에 대한 예외 처리 추가" 작업과 일치합니다.
feature/schedule/src/main/java/com/yapp/feature/schedule/ScheduleViewModel.kt (5)
91-103: 로딩 상태 관리 개선API 호출 전에
isLoading = true로 설정하고, 성공/실패 각 케이스에서isLoading = false로 명시적으로 처리하는 방식으로 일관성 있게 개선되었습니다. 이제 더 나은 사용자 경험을 제공할 수 있습니다.
105-117: Result 패턴을 활용한 에러 처리 개선Kotlin의
Result타입과onSuccess/onFailure확장 함수를 활용하여 성공과 실패 케이스를 명확하게 분리했습니다. 이 패턴은 코드 가독성을 높이고 에러 처리를 보다 명시적으로 만듭니다.
119-138: 일관된 로딩 상태 관리 패턴 적용
refreshScheduleInfo함수에서도 동일한 패턴으로 로딩 상태를 관리하도록 리팩토링되었습니다. 이는 코드 전체에 걸쳐 일관된 상태 관리 방식을 제공합니다.
140-157: 리프레시 함수의 에러 처리 개선리프레시 함수에서도 로딩 상태를 명확하게 관리하고 있으며,
onFailure블록에서 로딩 상태를 false로 리셋하는 로직이 추가되었습니다. 이를 통해 에러 발생 시에도 UI가 로딩 상태에 머물지 않도록 보장합니다.
159-176: 세션 리프레시 로직 일관성 향상세션 데이터 리프레시 함수도 동일한 패턴으로 구현되어 코드베이스 전체의 일관성을 유지하고 있습니다. 이러한 일관된 패턴은 유지보수성을 향상시킵니다.
feature/home/src/main/java/com/yapp/feature/home/HomeViewModel.kt (1)
136-145:upcomingSessionId의 최신화 여부를 확인해주세요.출석 성공 시
sessionList내 단일 세션의attendanceStatus만 갱신하고, 리스트가 보유한upcomingSessionId는 그대로 유지됩니다.
만약 “출석을 마친 세션”이 더 이상 ‘다가오는 세션’으로 간주되지 않는다면, 이 필드도 null 처리하거나 다음 세션 ID로 교체해야 합니다. 로직 검증 부탁드립니다.feature/home/src/main/java/com/yapp/feature/home/component/HomeHeader.kt (1)
165-170: null 허용 시간 값 처리 확인 필요
startTime/endTime이null이 될 수 있도록 변경되었지만,
아래formatTimeRange헬퍼에서null입력을 제대로 처리하는지 확인이 필요합니다. 미처리 시 NPE 또는"null"문자열이 노출될 수 있습니다.
해당 함수 내부에서null을 빈 문자열로 치환하는지 다시 한 번 점검해주세요.feature/home/src/main/java/com/yapp/feature/home/HomeScreen.kt (1)
96-109: 세로 스크롤 & 스냅핑 중첩 여부 점검
Column에verticalScroll를 부여하면서, 내부에LazyRow(스냅핑)·PullToRefresh가 중첩돼 있습니다. 스크롤 제스처가 겹칠 때 드래그가 먹히지 않거나 끊기는 현상이 보고된 바 있으니 실제 단말에서 제스처 충돌 여부를 꼭 검증해주세요.feature/home/src/main/java/com/yapp/feature/home/component/HomeAttendaceContents.kt (1)
167-183:isAttended분기에서 Chip 텍스트가 문맥과 맞지 않을 가능성
isAttended == true(이미 출석 완료) 인 경우에 “세션 진행중”(session_in_progress) Chip 을 띄우고 있습니다. 출석을 마쳤다면 세션이 진행 중이 아닐 수도 있어 사용자 혼란이 생길 수 있습니다.status값을 확인해 출석 완료, 진행 중 등을 분리 렌더링하는지 한 번 더 확인 부탁드립니다.
| data class HomeSessionList( | ||
| val sessions: List<HomeSession>, | ||
| val upcomingSessionId: String? | ||
| ) |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
upcomingSessionId의 널 처리 정책을 명확히 해주세요.
현재 String? 으로 선언되어 있지만,
실제로 “다가오는 세션이 없다”는 상태가 자주 조회된다면 null 대신 ""(빈 문자열)·Optional 패턴·별도 상태 객체 등으로 표현하는 방안도 고려해볼 수 있습니다.
특히 Kotlin 컬렉션 검색 시 null 체크 분기보다, non-null 보장 값이 있으면 호출부 로직이 간결해집니다.
| data class HomeSession( | ||
| val id: String, | ||
| val name: String, | ||
| val place: String, | ||
| val date: String, | ||
| val dayOfWeek: String, | ||
| val relativeDays: Int, | ||
| val startTime: String?, | ||
| val endTime: String?, | ||
| val progressPhase: ScheduleProgressPhase, | ||
| val attendanceStatus: AttendanceStatus? | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
도메인 모델에서 날짜·시간을 String으로 보유하면 파싱‧포매팅 책임이 분산될 위험이 있습니다.
date, startTime, endTime을 모두 String으로 유지하면
- API 포맷이 변경될 때 영향을 받는 범위가 넓어지고
- UI/도메인/데이터 계층마다 반복 파싱이 발생하며
- 잘못된 형식-런타임 오류를 컴파일 단계에서 걸러내기 어렵습니다.
java.time.LocalDate·LocalTime·LocalDateTime(혹은 kotlinx-datetime)으로 타입을 명시하면 형 안전성과 연산 편의성이 크게 향상됩니다.
필요 시 레이어 간 DTO ↔︎ Domain 변환에서만 문자열 ↔︎ 날짜 타입 변환을 수행하도록 리팩터링을 권장드립니다.
| when (intent) { | ||
| HomeIntent.EnterHomeScreen -> { loadHomeInfo( reduce,postSideEffect) } | ||
| HomeIntent.EnterHomeScreen -> { | ||
| loadSessionInfo(reduce, postSideEffect) | ||
| loadUpcomingSessionInfo(reduce, postSideEffect) | ||
| } | ||
| HomeIntent.RefreshUpcomingSession -> { | ||
| loadUpcomingSessionInfo(reduce, postSideEffect) | ||
| } | ||
| HomeIntent.ClickShowAllSession -> postSideEffect(HomeSideEffect.NavigateToSchedule) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
isLoading 플래그가 두 개의 병렬 코루틴에서 서로 덮어써 UI 깜빡임을 유발할 수 있습니다.
EnterHomeScreen에서 loadSessionInfo()와 loadUpcomingSessionInfo()를 각각 별도 코루틴으로 실행할 때:
- 첫 번째 작업이 끝나면
isLoading=false로 바뀌어 로딩 인디케이터가 사라짐. - 두 번째 작업이 아직 진행 중일 수 있어 사용자에게 ‘멈춘 듯’한 인상을 줄 가능성.
해결 방향 예시:
- loadSessionInfo(...)
- loadUpcomingSessionInfo(...)
+ viewModelScope.launch {
+ reduce { copy(isLoading = true) }
+ coroutineScope {
+ launch { loadSessionInfoInternal() }
+ launch { loadUpcomingSessionInfoInternal() }
+ }
+ reduce { copy(isLoading = false) }
+ }또는 isSessionLoading/isUpcomingLoading과 같은 세부 플래그를 별도로 두고, UI에서 any { it } 로 종합 표시하도록 분리하면 더욱 명확합니다.
Also applies to: 99-121
| }.onFailure { | ||
| reduce { copy(showAttendanceCodeError = true) } | ||
| it.record() | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
모든 실패를 동일한 “코드 오류” 표시로 처리하면 원인 파악이 어려워집니다.
네트워크 오류, 토큰 만료, 서버 5xx 등 출석 코드 자체와 무관한 실패까지 showAttendanceCodeError=true 로 표시됩니다.
오류 유형별로 분기하여
- 사용-재시도 안내(
SnackBar/Toast) - 자동 재로그인 유도
- 실제 코드 불일치 경고
등 차별화된 UX를 제공하는 것이 좋습니다.
feature/home/src/main/java/com/yapp/feature/home/dialog/AttendanceDialog.kt
Show resolved
Hide resolved
| val focusRequesters = remember { List(codeLength) { FocusRequester() } } | ||
| val isAllValuesEntered by remember(values) { | ||
| derivedStateOf { | ||
| values.all { it.isNotEmpty() } | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
FocusRequester 크기 불일치 가능성
codeLength = 4로 고정했지만, code 파라미터 길이가 4가 아닐 경우 IndexOutOfBoundsException이 발생할 수 있습니다.
- val codeLength = 4
- val focusRequesters = remember { List(codeLength) { FocusRequester() } }
+ val focusRequesters = remember { List(code.size) { FocusRequester() } }또는 require(code.size == 4)와 같이 사전 검증을 권장드립니다.
feature/home/src/main/java/com/yapp/feature/home/dialog/AttendanceDialog.kt
Show resolved
Hide resolved
feature/home/src/main/java/com/yapp/feature/home/component/HomeAttendaceContents.kt
Outdated
Show resolved
Hide resolved
feature/home/src/main/java/com/yapp/feature/home/component/HomeAttendaceContents.kt
Outdated
Show resolved
Hide resolved
DongChyeon
changed the title
2 participants
💡 Issue
🌱 Key changes
v1/sessions/결과값에서 upcomingSesssionId와 같은 객체를 사용-> 임박한 세션 정보를
v1/sessions/upcoming에서 불러오도록 수정->
relativeDays필드값을 이용해 오늘의 세션인지 판단 및 날짜 계산 로직 최소화-> 개인적인 의견이지만 ViewModel에서는 도메인 관련 데이터만 들고 있고 UI에서 분기처리하는 것이 좋다고 생각했습니다
✅ To Reviewers
feature 단에 convert나 model이 있는 것이 다른 모듈에 비해 통일성이 없는 것 같아서... 제거했습니다.
📸 스크린샷
default.mp4
Summary by CodeRabbit
신규 기능
버그 수정
리팩터링
문서화
예외 및 오류 처리