Release: join a study flow, analytics identity fixes - #264
Merged
Conversation
MR-72 Update DefaultAppEventPayloadHandler.java
…dd cancel button drawable.
fix: [MR-84] added container version to firestore payload
Add ID confirmation dialog UI and improve project configuration
- Persist confirmed study_user_id after successful study enrollment - Store the study ID alongside the existing pseudoId using a dedicated preference key - Append study_user_id only to app_launch Firebase event parameters when available - Leave unenrolled app_launch events unchanged by omitting study_user_id - Add unit coverage for app_launch events with and without a persisted study_user_id - Restore the existing language_selected analytics test invocation so the test class passes
feat: add study user id to app launch analytics
feat: replace toast with custom success dialog
feat: joined study additions
refactor: Disabling cr_user_id modification!
…nk language handling - Preserve the original cr_user_id while storing study_user_id separately. - Make study enrollment write-once so later app links do not reopen the modal or overwrite the saved phone-number ID. - Fix Play Store deferred deeplink parsing so language, source, and campaign ID are extracted independently.
Aj 656 joined study edits
📝 WalkthroughWalkthroughThis PR adds study enrollment functionality triggered by deep-link intents, enabling users to confirm and register with a study via study_user_id and optional consent fields. The flow includes a confirmation dialog with animated button, success notification, analytics event tracking with study user ID enrichment, and updated deep-link intent filters for multiple app domains. ChangesStudy Enrollment with Intent Handling and Analytics
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 2❌ Failed checks (2 warnings)
✅ 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 |
amitsinghsutara
approved these changes
Jun 2, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 `@app/src/main/java/org/curiouslearning/container/MainActivity.java`:
- Line 257: Move the RecyclerView initialization (initRecyclerView()) to execute
before handling any incoming intents so that any intent-driven loadApps() calls
cannot observe or touch apps.webApps before the adapter/list is ready;
specifically, call initRecyclerView() prior to handleIncomingIntent(getIntent())
in MainActivity (ensure initRecyclerView() runs before any code paths that call
loadApps(), including observer callbacks triggered by incoming intent language
data) and adjust other spots where loadApps() may run (the loadApps()/apps
observer regions around the lines noted) to guarantee the RecyclerView and its
adapter are initialized first.
- Around line 329-342: The current check requires study_consent=="true" to start
enrollment, which blocks links that only include study_user_id; change the logic
in the block that handles newIdRaw (around variables newIdRaw, studyConsent,
newId, handledStudyEnrollmentLink) so that enrollment proceeds whenever newId is
present (set handledStudyEnrollmentLink = true and show the confirmation flow
when newId is non-empty), and only treat studyConsent as an optional flag —
i.e., if "true".equals(studyConsent) then record/store/log consent (or set the
consent-related state), otherwise continue enrollment without blocking; leave
cachePseudoId() and prefs.contains("pseudoId") behavior as-is.
In `@app/src/main/res/layout/dialog_confirm_id.xml`:
- Around line 7-13: Replace the hard-coded fixed width on the ConstraintLayout
by making it responsive: change the ConstraintLayout's android:layout_width from
"320dp" to a fluid value (e.g., "match_parent" or "wrap_content" with side
margins) and add a cap using android:maxWidth="320dp" (and keep the existing
android:layout_margin/padding) so the dialog can shrink on small viewports but
never exceed 320dp; update the ConstraintLayout attributes
(android:layout_width, android:maxWidth, android:layout_margin or start/end
margins) accordingly.
In `@app/src/main/res/layout/dialog_enrollment_success.xml`:
- Around line 17-25: Replace the lone emoji TextView with two accessible
elements: keep the decorative emoji TextView but set its text via layout (or
keep) and mark it decorative with android:contentDescription="`@null`" and
android:importantForAccessibility="no"; add a new TextView (or replace text)
that uses a localized string resource (e.g. `@string/enrollment_success`) for the
success copy and ensure it is reachable by accessibility (no null
contentDescription) and set android:accessibilityLiveRegion="polite" so TalkBack
announces it; also confirm MainActivity’s auto-dismiss behavior still allows the
announcement (e.g. MainActivity should not immediately clear focus or close the
dialog before the live region announces).
In `@app/src/main/res/values/strings.xml`:
- Around line 17-20: The copy currently refers to “Curious Reader User ID” but
the flow writes `study_user_id`; update the string resources to use “study ID”
terminology instead: change confirm_id_message to ask “Would you like to set
your study ID to the following value?”, change confirm_id_placeholder from
"[phone]" to a neutral "[study ID]" (or appropriate example), and keep
confirm_id_button_text/confirm_id_button_description as-is; ensure the new
wording aligns with `study_user_id` and does not imply overwriting `cr_user_id`.
In
`@app/src/test/java/org/curiouslearning/container/AnalyticsUtilsCustomEventsTest.java`:
- Around line 158-186: The test testLogJoinedStudyEvent is asserting that the
event Bundle contains "app_info.version" but AnalyticsUtils.logJoinedStudyEvent
no longer populates that key; update the test to remove the assertion
assertEquals("2.34.3", b.getString("app_info.version")) (and any related
expectations) so it no longer expects that key, leaving the other assertions and
verifications for AnalyticsUtils.logJoinedStudyEvent,
FirebaseAnalytics.logEvent, and setUserProperty intact; alternatively, if you
prefer to keep the assertion, modify AnalyticsUtils.logJoinedStudyEvent to
include "app_info.version" in the Bundle before calling
FirebaseAnalytics.logEvent.
🪄 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.yaml
Review profile: CHILL
Plan: Pro
Run ID: e893af02-68ff-42ab-ac7e-fc55eda0a136
📒 Files selected for processing (14)
app/build.gradleapp/fastlane/debug_apks/app-debug.apkapp/src/main/AndroidManifest.xmlapp/src/main/java/org/curiouslearning/container/MainActivity.javaapp/src/main/java/org/curiouslearning/container/core/subapp/handler/DefaultAppEventPayloadHandler.javaapp/src/main/java/org/curiouslearning/container/firebase/AnalyticsUtils.javaapp/src/main/java/org/curiouslearning/container/installreferrer/InstallReferrerManager.javaapp/src/main/res/drawable/bg_button_confirm.xmlapp/src/main/res/drawable/bg_dialog.xmlapp/src/main/res/drawable/bg_id_field.xmlapp/src/main/res/layout/dialog_confirm_id.xmlapp/src/main/res/layout/dialog_enrollment_success.xmlapp/src/main/res/values/strings.xmlapp/src/test/java/org/curiouslearning/container/AnalyticsUtilsCustomEventsTest.java
| + language.substring(1).toLowerCase(); | ||
| } | ||
| } | ||
| handleIncomingIntent(getIntent()); |
There was a problem hiding this comment.
Move initRecyclerView() before any intent-driven loadApps() call.
Line 257 can route a deep link into loadApps() before Line 267 initializes apps. If the incoming intent carries language, the observer can dispatch immediately and hit apps.webApps in loadApps(), so a cold start via deep link can crash before the RecyclerView is ready.
Suggested fix
InstallReferrerManager installReferrerManager = new InstallReferrerManager(getApplicationContext(),
referrerCallback);
installReferrerManager.checkPlayStoreAvailability();
+ initRecyclerView();
handleIncomingIntent(getIntent());
audioPlayer = new AudioPlayer();
FirebaseApp.initializeApp(this);
@@
- initRecyclerView();
Log.d(TAG, "onCreate: Selected language: " + selectedLanguage);Also applies to: 267-267, 365-377
🤖 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 `@app/src/main/java/org/curiouslearning/container/MainActivity.java` at line
257, Move the RecyclerView initialization (initRecyclerView()) to execute before
handling any incoming intents so that any intent-driven loadApps() calls cannot
observe or touch apps.webApps before the adapter/list is ready; specifically,
call initRecyclerView() prior to handleIncomingIntent(getIntent()) in
MainActivity (ensure initRecyclerView() runs before any code paths that call
loadApps(), including observer callbacks triggered by incoming intent language
data) and adjust other spots where loadApps() may run (the loadApps()/apps
observer regions around the lines noted) to guarantee the RecyclerView and its
adapter are initialized first.
Comment on lines
+329
to
+342
| String newIdRaw = data.getQueryParameter("study_user_id"); | ||
| String confirmationMessageRaw = data.getQueryParameter("confirmation_message"); | ||
| String studyConsent = data.getQueryParameter("study_consent"); | ||
|
|
||
| // Verify or generate cr_user_id before processing | ||
| if (!prefs.contains("pseudoId")) { | ||
| cachePseudoId(); | ||
| } | ||
|
|
||
| if (newIdRaw != null && !newIdRaw.isEmpty()) { | ||
| String newId = newIdRaw.replaceAll("[^0-9]", ""); | ||
|
|
||
| if ("true".equals(studyConsent) && !newId.isEmpty()) { | ||
| handledStudyEnrollmentLink = true; |
There was a problem hiding this comment.
Treat study_consent as optional when starting enrollment.
Line 341 makes study_consent=true mandatory, so links that only carry study_user_id never reach the confirmation dialog. That contradicts the flow described in the PR, where consent is stored/logged when present but should not block enrollment itself.
🤖 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 `@app/src/main/java/org/curiouslearning/container/MainActivity.java` around
lines 329 - 342, The current check requires study_consent=="true" to start
enrollment, which blocks links that only include study_user_id; change the logic
in the block that handles newIdRaw (around variables newIdRaw, studyConsent,
newId, handledStudyEnrollmentLink) so that enrollment proceeds whenever newId is
present (set handledStudyEnrollmentLink = true and show the confirmation flow
when newId is non-empty), and only treat studyConsent as an optional flag —
i.e., if "true".equals(studyConsent) then record/store/log consent (or set the
consent-related state), otherwise continue enrollment without blocking; leave
cachePseudoId() and prefs.contains("pseudoId") behavior as-is.
Comment on lines
+7
to
+13
| <androidx.constraintlayout.widget.ConstraintLayout | ||
| android:layout_width="320dp" | ||
| android:layout_height="wrap_content" | ||
| android:layout_gravity="center" | ||
| android:layout_margin="24dp" | ||
| android:padding="28dp" | ||
| android:background="@drawable/bg_dialog"> |
There was a problem hiding this comment.
Make the confirmation dialog width responsive.
Lines 8-12 hard-code 320dp plus 24dp margins, which can exceed a 320dp viewport and clip the enrollment UI on small phones or split-screen. Use a width that can shrink with the screen and cap it with a max width instead.
🤖 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 `@app/src/main/res/layout/dialog_confirm_id.xml` around lines 7 - 13, Replace
the hard-coded fixed width on the ConstraintLayout by making it responsive:
change the ConstraintLayout's android:layout_width from "320dp" to a fluid value
(e.g., "match_parent" or "wrap_content" with side margins) and add a cap using
android:maxWidth="320dp" (and keep the existing android:layout_margin/padding)
so the dialog can shrink on small viewports but never exceed 320dp; update the
ConstraintLayout attributes (android:layout_width, android:maxWidth,
android:layout_margin or start/end margins) accordingly.
Comment on lines
+17
to
+25
| <TextView | ||
| android:layout_width="wrap_content" | ||
| android:layout_height="wrap_content" | ||
| android:text="🎉" | ||
| android:textSize="80sp" | ||
| app:layout_constraintTop_toTopOf="parent" | ||
| app:layout_constraintBottom_toBottomOf="parent" | ||
| app:layout_constraintStart_toStartOf="parent" | ||
| app:layout_constraintEnd_toEndOf="parent" /> |
There was a problem hiding this comment.
Add actual success copy here, not just an emoji.
This dialog only exposes 🎉. Combined with the 2-second auto-dismiss in MainActivity, TalkBack users get no reliable confirmation that enrollment succeeded. Add a localized success message and treat the emoji as decorative.
🤖 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 `@app/src/main/res/layout/dialog_enrollment_success.xml` around lines 17 - 25,
Replace the lone emoji TextView with two accessible elements: keep the
decorative emoji TextView but set its text via layout (or keep) and mark it
decorative with android:contentDescription="`@null`" and
android:importantForAccessibility="no"; add a new TextView (or replace text)
that uses a localized string resource (e.g. `@string/enrollment_success`) for the
success copy and ensure it is reachable by accessibility (no null
contentDescription) and set android:accessibilityLiveRegion="polite" so TalkBack
announces it; also confirm MainActivity’s auto-dismiss behavior still allows the
announcement (e.g. MainActivity should not immediately clear focus or close the
dialog before the live region announces).
Comment on lines
+17
to
+20
| <string name="confirm_id_message">Would you like to set your Curious Reader User ID to the following value?</string> | ||
| <string name="confirm_id_placeholder">[phone]</string> | ||
| <string name="confirm_id_button_text">✔</string> | ||
| <string name="confirm_id_button_description">Confirm</string> |
There was a problem hiding this comment.
Use “study ID” terminology in this copy.
These strings say the app is setting the “Curious Reader User ID”, but the new flow writes study_user_id and preserves cr_user_id separately. In an enrollment/consent flow, that wording is misleading.
🤖 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 `@app/src/main/res/values/strings.xml` around lines 17 - 20, The copy currently
refers to “Curious Reader User ID” but the flow writes `study_user_id`; update
the string resources to use “study ID” terminology instead: change
confirm_id_message to ask “Would you like to set your study ID to the following
value?”, change confirm_id_placeholder from "[phone]" to a neutral "[study ID]"
(or appropriate example), and keep
confirm_id_button_text/confirm_id_button_description as-is; ensure the new
wording aligns with `study_user_id` and does not imply overwriting `cr_user_id`.
Comment on lines
+158
to
+186
| @Test | ||
| public void testLogJoinedStudyEvent() { | ||
| FirebaseAnalytics spyFA = Mockito.spy(FirebaseAnalytics.getInstance(context)); | ||
| try (MockedStatic<FirebaseAnalytics> faStatic = Mockito.mockStatic(FirebaseAnalytics.class)) { | ||
| faStatic.when(() -> FirebaseAnalytics.getInstance(context)).thenReturn(spyFA); | ||
|
|
||
| AnalyticsUtils.logJoinedStudyEvent( | ||
| context, | ||
| "12345", | ||
| "Nepali", | ||
| "2.34.3", | ||
| "12345", | ||
| "true" | ||
| ); | ||
|
|
||
| ArgumentCaptor<Bundle> captor = ArgumentCaptor.forClass(Bundle.class); | ||
| verify(spyFA).logEvent(eq("joined_study"), captor.capture()); | ||
| Bundle b = captor.getValue(); | ||
| assertEquals("Nepali", b.getString("cr_language")); | ||
| assertEquals("2.34.3", b.getString("app_info.version")); | ||
| assertEquals("12345", b.getString("cr_user_id")); | ||
| assertEquals("test_source", b.getString("source")); | ||
| assertEquals("test_campaign", b.getString("campaign_id")); | ||
| assertEquals("12345", b.getString("study_user_id")); | ||
| assertEquals("true", b.getString("study_consent")); | ||
| verify(spyFA).setUserProperty("source", "test_source"); | ||
| verify(spyFA).setUserProperty("campaign_id", "test_campaign"); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check if logJoinedStudyEvent includes app_info.version in its implementation
rg -A 30 'logJoinedStudyEvent' --type=java -g '!*Test*' | rg -C 5 'app_info.version'Repository: curiouslearning/CRcontainer
Length of output: 53
Fix testLogJoinedStudyEvent expecting app_info.version when joined_study no longer sets it.
app_info.version isn’t present in non-test Java code around logJoinedStudyEvent, so assertEquals("2.34.3", b.getString("app_info.version")); will fail. Update the test to stop asserting this field (or add app_info.version back to the event payload).
🤖 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
`@app/src/test/java/org/curiouslearning/container/AnalyticsUtilsCustomEventsTest.java`
around lines 158 - 186, The test testLogJoinedStudyEvent is asserting that the
event Bundle contains "app_info.version" but AnalyticsUtils.logJoinedStudyEvent
no longer populates that key; update the test to remove the assertion
assertEquals("2.34.3", b.getString("app_info.version")) (and any related
expectations) so it no longer expects that key, leaving the other assertions and
verifications for AnalyticsUtils.logJoinedStudyEvent,
FirebaseAnalytics.logEvent, and setUserProperty intact; alternatively, if you
prefer to keep the assertion, modify AnalyticsUtils.logJoinedStudyEvent to
include "app_info.version" in the Bundle before calling
FirebaseAnalytics.logEvent.
miguelccodev
approved these changes
Jun 2, 2026
dz4va
dismissed
coderabbitai[bot]’s stale review
June 2, 2026 17:57
Dismissing for now as all of our test cases aren't failing
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
5 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.
Changes
ID display.
app links.
How to test
Ref: AJ-650
Summary by CodeRabbit
New Features
Chores
Tests