Conversation
- Adding Firebase Analytics SDK dependency - Creating Analytics manager and use case infrastructure - Implementing analytics tracking for various user actions (login, signup, travel management, deeplinks, etc.) - Updating app configuration to support Firebase - Version bump to 1.0.2
Peter1119
reviewed
Dec 11, 2025
Contributor
Peter1119
left a comment
Peter1119
left a comment
There was a problem hiding this comment.
지금 드는 생각은
feature에서 하지 않고 많은 부분은
usecase에서 할 수도 있지 않을까라는 생각입니다.
그럼 의존성에 대한 것을 많이 줄 일 수도 있고요
| func trackExpenseDelete(travelId: String, expenseId: String, source: String) | ||
| } | ||
|
|
||
| public struct NoOpAnalyticsManager: AnalyticsManaging { |
Contributor
Author
There was a problem hiding this comment.
@Peter1119 이거 firebase 의존성 해결 때매 만들어놨는데 제거 하겠습니다
Comment on lines
+7
to
+24
| func trackDeeplinkOpen(deeplink: String, type: String) | ||
| func trackExpenseOpenDetail(travelId: String, expenseId: String, source: String) | ||
| func trackLoginSuccess(socialType: String, isFirst: Bool?) | ||
| func trackSignupSuccess(socialType: String) | ||
|
|
||
| // Travel | ||
| func trackTravelUpdate(_ travelId: String) | ||
| func trackTravelDelete(_ travelId: String) | ||
| func trackTravelLeave(travelId: String, userId: String?) | ||
| func trackTravelMemberLeave(travelId: String, memberId: String, role: String?) | ||
| func trackTravelOwnerDelegate(travelId: String, newOwnerId: String) | ||
|
|
||
| // Additional Events from CSV | ||
| func trackExpenseView(travelId: String, tab: String, expenseDate: String) | ||
| func trackExpenseCreateSuccess(travelId: String, expenseId: String, amount: Double, currency: String, category: String, payerId: String) | ||
| func trackExpenseCreateFailure(travelId: String, amount: Double, currency: String, category: String, payerId: String, errorCode: String) | ||
| func trackExpenseUpdate(travelId: String, expenseId: String, amount: Double, currency: String, category: String, payerId: String) | ||
| func trackExpenseDelete(travelId: String, expenseId: String, source: String) |
Contributor
There was a problem hiding this comment.
인터페이스가 너무 많은데
하나로 줄이고 들어가는 데이터 타입을 여러개로 할 수 있지 않을까요 ?
예를 들면 들어가는 것을 enum으로 정의하고
enum의 연관값으로 파라미터를 넣으면 되지 않을까 싶습니다 ~!
Comment on lines
+4
to
+21
| public protocol AnalyticsUseCaseProtocol: Sendable { | ||
| func trackDeeplinkOpen(deeplink: String, type: String) | ||
| func trackExpenseOpenDetail(travelId: String, expenseId: String, source: String) | ||
| func trackLoginSuccess(socialType: String, isFirst: Bool?) | ||
| func trackSignupSuccess(socialType: String) | ||
| func trackTravelUpdate(_ travelId: String) | ||
| func trackTravelDelete(_ travelId: String) | ||
| func trackTravelLeave(travelId: String, userId: String?) | ||
| func trackTravelMemberLeave(travelId: String, memberId: String, role: String?) | ||
| func trackTravelOwnerDelegate(travelId: String, newOwnerId: String) | ||
| } | ||
|
|
||
| public struct AnalyticsUseCase: AnalyticsUseCaseProtocol { | ||
| private let manager: any AnalyticsManaging | ||
|
|
||
| public init(manager: any AnalyticsManaging) { | ||
| self.manager = manager | ||
| } |
Contributor
There was a problem hiding this comment.
manager를 한번 더 감싼 의도를 알 수 있을까요 ?
Contributor
Author
There was a problem hiding this comment.
@Peter1119 현재 코드에서는 별다른 이점이 없어서 제거하는 것이 좋을 것 같습니다
Comment on lines
+192
to
+195
| return .merge( | ||
| .send(.delegate(.trackExpenseOpenDetail(travelId: travelId, expenseId: expenseId, source: "deeplink"))), | ||
| .send(.router(.routeAction(id: routeIndex, action: .settlementCoordinator(.navigateToExpenseTab(expenseId))))) | ||
| ) |
Contributor
There was a problem hiding this comment.
delegate는 상위에 요청하는 것이고 scope는 자식에게 요청할때 쓰는 단어가 아니었나요 ?
그리고 coordinator에서 tracking요청에 대한 action을 주입하는건 너무 과한 것 같아요
DelegateAction의 정의도 달라지는게 좋겟네요 .
…/firebaseEvent # Conflicts: # Features/Travel/Sources/Reducer/MemberSettingFeature.swift
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.
🔗 관련 이슈
✨ 작업 내용
📝 참고 사항
Motivation 🥳 (코드를 추가/변경하게 된 이유)
Key Changes 🔥 (주요 구현/변경 사항)
// 로그인 성공 이벤트
trackLoginSuccess(socialType: "google", isFirst: false)
// 회원가입 성공 이벤트
trackSignupSuccess(socialType: "apple")
// 여행 관련 이벤트
trackTravelUpdate(travelId)
trackTravelDelete(travelId)
#if DEBUG
setenv("FIRAnalyticsDebugEnabled", "1", 1)
setenv("FIRDebugEnabled", "1", 1)
#endif
To Reviewers 🙏 (리뷰어에게 전달하고 싶은 말)
Reference 🔗
Close Issues 🔒 (닫을 Issue)