CCCT-2439 Launch Connect Apps Without Showing The Login Screen

[conroy-ricketts]

CCCT-2439

Product Description

When a worker launches an installed app from a Connect opportunity, it now opens directly behind a single progress dialog instead of briefly flashing the LoginActivity and app-seating screens. Backing out of the app's Home screen returns to the Connect opportunities list, as before.

Here is the new login flow (I intentionally slowed it down for this demo so every phase is shown):

Screen_Recording_20260605_131211_CommCare.Debug.mp4

Here is the old login flow (I intentionally slowed it down for this demo so every phase is shown):

Screen_Recording_20260605_131613_CommCare.Debug.mp4

Technical Summary

Phase 3 of the headless-login work (builds on CCCT-2437/CCCT-2438). ConnectAppLauncher runs the seat + PersonalID login headlessly off the opportunities list and reports a LaunchOutcome; LaunchOutcomeRouter/LaunchActions is a thin seam (unit-tested without a fragment) that maps each outcome to a UI/nav action. ConnectJobsListsFragment drives it on viewLifecycleOwner.lifecycleScope behind a modal CustomProgressDialog.

A few decisions worth knowing before the diff:

• Failures reuse existing surfaces: TokenDenied → TokenExceptionHandler; a seat failure → DispatchActivity (which reaches recovery for a corrupted app); credential/other failures → fall back to the legacy ConnectAppUtils.launchApp, which shows LoginActivity with its usual error handling (no new error UI/strings).
• Reverted phase-2 over-engineering: this branch also inlines DispatchActivity's post-login intent handling back to its master form and deletes the now-unused PostLoginRouter/PostLoginDestination/LaunchContext (+ test). They were added in CCCT-2438 on the expectation phase 3 would consume them, but phase 3 doesn't — route() only ever produced Home there, so the round-trip was behavior-identical to the inline version.
• Trimmed the seating result API: SeatResult is now just Success/Failed, dropping the plan's per-reason SeatFailure enum and the SeatResultCallback seam. No caller acts on the reason — a seat failure is already surfaced through the app's STATE_CORRUPTED state that DispatchActivity detects — so the extra detail was unused.

Open Question

Open question for the team (not addressed here): the progress dialog shows the same text in both its title and body during the seating and login phases (pre-existing for the login phase; introduced for seating to match). Is this worth tweaking?

Safety Assurance

Safety story

What gives me confidence:

• I ran the change on a device and launched apps from the Connect opportunities list end-to-end, confirming the single-dialog open, the seating/login dialog text, and back-from-Home returning to the opportunities list.
• The launcher and outcome routing are covered by JVM unit tests.
• Diff is scoped to the Connect launch path; failures route into pre-existing handlers rather than new code.

Risks to review:

• I did not manually exercise the failure paths (token denied, no network, seat failure, missing Connect user). They're unit-tested at the launcher/router level and delegate to existing handlers, but haven't been run on a device.
• Home is launched directly (not via DispatchActivity), so the session stays active with the Connect screen on the back stack and the HOME_SCREEN-for-result logout/back contract is bypassed for this entry point — worth confirming logout/back behavior is acceptable.
• No Robolectric fragment test for ConnectJobsListsFragment (no fragment-Robolectric harness exists in the suite); the routing decision is covered via the LaunchActions seam instead.

Automated test coverage

• ConnectAppLauncherTest — phase/credential resolution, seat-failure short-circuit, and LoginResult → LaunchOutcome mapping for each failure variant.
• LaunchOutcomeRouterTest — every outcome dismisses progress first and dispatches the expected LaunchActions call.

[conroy-ricketts]

Suggested Review Order

• app/src/org/commcare/connect/ConnectAppLauncher.kt — new core: the headless seat+login flow and the LaunchOutcome result type
• app/src/org/commcare/connect/LaunchOutcomeRouter.kt — the LaunchActions seam mapping each outcome to a nav/UI action
• app/src/org/commcare/fragments/connect/ConnectJobsListsFragment.java — the caller: progress dialog, lifecycle scope, and the action implementations
• app/src/org/commcare/activities/HomeScreenBaseActivity.java — launchHome/buildHomeIntent used on success
• app/src/org/commcare/activities/DispatchActivity.java — Connect home routing plus the inlined post-login handling (phase-2 revert)
• app/src/org/commcare/login/PostLoginRouter.kt, PostLoginDestination.kt, app/unit-tests/.../PostLoginRouterTest.kt (deleted) — the reverted, now-unused routing abstraction
• app/unit-tests/src/org/commcare/connect/ConnectAppLauncherTest.kt — outcome/credential/seat-failure coverage
• app/unit-tests/src/org/commcare/connect/LaunchOutcomeRouterTest.kt — outcome→action dispatch coverage
• docs/commcare/login-engine.md — high-level doc for phases 1–3

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 25.86%. Comparing base (d6eed61) to head (c663ef9).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #3753      +/-   ##
============================================
+ Coverage     25.80%   25.86%   +0.05%     
- Complexity     4364     4386      +22     
============================================
  Files           947      949       +2     
  Lines         56993    57125     +132     
  Branches       6782     6804      +22     
============================================
+ Hits          14709    14776      +67     
- Misses        40455    40521      +66     
+ Partials       1829     1828       -1     

☔ 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.

[conroy-ricketts]

@coderabbitai full review

[coderabbitai]

✅ Action performed

Full review finished.

[coderabbitai]

Need an answer fast? Review this PR in Change Stack to ask focused questions about the PR or a changed range.

📝 Walkthrough

Walkthrough

This PR refactors the Connect app launch flow by introducing ConnectAppLauncher (a background launcher that closes sessions, seats apps, resolves credentials, and executes password-mode login) and LaunchOutcomeRouter (which deterministically dispatches terminal outcomes to outcome-specific handlers). It removes the obsolete PostLoginDestination/PostLoginRouter abstraction, centralizes home-screen intent building via HomeScreenBaseActivity.buildHomeIntent, simplifies DispatchActivity login result handling, and integrates the new launcher into ConnectJobsListsFragment with progress dialog lifecycle management. Comprehensive tests validate launcher behavior (happy path, credential resolution, seat failures, login failures) and router dispatch logic. Documentation is updated to describe the new flow and app seating mechanism.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

The PR spans multiple architectural layers (new launcher/router abstractions, activity simplification, fragment integration) with dense logic in the launcher and fragment outcome handling. Multiple files are affected across activities, fragments, and tests. The removal of old abstractions requires tracing usage across multiple locations. The new sealed-type outcome model and dispatch pattern introduce new concepts that require careful validation against the test coverage.

Possibly related PRs

• dimagi/commcare-android#3721: Modifies DispatchActivity's home-launch intent handling and login/session-related intent extras.
• dimagi/commcare-android#3741: Adds StandardHomeActivity logic to react to PERSONALID_MANAGED_LOGIN intent extra, which this PR updates in HomeScreenBaseActivity.buildHomeIntent.

Suggested labels

skip-integration-tests

Suggested reviewers

• avazirna
• OrangeAndGreen
• Jignesh-dimagi

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 3.85% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically summarizes the main objective of the PR: launching Connect apps without showing the login screen.
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.
Description check	✅ Passed	The pull request description is comprehensive and follows the template structure with all major sections completed: Product Description with user-facing effects and video demos, Technical Summary with linked ticket and design decisions, and Safety Assurance with confidence factors, tested scenarios, and identified risks.

_{✏️ 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 CCCT-2439-connect-login-silent-launch-path

---

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

[coderabbitai]

Actionable comments posted: 4

🧹 Nitpick comments (4)

app/src/org/commcare/fragments/connect/ConnectJobsListsFragment.java (1)

79-80: ⚡ Quick win

Remove the newly added inline comment in Java source.

This addition conflicts with the repository rule for Java/Kotlin source comments.

As per coding guidelines **/*.{kt,java}: "Do not add any in-code comments unless explicitly requested".

🤖 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/org/commcare/fragments/connect/ConnectJobsListsFragment.java` around
lines 79 - 80, Remove the newly added inline comment attached to the
LAUNCH_DIALOG_TASK_ID constant in ConnectJobsListsFragment (the line starting
with "Negative so it can't collide..."); leave only the field declaration
"private static final int LAUNCH_DIALOG_TASK_ID = -10;" in the
ConnectJobsListsFragment class to comply with the repository rule banning
in-code comments in Java/Kotlin files.

app/src/org/commcare/connect/LaunchOutcomeRouter.kt (1)

3-3: ⚡ Quick win

Remove newly added in-code comments in this Kotlin file.

The added comments conflict with the repo rule for Java/Kotlin source files.

As per coding guidelines **/*.{kt,java}: "Do not add any in-code comments unless explicitly requested".

Also applies to: 18-18

🤖 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/org/commcare/connect/LaunchOutcomeRouter.kt` at line 3, Remove the
newly added in-code comments in LaunchOutcomeRouter.kt (e.g., the comment
mentioning "Actions a caller runs for a [LaunchOutcome]; a seam so the mapping
is testable without a fragment" and the other comment at the same file) so the
file complies with the repository rule forbidding in-code comments in .kt/.java
files; locate any occurrences referencing LaunchOutcome or similar inline
comments and delete them (if explanation is required, move it to external
documentation or a separate README rather than leaving inline comments).

app/src/org/commcare/connect/ConnectAppLauncher.kt (1)

23-26: ⚡ Quick win

Remove newly added in-code comments in Kotlin segments.

These added KDoc/inline comments violate the repo rule for Java/Kotlin files.

As per coding guidelines **/*.{kt,java}: "Do not add any in-code comments unless explicitly requested".

Also applies to: 41-44, 60-60

🤖 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/org/commcare/connect/ConnectAppLauncher.kt` around lines 23 - 26,
Remove the newly added KDoc/inline comments in the Kotlin file: delete the
comment block describing the "Terminal result of a Connect launch" and any other
in-code KDoc/comments around the sealed outcome types (e.g., the comment
referencing [Retryable]) as well as the other comment blocks at the other
ranges; leave the Kotlin declarations (such as the sealed result types and the
Retryable symbol) untouched and ensure no in-code comments remain in this file
per the Java/Kotlin guideline.

app/unit-tests/src/org/commcare/connect/ConnectAppLauncherTest.kt (1)

60-60: 💤 Low value

Remove unused stub.

The reportCccAppFailedAutoLogin stub is never called or verified in any test method. Per coding guidelines, unused code should be cleaned up.

♻️ Proposed cleanup

         mockkStatic(FirebaseAnalyticsUtil::class)
         every { FirebaseAnalyticsUtil.reportCccAppLaunch(any(), any()) } returns Unit
-        every { FirebaseAnalyticsUtil.reportCccAppFailedAutoLogin(any()) } returns Unit
     }

🤖 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/unit-tests/src/org/commcare/connect/ConnectAppLauncherTest.kt` at line
60, Remove the unused mock/stub for
FirebaseAnalyticsUtil.reportCccAppFailedAutoLogin in ConnectAppLauncherTest (the
line using every { FirebaseAnalyticsUtil.reportCccAppFailedAutoLogin(any()) }
returns Unit) since it is never invoked or asserted; simply delete that line to
clean up the test setup and ensure no other tests rely on that stub.

🤖 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/org/commcare/activities/DispatchActivity.java`:
- Around line 511-516: Ensure every RESULT_OK exit from LoginActivity supplies
an Intent with the login-result extras DispatchActivity expects: always call
setResult(RESULT_OK, intent) where intent includes LoginActivity.LOGIN_MODE
(serializable LoginMode), LoginActivity.MANUAL_SWITCH_TO_PW_MODE (boolean),
DispatchActivity.PERSONALID_MANAGED_LOGIN (boolean) and
DispatchActivity.CONNECT_MANAGED_LOGIN (boolean); specifically update the
shouldFinish() path, the RecoveryMeasuresHelper.recoveryMeasuresPending()
return, and handleConnectButtonPress() to populate those keys (or centralize
into a helper that builds the result Intent) so DispatchActivity’s reads of
lastLoginMode, userManuallyEnteredPasswordMode, personalIdManagedLogin and
connectManagedLogin never see a null/missing Intent.

In `@app/src/org/commcare/connect/ConnectAppLauncher.kt`:
- Around line 89-91: The code in ConnectAppLauncher.kt uses locale-sensitive
lowercase() when normalizing the login identifier; update the connectUsername
usage so username = connectUsername(context)?.trim()?.lowercase(Locale.ROOT) (or
Locale.ENGLISH) instead of lowercase(), and add the necessary import for
java.util.Locale; keep the existing null/empty check using
username.isNullOrEmpty() unchanged.

In `@app/src/org/commcare/fragments/connect/ConnectJobsListsFragment.java`:
- Around line 299-317: The dialog is being shown with launchDialog.showNow(...)
which can throw IllegalStateException if the fragment manager state is already
saved; update showLaunchDialog(...) to first check
getChildFragmentManager().isStateSaved() and if true, return early (do this
check before calling dismissLaunchDialog() so you don't null out launchDialog
and cause updateLaunchProgress() to NPE), otherwise call dismissLaunchDialog()
and use launchDialog.show(...) (not showNow) to safely enqueue the dialog;
reference the methods/fields showLaunchDialog, dismissLaunchDialog,
launchDialog, updateLaunchProgress, ConnectAppLauncher callbacks,
showingSyncDialog, LAUNCH_DIALOG_TASK_ID and LAUNCH_DIALOG_TAG when making the
change.

In `@app/unit-tests/src/org/commcare/connect/ConnectAppLauncherTest.kt`:
- Around line 33-37: Reset the mutable capture lists before each test to avoid
cross-test pollution: in the setUp() method clear the mutable lists `seatedApps`
and `capturedRequests` (and if present reinitialize `seatResult`, `loginAnswer`,
and `username` to their defaults) so each test starts with fresh state; locate
the fields `seatedApps`, `capturedRequests`, `seatResult`, `loginAnswer`, and
`username` in ConnectAppLauncherTest and add calls to clear or reassign them in
the test class's `setUp()` method.

---

Nitpick comments:
In `@app/src/org/commcare/connect/ConnectAppLauncher.kt`:
- Around line 23-26: Remove the newly added KDoc/inline comments in the Kotlin
file: delete the comment block describing the "Terminal result of a Connect
launch" and any other in-code KDoc/comments around the sealed outcome types
(e.g., the comment referencing [Retryable]) as well as the other comment blocks
at the other ranges; leave the Kotlin declarations (such as the sealed result
types and the Retryable symbol) untouched and ensure no in-code comments remain
in this file per the Java/Kotlin guideline.

In `@app/src/org/commcare/connect/LaunchOutcomeRouter.kt`:
- Line 3: Remove the newly added in-code comments in LaunchOutcomeRouter.kt
(e.g., the comment mentioning "Actions a caller runs for a [LaunchOutcome]; a
seam so the mapping is testable without a fragment" and the other comment at the
same file) so the file complies with the repository rule forbidding in-code
comments in .kt/.java files; locate any occurrences referencing LaunchOutcome or
similar inline comments and delete them (if explanation is required, move it to
external documentation or a separate README rather than leaving inline
comments).

In `@app/src/org/commcare/fragments/connect/ConnectJobsListsFragment.java`:
- Around line 79-80: Remove the newly added inline comment attached to the
LAUNCH_DIALOG_TASK_ID constant in ConnectJobsListsFragment (the line starting
with "Negative so it can't collide..."); leave only the field declaration
"private static final int LAUNCH_DIALOG_TASK_ID = -10;" in the
ConnectJobsListsFragment class to comply with the repository rule banning
in-code comments in Java/Kotlin files.

In `@app/unit-tests/src/org/commcare/connect/ConnectAppLauncherTest.kt`:
- Line 60: Remove the unused mock/stub for
FirebaseAnalyticsUtil.reportCccAppFailedAutoLogin in ConnectAppLauncherTest (the
line using every { FirebaseAnalyticsUtil.reportCccAppFailedAutoLogin(any()) }
returns Unit) since it is never invoked or asserted; simply delete that line to
clean up the test setup and ensure no other tests rely on that stub.

🪄 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: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 5a270819-7f56-4402-a1cb-1ebf071daae2

📥 Commits

Reviewing files that changed from the base of the PR and between 94d61c1 and b847f7f.

📒 Files selected for processing (12)

• RELEASES.md
• app/src/org/commcare/activities/DispatchActivity.java
• app/src/org/commcare/activities/HomeScreenBaseActivity.java
• app/src/org/commcare/connect/ConnectAppLauncher.kt
• app/src/org/commcare/connect/LaunchOutcomeRouter.kt
• app/src/org/commcare/fragments/connect/ConnectJobsListsFragment.java
• app/src/org/commcare/login/PostLoginDestination.kt
• app/src/org/commcare/login/PostLoginRouter.kt
• app/unit-tests/src/org/commcare/connect/ConnectAppLauncherTest.kt
• app/unit-tests/src/org/commcare/connect/LaunchOutcomeRouterTest.kt
• app/unit-tests/src/org/commcare/login/PostLoginRouterTest.kt
• docs/commcare/login-engine.md

💤 Files with no reviewable changes (3)

• app/unit-tests/src/org/commcare/login/PostLoginRouterTest.kt
• app/src/org/commcare/login/PostLoginDestination.kt
• app/src/org/commcare/login/PostLoginRouter.kt

[OrangeAndGreen]

Nit: This function no longer takes a result so the name is misleading. Maybe rename it to setResultAndFinish instead?

[conroy-ricketts]

Good eye!

[conroy-ricketts]

8153e70

The base branch was changed.