Aj 656 joined study edits - #262
Merged
Merged
Conversation
…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.
📝 WalkthroughWalkthroughThis PR coordinates version metadata updates (versionCode 73→83, versionName 2.34.3→2.34.10), manifest launch mode configuration, MainActivity deep-link enrollment flow refinements with state gating and intent clearing, enrollment success dialog lifecycle management, confirmation dialog scrolling UI, analytics event payload adjustments, and referrer parameter extraction logic. ChangesStudy Enrollment and Dialog Flow
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (3 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
May 29, 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/src/test/java/org/curiouslearning/container/AnalyticsUtilsCustomEventsTest.java (1)
177-177:⚠️ Potential issue | 🟠 Major | ⚡ Quick winStale assertion will fail:
app_info.versionwas removed from thejoined_studypayload.
logJoinedStudyEventno longer putsapp_info.versioninto the bundle, sob.getString("app_info.version")returnsnulland this assertion fails. Remove it to match the updated payload.💚 Proposed fix
assertEquals("Nepali", b.getString("cr_language")); - assertEquals("2.34.3", b.getString("app_info.version")); assertEquals("12345", b.getString("cr_user_id"));🤖 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` at line 177, The test contains a stale assertion expecting app_info.version in the joined_study payload; remove the assertEquals("2.34.3", b.getString("app_info.version")) from AnalyticsUtilsCustomEventsTest (the test that calls logJoinedStudyEvent / inspects the joined_study bundle) so the test matches the updated payload that no longer includes app_info.version.app/src/main/java/org/curiouslearning/container/MainActivity.java (1)
508-543:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix dialog lifecycle cleanup in
MainActivityto avoid window leaks.
showSuccessDialog()creates a localDialogand schedules a 2sHandler.postDelayeddismissal;onPause()currently only pauses animations/removes overlay callbacks and never dismisses/cancels this dialog, so the window can leak if the activity is destroyed during the delay.showConfirmIdDialog()is also activity-scoped and never dismissed fromonPause/onDestroy(noonDestroyoverride present), so it can stay open indefinitely.- Apply the same lifecycle-style cleanup used in
showLanguagePopup()(guarding withisFinishing()/isDestroyed()for showing) and add explicit dismissal + cancel the pending delayed runnable(s) inonPause(oronStop).🤖 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 508 - 543, The showSuccessDialog and showConfirmIdDialog create local Dialogs and delayed Runnables that can leak windows; refactor by promoting the local variables to activity fields (e.g., successDialog, successHandler, successDismissRunnable, confirmIdDialog) so they can be accessed from lifecycle methods, guard any show calls with isFinishing()/isDestroyed() like showLanguagePopup does, and in onPause()/onStop()/onDestroy explicitly dismiss() and cancel() those dialogs and call successHandler.removeCallbacks(successDismissRunnable) (and similar for confirmIdDialog) and null out the references and update isShowingEnrollmentSuccess/dismissActionDelivered accordingly to prevent double-callbacks and leaks.
🧹 Nitpick comments (1)
app/src/main/java/org/curiouslearning/container/installreferrer/InstallReferrerManager.java (1)
303-310: 💤 Low valueConsider parsing the deferred deeplink once and reusing the Uri object.
The
deeplinkis parsed at line 305 and again at line 318. Parsing once and reusing theUriobject would eliminate the duplicate work and improve efficiency slightly.♻️ Proposed refactor to eliminate duplicate parsing
- String deferredLanguage = ""; - if (!TextUtils.isEmpty(deeplink)) { - Uri deeplinkUri = Uri.parse(deeplink); - String language = deeplinkUri.getQueryParameter("language"); - if (!TextUtils.isEmpty(language)) { - deferredLanguage = language; - } - } - callback.onReferrerReceived(deferredLanguage, referrerUrl); + String deferredLanguage = ""; + Uri deeplinkUri = null; + if (!TextUtils.isEmpty(deeplink)) { + deeplinkUri = Uri.parse(deeplink); + String language = deeplinkUri.getQueryParameter("language"); + if (!TextUtils.isEmpty(language)) { + deferredLanguage = language; + } + } + callback.onReferrerReceived(deferredLanguage, referrerUrl); String source = null; String campaign_id = null; // First, try to extract source and campaign_id from deferred_deeplink (highest priority) if (deeplink != null && !deeplink.isEmpty()) { - Uri deeplinkUri = Uri.parse(deeplink); + if (deeplinkUri == null) { + deeplinkUri = Uri.parse(deeplink); + } source = deeplinkUri.getQueryParameter("source");🤖 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/installreferrer/InstallReferrerManager.java` around lines 303 - 310, In InstallReferrerManager, avoid parsing the same deeplink string twice: after confirming deeplink is not empty, parse it once into a local Uri (e.g., deeplinkUri) and reuse that Uri when extracting the "language" query parameter and anywhere else the code currently reparses deeplink; ensure you keep the TextUtils.isEmpty checks and null-safety around deeplinkUri.getQueryParameter calls so deferredLanguage assignment and subsequent logic use the single parsed Uri instance.
🤖 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/AndroidManifest.xml`:
- Around line 52-54: The manifest should prevent creating a second MainActivity
during enrollment deep-links: change android:launchMode from "singleTop" to
"singleTask" for MainActivity so incoming study_user_id intents always route to
the existing activity (triggering onNewIntent and handleIncomingIntent) instead
of creating a new instance that can re-run onCreate and bypass the instance
guards (isHandlingIdConfirmation / isShowingEnrollmentSuccess) before
prefs[AnalyticsUtils.STUDY_USER_ID] is persisted; keep
android:documentLaunchMode="never" and ensure handleIncomingIntent remains
invoked from onNewIntent.
---
Outside diff comments:
In `@app/src/main/java/org/curiouslearning/container/MainActivity.java`:
- Around line 508-543: The showSuccessDialog and showConfirmIdDialog create
local Dialogs and delayed Runnables that can leak windows; refactor by promoting
the local variables to activity fields (e.g., successDialog, successHandler,
successDismissRunnable, confirmIdDialog) so they can be accessed from lifecycle
methods, guard any show calls with isFinishing()/isDestroyed() like
showLanguagePopup does, and in onPause()/onStop()/onDestroy explicitly dismiss()
and cancel() those dialogs and call
successHandler.removeCallbacks(successDismissRunnable) (and similar for
confirmIdDialog) and null out the references and update
isShowingEnrollmentSuccess/dismissActionDelivered accordingly to prevent
double-callbacks and leaks.
In
`@app/src/test/java/org/curiouslearning/container/AnalyticsUtilsCustomEventsTest.java`:
- Line 177: The test contains a stale assertion expecting app_info.version in
the joined_study payload; remove the assertEquals("2.34.3",
b.getString("app_info.version")) from AnalyticsUtilsCustomEventsTest (the test
that calls logJoinedStudyEvent / inspects the joined_study bundle) so the test
matches the updated payload that no longer includes app_info.version.
---
Nitpick comments:
In
`@app/src/main/java/org/curiouslearning/container/installreferrer/InstallReferrerManager.java`:
- Around line 303-310: In InstallReferrerManager, avoid parsing the same
deeplink string twice: after confirming deeplink is not empty, parse it once
into a local Uri (e.g., deeplinkUri) and reuse that Uri when extracting the
"language" query parameter and anywhere else the code currently reparses
deeplink; ensure you keep the TextUtils.isEmpty checks and null-safety around
deeplinkUri.getQueryParameter calls so deferredLanguage assignment and
subsequent logic use the single parsed Uri instance.
🪄 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: 8f29ec28-6d78-4880-af06-cb0e8574f9e6
📒 Files selected for processing (7)
app/build.gradleapp/src/main/AndroidManifest.xmlapp/src/main/java/org/curiouslearning/container/MainActivity.javaapp/src/main/java/org/curiouslearning/container/firebase/AnalyticsUtils.javaapp/src/main/java/org/curiouslearning/container/installreferrer/InstallReferrerManager.javaapp/src/main/res/layout/dialog_confirm_id.xmlapp/src/test/java/org/curiouslearning/container/AnalyticsUtilsCustomEventsTest.java
Comment on lines
+52
to
54
| android:launchMode="singleTop" | ||
| android:documentLaunchMode="never" | ||
| android:exported="true"> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect MainActivity onCreate/onNewIntent and how enrollment state (handledStudyEnrollmentLink,
# isHandlingIdConfirmation, stored study_user_id) is read/written to confirm cross-instance safety.
fd MainActivity.java --exec rg -nP -C3 '(onCreate|onNewIntent|handleIncomingIntent|getIntent\(\)|isHandlingIdConfirmation|isShowingEnrollmentSuccess)' {}
# Check how WebApp is launched relative to MainActivity (task affinity / flags).
fd WebApp.java --exec rg -nP -C3 '(startActivity|Intent|FLAG_ACTIVITY)' {}Repository: curiouslearning/CRcontainer
Length of output: 7322
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Inspect AndroidManifest activity declarations around the reported lines and identify MainActivity/WebApp attributes.
MANIFEST="app/src/main/AndroidManifest.xml"
echo "== Manifest: Main/WebApp declarations around lines 35-120 =="
nl -ba "$MANIFEST" | sed -n '35,120p'
echo "== Manifest: find MainActivity and WebApp activity blocks =="
rg -n "activity|MainActivity|WebApp" "$MANIFEST"
# 2) Dump full handleIncomingIntent for cross-instance reasoning.
MAIN="app/src/main/java"
# Find MainActivity.java path(s)
echo "== Locating MainActivity.java =="
fd -a "MainActivity.java" "$MAIN" || true
MAIN_FILE="$(fd -a "MainActivity.java" "$MAIN" | head -n 1)"
echo "Using: $MAIN_FILE"
echo "== MainActivity.handleIncomingIntent (expanded context) =="
# Print from handleIncomingIntent declaration through nearby logic.
python3 - <<'PY'
import re,sys,subprocess,os, pathlib, json, textwrap
main_file = subprocess.check_output(["bash","-lc", "fd -a 'MainActivity.java' app/src/main/java | head -n 1"], text=True).strip()
print("Main file:", main_file)
# Read file
with open(main_file,'r',encoding='utf-8') as f:
lines = f.readlines()
# find handleIncomingIntent method bounds by matching signature and braces depth
start=None
sig_pat=re.compile(r'\bvoid\s+handleIncomingIntent\s*\(\s*Intent\s+\w*\s*\)')
for i,l in enumerate(lines):
if 'handleIncomingIntent' in l and re.search(r'handleIncomingIntent\s*\(', l):
if start is None and 'Intent' in l:
start=i
break
if start is None:
# fallback: locate exact method name
for i,l in enumerate(lines):
if re.search(r'\bhandleIncomingIntent\s*\(', l):
start=i
break
if start is None:
print("Could not locate handleIncomingIntent")
sys.exit(0)
# Find braces balance from first '{' after start
j=start
while j < len(lines) and '{' not in lines[j]:
j+=1
if j>=len(lines):
print("Could not find opening brace")
sys.exit(0)
brace=0
end=None
for k in range(j, len(lines)):
brace += lines[k].count('{')
brace -= lines[k].count('}')
if brace==0:
end=k
break
print(f"Method lines: {start+1}-{end+1}")
print("".join(lines[start:end+1]))
PY
# 3) Find where STUDY_USER_ID is written (to evaluate write-once gating across instances).
echo "== Where STUDY_USER_ID is stored/updated =="
rg -n "STUDY_USER_ID|STUDY_USER|study_user_id" app/src/main/java
# 4) Inspect WebApp start usage (at least where MainActivity launches it) to see flags/task behavior.
echo "== WebApp start usage (MainActivity and others) =="
rg -n "class WebApp|startActivity\\(|WebApp" app/src/main/javaRepository: curiouslearning/CRcontainer
Length of output: 193
🏁 Script executed:
#!/bin/bash
set -euo pipefail
MANIFEST="app/src/main/AndroidManifest.xml"
echo "== Tools =="
command -v awk >/dev/null && echo "awk ok"
command -v sed >/dev/null && echo "sed ok"
command -v rg >/dev/null && echo "rg ok"
command -v fd >/dev/null && echo "fd ok"
command -v python3 >/dev/null && echo "python3 ok"
echo
echo "== Manifest lines around 45-70 (to include launchMode/documentLaunchMode/exported) =="
awk 'NR>=45 && NR<=70 {printf "%d:%s\n", NR, $0}' "$MANIFEST"
echo
echo "== Manifest: find MainActivity and WebApp activity blocks =="
rg -n "activity|MainActivity|WebApp|launchMode|documentLaunchMode|exported" "$MANIFEST"
echo
echo "== Locate MainActivity.java under app/src/main/java =="
fd -a -t f "MainActivity.java" app/src/main/java || true
MAIN_FILE="$(fd -a -t f "MainActivity.java" app/src/main/java | head -n 1 || true)"
echo "Main file: ${MAIN_FILE:-<not found>}"
echo
echo "== MainActivity.handleIncomingIntent implementation (full method) =="
python3 - <<'PY'
import re,sys,subprocess,os, pathlib
# Find MainActivity.java
main_files = subprocess.check_output(["bash","-lc", "fd -a -t f 'MainActivity.java' app/src/main/java | head -n 1"], text=True).strip().splitlines()
main_file = main_files[0] if main_files and main_files[0] else None
if not main_file:
print("MainActivity.java not found")
sys.exit(0)
p = pathlib.Path(main_file)
text = p.read_text(encoding="utf-8", errors="ignore")
lines = text.splitlines(True)
# Find method signature line
sig_re = re.compile(r'\bvoid\s+handleIncomingIntent\s*\(\s*Intent\b')
start = None
for i,l in enumerate(lines):
if sig_re.search(l):
start = i
break
if start is None:
# fallback: any 'handleIncomingIntent('
for i,l in enumerate(lines):
if 'handleIncomingIntent' in l and '(' in l:
start = i
break
if start is None:
print("handleIncomingIntent not found")
sys.exit(0)
# find opening brace
j = start
while j < len(lines) and '{' not in lines[j]:
j += 1
if j >= len(lines):
print("Opening brace not found")
sys.exit(0)
brace = 0
end = None
for k in range(j, len(lines)):
brace += lines[k].count('{')
brace -= lines[k].count('}')
if brace == 0:
end = k
break
print(f"File: {main_file}")
print(f"Method lines: {start+1}-{end+1}")
print(''.join(lines[start:end+1]))
PY
echo
echo "== Where study_user_id / STUDY_USER_ID is read/written in MainActivity.java (and related prefs keys) =="
if [ -n "${MAIN_FILE:-}" ] && [ -f "$MAIN_FILE" ]; then
rg -n "STUDY_USER_ID|study_user_id|STUDY_USER|handledStudyEnrollmentLink|isHandlingIdConfirmation|isShowingEnrollmentSuccess" "$MAIN_FILE"
fi
echo
echo "== WebApp startActivity usage / intent flags in Java =="
rg -n "startActivity\\(|new Intent\\(|WebApp" app/src/main/javaRepository: curiouslearning/CRcontainer
Length of output: 29240
🏁 Script executed:
#!/bin/bash
set -euo pipefail
MAIN="app/src/main/java/org/curiouslearning/container/MainActivity.java"
echo "== Show MainActivity onCreate/onNewIntent (for intent handling entrypoints) =="
python3 - <<'PY'
import re, pathlib
p = pathlib.Path("app/src/main/java/org/curiouslearning/container/MainActivity.java")
text = p.read_text(encoding="utf-8", errors="ignore").splitlines(True)
def print_method(name):
# locate signature like 'protected void onCreate(' or 'protected void onNewIntent('
for i,l in enumerate(text):
if re.search(r'\b%s\s*\(' % re.escape(name), l):
# locate method body braces balance
start=i
# find first '{' after start
j=start
while j < len(text) and '{' not in text[j]:
j+=1
brace=0
end=None
for k in range(j, len(text)):
brace += text[k].count('{')
brace -= text[k].count('}')
if brace==0:
end=k
break
print(f"\n--- {name} lines {start+1}-{end+1} ---")
print(''.join(text[start:end+1]))
return True
return False
print_method("onCreate")
print_method("onNewIntent")
PY
echo
echo "== Show showConfirmIdDialog around prefs write (editor.putString) =="
# print a focused window around the editor.putString and related gating flags
rg -n "editor\\.putString\\(AnalyticsUtils\\.STUDY_USER_ID" -n "$MAIN"
LINE=$(rg -n "editor\\.putString\\(AnalyticsUtils\\.STUDY_USER_ID" "$MAIN" | head -n1 | cut -d: -f1)
START=$((LINE-40))
END=$((LINE+40))
awk -v s=$START -v e=$END 'NR>=s && NR<=e {print NR ":" $0}' "$MAIN"
echo
echo "== showConfirmIdDialog full gating checks (storedStudyUserId + isHandlingIdConfirmation) =="
# show from method start to end by printing region around 'showConfirmIdDialog('
# first find method signature line
METHOD_LINE=$(rg -n "showConfirmIdDialog\\(" "$MAIN" | head -n1 | cut -d: -f1)
START=$((METHOD_LINE-10))
END=$((METHOD_LINE+140))
awk -v s=$START -v e=$END 'NR>=s && NR<=e {print NR ":" $0}' "$MAIN"
echo
echo "== DeepLinkHelper: how it starts MainActivity (flags/task-affinity) =="
rg -n "class DeepLinkHelper|new Intent\\(activity, MainActivity\\.class\\)|startActivity\\(mainIntent\\)" app/src/main/java/org/curiouslearning/container/utilities/DeepLinkHelper.java
awk 'NR>=1 && NR<=120 {print NR ":" $0}' app/src/main/java/org/curiouslearning/container/utilities/DeepLinkHelper.java
echo
echo "== AndroidManifest: show WebApp activity block (launchMode/task flags if any) =="
awk 'NR>=20 && NR<=60 {print NR ":" $0}' app/src/main/AndroidManifest.xmlRepository: curiouslearning/CRcontainer
Length of output: 25574
Fix enrollment deep-link gating: singleTop doesn’t prevent a second MainActivity instance from showing the confirm dialog while WebApp is foreground.
android:launchMode="singleTop"
android:documentLaunchMode="never"
android:exported="true">study_user_id deep links are handled from onCreate and onNewIntent via handleIncomingIntent. With launchMode="singleTop", onNewIntent only fires when MainActivity is already the top activity; if .WebApp is in the foreground, a new MainActivity instance can be created and onCreate will re-run the enrollment handling.
The “in-flight” guards (isHandlingIdConfirmation / isShowingEnrollmentSuccess) are instance fields, while the durable write-once check (prefs[AnalyticsUtils.STUDY_USER_ID]) is only written after the user taps confirm in showConfirmIdDialog. So there’s a window where a second instance can show the confirmation dialog again before the first instance persists STUDY_USER_ID.
documentLaunchMode="never" looks correct.
🤖 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/AndroidManifest.xml` around lines 52 - 54, The manifest should
prevent creating a second MainActivity during enrollment deep-links: change
android:launchMode from "singleTop" to "singleTask" for MainActivity so incoming
study_user_id intents always route to the existing activity (triggering
onNewIntent and handleIncomingIntent) instead of creating a new instance that
can re-run onCreate and bypass the instance guards (isHandlingIdConfirmation /
isShowingEnrollmentSuccess) before prefs[AnalyticsUtils.STUDY_USER_ID] is
persisted; keep android:documentLaunchMode="never" and ensure
handleIncomingIntent remains invoked from onNewIntent.
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.
Changes
How to test
Ref:
Summary by CodeRabbit
New Features
Bug Fixes
Chores