Improved form entry cleanup #3519
Conversation
…er code knows when form entry is in progress).
📝 WalkthroughWalkthroughIn FormEntryActivity's onDestroy method, a null assignment for mFormController has been added before TextToSpeech shutdown and the super.onDestroy call. This clears the static form controller reference during activity destruction without altering existing control flow or functional behavior. Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![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 `@app/src/org/commcare/activities/FormEntryActivity.java`:
- Line 1255: The onDestroy() currently unconditionally sets mFormController =
null which breaks the config-change survival path and can trigger the
SessionUnavailableException in the subsequent instance; change the logic in
FormEntryActivity.onDestroy so that mFormController is only cleared when the
Activity is actually finishing (use isFinishing() or equivalent) and leave
mFormController intact across rotations/config changes (refer to the
mFormController field and the code paths around SessionUnavailableException and
the config-change survival logic).
| } | ||
| } | ||
|
|
||
| mFormController = null; |
There was a problem hiding this comment.
Avoid clearing mFormController during configuration changes.
onDestroy() is invoked on rotations; clearing the static controller can make the next instance hit the SessionUnavailableException path (Line 242) since mFormController is used to survive config changes (see Line 733). Guard the nulling to only run when the activity is truly finishing.
💡 Suggested fix
- mFormController = null;
+ if (!isChangingConfigurations()) {
+ mFormController = null;
+ }📝 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.
| mFormController = null; | |
| if (!isChangingConfigurations()) { | |
| mFormController = null; | |
| } |
🤖 Prompt for AI Agents
In `@app/src/org/commcare/activities/FormEntryActivity.java` at line 1255, The
onDestroy() currently unconditionally sets mFormController = null which breaks
the config-change survival path and can trigger the SessionUnavailableException
in the subsequent instance; change the logic in FormEntryActivity.onDestroy so
that mFormController is only cleared when the Activity is actually finishing
(use isFinishing() or equivalent) and leave mFormController intact across
rotations/config changes (refer to the mFormController field and the code paths
around SessionUnavailableException and the config-change survival logic).
https://dimagi.atlassian.net/browse/CCCT-2025
Product Description
No user visible change.
Technical Summary
Nulling out mFormController when FormEntryActivity shuts down (so other code knows when form entry is in progress).
This work is being done as part of a ticket to better handle kicking the user out when their PersonalID configuration is invalidated. But if the user is in the middle of form entry, we want to let them finish before interrupting their experience. This change will support that work.
Feature Flag
None
Safety Assurance
Safety story
Tested entering a form and backing out, then entering again. No issues encountered
Automated test coverage
Existing tests should cover this part of the code well (exiting form entry in various ways).
QA Plan
Test the functionality around exiting form entry (backing out or completing the form).