Skip to content
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,6 @@ interface Navigator {

fun navigateToGroupChat(
Comment thread
kongwoojin marked this conversation as resolved.
context: Context,
postId: Int
extraPostId: Int
): Intent
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,5 @@ const val EXTRA_CHAT_ROOM_ID = "chat_room_id"
const val EXTRA_BOARD_ID = "extra_board_id"
const val EXTRA_CLUB_ID = "extra_club_id"
const val EXTRA_EVENT_ID = "extra_event_id"
const val EXTRA_POST_ID = "extra_post_id"
const val EXTRA_NAV_TYPE = "extra_nav_type"
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,14 @@ import androidx.compose.foundation.background
import androidx.compose.foundation.layout.consumeWindowInsets
import androidx.compose.foundation.layout.padding
import androidx.compose.material3.Scaffold
import androidx.compose.runtime.LaunchedEffect
import androidx.compose.ui.Modifier
import androidx.navigation.compose.NavHost
import androidx.navigation.compose.rememberNavController
import dagger.hilt.android.AndroidEntryPoint
import `in`.koreatech.koin.core.designsystem.theme.RebrandKoinTheme
import `in`.koreatech.koin.core.designsystem.util.enableEdgeToEdgeWithLightStatusBar
import `in`.koreatech.koin.core.navigation.utils.EXTRA_ID
import `in`.koreatech.koin.feature.callvan.navigation.CallvanNavType
import `in`.koreatech.koin.feature.callvan.navigation.koinCallvanGraph

Expand All @@ -36,6 +38,15 @@ class CallvanActivity : ComponentActivity() {
RebrandKoinTheme {
val navController = rememberNavController()

val targetId = intent.getIntExtra(EXTRA_ID, -1)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Minor] remember 없이 recomposition마다 intent extra를 재읽음

targetIdsetContent {} 내부에서 remember 없이 읽히면, recomposition이 발생할 때마다 intent.getIntExtra()가 재호출됩니다. 이 값은 Activity 생명주기 동안 변하지 않으므로 remember로 캐싱하는 것이 Compose 관례에 맞습니다.

Suggested change
val targetId = intent.getIntExtra(EXTRA_ID, -1)
val targetId = remember { intent.getIntExtra(EXTRA_ID, -1) }


// Scheme url from backend is not matching with deeplink url in navigation
// So, let's navigate to correct deeplink from here
Comment thread
kongwoojin marked this conversation as resolved.
LaunchedEffect(Unit) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Minor] LaunchedEffect(Unit) 키 — onNewIntent 시 재탐색 불가

Unit을 키로 사용하면 Composition이 처음 진입할 때 단 한 번만 실행됩니다. 만약 이 Activity가 android:launchMode="singleTop" 또는 FLAG_ACTIVITY_SINGLE_TOP으로 재사용되어 onNewIntent()가 호출된다면, 새로운 targetId로 네비게이션이 트리거되지 않습니다.

현재 구조에서 onNewIntent를 처리하지 않는다면 이 패턴은 괜찮지만, 주석이나 TODO로 이 제약을 명시해두는 것을 권장합니다.

Suggested change
LaunchedEffect(Unit) {
// NOTE: LaunchedEffect(Unit) intentionally runs only once per composition entry.
// If this Activity needs to handle onNewIntent (e.g. singleTop), use
// rememberUpdatedState + LaunchedEffect(targetId) instead.
// TODO: Remove this workaround once backend deeplink scheme is aligned with nav graph.
LaunchedEffect(Unit) {

if (targetId == -1) return@LaunchedEffect
navController.navigate(CallvanNavType.CallvanDetail(targetId))
}
Comment on lines +45 to +48
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Major] 화면 회전 시 중복 네비게이션 버그

LaunchedEffect(Unit)은 컴포지션이 새로 시작될 때마다 실행됩니다. rememberNavController()는 백스택 상태를 복원하지만, 화면 회전 후 재구성 시 LaunchedEffect(Unit) 블록이 다시 실행되어 이미 CallvanDetail이 스택에 있는 상태에서 또 navigate를 호출하게 됩니다. 결과적으로 백스택에 CallvanDetail이 중복으로 쌓입니다.

또한, 이 코드에는 더 우아한 기존 패턴이 존재합니다. EXTRA_NAVIGATE_TO_CREATEstartDestination으로 처리하는 반면, EXTRA_IDLaunchedEffect로 처리하는 것은 일관성이 없습니다.

권장 수정 방법:

Suggested change
LaunchedEffect(Unit) {
if (targetId == -1) return@LaunchedEffect
navController.navigate(CallvanNavType.CallvanDetail(targetId))
}
var hasNavigated by rememberSaveable { mutableStateOf(false) }
val targetId = intent.getIntExtra(EXTRA_ID, -1)
// Scheme url from backend is not matching with deeplink url in navigation
// Navigate to the correct destination from here instead of relying on URI matching.
LaunchedEffect(Unit) {
if (targetId == -1 || hasNavigated) return@LaunchedEffect
hasNavigated = true
navController.navigate(CallvanNavType.CallvanDetail(targetId))
}

또는 기존 startDestination 패턴을 따라 아예 startDestination에서 처리하는 방법도 고려해 보세요.


Scaffold { innerPadding ->
NavHost(
modifier = Modifier
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import androidx.activity.compose.rememberLauncherForActivityResult
import androidx.activity.result.contract.ActivityResultContracts
import androidx.compose.runtime.LaunchedEffect
import androidx.compose.runtime.collectAsState
import androidx.compose.runtime.remember
import androidx.compose.ui.platform.LocalContext
import androidx.navigation.NavController
import androidx.navigation.NavGraphBuilder
Expand Down Expand Up @@ -83,14 +82,16 @@ fun NavGraphBuilder.koinCallvanGraph(
composable<CallvanNavType.CallvanChat> {
val navigator = rememberNavigator()
val context = LocalContext.current
val intent = remember { navigator.navigateToGroupChat(context, it.toRoute<CallvanNavType.CallvanChat>().postId) }
val launcher = rememberLauncherForActivityResult(
contract = ActivityResultContracts.StartActivityForResult()
) {
navController.popBackStack()
}
LaunchedEffect(Unit) {
Comment thread
kongwoojin marked this conversation as resolved.
launcher.launch(intent)
navigator.navigateToGroupChat(
context,
it.toRoute<CallvanNavType.CallvanChat>().postId
).let(launcher::launch)
}
Comment thread
kongwoojin marked this conversation as resolved.
}

Expand Down
1 change: 1 addition & 0 deletions feature/chat/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ dependencies {
implementation(projects.core.onboarding)
implementation(projects.core.designsystem)
implementation(projects.core.analytics)
implementation(projects.core.navigation)

implementation(libs.androidx.core.ktx)
implementation(libs.androidx.appcompat)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
package `in`.koreatech.koin.feature.chat.ui.groupchat

import android.content.Context
import android.content.Intent
import android.content.pm.ActivityInfo
import android.os.Bundle
import androidx.activity.ComponentActivity
Expand All @@ -26,10 +24,4 @@ class GroupChatActivity : ComponentActivity() {
}
}
}

companion object {
fun createIntent(context: Context, postId: Int): Intent = Intent(context, GroupChatActivity::class.java).apply {
putExtra(GroupChatViewModel.POST_ID, postId)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import androidx.lifecycle.SavedStateHandle
import androidx.lifecycle.ViewModel
import androidx.lifecycle.viewModelScope
import dagger.hilt.android.lifecycle.HiltViewModel
import `in`.koreatech.koin.core.navigation.utils.EXTRA_POST_ID
import `in`.koreatech.koin.domain.model.callvan.CallvanChatMessage.CallvanMessage
import `in`.koreatech.koin.domain.model.upload.PreSignedUrlDomain
import `in`.koreatech.koin.domain.usecase.callvan.GetCallvanChatMessagesUseCase
Expand Down Expand Up @@ -42,7 +43,7 @@ class GroupChatViewModel @Inject constructor(
) : ViewModel(), ContainerHost<GroupChatState, GroupChatSideEffect> {

override val container = container<GroupChatState, GroupChatSideEffect>(GroupChatState()) {
Comment thread
kongwoojin marked this conversation as resolved.
val postId: Int? = savedStateHandle[POST_ID]
val postId: Int? = savedStateHandle[EXTRA_POST_ID]
checkNotNull(postId)
Comment on lines +46 to 47
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Major] CALLVAN_CHAT 스킴 URL에 postId 누락 시 앱 크래시

SchemeType.CALLVAN_CHAT으로 라우팅될 때, SchemeActivity는 URL에서 ?postId=X 쿼리 파라미터를 추출합니다. 만약 백엔드 알림 URL에 postId가 빠져 있거나 URL 형식이 변경된다면:

  1. getPostIdFromUrl()null 반환
  2. .filter { it.second != null }에 의해 필터링됨
  3. GroupChatActivityEXTRA_POST_ID 없이 실행됨
  4. savedStateHandle[EXTRA_POST_ID]null
  5. checkNotNull(postId)IllegalStateException 크래시

checkNotNull은 개발 단계 early-fail에 유용하지만, 프로덕션 환경에서는 사용자가 앱을 강제종료 당하는 경험을 합니다. 최소한 에러 상태를 UI에 표시하고 gracefully 종료하도록 처리해야 합니다.

Suggested change
val postId: Int? = savedStateHandle[EXTRA_POST_ID]
checkNotNull(postId)
val postId: Int? = savedStateHandle[EXTRA_POST_ID]
if (postId == null) {
postSideEffect(GroupChatSideEffect.FailedToLoadMessages)
return@container
}

혹은 checkNotNull 유지 시, 해당 케이스를 SchemeActivity에서 사전 검증하여 ID 없이는 GroupChatActivity를 실행하지 않도록 방어하는 로직을 추가하는 것을 권장합니다.

getCallvanPostDetail(postId)
}
Expand Down Expand Up @@ -226,7 +227,6 @@ class GroupChatViewModel @Inject constructor(
companion object {
const val MAX_USER_COLORS = 8
const val POLLING_INTERVAL_MS = 1000L
const val POST_ID = "post_id"
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package `in`.koreatech.koin.navigation
import android.content.Context
import android.content.Intent
import `in`.koreatech.koin.core.navigation.Navigator
import `in`.koreatech.koin.core.navigation.utils.EXTRA_POST_ID
import `in`.koreatech.koin.core.navigation.utils.buildDeepLinkIntent
import `in`.koreatech.koin.core.navigation.utils.buildIntent
import `in`.koreatech.koin.core.navigation.utils.isValidDeepLink
Expand Down Expand Up @@ -69,8 +70,14 @@ class NavigatorImpl @Inject constructor() : Navigator {
}
}

override fun navigateToGroupChat(context: Context, postId: Int): Intent {
return GroupChatActivity.createIntent(context, postId).apply {
override fun navigateToGroupChat(
context: Context,
extraPostId: Int
): Intent {
return context.buildIntent(
GroupChatActivity::class.java,
EXTRA_POST_ID to extraPostId
).apply {
flags = Intent.FLAG_ACTIVITY_CLEAR_TOP
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package `in`.koreatech.koin.navigation

import `in`.koreatech.koin.feature.article.ArticleActivity
import `in`.koreatech.koin.feature.callvan.CallvanActivity
import `in`.koreatech.koin.feature.chat.ui.groupchat.GroupChatActivity
import `in`.koreatech.koin.feature.chat.ui.room.ChatRoomActivity
import `in`.koreatech.koin.feature.club.ui.ClubActivity
import `in`.koreatech.koin.feature.dining.ui.DiningActivity
Expand All @@ -17,8 +19,10 @@ enum class SchemeType(
CHAT("chat", ChatRoomActivity::class.java),
CLUB_RECRUIT("club-recruitment", ClubActivity::class.java),
CLUB("club", ClubActivity::class.java),

LOST_AND_FOUND("articles/lost-item", LostAndFoundActivity::class.java);
LOST_AND_FOUND("articles/lost-item", LostAndFoundActivity::class.java),
Comment thread
kongwoojin marked this conversation as resolved.
CALLVAN("callvan", CallvanActivity::class.java),
CALLVAN_CHAT("callvan-chat", GroupChatActivity::class.java)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Trivial] GroupChatActivity를 SchemeType에 직접 노출하는 구조 검토 필요

CALLVAN_CHAT("callvan-chat", GroupChatActivity::class.java) 항목은 GroupChatActivitycallvan 기능 외부(:koin 앱 모듈)에서 직접 참조됨을 의미합니다. GroupChatActivity는 callvan 채팅 전용 Activity임에도 불구하고 :feature:chat 모듈 내부에 있어, 모듈 경계가 약해질 수 있습니다.

중장기적으로는 CallvanActivity를 통해 내비게이션을 통합하거나(CALLVAN으로 진입 후 내부에서 라우팅), 또는 GroupChatActivity:feature:callvan 모듈로 이동하는 방안을 검토해볼 수 있습니다. 이번 PR의 직접적인 변경사항은 아니지만, 현재 코드의 CallvanActivity.kt 주석("Scheme url from backend is not matching")과 함께 아키텍처 관점에서 향후 개선 포인트로 남겨둘 만합니다.

;
Comment thread
kongwoojin marked this conversation as resolved.

companion object {
fun fromType(type: String?): SchemeType? {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import android.content.Intent
import android.net.Uri
import android.os.Bundle
import androidx.core.content.getSystemService
import androidx.core.net.toUri
import dagger.hilt.android.AndroidEntryPoint
import `in`.koreatech.koin.core.activity.ActivityBase
import `in`.koreatech.koin.core.analytics.AnalyticsConstant
Expand All @@ -19,6 +20,7 @@ import `in`.koreatech.koin.core.navigation.utils.EXTRA_CLUB_ID
import `in`.koreatech.koin.core.navigation.utils.EXTRA_EVENT_ID
import `in`.koreatech.koin.core.navigation.utils.EXTRA_ID
import `in`.koreatech.koin.core.navigation.utils.EXTRA_NAV_TYPE
import `in`.koreatech.koin.core.navigation.utils.EXTRA_POST_ID
import `in`.koreatech.koin.core.navigation.utils.EXTRA_TYPE
import `in`.koreatech.koin.core.navigation.utils.EXTRA_URL
import `in`.koreatech.koin.core.navigation.utils.toHost
Expand Down Expand Up @@ -125,36 +127,41 @@ class SchemeActivity : ActivityBase() {
Pair(EXTRA_CHAT_ROOM_ID, getChatRoomIdFromUrl(url)),
Pair(EXTRA_BOARD_ID, getBoardIdFromUrl(url)),
Pair(EXTRA_CLUB_ID, getClubIdFromUrl(url)),
Pair(EXTRA_EVENT_ID, getEventIdFromUrl(url))
Pair(EXTRA_EVENT_ID, getEventIdFromUrl(url)),
Pair(EXTRA_POST_ID, getPostIdFromUrl(url))
).filter { it.second != null }
}

private fun getIdFromUrl(url: String): Int? {
return Uri.parse(url).getQueryParameter("id")?.toIntOrNull()
return url.toUri().getQueryParameter("id")?.toIntOrNull()
}

private fun getArticleIdFromUrl(url: String): Int? {
return Uri.parse(url).getQueryParameter("articleId")?.toIntOrNull()
return url.toUri().getQueryParameter("articleId")?.toIntOrNull()
}
Comment thread
kongwoojin marked this conversation as resolved.

private fun getChatRoomIdFromUrl(url: String): Int? {
return Uri.parse(url).getQueryParameter("chatRoomId")?.toIntOrNull()
return url.toUri().getQueryParameter("chatRoomId")?.toIntOrNull()
}

private fun getBoardIdFromUrl(url: String): Int? {
return Uri.parse(url).getQueryParameter("board-id")?.toIntOrNull()
return url.toUri().getQueryParameter("board-id")?.toIntOrNull()
}

private fun getKeywordFromUrl(url: String): String? {
return Uri.parse(url).getQueryParameter("keyword")
return url.toUri().getQueryParameter("keyword")
}

private fun getClubIdFromUrl(url: String): Int? {
return Uri.parse(url).getQueryParameter("clubId")?.toIntOrNull()
return url.toUri().getQueryParameter("clubId")?.toIntOrNull()
}

private fun getEventIdFromUrl(url: String): Int? {
return Uri.parse(url).getQueryParameter("eventId")?.toIntOrNull()
return url.toUri().getQueryParameter("eventId")?.toIntOrNull()
}

private fun getPostIdFromUrl(url: String): Int? {
return url.toUri().getQueryParameter("postId")?.toIntOrNull()
Comment thread
kongwoojin marked this conversation as resolved.
}

private fun navigateToActivity(intent: Intent) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import `in`.koreatech.koin.core.navigation.utils.EXTRA_CLUB_ID
import `in`.koreatech.koin.core.navigation.utils.EXTRA_EVENT_ID
import `in`.koreatech.koin.core.navigation.utils.EXTRA_ID
import `in`.koreatech.koin.core.navigation.utils.EXTRA_NAV_TYPE
import `in`.koreatech.koin.core.navigation.utils.EXTRA_POST_ID
import `in`.koreatech.koin.core.navigation.utils.EXTRA_TYPE
import `in`.koreatech.koin.core.onboarding.OnboardingManager
import `in`.koreatech.koin.core.toast.ToastUtil
Expand Down Expand Up @@ -244,6 +245,7 @@ class SplashActivity : ActivityBase() {
val targetChatId = intent.getIntExtra(EXTRA_CHAT_ROOM_ID, -1)
val targetClubId = intent.getIntExtra(EXTRA_CLUB_ID, -1)
val targetEventId = intent.getIntExtra(EXTRA_EVENT_ID, -1)
val targetPostId = intent.getIntExtra(EXTRA_POST_ID, -1)
Comment thread
kongwoojin marked this conversation as resolved.
val type = intent.getStringExtra(EXTRA_TYPE) ?: ""
val navType = intent.getStringExtra(EXTRA_NAV_TYPE) ?: ""

Expand All @@ -253,14 +255,13 @@ class SplashActivity : ActivityBase() {
navigator.navigateTo(
context = this@SplashActivity,
type = Pair(EXTRA_TYPE, type),
*arrayOf(
Pair(EXTRA_ID, targetId),
Pair(EXTRA_BOARD_ID, targetBoardId),
Pair(EXTRA_ARTICLE_ID, targetArticleId),
Pair(EXTRA_CHAT_ROOM_ID, targetChatId),
Pair(EXTRA_CLUB_ID, targetClubId),
Pair(EXTRA_EVENT_ID, targetEventId)
)
Pair(EXTRA_ID, targetId),
Pair(EXTRA_BOARD_ID, targetBoardId),
Pair(EXTRA_ARTICLE_ID, targetArticleId),
Pair(EXTRA_CHAT_ROOM_ID, targetChatId),
Pair(EXTRA_CLUB_ID, targetClubId),
Pair(EXTRA_EVENT_ID, targetEventId),
Pair(EXTRA_POST_ID, targetPostId)
Comment thread
kongwoojin marked this conversation as resolved.
)
Comment thread
kongwoojin marked this conversation as resolved.
} else {
Intent(this@SplashActivity, MainActivity::class.java)
Expand Down
Loading