Fixing obsolete app selector code #3006
Conversation
… in app selector is selected.
📝 WalkthroughWalkthroughThe changes modify the Sequence Diagram(s)sequenceDiagram
participant User
participant LoginActivity
participant DropdownList
participant ConnectManager
User->>LoginActivity: Initiate login process
LoginActivity->>DropdownList: Check if list is not empty
DropdownList-->>LoginActivity: Return selected app ID (if available)
alt App selector is visible and selected app ID ≠ seated app ID
LoginActivity->>ConnectManager: Update state to "Unmanaged"
else No further state update
LoginActivity->>ConnectManager: Continue with existing state flow
end
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code Definitions (1)app/src/org/commcare/activities/LoginActivity.java (1)
🔇 Additional comments (1)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@damagatchi retest this please |
shubham1g5
left a comment
shubham1g5
left a comment
There was a problem hiding this comment.
@pm-dimagi FYI that you will need to include this in phase 2 once merged.
| int selectorIndex = uiController.getSelectedAppIndex(); | ||
| if (selectorIndex > 0) { | ||
| String selectedAppId = appIdDropdownList.size() > 0 ? appIdDropdownList.get(selectorIndex) : ""; | ||
| String selectedAppId = !appIdDropdownList.isEmpty() ? appIdDropdownList.get(selectorIndex) : ""; |
There was a problem hiding this comment.
is it possible here for selectorIndex to be -1 i.e. no selection ?
There was a problem hiding this comment.
Not that I'm aware of. The code for getSelectedAppIndex just calls to the Spinner to get the position, and it sounds like that never returns -1 unless the developer does something fancy like call setSelection(-1), and the only code we have that calls setSelection first contains some logic to ensure that the value is always between 0 and numItems-1.
cross-request: dimagi/commcare-core#1455
Product Description
Fixes a bug where the app would attempt to navigate the user to the Connect menu if they select the first app in the dropdown list, instead of the selected app. This code should have been removed when we removed the "Go to Connect Menu" option as the first item in the dropdown list.
Technical Summary
Removed obsolete code that redirects to Connect menu when first item in app selector is selected.
Feature Flag
ConnectID
Safety Assurance
Safety story
Discovered via local testing, and fix verifide the same way.
Automated test coverage
No automated tests for ConnectID yet.
QA Plan
This was a pretty specific bug, but generally QA might be able to improve the testing by trying more single- and multi-app configurations from the login page.
Labels and Review