Login: App switching fix

[OrangeAndGreen]

https://dimagi.atlassian.net/browse/CI-370

Product Description

Fixes a bug that could prevent a user from switching to a different app on the login page.

Technical Summary

Once the login page was launched with a presetAppId, it would no longer be possible to seat a different app.
If the user attempted to do so, the app would seat the new app and then immediately seat the preset app again.
When the user selects a new app, the code now updates the presetAppId to match the user's new selection (the original selection that brught us to the page no longer matters).

Feature Flag

None

Safety Assurance

Safety story

I tested before and after the fix and verified that the UX is as desired.

Automated test coverage

None

QA Plan

-Login to an app
-Use the side drawer to select a different app from the menu
-On the login page, choose a different app rather than logging into the previously selected app
-Observe that the new app is seated correctly and the code doesn't re-seat the previous app.

[coderabbitai]

📝 Walkthrough

Walkthrough

The change modifies how the presetAppId field is assigned in LoginActivity when users select an app. Previously, this field may not have been consistently updated across all app-selection paths. Now, both the spinner selection path (onItemSelected) and the drawer navigation path (handleDrawerItemClick) explicitly assign the selected app ID to presetAppId. This ensures the field reliably reflects the most recently selected app for use in subsequent operations, particularly in the onResume connect/login flow.

Sequence Diagram

sequenceDiagram
    participant User
    participant UI as LoginActivity UI
    participant LoginActivity
    participant OnResume as onResume Flow

    User->>UI: Select app (spinner or drawer)
    UI->>LoginActivity: onItemSelected or handleDrawerItemClick
    
    rect rgb(200, 220, 255)
        Note over LoginActivity: NEW: Set presetAppId = appId/recordId
        LoginActivity->>LoginActivity: presetAppId = selectedAppId
    end
    
    User->>LoginActivity: Activity resumed
    LoginActivity->>OnResume: onResume() called
    
    rect rgb(220, 240, 220)
        Note over OnResume: Uses presetAppId for connect/login
        OnResume->>OnResume: Proceed with auth using presetAppId
    end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Areas requiring extra attention:
  • Verify both onItemSelected and handleDrawerItemClick correctly assign presetAppId in all branching paths
  • Confirm that onResume correctly uses the updated presetAppId for the connect/login flow
  • Check for any edge cases where presetAppId initialization or state might be inconsistent between app-selection paths
  • Validate that no existing functionality is broken by the introduction of presetAppId assignment in drawer interactions

Possibly related PRs

• Fixing obsolete app selector code #3006: Both PRs modify app-selection handling in LoginActivity to ensure the chosen app ID is correctly determined and persisted for subsequent use.

Suggested labels

QA Note, skip-integration-tests

Suggested reviewers

• shubham1g5
• Jignesh-dimagi

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely summarizes the main change: fixing an app switching bug on the login page, which matches the core purpose of the changeset.
Description check	✅ Passed	The description includes all major required sections: product description, technical summary with ticket link, feature flag status, safety story with testing details, automated test coverage status, and a QA plan with clear reproduction steps.

✨ 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 login_app_switch_fix

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a92fc3e and faf586b.

📒 Files selected for processing (1)

• app/src/org/commcare/activities/LoginActivity.java (2 hunks)

🧰 Additional context used

🧠 Learnings (1)

📓 Common learnings

Learnt from: OrangeAndGreen
Repo: dimagi/commcare-android PR: 3248
File: app/src/org/commcare/connect/MessageManager.java:0-0
Timestamp: 2025-07-29T14:54:47.940Z
Learning: User OrangeAndGreen organizes messaging-related changes into separate PRs rather than mixing them with other Connect work, which aligns with their preference for handling code issues in separate PRs when they are not directly related to the main changes.

Learnt from: shubham1g5
Repo: dimagi/commcare-android PR: 0
File: :0-0
Timestamp: 2025-05-08T11:08:18.530Z
Learning: PR #3048 "Phase 4 Connect PR" introduces a substantial feature called "Connect" to the CommCare Android app, which includes messaging, job management, delivery tracking, payment processing, authentication flows, and learning modules. It follows a modern architecture using Navigation Components with three navigation graphs, segregated business logic in Manager classes, and proper database persistence.

Learnt from: Jignesh-dimagi
Repo: dimagi/commcare-android PR: 3093
File: app/res/navigation/nav_graph_connect_messaging.xml:41-45
Timestamp: 2025-05-09T10:57:41.073Z
Learning: In the CommCare Android codebase, the navigation graph for Connect messaging (`nav_graph_connect_messaging.xml`) intentionally uses `channel_id` as the argument name in the connectMessageFragment, despite using `channelId` in other parts of the same navigation graph. This naming difference is by design in the refactored code.

Learnt from: OrangeAndGreen
Repo: dimagi/commcare-android PR: 3108
File: app/src/org/commcare/activities/StandardHomeActivityUIController.java:0-0
Timestamp: 2025-06-20T13:21:20.908Z
Learning: User OrangeAndGreen prefers to handle code issues in separate PRs rather than immediately fixing them in the current PR when they are not directly related to the main changes.

Learnt from: OrangeAndGreen
Repo: dimagi/commcare-android PR: 3248
File: app/src/org/commcare/connect/network/ApiPersonalId.java:0-0
Timestamp: 2025-07-29T14:56:40.681Z
Learning: User OrangeAndGreen prefers to defer ApiPersonalId refactoring (specifically InputStream resource management improvements) to separate PRs rather than mixing them with Connect feature work, even when the code is already in master.

🔇 Additional comments (2)

app/src/org/commcare/activities/LoginActivity.java (2)

759-765: LGTM! Correct fix for spinner-based app selection.

Updating presetAppId when the user selects an app from the spinner ensures their choice persists across activity lifecycle events. This prevents the bug where the original preset app would be re-seated after the user switched to a different app.

---

1031-1044: LGTM! Consistent fix for drawer-based app selection.

This change mirrors the spinner fix at line 763, ensuring that both app selection paths consistently update presetAppId. The null check on line 1033 provides safety, and the logic correctly prevents the preset app from overriding the user's drawer selection.

---

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

[conroy-ricketts]

Love the simple fixes!

[Jignesh-dimagi]

@OrangeAndGreen Changes LGTM but just a question: This bug arises due to switching of app from side navigation? We have not faced this issue previously.

[OrangeAndGreen]

@Jignesh-dimagi Yeah, this behavior would only arise in the special occasion where the Login page was launched with a presetAppId and then the user tried to switch via the side menu. The page would seat the app that the user chose, but then immediately reseat the previous app since it was specified as the preset when the activity was launched.