Fix/#111: MixPanel 슈퍼 속성 누락 및 FeedExited lastVisibleItemIndex 오류 수정#112
Merged
DongChyeon merged 3 commits intoMay 11, 2026
Conversation
- Analytics 인터페이스에 identify(userId: String?) 추가 - MixpanelAnalytics 초기화 시 platform, app_version, user_id(null) 슈퍼 속성 등록 - DataStore에 userId(Long) 저장 레이어 추가 (datastore → repository) - 로그인 후 fetchAndStoreUserProfile()에서 userId DataStore 저장 - BuyOrNotViewModel에서 userId 변화 감지 → analytics.identify() 호출
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
Walkthrough사용자 ID 관리 계층을 DataStore부터 도메인까지 추가하고, 로그인 시 저장된 ID를 Mixpanel 분석에 전달하여 사용자 식별을 수행합니다. Mixpanel 슈퍼 속성(platform, app_version, user_id)을 등록하고, 앱 초기화 시 BuyOrNotViewModel에서 userId 변경을 모니터링하여 analytics.identify를 호출합니다. 또한 HomeScreen의 피드 종료 이벤트에서 마지막 보이는 아이템 인덱스를 올바르게 기록합니다. Changes사용자 ID 식별 및 분석 통합
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 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 |
DongChyeon
changed the title
![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
🧹 Nitpick comments (2)
core/analytics/src/main/java/com/sseotdabwa/buyornot/core/analytics/di/AnalyticsModule.kt (1)
33-38: ⚡ Quick win앱 버전 추출이 대체로 안전하지만 방어적 처리를 고려하세요.
getPackageInfo(context.packageName, 0)는 자신의 패키지에 대해서는 예외가 발생하지 않아야 하지만,PackageManager.NameNotFoundException이 이론적으로 가능합니다. Singleton 생성 시점에 예외가 발생하면 앱이 시작되지 않으므로, 방어적으로 try-catch를 추가하는 것이 좋습니다.🛡️ 제안: 예외 처리 추가
- val appVersion = - context.packageManager - .getPackageInfo(context.packageName, 0) - .versionName - ?: "unknown" + val appVersion = try { + context.packageManager + .getPackageInfo(context.packageName, 0) + .versionName + ?: "unknown" + } catch (e: Exception) { + "unknown" + } MixpanelAnalytics(mixpanel, appVersion)🤖 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 `@core/analytics/src/main/java/com/sseotdabwa/buyornot/core/analytics/di/AnalyticsModule.kt` around lines 33 - 38, The code that reads appVersion via context.packageManager.getPackageInfo(...) should be made defensive: wrap the call in a try-catch for PackageManager.NameNotFoundException (and optionally Exception) when constructing appVersion so a failure returns a safe fallback like "unknown" instead of throwing during singleton initialization; update the block that computes appVersion (the code that assigns appVersion and then constructs MixpanelAnalytics) to catch the exception, log the error (using your project's logger) and set appVersion = "unknown" before passing it into MixpanelAnalytics.core/datastore/src/main/java/com/sseotdabwa/buyornot/core/datastore/UserPreferencesDataSourceImpl.kt (1)
53-54: 💤 Low value0L을 "미로그인" 센티널 값으로 사용 중입니다.
현재 구현은 userId가 없을 때 0L을 반환하며, 이는 ViewModel에서 null로 변환되어야 합니다. 이는 암묵적인 결합을 만들지만, 일반적으로 백엔드에서 0을 유효한 사용자 ID로 사용하지 않으므로 실용적인 접근입니다.
더 명시적인 대안으로는
Flow<Long?>을 사용할 수 있지만, 현재 구현도 충분히 동작합니다.🤖 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 `@core/datastore/src/main/java/com/sseotdabwa/buyornot/core/datastore/UserPreferencesDataSourceImpl.kt` around lines 53 - 54, The current userId Flow in UserPreferencesDataSourceImpl returns 0L as a sentinel for "not logged in" (override val userId: Flow<Long> = context.userPreferencesDataStore.data.map { it[Keys.USER_ID] ?: 0L }), which obscures semantics; change the API to return a nullable Flow<Long?> by updating the userId declaration to Flow<Long?> and map missing values to null (map { it[Keys.USER_ID] }) so callers (e.g., ViewModel) can explicitly handle logged-out state; update any call sites expecting Flow<Long> to accept Flow<Long?> accordingly.
🤖 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
`@core/analytics/src/main/java/com/sseotdabwa/buyornot/core/analytics/MixpanelAnalytics.kt`:
- Around line 26-33: The identify(userId: String?) implementation sets a user_id
super property but doesn't clear Mixpanel's internal distinct_id on logout;
update identify to call mixpanel.reset() when userId is null (or add and call a
new Analytics.reset() method that invokes mixpanel.reset()) and when a non-null
userId is provided still call mixpanel.identify(userId) and set the super
property via mixpanel.registerSuperProperties; ensure both identify and the
new/reset method use mixpanel.reset()/mixpanel.identify and
mixpanel.registerSuperProperties consistently so events after logout get a fresh
anonymous distinct_id.
---
Nitpick comments:
In
`@core/analytics/src/main/java/com/sseotdabwa/buyornot/core/analytics/di/AnalyticsModule.kt`:
- Around line 33-38: The code that reads appVersion via
context.packageManager.getPackageInfo(...) should be made defensive: wrap the
call in a try-catch for PackageManager.NameNotFoundException (and optionally
Exception) when constructing appVersion so a failure returns a safe fallback
like "unknown" instead of throwing during singleton initialization; update the
block that computes appVersion (the code that assigns appVersion and then
constructs MixpanelAnalytics) to catch the exception, log the error (using your
project's logger) and set appVersion = "unknown" before passing it into
MixpanelAnalytics.
In
`@core/datastore/src/main/java/com/sseotdabwa/buyornot/core/datastore/UserPreferencesDataSourceImpl.kt`:
- Around line 53-54: The current userId Flow in UserPreferencesDataSourceImpl
returns 0L as a sentinel for "not logged in" (override val userId: Flow<Long> =
context.userPreferencesDataStore.data.map { it[Keys.USER_ID] ?: 0L }), which
obscures semantics; change the API to return a nullable Flow<Long?> by updating
the userId declaration to Flow<Long?> and map missing values to null (map {
it[Keys.USER_ID] }) so callers (e.g., ViewModel) can explicitly handle
logged-out state; update any call sites expecting Flow<Long> to accept
Flow<Long?> accordingly.
🪄 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.yml
Review profile: CHILL
Plan: Pro
Run ID: 853eb55c-f228-4645-9b0c-0331f58c4936
📒 Files selected for processing (12)
app/src/main/java/com/sseotdabwa/buyornot/ui/BuyOrNotViewModel.ktcore/analytics/src/main/java/com/sseotdabwa/buyornot/core/analytics/Analytics.ktcore/analytics/src/main/java/com/sseotdabwa/buyornot/core/analytics/DebugAnalytics.ktcore/analytics/src/main/java/com/sseotdabwa/buyornot/core/analytics/MixpanelAnalytics.ktcore/analytics/src/main/java/com/sseotdabwa/buyornot/core/analytics/di/AnalyticsModule.ktcore/data/src/main/java/com/sseotdabwa/buyornot/core/data/repository/UserPreferencesRepositoryImpl.ktcore/datastore/src/main/java/com/sseotdabwa/buyornot/core/datastore/UserPreferences.ktcore/datastore/src/main/java/com/sseotdabwa/buyornot/core/datastore/UserPreferencesDataSource.ktcore/datastore/src/main/java/com/sseotdabwa/buyornot/core/datastore/UserPreferencesDataSourceImpl.ktdomain/src/main/java/com/sseotdabwa/buyornot/domain/repository/UserPreferencesRepository.ktfeature/auth/src/main/java/com/sseotdabwa/buyornot/feature/auth/ui/LoginViewModel.ktfeature/home/src/main/java/com/sseotdabwa/buyornot/feature/home/ui/HomeScreen.kt
saveState/restoreState 네비게이션에서 DisposableEffect(Unit)의 onDispose가 호출되지 않는 문제를 ON_STOP 라이프사이클 이벤트로 대체하여 수정 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
saveState 네비게이션 환경에서 HomeFeedList가 Composition에 유지되므로 재방문마다 enterTimeMs를 갱신하지 않으면 누적 시간이 전송되는 문제 수정 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
DongChyeon
deleted the
bugfix/#111-fix-analytics-super-properties-and-last-visible-index
branch
12 tasks
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
1 participant
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.
🛠 Related issue
closed #111
어떤 변경사항이 있었나요?
✅ CheckPoint
PR이 다음 요구 사항을 충족하는지 확인하세요.
✏️ Work Description
MixpanelAnalytics초기화 시platform("android"),app_version슈퍼 속성 등록Analytics.identify(userId)인터페이스 추가 — 로그인/로그아웃 시user_id슈퍼 속성 갱신 (비로그인 시null)UserPreferencesDataStore에userId저장 추가 — 로그인 후users/me응답의 userId를 DataStore에 영속화BuyOrNotViewModel에서userIdFlow 구독 →analytics.identify()호출로 앱 전역에서 사용자 식별 유지HomeScreen.ktlastVisibleItemIndex를listState.firstVisibleItemIndex대신listState.layoutInfo.visibleItemsInfo.lastOrNull()?.index로 수정DisposableEffect(Unit)→LifecycleEventEffect(ON_STOP)교체 — saveState 네비게이션 환경에서onDispose가 호출되지 않아feed_exited이벤트가 누락되던 문제 수정😅 Uncompleted Tasks
N/A
📢 To Reviewers
user_id슈퍼 속성은String?으로, 비로그인 상태에서는JSONObject.NULL로 Mixpanel에 등록되어 속성 자체는 유지되지만 null 값으로 전송됩니다.app_version은AnalyticsModule에서PackageManager로 런타임에 가져오며 DI로 주입합니다.feed_exited누락 원인: Jetpack Navigation의saveState = true설정 시 홈 화면이 Composition에서 제거되지 않아DisposableEffect.onDispose가 호출되지 않았습니다.LifecycleEventEffect(ON_STOP)으로 변경하여 다른 탭 이동 및 백그라운드 전환 시 모두 정상 호출됩니다.