feat(loan): implement dynamic datatable steps for new loan creation#2693
Open
TheKalpeshPawar wants to merge 15 commits into
Open
feat(loan): implement dynamic datatable steps for new loan creation#2693TheKalpeshPawar wants to merge 15 commits into
TheKalpeshPawar wants to merge 15 commits into
Conversation
Loan products in Fineract can require one or more populated datatables
before the loan can be created. The Android client previously ignored
this and POSTed `/loans` directly, which returned 403:
"The datatable [DATOS_RAZON_SOCIAL, DATOS_DIRECCION, …]
needs to be filled in before the current action can be proceeded"
This change adds inline datatable steps to NewLoanAccountScreen's
stepper, between Schedule and Preview. The step count and step labels
come from `loanTemplate.dataTables` (filtered for nulls). Each step
renders a fully dynamic form derived from `columnHeaderData`, with
per-column widgets keyed by `columnDisplayType` (STRING, INTEGER,
DECIMAL, DATE, BOOLEAN, CODELOOKUP/CODEVALUE). Dropdown values are
sourced from each column's `columnValues`; the picked code id is
persisted in submit payload, not the display string. PK columns and
`created_at`/`updated_at` audit columns are filtered out.
Products with no datatables (e.g. products 2-6, 9, 15) see the same
5-step flow as before. Products with mixed null entries (7) or all
null entries (8, 10) handle gracefully via `.filterNotNull()`. Web-app
parity verified against ~/OpenSource/Mifos/web-app/
src/app/loans/create-loans-account/create-loans-account.component.{ts,html}.
Wire fixes along the way:
- @SerialName("datatables") on both LoanTemplate.dataTables (decode) and
LoansPayload.dataTables (encode) — Fineract uses lowercase, Kotlin
field was camelCase, so the field defaulted to empty.
- LoanTemplate.dataTables typed as ArrayList<DataTableEntity?> — some
products return [null,null,...] which previously threw inside
kotlinx-serialization and bubbled up as deserialization failure.
- @SerialName on ColumnHeader for columnName / isColumnNullable /
isColumnPrimaryKey — without these, dataTableColumnName + filter
conditions were always null/false, making the dynamic form blank.
- GetDataTablesResponseMapper now propagates columnName (legacy path).
- MifosStepper already supports dynamic step count + LazyRow scroll;
no UI-component changes needed.
Files:
- feature/loan/.../pages/DatatableStepPage.kt NEW
Fully dynamic per-table form renderer with Back/Next.
- feature/loan/.../NewLoanAccountViewModel.kt
+ datatableValues state (Map<Int, Map<String, Any>>)
+ UpdateDatatableField action
+ computed totalSteps based on filtered datatable count
+ inline datatable-payload assembly in submitLoanApplication
- feature/loan/.../NewLoanAccountScreen.kt
Dynamic step list (buildList) with datatable steps between
Schedule and Preview.
- core/database/.../ColumnHeader.kt @SerialName mappings
- core/database/.../LoanTemplate.kt @SerialName + nullable element
- core/network/.../LoansPayload.kt @SerialName for submit side
- core/network/.../GetDataTablesResponseMapper.kt legacy mapper fix
- feature/data-table/.../* encoder-side polymorphic SerializersModule
on DataTableNavigation.navigateDataTableList; DataTableListScreen
stateful inputs + columnPrimaryKey filter fix; DataTableColumnType
constants replacing the missing legacy BaseFormWidget.SCHEMA_KEY_*.
(The DataTableListScreen path remains used by the client-creation
flow; the loan-creation path is now inline.)
.gitignore: exclude .claude/, .mcp.json, CLAUDE.md, graphify-out/
Three follow-up fixes after device testing of the dynamic datatable
stepper:
1. **Label heuristic (v3, longer-wins).** Codelookup columns are stored
as `{columnCode}_cd_{descriptiveName}`. Either half can be the more
descriptive one — and Fineract sometimes truncates the suffix to the
column-length limit. v1 (suffix only) collapsed `YesNo_cd_*` pairs
in product 7; v2 (suffix only) truncated `CREDITO_DE_ALGUNA_I`.
v3 picks whichever side is LONGER, which gets all three cases right:
STATE_cd_ESTADO → ESTADO
YesNo_cd_YA_CUENTA_CON_UN_NEGOCIO → YA_CUENTA_CON_UN_NEGOCIO
YesNo_cd_QUIERE_EMPRENDER_UN_NEGOCIO → QUIERE_EMPRENDER_UN_NEGOCIO
CREDITO_…_FINANCIERA_cd_CREDITO_DE_ALGUNA_I → CREDITO_…_FINANCIERA
2. **FK column blacklist.** `loan_id` was rendering as an INTEGER input
on INSTITUCION_FINANCIERA (product 1). The column has
`isColumnPrimaryKey: false` so the PK filter didn't catch it — it's
actually a foreign key to the parent m_loan, auto-populated server-
side. Added the conventional Fineract FK column names
(loan_id / client_id / group_id / savings_id / share_id / office_id)
to SYSTEM_COLUMNS in both DatatableStepPage and the ViewModel
payload assembly.
3. **Date payload format.** CONDICION.FECHA was rejected with "invalid
value". Root cause: payload declared
`dateFormat = ApiDateFormatter.DATE_FORMAT` ("dd MMMM yyyy") but the
DateFieldRow widget actually emits `dd-MM-yyyy` (via
DateHelper.getDateAsStringFromLong → SHORT_MONTH). The two must
match. Switched the datatable payload to
`dateFormat = DateHelper.SHORT_MONTH` to mirror what the existing
loan POST does (NewLoanAccountViewModel.kt:275 was already using
DateHelper.SHORT_MONTH for expectedDisbursementDate etc.).
Files: NewLoanAccountViewModel.kt (date format + FK blacklist) and
pages/DatatableStepPage.kt (label v3 heuristic + FK blacklist).
Not addressed (upstream): geolocalizacion-del-registro and
region-del-producto are not present in product 7's
/loans/template response — verified via recursive search across all
nested fields + cross-checked against web-app source. Both clients
(Android + web-app) leave those steps empty; fix belongs in the
Fineract product schema.
Manual cleanups on top of the dynamic-datatable feature:
- Drop verbose GAP-DT-* markers from comments; keep the explanations
that still aid future readers.
- Rename `ColumnHeader.isUserEditable()` →
`ColumnHeader.isNotAutoGeneratedOrNull()`; the new name describes
what is being checked rather than the intent of the caller.
- Remove unused imports (`SelectableDates`, `ExperimentalTime`).
- Inline doc trimming.
- `displayLabel()` reverted from the longer-wins heuristic back to
`substringBefore("_cd_")` — prefer the columnCode prefix as the
label. Trade-off accepted: YesNo_cd_* pairs collapse to the same
"YesNo" label, but the simpler rule is easier to reason about and
matches the bulk of the field labels seen in real product
templates.
The dynamic datatable form stored every cell as `Any`, which forced
unchecked `as? String` / `as? Boolean` / `as? Int` casts at every read
and made the storage→submit boundary easy to get wrong (e.g. caller
emits a Boolean for an INTEGER column, deferred-fails at the server).
Introduce a sealed interface with three concrete variants:
- Text(text: String) : STRING / TEXT / INTEGER / DECIMAL / FLOAT / DATE
(numeric parsing is deferred to submit time)
- Bool(checked: Bool) : BOOLEAN
- Code(id: Int) : CODELOOKUP / CODEVALUE (Fineract code id, not
the display string)
Plus `asPayloadValue(): Any` as the single boundary where the typed
form value crosses into the `Map<String, Any>` shape the
`DataTablePayload.data` Fineract contract requires.
Touch points:
- NEW pages/DatatableFieldValue.kt
- pages/DatatableStepPage.kt
values: Map<String, DatatableFieldValue>
onValueChange: (String, DatatableFieldValue) -> Unit
Each inner Row helper keeps its concrete type (String / Boolean /
Int); wrapping into the sealed variant happens at the
DatatableField dispatch. Unsafe `as? String` casts on the cell
value are now defensive casts to the specific variant.
- NewLoanAccountViewModel.kt
State.datatableValues: Map<Int, Map<String, DatatableFieldValue>>
Action.UpdateDatatableField.value: DatatableFieldValue
buildDatatablePayloadMap now consumes the typed map.
Replaced `coerceDatatableValue(Any, String?): Any` with
`ColumnHeader.encodePayloadValue(DatatableFieldValue): Any`,
which does both variant dispatch AND column-aware numeric
coercion (INTEGER -> toIntOrNull, DECIMAL/FLOAT -> toDoubleOrNull)
in one place.
Wire format unchanged — same primitives reach the POST /loans body.
Verified on device with product 1 (5 datatables, all categories of
column display type exercised).
Trade-offs considered:
- Plain enum + side fields → reintroduces the unsafety we removed.
- @JvmInline value classes on a sealed interface → boxed immediately
when upcast to the interface type (the common case here), and
Compose stability tracking is rougher with value classes. Plain
data classes are correct.
|
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:
WalkthroughRefactors datatable flows: removes widget DTO plumbing, introduces typed per-cell values, reworks DataTable list UI/ViewModel to collect per-table field state and coerce payloads, and updates navigation/serialization and loan/client flows to support inline datatable editing. ChangesA single coherent refactor touching serialization, navigation, DataTable list UI/VM, typed cell values, NewLoan integration, datatable step UI, and small supporting edits. Developer checkpoints (expand)
Datatable Field Editing Refactoring — developer checkpoints
Sequence Diagram (high-level flow) sequenceDiagram
participant User
participant NewLoanAccountScreen
participant NavController
participant DataTableListScreen
participant DataTableListViewModel
User->>NewLoanAccountScreen: tap Submit when loan has datatables
NewLoanAccountScreen->>NavController: navigateDataTableList(dataTables, payload, requestType)
NavController->>DataTableListScreen: open with encoded args
DataTableListScreen->>DataTableListViewModel: initial state (data tables, payload)
User->>DataTableListScreen: edit cell -> onFieldChanged(tableIdx,column,value)
DataTableListScreen->>DataTableListViewModel: dispatch OnFieldChanged(tableIdx,column,value)
DataTableListViewModel->>DataTableListViewModel: buildPayloadMap() -> create*Account() request
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (2 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/newLoanAccount/NewLoanAccountViewModel.kt (1)
892-916:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReset datatable input state when loading a new product template.
In
NewLoanAccountViewModel.kt,datatableValuesis preserved across template reloads. Switching product can keep stale table-index/column values and submit incorrect datatable data. CleardatatableValues(and ideally reset step index) whenproductIdchanges/template loads.Also applies to: 1080-1080
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/newLoanAccount/NewLoanAccountViewModel.kt` around lines 892 - 916, When loading a new product template in loadLoanAccountTemplate, the ViewModel currently preserves stale datatableValues (and step index) which can submit incorrect datatable data; update the mutableStateFlow when handling DataState.Loading and DataState.Success to clear/reset datatableValues and reset any step index (e.g., datatableValues = emptyMap() and step index property such as currentStep or stepIndex = 0) alongside existing updates to productId, isOverLayLoadingActive, and loanTemplate so the state is fresh for the new template.
🧹 Nitpick comments (2)
feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccount/LoanAccountScreen.kt (1)
625-627: ⚡ Quick winRemove unnecessary
ArrayListconversion.The
dataTablefunction acceptsList<DataTableEntity>, andfilterNotNull()already returns aList. TheArrayListwrapper on line 627 is redundant.♻️ Suggested simplification
val realDataTables = loanTemplate.dataTables.filterNotNull() if (realDataTables.isNotEmpty()) { - dataTable(ArrayList(realDataTables), loadPayload) + dataTable(realDataTables, loadPayload) } else { createLoanAccount(loadPayload) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccount/LoanAccountScreen.kt` around lines 625 - 627, The code constructs an unnecessary ArrayList when calling dataTable; remove the redundant conversion by passing the kotlin List returned from loanTemplate.dataTables.filterNotNull() (realDataTables) directly to the dataTable(...) call, i.e., replace dataTable(ArrayList(realDataTables), loadPayload) with dataTable(realDataTables, loadPayload) so you call dataTable with the existing List<DataTableEntity> from realDataTables.feature/data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableList/DataTableListScreen.kt (1)
96-104: 🏗️ Heavy liftCollapse the content-layer callbacks into a single
onAction.This change adds another top-level callback to a Layer-2 screen that already takes several separate lambdas. That makes the route/content boundary harder to maintain and keeps pushing this screen away from the repo's
state + onActioncontract. As per coding guidelines, "Layer 2 (Stateful/Content Composable —*Screenor*ScreenContent) MUST only takestate... and a singleonActionlambda."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@feature/data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableList/DataTableListScreen.kt` around lines 96 - 104, The DataTableListScreen currently accepts multiple top-level callbacks (clientCreated, onSaveClicked, onFieldChanged, onBackPressed) which violates the project's Layer-2 contract; replace these with a single onAction: (DataTableListAction) -> Unit parameter on DataTableListScreen and implement a sealed class or interface DataTableListAction (e.g., BackPressed, ClientCreated(payload: ClientPayloadEntity), SaveClicked, FieldChanged(tableIndex: Int, columnName: String, value: Any)) to represent each event; update DataTableListScreen usages/children to dispatch DataTableListAction values instead of calling the separate lambdas and update callers to handle the single onAction instead of the multiple callbacks.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@feature/data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableList/DataTableListScreen.kt`:
- Around line 292-297: The date picker currently forces future-only selection in
the rememberDatePickerState by supplying a SelectableDates implementation whose
isSelectableDate returns utcTimeMillis >=
Clock.System.now().toEpochMilliseconds(), which blocks valid past dates for
schema-driven fields; update the logic in DataTableListScreen where
rememberDatePickerState is created (and where selectedDate is passed) to stop
enforcing a global future-only rule — either remove the selectableDates
parameter or make it conditional based on the datatable column metadata (e.g., a
futureOnly flag from the column schema) so past dates remain selectable for
columns that are not explicitly future-only.
- Around line 141-146: The current composition calls onBackPressed() outside the
LaunchedEffect (when uiState.message != null), which can trigger premature
navigation; move the onBackPressed() call into the LaunchedEffect that displays
the snackbar so navigation happens after snackBarHostState.showSnackbar
completes. Locate the block using uiState.message, the LaunchedEffect(key1 =
message) and snackBarHostState.showSnackbar(message) and invoke onBackPressed()
inside that LaunchedEffect after showSnackbar returns (or after handling its
result) so the back navigation runs as a side-effect, not during composition.
- Around line 256-279: The dropdown currently passes the visible label to
onFieldChanged; change it so UI still displays columnHeader.columnValues.map {
it.value } but when an option is selected call onFieldChanged(name, selectedId)
where selectedId = columnHeader.columnValues[index].id (use
onOptionSelected(index, item) instead of ignoring the index), and set
selectedValue to the display label; also when initializing selectedValue
(selectedValue by rememberSaveable...) if there is an existing form value id,
resolve it to the matching columnHeader.columnValues.find { it.id == existingId
}?.value so the label shows while the payload stores the id. Ensure you update
the MifosTextFieldDropdown handlers (value/onValueChanged/onOptionSelected) and
references to selectedValue, columnHeader.columnValues, MifosTextFieldDropdown,
and onFieldChanged accordingly.
In
`@feature/data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableList/DataTableListViewModel.kt`:
- Around line 149-173: The buildPayloadMap/coerce logic currently converts
unparsable numeric strings into 0/0.0 which incorrectly serializes cleared
optional fields; change coerce to return null for empty or unparsable numeric
strings (e.g., if value is "" or toIntOrNull()/toDoubleOrNull() returns null)
and update buildPayloadMap to only put entries when coerce(...) != null (use the
ColumnHeader.dataTableColumnName and header.columnDisplayType to locate the
field); this will omit cleared numeric inputs instead of serializing them as
zero.
In
`@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/newLoanAccount/NewLoanAccountScreen.kt`:
- Line 157: Replace the hardcoded fallback title "Datatable ${idx + 1}" with a
localized formatted string resource and use it in the Step call: change the
expression inside Step(...) where table.registeredTableName ?: "Datatable ${idx
+ 1}" is used to fetch a string resource (with a single integer format argument)
and pass (idx + 1) as the format parameter; add the corresponding localized
string resource (e.g., "datatable_title" = "Datatable %d") and ensure the code
calls the platform-appropriate string lookup (e.g., stringResource/getString or
the project’s i18n helper) so Step(...) receives the localized fallback when
table.registeredTableName is null.
In
`@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/newLoanAccount/NewLoanAccountViewModel.kt`:
- Around line 271-329: The payload assembly in NewLoanAccountViewModel uses
direct indexing like
state.loanTemplate?.loanOfficerOptions[state.loanOfficerIndex]?.id (and similar
for repaymentFrequencyDaysOfWeekType, repaymentFrequencyNthDayType,
loanPurposeOptions, fundOptions, accountLinkingOptions,
transactionProcessingStrategyOptions, etc.) after only checking for -1, which
can still throw IndexOutOfBoundsException if the options list changed; fix by
replacing direct indexing with safe access using getOrNull(e.g.
state.loanTemplate?.loanOfficerOptions?.getOrNull(state.loanOfficerIndex)?.id)
for every place that currently does options[index] so each optional list access
returns null instead of crashing and keep the existing -1 checks or remove them
if redundant.
- Around line 1229-1232: The current mapping in the DatatableFieldValue.Text
branch silently converts invalid numeric strings to 0/0.0 (see
DatatableFieldValue.Text, columnDisplayType and value.text), which can corrupt
data; change the INTEGER/DECIMAL/FLOAT arms to stop coercing to zero and instead
return a null/absent value or propagate a validation error (e.g., return null or
throw a controlled ValidationException) so the caller or NewLoanAccountViewModel
can surface an error or omit the field; update any callers to handle the
null/validation result accordingly.
In
`@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/newLoanAccount/pages/DatatableStepPage.kt`:
- Around line 274-277: DatatableStepPage currently hardcodes the DatePicker
action labels ("OK" and "Cancel"); update the Text/TextButton usages inside the
DatePicker dialog (in DatatableStepPage, where showDatePicker is toggled) to use
localized string resources instead of hardcoded literals, add corresponding
entries for these keys in the project's strings (e.g., OK and Cancel
translations), and replace the literals with calls to the platform's string
lookup (stringResource or equivalent) so the Text composables and TextButton
onClick labels pull from resources for translation consistency.
- Around line 319-325: The code creates options by dropping null labels but
leaves ids intact (variables options, ids, currentDisplay), causing index
misalignment; fix by building a single filtered list of pairs from
header.columnValues (e.g., mapNotNull { cv -> cv.value?.let { v -> cv.id to v }
}) and derive both ids and labels from that filteredPairs so indices always
match, then update the remember block (currentDisplay) to use filteredPairs for
lookup; apply the same change to the other occurrence that also constructs
options/ids (the second block around the dropdown logic).
- Around line 285-286: DatatableStepPage is rendering a fallback date via `value
= currentValue ?: DateHelper.getDateAsStringFromLong(selectedMillis)`, which
shows a date even when `currentValue` is null; change this so the UI renders
empty when no value exists (e.g., use an empty string fallback) or explicitly
initialize the state when the page is first shown. Update the logic around
`currentValue` and `selectedMillis` in DatatableStepPage: either set
`currentValue` from `DateHelper.getDateAsStringFromLong(selectedMillis)` at
initialization (so the field is truly stored) or keep `currentValue` nullable
and use `value = currentValue ?: ""` (and ensure submit logic treats empty
string as no-value). Ensure the change touches the same symbols (`currentValue`,
`selectedMillis`, and `DateHelper.getDateAsStringFromLong`) so the renderer and
submit path remain consistent.
---
Outside diff comments:
In
`@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/newLoanAccount/NewLoanAccountViewModel.kt`:
- Around line 892-916: When loading a new product template in
loadLoanAccountTemplate, the ViewModel currently preserves stale datatableValues
(and step index) which can submit incorrect datatable data; update the
mutableStateFlow when handling DataState.Loading and DataState.Success to
clear/reset datatableValues and reset any step index (e.g., datatableValues =
emptyMap() and step index property such as currentStep or stepIndex = 0)
alongside existing updates to productId, isOverLayLoadingActive, and
loanTemplate so the state is fresh for the new template.
---
Nitpick comments:
In
`@feature/data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableList/DataTableListScreen.kt`:
- Around line 96-104: The DataTableListScreen currently accepts multiple
top-level callbacks (clientCreated, onSaveClicked, onFieldChanged,
onBackPressed) which violates the project's Layer-2 contract; replace these with
a single onAction: (DataTableListAction) -> Unit parameter on
DataTableListScreen and implement a sealed class or interface
DataTableListAction (e.g., BackPressed, ClientCreated(payload:
ClientPayloadEntity), SaveClicked, FieldChanged(tableIndex: Int, columnName:
String, value: Any)) to represent each event; update DataTableListScreen
usages/children to dispatch DataTableListAction values instead of calling the
separate lambdas and update callers to handle the single onAction instead of the
multiple callbacks.
In
`@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccount/LoanAccountScreen.kt`:
- Around line 625-627: The code constructs an unnecessary ArrayList when calling
dataTable; remove the redundant conversion by passing the kotlin List returned
from loanTemplate.dataTables.filterNotNull() (realDataTables) directly to the
dataTable(...) call, i.e., replace dataTable(ArrayList(realDataTables),
loadPayload) with dataTable(realDataTables, loadPayload) so you call dataTable
with the existing List<DataTableEntity> from realDataTables.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b2125ba5-36c0-4660-9733-5af19ebabfd3
📒 Files selected for processing (17)
.gitignorecmp-navigation/src/commonMain/kotlin/cmp/navigation/authenticated/AuthenticatedNavigation.ktcore/database/src/commonMain/kotlin/com/mifos/room/entities/noncore/ColumnHeader.ktcore/database/src/commonMain/kotlin/com/mifos/room/entities/templates/loans/LoanTemplate.ktcore/network/src/commonMain/kotlin/com/mifos/core/network/mappers/dataTable/GetDataTablesResponseMapper.ktcore/network/src/commonMain/kotlin/com/mifos/core/network/model/LoansPayload.ktfeature/client/src/commonMain/kotlin/com/mifos/feature/client/navigation/ClientNavigation.ktfeature/data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableList/DataTableColumnType.ktfeature/data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableList/DataTableListNavArgs.ktfeature/data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableList/DataTableListScreen.ktfeature/data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableList/DataTableListViewModel.ktfeature/data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/navigation/DataTableNavigation.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccount/LoanAccountScreen.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/newLoanAccount/NewLoanAccountScreen.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/newLoanAccount/NewLoanAccountViewModel.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/newLoanAccount/pages/DatatableFieldValue.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/newLoanAccount/pages/DatatableStepPage.kt
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
**/{core-base,core}/**/*.kt
⚙️ CodeRabbit configuration file
**/{core-base,core}/**/*.kt: Critical Module Change Detection:
- Changes in
core-basemodule must be treated as high-impact.- Flag any PR that modifies files inside
core-basefor careful review.- Verify that modifications in
core-baseare necessary and minimal.
Output:- Clearly highlight that
core-baseis a shared foundational module and requires extra review attention.
Files:
core/database/src/commonMain/kotlin/com/mifos/room/entities/noncore/ColumnHeader.ktcore/database/src/commonMain/kotlin/com/mifos/room/entities/templates/loans/LoanTemplate.ktcore/network/src/commonMain/kotlin/com/mifos/core/network/mappers/dataTable/GetDataTablesResponseMapper.ktcore/network/src/commonMain/kotlin/com/mifos/core/network/model/LoansPayload.kt
**/*.kt
⚙️ CodeRabbit configuration file
**/*.kt: Additional Code Review Guidelines:
- Null Safety & Stability
- Avoid using
!!operator- Handle null cases explicitly using safe calls or proper state handling
- Do not assume values are always non-null without guarantees
- Architecture Boundaries
- ViewModel must not depend on specific network/library implementations
- Ensure proper separation between data, domain, and presentation layers
- Do not format data (currency, dates, calculations) inside the UI layer
- All formatting must be handled in the ViewModel and exposed via state (e.g., StateFlow)
- Performance Considerations
- Avoid unnecessary recompositions in Compose
- Do not attach heavy logic to frequently changing states (e.g., scrollState)
- Prefer lifting state up instead of recomputing in child composables
- Compose & Navigation Best Practices
- NEVER trigger navigation functions or side-effects directly during composition
- Always wrap navigation calls inside
LaunchedEffectorEventsEffectto avoid repeated execution on recomposition- Avoid triggering intensive side-effects during recomposition
- Navigation routes must be type-safe.
- Ensure all route classes or objects used for navigation are annotated with
@Serializable.
- UI Structure
- Dialogs must be separated into their own composables
- Do not embed dialogs inline within complex main screens
- Localization Consistency
- Ensure all supported languages are updated consistently across modules
- Verify translations exist for newly added UI strings
- Code Cleanliness
- Avoid unnecessary inline comments unless critical
- Remove leftover debug or commented code
- Focus on correctness, readability, and maintainability over cosmetic nitpicks.
- Avoid reviewing README, config, or asset files.
- Prioritize identifying bugs, performance issues, and architectural concerns.
- Naming & Intent Rules:
- Follow the official Kotlin Coding Conventions:
https://kotlinlang.org/docs/coding-conventions.html- Use self-explanatory ...
Files:
core/database/src/commonMain/kotlin/com/mifos/room/entities/noncore/ColumnHeader.ktcore/database/src/commonMain/kotlin/com/mifos/room/entities/templates/loans/LoanTemplate.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccount/LoanAccountScreen.ktfeature/data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableList/DataTableColumnType.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/newLoanAccount/pages/DatatableFieldValue.ktfeature/data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableList/DataTableListNavArgs.ktfeature/client/src/commonMain/kotlin/com/mifos/feature/client/navigation/ClientNavigation.ktcore/network/src/commonMain/kotlin/com/mifos/core/network/mappers/dataTable/GetDataTablesResponseMapper.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/newLoanAccount/NewLoanAccountScreen.ktfeature/data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/navigation/DataTableNavigation.ktcmp-navigation/src/commonMain/kotlin/cmp/navigation/authenticated/AuthenticatedNavigation.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/newLoanAccount/pages/DatatableStepPage.ktcore/network/src/commonMain/kotlin/com/mifos/core/network/model/LoansPayload.ktfeature/data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableList/DataTableListViewModel.ktfeature/data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableList/DataTableListScreen.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/newLoanAccount/NewLoanAccountViewModel.kt
**/*Screen.kt
⚙️ CodeRabbit configuration file
**/*Screen.kt: Screen architecture rules:Each screen must follow a 2-layer structure:
Layer 1 (Entry/Route Composable):
- Function Name: Typically
*ScreenRouteor the entry-point composable.- Responsibilities: Inject ViewModel (e.g.,
koinViewModel), take navigation lambdas, collect theStateFlow, and handle ViewModel events.- Logic: Should only handle ViewModel interaction, state collection, and triggering navigation in response to ViewModel events.
Layer 2 (Stateful/Content Composable —
*Screenor*ScreenContent):
- Parameters: MUST only take
state(the UI state object) and a singleonActionlambda (e.g.,onAction: (FeatureAction) -> Unit).- Responsibilities: Render the UI based strictly on the provided
state.- Rule: MUST NOT pass multiple separate lambda functions for different UI interactions; consolidate them into the single
onAction.- Constraint: Must NOT contain any business logic or ViewModel/Navigation references.
Internal/private helper composables (e.g., dialogs, sections, sub-components):
- These are NOT subject to the single
onActionrule.- They may accept specific, focused lambdas (e.g.,
onRetry: () -> Unit) or a narrowedonActionas appropriate.- However, they must NOT be passed the ViewModel or navigation controllers directly.
UI consistency:
- Avoid hardcoded values (dp, sp, padding, fontSize, colors)
- Use DesignToken, KptTheme, AppColors, MifosTypography for spacing, typography, and colors
Code quality:
- Keep Composables small and readable
- Avoid deeply nested UI
Flag:
- Missing Layer 2 (
*Screen/*ScreenContent) separation from the entry-point composable- Layer 2 (
*Screen/*ScreenContent) receiving multiple separate lambdas instead of a singleonAction- UI logic inside the entry-point composable
- Business logic inside any Composable
- Hardcoded strings instead of using string resources
- Hardcoded dp/sp values
- Direct styling instead of using DesignToken or KptTheme
Files:
feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccount/LoanAccountScreen.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/newLoanAccount/NewLoanAccountScreen.ktfeature/data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableList/DataTableListScreen.kt
**/network/**/*.kt
⚙️ CodeRabbit configuration file
**/network/**/*.kt: Network Layer API Guidelines:
- One-shot API calls MUST use
suspend, return raw responseT, and MUST NOT use Flow or DataState.- Streaming APIs MUST return
Flow<T>and MUST NOT usesuspendor DataState.
Files:
core/network/src/commonMain/kotlin/com/mifos/core/network/mappers/dataTable/GetDataTablesResponseMapper.ktcore/network/src/commonMain/kotlin/com/mifos/core/network/model/LoansPayload.kt
**/*ViewModel.kt
⚙️ CodeRabbit configuration file
**/*ViewModel.kt: MVI architecture rules:
- All new features must follow MVI
- ViewModel must extend BaseViewModel
- The ViewModel MUST maintain a single UI state (e.g., a single StateFlow)
instead of multiple separate state variables.Required:
- Use *State, *Event, *Action
- Naming must be consistent (FeatureViewModel, FeatureState, FeatureEvent, FeatureAction)
- Follow unidirectional flow:
Action → ViewModel → State → UIInternal reducer/action architecture rules:
Async operations MUST NOT directly mutate UI state repeatedly inside
Flow collectors, suspend callbacks, or repository result handlers.Repository/network/database results MUST be converted into internal
actions usingsendAction(...).
handleAction(...)must act as the primary reducer responsible for:
- state mutation
- reducer-style state transitions
- triggering follow-up actions
Large async methods must be split into:
- async collection layer
- internal action dispatching
- reducer/state handling
Avoid directly calling another business/data-loading method from
repository collectors or async callbacks.
Prefer dispatching follow-up internal actions instead.Preferred pattern:
repository result
-> sendAction(...)
-> handleAction(...)
-> mutableStateFlow.update { ... }Anti-pattern examples:
mutableStateFlow.update { ... } inside collect { }
fetchX() -> directly calls fetchY() inside async collector
large methods mixing:
- collection
- state mutation
- navigation
- business chainingFlag:
- Multiple
mutableStateFlow.update {}calls inside collect { }- Direct state mutation inside async repository callbacks
- Async methods performing both collection and reducer logic
- Direct business-flow chaining from async collectors
- Missing internal reducer actions for async results
- Missing *State / *Event / *Action
- ViewModel not...
Files:
feature/data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableList/DataTableListViewModel.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/newLoanAccount/NewLoanAccountViewModel.kt
🧠 Learnings (3)
📚 Learning: 2026-02-06T13:15:16.968Z
Learnt from: sahilshivekar
Repo: openMF/android-client PR: 2603
File: feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanTransaction/LoanTransactionsViewModel.kt:43-106
Timestamp: 2026-02-06T13:15:16.968Z
Learning: Guideline: When a Kotlin function parameter is nullable (e.g., balance: Double?, currencyCode: String?, maximumFractionDigits: Int?) and downstream calls require a non-null value, add null-safety handling in all implementations. Specifically, avoid calling Currency.getInstance(currencyCode) with a possibly null currencyCode; provide a safe default (e.g., currencyCode ?: "$") or validate before use. Ensure all platform targets (Android/Desktop/Native) follow consistent null handling, and consider centralizing currencyCode normalization in the common layer if feasible. Add tests covering null currencyCode to prevent NPEs.
Applied to files:
core/database/src/commonMain/kotlin/com/mifos/room/entities/noncore/ColumnHeader.ktcore/database/src/commonMain/kotlin/com/mifos/room/entities/templates/loans/LoanTemplate.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccount/LoanAccountScreen.ktfeature/data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableList/DataTableColumnType.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/newLoanAccount/pages/DatatableFieldValue.ktfeature/data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableList/DataTableListNavArgs.ktfeature/client/src/commonMain/kotlin/com/mifos/feature/client/navigation/ClientNavigation.ktcore/network/src/commonMain/kotlin/com/mifos/core/network/mappers/dataTable/GetDataTablesResponseMapper.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/newLoanAccount/NewLoanAccountScreen.ktfeature/data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/navigation/DataTableNavigation.ktcmp-navigation/src/commonMain/kotlin/cmp/navigation/authenticated/AuthenticatedNavigation.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/newLoanAccount/pages/DatatableStepPage.ktcore/network/src/commonMain/kotlin/com/mifos/core/network/model/LoansPayload.ktfeature/data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableList/DataTableListViewModel.ktfeature/data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableList/DataTableListScreen.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/newLoanAccount/NewLoanAccountViewModel.kt
📚 Learning: 2026-04-01T05:03:14.323Z
Learnt from: kartikey004
Repo: openMF/mifos-x-field-officer-app PR: 2659
File: feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDisbursement/LoanAccountDisbursementScreen.kt:190-195
Timestamp: 2026-04-01T05:03:14.323Z
Learning: In this repo, existing `SelectableDates` / `SelectableDates.isSelectableDate(utcTimeMillis: Long)` implementations use `Clock.System.now().toEpochMilliseconds()` (UTC epoch millis) for date-boundary checks. During PR reviews, do not flag these checks for not using a timezone-aware `LocalDate` start-of-day approach; treat it as an established project-wide pattern. If a change is desired, handle it as a coordinated project-wide improvement rather than as a per-PR review issue.
Applied to files:
core/database/src/commonMain/kotlin/com/mifos/room/entities/noncore/ColumnHeader.ktcore/database/src/commonMain/kotlin/com/mifos/room/entities/templates/loans/LoanTemplate.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccount/LoanAccountScreen.ktfeature/data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableList/DataTableColumnType.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/newLoanAccount/pages/DatatableFieldValue.ktfeature/data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableList/DataTableListNavArgs.ktfeature/client/src/commonMain/kotlin/com/mifos/feature/client/navigation/ClientNavigation.ktcore/network/src/commonMain/kotlin/com/mifos/core/network/mappers/dataTable/GetDataTablesResponseMapper.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/newLoanAccount/NewLoanAccountScreen.ktfeature/data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/navigation/DataTableNavigation.ktcmp-navigation/src/commonMain/kotlin/cmp/navigation/authenticated/AuthenticatedNavigation.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/newLoanAccount/pages/DatatableStepPage.ktcore/network/src/commonMain/kotlin/com/mifos/core/network/model/LoansPayload.ktfeature/data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableList/DataTableListViewModel.ktfeature/data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableList/DataTableListScreen.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/newLoanAccount/NewLoanAccountViewModel.kt
📚 Learning: 2026-02-16T08:37:28.351Z
Learnt from: kartikey004
Repo: openMF/android-client PR: 2610
File: feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientIdentifiersList/ClientIdentifiersListScreen.kt:60-60
Timestamp: 2026-02-16T08:37:28.351Z
Learning: In Kotlin files under the android-client module, replace MaterialTheme references with the established KptTheme import: import template.core.base.designsystem.theme.KptTheme. This should be applied consistently across files where KptTheme is used, replacing any MaterialTheme imports with the correct KptTheme import path.
Applied to files:
feature/client/src/commonMain/kotlin/com/mifos/feature/client/navigation/ClientNavigation.kt
🔇 Additional comments (11)
feature/data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableList/DataTableColumnType.kt (1)
1-33: LGTM!.gitignore (1)
101-107: LGTM!feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/newLoanAccount/pages/DatatableFieldValue.kt (1)
28-34: LGTM!core/database/src/commonMain/kotlin/com/mifos/room/entities/noncore/ColumnHeader.kt (1)
14-15: LGTM!Also applies to: 39-40, 44-45, 47-48
core/database/src/commonMain/kotlin/com/mifos/room/entities/templates/loans/LoanTemplate.kt (1)
43-44: LGTM!Also applies to: 121-122
core/network/src/commonMain/kotlin/com/mifos/core/network/model/LoansPayload.kt (1)
13-14: LGTM!Also applies to: 48-49
core/network/src/commonMain/kotlin/com/mifos/core/network/mappers/dataTable/GetDataTablesResponseMapper.kt (1)
36-37: LGTM!feature/client/src/commonMain/kotlin/com/mifos/feature/client/navigation/ClientNavigation.kt (1)
118-119: LGTM!Also applies to: 128-129, 523-525, 533-534
feature/data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableList/DataTableListNavArgs.kt (1)
16-24: LGTM!cmp-navigation/src/commonMain/kotlin/cmp/navigation/authenticated/AuthenticatedNavigation.kt (1)
16-17: LGTM!Also applies to: 22-23, 168-176
feature/data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/navigation/DataTableNavigation.kt (1)
134-142: Polymorphic nav args: encode/decodeJsonconfig is symmetric
navigateDataTableListencodesDataTableListNavArgswithnavArgJsonusingpolymorphic(Any::class)and theLoansPayload/GroupLoanPayload/ClientPayloadEntitysubclasses in DataTableNavigation.kt.DataTableListViewModeldecodes with its ownjsonconfigured with the exact same polymorphic module before callingjson.decodeFromString<DataTableListNavArgs>(...)in DataTableListViewModel.kt.
| ) | ||
| realDataTables.forEachIndexed { idx, table -> | ||
| add( | ||
| Step(table.registeredTableName ?: "Datatable ${idx + 1}") { |
There was a problem hiding this comment.
Replace hardcoded datatable fallback title with a string resource.
In NewLoanAccountScreen.kt, "Datatable ${idx + 1}" should use a localized resource (with format arg) to keep translations consistent.
As per coding guidelines, "Flag: Hardcoded strings instead of using string resources."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/newLoanAccount/NewLoanAccountScreen.kt`
at line 157, Replace the hardcoded fallback title "Datatable ${idx + 1}" with a
localized formatted string resource and use it in the Step call: change the
expression inside Step(...) where table.registeredTableName ?: "Datatable ${idx
+ 1}" is used to fetch a string resource (with a single integer format argument)
and pass (idx + 1) as the format parameter; add the corresponding localized
string resource (e.g., "datatable_title" = "Datatable %d") and ensure the code
calls the platform-appropriate string lookup (e.g., stringResource/getString or
the project’s i18n helper) so Step(...) receives the localized fallback when
table.registeredTableName is null.
Contributor
|
@TheKalpeshPawar Please address the CodeRabbitAI suggestions relevant to this PR. |
Contributor
|
@coderabbitai Improve PR title based on this: https://github.com/openMF/mifos-pay/wiki/Commit-style-guide. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Fetch the commit style guide wiki page
curl -s "https://github.com/openMF/mifos-pay/wiki/Commit-style-guide" | python3 -c "
import sys, re
html = sys.stdin.read()
# Extract text between wiki-body div
match = re.search(r'<div[^>]*class=\"[^\"]*markdown-body[^\"]*\"[^>]*>(.*?)</div>', html, re.DOTALL)
if match:
text = match.group(1)
# Strip HTML tags
text = re.sub(r'<[^>]+>', '', text)
# Clean up whitespace
text = re.sub(r'\n{3,}', '\n\n', text)
print(text.strip())
else:
# Try to find any substantial text
text = re.sub(r'<[^>]+>', '', html)
text = re.sub(r'\n{3,}', '\n\n', text)
# Print first 3000 chars
print(text[:3000])
"Length of output: 2380 🏁 Script executed: #!/bin/bash
gh pr edit 2693 --repo openMF/mifos-x-field-officer-app \
--title "feat(loan): implement dynamic datatable steps for new loan creation"Length of output: 691
Why this title?
|
biplab1
changed the title
- Drop future-only date restriction so schema-driven date columns accept past dates - Omit empty/unparsable numeric datatable fields from payload instead of sending 0/0.0 - Render empty date field when no value is set rather than a default fallback date - Build index-aligned id/label pairs for code dropdowns to avoid mismatched selections - Localize date picker action labels
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
feature/data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableList/DataTableListScreen.kt (3)
281-288:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't render today's date before the user picks one.
This field looks prefilled from the start, but nothing is written to the payload until the confirm action calls
onFieldChanged(...).DataTableListViewModel.ktonly serializes fields present informValues, so an untouched date can appear filled in the UI and still be omitted from submission.Suggested fix
- var selectedDate by rememberSaveable(name) { - mutableLongStateOf( - Clock.System.now().toEpochMilliseconds(), - ) - } + var selectedDate by rememberSaveable(name) { mutableStateOf<Long?>(null) } val datePickerState = rememberDatePickerState( initialSelectedDateMillis = selectedDate, ) @@ MifosDatePickerTextField( - value = DateHelper.getDateAsStringFromLong(selectedDate), + value = selectedDate?.let(DateHelper::getDateAsStringFromLong).orEmpty(), label = name, openDatePicker = { showDatePicker = true }, )Also applies to: 299-304, 321-323
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@feature/data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableList/DataTableListScreen.kt` around lines 281 - 288, The UI pre-fills the date by initializing `selectedDate` to today's millis and passing it to `rememberDatePickerState`, causing the field to appear filled even if `onFieldChanged(...)` was never called; change `selectedDate` to a nullable `Long?` (start null via `rememberSaveable`), pass null (or omit initial date) to `rememberDatePickerState` so the picker shows no selection initially, and only set `selectedDate` and call `onFieldChanged(...)` when the user confirms; update the other similar blocks that use `selectedDate`, `rememberSaveable`, and `rememberDatePickerState` (the occurrences mentioned) to follow the same nullable/empty-initialization pattern.
231-245:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse
KeyboardType.DecimalforDECIMAL/FLOATcolumns.
In DataTableListScreen.kt, the numeric branch setskeyboardType = KeyboardType.NumberforINTEGER,DECIMAL, andFLOAT, which can block decimal separator input on many keyboards.Suggested fix
MifosOutlinedTextField( value = value, onValueChange = { value = it onFieldChanged(name, it) }, label = name, - keyboardType = KeyboardType.Number, + keyboardType = when (columnHeader.columnDisplayType) { + DataTableColumnType.INTEGER -> KeyboardType.Number + else -> KeyboardType.Decimal + }, modifier = Modifier .fillMaxWidth() .padding(vertical = KptTheme.spacing.sm), )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@feature/data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableList/DataTableListScreen.kt` around lines 231 - 245, The numeric branch handling DataTableColumnType in DataTableListScreen's column rendering currently sets keyboardType = KeyboardType.Number for INTEGER, DECIMAL, and FLOAT; change it so INTEGER continues to use KeyboardType.Number but DECIMAL and FLOAT use KeyboardType.Decimal by branching on the enum (e.g., when DataTableColumnType.DECIMAL or FLOAT) and set keyboardType accordingly on the MifosOutlinedTextField instance (the value/onValueChange/label/keyboardType props in the DataTableListScreen rendering).
136-138:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix
clientCreated(client)being invoked during composition inDataTableListScreen
clientCreated(client)is called directly inside theDataTableListUiState.Successcomposable branch, so it can re-run on recomposition. Wrap it in a side-effect (e.g.,LaunchedEffect) instead.Suggested fix
is DataTableListUiState.Success -> { uiState.client?.let { client -> - clientCreated(client) + LaunchedEffect(client) { + clientCreated(client) + } } ?: run { if (uiState.message != null) { val message = stringResource(uiState.message) LaunchedEffect(key1 = message) { snackBarHostState.showSnackbar(message) onBackPressed() } } } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@feature/data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableList/DataTableListScreen.kt` around lines 136 - 138, In DataTableListScreen, avoid invoking clientCreated(client) directly during composition (inside the DataTableListUiState.Success branch); instead wrap the call in a Compose side-effect such as LaunchedEffect keyed by the client (e.g., client.id or the client object) so the side-effect runs only when the client value changes; update the branch where DataTableListUiState.Success is handled to call LaunchedEffect(...) { clientCreated(client) } rather than calling clientCreated(client) directly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In
`@feature/data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableList/DataTableListScreen.kt`:
- Around line 281-288: The UI pre-fills the date by initializing `selectedDate`
to today's millis and passing it to `rememberDatePickerState`, causing the field
to appear filled even if `onFieldChanged(...)` was never called; change
`selectedDate` to a nullable `Long?` (start null via `rememberSaveable`), pass
null (or omit initial date) to `rememberDatePickerState` so the picker shows no
selection initially, and only set `selectedDate` and call `onFieldChanged(...)`
when the user confirms; update the other similar blocks that use `selectedDate`,
`rememberSaveable`, and `rememberDatePickerState` (the occurrences mentioned) to
follow the same nullable/empty-initialization pattern.
- Around line 231-245: The numeric branch handling DataTableColumnType in
DataTableListScreen's column rendering currently sets keyboardType =
KeyboardType.Number for INTEGER, DECIMAL, and FLOAT; change it so INTEGER
continues to use KeyboardType.Number but DECIMAL and FLOAT use
KeyboardType.Decimal by branching on the enum (e.g., when
DataTableColumnType.DECIMAL or FLOAT) and set keyboardType accordingly on the
MifosOutlinedTextField instance (the value/onValueChange/label/keyboardType
props in the DataTableListScreen rendering).
- Around line 136-138: In DataTableListScreen, avoid invoking
clientCreated(client) directly during composition (inside the
DataTableListUiState.Success branch); instead wrap the call in a Compose
side-effect such as LaunchedEffect keyed by the client (e.g., client.id or the
client object) so the side-effect runs only when the client value changes;
update the branch where DataTableListUiState.Success is handled to call
LaunchedEffect(...) { clientCreated(client) } rather than calling
clientCreated(client) directly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ea1b74f9-9f29-4761-b16e-79bdb7e62878
📒 Files selected for processing (4)
feature/data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableList/DataTableListScreen.ktfeature/data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableList/DataTableListViewModel.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/newLoanAccount/NewLoanAccountViewModel.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/newLoanAccount/pages/DatatableStepPage.kt
🚧 Files skipped from review as they are similar to previous changes (3)
- feature/data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableList/DataTableListViewModel.kt
- feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/newLoanAccount/pages/DatatableStepPage.kt
- feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/newLoanAccount/NewLoanAccountViewModel.kt
📜 Review details
⏰ 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 context used
📓 Path-based instructions (2)
**/*Screen.kt
⚙️ CodeRabbit configuration file
**/*Screen.kt: Screen architecture rules:Each screen must follow a 2-layer structure:
Layer 1 (Entry/Route Composable):
- Function Name: Typically
*ScreenRouteor the entry-point composable.- Responsibilities: Inject ViewModel (e.g.,
koinViewModel), take navigation lambdas, collect theStateFlow, and handle ViewModel events.- Logic: Should only handle ViewModel interaction, state collection, and triggering navigation in response to ViewModel events.
Layer 2 (Stateful/Content Composable —
*Screenor*ScreenContent):
- Parameters: MUST only take
state(the UI state object) and a singleonActionlambda (e.g.,onAction: (FeatureAction) -> Unit).- Responsibilities: Render the UI based strictly on the provided
state.- Rule: MUST NOT pass multiple separate lambda functions for different UI interactions; consolidate them into the single
onAction.- Constraint: Must NOT contain any business logic or ViewModel/Navigation references.
Internal/private helper composables (e.g., dialogs, sections, sub-components):
- These are NOT subject to the single
onActionrule.- They may accept specific, focused lambdas (e.g.,
onRetry: () -> Unit) or a narrowedonActionas appropriate.- However, they must NOT be passed the ViewModel or navigation controllers directly.
UI consistency:
- Avoid hardcoded values (dp, sp, padding, fontSize, colors)
- Use DesignToken, KptTheme, AppColors, MifosTypography for spacing, typography, and colors
Code quality:
- Keep Composables small and readable
- Avoid deeply nested UI
Flag:
- Missing Layer 2 (
*Screen/*ScreenContent) separation from the entry-point composable- Layer 2 (
*Screen/*ScreenContent) receiving multiple separate lambdas instead of a singleonAction- UI logic inside the entry-point composable
- Business logic inside any Composable
- Hardcoded strings instead of using string resources
- Hardcoded dp/sp values
- Direct styling instead of using DesignToken or KptTheme
Files:
feature/data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableList/DataTableListScreen.kt
**/*.kt
⚙️ CodeRabbit configuration file
**/*.kt: Additional Code Review Guidelines:
- Null Safety & Stability
- Avoid using
!!operator- Handle null cases explicitly using safe calls or proper state handling
- Do not assume values are always non-null without guarantees
- Architecture Boundaries
- ViewModel must not depend on specific network/library implementations
- Ensure proper separation between data, domain, and presentation layers
- Do not format data (currency, dates, calculations) inside the UI layer
- All formatting must be handled in the ViewModel and exposed via state (e.g., StateFlow)
- Performance Considerations
- Avoid unnecessary recompositions in Compose
- Do not attach heavy logic to frequently changing states (e.g., scrollState)
- Prefer lifting state up instead of recomputing in child composables
- Compose & Navigation Best Practices
- NEVER trigger navigation functions or side-effects directly during composition
- Always wrap navigation calls inside
LaunchedEffectorEventsEffectto avoid repeated execution on recomposition- Avoid triggering intensive side-effects during recomposition
- Navigation routes must be type-safe.
- Ensure all route classes or objects used for navigation are annotated with
@Serializable.
- UI Structure
- Dialogs must be separated into their own composables
- Do not embed dialogs inline within complex main screens
- Localization Consistency
- Ensure all supported languages are updated consistently across modules
- Verify translations exist for newly added UI strings
- Code Cleanliness
- Avoid unnecessary inline comments unless critical
- Remove leftover debug or commented code
- Focus on correctness, readability, and maintainability over cosmetic nitpicks.
- Avoid reviewing README, config, or asset files.
- Prioritize identifying bugs, performance issues, and architectural concerns.
- Naming & Intent Rules:
- Follow the official Kotlin Coding Conventions:
https://kotlinlang.org/docs/coding-conventions.html- Use self-explanatory ...
Files:
feature/data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableList/DataTableListScreen.kt
🧠 Learnings (2)
📚 Learning: 2026-02-06T13:15:16.968Z
Learnt from: sahilshivekar
Repo: openMF/android-client PR: 2603
File: feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanTransaction/LoanTransactionsViewModel.kt:43-106
Timestamp: 2026-02-06T13:15:16.968Z
Learning: Guideline: When a Kotlin function parameter is nullable (e.g., balance: Double?, currencyCode: String?, maximumFractionDigits: Int?) and downstream calls require a non-null value, add null-safety handling in all implementations. Specifically, avoid calling Currency.getInstance(currencyCode) with a possibly null currencyCode; provide a safe default (e.g., currencyCode ?: "$") or validate before use. Ensure all platform targets (Android/Desktop/Native) follow consistent null handling, and consider centralizing currencyCode normalization in the common layer if feasible. Add tests covering null currencyCode to prevent NPEs.
Applied to files:
feature/data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableList/DataTableListScreen.kt
📚 Learning: 2026-04-01T05:03:14.323Z
Learnt from: kartikey004
Repo: openMF/mifos-x-field-officer-app PR: 2659
File: feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDisbursement/LoanAccountDisbursementScreen.kt:190-195
Timestamp: 2026-04-01T05:03:14.323Z
Learning: In this repo, existing `SelectableDates` / `SelectableDates.isSelectableDate(utcTimeMillis: Long)` implementations use `Clock.System.now().toEpochMilliseconds()` (UTC epoch millis) for date-boundary checks. During PR reviews, do not flag these checks for not using a timezone-aware `LocalDate` start-of-day approach; treat it as an established project-wide pattern. If a change is desired, handle it as a coordinated project-wide improvement rather than as a per-PR review issue.
Applied to files:
feature/data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableList/DataTableListScreen.kt
🔇 Additional comments (1)
feature/data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableList/DataTableListScreen.kt (1)
251-272: LGTM!
biplab1
reviewed
Jun 1, 2026
biplab1
requested changes
Jun 2, 2026
biplab1
left a comment
biplab1
left a comment
Contributor
There was a problem hiding this comment.
Please address the following suggestions.
Contributor
|
Please attach the screenshot of web-app screen showing the data table associated with the loan account. |
Fold the separate ui-state, data-table-list, form-values and user-status flows into a single DataTableListState exposed via stateFlow, move navigation and client-created results to one-shot events, and drive field updates and save through handleAction. DataTableListScreen now consumes stateFlow/eventFlow and dispatches via trySendAction. Removes the standalone DataTableListUiState.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@feature/data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableList/DataTableListScreen.kt`:
- Around line 114-119: DataTableListState now owns formValues but the UI only
receives dataTableList, causing controls (e.g., in TableColumnHeader) to use
local defaults; update DataTableListContent to accept and forward
state.formValues (add a formValues parameter) and propagate those values to
TableColumnHeader so each control reads its value from formValues rather than
inventing defaults; adjust the call sites that create DataTableListContent
(including the other occurrence at the 131-135 region) and update
TableColumnHeader to derive its control values from the passed formValues map
for each table/column.
- Around line 77-85: The event collector captures a stale state.userStatus and
some UI inputs (TableColumnHeader) use local rememberSaveable defaults instead
of the reducer-owned DataTableListState.formValues; fix by using
rememberUpdatedState(state.userStatus) (or snapshotFlow) inside
DataTableListScreen and call clientCreated with that updated reference when
handling DataTableListEvent.ClientCreated, and change TableColumnHeader and
other editable controls to render their value directly from
DataTableListState.formValues (remove “now”/local rememberSaveable
initializers), accept value/onValueChange from state and dispatch updates to the
reducer so the submitted payload always matches the displayed UI.
In
`@feature/data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableList/DataTableListViewModel.kt`:
- Around line 83-89: The collectors in DataTableListViewModel (e.g., the
prefManager.userInfo.map { ... }.collect block, the two repository collect { }
blocks, and createClient callbacks) must not call mutableStateFlow.update or
sendEvent directly; instead define the suggested sealed DataTableListAction
variants (UserStatusLoaded, LoanCreateStateChanged, GroupLoanCreateStateChanged,
ClientCreateSucceeded, ClientCreateFailed) and convert collected results into
those action instances (e.g., prefManager.userInfo.map {
DataTableListAction.UserStatusLoaded(it.userStatus) } ) and call
sendAction/collect(::sendAction) from the collectors; then move all state
mutations and event sends into handleAction(...) so it becomes the single
reducer for updating mutableStateFlow and emitting events.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8c6e8d2d-f480-4a1f-9294-81eb9a3492bd
📒 Files selected for processing (3)
feature/data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableList/DataTableListScreen.ktfeature/data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableList/DataTableListUiState.ktfeature/data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableList/DataTableListViewModel.kt
💤 Files with no reviewable changes (1)
- feature/data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableList/DataTableListUiState.kt
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
**/*ViewModel.kt
⚙️ CodeRabbit configuration file
**/*ViewModel.kt: MVI architecture rules:
- All new features must follow MVI
- ViewModel must extend BaseViewModel
- The ViewModel MUST maintain a single UI state (e.g., a single StateFlow)
instead of multiple separate state variables.Required:
- Use *State, *Event, *Action
- Naming must be consistent (FeatureViewModel, FeatureState, FeatureEvent, FeatureAction)
- Follow unidirectional flow:
Action → ViewModel → State → UIInternal reducer/action architecture rules:
Async operations MUST NOT directly mutate UI state repeatedly inside
Flow collectors, suspend callbacks, or repository result handlers.Repository/network/database results MUST be converted into internal
actions usingsendAction(...).
handleAction(...)must act as the primary reducer responsible for:
- state mutation
- reducer-style state transitions
- triggering follow-up actions
Large async methods must be split into:
- async collection layer
- internal action dispatching
- reducer/state handling
Avoid directly calling another business/data-loading method from
repository collectors or async callbacks.
Prefer dispatching follow-up internal actions instead.Preferred pattern:
repository result
-> sendAction(...)
-> handleAction(...)
-> mutableStateFlow.update { ... }Anti-pattern examples:
mutableStateFlow.update { ... } inside collect { }
fetchX() -> directly calls fetchY() inside async collector
large methods mixing:
- collection
- state mutation
- navigation
- business chainingFlag:
- Multiple
mutableStateFlow.update {}calls inside collect { }- Direct state mutation inside async repository callbacks
- Async methods performing both collection and reducer logic
- Direct business-flow chaining from async collectors
- Missing internal reducer actions for async results
- Missing *State / *Event / *Action
- ViewModel not...
Files:
feature/data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableList/DataTableListViewModel.kt
**/*.kt
⚙️ CodeRabbit configuration file
**/*.kt: Additional Code Review Guidelines:
- Null Safety & Stability
- Avoid using
!!operator- Handle null cases explicitly using safe calls or proper state handling
- Do not assume values are always non-null without guarantees
- Architecture Boundaries
- ViewModel must not depend on specific network/library implementations
- Ensure proper separation between data, domain, and presentation layers
- Do not format data (currency, dates, calculations) inside the UI layer
- All formatting must be handled in the ViewModel and exposed via state (e.g., StateFlow)
- Performance Considerations
- Avoid unnecessary recompositions in Compose
- Do not attach heavy logic to frequently changing states (e.g., scrollState)
- Prefer lifting state up instead of recomputing in child composables
- Compose & Navigation Best Practices
- NEVER trigger navigation functions or side-effects directly during composition
- Always wrap navigation calls inside
LaunchedEffectorEventsEffectto avoid repeated execution on recomposition- Avoid triggering intensive side-effects during recomposition
- Navigation routes must be type-safe.
- Ensure all route classes or objects used for navigation are annotated with
@Serializable.
- UI Structure
- Dialogs must be separated into their own composables
- Do not embed dialogs inline within complex main screens
- Localization Consistency
- Ensure all supported languages are updated consistently across modules
- Verify translations exist for newly added UI strings
- Code Cleanliness
- Avoid unnecessary inline comments unless critical
- Remove leftover debug or commented code
- Focus on correctness, readability, and maintainability over cosmetic nitpicks.
- Avoid reviewing README, config, or asset files.
- Prioritize identifying bugs, performance issues, and architectural concerns.
- Naming & Intent Rules:
- Follow the official Kotlin Coding Conventions:
https://kotlinlang.org/docs/coding-conventions.html- Use self-explanatory ...
Files:
feature/data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableList/DataTableListViewModel.ktfeature/data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableList/DataTableListScreen.kt
**/*Screen.kt
⚙️ CodeRabbit configuration file
**/*Screen.kt: Screen architecture rules:Each screen must follow a 2-layer structure:
Layer 1 (Entry/Route Composable):
- Function Name: Typically
*ScreenRouteor the entry-point composable.- Responsibilities: Inject ViewModel (e.g.,
koinViewModel), take navigation lambdas, collect theStateFlow, and handle ViewModel events.- Logic: Should only handle ViewModel interaction, state collection, and triggering navigation in response to ViewModel events.
Layer 2 (Stateful/Content Composable —
*Screenor*ScreenContent):
- Parameters: MUST only take
state(the UI state object) and a singleonActionlambda (e.g.,onAction: (FeatureAction) -> Unit).- Responsibilities: Render the UI based strictly on the provided
state.- Rule: MUST NOT pass multiple separate lambda functions for different UI interactions; consolidate them into the single
onAction.- Constraint: Must NOT contain any business logic or ViewModel/Navigation references.
Internal/private helper composables (e.g., dialogs, sections, sub-components):
- These are NOT subject to the single
onActionrule.- They may accept specific, focused lambdas (e.g.,
onRetry: () -> Unit) or a narrowedonActionas appropriate.- However, they must NOT be passed the ViewModel or navigation controllers directly.
UI consistency:
- Avoid hardcoded values (dp, sp, padding, fontSize, colors)
- Use DesignToken, KptTheme, AppColors, MifosTypography for spacing, typography, and colors
Code quality:
- Keep Composables small and readable
- Avoid deeply nested UI
Flag:
- Missing Layer 2 (
*Screen/*ScreenContent) separation from the entry-point composable- Layer 2 (
*Screen/*ScreenContent) receiving multiple separate lambdas instead of a singleonAction- UI logic inside the entry-point composable
- Business logic inside any Composable
- Hardcoded strings instead of using string resources
- Hardcoded dp/sp values
- Direct styling instead of using DesignToken or KptTheme
Files:
feature/data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableList/DataTableListScreen.kt
🧠 Learnings (2)
📚 Learning: 2026-02-06T13:15:16.968Z
Learnt from: sahilshivekar
Repo: openMF/android-client PR: 2603
File: feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanTransaction/LoanTransactionsViewModel.kt:43-106
Timestamp: 2026-02-06T13:15:16.968Z
Learning: Guideline: When a Kotlin function parameter is nullable (e.g., balance: Double?, currencyCode: String?, maximumFractionDigits: Int?) and downstream calls require a non-null value, add null-safety handling in all implementations. Specifically, avoid calling Currency.getInstance(currencyCode) with a possibly null currencyCode; provide a safe default (e.g., currencyCode ?: "$") or validate before use. Ensure all platform targets (Android/Desktop/Native) follow consistent null handling, and consider centralizing currencyCode normalization in the common layer if feasible. Add tests covering null currencyCode to prevent NPEs.
Applied to files:
feature/data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableList/DataTableListViewModel.ktfeature/data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableList/DataTableListScreen.kt
📚 Learning: 2026-04-01T05:03:14.323Z
Learnt from: kartikey004
Repo: openMF/mifos-x-field-officer-app PR: 2659
File: feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDisbursement/LoanAccountDisbursementScreen.kt:190-195
Timestamp: 2026-04-01T05:03:14.323Z
Learning: In this repo, existing `SelectableDates` / `SelectableDates.isSelectableDate(utcTimeMillis: Long)` implementations use `Clock.System.now().toEpochMilliseconds()` (UTC epoch millis) for date-boundary checks. During PR reviews, do not flag these checks for not using a timezone-aware `LocalDate` start-of-day approach; treat it as an established project-wide pattern. If a change is desired, handle it as a coordinated project-wide improvement rather than as a per-PR review issue.
Applied to files:
feature/data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableList/DataTableListViewModel.ktfeature/data-table/src/commonMain/kotlin/com/mifos/feature/dataTable/dataTableList/DataTableListScreen.kt
Contributor
Author
|
@biplab1 I am not able to login into the web app. |
Contributor
|
@TheKalpeshPawar @biplab1 we are testing it |
Contributor
|
@TheKalpeshPawar @biplab1 we got the following error (we are testing it using the mifos-bank-1) WhatsApp.Video.2026-06-11.at.9.11.57.PM.mp4 |
Contributor
|
@TheKalpeshPawar @biplab1 we identified the issue, it was a missing grant on the back end. It is working now. This PR can be merged! Thank you! |
niyajali
requested changes
Jun 13, 2026
| // TODO() | ||
| dataTable = { dataTables, payload -> | ||
| navController.navigateDataTableList( | ||
| dataTableList = dataTables, |
Collaborator
There was a problem hiding this comment.
Don't pass huge data via navigation arguments instead pass the ID'S and fetch the required data on specific screen
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
TheKalpeshPawar
force-pushed
the
feat/loan-datatable-dynamic-steps
branch
from
dc0f4aa to
2cc043c
Compare
Signed-off-by: thekalpeshpawar <pawarkalpesh@proton.me>
TheKalpeshPawar
force-pushed
the
feat/loan-datatable-dynamic-steps
branch
from
150d241 to
a888cd2
Compare
biplab1
requested changes
Jun 18, 2026
biplab1
left a comment
biplab1
left a comment
Contributor
There was a problem hiding this comment.
Please address the following minor changes.
Contributor
There was a problem hiding this comment.
Please revert this change.
Signed-off-by: thekalpeshpawar <pawarkalpesh@proton.me>
Contributor
|
@niyajali Please review and merge this if everything looks good. |
niyajali
requested changes
Jun 18, 2026
| } | ||
|
|
||
| private val navArgJson = Json { | ||
| serializersModule = SerializersModule { |
Collaborator
There was a problem hiding this comment.
Still you're passing these data, can you tell why we need such data on the specific screen, can't we fetch the datatable details using ID itself?
Collaborator
There was a problem hiding this comment.
Browse the API docs and find a better solution/approach
TheKalpeshPawar
force-pushed
the
feat/loan-datatable-dynamic-steps
branch
from
c203661 to
b37ff46
Compare
Signed-off-by: thekalpeshpawar <pawarkalpesh@proton.me>
TheKalpeshPawar
force-pushed
the
feat/loan-datatable-dynamic-steps
branch
from
b37ff46 to
6d02513
Compare
|
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
4 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.
Fixes - Jira-#MIFOSAC-706
screen-20260525-215619-1779726171393.2.mp4