fix(client): Fixed the dropdown default visible account.

[techsavvy185]

Fixes - Jira-#623
Screen_recording_20260114_171644.webm

Please make sure these boxes are checked before submitting your pull request - thanks!

• Run the static analysis check ./gradlew check or ci-prepush.sh to make sure you didn't break anything

• If you have multiple commits please combine them into one commit by squashing them.

Summary by CodeRabbit

• New Features

  • Clients can now have a default savings account on their profile.
  • The app automatically identifies and selects a client’s default savings account when loading account info.

• Bug Fixes

  • Improved error handling when locating a client’s default account — lookup errors are logged and no longer block account loading.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

Added a nullable savingsAccountId to ClientEntity. UpdateDefaultAccountViewModel now fetches the client after accounts load, sets currentSelectedIndex when a matching savings account is found, and logs lookup errors via Logger without failing the accounts load.

Changes

Cohort / File(s)	Summary
Client Data Model core/database/src/commonMain/kotlin/com/mifos/room/entities/client/ClientEntity.kt	Added nullable savingsAccountId: Long? = null to the client entity.
Account Selection Logic (ViewModel) feature/client/src/commonMain/kotlin/.../clientUpdateDefaultAccount/UpdateDefaultAccountViewModel.kt	After loading accounts, added a private suspend getClientSavingsAccountId() call that fetches the client, finds the account index matching client.savingsAccountId, updates currentSelectedIndex if found, and logs exceptions via Logger (errors do not abort account loading).

Sequence Diagram

sequenceDiagram
    participant VM as UpdateDefaultAccountViewModel
    participant Repo as Repository
    participant Accounts as AccountsList
    participant Logger as Logger
    participant UI as UIState

    VM->>Repo: loadAccounts()
    Repo-->>VM: accounts list
    VM->>Repo: getClient(clientId)
    Repo-->>VM: client (savingsAccountId)
    VM->>Accounts: find index where account.id == savingsAccountId
    alt found
        VM->>VM: set currentSelectedIndex
        VM->>UI: update selection
    else not found
        VM->>UI: leave selection unchanged
    end
    opt error
        VM->>Logger: log exception
    end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I hopped through lists and scanned each name,

A tiny id lit up my game,
I nudged the choice to set it right,
Logged a hiccup, then held on tight,
A happy hop to default fame!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ 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%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: fixing the dropdown default visible account behavior, which aligns with the PR objectives to resolve MIFOSAC-623.

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

✨ Finishing touches

• 📝 Generate docstrings

---

🧹 Recent nitpick comments

feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientUpdateDefaultAccount/UpdateDefaultAccountViewModel.kt (1)

61-65: Pre-selection error handling is now correct.

The fix addresses the past review concern—pre-selection failure no longer triggers an error dialog, which maintains good UX when accounts have already loaded successfully.

Optional improvement: Consider passing the exception to the logger for better stack traces during debugging:

             } catch (e: Exception) {
-                Logger.e(e.message ?: "Unable to fetch Default savings account.")
+                Logger.e("Unable to fetch Default savings account.", e)
             }

---

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2789344 and de76fcb.

📒 Files selected for processing (1)

• feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientUpdateDefaultAccount/UpdateDefaultAccountViewModel.kt

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: PR Checks / Static Analysis Check

🔇 Additional comments (1)

feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientUpdateDefaultAccount/UpdateDefaultAccountViewModel.kt (1)

100-110: LGTM!

The pre-selection logic is clean and handles the edge cases correctly:

• If no matching account is found (index == -1), the selection remains at the default index
• Nullable savingsAccountId is handled gracefully by the equality check

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

---

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

[biplab1]

@techsavvy185 Could you please address the issue raised by @coderabbitai?

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Fetch all review comments from CodeRabbit on this PR
gh pr view 2574 --json comments,reviews --jq '.reviews[] | select(.author.login == "coderabbitai[bot]") | {state: .state, body: .body, createdAt: .createdAt}'

Length of output: 0

---

🏁 Script executed:

#!/bin/bash
# Also fetch review comments on specific files
gh api "/repos/openMF/android-client/pulls/2574/comments" --jq '.[] | select(.user.login == "coderabbitai[bot]") | {path: .path, body: .body, line: .line, created_at: .created_at}'

Length of output: 0

---

🏁 Script executed:

#!/bin/bash
# Get all comments on the PR without filtering by user
gh pr view 2574 --json comments --jq '.comments[] | {author: .author.login, body: .body, createdAt: .createdAt}'

Length of output: 14950

---

🏁 Script executed:

#!/bin/bash
# Get all review comments without filtering
gh api "/repos/openMF/android-client/pulls/2574/comments" --jq '.[] | {author: .user.login, path: .path, body: .body, line: .line}'

Length of output: 0

---

🏁 Script executed:

#!/bin/bash
# Get all reviews on the PR
gh api "/repos/openMF/android-client/pulls/2574/reviews" --jq '.[] | {author: .user.login, state: .state, body: .body, submitted_at: .submitted_at}'

Length of output: 0

---

@biplab1 @techsavvy185

Thank you for following up! Let me clarify the issue I raised in my review.

The Issue

In UpdateDefaultAccountViewModel.kt, after successfully loading the savings accounts, the code calls getClientSavingsAccountId() to pre-select the client's default account. However, if repo.getClient() throws an exception inside getClientSavingsAccountId(), it will be caught by the outer catch block, which then sets the dialogState to Error.

The problem: Even though the accounts were loaded successfully and the dropdown is fully functional, an error dialog would be shown to the user just because the pre-selection feature failed. This degrades the user experience unnecessarily.

Recommended Solution

Since pre-selecting the default account is a secondary enhancement (nice-to-have), it shouldn't break the main account loading flow. Wrap the getClientSavingsAccountId() call in its own try-catch block:

mutableStateFlow.update {
    it.copy(
        accounts = options,
        dialogState = null,
    )
}
try {
    getClientSavingsAccountId()
} catch (e: Exception) {
    // Pre-selection failed; dropdown still functional with loaded accounts
    // Optionally log the error for debugging
}

This way, if the pre-selection fails for any reason, the dropdown will still work with all the loaded accounts—the user just won't have a pre-selected default, which is acceptable fallback behavior.

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In
`@feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientUpdateDefaultAccount/UpdateDefaultAccountViewModel.kt`:
- Around line 60-70: The catch block for getClientSavingsAccountId() currently
sets mutableStateFlow.update to an UpdateDefaultAccountState.DialogState.Error
which surfaces a non-critical pre-selection failure to the user; instead,
swallow or log the exception and do not modify dialogState. Replace the catch
body in UpdateDefaultAccountViewModel (the try invoking
getClientSavingsAccountId) to call the logger (or a debug/logging method
available in this class) with the exception (e.g., logger.warn or log(e)) and
return/ignore without calling mutableStateFlow.update or creating
UpdateDefaultAccountState.DialogState.Error so the UI remains unchanged.

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 04af08a and 2789344.

📒 Files selected for processing (1)

• feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientUpdateDefaultAccount/UpdateDefaultAccountViewModel.kt

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: PR Checks / Static Analysis Check

🔇 Additional comments (1)

feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientUpdateDefaultAccount/UpdateDefaultAccountViewModel.kt (1)

105-115: LGTM!

The function logic correctly retrieves the client's saved account ID and updates the selected index when a match is found. The -1 check properly handles the case where no matching account exists.

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

[biplab1]

Looks good to me. This can be merged.