[Feature] 레거시 MainActivity를 NewMainActivity로 교체#1533
Merged
Conversation
Contributor
Walkthrough기존 Changes메인 네비게이션 통합
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant SplashActivity
participant MainActivity
participant NavController
participant ArticleFragment
SplashActivity->>MainActivity: koin://article/navigation 전달
MainActivity->>NavController: 아티클 목적지와 인자 전달
NavController->>ArticleFragment: ArticleFragment 표시
ArticleFragment->>NavController: 상세 또는 키워드 화면 이동
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ 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 |
![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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
koin/src/main/java/in/koreatech/koin/ui/splash/SplashActivity.kt (1)
209-223: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win외부 딥링크 진입 시 게시글 ID 유실 문제 수정
현재 코드는 하드코딩된
"koin://article/navigation".toUri()만을 사용하여MainActivity로 인텐트를 전달하고 있습니다.
이로 인해 사용자가 외부(예: 웹)에서 특정 게시글 딥링크를 탭하여 진입한 경우, 기존intent.data에 존재하던id값이 유실되어 게시글 상세 화면으로 이동하지 않고 게시판 목록에 머무르는 버그가 발생합니다. 원본 URI에서id를 추출하여 새 딥링크에article_id및fragment파라미터로 함께 구성해 주어야 합니다.🐛 제안하는 수정안
private fun gotoArticleActivityOrDelay(state: TokenState) { lifecycleScope.launch { delay() val intent = Intent(this@SplashActivity, MainActivity::class.java).apply { - data = "koin://article/navigation".toUri() + val articleId = this@SplashActivity.intent.data?.getQueryParameter("id") + val uriBuilder = Uri.parse("koin://article/navigation").buildUpon() + if (articleId != null) { + uriBuilder.appendQueryParameter("fragment", "article_detail") + uriBuilder.appendQueryParameter("article_id", articleId) + } + data = uriBuilder.build() } startActivity(intent) overridePendingTransition(R.anim.fade, R.anim.hold)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@koin/src/main/java/in/koreatech/koin/ui/splash/SplashActivity.kt` around lines 209 - 223, Update gotoArticleActivityOrDelay so the MainActivity intent preserves an external article deep link: extract the original intent.data id, then include it as the article_id and fragment parameters when constructing the koin://article/navigation URI. Keep the existing navigation behavior for links without an id and continue using the resulting URI as the intent data.
🧹 Nitpick comments (4)
koin/src/main/java/in/koreatech/koin/ui/navigation/KoinNavigationDrawerActivity.kt (1)
642-645: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
String.toUri()확장 함수 사용 권장SonarCloud 정적 분석의 제안과 같이, 코틀린 환경에서의 가독성을 높이기 위해
Uri.parse()대신 KTX 확장 함수인.toUri()사용을 고려해 보세요. (참고:SplashActivity에서는 이미.toUri()를 일관되게 사용하고 있습니다)✨ 제안하는 수정안
- val intent = Intent(Intent.ACTION_VIEW).apply { - data = Uri.parse("koin://article/navigation") - `package` = packageName - } + val intent = Intent(Intent.ACTION_VIEW).apply { + data = "koin://article/navigation".toUri() + `package` = packageName + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@koin/src/main/java/in/koreatech/koin/ui/navigation/KoinNavigationDrawerActivity.kt` around lines 642 - 645, Update the Intent construction in KoinNavigationDrawerActivity to replace Uri.parse with the Kotlin KTX String.toUri() extension for the article navigation URI, preserving the existing URI value and packageName assignment.Source: Linters/SAST tools
koin/src/main/java/in/koreatech/koin/ui/main/activity/MainActivity.kt (1)
76-80: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value권한 결과 처리 조건문의 역전된 논리 주석 수정
permissionGranted가false인 경우(권한 거부됨)if (!permissionGranted)조건은true가 되어 해당 블록이 실행됩니다. 그러나 주석은handle permission granted(권한 허용됨)로 역전되어 작성되어 있습니다. 혼동을 방지하기 위해 긍정문으로 수정하거나 주석을 올바르게 변경해 주세요.💡 제안하는 수정안
- if (!permissionGranted) { - // handle permission granted - } else { - // handle permission denied - } + if (permissionGranted) { + // handle permission granted + } else { + // handle permission denied + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@koin/src/main/java/in/koreatech/koin/ui/main/activity/MainActivity.kt` around lines 76 - 80, Correct the permission-result comments in the conditional around permissionGranted: the !permissionGranted branch must describe denied permission, while the else branch must describe granted permission. Keep the existing condition and control flow unchanged.koin/src/main/java/in/koreatech/koin/ui/main/viewmodel/MainActivityViewModel.kt (1)
53-57: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low value
CancellationException처리 추가 권장코루틴 내에서
Exception을 포괄적으로 catch하면 코루틴 취소 시 발생하는CancellationException까지 무시하게 될 수 있습니다. 구조화된 동시성(Structured Concurrency)을 유지하기 위해CancellationException은 다시 던지도록(rethrow) 처리하는 것을 권장합니다.♻️ 제안하는 수정안
try { updateDeviceTokenUseCase() } catch (e: Exception) { + if (e is CancellationException) throw e Timber.e("Failed Update Fcm Token : ${e.message}") }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@koin/src/main/java/in/koreatech/koin/ui/main/viewmodel/MainActivityViewModel.kt` around lines 53 - 57, Update the exception handling around updateDeviceTokenUseCase in MainActivityViewModel so CancellationException is caught separately and rethrown, while retaining the existing Timber.e logging for other exceptions.koin/src/main/java/in/koreatech/koin/ui/main/fragment/ArticleFragment.kt (1)
96-96: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value뷰모델을 통한 바텀 네비게이션 가시성 제어 권장
현재
(activity as? MainActivity)로 액티비티에 직접 접근하여 바텀 네비게이션을 제어하고 있습니다. 프래그먼트와 액티비티 간의 결합도를 낮추기 위해, 공유 뷰모델(MainActivityViewModel)에 가시성 상태를 두고 액티비티에서 이를 관찰하여 제어하는 방식을 권장합니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@koin/src/main/java/in/koreatech/koin/ui/main/fragment/ArticleFragment.kt` at line 96, Update ArticleFragment’s bottom-navigation visibility flow to stop calling MainActivity.setBottomNavigationVisible directly. Store the isArticleList visibility state in the shared MainActivityViewModel, then have MainActivity observe that state and control the bottom navigation accordingly, preserving the existing visibility behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@koin/src/main/java/in/koreatech/koin/ui/main/activity/MainActivity.kt`:
- Line 158: Override MainActivity.onNewIntent to receive intents delivered while
the existing activity instance is running, call setIntent with the new intent,
and invoke the existing handleIntent flow so deep links and push notifications
trigger the same navigation as onCreate.
- Around line 108-115: Update the bottom navigation listener setup around
setupWithNavController and setOnItemSelectedListener so reselect events
explicitly invoke the same navigation logging flow. Override
setOnItemReselectedListener while preserving the default NavController reselect
behavior, including popUpTo and state restoration, and avoid duplicating or
bypassing the existing navigation handling.
---
Outside diff comments:
In `@koin/src/main/java/in/koreatech/koin/ui/splash/SplashActivity.kt`:
- Around line 209-223: Update gotoArticleActivityOrDelay so the MainActivity
intent preserves an external article deep link: extract the original intent.data
id, then include it as the article_id and fragment parameters when constructing
the koin://article/navigation URI. Keep the existing navigation behavior for
links without an id and continue using the resulting URI as the intent data.
---
Nitpick comments:
In `@koin/src/main/java/in/koreatech/koin/ui/main/activity/MainActivity.kt`:
- Around line 76-80: Correct the permission-result comments in the conditional
around permissionGranted: the !permissionGranted branch must describe denied
permission, while the else branch must describe granted permission. Keep the
existing condition and control flow unchanged.
In `@koin/src/main/java/in/koreatech/koin/ui/main/fragment/ArticleFragment.kt`:
- Line 96: Update ArticleFragment’s bottom-navigation visibility flow to stop
calling MainActivity.setBottomNavigationVisible directly. Store the
isArticleList visibility state in the shared MainActivityViewModel, then have
MainActivity observe that state and control the bottom navigation accordingly,
preserving the existing visibility behavior.
In
`@koin/src/main/java/in/koreatech/koin/ui/main/viewmodel/MainActivityViewModel.kt`:
- Around line 53-57: Update the exception handling around
updateDeviceTokenUseCase in MainActivityViewModel so CancellationException is
caught separately and rethrown, while retaining the existing Timber.e logging
for other exceptions.
In
`@koin/src/main/java/in/koreatech/koin/ui/navigation/KoinNavigationDrawerActivity.kt`:
- Around line 642-645: Update the Intent construction in
KoinNavigationDrawerActivity to replace Uri.parse with the Kotlin KTX
String.toUri() extension for the article navigation URI, preserving the existing
URI value and packageName assignment.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 329e7acf-bfe0-44a8-a387-05755c63ebe4
📒 Files selected for processing (18)
feature/article/src/main/AndroidManifest.xmlfeature/home/src/main/java/in/koreatech/koin/feature/home/profile/Constant.ktfeature/home/src/main/java/in/koreatech/koin/feature/home/profile/ProfileScreen.ktkoin/detekt-baseline.xmlkoin/src/main/AndroidManifest.xmlkoin/src/main/java/in/koreatech/koin/navigation/SchemeType.ktkoin/src/main/java/in/koreatech/koin/ui/main/activity/MainActivity.ktkoin/src/main/java/in/koreatech/koin/ui/main/fragment/ArticleFragment.ktkoin/src/main/java/in/koreatech/koin/ui/main/fragment/CategoryFragment.ktkoin/src/main/java/in/koreatech/koin/ui/main/fragment/HomeFragment.ktkoin/src/main/java/in/koreatech/koin/ui/main/fragment/NotificationFragment.ktkoin/src/main/java/in/koreatech/koin/ui/main/fragment/ProfileFragment.ktkoin/src/main/java/in/koreatech/koin/ui/main/viewmodel/MainActivityViewModel.ktkoin/src/main/java/in/koreatech/koin/ui/navigation/KoinNavigationDrawerActivity.ktkoin/src/main/java/in/koreatech/koin/ui/newmain/ArticleHostViewModel.ktkoin/src/main/java/in/koreatech/koin/ui/newmain/NewMainActivity.ktkoin/src/main/java/in/koreatech/koin/ui/splash/SplashActivity.ktkoin/src/main/res/navigation/nav_graph_main.xml
💤 Files with no reviewable changes (2)
- koin/src/main/java/in/koreatech/koin/ui/newmain/ArticleHostViewModel.kt
- koin/src/main/java/in/koreatech/koin/ui/newmain/NewMainActivity.kt
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
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
3 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.
PR 개요
PR 체크리스트
작업사항
작업사항의 상세한 설명
논의 사항
스크린샷
추가내용
Summary by CodeRabbit