Prevent logout when filling questionnaire by FikriMilano · Pull Request #3830 · opensrp/fhircore

FikriMilano · 2026-03-04T06:36:29Z

IMPORTANT: Where possible all PRs must be linked to a Github issue

Fixes #3829

Engineer Checklist

I have written Unit tests for any new feature(s) and edge cases for bug fixes
I have added any strings visible on UI components to the strings.xml file
I have updated the CHANGELOG.md file for any notable changes to the codebase
I have run ./gradlew spotlessApply and ./gradlew spotlessCheck to check my code follows the project's style guide
I have built and run the FHIRCore app to verify my change fixes the issue and/or does not break the app
I have checked that this PR does NOT introduce breaking changes that require an update to Content and/or Configs? If it does add a sample here or a link to exactly what changes need to be made to the content.

Code Reviewer Checklist

I have verified Unit tests have been written for any new feature(s) and edge cases
I have verified any strings visible on UI components are in the strings.xml file
I have verifed the CHANGELOG.md file has any notable changes to the codebase
I have verified the solution has been implemented in a configurable and generic way for reuseable components
I have built and run the FHIRCore app to verify the change fixes the issue and/or does not break the app

codecov · 2026-03-04T06:57:10Z

Codecov Report

❌ Patch coverage is 0% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 25.2%. Comparing base (aaebbb6) to head (702fe89).
⚠️ Report is 10 commits behind head on main.

Files with missing lines	Patch %	Lines
...re/engine/data/remote/shared/TokenAuthenticator.kt	0.0%	7 Missing ⚠️

