Merged
Conversation
워크스루기존 에러 다이얼로그 시스템(ErrorDialogSpec, ErrorEventHelper)을 새로운 일반 목적의 다이얼로그 시스템(DialogSpec, EventHelper)으로 마이그레이션합니다. 로그인 필수 화면에서 토스트 대신 다이얼로그를 표시하고, 새로운 이벤트 기반 아키텍처로 다이얼로그 처리를 통합합니다. 변경사항
시퀀스 다이어그램sequenceDiagram
participant UI as UI Layer
participant Presenter
participant DialogEvents
participant EventHelper
participant MainActivity
UI->>Presenter: 액션 트리거 (로그인 필요)
Presenter->>DialogEvents: postLoginRequiredDialog(onConfirm)
DialogEvents->>DialogEvents: DialogSpec 생성<br/>(로그인 필수 메시지)
DialogEvents->>EventHelper: sendEvent(ReedEvent.ShowDialog)
EventHelper->>MainActivity: eventFlow 발행
MainActivity->>MainActivity: dialogSpec 상태 업데이트
MainActivity->>UI: ReedDialog 렌더링
UI->>UI: 사용자 확인 클릭
UI->>MainActivity: onConfirm 콜백
MainActivity->>Presenter: 로그인 화면 네비게이션
sequenceDiagram
participant Presenter
participant DialogEvents
participant EventHelper
participant MainActivity as MainActivity<br/>(다이얼로그 렌더링)
Presenter->>DialogEvents: postErrorDialog(errorScope, exception, confirmLabel, onConfirm)
DialogEvents->>DialogEvents: 예외 분석<br/>(네트워크/HTTP 오류)
DialogEvents->>DialogEvents: DialogSpec 구성<br/>(title, message)
DialogEvents->>EventHelper: sendEvent(ReedEvent.ShowDialog)
EventHelper->>MainActivity: eventFlow 발행
MainActivity->>MainActivity: spec 상태 저장
MainActivity->>MainActivity: ReedDialog 표시
activate MainActivity
note right of MainActivity: 사용자 상호작용 대기
MainActivity->>MainActivity: onConfirm() 실행
deactivate MainActivity
예상 코드 리뷰 노력🎯 4 (복잡) | ⏱️ ~60분 주의 깊게 검토해야 할 영역:
시
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used🧠 Learnings (2)📓 Common learnings📚 Learning: 2025-07-09T01:14:29.836ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (3)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
core/common/src/main/kotlin/com/ninecraft/booket/core/common/event/EventHelper.kt (1)
8-15: 채널 생명주기 관리를 고려하세요.싱글톤 객체에서
Channel을 사용하고 있지만 닫히지 않아 잠재적인 리소스 누수가 발생할 수 있습니다. 테스트나 앱 종료 시 채널을 정리할 방법이 없습니다.이벤트 브로드캐스팅 패턴에 더 적합한
SharedFlow를 사용하는 것을 고려해보세요:-import kotlinx.coroutines.channels.Channel -import kotlinx.coroutines.flow.receiveAsFlow +import kotlinx.coroutines.flow.MutableSharedFlow +import kotlinx.coroutines.flow.asSharedFlow import java.util.UUID object EventHelper { - private val _eventFlow = Channel<ReedEvent>(Channel.BUFFERED) - val eventFlow = _eventFlow.receiveAsFlow() + private val _eventFlow = MutableSharedFlow<ReedEvent>( + replay = 0, + extraBufferCapacity = 64 + ) + val eventFlow = _eventFlow.asSharedFlow() - fun sendEvent(event: ReedEvent) { - _eventFlow.trySend(event) + suspend fun sendEvent(event: ReedEvent) { + _eventFlow.emit(event) } }
SharedFlow는 여러 구독자를 지원하고 리소스 정리가 필요하지 않으며, 이벤트 브로드캐스팅에 더 적합한 의미를 가집니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
core/common/src/main/kotlin/com/ninecraft/booket/core/common/constants/DialogSpec.kt(1 hunks)core/common/src/main/kotlin/com/ninecraft/booket/core/common/constants/ErrorDialogSpec.kt(0 hunks)core/common/src/main/kotlin/com/ninecraft/booket/core/common/event/DialogEvents.kt(1 hunks)core/common/src/main/kotlin/com/ninecraft/booket/core/common/event/ErrorEventHelper.kt(0 hunks)core/common/src/main/kotlin/com/ninecraft/booket/core/common/event/EventHelper.kt(1 hunks)core/common/src/main/kotlin/com/ninecraft/booket/core/common/utils/HandleException.kt(2 hunks)feature/library/src/main/kotlin/com/ninecraft/booket/feature/library/LibraryPresenter.kt(2 hunks)feature/login/src/main/kotlin/com/ninecraft/booket/feature/login/LoginPresenter.kt(1 hunks)feature/main/src/main/kotlin/com/ninecraft/booket/feature/main/MainActivity.kt(3 hunks)feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchPresenter.kt(2 hunks)feature/splash/src/main/kotlin/com/ninecraft/booket/splash/SplashPresenter.kt(2 hunks)
💤 Files with no reviewable changes (2)
- core/common/src/main/kotlin/com/ninecraft/booket/core/common/event/ErrorEventHelper.kt
- core/common/src/main/kotlin/com/ninecraft/booket/core/common/constants/ErrorDialogSpec.kt
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: easyhooon
Repo: YAPP-Github/Reed-Android PR: 174
File: feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchPresenter.kt:128-133
Timestamp: 2025-08-28T12:25:54.058Z
Learning: In BookSearchPresenter.kt, when a guest user tries to register a book and is redirected to login, the bottom sheet (isBookRegisterBottomSheetVisible) and selection state (selectedBookIsbn, selectedBookStatus) are intentionally kept open/preserved so that when the user returns from login, they can continue from where they left off without losing context.
Learnt from: seoyoon513
Repo: YAPP-Github/Reed-Android PR: 46
File: feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/component/InfiniteLazyColumn.kt:83-95
Timestamp: 2025-07-14T00:46:03.843Z
Learning: seoyoon513과 팀은 한국어 주석을 선호하며, 한국어 주석을 영어로 번역하라는 제안을 하지 않아야 함
📚 Learning: 2025-08-28T12:25:54.058Z
Learnt from: easyhooon
Repo: YAPP-Github/Reed-Android PR: 174
File: feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchPresenter.kt:128-133
Timestamp: 2025-08-28T12:25:54.058Z
Learning: In BookSearchPresenter.kt, when a guest user tries to register a book and is redirected to login, the bottom sheet (isBookRegisterBottomSheetVisible) and selection state (selectedBookIsbn, selectedBookStatus) are intentionally kept open/preserved so that when the user returns from login, they can continue from where they left off without losing context.
Applied to files:
feature/login/src/main/kotlin/com/ninecraft/booket/feature/login/LoginPresenter.ktfeature/library/src/main/kotlin/com/ninecraft/booket/feature/library/LibraryPresenter.ktfeature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchPresenter.ktfeature/splash/src/main/kotlin/com/ninecraft/booket/splash/SplashPresenter.kt
📚 Learning: 2025-07-09T01:14:29.836Z
Learnt from: seoyoon513
Repo: YAPP-Github/Reed-Android PR: 35
File: feature/login/src/main/kotlin/com/ninecraft/booket/feature/login/TermsAgreementScreen.kt:127-127
Timestamp: 2025-07-09T01:14:29.836Z
Learning: In the Reed-Android project's TermsAgreementScreen.kt, the OnTermDetailClick event is intentionally passed an empty string for the URL parameter because the actual URLs for terms detail pages haven't been decided yet. This is a temporary implementation that will be updated once the URLs are finalized.
Applied to files:
feature/main/src/main/kotlin/com/ninecraft/booket/feature/main/MainActivity.ktfeature/splash/src/main/kotlin/com/ninecraft/booket/splash/SplashPresenter.kt
📚 Learning: 2025-07-16T15:54:19.322Z
Learnt from: easyhooon
Repo: YAPP-Github/Reed-Android PR: 52
File: feature/main/src/main/kotlin/com/ninecraft/booket/feature/main/screens/DelegatingNavigator.kt:0-0
Timestamp: 2025-07-16T15:54:19.322Z
Learning: In the Reed-Android project using Circuit architecture, DelegatingNavigator was restored because NavigableCircuitContent can only inject a single navigator, but the dual-navigator architecture (childNavigator for bottom navigation screens, rootNavigator for full-screen screens) requires a delegating component to work within Circuit's constraint.
Applied to files:
feature/main/src/main/kotlin/com/ninecraft/booket/feature/main/MainActivity.kt
🧬 Code graph analysis (2)
feature/library/src/main/kotlin/com/ninecraft/booket/feature/library/LibraryPresenter.kt (1)
core/common/src/main/kotlin/com/ninecraft/booket/core/common/event/DialogEvents.kt (1)
postLoginRequiredDialog(50-59)
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchPresenter.kt (1)
core/common/src/main/kotlin/com/ninecraft/booket/core/common/event/DialogEvents.kt (1)
postLoginRequiredDialog(50-59)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: ci-build
- GitHub Check: Compose Stability Check
🔇 Additional comments (9)
feature/login/src/main/kotlin/com/ninecraft/booket/feature/login/LoginPresenter.kt (1)
10-10: LGTM - 임포트 경로 리팩토링이 적절합니다.
postErrorDialog함수의 임포트 경로가utils패키지에서event패키지로 변경되어 다이얼로그 이벤트의 중앙 관리를 지원합니다. 함수 사용법은 동일하게 유지되었습니다.feature/library/src/main/kotlin/com/ninecraft/booket/feature/library/LibraryPresenter.kt (1)
129-135: LGTM - 로그인 필요 다이얼로그가 올바르게 구현되었습니다.게스트 사용자가 내 서재 검색 기능에 진입할 때, 이전의 토스트 메시지와 직접 네비게이션 방식 대신
postLoginRequiredDialog를 사용하여 사용자에게 로그인 안내 다이얼로그를 먼저 보여줍니다. 사용자가 확인 버튼을 클릭하면 로그인 화면으로 이동하는 흐름이 PR 목표와 일치합니다.feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchPresenter.kt (1)
123-132: LGTM - 상태 보존이 올바르게 유지되고 있습니다.게스트 사용자가 도서 등록을 시도할 때, 사전 가드 패턴을 사용하여 로그인 다이얼로그를 먼저 표시하고 early return합니다. 중요한 점은
isBookRegisterBottomSheetVisible,selectedBookIsbn,selectedBookStatus상태가 초기화되지 않아서, 사용자가 로그인 후 돌아왔을 때 이전 작업을 이어갈 수 있도록 구현되어 있습니다.Based on learnings
core/common/src/main/kotlin/com/ninecraft/booket/core/common/constants/DialogSpec.kt (1)
3-10: LGTM - 다이얼로그 설정을 위한 깔끔한 데이터 클래스입니다.
DialogSpec은 다이얼로그의 메시지, 버튼 레이블, 콜백 등을 설정하기 위한 명확한 구조를 제공합니다. 필수 속성과 선택적 속성이 적절히 구분되어 있으며,onConfirm과onDismissRequest를() -> Unit타입으로 정의하여 호출자가 필요에 따라 코루틴을 직접 처리할 수 있는 유연성을 제공합니다.feature/main/src/main/kotlin/com/ninecraft/booket/feature/main/MainActivity.kt (1)
15-17: LGTM - 다이얼로그 시스템 리팩토링이 일관되게 적용되었습니다.
ErrorDialogSpec/ErrorEventHelper에서DialogSpec/EventHelper/ReedEvent로의 전환이 잘 이루어졌습니다. 이벤트 수집, 상태 관리, 다이얼로그 렌더링이 모두 새로운 추상화에 맞게 업데이트되었으며, API 변경 사항이 일관성 있게 적용되었습니다.Also applies to: 57-67, 74-77
core/common/src/main/kotlin/com/ninecraft/booket/core/common/event/DialogEvents.kt (2)
8-48: LGTM - 에러 다이얼로그 로직이 잘 중앙화되었습니다.
postErrorDialog함수는 네트워크 에러, HTTP 예외, ErrorScope에 따라 적절한 타이틀과 메시지를 매핑하여 사용자 친화적인 에러 다이얼로그를 표시합니다. 예외 처리 로직이 명확하고 포괄적입니다.
50-59: LGTM - 로그인 필요 다이얼로그가 사용자 친화적으로 구현되었습니다.
postLoginRequiredDialog함수는 명확한 한국어 메시지와 함께 "로그인 하기" 확인 버튼과 "닫기" 취소 버튼을 제공하여 사용자가 로그인 필요 상황을 쉽게 이해하고 선택할 수 있도록 합니다.core/common/src/main/kotlin/com/ninecraft/booket/core/common/utils/HandleException.kt (1)
4-4: LGTM - 새로운 다이얼로그 API와 일관되게 업데이트되었습니다.
postErrorDialog임포트 경로가event패키지로 변경되었고, 파라미터 이름이action에서onConfirm으로 변경되어 콜백의 의미가 더 명확해졌습니다. 에러 핸들링 로직은 변경 없이 유지됩니다.Also applies to: 25-25
feature/splash/src/main/kotlin/com/ninecraft/booket/splash/SplashPresenter.kt (1)
11-11: LGTM - 새로운 다이얼로그 API로 올바르게 마이그레이션되었습니다.
postErrorDialog호출이 새로운 API 시그니처에 맞게 업데이트되었습니다.buttonLabelResId와action파라미터가confirmLabel과onConfirm으로 변경되었으며, 리소스 ID 대신 직접 문자열을 사용하는 것은 새로운 API 설계에 따른 의도된 변경입니다.Also applies to: 66-67
Comment on lines
+12
to
+14
| fun sendEvent(event: ReedEvent) { | ||
| _eventFlow.trySend(event) | ||
| } |
There was a problem hiding this comment.
이벤트 전송 실패 처리가 누락되었습니다.
trySend()는 채널이 닫혔거나 버퍼가 가득 찬 경우 실패할 수 있지만, 반환된 결과를 확인하지 않아 이벤트가 조용히 손실될 수 있습니다. 로그인 요청 다이얼로그와 같은 중요한 이벤트가 사용자에게 표시되지 않을 수 있습니다.
다음 diff를 적용하여 실패한 전송을 처리하세요:
fun sendEvent(event: ReedEvent) {
- _eventFlow.trySend(event)
+ _eventFlow.trySend(event).onFailure {
+ // 실패 시 로깅 또는 대체 처리
+ android.util.Log.e("EventHelper", "Failed to send event: $event", it)
+ }
}또는 suspend 함수로 변경하여 확실한 전송을 보장할 수도 있습니다:
-fun sendEvent(event: ReedEvent) {
- _eventFlow.trySend(event)
+suspend fun sendEvent(event: ReedEvent) {
+ _eventFlow.send(event)
}🤖 Prompt for AI Agents
In
core/common/src/main/kotlin/com/ninecraft/booket/core/common/event/EventHelper.kt
around lines 12 to 14, the call to _eventFlow.trySend(event) ignores the
SendResult so events can be silently lost; update the function to handle
failures by checking the TrySendResult and reacting (e.g., if result.isSuccess
do nothing, else log an error and/or enqueue the event, throw or notify caller)
or convert sendEvent to a suspend function and use _eventFlow.emit(event) (or
_eventFlow.send(event)) to guarantee delivery and propagate exceptions.
easyhooon
reviewed
Nov 5, 2025
easyhooon
approved these changes
Nov 5, 2025
Contributor
easyhooon
left a comment
easyhooon
left a comment
There was a problem hiding this comment.
UUID 필요하면 남기고, 없어도 Channel 기반 이벤트 처리에 문제가 없으면 지우고 merge 하면 될 것 같습니다! 수고하셨습니다~
Contributor
|
게스트모드에서 로그인 화면으로 이동은 사실 error 상황은 아니고 자연스러운 플로우라 errorEvent로 처리하는게 애매했군요. 범용적인 네이밍으로 변경한거 좋은 것 같습니다! |
Contributor
Author
|
@easyhooon |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🔗 관련 이슈
📙 작업 설명
(
ErrorEventHelper,ErrorDialogSpec->EventHelper,DialogSpec)DialogEvents.kt파일에서 관리하도록 수정했습니다(토스트 & 로그인 화면 이동 -> 로그인 요청 다이얼로그를 통한 로그인 화면 이동)
🧪 테스트 내역
📸 스크린샷 또는 시연 영상
✅ 책 등록
Reed_._.mp4
✅ 내 서재 검색
Reed_._.mp4
💬 추가 설명 or 리뷰 포인트
Summary by CodeRabbit
릴리스 노트
새 기능
개선