[Fix] #883 - 콕찌르기 알림탭 진입 방식 수정#885
Conversation
Summary by CodeRabbit
WalkthroughPR은 PokeFeature의 라우팅 파라미터 ChangesPoke 라우팅 단순화
Sequence DiagramsequenceDiagram
participant App as ApplicationCoordinator
participant PokeC as PokeCoordinator
participant Builder as PokeBuilder
participant Nav as NavigationController
App->>PokeC: start()
PokeC->>PokeC: showPokeMain()
PokeC->>Builder: makePokeMain(coordinator:)
Builder->>Nav: setViewControllers([pokeMain.vc])
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
yungu0010
left a comment
yungu0010
left a comment
There was a problem hiding this comment.
리뷰 확인 부탁드립니다~!
또한 콕찌르기 알림에서 뒤로가기를 할 경우 계속 네트워크 에러가 발생하고 있어 이 부분도 확인이 필요해보입니다..!
질문) 솝탬프는 알림탭을 통해 진입했을 때 바텀시트 형태로 나타나게 되는데 콕찌르기와 통일하지 않아도 되나요? 같은 앱 서비스인데 진입 경로가 다른 점이 UX적으로 어색하게 느껴져 질문드려요!(아직 논의되지 않은 사안이라면 앱팀 회의 때 전달해주시면 감사하겠습니다!)
Simulator.Screen.Recording.-.iPhone.17.Pro.-.2026-05-20.at.07.17.54.mov
| case .deepLink(let url): | ||
| self.notificationHandler.receive(deepLink: url) | ||
| guard let deepLink = self.notificationHandler.deepLink.value else { return } | ||
| guard let deepLink = self.notificationHandler.deepLink.value else { return } |
There was a problem hiding this comment.
의미없는 공백은 지워주세요!
| guard let deepLink = self.notificationHandler.deepLink.value else { return } | |
| guard let deepLink = self.notificationHandler.deepLink.value else { return } |
| coordinator.runTabBarFlow(initSelectedTabType: .poke) | ||
| return coordinator.runPokeFlow() |
There was a problem hiding this comment.
runTabBarFlow 내에서 이미 runPokeFlow를 호출하고 있기 때문에 return coordiantor.runPokeFlow()는 중복 호출로 보여집니다!
1. 알림탭 > 콕찌르기 탭 진입을 n회 했을 때
해당 플로우를 실행할 때 마다 HomeVC와 SoptlogVC가 재생성되어 중복 인스턴스가 생기고 있음을 확인했습니다.
2026-05-20.7.00.12.mov
누수가 발생하고 있어 runTabBarFlow로 모든 VC를 재생성하기 보다 탭 바 인덱스만 교체해주는 것이 좋을 것 같아요.
솝트앱은 탭바가 자주 변경되는 환경인만큼 탭바 인덱스로 접근하는 대신 TabBarItemType를 사용해서 전환하는 것이 유지보수 측면에서 좋을 것 같습니다.
2. isDestination 분기처리 제거
isDestination는 /poke가 딥링크의 목적지인 경우를 나타내는데, 이제는 /poke가 딥링크의 목적지이든, 지금처럼 /poke/콕찌르기 상세가 딥링크의 목적지이든 상관없이 탭바로 띄워주기 때문에 isDestination을 통한 분기처리도 필요 없을 것 같아요.
앗 PokeNotificationListDeepLink가 PokeCoordinator를 필요로 하는군요..! 그런데 runPokeFlow()를 호출해버리면 start 메소드가 불필요하게 호출되니까,, ApplicationCoordinator에 PokeCoordinator를 저장하고 coordinator.pokeCoordinator를 반환해야할 것 같습니다..
혹시 더 좋은 방법이 있을까요?
There was a problem hiding this comment.
1. 알림탭 -> 콕찌르기 진입 시 메모리 누수 해결
기존 메모리 누수의 원인이었던runPokeFlow()호출하는 부분을 제거하고 딥링크를 받아오는 시점에 딥링크의 첫번째 요소를 검사하고 탭 정보를 객체에 주입하도록 코드를 수정했어요 .
현재는 모든 경우에 탭 인덱스를 바꾸는게 필요한 상황은 아닌지라 콕찌르기 부분만 반영을 했는데, 이 부분은 회의때 원래의 기획이 어땠는지 다시 물어봐야할 것 같아요.
deepLinkData 생성 시점에 진입이 필요한 탭 정보를 옵셔널로 주입합니다. 현재는 콕찌르기가 아니라면 기존과 동일하게 동작합니다.
private func parseDeepLink(with deepLink: String?) {
guard let deepLink else { return }
do {
let deepLinkData = try deepLinkParser.parse(with: deepLink)
let targetTap = resolveTargetTap(deepLinkData: deepLinkData)
let deepLinkComponents = DeepLinkComponents(deepLinkData: deepLinkData, targetTap: targetTap)
self.deepLink.send(deepLinkComponents)
} catch {
self.handleLinkError(error: error)
}
}ApplicationCoordinator 에서 화면 전환을 수행하기 전에 딥링크 객체에서 전환이 필요한 탭 정보를 확인하고, 필요한 경우 tabBarController 의 인덱스를 변경
private func handleNewDeepLink(deepLink: DeepLinkComponentsExecutable) {
self.rootNavigationController.popToRootViewController(animated: false)
if let targetTap = deepLink.targetTap {
tabBarController?.selectedIndex = targetTap.getTabIndex(userType: UserDefaultKeyList.Auth.getUserType())
}
deepLink.execute(coordinator: self)
}실제 딥링크를 실행하고 화면 전환은 코디네이터에서 이뤄지기 때문에 탭 전환도 ApplicationCoordinator 에서 받아온 딥링크로 tabBarController 인덱스를 변경하도록 했어요. 혹시 방식이 이상하거나 좋은 방법이 있다면 추가적으로 코멘트 달아주세요.
2. isDestination 분기처리
/poke 가 목적지라는게 알림을 통해서 들어갔을떄 콕찌르기 화면이 바로 나타나는 경우를 말씀하시는 걸까요? 이 부분은 제가 자세하게 이해를 못했는데, 그렇다면 만약 다른 알림또한 탭을 통해서 진입을 한다면 ApplicationCoordinator 가 다른 코디네이터도 전부 소유를 하도록 하는걸까요?
There was a problem hiding this comment.
1. 탭 정보 주입
ApplicationCoordinator에서 딥링크에 따라 탭바 인덱스를 수정하려고 DeepLinkComponentsExecutable, DeepLinkComponents, NotificationHandler에도 탭바 인덱스에 대한 정보를 넘겨주는 것으로 보입니다.. !
그런데 왜 /poke/notification-list 등의 딥링크일 때 ApplicationCoordinator에서 탭바를 전환해야하는지 궁금해요.
지금 딥링크 구조는 /를 단위로 파싱하여서 OODeepLink.swift에서 단계별로 화면전환을 수행하고, 그 다음(/ 다음에 오는 딥링크 컴포넌트)에 필요한 코디네이터를 반환 -> 파싱된 다음 딥링크 컴포넌트에서 같은 과정을 수행하는 것으로 이해했습니다.
그러면 ApplicationCoordinator에서는 구체적인 동작을 알 필요 없이 deepLink.excute()만 호출하고
실질적인 탭바 인덱스 전환은 PokeDeepLink에서 수정해주면 훨씬 더 단순할 것 같아요.
import Core
public struct PokeDeepLink: DeepLinkExecutable {
public let name = "poke"
public let children: [DeepLinkExecutable] = [PokeNotificationListDeepLink()]
public var isDestination: Bool = false
public func execute(with coordinator: Coordinator, queryItems: [URLQueryItem]?) -> Coordinator? {
guard let coordinator = coordinator as? ApplicationCoordinator else { return nil }
let userType = UserDefaultKeyList.Auth.getUserType()
coordinator.tabBarController?.selectedIndex = TabBarItemType.poke.getTabIndex(userType: userType)
if self.isDestination == true {
return coordinator
}
return coordinator.runPokeFlow()
}
}2. isDestination 분기처리
네 /poke는 딥링크의 최종 목적지가 /poke이기 때문에 콕찌르기 메인만 띄워주면 되는 상황입니다. 그 경우가 isDestination == true인 경우예요!
만약 다른 알림또한 탭을 통해서 진입을 한다면 ApplicationCoordinator 가 다른 코디네이터도 전부 소유를 하도록 하는걸까요?
이 말은 콕찌르기 외에 솝탬프나 솝트로그 등을 의미하는 걸까요? 만약 탭바에 있는 화면들이 딥링크로 도착하게 된다면 HomeCoordinator, StampCoordinator, PokeCoordinator, SoplotCoordinator 모두 ApplicationCoodinator가 소유하는 방향으로 수정되어야할 것 같습니다.
현재 구조는 탭바가 도입되기 전의 구조이고 딥링크에서는 탭바를 사용하지 않기 때문에 문제가 되지 않았지만, 콕찌르기와 같이 딥링크에서도 탭바를 통해 진입해야한다면 Coordinator.start()를 호출할 이유가 없다고 생각합니다!
![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
🤖 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
`@SOPT-iOS/Projects/Features/RootFeature/Sources/NotificationHelpers/NotificationHandler.swift`:
- Around line 94-97: The fallback deep link builder makeComponentsForEmptyLink
currently hardcodes targetTap: .poke while parsing "home/notification/detail",
which can misalign path vs tab; change it to derive the targetTap from the
parsed deepLinkData (or pass nil) when constructing DeepLinkComponents so the
tab matches the deepLinkParser.parse result (use deepLinkData's tab property if
present instead of always using .poke).
🪄 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: 4b30e4b0-e4ce-49dc-a1cb-cf41715ef2b6
📒 Files selected for processing (8)
SOPT-iOS/Projects/Features/BaseFeatureDependency/Sources/Coordinator/DeepLink/DeepLinkComponentsExecutable.swiftSOPT-iOS/Projects/Features/PokeFeature/Interface/Sources/PokeMainPresentable.swiftSOPT-iOS/Projects/Features/PokeFeature/Sources/Coordinator/LegacyPokeCoordinator.swiftSOPT-iOS/Projects/Features/PokeFeature/Sources/PokeMainScene/ViewModel/PokeMainViewModel.swiftSOPT-iOS/Projects/Features/RootFeature/Sources/ApplicationCoordinator.swiftSOPT-iOS/Projects/Features/RootFeature/Sources/DeepLinks/PokeDeepLink.swiftSOPT-iOS/Projects/Features/RootFeature/Sources/NotificationHelpers/DeepLinkComponents.swiftSOPT-iOS/Projects/Features/RootFeature/Sources/NotificationHelpers/NotificationHandler.swift
💤 Files with no reviewable changes (2)
- SOPT-iOS/Projects/Features/PokeFeature/Interface/Sources/PokeMainPresentable.swift
- SOPT-iOS/Projects/Features/PokeFeature/Sources/PokeMainScene/ViewModel/PokeMainViewModel.swift
| private func makeComponentsForEmptyLink(notificationId: String) -> DeepLinkComponents? { | ||
| let deepLinkData = try? deepLinkParser.parse(with: "home/notification/detail?id=\(notificationId)") | ||
| return DeepLinkComponents(deepLinkData: deepLinkData) | ||
| return DeepLinkComponents(deepLinkData: deepLinkData, targetTap: .poke) | ||
| } |
There was a problem hiding this comment.
빈 링크 fallback의 탭 지정이 딥링크 경로와 불일치합니다.
Line 95에서 home/notification/detail을 만들고 Line 96에서 targetTap: .poke를 고정하면, 실행 전에 포크 탭으로 강제 전환되어 경로/탭이 어긋날 수 있습니다. deepLinkData에서 계산한 탭(또는 nil)을 쓰는 방식으로 맞춰주세요.
수정 제안
private func makeComponentsForEmptyLink(notificationId: String) -> DeepLinkComponents? {
- let deepLinkData = try? deepLinkParser.parse(with: "home/notification/detail?id=\(notificationId)")
- return DeepLinkComponents(deepLinkData: deepLinkData, targetTap: .poke)
+ guard let deepLinkData = try? deepLinkParser.parse(with: "home/notification/detail?id=\(notificationId)") else {
+ return nil
+ }
+ let targetTap = resolveTargetTap(deepLinkData: deepLinkData)
+ return DeepLinkComponents(deepLinkData: deepLinkData, targetTap: targetTap)
}🤖 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
`@SOPT-iOS/Projects/Features/RootFeature/Sources/NotificationHelpers/NotificationHandler.swift`
around lines 94 - 97, The fallback deep link builder makeComponentsForEmptyLink
currently hardcodes targetTap: .poke while parsing "home/notification/detail",
which can misalign path vs tab; change it to derive the targetTap from the
parsed deepLinkData (or pass nil) when constructing DeepLinkComponents so the
tab matches the deepLinkParser.parse result (use deepLinkData's tab property if
present instead of always using .poke).
|
|
||
| public struct PokeDeepLink: DeepLinkExecutable { | ||
| public let name = "poke" | ||
| public static let path = "poke" |
There was a problem hiding this comment.
static let path로 변경하신 이유가 있나요?
2 participants
🌴 PR 요약
알림탭에서 콕찌르기 진입 시 이중 BottomSheet로 인해서 로띠가 나타나지 않는 문제를 수정합니다.
🌱 작업한 브랜치
🌱 PR Point
isRouteFromTabBar 플래그를 제거하여 기존 BottomSheet로 알림을 띄우던 방식에서 탭화면으로 띄우는 방식으로 수정했습니다.
📌 참고 사항
📸 스크린샷
📮 관련 이슈