Skip to content

CI-915 Keep The Login Pipeline Alive Across Rotation - #3890

Open
OrangeAndGreen wants to merge 3 commits into
CI-915-progress-dialog-state-loss-fixfrom
CI-915-login-rotation-viewmodel
Open

CI-915 Keep The Login Pipeline Alive Across Rotation#3890
OrangeAndGreen wants to merge 3 commits into
CI-915-progress-dialog-state-loss-fixfrom
CI-915-login-rotation-viewmodel

Conversation

@OrangeAndGreen

@OrangeAndGreen OrangeAndGreen commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

CI-915

Stacked on PR #3881 that fixes the initial crash when trying to show the dialog on resume.

Product Description

Rotating the device during login no longer breaks it. Previously, a rotation mid-sync aborted the data pull, left the progress dialog frozen part-way, and never completed or failed the login — the user had to force-quit and start over.

Technical Summary

  • The login pipeline was launched on LoginActivity's lifecycleScope, which is cancelled at onDestroy
    • A rotation therefore cancelled the coroutine
    • And SyncOperations cancelled DataPullTask from its invokeOnCancellation handler
  • So the restore was aborted, no result was ever delivered, and the dialog was orphaned

Pre-2.64 this was handled by the task-connector framework, but the headless login engine opted out of that and uses HeadlessTaskConnector.connectTask instead.

This PR introduces LoginViewModel to own the job in viewModelScope so it can survive configuration changes.

A couple extra notes:

  • currentLoginPhase stayed in LoginActivity so it gets reset on activity recreation and is used to show the dialog again
  • The result is single-use so complete login doesn't get replayed if the screen rotates after the job completes

Safety Assurance

Safety story

  • I verified that I am no longer able to reproduce either of the issues before related to sync during login:
  1. App does not crash on resume if sent to background during sync
  2. Sync dialog is not orphaned if the screen is rotated during sync
  • Did some general usage testing around the login process and made sure everything appears to work normally

Automated test coverage

Four tests added to LoginProgressDialogLifecycleTest:

  • rotation keeps the login running and puts its dialog back
  • stop button cancels a pipeline that survived rotation
  • a consumed result is not redelivered after rotation
  • a finished login leaves no dialog behind on rotation

@OrangeAndGreen OrangeAndGreen self-assigned this Aug 28, 2026
@OrangeAndGreen

Copy link
Copy Markdown
Contributor Author

Suggested Review Order

  • app/src/org/commcare/login/LoginController.kt — the deleted start() bridge; the bug's origin, and smallest context to load first
  • app/src/org/commcare/login/LoginViewModel.kt — its replacement and the new owner of the job; the comments explain the three non-obvious constraints
  • app/src/org/commcare/activities/LoginActivity.java — the consumer: two observers replace the two activity-bound callbacks
  • app/unit-tests/src/org/commcare/activities/LoginProgressDialogLifecycleTest.kt — the four new rotation tests pin the behaviour described above
  • RELEASES.md — release note and QA steps; each is its own commit

@codecov

codecov Bot commented Aug 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 28.14%. Comparing base (14fec4a) to head (a8702ec).

Additional details and impacted files
@@                             Coverage Diff                             @@
##             CI-915-progress-dialog-state-loss-fix    #3890      +/-   ##
===========================================================================
+ Coverage                                    27.92%   28.14%   +0.21%     
- Complexity                                    4885     4923      +38     
===========================================================================
  Files                                          987      988       +1     
  Lines                                        58969    58996      +27     
  Branches                                      7022     7023       +1     
===========================================================================
+ Hits                                         16465    16602     +137     
+ Misses                                       40525    40405     -120     
- Partials                                      1979     1989      +10     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@OrangeAndGreen
OrangeAndGreen marked this pull request as ready for review August 28, 2026 19:24
OrangeAndGreen and others added 3 commits August 28, 2026 15:34
The login pipeline ran on LoginActivity's lifecycleScope, which is cancelled
at onDestroy. Rotating mid-login therefore tore it down: SyncOperations
cancels DataPullTask when its continuation is cancelled, so the restore was
aborted part-way, no result was ever delivered, and the progress dialog was
left up for a login that was no longer running.

Pre-2.64 this was handled by the task-connector framework - DataPullTask was
held in TaskConnectorViewModel and re-attached to the recreated activity. The
headless login engine opted out of that (HeadlessTaskConnector.connectTask is
a no-op), so restore an equivalent: LoginViewModel owns the job in
viewModelScope, which survives configuration changes and is cancelled only
when the activity genuinely finishes.

Progress and result are exposed as LiveData so the recreated activity
re-subscribes instead of being handed callbacks bound to the destroyed
instance. Replaying progress is what rebuilds the dialog after a rotation;
currentLoginPhase deliberately stays on the activity, because it is the
phase-vs-null mismatch on a fresh instance that triggers the re-show. The
result is single-use so a rotation after login completes does not hand the
same outcome to the next activity.

LoginController.start() existed only to bridge into lifecycleScope and had a
single caller, so it is removed rather than adapted; the ViewModel calls
performLogin directly, as ConnectAppLauncher already does. Passing the
application context also drops the activity reference the pipeline's
collaborators used to hold for its whole duration.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.

1 participant