Phase 2: Single Activity로 전환#714
Open
l2hyunwoo wants to merge 16 commits into
Open
Conversation
Contributor
Walkthrough여러 Activity를 제거하고 HostActivity를 런처로 통합하며, 상태형 중앙 네비게이터( Changes
Sequence Diagram(s)sequenceDiagram
participant FCM as Firebase/MessagingService
participant Work as WorkManager/Worker
participant IntentFactory as DeepLinkIntentFactory
participant System as OS (startActivity)
participant Host as HostActivity
participant Nav as Navigator
participant Entry as EntryBuilder/Registry
participant Screen as Composable Screen
FCM->>IntentFactory: createMainIntent(...) / createNoticeWebIntent(...)
Work->>IntentFactory: createMainIntent(...)
IntentFactory->>System: startActivity(Intent targeting HostActivity)
System->>Host: onCreate(intent) / onNewIntent(intent)
Host->>Nav: setPendingDeepLink(key) (onCreate) / navigate(key) (onNewIntent)
Nav->>Nav: backStack 업데이트 (add / replace)
Host->>Entry: resolve EntryBuilder for backStack.last()
Entry->>Screen: compose UI for key
Screen->>Nav: navigate(otherKey) / goBack()
Nav->>Nav: backStack 변경 (add/remove)
Host->>Entry: resolve updated EntryBuilder
Entry->>Screen: compose new UI
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
l2hyunwoo
force-pushed
the
feature/nav3-entry-builder-pattern
branch
from
125c671 to
929b9f6
Compare
l2hyunwoo
force-pushed
the
feature/nav-3-single-activity
branch
from
4e8ac75 to
535c816
Compare
Base automatically changed from
feature/nav3-entry-builder-pattern
to
feature/nav-3-base
March 29, 2026 13:02
l2hyunwoo
force-pushed
the
feature/nav-3-single-activity
branch
from
535c816 to
95e893c
Compare
NavKey 기반 네비게이션을 Activity 생명주기에서 분리하기 위한 추상화 인터페이스 추가
SplashActivity를 삭제하고 SplashEntryBuilder로 전환
OnboardingActivity를 삭제하고 OnboardingEntryBuilder로 전환
AuthActivity를 삭제하고 AuthFlowEntryBuilder로 전환
MainActivity, ArchiveActivity, SearchActivity, OpenSourceActivity를 삭제하고 MainHubEntryBuilder, ArchiveEntryBuilder, SearchEntryBuilder, OpenSourceEntryBuilder로 전환
EditDepartments, EditSubscription, Feedback, KuringBot, LibrarySeat, Notification, NoticeWeb, NotionView, Club(Detail/Onboarding/Subscription) Activity를 삭제하고 각각 EntryBuilder로 전환
…t 업데이트 NavigationController 기반으로 KuringNavigatorImpl 전환하고 AndroidManifest에서 삭제된 Activity 참조 정리
Nav3 스타일의 Navigator 클래스를 core/navigation에 추가하고 더 이상 필요 없는 NavigationController 인터페이스를 삭제
Intent 생성 메서드를 KuringNavigator에서 분리하여 DeepLinkIntentFactory 인터페이스로 추출. KuringMessagingService와 ReEngagementNotificationWork가 새 인터페이스를 사용하도록 전환
NavigationController 브릿지를 제거하고 Navigator.backStack을 NavDisplay에 직접 전달. 빈 backStack에서 뒤로가기 시 앱 종료 처리 추가
- LocalNavigator 타입을 KuringNavigator에서 Navigator로 변경 - 모든 navigateTo*(activity) 호출을 navigator.navigate(NavKey)로 전환 - activity?.finish()를 navigator.goBack()으로 교체 - Splash→Onboarding/MainHub, Onboarding→MainHub에서 replaceAll 사용 - OssLicensesMenu 로직을 OpenSourceEntryBuilder 내부로 이동 - feature/auth, club, library에 composeLocals 의존 추가
Navigator 싱글톤으로 완전히 대체되었으므로 레거시 코드 제거
l2hyunwoo
force-pushed
the
feature/nav-3-base
branch
from
45455e9 to
888ff3d
Compare
l2hyunwoo
force-pushed
the
feature/nav-3-single-activity
branch
from
95e893c to
6a73077
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (3)
feature/notice_detail/src/main/java/com/ku_stacks/ku_ring/notice_detail/noticeweb/NoticeWebScreen.kt (1)
242-242:AuthFlowKey("SIGN_IN")문자열 리터럴 대신 상수나 팩토리로 교체하세요.하드코딩된 문자열은 오타 위험이 있으므로, 가능하면 Auth 모듈에서 제공하는 상수나 타입-안전 팩토리 메서드를 사용하여 키 문자열을 중앙에서 관리하기를 권장합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/notice_detail/src/main/java/com/ku_stacks/ku_ring/notice_detail/noticeweb/NoticeWebScreen.kt` at line 242, Replace the hardcoded "SIGN_IN" literal passed to navigator.navigate(AuthFlowKey("SIGN_IN")) with the Auth module's defined constant or type-safe factory for the auth flow key (e.g., use an exported SIGN_IN constant or an AuthFlowKey.create/signIn factory); update the call to navigator.navigate(...) to use that symbol and add the appropriate import for the Auth module so the key is centrally managed and typo-safe.app/src/main/java/com/ku_stacks/ku_ring/HostActivity.kt (1)
86-95: route extra 키는 공용 계약으로 빼두는 편이 좋습니다.지금은
HostActivity와DeepLinkIntentFactoryImpl가"MAIN_SCREEN_ROUTE"를 각각 들고 있어서 한쪽만 바뀌어도 메인 탭 딥링크가 조용히 깨집니다. navigation 공용 상수/contract로 올려 두 파일이 같은 심볼을 참조하게 만드는 편이 안전합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/ku_stacks/ku_ring/HostActivity.kt` around lines 86 - 95, HostActivity currently defines INTENT_KEY_ROUTE = "MAIN_SCREEN_ROUTE" while DeepLinkIntentFactoryImpl duplicates the same literal; extract that string into a single shared constant (e.g., NavigationContract.MAIN_SCREEN_ROUTE or DeepLinkContract.MAIN_SCREEN_ROUTE) in a common module/class and update HostActivity (remove INTENT_KEY_ROUTE) and DeepLinkIntentFactoryImpl to reference the new shared symbol so both use the same intent extra key when creating/reading the route (see usage points navigator.navigate(MainHubKey(startTab = route)) and any intent creation in DeepLinkIntentFactoryImpl).feature/main/src/main/java/com/ku_stacks/ku_ring/main/MainScreen.kt (1)
155-159: 공지 상세 매핑은 Notice 쪽으로 내려주세요.
toWebViewNotice()와NoticeWebKey생성까지 여기서 처리하면MainScreen이 notice 도메인 모델을 직접 알게 됩니다. 이 파일은 탭 컨테이너 역할만 유지하고, 공지 전용 변환은 notice ViewModel/네비게이션 레이어에서 완성된 목적지만 넘기도록 분리하는 편이 안전합니다.As per coding guidelines "Keep MainScreen logic tab-agnostic; move sub-feature business logic to respective sub-ViewModels".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/main/src/main/java/com/ku_stacks/ku_ring/main/MainScreen.kt` around lines 155 - 159, MainScreen currently converts notice models and builds navigation keys in onNoticeClick (calls toWebViewNotice() and constructs NoticeWebKey), which leaks notice-domain logic into the tab container; move the mapping and NoticeWebKey creation into the notice feature (e.g., the Notice ViewModel or a notice-specific navigator helper). Replace the onNoticeClick handler in MainScreen to simply forward the raw notice item or an ID via a single callback (e.g., onNoticeSelected(id or model)), then implement the conversion toWebViewNotice() and NoticeWebKey construction inside the NoticeViewModel or notice navigation layer and invoke navigator.navigate(...) there.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/java/com/ku_stacks/ku_ring/HostActivity.kt`:
- Around line 30-35: The current handling of external intents in HostActivity
(onCreate/onNewIntent -> handleIntent) navigates immediately and can bypass the
Splash initialization flow and accumulate deep links on singleTask re-entry;
change handleIntent usage so external intents are deferred until after the
Splash flow completes (i.e., process the intent after SplashKey is resolved) or,
if you must navigate immediately, use Navigator.replaceAll(...) to rebuild the
root stack instead of pushing MainHubKey/NoticeWebKey directly; update logic in
onCreate/onNewIntent/handleIntent to either queue the incoming intent and
consume it after SplashKey finishes, or call
navigator.replaceAll(listOf(SplashKey, /* then target key */)) so the
initialization flow is preserved and deep links do not stack repeatedly.
In `@core/navigation/src/main/java/com/ku_stacks/ku_ring/navigation/Navigator.kt`:
- Around line 18-20: goBack() currently removes the last entry even when the
backStack only contains the root, which can leave the navigation state empty;
update Navigator.goBack to check backStack size and only removeLastOrNull when
backStack.size > 1 (keep at least one root entry), returning true if an item was
removed and false otherwise so the root is never popped.
In
`@domain/navigation/src/main/java/com/ku_stacks/ku_ring/navigation/DeepLinkIntentFactory.kt`:
- Around line 3-16: DeepLinkIntentFactory currently exposes Android types
(Context/Intent) via createMainIntent and createNoticeWebIntent which couples
domain/navigation to the Android framework; change the contract to return
domain-only route/state models (e.g., use MainScreenRoute and a new
WebViewNotice or NoticeRoute data class) and remove any android.* imports from
this file, leaving only pure Kotlin/domain types; then implement the actual
Intent creation and Context usage in the app navigator (implement the routing
logic in app/navigator/KuringNavigatorImpl.kt) so
createMainIntent/createNoticeWebIntent become domain-only methods that produce
route objects and the Android-specific conversion to Intent happens in the app
layer.
In
`@feature/auth/src/main/java/com/ku_stacks/ku_ring/auth/AuthFlowEntryBuilder.kt`:
- Around line 23-27: The code uses a magic string check on key.entryPoint
("SIGN_OUT") in AuthFlowEntryBuilder.kt which is not type-safe; change
AuthFlowKey from a raw String to a sealed class or enum (e.g.,
AuthFlowKey.EntryPoint { SIGN_IN, SIGN_OUT }) so callers pass a typed value,
then replace the if (key.entryPoint == "SIGN_OUT") branching with a when/on enum
branch that returns AuthDestination.SignOut or AuthDestination.SignIn
accordingly; also update any Compose Navigation route definitions to pass the
typed argument (or use nav arguments) so routing is type-safe per
feature/**/*Route.kt guidelines.
In
`@feature/club/src/main/java/com/ku_stacks/ku_ring/club/detail/ClubDetailEntryBuilder.kt`:
- Around line 28-29: The ViewModel (ClubDetailViewModel) is being created with
hiltViewModel() before its clubId is set, so its init load runs with a
wrong/empty ID; move or change initialization so clubId is assigned prior to
triggering load: either pass the clubId into the ViewModel factory/creation
(provide a SavedStateHandle or use a parameterized hilt-assisted injection) or
set clubId immediately on creation and then call an explicit load method (e.g.,
viewModel.loadClub()) instead of relying on init; update ClubDetailEntryBuilder
to ensure clubId is provided to ClubDetailViewModel before any network/load
logic runs.
In
`@feature/club/src/main/java/com/ku_stacks/ku_ring/club/detail/ClubDetailViewModel.kt`:
- Line 28: Make clubId immutable from outside by changing its setter to private
(e.g., var clubId by mutableIntStateOf(...) private set) and add a public setter
method setClubId(newId: Int) that updates clubId (using savedStateHandle if
needed) and then calls loadClub() to trigger re-fetch; reference the existing
clubId property, CLUB_ID_KEY/savedStateHandle for initial value, and the
loadClub() method when implementing setClubId to ensure state changes and
loading stay within the ViewModel’s API and preserve unidirectional data flow.
In
`@feature/onboarding/src/main/java/com/ku_stacks/ku_ring/onboarding/OnboardingEntryBuilder.kt`:
- Around line 32-35: 분석 이벤트 호출 analytics.click에서 전달하는 screenClass 값이 현재 UI 구조와
일치하지 않습니다; analytics.click(...) 호출의 screenClass 매개변수를 화면 단위 실제 이름(예:
"OnboardingScreen" 또는 해당 Compose 엔트리의 실제 이름)으로 변경해 주세요 so that the analytics
classification matches the EntryBuilder/Compose screen; update the string in the
analytics.click call (refer to the analytics.click invocation around the
OnboardingEntryBuilder) to the correct screen unit name.
---
Nitpick comments:
In `@app/src/main/java/com/ku_stacks/ku_ring/HostActivity.kt`:
- Around line 86-95: HostActivity currently defines INTENT_KEY_ROUTE =
"MAIN_SCREEN_ROUTE" while DeepLinkIntentFactoryImpl duplicates the same literal;
extract that string into a single shared constant (e.g.,
NavigationContract.MAIN_SCREEN_ROUTE or DeepLinkContract.MAIN_SCREEN_ROUTE) in a
common module/class and update HostActivity (remove INTENT_KEY_ROUTE) and
DeepLinkIntentFactoryImpl to reference the new shared symbol so both use the
same intent extra key when creating/reading the route (see usage points
navigator.navigate(MainHubKey(startTab = route)) and any intent creation in
DeepLinkIntentFactoryImpl).
In `@feature/main/src/main/java/com/ku_stacks/ku_ring/main/MainScreen.kt`:
- Around line 155-159: MainScreen currently converts notice models and builds
navigation keys in onNoticeClick (calls toWebViewNotice() and constructs
NoticeWebKey), which leaks notice-domain logic into the tab container; move the
mapping and NoticeWebKey creation into the notice feature (e.g., the Notice
ViewModel or a notice-specific navigator helper). Replace the onNoticeClick
handler in MainScreen to simply forward the raw notice item or an ID via a
single callback (e.g., onNoticeSelected(id or model)), then implement the
conversion toWebViewNotice() and NoticeWebKey construction inside the
NoticeViewModel or notice navigation layer and invoke navigator.navigate(...)
there.
In
`@feature/notice_detail/src/main/java/com/ku_stacks/ku_ring/notice_detail/noticeweb/NoticeWebScreen.kt`:
- Line 242: Replace the hardcoded "SIGN_IN" literal passed to
navigator.navigate(AuthFlowKey("SIGN_IN")) with the Auth module's defined
constant or type-safe factory for the auth flow key (e.g., use an exported
SIGN_IN constant or an AuthFlowKey.create/signIn factory); update the call to
navigator.navigate(...) to use that symbol and add the appropriate import for
the Auth module so the key is centrally managed and typo-safe.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 948a7223-596d-4daa-bcf8-b4ab50aef4b4
📒 Files selected for processing (58)
app/src/main/AndroidManifest.xmlapp/src/main/java/com/ku_stacks/ku_ring/HostActivity.ktapp/src/main/java/com/ku_stacks/ku_ring/di/NavigatorModule.ktapp/src/main/java/com/ku_stacks/ku_ring/navigator/DeepLinkIntentFactoryImpl.ktapp/src/main/java/com/ku_stacks/ku_ring/navigator/KuringNavigatorImpl.ktcore/compose-locals/build.gradle.ktscore/compose-locals/src/main/java/com/ku_stacks/ku_ring/compose/locals/di/CompositionLocalEntryPoint.ktcore/firebase-messaging/src/main/java/com/ku_stacks/ku_ring/firebase/messaging/KuringMessagingService.ktcore/navigation/src/main/java/com/ku_stacks/ku_ring/navigation/Navigator.ktcore/work/src/main/java/com/ku_stacks/ku_ring/work/ReEngagementNotificationWork.ktdomain/navigation/src/main/java/com/ku_stacks/ku_ring/navigation/DeepLinkIntentFactory.ktdomain/navigation/src/main/java/com/ku_stacks/ku_ring/navigation/KuringNavigator.ktfeature/auth/build.gradle.ktsfeature/auth/src/main/java/com/ku_stacks/ku_ring/auth/AuthActivity.ktfeature/auth/src/main/java/com/ku_stacks/ku_ring/auth/AuthFlowEntryBuilder.ktfeature/club/build.gradle.ktsfeature/club/src/main/java/com/ku_stacks/ku_ring/club/detail/ClubDetailActivity.ktfeature/club/src/main/java/com/ku_stacks/ku_ring/club/detail/ClubDetailEntryBuilder.ktfeature/club/src/main/java/com/ku_stacks/ku_ring/club/detail/ClubDetailViewModel.ktfeature/club/src/main/java/com/ku_stacks/ku_ring/club/onboarding/ClubOnboardingActivity.ktfeature/club/src/main/java/com/ku_stacks/ku_ring/club/onboarding/ClubOnboardingEntryBuilder.ktfeature/club/src/main/java/com/ku_stacks/ku_ring/club/subscription/ClubSubscriptionActivity.ktfeature/club/src/main/java/com/ku_stacks/ku_ring/club/subscription/ClubSubscriptionEntryBuilder.ktfeature/edit_departments/src/main/java/com/ku_stacks/ku_ring/edit_departments/EditDepartmentsActivity.ktfeature/edit_departments/src/main/java/com/ku_stacks/ku_ring/edit_departments/EditDepartmentsEntryBuilder.ktfeature/edit_subscription/src/main/java/com/ku_stacks/ku_ring/edit_subscription/EditSubscriptionActivity.ktfeature/edit_subscription/src/main/java/com/ku_stacks/ku_ring/edit_subscription/EditSubscriptionEntryBuilder.ktfeature/feedback/src/main/java/com/ku_stacks/ku_ring/feedback/feedback/FeedbackActivity.ktfeature/feedback/src/main/java/com/ku_stacks/ku_ring/feedback/feedback/FeedbackEntryBuilder.ktfeature/kuringbot/src/main/java/com/ku_stacks/ku_ring/kuringbot/KuringBotActivity.ktfeature/kuringbot/src/main/java/com/ku_stacks/ku_ring/kuringbot/KuringBotEntryBuilder.ktfeature/library/build.gradle.ktsfeature/library/src/main/java/com/ku_stacks/ku_ring/library/LibrarySeatActivity.ktfeature/library/src/main/java/com/ku_stacks/ku_ring/library/LibrarySeatEntryBuilder.ktfeature/main/build.gradle.ktsfeature/main/src/main/java/com/ku_stacks/ku_ring/main/MainActivity.ktfeature/main/src/main/java/com/ku_stacks/ku_ring/main/MainHubEntryBuilder.ktfeature/main/src/main/java/com/ku_stacks/ku_ring/main/MainScreen.ktfeature/main/src/main/java/com/ku_stacks/ku_ring/main/archive/ArchiveActivity.ktfeature/main/src/main/java/com/ku_stacks/ku_ring/main/archive/ArchiveEntryBuilder.ktfeature/main/src/main/java/com/ku_stacks/ku_ring/main/archive/compose/ArchiveScreen.ktfeature/main/src/main/java/com/ku_stacks/ku_ring/main/notice/compose/NoticeScreen.ktfeature/main/src/main/java/com/ku_stacks/ku_ring/main/search/SearchActivity.ktfeature/main/src/main/java/com/ku_stacks/ku_ring/main/search/SearchEntryBuilder.ktfeature/main/src/main/java/com/ku_stacks/ku_ring/main/setting/compose/OpenSourceActivity.ktfeature/main/src/main/java/com/ku_stacks/ku_ring/main/setting/compose/OpenSourceEntryBuilder.ktfeature/notice_detail/src/main/java/com/ku_stacks/ku_ring/notice_detail/NoticeWebActivity.ktfeature/notice_detail/src/main/java/com/ku_stacks/ku_ring/notice_detail/NoticeWebEntryBuilder.ktfeature/notice_detail/src/main/java/com/ku_stacks/ku_ring/notice_detail/noticeweb/NoticeWebScreen.ktfeature/notification/src/main/java/com/ku_stacks/ku_ring/notification/NotificationActivity.ktfeature/notification/src/main/java/com/ku_stacks/ku_ring/notification/NotificationEntryBuilder.ktfeature/notion/src/main/java/com/ku_stacks/ku_ring/notion/NotionViewActivity.ktfeature/onboarding/build.gradle.ktsfeature/onboarding/src/main/java/com/ku_stacks/ku_ring/onboarding/OnboardingActivity.ktfeature/onboarding/src/main/java/com/ku_stacks/ku_ring/onboarding/OnboardingEntryBuilder.ktfeature/splash/build.gradle.ktsfeature/splash/src/main/java/com/ku_stacks/ku_ring/splash/SplashActivity.ktfeature/splash/src/main/java/com/ku_stacks/ku_ring/splash/SplashEntryBuilder.kt
💤 Files with no reviewable changes (20)
- feature/library/src/main/java/com/ku_stacks/ku_ring/library/LibrarySeatActivity.kt
- feature/club/src/main/java/com/ku_stacks/ku_ring/club/subscription/ClubSubscriptionActivity.kt
- feature/edit_subscription/src/main/java/com/ku_stacks/ku_ring/edit_subscription/EditSubscriptionActivity.kt
- feature/kuringbot/src/main/java/com/ku_stacks/ku_ring/kuringbot/KuringBotActivity.kt
- feature/club/src/main/java/com/ku_stacks/ku_ring/club/onboarding/ClubOnboardingActivity.kt
- feature/feedback/src/main/java/com/ku_stacks/ku_ring/feedback/feedback/FeedbackActivity.kt
- feature/notification/src/main/java/com/ku_stacks/ku_ring/notification/NotificationActivity.kt
- feature/notion/src/main/java/com/ku_stacks/ku_ring/notion/NotionViewActivity.kt
- feature/edit_departments/src/main/java/com/ku_stacks/ku_ring/edit_departments/EditDepartmentsActivity.kt
- feature/main/src/main/java/com/ku_stacks/ku_ring/main/setting/compose/OpenSourceActivity.kt
- feature/club/src/main/java/com/ku_stacks/ku_ring/club/detail/ClubDetailActivity.kt
- feature/onboarding/src/main/java/com/ku_stacks/ku_ring/onboarding/OnboardingActivity.kt
- feature/notice_detail/src/main/java/com/ku_stacks/ku_ring/notice_detail/NoticeWebActivity.kt
- feature/auth/src/main/java/com/ku_stacks/ku_ring/auth/AuthActivity.kt
- feature/main/src/main/java/com/ku_stacks/ku_ring/main/archive/ArchiveActivity.kt
- feature/main/src/main/java/com/ku_stacks/ku_ring/main/search/SearchActivity.kt
- feature/splash/src/main/java/com/ku_stacks/ku_ring/splash/SplashActivity.kt
- feature/main/src/main/java/com/ku_stacks/ku_ring/main/MainActivity.kt
- domain/navigation/src/main/java/com/ku_stacks/ku_ring/navigation/KuringNavigator.kt
- app/src/main/java/com/ku_stacks/ku_ring/navigator/KuringNavigatorImpl.kt
Comment on lines
+3
to
+16
| import android.content.Context | ||
| import android.content.Intent | ||
| import com.ku_stacks.ku_ring.domain.WebViewNotice | ||
|
|
||
| interface DeepLinkIntentFactory { | ||
| fun createMainIntent(context: Context, route: MainScreenRoute = MainScreenRoute.Notice): Intent | ||
| fun createNoticeWebIntent( | ||
| context: Context, | ||
| url: String?, | ||
| articleId: String?, | ||
| id: Int?, | ||
| category: String?, | ||
| subject: String?, | ||
| ): Intent |
Contributor
There was a problem hiding this comment.
도메인 모듈에 Android 타입 의존이 들어와 아키텍처 경계를 깨고 있습니다.
DeepLinkIntentFactory가 Context/Intent를 직접 노출해 domain/navigation이 Android 프레임워크에 결합됩니다. 이 계약은 app 네비게이터 계층으로 이동하고, 도메인에는 route/state 모델만 남기는 구조로 분리해 주세요.
As per coding guidelines domain/**/src/main/java/**/*.kt: Domain module Kotlin files must not import Android Framework classes (android.* or androidx.*), and domain/navigation/**/*.kt: Define navigation state in domain:navigation module and implement navigation routing in app/navigator/KuringNavigatorImpl.kt.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@domain/navigation/src/main/java/com/ku_stacks/ku_ring/navigation/DeepLinkIntentFactory.kt`
around lines 3 - 16, DeepLinkIntentFactory currently exposes Android types
(Context/Intent) via createMainIntent and createNoticeWebIntent which couples
domain/navigation to the Android framework; change the contract to return
domain-only route/state models (e.g., use MainScreenRoute and a new
WebViewNotice or NoticeRoute data class) and remove any android.* imports from
this file, leaving only pure Kotlin/domain types; then implement the actual
Intent creation and Context usage in the app navigator (implement the routing
logic in app/navigator/KuringNavigatorImpl.kt) so
createMainIntent/createNoticeWebIntent become domain-only methods that produce
route objects and the Android-specific conversion to Intent happens in the app
layer.
Comment on lines
+28
to
+29
| val viewModel = hiltViewModel<ClubDetailViewModel>() | ||
| viewModel.clubId = key.clubId |
Contributor
There was a problem hiding this comment.
초기 로딩이 잘못된 clubId로 실행될 수 있습니다.
Line 28에서 ViewModel이 생성될 때 이미 init 로딩이 수행되는데, Line 29 대입은 그 이후라서 첫 요청이 잘못된 ID로 나가고 재조회도 자동으로 일어나지 않습니다. 현재 구조는 상세 데이터 로딩 실패로 이어질 수 있습니다.
수정 방향 예시
+import androidx.compose.runtime.LaunchedEffect
...
val viewModel = hiltViewModel<ClubDetailViewModel>()
- viewModel.clubId = key.clubId
+ LaunchedEffect(key.clubId) {
+ viewModel.setClubId(key.clubId)
+ }// ClubDetailViewModel.kt
- var clubId by mutableIntStateOf(savedStateHandle.get<Int>(CLUB_ID_KEY) ?: -1)
+ var clubId by mutableIntStateOf(savedStateHandle.get<Int>(CLUB_ID_KEY) ?: -1)
+ private set
+
+ fun setClubId(newClubId: Int) {
+ if (clubId == newClubId) return
+ clubId = newClubId
+ loadClub()
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@feature/club/src/main/java/com/ku_stacks/ku_ring/club/detail/ClubDetailEntryBuilder.kt`
around lines 28 - 29, The ViewModel (ClubDetailViewModel) is being created with
hiltViewModel() before its clubId is set, so its init load runs with a
wrong/empty ID; move or change initialization so clubId is assigned prior to
triggering load: either pass the clubId into the ViewModel factory/creation
(provide a SavedStateHandle or use a parameterized hilt-assisted injection) or
set clubId immediately on creation and then call an explicit load method (e.g.,
viewModel.loadClub()) instead of relying on init; update ClubDetailEntryBuilder
to ensure clubId is provided to ClubDetailViewModel before any network/load
logic runs.
Collaborator
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
app/src/main/java/com/ku_stacks/ku_ring/HostActivity.kt (1)
35-37:⚠️ Potential issue | 🟠 Major외부 인텐트를 즉시
navigate()하면 스플래시 초기화 우회 + 스택 누적 문제가 계속 남습니다.Line 35/Line 63에서 즉시 처리된 딥링크가 Line 74/Line 89의
navigate로 바로 push되어, cold start에서는 초기 플로우를 건너뛰고 singleTask 재진입에서는 목적지가 계속 쌓일 수 있습니다. 스플래시 완료 후 소비하거나, 최소한 루트 스택 재구성 방식으로 처리해 누적을 막아주세요.Also applies to: 61-64, 73-90
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/ku_stacks/ku_ring/HostActivity.kt` around lines 35 - 37, handleIntent immediately calls navigate on incoming external intents causing splash flow to be skipped and destinations to accumulate; modify HostActivity so handleIntent only records the incoming deep link (e.g., store it in a pendingIntent/deepLink field) instead of calling navigate directly, then after the splash/initial flow completes (in your splash completion callback or onPostSplash method) consume the pending deep link and perform navigation, making sure to reconstruct the root stack (clearBackStack or use a singleTop/singleTask-safe nav method) before pushing the destination to avoid duplicate stacking; update references to handleIntent, navigate, and the splash completion handler to implement this deferred-and-consume behavior.
🧹 Nitpick comments (2)
core/navigation/src/main/java/com/ku_stacks/ku_ring/navigation/keys/AppLifecycleKeys.kt (1)
10-14: enum 직렬화 값은 명시적으로 고정해두는 것을 권장합니다.현재는 enum 이름(
SIGN_IN,SIGN_OUT)이 그대로 직렬화 포맷이 됩니다. 추후 enum 상수명 리네임 시 직렬화 호환성이 깨질 수 있어@SerialName으로 wire format을 고정하는 편이 안전합니다.제안 diff
import androidx.navigation3.runtime.NavKey +import kotlinx.serialization.SerialName import kotlinx.serialization.Serializable `@Serializable` data object SplashKey : NavKey `@Serializable` data object OnboardingKey : NavKey `@Serializable` data class AuthFlowKey(val entryPoint: AuthEntryPoint) : NavKey `@Serializable` enum class AuthEntryPoint { - SIGN_IN, - SIGN_OUT, + `@SerialName`("sign_in") + SIGN_IN, + `@SerialName`("sign_out") + SIGN_OUT, }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/navigation/src/main/java/com/ku_stacks/ku_ring/navigation/keys/AppLifecycleKeys.kt` around lines 10 - 14, The AuthEntryPoint enum's serialized wire values rely on constant names which can break compatibility if constants are renamed; update AuthEntryPoint (keeping the `@Serializable` annotation) to use explicit `@SerialName` annotations on each enum constant (e.g., annotate SIGN_IN and SIGN_OUT with stable string names) so the wire format is fixed and renames won't break serialization; ensure the kotlinx.serialization.SerialName import is added if missing.app/src/main/java/com/ku_stacks/ku_ring/navigator/DeepLinkIntentFactoryImpl.kt (1)
13-14: 하드코딩된HostActivity문자열을 타입 안전한 Intent 생성으로 변경하세요.현재 방식은 리팩터링(패키지/클래스명 변경) 시 런타임 오류로만 드러납니다.
Intent(context, HostActivity::class.java)로 교체하면 컴파일 시점에 검증되어 더 안전합니다.제안 diff
+import com.ku_stacks.ku_ring.HostActivity + class DeepLinkIntentFactoryImpl `@Inject` constructor() : DeepLinkIntentFactory { override fun createMainIntent(context: Context, route: MainScreenRoute): Intent { - return Intent().setClassName(context, "com.ku_stacks.ku_ring.HostActivity").apply { + return Intent(context, HostActivity::class.java).apply { putExtra(DeepLinkIntentFactory.INTENT_KEY_ROUTE, route.route) } } override fun createNoticeWebIntent( context: Context, url: String?, articleId: String?, id: Int?, category: String?, subject: String?, ): Intent { if (url == null || articleId == null || category == null || id == null) { throw IllegalArgumentException("intent parameters shouldn't be null: $url, $articleId, $id, $category") } - return Intent().setClassName(context, "com.ku_stacks.ku_ring.HostActivity").apply { + return Intent(context, HostActivity::class.java).apply { putExtra( WebViewNotice.EXTRA_KEY, WebViewNotice(url, articleId, id, category, subject.orEmpty()), ) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/ku_stacks/ku_ring/navigator/DeepLinkIntentFactoryImpl.kt` around lines 13 - 14, Replace the hardcoded class-name Intent creation in DeepLinkIntentFactoryImpl (currently using Intent().setClassName(context, "com.ku_stacks.ku_ring.HostActivity")) with a type-safe constructor Intent(context, HostActivity::class.java); keep the existing extras (DeepLinkIntentFactory.INTENT_KEY_ROUTE and route.route) and update imports to include the HostActivity class so compilation catches renames/refactors instead of causing runtime errors.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@app/src/main/java/com/ku_stacks/ku_ring/HostActivity.kt`:
- Around line 35-37: handleIntent immediately calls navigate on incoming
external intents causing splash flow to be skipped and destinations to
accumulate; modify HostActivity so handleIntent only records the incoming deep
link (e.g., store it in a pendingIntent/deepLink field) instead of calling
navigate directly, then after the splash/initial flow completes (in your splash
completion callback or onPostSplash method) consume the pending deep link and
perform navigation, making sure to reconstruct the root stack (clearBackStack or
use a singleTop/singleTask-safe nav method) before pushing the destination to
avoid duplicate stacking; update references to handleIntent, navigate, and the
splash completion handler to implement this deferred-and-consume behavior.
---
Nitpick comments:
In
`@app/src/main/java/com/ku_stacks/ku_ring/navigator/DeepLinkIntentFactoryImpl.kt`:
- Around line 13-14: Replace the hardcoded class-name Intent creation in
DeepLinkIntentFactoryImpl (currently using Intent().setClassName(context,
"com.ku_stacks.ku_ring.HostActivity")) with a type-safe constructor
Intent(context, HostActivity::class.java); keep the existing extras
(DeepLinkIntentFactory.INTENT_KEY_ROUTE and route.route) and update imports to
include the HostActivity class so compilation catches renames/refactors instead
of causing runtime errors.
In
`@core/navigation/src/main/java/com/ku_stacks/ku_ring/navigation/keys/AppLifecycleKeys.kt`:
- Around line 10-14: The AuthEntryPoint enum's serialized wire values rely on
constant names which can break compatibility if constants are renamed; update
AuthEntryPoint (keeping the `@Serializable` annotation) to use explicit
`@SerialName` annotations on each enum constant (e.g., annotate SIGN_IN and
SIGN_OUT with stable string names) so the wire format is fixed and renames won't
break serialization; ensure the kotlinx.serialization.SerialName import is added
if missing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: abadaa66-7c6e-4fc9-91a8-0ed1236c6e8a
📒 Files selected for processing (12)
app/src/main/java/com/ku_stacks/ku_ring/HostActivity.ktapp/src/main/java/com/ku_stacks/ku_ring/navigator/DeepLinkIntentFactoryImpl.ktcore/navigation/src/main/java/com/ku_stacks/ku_ring/navigation/Navigator.ktcore/navigation/src/main/java/com/ku_stacks/ku_ring/navigation/keys/AppLifecycleKeys.ktdomain/navigation/src/main/java/com/ku_stacks/ku_ring/navigation/DeepLinkIntentFactory.ktfeature/auth/src/main/java/com/ku_stacks/ku_ring/auth/AuthFlowEntryBuilder.ktfeature/club/src/main/java/com/ku_stacks/ku_ring/club/detail/ClubDetailEntryBuilder.ktfeature/club/src/main/java/com/ku_stacks/ku_ring/club/detail/ClubDetailViewModel.ktfeature/kuringbot/src/main/java/com/ku_stacks/ku_ring/kuringbot/KuringBotEntryBuilder.ktfeature/main/src/main/java/com/ku_stacks/ku_ring/main/MainScreen.ktfeature/notice_detail/src/main/java/com/ku_stacks/ku_ring/notice_detail/noticeweb/NoticeWebScreen.ktfeature/onboarding/src/main/java/com/ku_stacks/ku_ring/onboarding/OnboardingEntryBuilder.kt
✅ Files skipped from review due to trivial changes (2)
- feature/notice_detail/src/main/java/com/ku_stacks/ku_ring/notice_detail/noticeweb/NoticeWebScreen.kt
- domain/navigation/src/main/java/com/ku_stacks/ku_ring/navigation/DeepLinkIntentFactory.kt
🚧 Files skipped from review as they are similar to previous changes (7)
- feature/kuringbot/src/main/java/com/ku_stacks/ku_ring/kuringbot/KuringBotEntryBuilder.kt
- feature/club/src/main/java/com/ku_stacks/ku_ring/club/detail/ClubDetailEntryBuilder.kt
- core/navigation/src/main/java/com/ku_stacks/ku_ring/navigation/Navigator.kt
- feature/club/src/main/java/com/ku_stacks/ku_ring/club/detail/ClubDetailViewModel.kt
- feature/auth/src/main/java/com/ku_stacks/ku_ring/auth/AuthFlowEntryBuilder.kt
- feature/onboarding/src/main/java/com/ku_stacks/ku_ring/onboarding/OnboardingEntryBuilder.kt
- feature/main/src/main/java/com/ku_stacks/ku_ring/main/MainScreen.kt
Collaborator
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
core/navigation/src/main/java/com/ku_stacks/ku_ring/navigation/Navigator.kt (1)
14-14:pendingDeepLink동시성 고려 필요.
pendingDeepLink가var로 선언되어 있어, 다른 스레드에서 동시 접근 시 race condition이 발생할 수 있습니다. 현재 코드에서는 대부분 메인 스레드에서 접근하지만, 방어적으로@Volatile어노테이션 또는AtomicReference를 사용하는 것이 안전합니다.Optional: Volatile 적용
- private var pendingDeepLink: NavKey? = null + `@Volatile` + private var pendingDeepLink: NavKey? = null🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/navigation/src/main/java/com/ku_stacks/ku_ring/navigation/Navigator.kt` at line 14, The field pendingDeepLink in Navigator is a mutable var and can suffer race conditions across threads; make access thread-safe by either marking pendingDeepLink as `@Volatile` (add the annotation above the declaration) or replace it with an AtomicReference<NavKey?> (change the type to AtomicReference and adjust reads/writes to use get()/set()/compareAndSet as needed), updating all usages in Navigator (reads/writes) to use the chosen concurrency-safe pattern.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/java/com/ku_stacks/ku_ring/HostActivity.kt`:
- Around line 93-97:
intent.getStringExtra(DeepLinkIntentFactory.INTENT_KEY_ROUTE)로 받은 route 값을 검증하지
않고 MainHubKey(startTab = route)를 반환하고 있어 잘못된 탭명이 들어오면 예기치 않은 동작이 발생할 수 있으니,
intent.getStringExtra(...)로 받은 route를 허용되는 탭 이름 집합(또는 기존 MainScreenRoute enum/상수
목록)과 대조하여 유효할 때만 MainHubKey에 전달하고, 유효하지 않다면 기본 탭으로 폴백하거나 null 반환하도록 변경하세요; 또는 더
나은 타입 안전성을 위해 MainHubKey의 startTab을 문자열 대신 enum/sealed class 타입으로 바꾸어 파싱/검증을
강제하세요.
In `@core/navigation/src/main/java/com/ku_stacks/ku_ring/navigation/Navigator.kt`:
- Around line 29-34: replaceAll currently clears pendingDeepLink unconditionally
causing deep links to be lost when onboarding occurs; change replaceAll(NavKey)
so it preserves pendingDeepLink when the new key is an onboarding entry (e.g.,
OnboardingKey) instead of consuming it, and add an explicit
consumePendingDeepLink(): Boolean (or similar) that appends pendingDeepLink to
backStack and nulls it only when onboarding is finished (call from the
onboarding completion flow, e.g., after OnboardingKey is replaced with
MainHubKey), referencing replaceAll, pendingDeepLink, backStack,
setPendingDeepLink and the new consumePendingDeepLink to control when the deep
link is applied.
---
Nitpick comments:
In `@core/navigation/src/main/java/com/ku_stacks/ku_ring/navigation/Navigator.kt`:
- Line 14: The field pendingDeepLink in Navigator is a mutable var and can
suffer race conditions across threads; make access thread-safe by either marking
pendingDeepLink as `@Volatile` (add the annotation above the declaration) or
replace it with an AtomicReference<NavKey?> (change the type to AtomicReference
and adjust reads/writes to use get()/set()/compareAndSet as needed), updating
all usages in Navigator (reads/writes) to use the chosen concurrency-safe
pattern.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2b4a34e3-ca69-4ab0-b94a-a136bc32213c
📒 Files selected for processing (4)
app/src/main/java/com/ku_stacks/ku_ring/HostActivity.ktapp/src/main/java/com/ku_stacks/ku_ring/navigator/DeepLinkIntentFactoryImpl.ktcore/navigation/src/main/java/com/ku_stacks/ku_ring/navigation/Navigator.ktcore/navigation/src/main/java/com/ku_stacks/ku_ring/navigation/keys/AppLifecycleKeys.kt
🚧 Files skipped from review as they are similar to previous changes (2)
- core/navigation/src/main/java/com/ku_stacks/ku_ring/navigation/keys/AppLifecycleKeys.kt
- app/src/main/java/com/ku_stacks/ku_ring/navigator/DeepLinkIntentFactoryImpl.kt
Comment on lines
+29
to
+34
| fun replaceAll(key: NavKey) { | ||
| backStack.clear() | ||
| backStack.add(key) | ||
| pendingDeepLink?.let { backStack.add(it) } | ||
| pendingDeepLink = null | ||
| } |
Contributor
There was a problem hiding this comment.
온보딩 플로우에서 딥링크가 손실될 수 있습니다.
replaceAll 로직 분석 결과:
- 앱 cold start 시
setPendingDeepLink(key)호출 - Splash 완료 후 온보딩 필요 시
replaceAll(OnboardingKey)호출 - backStack =
[OnboardingKey, deepLinkKey]— 딥링크가 스택 상단에 추가됨 - 사용자가 온보딩 완료 시
replaceAll(MainHubKey())호출 → 딥링크 손실
온보딩이 필요한 경우 딥링크를 별도로 보존하거나, 온보딩 완료 후에만 pendingDeepLink를 소비하도록 수정이 필요합니다.
수정 제안: 온보딩 플로우에서 딥링크 보존
fun replaceAll(key: NavKey) {
backStack.clear()
backStack.add(key)
- pendingDeepLink?.let { backStack.add(it) }
- pendingDeepLink = null
+ // 온보딩 화면으로 전환 시에는 딥링크를 보존
+ // MainHubKey로 전환 시에만 딥링크 소비
+ if (key is MainHubKey) {
+ pendingDeepLink?.let { backStack.add(it) }
+ pendingDeepLink = null
+ }
}또는 consumePendingDeepLink(): Boolean 메서드를 별도로 분리하여 호출 시점을 명시적으로 제어할 수 있습니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@core/navigation/src/main/java/com/ku_stacks/ku_ring/navigation/Navigator.kt`
around lines 29 - 34, replaceAll currently clears pendingDeepLink
unconditionally causing deep links to be lost when onboarding occurs; change
replaceAll(NavKey) so it preserves pendingDeepLink when the new key is an
onboarding entry (e.g., OnboardingKey) instead of consuming it, and add an
explicit consumePendingDeepLink(): Boolean (or similar) that appends
pendingDeepLink to backStack and nulls it only when onboarding is finished (call
from the onboarding completion flow, e.g., after OnboardingKey is replaced with
MainHubKey), referencing replaceAll, pendingDeepLink, backStack,
setPendingDeepLink and the new consumePendingDeepLink to control when the deep
link is applied.
Collaborator
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
app/src/main/java/com/ku_stacks/ku_ring/HostActivity.kt (1)
63-74:⚠️ Potential issue | 🟠 MajorSplash가 아직 top일 때는 새 인텐트도 defer해야 합니다.
지금은
onNewIntent()가 항상deferred = false로 들어와서, singleTask 인스턴스가 아직SplashKey를 표시 중일 때 Line 73에서 바로navigate(key)합니다. 그러면 splash 초기화/업데이트 분기를 우회할 수 있으니, 현재 top이SplashKey면 cold start와 동일하게 pending deep link로 보류하는 쪽이 안전합니다.수정 예시
private fun handleIntent(intent: Intent, deferred: Boolean) { val key = extractNavKey(intent) ?: return - if (deferred) { + val shouldDefer = deferred || navigator.backStack.lastOrNull() == SplashKey + if (shouldDefer) { navigator.setPendingDeepLink(key) } else { navigator.navigate(key) } }import com.ku_stacks.ku_ring.navigation.keys.SplashKey🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/ku_stacks/ku_ring/HostActivity.kt` around lines 63 - 74, onNewIntent currently calls handleIntent(intent, deferred = false) which causes navigate(key) to run even when the Splash screen is still the top; change the logic so that new intents are deferred if the current top navigation key is SplashKey. In practice, import SplashKey, detect the current top key (e.g., via navigator.currentTopKey or equivalent), and call handleIntent(intent, deferred = true) when that top is a SplashKey; keep using navigator.setPendingDeepLink(key) for deferred handling and navigator.navigate(key) otherwise, and update any reference to extractNavKey, onNewIntent, handleIntent, navigator.setPendingDeepLink, and navigator.navigate accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@feature/notification/src/main/java/com/ku_stacks/ku_ring/notification/NotificationEntryBuilder.kt`:
- Around line 26-39: The click handler in NotificationEntryBuilder
(onNotificationClick) only handles NotificationContent.Notice, leaving
NotificationContent.Club and NotificationContent.Common unhandled so taps on
those items do nothing; update onNotificationClick to use a when on
notification.content covering Notice, Club, and Common (or an else branch) and
for each branch either call navigator.navigate with the appropriate destination
(e.g., a Club or Common navigation key) or explicitly handle no-op/logging so
the behavior is deterministic; reference the existing Notice handling that uses
NoticeWebKey and navigator.navigate as the pattern to follow when adding
Club/Common routing or an explicit branch.
In
`@feature/splash/src/main/java/com/ku_stacks/ku_ring/splash/SplashEntryBuilder.kt`:
- Around line 77-84: 현재 consumePendingDeepLink()는 pending 키를 backStack에
append하므로 MainHubKey같은 루트 키를 가진 cold-start 딥링크에서 backStack에 중복된 루트가 생깁니다;
SplashEntryBuilder.kt의 흐름에서 MainHubKey로 네비게이트할 때는 pending deep link를 append하지 않고
replaceAll로 루트 교체해야 합니다. 수정 방법: navigator.replaceAll(MainHubKey()) 후 바로
navigator.consumePendingDeepLink() 하지 말고 대신 Navigator의 pending 값을 확인해 pending이
MainHubKey 타입(루트 역할)이면 navigator.replaceAll(pendingKey)로 교체하고 아니면 기존처럼
navigator.consumePendingDeepLink()로 append하도록 분기 처리(참고 심볼:
navigator.replaceAll(), navigator.consumePendingDeepLink(), MainHubKey,
OnboardingKey 및 OnboardingEntryBuilder의 동일 패턴도 동일하게 적용).
- Around line 52-64: getUserData() is currently fire-and-forget (launched inside
viewModelScope) which allows updateDepartmentsFromRemote() /
checkUpdateRequired() to run before preferences are actually updated; change
getUserData() to be suspend (or provide a suspend wrapper that completes when
preferences are written) and call it with suspend semantics from the
LaunchedEffect, or have getUserData return a Job/Deferred and await it in
SplashEntryBuilder's LaunchedEffect before invoking
updateDepartmentsFromRemote() and viewModel.checkUpdateRequired(...); ensure you
await completion of getUserData() (referencing getUserData(),
updateDepartmentsFromRemote(), checkUpdateRequired(), and the LaunchedEffect
block) so onboarding branching based on screenState only runs after preferences
update.
---
Duplicate comments:
In `@app/src/main/java/com/ku_stacks/ku_ring/HostActivity.kt`:
- Around line 63-74: onNewIntent currently calls handleIntent(intent, deferred =
false) which causes navigate(key) to run even when the Splash screen is still
the top; change the logic so that new intents are deferred if the current top
navigation key is SplashKey. In practice, import SplashKey, detect the current
top key (e.g., via navigator.currentTopKey or equivalent), and call
handleIntent(intent, deferred = true) when that top is a SplashKey; keep using
navigator.setPendingDeepLink(key) for deferred handling and
navigator.navigate(key) otherwise, and update any reference to extractNavKey,
onNewIntent, handleIntent, navigator.setPendingDeepLink, and navigator.navigate
accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f62a51d3-756e-4bbc-a119-7f8dddc4f20b
📒 Files selected for processing (7)
app/src/main/java/com/ku_stacks/ku_ring/HostActivity.ktcore/navigation/src/main/java/com/ku_stacks/ku_ring/navigation/Navigator.ktfeature/club/src/main/java/com/ku_stacks/ku_ring/club/subscription/ClubSubscriptionEntryBuilder.ktfeature/main/src/main/java/com/ku_stacks/ku_ring/main/MainScreen.ktfeature/notification/src/main/java/com/ku_stacks/ku_ring/notification/NotificationEntryBuilder.ktfeature/onboarding/src/main/java/com/ku_stacks/ku_ring/onboarding/OnboardingEntryBuilder.ktfeature/splash/src/main/java/com/ku_stacks/ku_ring/splash/SplashEntryBuilder.kt
✅ Files skipped from review due to trivial changes (1)
- feature/main/src/main/java/com/ku_stacks/ku_ring/main/MainScreen.kt
🚧 Files skipped from review as they are similar to previous changes (2)
- feature/club/src/main/java/com/ku_stacks/ku_ring/club/subscription/ClubSubscriptionEntryBuilder.kt
- core/navigation/src/main/java/com/ku_stacks/ku_ring/navigation/Navigator.kt
Comment on lines
+26
to
+39
| onNotificationClick = { notification -> | ||
| val content = notification.content | ||
| if (content is NotificationContent.Notice) { | ||
| navigator.navigate( | ||
| NoticeWebKey( | ||
| url = content.fullUrl, | ||
| articleId = content.articleId, | ||
| id = notification.id.toString(), | ||
| category = content.noticeCategory, | ||
| subject = content.subject, | ||
| ) | ||
| ) | ||
| } | ||
| }, |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# NotificationContent subtype와 NotificationScreen의 클릭 전달 경로를 확인합니다.
fd 'Notification\.kt$' -x sed -n '1,120p' {}
fd 'NotificationScreen\.kt$' -x sed -n '1,260p' {}
rg -nC3 -g '*.kt' 'NotificationContent\.(Notice|Club|Common)|onNotificationClick\s*=|onNotificationClick\('Repository: ku-ring/KU-Ring-Android
Length of output: 17117
Notice 외의 알림 클릭 시 라우팅 처리가 누락되어 있습니다.
도메인 모델상 NotificationContent는 Notice, Club, Common 세 가지 서브타입을 가지고 있으며, NotificationScreen에서는 모든 타입의 알림을 렌더링하고 클릭 가능하게 제공합니다. 그러나 NotificationEntryBuilder에서는 Notice만 처리하므로, Club이나 Common 타입의 알림을 탭하면 onNotificationClick 콜백이 실행되지만 네비게이션이 발생하지 않습니다.
의도적으로 이동 대상이 없다면 when에서 각 분기를 명시하고, 아니라면 해당 타입들에 대한 라우팅을 연결해야 합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@feature/notification/src/main/java/com/ku_stacks/ku_ring/notification/NotificationEntryBuilder.kt`
around lines 26 - 39, The click handler in NotificationEntryBuilder
(onNotificationClick) only handles NotificationContent.Notice, leaving
NotificationContent.Club and NotificationContent.Common unhandled so taps on
those items do nothing; update onNotificationClick to use a when on
notification.content covering Notice, Club, and Common (or an else branch) and
for each branch either call navigator.navigate with the appropriate destination
(e.g., a Club or Common navigation key) or explicitly handle no-op/logging so
the behavior is deterministic; reference the existing Notice handling that uses
NoticeWebKey and navigator.navigate as the pattern to follow when adding
Club/Common routing or an explicit branch.
Comment on lines
+52
to
+64
| LaunchedEffect(Unit) { | ||
| // WorkManager re-engagement notification 예약 | ||
| enqueueReengagementNotificationWork(context) | ||
|
|
||
| // 사용자 유효성 검증 | ||
| viewModel.getUserData() | ||
|
|
||
| // 원격 학과 데이터 업데이트 | ||
| viewModel.updateDepartmentsFromRemote() | ||
|
|
||
| // 최소 앱 버전 체크 (500ms delay 포함) | ||
| delay(500) | ||
| viewModel.checkUpdateRequired(context.getAppVersionName()) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="$(fd -i 'SplashViewModel.kt' | head -n 1)"
if [ -z "$file" ]; then
echo "SplashViewModel.kt not found" >&2
exit 1
fi
echo "== $file =="
sed -n '1,260p' "$file"
echo
echo "== Relevant definitions =="
rg -n -C4 'class\s+SplashViewModel\b|suspend\s+fun\s+getUserData\b|fun\s+getUserData\b|suspend\s+fun\s+updateDepartmentsFromRemote\b|fun\s+updateDepartmentsFromRemote\b|suspend\s+fun\s+checkUpdateRequired\b|fun\s+checkUpdateRequired\b|splashScreenState' "$file"Repository: ku-ring/KU-Ring-Android
Length of output: 3849
getUserData()를 suspend 함수로 변경하거나 완료를 대기해야 합니다.
getUserData()는 viewModelScope.launch로 정의되어 있어 fire-and-forget 방식으로 동작합니다. 현재 코드는 getUserData()를 호출한 직후 즉시 updateDepartmentsFromRemote()로 진행하므로, 사용자 preference 갱신이 완료되기 전에 onboarding 분기 로직(screenState 관찰)이 실행될 수 있습니다.
getUserData() 호출 후 preferences가 실제로 업데이트될 때까지 대기하도록 수정하세요. suspend 함수로 변경하거나, LaunchedEffect 내에서 명시적으로 완료를 기다려야 합니다.
Also applies to: 68-79
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@feature/splash/src/main/java/com/ku_stacks/ku_ring/splash/SplashEntryBuilder.kt`
around lines 52 - 64, getUserData() is currently fire-and-forget (launched
inside viewModelScope) which allows updateDepartmentsFromRemote() /
checkUpdateRequired() to run before preferences are actually updated; change
getUserData() to be suspend (or provide a suspend wrapper that completes when
preferences are written) and call it with suspend semantics from the
LaunchedEffect, or have getUserData return a Job/Deferred and await it in
SplashEntryBuilder's LaunchedEffect before invoking
updateDepartmentsFromRemote() and viewModel.checkUpdateRequired(...); ensure you
await completion of getUserData() (referencing getUserData(),
updateDepartmentsFromRemote(), checkUpdateRequired(), and the LaunchedEffect
block) so onboarding branching based on screenState only runs after preferences
update.
Comment on lines
+77
to
+84
| delay(500) | ||
| val onboardingRequired = preferences.firstRunFlag && preferences.subscription.isEmpty() | ||
| if (onboardingRequired) { | ||
| createNotificationChannel(context) | ||
| navigator.replaceAll(OnboardingKey) | ||
| } else { | ||
| navigator.replaceAll(MainHubKey()) | ||
| navigator.consumePendingDeepLink() |
Contributor
There was a problem hiding this comment.
루트형 딥링크는 append가 아니라 root replacement로 처리해야 합니다.
Navigator.consumePendingDeepLink()는 pending 키를 그대로 backStack.add(...) 합니다 (core/navigation/src/main/java/com/ku_stacks/ku_ring/navigation/Navigator.kt:30-38). 그래서 pending 값이 MainHubKey(startTab = ...)인 cold start 탭 딥링크에서는 여기서 [MainHubKey(), MainHubKey(route)] 스택이 만들어지고, 뒤로 가기가 앱 종료 대신 기본 탭으로 떨어집니다. MainHubKey처럼 루트 역할인 키는 append하지 말고 root replacement로 처리해야 합니다. 같은 패턴이 feature/onboarding/src/main/java/com/ku_stacks/ku_ring/onboarding/OnboardingEntryBuilder.kt Line 31-32에도 있습니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@feature/splash/src/main/java/com/ku_stacks/ku_ring/splash/SplashEntryBuilder.kt`
around lines 77 - 84, 현재 consumePendingDeepLink()는 pending 키를 backStack에
append하므로 MainHubKey같은 루트 키를 가진 cold-start 딥링크에서 backStack에 중복된 루트가 생깁니다;
SplashEntryBuilder.kt의 흐름에서 MainHubKey로 네비게이트할 때는 pending deep link를 append하지 않고
replaceAll로 루트 교체해야 합니다. 수정 방법: navigator.replaceAll(MainHubKey()) 후 바로
navigator.consumePendingDeepLink() 하지 말고 대신 Navigator의 pending 값을 확인해 pending이
MainHubKey 타입(루트 역할)이면 navigator.replaceAll(pendingKey)로 교체하고 아니면 기존처럼
navigator.consumePendingDeepLink()로 append하도록 분기 처리(참고 심볼:
navigator.replaceAll(), navigator.consumePendingDeepLink(), MainHubKey,
OnboardingKey 및 OnboardingEntryBuilder의 동일 패턴도 동일하게 적용).
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
1 participant
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.
Summary by CodeRabbit
개선 사항
새로운 기능
버그 수정