❌ Your patch check has failed because the patch coverage (0.0%) is below the target coverage (60.0%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

@@            Coverage Diff            @@
##              main   #3830     +/-   ##
=========================================
+ Coverage     25.0%   25.2%   +0.1%     
  Complexity     844     844             
=========================================
  Files          297     299      +2     
  Lines        16102   15884    -218     
  Branches      2689    2637     -52     
=========================================
- Hits          4038    4013     -25     
+ Misses       11580   11389    -191     
+ Partials       484     482      -2

Flag	Coverage Δ
engine	`60.7% <0.0%> (-0.3%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
...register/fhircore/quest/ui/main/AppMainActivity.kt	`0.8% <ø> (-3.3%)`	⬇️
...re/quest/ui/questionnaire/QuestionnaireActivity.kt	`0.7% <ø> (+0.1%)`	⬆️
...re/engine/data/remote/shared/TokenAuthenticator.kt	`72.7% <0.0%> (-1.7%)`	⬇️

... and 7 files with indirect coverage changes

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

Copilot

Pull request overview

This PR addresses issue #3829 by preventing automatic logout/login redirection while a user is actively filling out a Questionnaire, and prompting the user to re-authenticate only after the form is saved/submitted.

Changes:

Add a suppression flag in TokenAuthenticator to prevent automatic login redirection during questionnaire completion.
Toggle suppression on/off from QuestionnaireActivity lifecycle.
After returning from a questionnaire, detect session expiry and show a non-cancellable “session expired, form saved” dialog that routes to LoginActivity.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.

File	Description
`android/quest/src/main/java/org/smartregister/fhircore/quest/ui/questionnaire/QuestionnaireActivity.kt`	Enables/disables auth redirect suppression while questionnaire UI is active.
`android/quest/src/main/java/org/smartregister/fhircore/quest/ui/main/AppMainActivity.kt`	Adds post-questionnaire refresh-token check and a session-expired confirmation dialog that navigates to login.
`android/engine/src/main/res/values/strings.xml`	Adds a new UI string for “form saved, session expired” messaging.
`android/engine/src/main/java/org/smartregister/fhircore/engine/data/remote/shared/TokenAuthenticator.kt`	Introduces `suppressLoginRedirect` and uses it to skip automatic login redirection on auth failure.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-24T19:57:02Z

...ine/src/main/java/org/smartregister/fhircore/engine/data/remote/shared/TokenAuthenticator.kt

@@ -118,12 +121,16 @@ constructor(
        bundle.containsKey(AccountManager.KEY_INTENT) -> {
          val launchIntent = bundle.get(AccountManager.KEY_INTENT) as? Intent

-          // Deletes session PIN to allow reset
-          secureSharedPreference.deleteSessionPin()
+          if (suppressLoginRedirect) {
+            Timber.w("Login redirect suppressed — questionnaire in progress")
+          } else {
+            // Deletes session PIN to allow reset
+            secureSharedPreference.deleteSessionPin()

-          if (launchIntent != null && !isLoginPageRendered) {
-            context.startActivity(launchIntent.putExtra(CANCEL_BACKGROUND_SYNC, true))
-            isLoginPageRendered = true
+            if (launchIntent != null && !isLoginPageRendered) {
+              context.startActivity(launchIntent.putExtra(CANCEL_BACKGROUND_SYNC, true))
+              isLoginPageRendered = true
+            }
          }


New suppression behavior (suppressLoginRedirect) changes the auth failure flow but isn’t covered by tests. There are existing Robolectric tests for TokenAuthenticator; please add coverage that verifies: (1) when suppression is enabled and KEY_INTENT is returned, the authenticator does not start the login intent, and (2) when suppression is disabled, the login intent is started as before (and any required cleanup like session PIN deletion still occurs).

Copilot · 2026-03-24T19:57:03Z

...est/src/main/java/org/smartregister/fhircore/quest/ui/questionnaire/QuestionnaireActivity.kt

@@ -607,6 +612,11 @@ class QuestionnaireActivity : BaseMultiLanguageActivity() {
    (supportFragmentManager.findFragmentByTag(QUESTIONNAIRE_FRAGMENT_TAG) as QuestionnaireFragment?)
      ?.getQuestionnaireResponse()

+  override fun onDestroy() {
+    tokenAuthenticator.suppressLoginRedirect = false
+    super.onDestroy()


tokenAuthenticator.suppressLoginRedirect is toggled as a global flag from this activity. Please add/extend Robolectric coverage to ensure it is set to true while the questionnaire is active and reliably reset to false when the activity finishes/destroys (including the common finish paths like submit, save-draft, and cancel). This helps prevent the app getting stuck in a permanently suppressed state if lifecycle paths change.

Copilot · 2026-03-24T19:57:03Z

android/engine/src/main/res/values/strings.xml

@@ -121,6 +121,7 @@
    <string name="percentage" translatable="false">%</string>
    <string name="logging_out">Logging out. Please wait…</string>
    <string name="session_expired">Session has been expired and must login again</string>


The session_expired string is grammatically incorrect ("Session has been expired and must login again"). Since this dialog/title is used with the new session-expired messaging, consider updating it to something like "Your session has expired. Please log in again."

Suggested change

<string name="session_expired">Session has been expired and must login again</string>

<string name="session_expired">Your session has expired. Please log in again.</string>

Copilot · 2026-03-24T19:57:04Z

android/quest/src/main/java/org/smartregister/fhircore/quest/ui/main/AppMainActivity.kt

+        if (
+          activityResult.resultCode == Activity.RESULT_OK &&
+            !onResultType.isNullOrBlank() &&
+            ActivityOnResultType.valueOf(onResultType) == ActivityOnResultType.QUESTIONNAIRE
+        ) {
+          onSubmitQuestionnaire(activityResult)
+        }
+
+        // Check if session expired while questionnaire was open
+        if (!tokenAuthenticator.isCurrentRefreshTokenActive()) {
+          delay(3000) // Let user see data was saved
+          showSessionExpiredDialog()


The new post-questionnaire session-expiry behavior in the startForResult callback isn’t covered by existing AppMainActivityTests. Please add tests to verify the dialog is only shown for questionnaire results (not for LOCATION or other result types), and only when the refresh token is inactive.

Suggested change

if (

activityResult.resultCode == Activity.RESULT_OK &&

!onResultType.isNullOrBlank() &&

ActivityOnResultType.valueOf(onResultType) == ActivityOnResultType.QUESTIONNAIRE

) {

onSubmitQuestionnaire(activityResult)

}

// Check if session expired while questionnaire was open

if (!tokenAuthenticator.isCurrentRefreshTokenActive()) {

delay(3000) // Let user see data was saved

showSessionExpiredDialog()

val isQuestionnaireResult =

activityResult.resultCode == Activity.RESULT_OK &&

!onResultType.isNullOrBlank() &&

ActivityOnResultType.valueOf(onResultType) == ActivityOnResultType.QUESTIONNAIRE

if (isQuestionnaireResult) {

onSubmitQuestionnaire(activityResult)

// Check if session expired while questionnaire was open

if (!tokenAuthenticator.isCurrentRefreshTokenActive()) {

delay(3000) // Let user see data was saved

showSessionExpiredDialog()

}

Copilot · 2026-03-24T19:57:04Z

android/quest/src/main/java/org/smartregister/fhircore/quest/ui/main/AppMainActivity.kt

+        }
+
+        // Check if session expired while questionnaire was open
+        if (!tokenAuthenticator.isCurrentRefreshTokenActive()) {
+          delay(3000) // Let user see data was saved
+          showSessionExpiredDialog()


The session-expiry check runs for every ActivityResult handled by startForResult, including non-questionnaire flows like the location settings intent (showLocationSettingsDialog() uses startForResult.launch(intent)). This can unexpectedly force a login prompt after returning from location settings (or any other future startForResult usage). Consider gating the refresh-token check so it only runs for questionnaire results (e.g., when onResultType == QUESTIONNAIRE, and ideally after a successful save/submit).

Suggested change

}

// Check if session expired while questionnaire was open

if (!tokenAuthenticator.isCurrentRefreshTokenActive()) {

delay(3000) // Let user see data was saved

showSessionExpiredDialog()

// Check if session expired while questionnaire was open

if (!tokenAuthenticator.isCurrentRefreshTokenActive()) {

delay(3000) // Let user see data was saved

showSessionExpiredDialog()

}

Copilot · 2026-03-24T19:57:04Z

android/quest/src/main/java/org/smartregister/fhircore/quest/ui/main/AppMainActivity.kt

+        if (!tokenAuthenticator.isCurrentRefreshTokenActive()) {
+          delay(3000) // Let user see data was saved
+          showSessionExpiredDialog()


delay(3000) is a hard-coded timing value in the result handler. If this delay is meant to be consistent UX behavior, consider extracting it to a named constant (or configurable value) to make the intent clear and avoid magic numbers scattered in callbacks.

Copilot · 2026-03-24T19:57:04Z

...ine/src/main/java/org/smartregister/fhircore/engine/data/remote/shared/TokenAuthenticator.kt

        bundle.containsKey(AccountManager.KEY_INTENT) -> {
          val launchIntent = bundle.get(AccountManager.KEY_INTENT) as? Intent

-          // Deletes session PIN to allow reset
-          secureSharedPreference.deleteSessionPin()
+          if (suppressLoginRedirect) {
+            Timber.w("Login redirect suppressed — questionnaire in progress")
+          } else {
+            // Deletes session PIN to allow reset
+            secureSharedPreference.deleteSessionPin()

-          if (launchIntent != null && !isLoginPageRendered) {
-            context.startActivity(launchIntent.putExtra(CANCEL_BACKGROUND_SYNC, true))
-            isLoginPageRendered = true
+            if (launchIntent != null && !isLoginPageRendered) {
+              context.startActivity(launchIntent.putExtra(CANCEL_BACKGROUND_SYNC, true))
+              isLoginPageRendered = true
+            }
          }


When suppressLoginRedirect is true, the KEY_INTENT branch skips both launching the login and deleting the session PIN. Previously, session PIN cleanup happened whenever re-authentication was required; leaving the PIN intact after an auth failure can change security/login behavior (e.g., stale PIN still present even though the session is invalid). Consider still clearing the session PIN even when redirect is suppressed, and only suppress the UI navigation portion.

ellykits · 2026-03-25T11:36:18Z

...ine/src/main/java/org/smartregister/fhircore/engine/data/remote/shared/TokenAuthenticator.kt

  private var isLoginPageRendered = false

+  /** When true, suppresses automatic LoginActivity launch (e.g., during questionnaire filling) */
+  var suppressLoginRedirect = false


Is there another approach that could be used instead of relying on this property of the class to know when the redirect is suppressed?

ellykits · 2026-03-25T11:38:14Z

...ine/src/main/java/org/smartregister/fhircore/engine/data/remote/shared/TokenAuthenticator.kt

+          if (suppressLoginRedirect) {
+            Timber.w("Login redirect suppressed — questionnaire in progress")
+          } else {
+            // Deletes session PIN to allow reset
+            secureSharedPreference.deleteSessionPin()

-          if (launchIntent != null && !isLoginPageRendered) {
-            context.startActivity(launchIntent.putExtra(CANCEL_BACKGROUND_SYNC, true))
-            isLoginPageRendered = true
+            if (launchIntent != null && !isLoginPageRendered) {
+              context.startActivity(launchIntent.putExtra(CANCEL_BACKGROUND_SYNC, true))
+              isLoginPageRendered = true
+            }
          }


The logging part of the if statement is unnecessary.

Prevent logout when filling questionnaire

926f034

Merge branch 'main' into 3829-prevent-auto-logout-on-token-expiry

702fe89

dubdabasoduba requested review from Copilot and ellykits March 24, 2026 19:52

dubdabasoduba assigned FikriMilano Mar 24, 2026

Copilot started reviewing on behalf of dubdabasoduba March 24, 2026 19:53 View session

Copilot AI reviewed Mar 24, 2026

View reviewed changes

ellykits reviewed Mar 25, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prevent logout when filling questionnaire#3830

Prevent logout when filling questionnaire#3830
FikriMilano wants to merge 2 commits intomainfrom
3829-prevent-auto-logout-on-token-expiry

FikriMilano commented Mar 4, 2026

Uh oh!

codecov bot commented Mar 4, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 24, 2026

Uh oh!

Copilot AI Mar 24, 2026

Uh oh!

Copilot AI Mar 24, 2026

Uh oh!

Copilot AI Mar 24, 2026

Uh oh!

Copilot AI Mar 24, 2026

Uh oh!

Copilot AI Mar 24, 2026

Uh oh!

Copilot AI Mar 24, 2026

Uh oh!

ellykits Mar 25, 2026

Uh oh!

ellykits Mar 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	<string name="session_expired">Session has been expired and must login again</string>
	<string name="session_expired">Your session has expired. Please log in again.</string>

Conversation

FikriMilano commented Mar 4, 2026

Uh oh!

codecov bot commented Mar 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

ellykits Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

ellykits Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov bot commented Mar 4, 2026 •

edited

Loading