Fix login redirection issues

[avazirna]

Product Description

This PR addresses two issues with the login redirection:

1. Fixes a RuntimeException when the redirection is triggered from a non-Activity context
2. Make the session-expiration broadcast package-scoped to ensure the login redirection is reliably received.

• When the receiver fails, users may be left in restricted areas. After a user action, an exception is thrown due to an invalid evaluation context and its handling causes CommCare to crash because of the issue described in the previous bullet (see video below). The initial exception is captured as a non-fatal: Crashlytics report.
  
  case_list_crash_after_session_expiration.mp4

Safety Assurance

Safety story

Minor improvements to ensure that the login redirection works as expected and without crashes.

Labels and Review

• Do we need to enhance the manual QA test coverage ? If yes, the "QA Note" label is set correctly
• Does the PR introduce any major changes worth communicating ? If yes, the "Release Note" label is set and a "Release Note" is specified in PR description.
• Risk label is set correctly
• The set of people pinged as reviewers is appropriate for the level of risk of the change

[coderabbitai]

📝 Walkthrough

Walkthrough

This pull request makes two targeted modifications to intent handling. First, the expireUserSession method in CommCareApplication now explicitly targets the USER_SESSION_EXPIRED broadcast to the application's own package using setPackage(). Second, in SessionRegistrationHelper, the login redirect logic is updated to use addFlags() instead of setFlags() and conditionally adds FLAG_ACTIVITY_NEW_TASK when the context is not an Activity instance, ensuring proper task stack management across different context types.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~15 minutes

Possibly related PRs

• Auto login app after its downloaded  #2973: Modifies intent flag handling with similar adjustments to FLAG_ACTIVITY_NEW_TASK and FLAG_ACTIVITY_CLEAR_TOP patterns in activity-launching code.

Suggested reviewers

• shubham1g5
• OrangeAndGreen

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Description check	❓ Inconclusive	The PR description covers product issues, safety story, and labels review, but lacks several template sections like Technical Summary (ticket link/design), Feature Flag, Automated test coverage, and QA Plan details.	Add missing sections: provide a ticket/document link, describe design decisions, specify automated test coverage, and detail the QA plan with test conditions.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely summarizes the main changes in the PR, which focus on fixing login redirection issues including crash prevention and broadcast scoping.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix-login-redirect-crash-from-non-activity-context

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[shubham1g5]

Are you also able to add details on the other eval context crash that this fixes and repro around it in context of session expiration.

[conroy-ricketts]

LGTM, I also like Shubham's suggestion

[shubham1g5]

The initial exception is captured as a non-fatal: Crashlytics report.

Does that mean that there is no other "crash" associated with this PR and only a non-fatal ?

[avazirna]

The initial exception is captured as a non-fatal: Crashlytics report.

Does that mean that there is no other "crash" associated with this PR and only a non-fatal ?

Yes, there are other crashes caused by "no appropriate evaluation context", but it's hard to tell whether these are a result of the login redirection issues or not, so this change is to rule those out.