Added Firebase analytics for computer vision.#804
Conversation
📝 Walkthrough
WalkthroughAdds Firebase Analytics dep, introduces a new CvAnalyticsUtil for event logging, and integrates analytics calls into ComputerVisionViewModel and ComputerVisionActivity to emit events for screen open, image selection, detection lifecycle, and XML generation/export. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as User / UI
participant Activity as ComputerVisionActivity
participant VM as ComputerVisionViewModel
participant UTIL as CvAnalyticsUtil
participant FB as Firebase Analytics
UI->>Activity: Open screen
Activity->>VM: lifecycle STARTED
VM->>UTIL: trackScreenOpened()
UTIL->>FB: logEvent "cv_screen_opened"
UI->>VM: Select image / Capture photo
VM->>UTIL: trackImageSelected(fromCamera)
UTIL->>FB: logEvent "cv_image_selected"
VM->>VM: load image and start detection
VM->>UTIL: trackDetectionStarted()
UTIL->>FB: logEvent "cv_detection_started"
VM->>VM: detection completes
VM->>UTIL: trackDetectionCompleted(success, count, durationMs)
UTIL->>FB: logEvent "cv_detection_completed"
VM->>VM: generate/export XML
VM->>UTIL: trackXmlGenerated(componentCount)
UTIL->>FB: logEvent "cv_xml_generated"
VM->>UTIL: trackXmlExported(toDownloads)
UTIL->>FB: logEvent "cv_xml_exported"
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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: 1
🤖 Fix all issues with AI agents
In
@cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/ui/viewmodel/ComputerVisionViewModel.kt:
- Around line 50-55: Analytics for camera-captured images is missing: modify the
handler for ComputerVisionEvent.ImageCaptured by updating the
handleCameraResult(uri: Uri, success: Boolean) method so that when success is
true it calls CvAnalyticsUtil.trackImageSelected(fromCamera = true) before
invoking loadImageFromUri(uri), and when success is false it preserves the
existing behavior of sending
ComputerVisionEffect.ShowToast(R.string.msg_image_capture_cancelled) via
viewModelScope; ensure references to ComputerVisionEvent.ImageCaptured,
handleCameraResult, CvAnalyticsUtil.trackImageSelected, loadImageFromUri, and
ComputerVisionEffect.ShowToast are used to locate and update the code.
🧹 Nitpick comments (2)
cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/ui/viewmodel/ComputerVisionViewModel.kt (1)
161-171: Move analytics call outside the state update lambda.The
trackDetectionCompletedcall inside_uiState.update { }is a side effect within a state transformation. This violates the expectation that update lambdas are pure functions.Proposed fix
mergeResult .onSuccess { mergedDetections -> + CvAnalyticsUtil.trackDetectionCompleted( + success = true, + detectionCount = mergedDetections.size, + durationMs = System.currentTimeMillis() - startTime + ) _uiState.update { - CvAnalyticsUtil.trackDetectionCompleted( - success = true, - detectionCount = mergedDetections.size, - durationMs = System.currentTimeMillis() - startTime - ) it.copy( detections = mergedDetections, currentOperation = CvOperation.Idle ) }cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/utils/CvAnalyticsUtils.kt (1)
7-14: Consider removing redundant timestamp parameters.Firebase Analytics automatically records the timestamp when each event is logged. Adding
putLong("timestamp", System.currentTimeMillis())to every event is redundant and increases payload size without adding value. The automatic timestamp is more reliable as it's captured at the Firebase SDK level.Example for one method
fun trackScreenOpened() { - analytics.logEvent("cv_screen_opened", Bundle().apply { - putLong("timestamp", System.currentTimeMillis()) - }) + analytics.logEvent("cv_screen_opened", null) }Apply similar changes to other methods, keeping only the meaningful custom parameters.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
cv-image-to-xml/build.gradle.ktscv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/ui/viewmodel/ComputerVisionViewModel.ktcv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/utils/CvAnalyticsUtils.kt
🔇 Additional comments (5)
cv-image-to-xml/build.gradle.kts (1)
51-54: LGTM! Firebase dependencies added correctly using BOM.Using
platform(libs.firebase.bom)ensures consistent versioning across Firebase libraries. Verify that thecom.google.gms.google-servicesplugin is applied in the main application module (not this library module) and thatgoogle-services.jsonis properly configured.cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/ui/viewmodel/ComputerVisionViewModel.kt (2)
43-46: LGTM! Screen open tracking appropriately placed in init.
213-218: LGTM! XML generation and export tracking implemented correctly.Analytics calls are properly placed outside state update lambdas and capture the relevant metadata.
cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/utils/CvAnalyticsUtils.kt (2)
28-35: Good event design with meaningful parameters.The
trackDetectionCompletedevent captures useful metrics (success status, detection count, duration) that will enable meaningful funnel analysis.
1-51: Overall structure is well-designed.Using a Kotlin
objectwith lazy initialization for Firebase Analytics is the correct pattern. Event naming withcv_prefix provides good namespacing. Consider adding a brief KDoc comment to the object describing its purpose.
...ain/java/org/appdevforall/codeonthego/computervision/ui/viewmodel/ComputerVisionViewModel.kt
Show resolved
Hide resolved
...e-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/utils/CvAnalyticsUtils.kt
Show resolved
Hide resolved
...e-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/utils/CvAnalyticsUtils.kt
Show resolved
Hide resolved
...ain/java/org/appdevforall/codeonthego/computervision/ui/viewmodel/ComputerVisionViewModel.kt
Outdated
Show resolved
Hide resolved
...ain/java/org/appdevforall/codeonthego/computervision/ui/viewmodel/ComputerVisionViewModel.kt
Outdated
Show resolved
Hide resolved
...ain/java/org/appdevforall/codeonthego/computervision/ui/viewmodel/ComputerVisionViewModel.kt
Outdated
Show resolved
Hide resolved
...e-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/utils/CvAnalyticsUtils.kt
Outdated
Show resolved
Hide resolved
…ate screen open event to be LifeCycle.Started event
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/ui/viewmodel/ComputerVisionViewModel.kt (1)
159-172: Move analytics call outside the_uiState.updatelambda.The
updatelambda should be a pure state transformation without side effects. Placing analytics calls inside risks issues if the lambda is retried due to CAS failures inMutableStateFlow.update().Proposed fix
mergeResult .onSuccess { mergedDetections -> + CvAnalyticsUtil.trackDetectionCompleted( + success = true, + detectionCount = mergedDetections.size, + durationMs = System.currentTimeMillis() - startTime + ) _uiState.update { - CvAnalyticsUtil.trackDetectionCompleted( - success = true, - detectionCount = mergedDetections.size, - durationMs = System.currentTimeMillis() - startTime - ) it.copy( detections = mergedDetections, currentOperation = CvOperation.Idle ) } Log.d(TAG, "Detection complete. ${mergedDetections.size} objects detected.") }
🤖 Fix all issues with AI agents
In
`@cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/ui/ComputerVisionActivity.kt`:
- Around line 126-132: The call to viewModel.onScreenStarted() inside
ComputerVisionActivity.observeViewModel fires every time the lifecycle reaches
STARTED; move that single-shot event out of repeatOnLifecycle to run only once
per Activity instance by either (A) calling viewModel.onScreenStarted() from
onCreate() instead of inside observeViewModel, or (B) add an Activity-level
Boolean flag (e.g., var screenOpened = false) and inside observeViewModel only
invoke viewModel.onScreenStarted() when screenOpened is false then set it true;
keep the viewModel.uiState.collect { state -> updateUi(state) } inside
repeatOnLifecycle as-is.
♻️ Duplicate comments (1)
cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/ui/viewmodel/ComputerVisionViewModel.kt (1)
115-123: Missing analytics tracking for camera-captured images.
trackImageSelected(fromCamera = false)is called for gallery selection, buthandleCameraResultloads an image on success without corresponding analytics tracking.Proposed fix
private fun handleCameraResult(uri: Uri, success: Boolean) { if (success) { + CvAnalyticsUtil.trackImageSelected(fromCamera = true) loadImageFromUri(uri) } else { viewModelScope.launch { _uiEffect.send(ComputerVisionEffect.ShowToast(R.string.msg_image_capture_cancelled)) } } }
🧹 Nitpick comments (2)
cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/utils/CvAnalyticsUtils.kt (2)
16-17: Remove extra blank line.object CvAnalyticsUtil { private const val TAG = "CvAnalyticsUtil" - private val analytics by lazy {
35-74: Consider removing redundanttimestampparameter.Firebase Analytics automatically records event timestamps server-side. Adding a custom timestamp to every event is redundant and increases payload size without benefit.
Example for trackScreenOpened
fun trackScreenOpened() { - logEvent(CV_SCREEN_OPENED, Bundle().apply { - putLong("timestamp", System.currentTimeMillis()) - }) + logEvent(CV_SCREEN_OPENED, Bundle()) }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/ui/ComputerVisionActivity.ktcv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/ui/viewmodel/ComputerVisionViewModel.ktcv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/utils/CvAnalyticsUtils.kt
⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build Universal APK
🔇 Additional comments (9)
cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/ui/ComputerVisionActivity.kt (1)
24-28: LGTM!Import additions are appropriate for the binding, model, viewmodel, and Koin integration.
cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/ui/viewmodel/ComputerVisionViewModel.kt (5)
49-52: LGTM!Analytics tracking for gallery image selection is correctly placed before loading the image.
89-91: LGTM!The
onScreenStarted()method properly delegates to the analytics utility. This addresses the past feedback to move tracking to a lifecycle-aware call.
135-136: LGTM!Detection start and failure tracking are correctly placed outside state update lambdas.
Also applies to: 141-143
214-215: LGTM!Analytics calls are correctly placed in the success callback, outside any state update lambdas.
245-246: LGTM!Analytics calls for download export are correctly placed.
cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/utils/CvAnalyticsUtils.kt (3)
14-25: LGTM!Good defensive initialization with try-catch to handle cases where Firebase Analytics is unavailable. The lazy initialization pattern is appropriate here.
27-33: LGTM!The
logEventwrapper with try-catch ensures analytics failures don't crash the app.
76-83: LGTM!Event names are properly centralized as constants, addressing previous feedback.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
...o-xml/src/main/java/org/appdevforall/codeonthego/computervision/ui/ComputerVisionActivity.kt
Show resolved
Hide resolved
…ured images; remove lambda side effect
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/ui/viewmodel/ComputerVisionViewModel.kt (1)
160-176: Track detection completion on merge failure too.The
mergeResultfailure path doesn’t emittrackDetectionCompleted, so analytics will miss completion for that failure case.✅ Proposed fix
- mergeResult - .onSuccess { mergedDetections -> + mergeResult + .onSuccess { mergedDetections -> CvAnalyticsUtil.trackDetectionCompleted( success = true, detectionCount = mergedDetections.size, durationMs = System.currentTimeMillis() - startTime ) _uiState.update { it.copy( detections = mergedDetections, currentOperation = CvOperation.Idle ) } Log.d(TAG, "Detection complete. ${mergedDetections.size} objects detected.") } - .onFailure { handleDetectionError(it) } + .onFailure { exception -> + CvAnalyticsUtil.trackDetectionCompleted( + success = false, + detectionCount = 0, + durationMs = System.currentTimeMillis() - startTime + ) + handleDetectionError(exception) + }
4 participants
No description provided.