Feature/#132 : 마이페이지 출석내역 내비게이션 연결 및 작업#133
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 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:
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 (
|
|
|
||
| @Singleton | ||
| @Provides | ||
| fun provideAttendanceApi(retrofit: Retrofit): AttendanceApi { | ||
| return retrofit.create() | ||
| } | ||
|
|
||
| @Singleton | ||
| @Provides | ||
| fun providesSessionsApi(retrofit: Retrofit) = retrofit.create<SessionApi>() | ||
| } |
There was a problem hiding this comment.
| @Singleton | |
| @Provides | |
| fun provideAttendanceApi(retrofit: Retrofit): AttendanceApi { | |
| return retrofit.create() | |
| } | |
| @Singleton | |
| @Provides | |
| fun providesSessionsApi(retrofit: Retrofit) = retrofit.create<SessionApi>() | |
| } | |
| @Singleton | |
| @Provides | |
| fun provideAttendanceApi(@AuthRetrofit retrofit: Retrofit): AttendanceApi { | |
| return retrofit.create(AttendanceApi::class.java) | |
| } | |
| @Singleton | |
| @Provides | |
| fun providesSessionsApi(@AuthRetrofit retrofit: Retrofit): Session { | |
| return retrofit.create(SessionApi::class.java) | |
| } | |
| } |
R: @AuthRetrofit 어노테이션을 붙여야 AuthInterceptor 가 붙은 레트로핏 클라이언트를 사용할 수 있어요.
There was a problem hiding this comment.
코멘트 적용 완료 처리했습니다 ~
| fun providePostsApi(retrofit: Retrofit): PostsApi { | ||
| return retrofit.create(PostsApi::class.java) | ||
| } | ||
|
|
||
| @Singleton | ||
| @Provides | ||
| fun provideAttendanceApi(@AuthRetrofit retrofit: Retrofit): AttendanceApi { | ||
| return retrofit.create() | ||
| } | ||
|
|
||
| @Singleton | ||
| @Provides | ||
| fun providesSessionsApi(@AuthRetrofit retrofit: Retrofit) = retrofit.create<SessionApi>() | ||
| } |
There was a problem hiding this comment.
A : 기능적으로는 문제 없지만, 생성하는 코드 스타일을 통일하면 좋을 것 같습니다
There was a problem hiding this comment.
반영했습니다 !
TaeseongYun
force-pushed
the
feature/#129-mypage-ui
branch
from
b750833 to
392fdd7
Compare
TaeseongYun
changed the base branch from
feature/#129-mypage-ui
to
release/v1.1.0
TaeseongYun
force-pushed
the
feature/#132-mypage-navigate
branch
from
776f421 to
367ac9c
Compare
DongChyeon
left a comment
DongChyeon
left a comment
There was a problem hiding this comment.
세션 API 쪽 작업하신거 저도 필요해서 cherry-pick 해서 사용했었는데
진욱님이 해당 커밋 쪽 보고 코드 리뷰 남겨주신 거 그대로 옮깁니다.
| LaunchedEffect(viewModel.store.sideEffects) { | ||
| viewModel.store.sideEffects.onEach { effect -> | ||
| when(effect) { | ||
| SideEffect.NavigateToBack -> { | ||
| navigateToBack() | ||
| } | ||
| } | ||
| }.launchIn(scope) | ||
| } | ||
| } |
There was a problem hiding this comment.
C: collectsWithLifeCycle 확장함수 만들어놔서 이거 써주시면 좋을거같아요
| data object OnEntryScreen : AttendHistoryIntent | ||
| data object OnClickBackButton : AttendHistoryIntent | ||
| } |
There was a problem hiding this comment.
C: Intent는 'On' 없이 그냥 EntryScreen으로 작성하고 있어서 컨벤션 맞춰주시면 감사하겠습니다.
| val alertMessage: String | ||
| get() { | ||
| return if (absenceCount >= 10) { | ||
| "결석이 10번 이상이 되면 출결에 문제가 생겨 재적처리가 될 수 있어요. 출석 관리에 조금 더 신경 써 주세요!" | ||
| } else { | ||
| "" | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
A: get을 쓰지 않더라도 매번 AttendHistoryState는 copy로 새로 생성되기 때문에 get이 없어도 될거에요
| internal class AttendHistoryUseCase @Inject constructor( | ||
| private val attendanceRepository: AttendanceRepository, | ||
| private val sessionRepository: SessionRepository | ||
| ){ | ||
| operator fun invoke() = combine( | ||
| attendanceRepository.getAttendanceStatistics(), | ||
| sessionRepository.getSessions() | ||
| ) { attendStatistics, sessions -> | ||
| attendStatistics to sessions | ||
| } | ||
| } |
There was a problem hiding this comment.
A:
combine 좋네요.
getAttendanceStatistics(), getSessions()가 모두 flow로 되어있던데 일반 suspend 함수로 바꾼 후 UseCase에서 flowOf로 만들어서 처리하는건 어떠세요?
Repository 단에서는 최대한 기본 suspend 함수로 처리하면 좋을 것 같아요.
만약 getAttendanceStatistics()만 별도로 사용하게 되는 경우, flow는 오버헤드가 있을 것 같습니다.
@TaeseongYun 이미 병합된거지만 리마인드차 멘션 드려요 |
2 participants
💡 Issue
🌱 Key changes
✅ To Reviewers
각 API 를 연결 했지만 현재 500, 403 에러가 발생하고 있는 상황입니다.
빠른 확인을 위하여 먼저 PR 을 올렸습니다.
📸 스크린샷
Screen_recording_20250418_104628.mp4