Skip to content

refactor: Disabling cr_user_id modification! - #261

Merged
dz4va merged 1 commit into
developfrom
amit_temp
May 29, 2026
Merged

refactor: Disabling cr_user_id modification!#261
dz4va merged 1 commit into
developfrom
amit_temp

Conversation

@amitsinghsutara

@amitsinghsutara amitsinghsutara commented May 29, 2026

Copy link
Copy Markdown
Collaborator

Summary by CodeRabbit

  • Bug Fixes

    • Confirmation messages are now truncated to 800 characters to prevent display issues.
    • Added validation to ensure a user identifier exists before processing enrollment confirmation.
  • Refactor

    • Improved language popup dropdown sizing calculation for better responsiveness.
    • Updated study enrollment data handling to streamline the confirmation process.

Review Change Stack

@amitsinghsutara
amitsinghsutara requested a review from dz4va May 29, 2026 06:35
@coderabbitai

coderabbitai Bot commented May 29, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

MainActivity refactors study user enrollment to cache pseudoId before processing confirmations, separates enrollment persistence (storing study_user_id under AnalyticsUtils.STUDY_USER_ID) from pseudoId management, and refactors dialog window formatting and language dropdown height calculations for clarity.

Changes

Study Enrollment Flow and Analytics

Layer / File(s) Summary
pseudoId caching in enrollment entry point
app/src/main/java/org/curiouslearning/container/MainActivity.java
handleIncomingIntent() now calls cachePseudoId() to ensure a pseudoId exists before processing incoming study_user_id confirmation; existing consent validation and message truncation remain intact.
Confirmation dialog UI setup
app/src/main/java/org/curiouslearning/container/MainActivity.java
showConfirmIdDialog() window background setup is reformatted with equivalent logic while keeping dialog wiring and new user id TextView assignment.
Enrollment persistence and analytics refactoring
app/src/main/java/org/curiouslearning/container/MainActivity.java
Confirm button handler stores newId under AnalyticsUtils.STUDY_USER_ID instead of overwriting pseudoId; analytics logging reads the stored pseudoId from preferences instead of using newId, updates debug overlay, reloads language apps, and shows success dialog with reformatted window background.
Language dropdown height calculation refactoring
app/src/main/java/org/curiouslearning/container/MainActivity.java
showLanguagePopup() dropdown sizing logic is reorganized into intermediate variables (screen metrics, reserved space, trigger location, available height, clamping, and safety minimum) while preserving computed result.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • curiouslearning/CRcontainer#259: Both PRs modify MainActivity's joined-study confirmation flow to use AnalyticsUtils.logJoinedStudyEvent() and its pseudoId/ID handling instead of prior analytics calls.
  • curiouslearning/CRcontainer#254: Both PRs modify MainActivity's intent handling and ID confirmation dialog flow including confirm-button persistence and analytics around pseudoId/study user id.
  • curiouslearning/CRcontainer#258: Both PRs modify ID confirmation/enrollment to persist and use AnalyticsUtils.STUDY_USER_ID instead of overwriting pseudoId and to drive the success-dialog UX.

Suggested reviewers

  • dz4va

Poem

🐰 A pseudoId now cached with care, before confirmations fill the air,
No more pseudoId overwrites—just store the STUDY_USER_ID right!
Dropdowns sized with variables clear, analytics flows like crystal here,
The enrollment dance, a refactored grace! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Tests & Lint & Coverage ❓ Inconclusive Repository clone failed, so this custom check could not run with code access. Retry the review run. If this persists, inspect pre-merge custom-check logs for infrastructure or agent runtime failures.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: disabling the modification of cr_user_id (pseudoId), which aligns with the core refactoring where the confirm button handler no longer overwrites the app's pseudoId with newId.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch amit_temp

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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`:
- Around line 906-907: The comment above the bottomReservedSpace calculation is
incorrect (says 20% while the code uses 10%); update the comment to state 10% to
match the computed value (int bottomReservedSpace = (int) (screenHeight *
0.10f)) so the intent and code align—locate the bottomReservedSpace calculation
near screenHeight usage in MainActivity and change the comment text 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.yaml

Review profile: CHILL

Plan: Pro

Run ID: 565c14b3-283e-4803-975e-3a1ea5c976a8

📥 Commits

Reviewing files that changed from the base of the PR and between b6e3eb5 and 7cc95e7.

📒 Files selected for processing (1)
  • app/src/main/java/org/curiouslearning/container/MainActivity.java

Comment on lines +906 to 907
// Reserve bottom space (20% of screen)
int bottomReservedSpace = (int) (screenHeight * 0.10f);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Comment contradicts the code (10% vs 20%).

The comment says 20% but the computation reserves 10% (0.10f). Per the refactor's intent of preserving behavior, the code is correct; align the comment to avoid a future "fix" that changes the reserved space.

📝 Proposed comment fix
-                        // Reserve bottom space (20% of screen)
+                        // Reserve bottom space (10% of screen)
                         int bottomReservedSpace = (int) (screenHeight * 0.10f);
📝 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
// Reserve bottom space (20% of screen)
int bottomReservedSpace = (int) (screenHeight * 0.10f);
// Reserve bottom space (10% of screen)
int bottomReservedSpace = (int) (screenHeight * 0.10f);
🤖 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 906 - 907, The comment above the bottomReservedSpace calculation is
incorrect (says 20% while the code uses 10%); update the comment to state 10% to
match the computed value (int bottomReservedSpace = (int) (screenHeight *
0.10f)) so the intent and code align—locate the bottomReservedSpace calculation
near screenHeight usage in MainActivity and change the comment text accordingly.

@dz4va
dz4va merged commit 5554aa3 into develop May 29, 2026
1 check passed
@amitsinghsutara
amitsinghsutara deleted the amit_temp branch June 18, 2026 04:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants