CCCT-2441 Silent App Launch For Job Intro And Download Screens#3765
CCCT-2441 Silent App Launch For Job Intro And Download Screens#3765conroy-ricketts wants to merge 21 commits into
Conversation
[AI] Migrated the job-intro and downloading screens to the silent Connect launch path and removed the now-dead IS_LAUNCH_FROM_CONNECT flag and its LoginActivity/DispatchActivity auto-login branches. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
[AI] Guarded the Connect app-launch controller against a destroyed fragment view so an in-flight launch fired from an async callback can no longer crash with IllegalStateException. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
[AI] Collapsed the redundant nested seated-app login check into a single condition and trimmed the login-engine doc to describe only the current Connect launch behavior. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
[AI] Made the job-intro and download launch surfaces drop themselves from the Connect back stack after a successful launch, deferring the pop until Home is in front so no blank frame flashes on the way to Home. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ining-connect-pages
[AI] Fixed a crash when launching freshly-installed Connect apps back-to-back: the already-logged-in shortcut now confirms the active session belongs to the seated app, so an install that re-seats a different app while a prior session is alive no longer skips re-login. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
[AI] Stopped offering endless retries on a failing Connect app launch: after the worker retries twice, the dialog now shows a dismiss-only "check your connection and try again later" message instead of another Retry prompt. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…-2441-new-login-path-for-intro-and-download
[AI] Added QA notes for the job-intro and post-download Connect launch surfaces, back-to-back launch stability, and the after-2-retries persistent-error dialog. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Suggested Review Order
|
📝 WalkthroughWalkthroughThis PR refactors the Connect app launch flow by removing legacy "launched from Connect" state tracking from Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
conroy-ricketts
requested review from
a team,
Jignesh-dimagi,
OrangeAndGreen and
shubham1g5
and removed request for
a team
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3765 +/- ##
============================================
- Coverage 26.01% 25.95% -0.07%
- Complexity 4420 4431 +11
============================================
Files 953 961 +8
Lines 57383 57618 +235
Branches 6829 6869 +40
============================================
+ Hits 14930 14956 +26
- Misses 40615 40826 +211
+ Partials 1838 1836 -2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
[FIXED] The code changes look great but I'm seeing a functionality issue. Here are steps to reproduce:
- Go to the opportunity list and press Resume on an opp where the learn/deliver app needs to be downloaded
- Wait while the Download page appears, the app downloads, and logs in
- Once on App Home, navigate back (i.e. swipe)
The code should navigate to the opportunity list, but instead it navigates to the Download page (showing complete progress)
|
[FIXED] I'm also getting a repeatable crash that seems like it might be related to these changes. Steps:
Here's the exception: |
|
@conroy-ricketts |
|
Noting the two issues I reported are resolved with the latest code |
[AI] Moved the post-launch back-stack pop out of ConnectAppLaunchController: the controller now reports success via an onLaunched callback and the transient fragments remove themselves via BaseConnectFragment.popSelfOnceHidden(), keeping fragment-stack management with the fragments. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
[AI] Invoked the post-launch onLaunched callback before starting Home so the caller's lifecycle observer is registered ahead of the resulting onStop. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
[AI] Tightened the already-logged-in session check to confirm the active session belongs to the current Connect user, not just any user with a key record in the seated app. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This is a great catch @Jignesh-dimagi @OrangeAndGreen @shubham1g5 I'm torn on the best approach here because this does not seem to be a simple fix (please correct me if I'm wrong). The crux of the issue is that we are finishing the Thoughts on this? Unfortunately, I think we may need to re-introduce some sort of "launched from Connect" flag inside |
I think if we close the app session (i.e. logout of the app) then DispatchActivity will route to LoginActivity instead of opening StandardHomeActivity again. |
|
Are we fine with closing the app session regardless of what the user was doing before they landed on the jobs list? i.e. do we care to preserve this behavior (user logs in to an app → opens job list but does not launch another app → user backs out → user sees the same app they were already logged in to)?: Screen_Recording_20260618_155948_CommCare.Debug.mp4 |
I was unable to view your video, as it stops at 2 seconds for some reason. However, I see two possible solutions here:
I personally prefer the second option, but I am not sure if it is feasible to distinguish a Connect user while on StandHomeActivity. |
We should not be closing app sessions unless user logs into another app, the reason is that there are several periodic tasks on CC Appside to sync data, heartbeat, app update etc that depends on user being logged into the app and we would want to keep the session alive as per normal CommCare session timing rules (we log out the user automatically every 24 hours) for both Connect and CommCare apps.
Think that's a decent solution for time-being and I presume this flow to undergo changes with the upcoming re-design which would be a better time to address this issue. |
|
Thanks for the input everyone! I think I'll try to find the best way of re-introducing a "launched from Connect" flag to fix this for now |
[AI] Moved the "session belongs to the current Connect user for this app" check out of ConnectAppLauncher into PersonalIdManager so it can be reused outside the launcher. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…-2441-new-login-path-for-intro-and-download
[AI] Anchored Connect-launched sessions back to LoginActivity on back-out and warm reopen via a session-scoped flag, leaving the session alive; renamed the launch controller's success callback to onLaunchSucceeded. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@OrangeAndGreen @Jignesh-dimagi @shubham1g5 I chose to keep the session open and added a session-scoped flag ( This fixes two issues:
Moreover, I explored the option of reporting both launch successes and failures to the fragment, and I think it's not necessary because all failures that can occur while launching a CC app is already handled by the launch controller before the user lands on the fragment. So I kept the success callback only and renamed it to I kept the "pop backstack" logic as-is for now to prevent the screen from flashing for the user when they back out of the CC app home screen. Commit - 392c0b1 I think that covers everything remaining that's been said so far. Please let me know if I missed anything or if there are any strong opinions |
conroy-ricketts
requested review from
Jignesh-dimagi,
OrangeAndGreen and
shubham1g5
This is the current requirement. However, as we move forward with the latest design changes, we need to inform the calling fragment of both success and failure. Please see the example below, where launch controller error processing is not required, yet the calling fragments still need to update the UI based on the error. Additionally, as previously discussed, we need to reduce the responsibility of the launch controller and avoid handling the fragment lifecycle within it. 
I think if launch controller starts the home activity first and then informs the calling fragment, where it will pop back and should not give flashing or UI jerk. |
[AI] Reworked back navigation for apps launched from Connect so back returns to the opportunities list and exits the app cleanly instead of collapsing the stack or looping. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…-2441-new-login-path-for-intro-and-download
[AI] Added an onLaunchFailed callback so a launching fragment is notified of terminal launch failures (retries exhausted, retry cancelled, or seat failure) without moving failure handling out of the controller. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@Jignesh-dimagi @shubham1g5 @OrangeAndGreen This is ready for another round of review. I proceeded with solution A as described in this doc. I believe the cons are worth it given that the app navigation behavior will change significantly with the Connect Redesign. I dev tested this to make sure the edge cases match the desired behavior. Commit - d49fb11.
@Jignesh-dimagi I implemented a minimal version of this for now (to be used later). Moving ownership of failure handling to the fragment (and reducing the responsibility of the launch controller) will be a decent scope increase and I think that's worth a new ticket. Commit - 2353f1c. |
[AI] Added QA notes covering exit-on-back from the opportunities list and the already-signed-in relaunch back behavior. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| * it on back; deferring to onStop keeps the pop from briefly flashing the destination beneath. | ||
| */ | ||
| protected fun popSelfOnceHidden() { | ||
| lifecycle.addObserver( |
There was a problem hiding this comment.
@conroy-ricketts Why its required to add the lifecycle just for popping the fragment off the back stack. Just NavHostFragment.findNavController(this).popBackStack() should be sufficient.
There was a problem hiding this comment.
This is to avoid the screen flashing on the user launching an app. Here's a quick video demonstrating this:
Screen_Recording_20260626_102539_CommCare.Debug.mp4
Notice how the Job List page flashes in the video when the app is launched.
If we simply use NavHostFragment.findNavController(this).popBackStack(), the screen flashes because the immediate pop happens while the ConnectActivity is still visible and in the foreground. So, popping the fragment at onStop guarantees that it happens while it's already hidden
I think a simple interface between the controller and the fragment could work here, but I will leave it up to you and accept your final call. |
| - Launching via a job's **Start** button (opportunity intro screen) and right after an **app download/install** finishes both open directly behind the single progress dialog (no login/app-setup flash); backing out of the app home returns to the opportunities list, not the intro/download screen. | ||
| - Launch several **not-yet-installed** apps back-to-back (open one, back out, open the next, repeat) and confirm none crash. | ||
| - After repeated launch failures, on the **third consecutive failure** (i.e. after retrying twice) the dialog shows a single **OK** "couldn't open the app after several tries — check your connection and try again later" message instead of another Retry prompt. | ||
| - After launching an app from Connect and backing out to the opportunities list, press back once more: verify the app exits cleanly rather than re-opening the launched app's home. |
There was a problem hiding this comment.
What does the phrase verify the app exits cleanly mean? I'm not sure I understand the exact context.
There was a problem hiding this comment.
By that I mean exact the entire mobile app, I'll tweak the wording here
| if (alreadyLoggedIn) { | ||
| intent.addFlags(Intent.FLAG_ACTIVITY_REORDER_TO_FRONT) | ||
| } |
There was a problem hiding this comment.
does something goes wrong if we always add FLAG_ACTIVITY_REORDER_TO_FRONT irrespective of alreadyLoggedIn ? Asking as FLAG_ACTIVITY_REORDER_TO_FRONT should be a no-op if there is no instance of activity in the task stack.
There was a problem hiding this comment.
Yes, doing this will break edge case 5 from the doc.
You're right about the no-op, but this is to avoid reusing a previous app's stale Home page due to our app-seating logic (the Connect Job Tile will show the stale information from the previous app)
There was a problem hiding this comment.
I see, think instead of trying to wire in alreadyLoggedIn in via callbacks, we can just calculate it directly here in ConnectAppLaunchController.kt ?
Also trying to understand the behaviour when alreadyLoggedIn is false here. In that case is the back behaviour different from when alreadyLoggedIn is true given we only apply this flag in one case and not other ?
There was a problem hiding this comment.
I see, think instead of trying to wire in alreadyLoggedIn in via callbacks, we can just calculate it directly here in ConnectAppLaunchController.kt ?
The goal here is to know if the user was already logged in before the app was launched. If we wait until this point in the code, a new login would have already happened and it would be difficult to distinguish between the user was "already logged in" vs "just now logged in"
Also trying to understand the behaviour when alreadyLoggedIn is false here. In that case is the back behaviour different from when alreadyLoggedIn is true given we only apply this flag in one case and not other
The back behavior would be the same in either case
There was a problem hiding this comment.
The goal here is to know if the user was already logged in before the app was launched. If we wait until this point in the code, a new login would have already happened and it would be difficult to distinguish between the user was "already logged in" vs "just now logged in"
yeah, I was thinking of calculating it in one of the launch methods itself, but keeping this as optional feedback.
One minor request to close this thread out - can we add a small code comment here on why we are adding the intent flag only when alreadyLoggedIn is set.
| if (appLaunchedFromConnect && isAtJobsList()) { | ||
| finishAffinity(); | ||
| return; | ||
| } | ||
| super.onBackPressed(); | ||
| } |
There was a problem hiding this comment.
I am not yet convinced that this is the best solution here and trying to understand the issue more, putting a few questions to clarify my understanding around this below -
-
Seems like what we are trying to do here is - if user come on Jobs List after launching a home page and clicks back, exit the App ?
-
I am curious why does that need to happen only when
appLaunchedFromConnectis true ? -
Separately Are you able to specify the exact use case this is trying to solve for from the doc and what's the behaviour without making these changes for those use cases ?
There was a problem hiding this comment.
Seems like what we are trying to do here is - if user come on Jobs List after launching a home page and clicks back, exit the App ?
Yes that's correct. User comes on Jobs List → Launches an app from Jobs List → Clicks back to land on Jobs List again → Clicks back again → Exit the mobile app.
I am curious why does that need to happen only when appLaunchedFromConnect is true ?
The appLaunchedFromConnect flag distinguishes between "the user launched an app from the Jobs List" and "the user is just browsing the Jobs List (i.e. did not launch an app from it)"
Separately Are you able to specify the exact use case this is trying to solve for from the doc and what's the behaviour without making these changes for those use cases ?
This solves for both edge case 2 and 3 from the doc. If we were to do a simple super.onBackPressed() instead, then this activity would finsh and fall through to the base screen below it (e.g. LoginActivity or DispatchActivity), which brings us back to the undesired "looping" state mentioned in the doc
There was a problem hiding this comment.
The appLaunchedFromConnect flag distinguishes between "the user launched an app from the Jobs List" and "the user is just browsing the Jobs List (i.e. did not launch an app from it)"
Yeah I understand what the flag does but not why the finishAffinity call is behind this flag i.e. why wouldn't we want to exit if there was no app launched from Opp List ?
Also one suggestion on isAtJobsList check. Think instead of isAtJobsList we want to check whether the user is at the start destination for a navigation path, so tomorrow if we make some other page the start in nav graph, this behaviour doesn't need to be changed.
There was a problem hiding this comment.
Yeah I understand what the flag does but not why the finishAffinity call is behind this flag i.e. why wouldn't we want to exit if there was no app launched from Opp List ?
Because if no app was launched, we'd want to return the user to the previous screen rather than completely exit the mobile app.
So, for example, if a user logs into a CommCare app from the Login page directly, then opens the Opp List from the side drawer, and then presses back, then we should return the user back to the CommCare app. I think exiting the mobile app in this scenario would be a bit jarring.
Another good example: Say the user opens the mobile app on their device (assuming PersonalID is already registered) and the first page they see is the Login page. If they open the Opp List page and immediately hit back, we should return to the Login page rather than exit the mobile app IMO.
Also one suggestion on isAtJobsList check. Think instead of isAtJobsList we want to check whether the user is at the start destination for a navigation path, so tomorrow if we make some other page the start in nav graph, this behaviour doesn't need to be changed.
Agreed!
There was a problem hiding this comment.
I think whatever back behaviour we keep here needs to be consistent across flows, It sounds more Jarring to me if sometimes back press from Opp List results in exiting the app while sometime not just based on what screen they launched from Opp List and not the history of user navigation.
Think a mental model we can adapt here is to treat the screens launched on clicking a navigation drawer menu item as Top level screens pressing back from which exits the app i.e. treating each of these screens as the first in the app task stack and then any navigation from these screens should place the launched activity on top of it. The drawer is more like changing tabs on the same page and similar behaviour is demonstrated by Gmail/ Gphotos as well which seems like a standard practice.
Listing out various scenarios based on that mental model -
- Top Level Navigation From drawer:
User navigates as Opp List -> Messaging(drawer) -> Work History (drawer) , on clicking back from "Work history" we exit the app instead of tracing back the flow given all items were accessed from drawer.
- Cross Feature flows but not from drawer:
User navigates as `Opp List -> Resume -> App Home" , on clicking abck from "App Home" we should go back to "Opp List" to trace back user navigation history correctly.
Also FYI @OrangeAndGreen @Jignesh-dimagi
@Jignesh-dimagi I'll create a new ticket for this today and make a note of this in the new ticket |
[AI] Clarified the Connect launch back-out QA note to specify the entire mobile app closes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@Jignesh-dimagi Ticket created here: https://dimagi.atlassian.net/browse/CCCT-2600 cc @OrangeAndGreen @shubham1g5 for visibility |
[AI] Exited from the Connect nav start destination rather than the hardcoded jobs-list fragment, so the back-out behavior follows the nav graph's start destination (covering notification/redirect entries too). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
4 participants
CCCT-2441
Product Description
Completes the rollout of the silent Connect app-launch path to its last two surfaces: starting a job from the opportunity intro screen, and the launch that runs right after an app finishes downloading/installing. These now open the app directly behind the single progress dialog — no
LoginActivity/SeatAppActivityflashing through — and backing out of the app returns to the originating Connect screen rather than the transient setup screen. Backing out past the opportunity list then exits the app instead of re-opening the launched app. After repeated launch failures (two retries), the failure dialog now shows a dismiss-only "try again later" message instead of offering endless retries.Here's a video of the retry prompt dialog:
Screen_Recording_20260617_142435_CommCare.Debug.mp4
Here's a video of the download and job intro pages. Note that I show two different cases: 1) the app was pre-installed and thus the download page is skipped, 2) the app was not pre-installed and we show the download page:
Screen_Recording_20260617_133207_CommCare.Debug.mp4
Technical Summary
ConnectJobIntroFragmentandConnectDownloadingFragmentontoConnectAppLaunchController, the shared silent-launch orchestration already used by the other Connect surfaces.DispatchActivitypath used to provide, but withoutDispatchActivity:ConnectActivityremembers a Connect-initiated launch and exits the page when the user backs out of the opportunity list, and an already-running app is brought forward withFLAG_ACTIVITY_REORDER_TO_FRONT.IS_LAUNCH_FROM_CONNECTflag, the auto-login branch inLoginActivity, and theCONNECT_MANAGED_LOGIN/redirectToConnectHomeback-out path inDispatchActivity— none have remaining writers once these two surfaces stop using the legacy path.onLaunchSucceeded/onLaunchFailed) while keeping ownership of the failure dialogs.LaunchOutcomeRouterescalates a retryable failure to a dismiss-only message onceMAX_LAUNCH_ATTEMPTS(3) is reached.Safety Assurance
Safety story
What gives me confidence:
NoSuchElementExceptioncrash before the session-shortcut fix and confirmed it no longer occurs after.LaunchOutcomeRouterandConnectAppLauncherunit tests, and the change is scoped to the Connect launch path.Risks to review:
LoginActivity/DispatchActivityby deleting the auto-login andCONNECT_MANAGED_LOGIN/redirectToConnectHomeback-out paths. This is behavior-preserving (those paths were only reachable via the now-removed flag) but worth a careful read.ConnectActivityback-stack handling (the exit-on-back andREORDER_TO_FRONTlaunch) are Android-bound and not unit-tested; they were verified manually on-device.Automated test coverage
LaunchOutcomeRouterTestcovers the retry → escalation threshold (below the limit prompts retry; at the limit shows the persistent error). The existingConnectAppLauncherTestcovers the launcher's seat / login / outcome seams. The fragment-side dialog and lifecycle code is not unit-covered.