feat(loan): implement assign loan officer screen#2664
Open
Anshjn1411 wants to merge 21 commits into
Open
Conversation
…Loan Officer screen for loan accounts in Approved status, accessible from the Loan Account Actions screen.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds an "Assign Loan Officer" feature: repository APIs and implementation to list/assign officers, network endpoint and request model, data mappers and domain models, Compose UI screen + ViewModel/state, navigation integration and profile-screen hooks, resources, and DI registration. Changes
Sequence DiagramsequenceDiagram
participant User as User
participant Screen as AssignLoanOfficerScreen
participant VM as AssignLoanOfficerViewModel
participant Repo as LoanAccountSummaryRepository
participant DM as DataManagerLoan / LoanService
User->>Screen: Open assign officer (loanId)
Screen->>VM: init(loanId)
VM->>Repo: getLoanForAssignOfficer(loanId)
Repo->>DM: fetch loan details
DM-->>Repo: loan entity
Repo-->>VM: DataState.Success(LoanForAssignOfficer)
VM->>Repo: getLoanOfficerStaffOptions(officeId)
Repo->>DM: fetch staff for office
DM-->>Repo: staff list
Repo-->>VM: DataState.Success(staff options)
VM-->>Screen: uiState.content(officers)
User->>Screen: select officer, pick date, submit
Screen->>VM: submit()
VM->>Repo: assignLoanOfficer(loanId, AssignLoanOfficerRequest)
Repo->>DM: POST assignLoanOfficer
DM-->>Repo: GenericResponse
Repo-->>VM: DataState.Success(response)
VM-->>Screen: emit success effect / navigate back
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Contributor
|
@Anshjn1411 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@core/data/src/commonMain/kotlin/com/mifos/core/data/repositoryImp/LoanAccountSummaryRepositoryImp.kt`:
- Around line 39-42: The filter in getLoanOfficersForOffice currently keeps
staff with isLoanOfficer == null; change the predicate to require an explicit
true so only confirmed loan officers are returned: update the map/filter on
dataManagerStaff.getStaffInOffice(officeId) to use staff.filter {
it.isLoanOfficer == true } (reference: getLoanOfficersForOffice,
dataManagerStaff.getStaffInOffice, StaffEntity.isLoanOfficer).
- Around line 45-53: In assignLoanOfficer (LoanAccountSummaryRepositoryImp) let
coroutine cancellation propagate by removing the try/catch that swallows
CancellationException and instead rethrow CancellationException while handling
all other exceptions in the existing .catch block; specifically, ensure any
caught CancellationException is immediately thrown (or not caught) and only
non-cancellation exceptions are emitted as DataState.Error in the flow.catch,
referencing assignLoanOfficer, DataState.Error, and the flow.catch operator for
the change.
In
`@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/assignLoanOfficer/AssignLoanOfficerViewModel.kt`:
- Around line 131-177: Add a cheap re-entry guard at the start of submit(): read
the current _uiState.value and return immediately if submitInProgress == true or
completed == true to prevent concurrent submissions; keep the existing
validation (selectedOfficerIndex, officer, loan, officerId) but perform the
in-flight/completed check before launching viewModelScope; reference the
submit() method and the _uiState fields (submitInProgress and completed) so
subsequent taps or callers are ignored until the current assignLoanOfficer flow
finishes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a23fd761-2dab-4387-bcb7-e2a4a32c5b94
📒 Files selected for processing (16)
core/common/src/commonMain/kotlin/com/mifos/core/common/utils/DateHelper.ktcore/data/src/commonMain/kotlin/com/mifos/core/data/repository/LoanAccountSummaryRepository.ktcore/data/src/commonMain/kotlin/com/mifos/core/data/repositoryImp/LoanAccountSummaryRepositoryImp.ktcore/model/src/commonMain/kotlin/com/mifos/core/model/objects/account/loan/AssignLoanOfficerRequest.ktcore/network/src/commonMain/kotlin/com/mifos/core/network/datamanager/DataManagerLoan.ktcore/network/src/commonMain/kotlin/com/mifos/core/network/services/LoanService.ktfeature/loan/src/commonMain/composeResources/values/strings.xmlfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/assignLoanOfficer/AssignLoanOfficerScreen.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/assignLoanOfficer/AssignLoanOfficerScreenRoute.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/assignLoanOfficer/AssignLoanOfficerUiState.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/assignLoanOfficer/AssignLoanOfficerViewModel.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/di/LoanModule.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountProfile/LoanAccountProfileNavigation.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountProfile/LoanAccountProfileScreen.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountProfile/LoanAccountProfileViewModel.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/navigation/LoanNavigation.kt
Author
I have Updated the branch to latest commit and also attach the screensort of mobile view . Thank you ! |
Arinyadav1
reviewed
Apr 6, 2026
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/assignLoanOfficer/AssignLoanOfficerScreen.kt`:
- Around line 91-99: The scaffold/back and Cancel handlers currently allow
navigation while a submit is in progress; update the MifosScaffold onBackPressed
(and any Cancel button click handlers, e.g., the Cancel lambda around lines
250-257) to first check uiState.submitInProgress and only call navigateBack/pop
if uiState.submitInProgress is false (i.e., guard with
!uiState.submitInProgress) so the screen is non-dismissible while
AssignLoanOfficerViewModel.submit is running; keep the existing submit button
disabling behavior unchanged.
- Around line 162-163: The dropdown labels currently use displayName ?:
"${it.firstname} ${it.lastname}".trim(), which treats blank displayName or blank
name parts as valid and can produce empty or "null null" labels; update the
officer label construction in the officerLabels mapping to build the label from
non-blank parts (e.g., filter firstname and lastname for isNullOrBlank, join
with a space) and if that result is still blank, fall back to a stable
identifier such as it.id or it.username so every officer label is meaningful and
never empty; adjust any related usage (officerLabels and the uiState.officers
mapping) accordingly.
In
`@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/assignLoanOfficer/AssignLoanOfficerViewModel.kt`:
- Around line 137-155: The current flow in AssignLoanOfficerViewModel allows
building an AssignLoanOfficerRequest where toLoanOfficerId equals the existing
loan.loanOfficerId, producing a no-op reassignment; before constructing the
request in the block that starts viewModelScope.launch, add a guard that
compares officerId to loan.loanOfficerId (only when loan.loanOfficerId > 0) and
if they are equal update _uiState to set submitInProgress = false and
submitError to a validation message (e.g., "Selected officer is the same as
current officer") and return early instead of creating AssignLoanOfficerRequest;
this prevents sending a redundant request and surfaces a clear validation error
to the UI.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9b349c1c-2395-468d-8c57-53c823c11b0e
📒 Files selected for processing (8)
core/data/src/commonMain/kotlin/com/mifos/core/data/repositoryImp/LoanAccountSummaryRepositoryImp.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/assignLoanOfficer/AssignLoanOfficerScreen.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/assignLoanOfficer/AssignLoanOfficerUiState.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/assignLoanOfficer/AssignLoanOfficerViewModel.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/di/LoanModule.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountProfile/LoanAccountProfileScreen.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountProfile/LoanAccountProfileViewModel.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/navigation/LoanNavigation.kt
✅ Files skipped from review due to trivial changes (2)
- feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/di/LoanModule.kt
- feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/assignLoanOfficer/AssignLoanOfficerUiState.kt
🚧 Files skipped from review as they are similar to previous changes (2)
- feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountProfile/LoanAccountProfileScreen.kt
- core/data/src/commonMain/kotlin/com/mifos/core/data/repositoryImp/LoanAccountSummaryRepositoryImp.kt
biplab1
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/assignLoanOfficer/AssignLoanOfficerScreen.kt (1)
224-239: Defensively ignore officer changes while submit is running.
submit()snapshots the selected officer before the request starts, but this callback can still updateselectedOfficerIndex. Guarding it on!uiState.submitInProgresskeeps the UI from drifting to a different selection than the one currently being submitted.Suggested change
- onOptionSelected = { index, _ -> onOfficerSelected(index) }, + onOptionSelected = { index, _ -> + if (!uiState.submitInProgress) onOfficerSelected(index) + },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/assignLoanOfficer/AssignLoanOfficerScreen.kt` around lines 224 - 239, The dropdown's onOptionSelected may change uiState.selectedOfficerIndex while submit() is running; update the MifosTextFieldDropdown usage so the onOptionSelected callback only invokes onOfficerSelected(index) when uiState.submitInProgress is false (i.e., guard the callback with !uiState.submitInProgress) to prevent the selection from drifting away from the snapshot captured by submit(); reference MifosTextFieldDropdown, onOfficerSelected, uiState.submitInProgress, and submit() when locating the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/assignLoanOfficer/AssignLoanOfficerViewModel.kt`:
- Around line 105-112: In the DataState.Success branch of
AssignLoanOfficerViewModel (where _uiState.update is called), detect when
dataState.data is empty and treat that as an explicit error/empty state: instead
of only setting officers and isLoading, also set a loadError (or a dedicated
noOfficers flag) and ensure the UIState reflects no options available so the
view can show a friendly message and enable/disable or hide the submit flow
appropriately; modify the update call in the Success branch to set loadError (or
noOfficers) when dataState.data.isEmpty() and leave officers empty and isLoading
false.
- Around line 93-104: In loadOfficers and assignLoanOfficer, handle exceptions
thrown by the repository (not just DataState.Error) by wrapping the suspend
calls to repository.getLoanOfficersForOffice(officeId) and
repository.assignLoanOfficer(...) in try/catch blocks; on catch update _uiState
(for loadOfficers: set isLoading = false and loadError = exception message or a
default, and for assignLoanOfficer: set submitInProgress = false and submitError
= exception message or a default) so the UI never remains stuck in a
loading/submitting state and errors surface; reference the functions
loadOfficers and assignLoanOfficer and the _uiState.update usage to locate where
to add these catch branches.
---
Nitpick comments:
In
`@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/assignLoanOfficer/AssignLoanOfficerScreen.kt`:
- Around line 224-239: The dropdown's onOptionSelected may change
uiState.selectedOfficerIndex while submit() is running; update the
MifosTextFieldDropdown usage so the onOptionSelected callback only invokes
onOfficerSelected(index) when uiState.submitInProgress is false (i.e., guard the
callback with !uiState.submitInProgress) to prevent the selection from drifting
away from the snapshot captured by submit(); reference MifosTextFieldDropdown,
onOfficerSelected, uiState.submitInProgress, and submit() when locating the
change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 209d5273-7f4c-4cb1-81f4-e802c1ce2ee7
📒 Files selected for processing (3)
feature/loan/src/commonMain/composeResources/values/strings.xmlfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/assignLoanOfficer/AssignLoanOfficerScreen.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/assignLoanOfficer/AssignLoanOfficerViewModel.kt
✅ Files skipped from review due to trivial changes (1)
- feature/loan/src/commonMain/composeResources/values/strings.xml
therajanmaurya
left a comment
therajanmaurya
left a comment
Member
There was a problem hiding this comment.
After successful assignment it should redirect to load detail again
| override fun assignLoanOfficer(loanId: Int, request: AssignLoanOfficerRequest): Flow<DataState<GenericResponse>> { | ||
| return flow { | ||
| emit(DataState.Loading) | ||
| emit(DataState.Success(dataManagerLoan.assignLoanOfficer(loanId, request))) |
Member
There was a problem hiding this comment.
I think we need to listen the response first
val loanAssignmentStatus = dataManagerLoan.assignLoanOfficer(loanId, request)
if true then DataState.Success else DataState.Error
niyajali
requested changes
Apr 7, 2026
| return mBaseApiManager.loanService.disburseLoan(loanId, loanDisbursement) | ||
| } | ||
|
|
||
| suspend fun assignLoanOfficer(loanId: Int, request: AssignLoanOfficerRequest): GenericResponse { |
Collaborator
There was a problem hiding this comment.
Wrap with try catch block
…her.io at required lines and also use BaseViewModel .
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
core/data/src/commonMain/kotlin/com/mifos/core/data/repository/LoanAccountSummaryRepository.kt (1)
29-30: 🛠️ Refactor suggestion | 🟠 MajorRemove the
StaffEntity-returning variant from this public contract.
getLoanOfficerStaffOptions()already gives callers a domain-safe shape for this flow, sogetLoanOfficersForOffice()only re-exposes a Room entity and duplicates the same API at a lower abstraction level. Either delete it or promote it to a domain model as well.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/data/src/commonMain/kotlin/com/mifos/core/data/repository/LoanAccountSummaryRepository.kt` around lines 29 - 30, The interface exposes a Room entity via getLoanOfficersForOffice(officeId: Int): Flow<DataState<List<StaffEntity>>> which duplicates the domain-safe API getLoanOfficerStaffOptions(officeId: Int): Flow<DataState<List<StaffOption>>>; remove the former from the public contract: delete the getLoanOfficersForOffice signature from LoanAccountSummaryRepository, update any implementing classes to stop providing it (or make it internal/private), and refactor call sites to use getLoanOfficerStaffOptions instead (or convert the implementation to emit StaffOption if callers require similar data).
🧹 Nitpick comments (1)
feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/assignLoanOfficer/AssignLoanOfficerUiState.kt (1)
22-29: Prefer a stable selected-officer key inContent.
selectedOfficerIndexties the form state to the current ordering ofofficers. If that list refreshes or re-sorts, the same index can resolve to a differentStaffOption. StoringselectedOfficerIdhere and deriving the visible index from the current list is safer.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/assignLoanOfficer/AssignLoanOfficerUiState.kt` around lines 22 - 29, Replace the unstable selectedOfficerIndex in the Content data class with a stable selectedOfficerId (nullable, matching StaffOption.id's type) and keep officers as the source of truth; update any code that read selectedOfficerIndex to compute the visible index via officers.indexOfFirst { it.id == selectedOfficerId } (or -1 if not found), and update constructors/usages of AssignLoanOfficerUiState.Content and any validation/submit logic (e.g., where officerShowError or submitInProgress depend on selection) to use selectedOfficerId instead of the index.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/assignLoanOfficer/AssignLoanOfficerViewModel.kt`:
- Around line 170-175: The ViewModel is emitting
AssignLoanOfficerEffect.ShowMessage followed by
AssignLoanOfficerEffect.NavigateBack which blocks because the screen's snackbar
suspension delays navigation; change the contract so a single atomic effect is
emitted instead: add a new effect like
AssignLoanOfficerEffect.ShowMessageAndNavigateBack (or alternatively emit a
NavigateBack with the message as a navigation argument) and use that from
AssignLoanOfficerViewModel success path and from showLoadError path; then update
the AssignLoanOfficerScreen effect collector to handle the new
ShowMessageAndNavigateBack (or read the nav arg) so the snackbar is shown
without blocking emission of the navigation pop.
---
Duplicate comments:
In
`@core/data/src/commonMain/kotlin/com/mifos/core/data/repository/LoanAccountSummaryRepository.kt`:
- Around line 29-30: The interface exposes a Room entity via
getLoanOfficersForOffice(officeId: Int): Flow<DataState<List<StaffEntity>>>
which duplicates the domain-safe API getLoanOfficerStaffOptions(officeId: Int):
Flow<DataState<List<StaffOption>>>; remove the former from the public contract:
delete the getLoanOfficersForOffice signature from LoanAccountSummaryRepository,
update any implementing classes to stop providing it (or make it
internal/private), and refactor call sites to use getLoanOfficerStaffOptions
instead (or convert the implementation to emit StaffOption if callers require
similar data).
---
Nitpick comments:
In
`@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/assignLoanOfficer/AssignLoanOfficerUiState.kt`:
- Around line 22-29: Replace the unstable selectedOfficerIndex in the Content
data class with a stable selectedOfficerId (nullable, matching StaffOption.id's
type) and keep officers as the source of truth; update any code that read
selectedOfficerIndex to compute the visible index via officers.indexOfFirst {
it.id == selectedOfficerId } (or -1 if not found), and update
constructors/usages of AssignLoanOfficerUiState.Content and any
validation/submit logic (e.g., where officerShowError or submitInProgress depend
on selection) to use selectedOfficerId instead of the index.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7bac2723-bb34-483b-a821-e57385ed36c6
📒 Files selected for processing (8)
core/data/src/commonMain/kotlin/com/mifos/core/data/mapper/loan/LoanAssignOfficerMapper.ktcore/data/src/commonMain/kotlin/com/mifos/core/data/repository/LoanAccountSummaryRepository.ktcore/data/src/commonMain/kotlin/com/mifos/core/data/repositoryImp/LoanAccountSummaryRepositoryImp.ktcore/model/src/commonMain/kotlin/com/mifos/core/model/entity/accounts/loan/LoanForAssignOfficer.ktcore/model/src/commonMain/kotlin/com/mifos/core/model/entity/accounts/loan/StaffOption.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/assignLoanOfficer/AssignLoanOfficerScreen.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/assignLoanOfficer/AssignLoanOfficerUiState.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/assignLoanOfficer/AssignLoanOfficerViewModel.kt
✅ Files skipped from review due to trivial changes (2)
- core/model/src/commonMain/kotlin/com/mifos/core/model/entity/accounts/loan/StaffOption.kt
- core/model/src/commonMain/kotlin/com/mifos/core/model/entity/accounts/loan/LoanForAssignOfficer.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- core/data/src/commonMain/kotlin/com/mifos/core/data/repositoryImp/LoanAccountSummaryRepositoryImp.kt
Comment on lines
+170
to
+175
| sendEvent( | ||
| AssignLoanOfficerEffect.ShowMessage( | ||
| getString(Res.string.feature_loan_assign_loan_officer_success), | ||
| ), | ||
| ) | ||
| sendEvent(AssignLoanOfficerEffect.NavigateBack) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Jetpack Compose Material3, does SnackbarHostState.showSnackbar suspend until the snackbar is dismissed or an action is performed?
💡 Result:
Web search failed: Server error: no LLM provider could handle the message
🏁 Script executed:
fd -t f "AssignLoanOfficerScreen.kt" --exec cat -n {} \;Repository: openMF/mifos-x-field-officer-app
Length of output: 12947
🏁 Script executed:
fd -t f "AssignLoanOfficerViewModel.kt" --exec cat -n {} \;Repository: openMF/mifos-x-field-officer-app
Length of output: 10027
🏁 Script executed:
# Search for SnackbarHostState usage patterns to understand showSnackbar behavior
rg "showSnackbar" -A 3 -B 1Repository: openMF/mifos-x-field-officer-app
Length of output: 50388
Emit ShowMessage and NavigateBack as a single effect or redesign the collection pattern.
The current effect collector in AssignLoanOfficerScreen.kt lines 72–77 calls showSnackbar(), which suspends until the snackbar dismisses. This blocks the effect stream, delaying the following NavigateBack by ~4 seconds. On load failures, this leaves users on the blank Error screen during this wait; on success, it unnecessarily delays navigation back to the previous screen.
Affected locations
- Lines 170–175 (success path)
- Lines 186–193 (error path via
showLoadError)
Consolidate these into a single effect (e.g., ShowMessageAndNavigateBack) or pass the message as a navigation argument and pop immediately.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/assignLoanOfficer/AssignLoanOfficerViewModel.kt`
around lines 170 - 175, The ViewModel is emitting
AssignLoanOfficerEffect.ShowMessage followed by
AssignLoanOfficerEffect.NavigateBack which blocks because the screen's snackbar
suspension delays navigation; change the contract so a single atomic effect is
emitted instead: add a new effect like
AssignLoanOfficerEffect.ShowMessageAndNavigateBack (or alternatively emit a
NavigateBack with the message as a navigation argument) and use that from
AssignLoanOfficerViewModel success path and from showLoadError path; then update
the AssignLoanOfficerScreen effect collector to handle the new
ShowMessageAndNavigateBack (or read the nav arg) so the snackbar is shown
without blocking emission of the navigation pop.
…AssignLoanOfficerRequest files
Contributor
|
@Anshjn1411 Please put the ui states in the ViewModel itself and get rid of feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/assignLoanOfficer/AssignLoanOfficerUiState.kt. |
biplab1
reviewed
May 8, 2026
Contributor
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
Author
Sir , we can do this but the other feature modules like loan account , repayment are having separate file for UiState ? |
- Rename `LoanForAssignOfficer` domain model to `AssignLoanOfficerLoan`. - Rename `LoanAssignOfficerMapper` to `AssignLoanOfficerMapper` and update mapper function names (`toDomain` to `toAssignLoanOfficerLoan`, `toDomain` to `toStaffOption`). - Move `AssignLoanOfficerUiState`, `AssignLoanOfficerEffect`, and `AssignLoanOfficerAction` from a standalone file to `AssignLoanOfficerViewModel.kt`. - Update repository, use case, and view model references to reflect the renamed classes and mapper functions.
|
Contributor
Those could be older implementations. For reference take a look at Client module. |
Contributor
|
@Anshjn1411 Please align with the latest recommendations made by me in another PR of yours: #2676 |
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…Loan Officer screen for loan accounts in Approved status, accessible from the Loan Account Actions screen.
Fixes - Jira-#680
mobile mobile Screensort and recording
ticket-680-mobile-video.mp4
Please make sure these boxes are checked before submitting your pull request - thanks!
[ ✔️] Run the static analysis check
./gradlew checkorci-prepush.shto make sure you didn't break anything[✔️ ] If you have multiple commits please combine them into one commit by squashing them.
Summary by CodeRabbit