[feat] #192 Firebase Analytics(GA4) 이벤트 로깅 추가#195
Open
[feat] #192 Firebase Analytics(GA4) 이벤트 로깅 추가#195
Conversation
Walkthrough프로젝트에 Changes
Sequence Diagram(s)sequenceDiagram
participant User as 사용자
participant App as 앱 (MainActivity)
participant VM as ViewModel (Archive/Map/Pose)
participant Logger as AnalyticsLogger (FirebaseAnalyticsLogger)
participant Firebase as Firebase Analytics SDK
User->>App: 앱 실행
App->>Logger: setUserProperty("platform","android")
Logger->>Firebase: setUserProperty(...)
App->>Logger: setUserProperty("app_version", VERSION_NAME)
Logger->>Firebase: setUserProperty(...)
User->>VM: 화면 진입
VM->>Logger: log(ScreenViewEvent)
Logger->>Firebase: logEvent(name, params)
Firebase-->>Logger: 응답
User->>VM: 사용자 동작 (예: 앨범 생성, 필터 토글)
VM->>Logger: log(ActionEvent with params)
Logger->>Firebase: logEvent(name, params)
Firebase-->>Logger: 응답
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
feature/auth/impl/src/main/kotlin/com/neki/android/feature/auth/impl/login/LoginViewModel.kt (1)
63-80:⚠️ Potential issue | 🔴 Critical수동 로그인 경로에서
app_open이벤트 로깅 누락.현재 코드에서는 자동 로그인 시(SplashViewModel.kt line 106)에만
analyticsLogger.log(GlobalAnalyticsEvent.AppOpen)이 기록되고 있으며, 수동 로그인(Kakao) 완료 후 Main으로 네비게이션하는 경로(line 71)에서는 이벤트가 기록되지 않고 있습니다. PR 설명의 "로그인/자동 로그인 완료 시점에 로깅"이라는 요구사항에 따르면, 두 로그인 경로에서 모두app_open이벤트를 기록해야 합니다.checkTermAgreementState()에서 Main으로 네비게이션할 때(또는 그 이후) AppOpen 로깅을 추가하시기 바랍니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/auth/impl/src/main/kotlin/com/neki/android/feature/auth/impl/login/LoginViewModel.kt` around lines 63 - 80, In checkTermAgreementState(), ensure the app_open event is logged on the manual login path: after retrieving userInfo (userRepository.getUserInfo()) and setting analyticsLogger.setUserId(userInfo.id.toString()) and authRepository.setCompletedOnboarding(true), call analyticsLogger.log(GlobalAnalyticsEvent.AppOpen) before posting LoginSideEffect.NavigateToMain so the AppOpen event is emitted for the manual (Kakao) login flow; also add the same analyticsLogger.log(GlobalAnalyticsEvent.AppOpen) in the onFailure branch only if you still navigate to Main there (or keep it out if you always navigate to Term), referencing checkTermAgreementState, analyticsLogger, GlobalAnalyticsEvent.AppOpen, and LoginSideEffect.NavigateToMain.feature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapViewModel.kt (1)
65-84:⚠️ Potential issue | 🟠 Major초기 검색 기준점이 없어 첫 재검색의
region_changed가 항상 false가 됩니다.
lastSearchCenter는 새로고침 이후에만 저장됩니다. 초기/자동 지도 로드 후 사용자가 멀리 이동해서 첫 재검색을 눌러도 Line 410에서 바로false가 반환되어map_re_search.region_changed가 실제 이동 여부를 반영하지 못합니다. 지도 영역을 로드할 때도 현재 검색 중심을 기준점으로 저장해 주세요.🛠️ 제안 변경 예시
is MapIntent.LoadPhotoBoothsByBounds -> { + lastSearchCenter = intent.mapBounds.center() loadPhotoBoothsByPolygon(intent.mapBounds, state, reduce, postSideEffect) }+private fun MapBounds.center(): LocLatLng = LocLatLng( + latitude = (northWest.latitude + southEast.latitude) / 2.0, + longitude = (northWest.longitude + southEast.longitude) / 2.0, +) + private fun isRegionChanged(currentCenter: LocLatLng, zoomLevel: Double): Boolean { val prev = lastSearchCenter ?: return falseAlso applies to: 409-419
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapViewModel.kt` around lines 65 - 84, The metric is wrong because lastSearchCenter is only set after manual refresh; update the initial load path so lastSearchCenter is initialized when the map first loads: in the MapIntent.LoadPhotoBoothsByBounds handling (and/or inside loadPhotoBoothsByPolygon), compute the current search center from intent.mapBounds (or from the same center calculation used by isRegionChanged) and assign it to lastSearchCenter before returning/dispatching the load; ensure the same assignment occurs for automatic/initial map load flows so isRegionChanged compares against a proper baseline.
🧹 Nitpick comments (2)
feature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapContract.kt (1)
37-37:ClickRefreshButton에 파생 가능한 카메라 상태가 다수 포함됨 — 도메인 응집도 저하 우려.
mapBounds,center,zoomLevel이 모두CameraPosition에서 파생되는 값이라 Intent 페이로드가 비대해지고 추후 유지보수 시 세 값의 일관성을 호출부가 보장해야 합니다. 카메라 상태를 하나의 VO(예:CameraSnapshot(bounds, center, zoom))로 감싸 Intent에 싣는 리팩터가 테스트/확장성 측면에서 유리합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapContract.kt` at line 37, ClickRefreshButton currently carries three separate, derivable camera fields (mapBounds, center, zoomLevel) which reduces domain cohesion; create a single value object (e.g., CameraSnapshot) that encapsulates MapBounds, LocLatLng center and zoomLevel (or derive from CameraPosition) and replace the ClickRefreshButton payload to accept that VO instead (update the MapIntent definition, any factory/constructors that create ClickRefreshButton, and usages that read mapBounds/center/zoomLevel to use CameraSnapshot.getBounds()/getCenter()/getZoom()).feature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapViewModel.kt (1)
196-211: Reducer 내부의 analytics side effect는 밖으로 빼는 편이 좋습니다.
reduce {}는 상태 변환만 담당하도록 두면 재실행/테스트/스토어 구현 변경 시 중복 로그 위험을 줄일 수 있습니다.updatedBrands와 이벤트 값을 reducer 밖에서 계산한 뒤 한 번만 log하도록 분리해 주세요.♻️ 리팩터링 방향 예시
-private fun handleClickBrand( - clickedBrand: Brand, - reduce: (MapState.() -> MapState) -> Unit, -) { - reduce { - val updatedBrands = brands.map { brand -> +private fun handleClickBrand( + clickedBrand: Brand, + state: MapState, + reduce: (MapState.() -> MapState) -> Unit, +) { + val updatedBrands = state.brands.map { brand -> + if (brand == clickedBrand) { + brand.copy(isChecked = !brand.isChecked) + } else { + brand + } + } + val checkedBrandNames = updatedBrands.filter { it.isChecked }.map { it.name } + + analyticsLogger.log( + MapAnalyticsEvent.MapBrandFilterToggle( + action = if (clickedBrand.isChecked) "deselect" else "select", + selectedCount = updatedBrands.count { it.isChecked }, + brandName = clickedBrand.name, + ), + ) + + reduce { + copy( + brands = updatedBrands.toImmutableList(), + mapMarkers = mapMarkers.map { photoBooth -> + photoBooth.copy( + isCheckedBrand = checkedBrandNames.isEmpty() || photoBooth.brandName in checkedBrandNames, + ) + }.toImmutableList(), + nearbyPhotoBooths = nearbyPhotoBooths.map { photoBooth -> + photoBooth.copy( + isCheckedBrand = checkedBrandNames.isEmpty() || photoBooth.brandName in checkedBrandNames, + ) + }.toImmutableList(), + ) + } - if (brand == clickedBrand) { - brand.copy(isChecked = !brand.isChecked) - } else { - brand - } - } - val checkedBrandNames = updatedBrands.filter { it.isChecked }.map { it.name } - analyticsLogger.log(...) - copy(...) - } }호출부도
handleClickBrand(intent.brand, state, reduce)로 맞춰야 합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapViewModel.kt` around lines 196 - 211, The analytics side effect inside the reduce block should be removed: compute updatedBrands and the MapAnalyticsEvent.MapBrandFilterToggle payload (including action, selectedCount, brandName) before calling reduce, then call reduce only to update state (using the updatedBrands) and immediately after call analyticsLogger.log once with the precomputed event; update the handler to use handleClickBrand(intent.brand, state, reduce) so the reducer does pure state transformation and logging happens externally (refer to symbols: reduce, updatedBrands, clickedBrand, analyticsLogger.log, MapAnalyticsEvent.MapBrandFilterToggle, handleClickBrand).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@core/analytics/src/main/kotlin/com/neki/android/core/analytics/event/AnalyticsEvent.kt`:
- Around line 3-8: Change AnalyticsEvent.params from Map<String, String> to
Map<String, Any> (update the sealed interface AnalyticsEvent accordingly) and
remove any .toString() uses in event implementations (e.g., totalSwipeCount,
count) so numeric values remain numeric; then update FirebaseAnalyticsLogger to
inspect each param value's runtime type and call the appropriate Firebase method
(putLong/putDouble/putString/etc.) for keys/values instead of always using
putString, handling nulls and booleans as appropriate.
In
`@core/analytics/src/main/kotlin/com/neki/android/core/analytics/logger/AnalyticsLogger.kt`:
- Around line 5-9: Add explicit clear/reset APIs to AnalyticsLogger so caller
can remove user id and properties instead of relying on nullable setters; update
the interface (AnalyticsLogger) to include either clearUserId() and
clearUserProperties() or a single reset() method, and implement these new
methods in the concrete analytics adapter to call
FirebaseAnalytics.resetAnalyticsData() (or equivalent) under the hood to
guarantee previous user identifiers/properties are cleared on logout/account
switch.
In
`@core/analytics/src/main/kotlin/com/neki/android/core/analytics/logger/FirebaseAnalyticsLogger.kt`:
- Around line 13-15: The Bundle construction in FirebaseAnalyticsLogger
currently forces all event params to strings; change the AnalyticsEvent.params
contract to Map<String, Any?> (or similar) and update the Bundle population in
FirebaseAnalyticsLogger (where you build the Bundle and call
FirebaseAnalytics.logEvent) to inspect each value and call the appropriate
Bundle.putString/putLong/putDouble (or skip nulls) so numeric counters like
count/album_count/selected_count/total_swipe_count are sent as long/double
rather than strings; ensure the updated param map type is reflected wherever
AnalyticsEvent.params is implemented.
In
`@feature/archive/impl/src/main/kotlin/com/neki/android/feature/archive/impl/album_detail/AlbumDetailViewModel.kt`:
- Line 316: In AlbumDetailViewModel locate the success branch that currently
calls analyticsLogger.log(ArchiveAnalyticsEvent.PhotoCopy) (the import/use of
ArchiveAnalyticsEvent in the photo-add-from-detail success flow) and replace
that event with ArchiveAnalyticsEvent.AlbumAddFromDetail so the
import-from-detail flow is logged correctly; ensure any surrounding context (the
success handler or method in AlbumDetailViewModel that handles adding photos
from the bottom sheet) still compiles and that no other PhotoCopy occurrences
are unintentionally changed.
In
`@feature/archive/impl/src/main/kotlin/com/neki/android/feature/archive/impl/main/ArchiveMainScreen.kt`:
- Around line 64-66: The current LaunchedEffect(Unit) in ArchiveMainScreen is
only run on first composition so viewModel.logArchivingView() won't fire when
returning to the tab; replace that LaunchedEffect usage with
LifecycleResumeEffect(Unit) so the effect runs on each lifecycle resume (tab
return) — update the call site in ArchiveMainScreen where LaunchedEffect(Unit)
wraps viewModel.logArchivingView() to use LifecycleResumeEffect(Unit) in the
same pattern used in MapScreen.kt.
In
`@feature/archive/impl/src/main/kotlin/com/neki/android/feature/archive/impl/photo_detail/PhotoDetailViewModel.kt`:
- Around line 205-210: In PhotoDetailViewModel.saveMemo (where
photoRepository.updateMemo is invoked), change analytics logging so
PhotoMemoCreate is emitted only for true creations (oldMemo.isBlank() &&
newMemo.isNotBlank()); for other cases (edit or delete) emit a different event
such as PhotoMemoSave or PhotoMemoUpdate via analyticsLogger.log, and keep
posting the existing side effect
(postSideEffect(PhotoDetailSideEffect.NotifyPhotoUpdated)).
In
`@feature/auth/impl/src/main/kotlin/com/neki/android/feature/auth/impl/login/LoginViewModel.kt`:
- Around line 66-68: 자동 로그인 경로과 약관 전환 시 User-ID가 누락되는 문제: 호출 지점인
SplashViewModel.fetchAuthState()의 자동 로그인 성공 분기와
LoginViewModel.checkTermAgreementState()에서 userRepository.getUserInfo() 실패 분기
모두에 analyticsLogger.setUserId(userInfo.id.toString())를 보장하도록 수정하세요; 구체적으로
LoginViewModel의 userRepository.getUserInfo().onSuccess {
analyticsLogger.setUserId(...) } 외에 자동 로그인 성공
콜백(SplashViewModel.fetchAuthState())에도 동일한 setUserId 호출을 추가하고,
LoginViewModel.checkTermAgreementState()의 getUserInfo 실패 경로에는 간단한 재시도(예: 1회 재조회)
로직을 넣어 재조회 후 성공 시 setUserId를 호출한 뒤 약관 화면으로 네비게이션하도록 구현하세요.
In
`@feature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapScreen.kt`:
- Around line 71-73: LaunchedEffect(Unit) in MapScreen currently runs only once
at initial composition, so viewModel.logMapView() won't fire on tab return;
replace the LaunchedEffect(Unit) usage with a LifecycleResumeEffect (or
equivalent lifecycle-aware effect) that calls viewModel.logMapView() on each
onResume, ensuring the "_view" event is recorded every time the tab is resumed.
Locate MapScreen and replace the LaunchedEffect(Unit) block that invokes
logMapView() with a LifecycleResumeEffect calling viewModel.logMapView(); apply
the same change to the corresponding screens in feature/pose and feature/archive
where LaunchedEffect(Unit) is used.
In
`@feature/pose/impl/src/main/java/com/neki/android/feature/pose/impl/main/PoseScreen.kt`:
- Around line 61-63: LaunchedEffect(Unit) only runs on first composition so
viewModel.logPoseView() is recorded once; because the Pose tab is kept by
rememberSaveableStateHolderNavEntryDecorator you must trigger logging on each
tab return instead. Update the logic to depend on the current tab key (replace
LaunchedEffect(Unit) with LaunchedEffect(currentTabKey) or similar observable
that changes when the Pose tab becomes active) or move the call to the tab
selection handler so viewModel.logPoseView() is invoked whenever the Pose tab is
selected.
In
`@feature/pose/impl/src/main/java/com/neki/android/feature/pose/impl/random/RandomPoseViewModel.kt`:
- Around line 94-97: 현재 포즈가 없을 때도 이벤트가 기록되는 버그가 있으니
RandomPoseIntent.ClickBookmarkIcon 처리에서
analyticsLogger.log(PoseAnalyticsEvent.PoseBookmark)를 state.currentPose ?:
return 체크 뒤로 옮겨 실제 currentPose가 존재할 때만 호출하도록 변경하고, 기존의
handleBookmarkToggle(currentPost.id, !currentPost.isBookmarked, reduce) 호출 흐름은
그대로 유지하여 로그는 currentPost 변수(또는 state.currentPose) 조회 이후에만 기록되게 하세요.
---
Outside diff comments:
In
`@feature/auth/impl/src/main/kotlin/com/neki/android/feature/auth/impl/login/LoginViewModel.kt`:
- Around line 63-80: In checkTermAgreementState(), ensure the app_open event is
logged on the manual login path: after retrieving userInfo
(userRepository.getUserInfo()) and setting
analyticsLogger.setUserId(userInfo.id.toString()) and
authRepository.setCompletedOnboarding(true), call
analyticsLogger.log(GlobalAnalyticsEvent.AppOpen) before posting
LoginSideEffect.NavigateToMain so the AppOpen event is emitted for the manual
(Kakao) login flow; also add the same
analyticsLogger.log(GlobalAnalyticsEvent.AppOpen) in the onFailure branch only
if you still navigate to Main there (or keep it out if you always navigate to
Term), referencing checkTermAgreementState, analyticsLogger,
GlobalAnalyticsEvent.AppOpen, and LoginSideEffect.NavigateToMain.
In
`@feature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapViewModel.kt`:
- Around line 65-84: The metric is wrong because lastSearchCenter is only set
after manual refresh; update the initial load path so lastSearchCenter is
initialized when the map first loads: in the MapIntent.LoadPhotoBoothsByBounds
handling (and/or inside loadPhotoBoothsByPolygon), compute the current search
center from intent.mapBounds (or from the same center calculation used by
isRegionChanged) and assign it to lastSearchCenter before returning/dispatching
the load; ensure the same assignment occurs for automatic/initial map load flows
so isRegionChanged compares against a proper baseline.
---
Nitpick comments:
In
`@feature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapContract.kt`:
- Line 37: ClickRefreshButton currently carries three separate, derivable camera
fields (mapBounds, center, zoomLevel) which reduces domain cohesion; create a
single value object (e.g., CameraSnapshot) that encapsulates MapBounds,
LocLatLng center and zoomLevel (or derive from CameraPosition) and replace the
ClickRefreshButton payload to accept that VO instead (update the MapIntent
definition, any factory/constructors that create ClickRefreshButton, and usages
that read mapBounds/center/zoomLevel to use
CameraSnapshot.getBounds()/getCenter()/getZoom()).
In
`@feature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapViewModel.kt`:
- Around line 196-211: The analytics side effect inside the reduce block should
be removed: compute updatedBrands and the MapAnalyticsEvent.MapBrandFilterToggle
payload (including action, selectedCount, brandName) before calling reduce, then
call reduce only to update state (using the updatedBrands) and immediately after
call analyticsLogger.log once with the precomputed event; update the handler to
use handleClickBrand(intent.brand, state, reduce) so the reducer does pure state
transformation and logging happens externally (refer to symbols: reduce,
updatedBrands, clickedBrand, analyticsLogger.log,
MapAnalyticsEvent.MapBrandFilterToggle, handleClickBrand).
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 567d037a-d6cd-447d-9748-dd12683f1376
📒 Files selected for processing (31)
app/build.gradle.ktsapp/src/main/java/com/neki/android/app/MainActivity.ktbuild-logic/src/main/java/com/neki/android/buildlogic/plugins/AndroidFeatureImplConventionPlugin.ktcore/analytics/build.gradle.ktscore/analytics/src/main/kotlin/com/neki/android/core/analytics/event/AnalyticsEvent.ktcore/analytics/src/main/kotlin/com/neki/android/core/analytics/event/ArchiveAnalyticsEvent.ktcore/analytics/src/main/kotlin/com/neki/android/core/analytics/event/GlobalAnalyticsEvent.ktcore/analytics/src/main/kotlin/com/neki/android/core/analytics/event/MapAnalyticsEvent.ktcore/analytics/src/main/kotlin/com/neki/android/core/analytics/event/PoseAnalyticsEvent.ktcore/analytics/src/main/kotlin/com/neki/android/core/analytics/logger/AnalyticsLogger.ktcore/analytics/src/main/kotlin/com/neki/android/core/analytics/logger/AnalyticsModule.ktcore/analytics/src/main/kotlin/com/neki/android/core/analytics/logger/FirebaseAnalyticsLogger.ktfeature/archive/impl/src/main/kotlin/com/neki/android/feature/archive/impl/album/AllAlbumViewModel.ktfeature/archive/impl/src/main/kotlin/com/neki/android/feature/archive/impl/album_detail/AlbumDetailViewModel.ktfeature/archive/impl/src/main/kotlin/com/neki/android/feature/archive/impl/main/ArchiveMainScreen.ktfeature/archive/impl/src/main/kotlin/com/neki/android/feature/archive/impl/main/ArchiveMainViewModel.ktfeature/archive/impl/src/main/kotlin/com/neki/android/feature/archive/impl/photo_detail/PhotoDetailViewModel.ktfeature/auth/impl/src/main/kotlin/com/neki/android/feature/auth/impl/login/LoginViewModel.ktfeature/auth/impl/src/main/kotlin/com/neki/android/feature/auth/impl/splash/SplashViewModel.ktfeature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapContract.ktfeature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapScreen.ktfeature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapViewModel.ktfeature/map/impl/src/main/java/com/neki/android/feature/map/impl/const/MapConst.ktfeature/photo-upload/impl/build.gradle.ktsfeature/pose/impl/src/main/java/com/neki/android/feature/pose/impl/detail/PoseDetailViewModel.ktfeature/pose/impl/src/main/java/com/neki/android/feature/pose/impl/main/PoseScreen.ktfeature/pose/impl/src/main/java/com/neki/android/feature/pose/impl/main/PoseViewModel.ktfeature/pose/impl/src/main/java/com/neki/android/feature/pose/impl/random/RandomPoseViewModel.ktfeature/select-album/impl/src/main/java/com/neki/android/feature/select_album/impl/SelectAlbumViewModel.ktgradle/libs.versions.tomlsettings.gradle.kts
Comment on lines
+5
to
+9
| interface AnalyticsLogger { | ||
| fun log(event: AnalyticsEvent) | ||
| fun setUserId(userId: String) | ||
| fun setUserProperty(key: String, value: String) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
FirebaseAnalytics Android setUserId null clear user id setUserProperty null clear property
💡 Result:
In Firebase Analytics for Android, setting setUserId(null) or setUserProperty(name, null) does not reliably clear the user ID or user properties in practice, despite documentation implying it should (e.g., setting null removes them). This is a known issue persisting as of 2025, particularly visible in DebugView where old values linger even if backend clears them. The reliable way to clear both user ID and user properties is to call FirebaseAnalytics.getInstance(context).resetAnalyticsData. This method "clears all analytics data for this app from the device and resets the app instance id", explicitly including resetting user ID and user properties (confirmed by Firebase team in 2024). After reset: - Optionally re-enable collection with setAnalyticsCollectionEnabled(true) if disabled. - A new app instance ID and session start automatically. - Do not need to set null on properties afterward. Code example: FirebaseAnalytics firebaseAnalytics = FirebaseAnalytics.getInstance(this); firebaseAnalytics.resetAnalyticsData; // Optional: firebaseAnalytics.setAnalyticsCollectionEnabled(true); Note: Official docs do not explicitly state null clears them for Android (examples show setting strings), and GitHub issues (#3602 open since 2022) report failures. Use resetAnalyticsData for privacy/log out scenarios.
Citations:
- 1: Firebase Analytics: Setting userID to null does not clear the userID firebase/firebase-android-sdk#3602
- 2: https://stackoverflow.com/questions/53077408/android-firebase-analytics-not-able-to-set-null-to-userid-and-userproperty
- 3: https://stackoverflow.com/questions/66619844/firebase-setting-user-property-to-null-or-blank-value-not-deleting-the-userprop
- 4: How do I clear or reset the setUserProperties when a user logs out? firebase/firebase-android-sdk#5426
- 5: https://firebase.google.com/docs/reference/android/com/google/firebase/analytics/FirebaseAnalytics
- 6: https://firebase.google.com/docs/reference/kotlin/com/google/firebase/analytics/FirebaseAnalytics
- 7: Analytics: Does resetAnalyticsData reset user ID and user properties? firebase/firebase-android-sdk#6107
- 8: https://firebase-dot-devsite-v2-prod.appspot.com/docs/reference/android/com/google/firebase/analytics/FirebaseAnalytics
사용자 식별자와 속성을 지울 수 있는 API를 추가하세요.
로그아웃/계정 전환 시 사용자 ID와 속성을 명시적으로 해제해야 하는 것은 맞습니다. 다만, 단순히 String?로 변경하는 것만으로는 문제가 해결되지 않습니다. Firebase Analytics Android에서 setUserId(null) 또는 setUserProperty(key, null)을 호출해도 실제로 값이 지워지지 않는 것이 알려진 문제입니다(GitHub issue #3602, 2022년부터 미해결). Firebase에서는 resetAnalyticsData()를 호출하는 것을 권장합니다.
대신 다음 중 하나로 구현하세요:
- 명시적인 clear/reset 메서드 추가:
fun clearUserId(),fun clearUserProperties() - 또는 전체 데이터 초기화:
fun reset()
이러한 메서드들이 내부적으로 Firebase의 resetAnalyticsData()를 호출하도록 구현하면, 로그아웃 시 이전 사용자의 식별자가 남지 않습니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@core/analytics/src/main/kotlin/com/neki/android/core/analytics/logger/AnalyticsLogger.kt`
around lines 5 - 9, Add explicit clear/reset APIs to AnalyticsLogger so caller
can remove user id and properties instead of relying on nullable setters; update
the interface (AnalyticsLogger) to include either clearUserId() and
clearUserProperties() or a single reset() method, and implement these new
methods in the concrete analytics adapter to call
FirebaseAnalytics.resetAnalyticsData() (or equivalent) under the hood to
guarantee previous user identifiers/properties are cleared on logout/account
switch.
| photoIds = state.importPhotoState.selectedPhotoIds.toList(), | ||
| targetFolderIds = listOf(albumId), | ||
| ).onSuccess { | ||
| analyticsLogger.log(ArchiveAnalyticsEvent.PhotoCopy) |
There was a problem hiding this comment.
앨범 상세 추가 플로우는 PhotoCopy가 아니라 AlbumAddFromDetail로 기록해야 합니다.
Line 316은 가져오기 바텀시트에서 현재 앨범에 사진을 추가하는 성공 처리입니다. 이 플로우를 photo_copy로 기록하면 복사 액션 지표가 부풀려지고, album_add_from_detail 지표가 누락됩니다.
수정안
-analyticsLogger.log(ArchiveAnalyticsEvent.PhotoCopy)
+analyticsLogger.log(ArchiveAnalyticsEvent.AlbumAddFromDetail)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| analyticsLogger.log(ArchiveAnalyticsEvent.PhotoCopy) | |
| analyticsLogger.log(ArchiveAnalyticsEvent.AlbumAddFromDetail) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@feature/archive/impl/src/main/kotlin/com/neki/android/feature/archive/impl/album_detail/AlbumDetailViewModel.kt`
at line 316, In AlbumDetailViewModel locate the success branch that currently
calls analyticsLogger.log(ArchiveAnalyticsEvent.PhotoCopy) (the import/use of
ArchiveAnalyticsEvent in the photo-add-from-detail success flow) and replace
that event with ArchiveAnalyticsEvent.AlbumAddFromDetail so the
import-from-detail flow is logged correctly; ensure any surrounding context (the
success handler or method in AlbumDetailViewModel that handles adding photos
from the bottom sheet) still compiles and that no other PhotoCopy occurrences
are unintentionally changed.
Comment on lines
66
to
+68
| userRepository.getUserInfo() | ||
| .onSuccess { userInfo -> | ||
| analyticsLogger.setUserId(userInfo.id.toString()) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP -C3 '\.setUserId\s*\(' --type=kotlinRepository: YAPP-Github/27th-App-Team-2-Android
Length of output: 1952
🏁 Script executed:
cat -n feature/auth/impl/src/main/kotlin/com/neki/android/feature/auth/impl/login/LoginViewModel.kt | head -100Repository: YAPP-Github/27th-App-Team-2-Android
Length of output: 3747
🏁 Script executed:
rg -n 'setUserId' --type=kotlinRepository: YAPP-Github/27th-App-Team-2-Android
Length of output: 654
🏁 Script executed:
fd -type f -name '*SplashViewModel*' -o -name '*AuthViewModel*' -o -name '*AutoLogin*' | grep -E '\.kt$'Repository: YAPP-Github/27th-App-Team-2-Android
Length of output: 249
🏁 Script executed:
fd -type f -name '*Splash*' -o -name '*Onboard*' -o -name '*App*' | grep -i '\.kt$'Repository: YAPP-Github/27th-App-Team-2-Android
Length of output: 249
🏁 Script executed:
rg -n 'getUserInfo\|setUserId' --type=kotlin -B2 -A2 | head -60Repository: YAPP-Github/27th-App-Team-2-Android
Length of output: 61
🏁 Script executed:
fd -type f -name '*MainActivity*' -o -name '*RootActivity*' | grep '\.kt$'Repository: YAPP-Github/27th-App-Team-2-Android
Length of output: 249
🏁 Script executed:
find . -name '*Activity.kt' -o -name '*Splash*' -o -name '*Root*' | head -20Repository: YAPP-Github/27th-App-Team-2-Android
Length of output: 984
🏁 Script executed:
rg -n 'NavigateToTerm|checkTermAgreementState' --type kotlinRepository: YAPP-Github/27th-App-Team-2-Android
Length of output: 997
🏁 Script executed:
find . -path '*/app/src/main/kotlin/*' -name '*.kt' | grep -i activity | head -10Repository: YAPP-Github/27th-App-Team-2-Android
Length of output: 61
🏁 Script executed:
cat -n feature/auth/impl/src/main/kotlin/com/neki/android/feature/auth/impl/splash/SplashViewModel.ktRepository: YAPP-Github/27th-App-Team-2-Android
Length of output: 7201
🏁 Script executed:
cat -n app/src/main/java/com/neki/android/app/MainActivity.ktRepository: YAPP-Github/27th-App-Team-2-Android
Length of output: 8291
setUserId 미설정 경로에서의 분석 데이터 일관성 문제 확인.
자동 로그인 시 SplashViewModel.fetchAuthState()에서 토큰 갱신 후 메인으로 이동하는 경로에서 setUserId()가 호출되지 않습니다. 또한 LoginViewModel.checkTermAgreementState()에서 getUserInfo() 실패 시 User-ID 설정 없이 약관 화면으로 진행됩니다.
결과적으로:
- 자동 로그인 시 로그되는 이벤트들이 User-ID 없이 기록
- 약관 화면에서의 이벤트들이 User-ID 없이 기록
- 수동 로그인 성공 시에만 User-ID 설정되어 세션 간 사용자 식별 불일치 발생
setUserId()를 자동 로그인 성공 시점과 실패 시 재시도 메커니즘에 추가하여 모든 이벤트가 일관된 User-ID로 추적되도록 개선이 필요합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@feature/auth/impl/src/main/kotlin/com/neki/android/feature/auth/impl/login/LoginViewModel.kt`
around lines 66 - 68, 자동 로그인 경로과 약관 전환 시 User-ID가 누락되는 문제: 호출 지점인
SplashViewModel.fetchAuthState()의 자동 로그인 성공 분기와
LoginViewModel.checkTermAgreementState()에서 userRepository.getUserInfo() 실패 분기
모두에 analyticsLogger.setUserId(userInfo.id.toString())를 보장하도록 수정하세요; 구체적으로
LoginViewModel의 userRepository.getUserInfo().onSuccess {
analyticsLogger.setUserId(...) } 외에 자동 로그인 성공
콜백(SplashViewModel.fetchAuthState())에도 동일한 setUserId 호출을 추가하고,
LoginViewModel.checkTermAgreementState()의 getUserInfo 실패 경로에는 간단한 재시도(예: 1회 재조회)
로직을 넣어 재조회 후 성공 시 setUserId를 호출한 뒤 약관 화면으로 네비게이션하도록 구현하세요.
ikseong00
approved these changes
Apr 19, 2026
Contributor
ikseong00
left a comment
ikseong00
left a comment
There was a problem hiding this comment.
확인했습니다!
붙인 곳이 많은데 고생하셨습니다.
코멘트 확인해주세요!
| implementation(libs.androidx.camera.camera2) | ||
| implementation(libs.androidx.camera.lifecycle) | ||
| implementation(libs.androidx.camera.compose) | ||
| implementation(libs.guava) |
Contributor
There was a problem hiding this comment.
요 의존성은 쓰이는 부분이 없는 것 같습니다!
| is SelectAlbumAction.CopyPhotos -> { | ||
| analyticsLogger.log(ArchiveAnalyticsEvent.PhotoCopy) | ||
| if (action.photoIds.size == 1) { | ||
| analyticsLogger.log(ArchiveAnalyticsEvent.AlbumAddFromDetail(albumCount = albums.size)) |
Contributor
There was a problem hiding this comment.
음, 이 이벤트는 PhotoDetail 에서 왔을 경우에 추가되는 이벤트인데,
AlbumDetail 에서 사진을 1개만 선택하고 넘어왔을 때도 이 케이스로 흘러갈 것 같아요!
SelectAlbumAction 에서 특정 flag 를 추가하는 건 어떨까요??
사실 withShowToast 가 PhotoDetail 에서 왔을 때에만 false 가 되어서, 이 불린 변수에 따른 처리를 하면 어떨까 합니다!
ex)
@Serializable
data class CopyPhotos(
val photoIds: List<Long>,
val fromDetail: Boolean, // 신규
val withShowToast: Boolean = true, // 기존
) : SelectAlbumAction
Comment on lines
+205
to
+211
| analyticsLogger.log( | ||
| MapAnalyticsEvent.MapBrandFilterToggle( | ||
| action = if (clickedBrand.isChecked) "deselect" else "select", | ||
| selectedCount = updatedBrands.count { it.isChecked }, | ||
| brandName = clickedBrand.name, | ||
| ), | ||
| ) |
Contributor
There was a problem hiding this comment.
reduce 블럭 밖으로 빼는건 어떨까요??
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
feature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapViewModel.kt (1)
265-287:⚠️ Potential issue | 🟡 MinorMapRouteClick 로그가 가드 실패 경로에서도 남습니다.
현재
MapRouteClick은 함수 진입 직후 무조건 기록되지만, 그 뒤에서state.currentLocLatLng == null이면 토스트만 띄우고 리턴하고(L275-278), 포커스된 부스가 없으면LaunchDirectionApp효과가 실행되지 않습니다(L279-287). 결과적으로 실제로는 외부 지도 앱이 실행되지 않은 클릭도map_route_click으로 집계되어 지표가 부풀려질 수 있습니다. 로깅 시점을LaunchDirectionApp을 실제로 post하는 시점(또는 가드 통과 이후)으로 옮기는 것을 권장드립니다.♻️ 제안 변경
- analyticsLogger.log( - MapAnalyticsEvent.MapRouteClick( - mapType = when (app) { - DirectionApp.KAKAO_MAP -> "kakao_map" - DirectionApp.NAVER_MAP -> "naver_map" - DirectionApp.GOOGLE_MAP -> "google_map" - }, - ), - ) reduce { copy(isShowDirectionBottomSheet = false) } if (state.currentLocLatLng == null) { postSideEffect(MapEffect.ShowToastMessage("현재 위치를 가져올 수 없습니다.")) return } state.mapMarkers.find { it.isFocused }?.let { focusedPhotoBooth -> + analyticsLogger.log( + MapAnalyticsEvent.MapRouteClick( + mapType = when (app) { + DirectionApp.KAKAO_MAP -> "kakao_map" + DirectionApp.NAVER_MAP -> "naver_map" + DirectionApp.GOOGLE_MAP -> "google_map" + }, + ), + ) postSideEffect( MapEffect.LaunchDirectionApp( app = app, startLocLatLng = state.currentLocLatLng, endLocLatLng = LocLatLng(focusedPhotoBooth.latitude, focusedPhotoBooth.longitude), ), ) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapViewModel.kt` around lines 265 - 287, Move the MapRouteClick analytics log so it only records when the direction flow actually proceeds: after checking state.currentLocLatLng != null and after finding a focused marker, just before calling postSideEffect(MapEffect.LaunchDirectionApp(...)). Specifically, remove the early analyticsLogger.log(MapAnalyticsEvent.MapRouteClick(...)) at the top of the function and insert the same analyticsLogger.log call immediately prior to postSideEffect(MapEffect.LaunchDirectionApp(...)) (i.e., after the null guard on state.currentLocLatLng and inside the mapMarkers.find { it.isFocused }?.let { ... } block), leaving other behavior like reduce { copy(isShowDirectionBottomSheet = false) } and the toast post on guard failure unchanged.
🧹 Nitpick comments (1)
feature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapViewModel.kt (1)
410-420: 최초 재검색 시region_changed의미 확인 및 임계값 상수화 제안.
lastSearchCenter가 null인 첫 재검색에서는 항상false가 반환됩니다. GA4 이벤트 스펙상 "처음 재검색은 region_changed=false"로 해석해도 되는지, 혹은 "이전 검색이 없으면 이벤트 자체를 생략/true 처리"해야 하는지 노션 이벤트 설계 문서와 한 번 맞춰봐 주세요.- 300/500/700/1000, 18/16/14 같은 상수는 PR 설명에 언급된 줌 임계값과 일치하지만, 현재는 매직 넘버로 흩어져 있어 향후 튜닝 시 누락되기 쉽습니다.
MapConst또는 이 ViewModel 상단의companion object로 추출하면 조정·테스트가 수월해집니다.♻️ 참고용 리팩터링 예시
+ companion object { + private const val REGION_CHANGE_THRESHOLD_ZOOM_18 = 300 + private const val REGION_CHANGE_THRESHOLD_ZOOM_16 = 500 + private const val REGION_CHANGE_THRESHOLD_ZOOM_14 = 700 + private const val REGION_CHANGE_THRESHOLD_DEFAULT = 1000 + } + private fun isRegionChanged(currentCenter: LocLatLng, zoomLevel: Double): Boolean { val prev = lastSearchCenter ?: return false val distance = calculateDistance(prev.latitude, prev.longitude, currentCenter.latitude, currentCenter.longitude) val threshold = when { - zoomLevel >= 18 -> 300 - zoomLevel >= 16 -> 500 - zoomLevel >= 14 -> 700 - else -> 1000 + zoomLevel >= 18 -> REGION_CHANGE_THRESHOLD_ZOOM_18 + zoomLevel >= 16 -> REGION_CHANGE_THRESHOLD_ZOOM_16 + zoomLevel >= 14 -> REGION_CHANGE_THRESHOLD_ZOOM_14 + else -> REGION_CHANGE_THRESHOLD_DEFAULT } return distance >= threshold }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapViewModel.kt` around lines 410 - 420, The isRegionChanged function returns false when lastSearchCenter is null and uses magic numbers for zoom thresholds and distances; confirm with the GA4/notion event spec whether the initial search with no previous center should emit region_changed=false, be omitted, or treated as true, then update isRegionChanged (and any event emission call sites) to follow that spec instead of defaulting to false; extract the numeric thresholds (18,16,14 and 300,500,700,1000) into named constants (either a MapConst object or the ViewModel's companion object) and replace the inline literals with those constants so future tuning is centralized (reference symbols: isRegionChanged, lastSearchCenter, calculateDistance).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@feature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapViewModel.kt`:
- Around line 265-287: Move the MapRouteClick analytics log so it only records
when the direction flow actually proceeds: after checking state.currentLocLatLng
!= null and after finding a focused marker, just before calling
postSideEffect(MapEffect.LaunchDirectionApp(...)). Specifically, remove the
early analyticsLogger.log(MapAnalyticsEvent.MapRouteClick(...)) at the top of
the function and insert the same analyticsLogger.log call immediately prior to
postSideEffect(MapEffect.LaunchDirectionApp(...)) (i.e., after the null guard on
state.currentLocLatLng and inside the mapMarkers.find { it.isFocused }?.let {
... } block), leaving other behavior like reduce {
copy(isShowDirectionBottomSheet = false) } and the toast post on guard failure
unchanged.
---
Nitpick comments:
In
`@feature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapViewModel.kt`:
- Around line 410-420: The isRegionChanged function returns false when
lastSearchCenter is null and uses magic numbers for zoom thresholds and
distances; confirm with the GA4/notion event spec whether the initial search
with no previous center should emit region_changed=false, be omitted, or treated
as true, then update isRegionChanged (and any event emission call sites) to
follow that spec instead of defaulting to false; extract the numeric thresholds
(18,16,14 and 300,500,700,1000) into named constants (either a MapConst object
or the ViewModel's companion object) and replace the inline literals with those
constants so future tuning is centralized (reference symbols: isRegionChanged,
lastSearchCenter, calculateDistance).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: e9820283-b0ad-4453-84c2-0cf4f2431b7f
📒 Files selected for processing (6)
core/analytics/src/main/kotlin/com/neki/android/core/analytics/event/AnalyticsEvent.ktcore/analytics/src/main/kotlin/com/neki/android/core/analytics/event/ArchiveAnalyticsEvent.ktcore/analytics/src/main/kotlin/com/neki/android/core/analytics/event/MapAnalyticsEvent.ktcore/analytics/src/main/kotlin/com/neki/android/core/analytics/event/PoseAnalyticsEvent.ktcore/analytics/src/main/kotlin/com/neki/android/core/analytics/logger/FirebaseAnalyticsLogger.ktfeature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapViewModel.kt
✅ Files skipped from review due to trivial changes (2)
- core/analytics/src/main/kotlin/com/neki/android/core/analytics/logger/FirebaseAnalyticsLogger.kt
- core/analytics/src/main/kotlin/com/neki/android/core/analytics/event/MapAnalyticsEvent.kt
🚧 Files skipped from review as they are similar to previous changes (2)
- core/analytics/src/main/kotlin/com/neki/android/core/analytics/event/AnalyticsEvent.kt
- core/analytics/src/main/kotlin/com/neki/android/core/analytics/event/PoseAnalyticsEvent.kt
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
2 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.
🔗 관련 이슈
📙 작업 설명
core:analytics모듈 생성 및 Firebase Analytics 로깅 인터페이스 구현ArchiveAnalyticsEvent,PoseAnalyticsEvent,MapAnalyticsEvent,GlobalAnalyticsEvent)20.0, 기본 줌 레벨14.0변경AndroidFeatureImplConventionPlugin에core:analytics공통 의존성 추가🧪 테스트 내역
📸 스크린샷 또는 시연 영상
💬 추가 설명 or 리뷰 포인트
CameraX와Guava의존성 충돌 발생 →feature:photo-upload모듈에guava의존성 명시적으로 추가하여 해결했습니다.Summary by CodeRabbit
릴리스 노트
New Features
Chores