Skip to content

feat(loan-account): Design & export repayment schedule#3111

Open
sahilshivekar wants to merge 1 commit intoopenMF:developmentfrom
sahilshivekar:feature/MM-412-Repayment-Schedule-Detail
Open

feat(loan-account): Design & export repayment schedule#3111
sahilshivekar wants to merge 1 commit intoopenMF:developmentfrom
sahilshivekar:feature/MM-412-Repayment-Schedule-Detail

Conversation

@sahilshivekar
Copy link
Contributor

@sahilshivekar sahilshivekar commented Feb 12, 2026

Fixes - MM-412

Before

old_repayment_schedule.mp4

After

Android

412_mm_repayment_table_android_working.mp4

Desktop

412_mm_repayment_table_desktop_working.mp4

WasmJS

412_mm_repayment_table_wasmjs_working.mp4

JS

412_mm_replayment_table_js_working.mp4

iOS

Not implemented and left as TODO, because the app is crashing on iOS, a new ticket on Jira will be created for iOS once this PR get's merged.

Summary by CodeRabbit

  • New Features

    • Export loan repayment schedules to PDF (landscape A4) across platforms
    • Redesigned repayment schedule into a table view with period rows, totals and an export button
    • New PDF generation API, page configuration, and reusable HTML template for PDF output
    • Added many loan-related localized strings for UI and PDF
  • Chores

    • Added HTML/PDF libraries and platform-specific PDF implementations and helpers

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 12, 2026

📝 Walkthrough

Walkthrough

Adds a cross-platform PDF export feature for loan repayment schedules: new HTML template generator, PageConfig model, multiplatform PdfGenerator implementations (Android, Desktop, JS/Wasm, native stub), composable helpers, UI table components and tokens, dependency additions (kotlinx-html, openhtmltopdf), and many loan-account strings.

Changes

Cohort / File(s) Summary
Gradle & Dependency Manifests
gradle/libs.versions.toml, core/ui/build.gradle.kts, feature/loan-account/build.gradle.kts, cmp-android/dependencies/...RuntimeClasspath.txt
Added versions and libraries for kotlinx-html and openhtmltopdf; added runtime classpath entries for kotlinx-html on Android; updated core/ui and feature gradles to include new deps.
Design System
core/designsystem/src/commonMain/kotlin/.../MifosTable.kt, core/designsystem/src/commonMain/kotlin/.../DesignToken.kt
Added MifosTableRow composable and table-related design tokens (bottom corner shape, small/medium/large table cell widths).
PDF Core (common)
core/ui/src/commonMain/kotlin/.../PageConfig.kt, .../PdfGenerator.kt, .../PdfGeneratorHelper.kt, .../HtmlTemplateGenerator.kt
Introduced PageSize/Orientation/PageConfig, an expect PdfGenerator API and expect rememberPdfGenerator() composable, plus HtmlTemplateGenerator abstract class and common CSS/styles.
PDF Platform Implementations
core/ui/src/androidMain/.../PdfGenerator.android.kt, ...Helper.android.kt, core/ui/src/desktopMain/.../PdfGenerator.desktop.kt, ...Helper.desktop.kt, core/ui/src/jsMain/.../PdfGenerator.js.kt, ...Helper.js.kt, core/ui/src/wasmJsMain/.../PdfGenerator.wasmJs.kt, ...Helper.wasmJs.kt, core/ui/src/nativeMain/.../PdfGenerator.native.kt, ...Helper.native.kt
Added actual PdfGenerator implementations per platform (Android: WebView+PrintManager; Desktop: OpenHTMLToPDF render and open file; JS/Wasm: hidden iframe + print; Native: TODO stub) and platform composable helpers returning PdfGenerator.
Loan feature — UI & ViewModel
feature/loan-account/src/commonMain/kotlin/.../RepaymentScheduleScreen.kt, .../RepaymentScheduleViewModel.kt
Refactored repayment schedule UI to table layout, added PDF export flow (new events/actions), new table data models and mapping helpers, dialog-based error handling, and wiring to rememberPdfGenerator.
Loan feature — PDF HTML generator
feature/loan-account/src/commonMain/kotlin/.../pdf/RepaymentScheduleHtmlGenerator.kt
New internal RepaymentScheduleHtmlGenerator (extends HtmlTemplateGenerator) producing XHTML/CSS table-based PDF HTML and RepaymentSchedulePdfStrings.
Localization
feature/loan-account/src/commonMain/composeResources/values*.xml, core/ui/src/commonMain/composeResources/values/strings.xml
Added ~37 loan-account string resources for repayment schedule UI and PDF export, plus core ok string.
Feature dependency usage
feature/loan-account/build.gradle.kts
Added implementation(libs.kotlinx.html) to loan-account commonMain.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Screen as RepaymentScheduleScreen
    participant VM as RepaymentScheduleViewModel
    participant HTML as RepaymentScheduleHtmlGenerator
    participant Pdf as PdfGenerator
    participant Platform

    User->>Screen: Tap "Export to PDF"
    Screen->>VM: send ExportToPdf action
    VM->>VM: emit ExportPdf event
    Screen->>HTML: instantiate with tableData & strings
    HTML->>Screen: generateHtml() -> htmlContent
    Screen->>Pdf: generateAndSharePdf(htmlContent, fileName, pageConfig)
    Pdf->>Platform: platform-specific render/print flow (WebView / OpenHTMLToPDF / iframe)
    Platform-->>User: system print dialog or opened PDF
    alt error
        Pdf->>VM: report PdfExportError
        VM->>Screen: update DialogState.Error
        Screen->>User: show error dialog
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Possibly related PRs

Suggested reviewers

  • revanthkumarJ
  • biplab1
  • mena-rizkalla

"🐰 I hopped through code and stitched a sheet,
Tables and tokens tidy and neat.
HTML to PDF, across each land,
From WebView, desktop, browser — all grand.
Hooray for exports! 🥕📄"

🚥 Pre-merge checks | ✅ 2 | ❌ 2
❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Merge Conflict Detection ⚠️ Warning ❌ Merge conflicts detected (22 files):

⚔️ cmp-android/dependencies/demoDebugRuntimeClasspath.txt (content)
⚔️ cmp-android/dependencies/demoReleaseRuntimeClasspath.txt (content)
⚔️ cmp-android/dependencies/prodDebugRuntimeClasspath.txt (content)
⚔️ cmp-android/dependencies/prodReleaseRuntimeClasspath.txt (content)
⚔️ cmp-ios/iosApp.xcodeproj/xcshareddata/xcschemes/cmp-ios.xcscheme (content)
⚔️ core/datastore/src/commonMain/kotlin/org/mifos/mobile/core/datastore/UserPreferencesDataSource.kt (content)
⚔️ core/datastore/src/commonMain/kotlin/org/mifos/mobile/core/datastore/UserPreferencesRepository.kt (content)
⚔️ core/datastore/src/commonMain/kotlin/org/mifos/mobile/core/datastore/UserPreferencesRepositoryImpl.kt (content)
⚔️ core/datastore/src/commonMain/kotlin/org/mifos/mobile/core/datastore/model/AppSettings.kt (content)
⚔️ core/designsystem/src/commonMain/kotlin/org/mifos/mobile/core/designsystem/icon/MifosIcons.kt (content)
⚔️ core/designsystem/src/commonMain/kotlin/org/mifos/mobile/core/designsystem/theme/DesignToken.kt (content)
⚔️ core/ui/build.gradle.kts (content)
⚔️ core/ui/src/commonMain/composeResources/values/strings.xml (content)
⚔️ feature/home/src/commonMain/composeResources/values/strings.xml (content)
⚔️ feature/home/src/commonMain/kotlin/org/mifos/mobile/feature/home/HomeScreen.kt (content)
⚔️ feature/home/src/commonMain/kotlin/org/mifos/mobile/feature/home/HomeViewModel.kt (content)
⚔️ feature/loan-account/build.gradle.kts (content)
⚔️ feature/loan-account/src/commonMain/composeResources/values-en/strings.xml (content)
⚔️ feature/loan-account/src/commonMain/composeResources/values/strings.xml (content)
⚔️ feature/loan-account/src/commonMain/kotlin/org/mifos/mobile/feature/loanaccount/loanAccountRepaymentSchedule/RepaymentScheduleScreen.kt (content)
⚔️ feature/loan-account/src/commonMain/kotlin/org/mifos/mobile/feature/loanaccount/loanAccountRepaymentSchedule/RepaymentScheduleViewModel.kt (content)
⚔️ gradle/libs.versions.toml (content)

These conflicts must be resolved before merging into development.
Resolve conflicts locally and push changes to this branch.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title clearly summarizes the main change: implementing design and export functionality for the loan repayment schedule, which aligns with the substantial changes across multiple files in the codebase.

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

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
⚔️ Resolve merge conflicts (beta)
  • Auto-commit resolved conflicts to branch feature/MM-412-Repayment-Schedule-Detail
  • Post resolved changes as copyable diffs in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 11

🤖 Fix all issues with AI agents
In
`@core/designsystem/src/commonMain/kotlin/org/mifos/mobile/core/designsystem/component/MifosTable.kt`:
- Around line 73-74: The loop in MifosTable.kt uses widths[index] inside
cells.forEachIndexed which will throw IndexOutOfBoundsException if cells and
widths differ; add a precondition at the start of the function (or just before
the loop) that verifies widths.size == cells.size and log/throw a clear
IllegalArgumentException if not, or change the iteration to pair cells and
widths safely (e.g., iterate over cells.zip(widths) or use getOrNull on widths)
so the code in the forEachIndexed block (referencing cells, widths, index)
cannot access an out-of-range index.

In
`@core/ui/src/androidMain/kotlin/org/mifos/mobile/core/ui/utils/pdf/PdfGenerator.android.kt`:
- Around line 40-119: generateAndSharePdf currently returns immediately because
WebView.loadDataWithBaseURL is async; bridge the callback-style PDF flow with
suspendCancellableCoroutine inside generateAndSharePdf so the suspend function
only returns after printing completes or fails. Inside the coroutine, create the
WebView and set WebViewClient.onPageFinished to build the nativeAdapter
(view?.createPrintDocumentAdapter(fileName)), obtain printManager, create
PrintAttributes via createPrintAttributes(pageConfig), then call
printManager.print(fileName, wrappedAdapter, attributes) and have the
wrappedAdapter resume the coroutine on successful onFinish or
resumeWithException on failures (e.g., null printManager, adapter errors); also
wire coroutine cancellation to destroy the WebView and cancel printing. Keep
references to WebView, nativeAdapter, wrappedAdapter, and ensure resume is
invoked exactly once.
- Around line 105-110: The WebView is only destroyed inside
PrintDocumentAdapter.onFinish (nativeAdapter.onFinish then view.post {
view.destroy() }), so if printing is cancelled earlier the WebView leaks; update
the print flow to also destroy the WebView on cancellation/error. Specifically,
in PdfGenerator.android.kt add a cleanup method (e.g., cleanupWebView()) that
posts view.destroy(), call this from the existing onFinish block, and also
attach a CancellationSignal.OnCancelListener (or override error/stop callbacks
used in your PrintDocumentAdapter onLayout/onWrite) to call cleanupWebView()
when the print is cancelled or fails; ensure every code path that aborts
printing invokes this cleanup to prevent leaks.
- Around line 34-38: The PdfGenerator currently stores the passed Activity
Context directly in its private var context, risking Activity memory leaks; in
the setContext(context: Context) method, replace storing the raw context with
context.applicationContext (i.e., assign this.context =
context.applicationContext) so PdfGenerator retains the application context
instead of the Activity context; update any nullable Context usages in
PdfGenerator to still work with applicationContext.
- Around line 44-47: The code currently silently returns when context is null
inside the withContext(Dispatchers.Main) block (see the assignment "val
appContext = context ?: return@withContext"), which hides the failure; change
this to throw an IllegalStateException (or otherwise raise an error) so the
caller's catch block can handle it—replace the null-coalescing return with
throwing IllegalStateException("Context is null in PdfGenerator.generate...")
(or similar) in the function where Pdf generation is performed to ensure
failures propagate.

In
`@core/ui/src/commonMain/kotlin/org/mifos/mobile/core/ui/utils/pdf/HtmlTemplateGenerator.kt`:
- Around line 27-47: The page title from getTitle() is injected raw into the
HTML template in generateHtml(), which can break the output if it contains
characters like <, >, or &; update generateHtml() to HTML-escape the title first
(e.g., val escapedTitle = escapeHtml(getTitle())) and then use that escapedTitle
in the template instead of getTitle(); reference the generateHtml() method and
getTitle() to locate the change and use a safe HTML-escaping utility appropriate
for the project (or add a small helper escapeHtml function) so subclasses like
RepaymentScheduleHtmlGenerator remain safe.

In
`@core/ui/src/desktopMain/kotlin/org/mifos/mobile/core/ui/utils/pdf/PdfGenerator.desktop.kt`:
- Around line 48-49: A temporary HTML file (htmlFile created with
File(downloadsDir, "$fileName.html") and written via
htmlFile.writeText(finalHtml)) is left in Downloads; ensure the file is deleted
after PDF rendering succeeds by adding cleanup logic that deletes htmlFile after
the PDF creation step completes (or in a finally block so it runs on both
success and failure), e.g., surround the HTML-write + PDF-render sequence with
try/finally and call htmlFile.deleteIfExists() (checking existence) so the
intermediate artifact is removed; reference the htmlFile, downloadsDir, fileName
and the PDF rendering call in PdfGenerator.desktop.kt when applying this change.

In
`@core/ui/src/wasmJsMain/kotlin/org/mifos/mobile/core/ui/utils/pdf/PdfGenerator.wasmJs.kt`:
- Around line 21-78: The generateAndSharePdf suspend function currently
schedules printing and cleanup inside setTimeout callbacks so it returns before
the print dialog/cleanup finish; update the KDoc for generateAndSharePdf in
PdfGenerator.wasmJs.kt to clearly state that callers cannot rely on the suspend
call to indicate print completion or success/failure and that errors are
delivered only to internal logs, not via exceptions. Also include guidance for
callers: either treat the function as fire-and-forget, or change the
implementation to explicitly await completion by using a
suspendCancellableCoroutine (resuming after the inner timeout/cleanup or on
error) if callers need a reliable success/failure signal. Ensure the KDoc
references the browser print API limitation and mentions the alternative
(suspendCancellableCoroutine) and current behavior so callers know how to
proceed.

In
`@feature/loan-account/src/commonMain/kotlin/org/mifos/mobile/feature/loanaccount/loanAccountRepaymentSchedule/pdf/RepaymentScheduleHtmlGenerator.kt`:
- Around line 202-204: The footer date is built from currentDate.month.name
(English enum) which isn’t localized; update the construction of dateString in
RepaymentScheduleHtmlGenerator so it’s locale-safe — either use
currentDate.toString() to produce an ISO yyyy-MM-dd value or replace the manual
month name with a localized string source (e.g., use RepaymentSchedulePdfStrings
or a platform-specific DateTimeFormatter tied to the user's Locale); modify the
code that builds dateString (the block referencing
currentDate.day/currentDate.month.name/currentDate.year) to use one of these
approaches.

In
`@feature/loan-account/src/commonMain/kotlin/org/mifos/mobile/feature/loanaccount/loanAccountRepaymentSchedule/RepaymentScheduleScreen.kt`:
- Around line 119-144: The catch block in the RepaymentScheduleEvent.ExportPdf
handler is catching all Exceptions and thus swallowing CancellationException;
update the launch block around createPdfStrings /
RepaymentScheduleHtmlGenerator.generateHtml / pdfGenerator.generateAndSharePdf
so that CancellationException is rethrown (or excluded) — e.g., catch specific
exceptions like IOException/IllegalStateException or catch Exception and
immediately rethrow if it is a CancellationException before handling, then keep
the existing
viewModel.trySendAction(RepaymentScheduleAction.PdfExportError(...)) for other
errors.

In `@gradle/libs.versions.toml`:
- Around line 92-93: Update the kotlinxHtml version entry in the dependency
catalog: change the kotlinxHtml = "0.11.0" declaration to kotlinxHtml = "0.12.0"
so the project uses the January 15, 2025 release (Kotlin 2.0.0 support and
attribute/platform fixes); verify any code or build scripts that reference
kotlinx-html artifacts still compile with the 0.12.0 API and run the Gradle
dependency refresh to ensure the new version is resolved.
🧹 Nitpick comments (12)
core/ui/src/commonMain/composeResources/values/strings.xml (1)

15-15: Nit: "Ok" should be "OK".

"OK" is the standard abbreviation in English UI text. Most style guides (Material Design, Apple HIG) use "OK" for button labels.

Proposed fix
-    <string name="ok">Ok</string>
+    <string name="ok">OK</string>
core/ui/src/commonMain/kotlin/org/mifos/mobile/core/ui/utils/pdf/HtmlTemplateGenerator.kt (1)

194-194: Magic placeholder string PAGE_CONFIG_PLACEHOLDER should be documented or extracted as a constant.

The comment /* PAGE_CONFIG_PLACEHOLDER */ appears to be a sentinel for downstream string replacement (presumably to inject page-size CSS). Consider extracting it as a companion object constant so both the generator and the consumer reference the same value, avoiding silent breakage if one side changes the string.

core/designsystem/src/commonMain/kotlin/org/mifos/mobile/core/designsystem/component/MifosTable.kt (1)

64-71: Unconditional clickable modifier causes ripple on every row, even when no click handler is provided.

The .clickable { onClick() } is always applied regardless of whether the caller passed a meaningful onClick. This produces an unintended ripple/touch feedback on non-interactive rows.

Consider making onClick nullable and applying .clickable conditionally:

♻️ Proposed fix
-    onClick: () -> Unit = {},
+    onClick: (() -> Unit)? = null,
 ) {
     ...
         Row(
             modifier = Modifier
                 .horizontalScroll(scrollState)
                 .height(IntrinsicSize.Min)
                 .padding(horizontal = edgeOffset)
                 .clip(cornerShape)
                 .background(backgroundColor)
-                .clickable { onClick() },
+                .then(
+                    if (onClick != null) Modifier.clickable { onClick() }
+                    else Modifier,
+                ),
         ) {
core/ui/src/nativeMain/kotlin/org/mifos/mobile/core/ui/utils/pdf/PdfGenerator.native.kt (2)

10-14: Unnecessary @OptIn(ExperimentalForeignApi::class) — no foreign API is used.

No kotlinx.cinterop APIs are called in this file. Remove the opt-in and the import to keep it clean until the actual iOS implementation lands.

Proposed diff
-@file:OptIn(ExperimentalForeignApi::class)
-
 package org.mifos.mobile.core.ui.utils.pdf
-
-import kotlinx.cinterop.ExperimentalForeignApi

19-22: Silent no-op may confuse users — consider throwing NotImplementedError.

The empty body silently succeeds, so on iOS the user taps "Export" and nothing visible happens. Using TODO("iOS PDF generation not yet implemented") would throw at runtime with a clear message, making it obvious during testing that this path is unimplemented rather than silently broken.

 actual class PdfGenerator {
     actual suspend fun generateAndSharePdf(htmlContent: String, fileName: String, pageConfig: PageConfig) {
-        // TODO
+        TODO("iOS PDF generation not yet implemented")
     }
 }
core/ui/src/wasmJsMain/kotlin/org/mifos/mobile/core/ui/utils/pdf/PdfGenerator.wasmJs.kt (1)

20-79: Near-complete duplication with the JS implementation (PdfGenerator.js.kt).

This file and PdfGenerator.js.kt are nearly identical — the only differences are println vs console.log and null return values in setTimeout callbacks. Consider extracting the shared logic into a common internal function (e.g., in a jsCommon or shared JS source set) to avoid maintaining two copies.

core/ui/src/desktopMain/kotlin/org/mifos/mobile/core/ui/utils/pdf/PdfGenerator.desktop.kt (1)

51-57: withHtmlContent(finalHtml, null)null base URI means relative resource paths won't resolve.

If the HTML ever references external resources (images, fonts, stylesheets) via relative paths, they won't load. This is fine as long as the HTML is fully self-contained, but worth noting with a comment for future maintainers. Since the HTML file is already written to disk, you could use its URI as the base URI at no extra cost:

-                        .withHtmlContent(finalHtml, null)
+                        .withHtmlContent(finalHtml, htmlFile.toURI().toString())
feature/loan-account/src/commonMain/kotlin/org/mifos/mobile/feature/loanaccount/loanAccountRepaymentSchedule/RepaymentScheduleScreen.kt (4)

435-453: Magic index constants (2, 4) in DisbursementRow are fragile.

If column order changes, these indices silently break. Consider using named constants or a mapping approach consistent with how RepaymentPeriodRow uses a named list of values.

Proposed improvement
+private const val DATE_COLUMN_INDEX = 2
+private const val BALANCE_COLUMN_INDEX = 4

 val cells: List<@Composable () -> Unit> = List(14) { index ->
     {
         // ...
         val text = when (index) {
-            2 -> data.disbursementDate
-            4 -> data.loanAmount
+            DATE_COLUMN_INDEX -> data.disbursementDate
+            BALANCE_COLUMN_INDEX -> data.loanAmount
             else -> ""
         }

291-293: Consider providing a stable key for items to improve LazyColumn performance.

Without a key, Compose uses positional indexing, which can cause unnecessary recompositions when items are reordered or the list changes.

Proposed fix
-                        items(tableData.periods.size) { index ->
-                            RepaymentPeriodRow(tableData.periods[index], columnWidths, scrollState)
+                        items(
+                            count = tableData.periods.size,
+                            key = { index -> tableData.periods[index].number },
+                        ) { index ->
+                            RepaymentPeriodRow(tableData.periods[index], columnWidths, scrollState)
                         }

384-530: Duplicated cell-rendering pattern across all four table composables.

RepaymentTableHeader, DisbursementRow, RepaymentPeriodRow, and RepaymentTableFooter all share the same Box(Modifier.fillMaxWidth().padding(...))Text(...) cell pattern. Consider extracting a small TableCell helper to reduce repetition.


316-316: Empty else branch.

If ScreenUiState is a sealed class/interface, the else branch is unnecessary and could mask future additions. If it's not sealed, consider logging or handling unexpected states explicitly.

feature/loan-account/src/commonMain/kotlin/org/mifos/mobile/feature/loanaccount/loanAccountRepaymentSchedule/pdf/RepaymentScheduleHtmlGenerator.kt (1)

233-248: Hardcoded color values in CSS.

Colors like #1976d2 (Material Blue) are hardcoded in the CSS string. This is fine for now but makes theming the PDF difficult. If this is acceptable for V1, consider adding a TODO for future parameterization.

@sahilshivekar sahilshivekar force-pushed the feature/MM-412-Repayment-Schedule-Detail branch from 3a3d3d7 to 6ab27f5 Compare February 13, 2026 16:03
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In
`@core/ui/src/jsMain/kotlin/org/mifos/mobile/core/ui/utils/pdf/PdfGenerator.js.kt`:
- Around line 47-81: The onload handler for the created iframe is attached too
early and may fire for the initial about:blank document; move the assignment of
iframe.onload so it is set after writing the HTML into the iframe document but
before calling frameDoc.close() (i.e., create iframe and append it, get
frameDoc, call frameDoc.open(), frameDoc.write(finalHtml), then set
iframe.onload to run the print + completionSignal.complete/completeExceptionally
inside the setTimeout, and finally call frameDoc.close()); ensure you still
throw if frameDoc is null and await completionSignal as before.

In `@feature/loan-account/src/commonMain/composeResources/values-en/strings.xml`:
- Around line 143-145: Add the missing English locale entries for the
loan-export error strings: define string resources named
feature_loan_export_pdf_error and feature_loan_export_pdf_error_title in
values-en (matching the default values/strings.xml text) so the values-en locale
contains the same keys as default and remains consistent with the other
duplicated strings.
🧹 Nitpick comments (2)
feature/loan-account/src/commonMain/kotlin/org/mifos/mobile/feature/loanaccount/loanAccountRepaymentSchedule/RepaymentScheduleScreen.kt (2)

437-455: Magic index numbers 2 and 4 are fragile and will silently break if column order changes.

The DisbursementRow uses hardcoded indices to place disbursementDate at position 2 and loanAmount at position 4. If the column order in RepaymentTableHeader is ever reordered, these will silently show data in wrong columns. Consider using named constants or deriving cell content from the column definitions.


386-532: Consider extracting the repeated cell-rendering pattern into a shared helper.

All four table composables (RepaymentTableHeader, DisbursementRow, RepaymentPeriodRow, RepaymentTableFooter) duplicate the same Box(Modifier.fillMaxWidth().padding(...)) { Text(...) } cell pattern. A small helper composable would reduce duplication.

Example helper
`@Composable`
private fun TableCell(
    text: String,
    style: TextStyle = MifosTypography.labelMedium,
) {
    Box(
        modifier = Modifier
            .fillMaxWidth()
            .padding(
                vertical = DesignToken.padding.small,
                horizontal = DesignToken.padding.extraSmall,
            ),
    ) {
        Text(text = text, style = style)
    }
}

@sahilshivekar sahilshivekar force-pushed the feature/MM-412-Repayment-Schedule-Detail branch from 6ab27f5 to c0b27f7 Compare February 13, 2026 16:38
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Fix all issues with AI agents
In `@cmp-android/dependencies/prodDebugRuntimeClasspath.txt`:
- Around line 423-424: Update the kotlinx-html dependency version from 0.11.0 to
0.12.0 in the project's versions catalog: find the version entry for
kotlinx-html (the libs.versions.toml key that controls
org.jetbrains.kotlinx:kotlinx-html and org.jetbrains.kotlinx:kotlinx-html-jvm)
and change its value to "0.12.0" so both coordinates (kotlinx-html and
kotlinx-html-jvm) use the 0.12.0 artifact built for Kotlin 2.x compatibility.

In
`@core/ui/src/nativeMain/kotlin/org/mifos/mobile/core/ui/utils/pdf/PdfGenerator.native.kt`:
- Around line 19-23: The iOS actual implementation of
PdfGenerator.generateAndSharePdf is a silent no-op; replace the // TODO comment
body with a call to TODO("...") so the method immediately throws when invoked on
iOS. Locate the actual class PdfGenerator and its suspend function
generateAndSharePdf(htmlContent: String, fileName: String, pageConfig:
PageConfig) and change the empty implementation to call TODO with a clear
message (e.g., that PDF generation is not implemented on iOS) so callers get a
visible failure instead of a silently swallowed call.

In
`@core/ui/src/wasmJsMain/kotlin/org/mifos/mobile/core/ui/utils/pdf/PdfGenerator.wasmJs.kt`:
- Around line 61-75: The onload handler can never fire if the iframe isn't
attached and currently the code uses a null-safe contentWindow?.print() then
completes success even if print didn't run; update PdfGenerator.wasmJs.kt to (1)
verify document.body before writing/appending — if document.body is null, fail
fast or throw rather than proceeding; (2) ensure the iframe is appended to the
DOM (appendChild) before writing so onload can fire; (3) replace the
contentWindow?.print() + unconditional completionSignal.complete(Unit) with an
explicit check of iframe.contentWindow (fail or completeExceptionally if null)
and only complete successfully after invoking print; and (4) add a
timeout/failsafe around completionSignal.await() (e.g., completeExceptionally
after N ms) so the caller won’t hang indefinitely.
🧹 Nitpick comments (3)
core/ui/src/nativeMain/kotlin/org/mifos/mobile/core/ui/utils/pdf/PdfGenerator.native.kt (1)

10-14: Premature ExperimentalForeignApi opt-in on a stub with no cinterop usage.

The file doesn't use any foreign API. This can be removed now and added back when the actual iOS implementation lands, keeping the code honest about its dependencies.

Proposed fix
-@file:OptIn(ExperimentalForeignApi::class)
-
 package org.mifos.mobile.core.ui.utils.pdf
-
-import kotlinx.cinterop.ExperimentalForeignApi
core/ui/src/wasmJsMain/kotlin/org/mifos/mobile/core/ui/utils/pdf/PdfGenerator.wasmJs.kt (1)

80-80: completionSignal.await() has no timeout — can hang indefinitely.

If onload never fires (e.g., due to a browser quirk or race condition), this suspends forever. Consider wrapping with withTimeout to provide a bounded wait.

Proposed fix
+import kotlinx.coroutines.withTimeout
+
 // Replace:
-            completionSignal.await()
+            withTimeout(30_000L) {
+                completionSignal.await()
+            }
feature/loan-account/src/commonMain/kotlin/org/mifos/mobile/feature/loanaccount/loanAccountRepaymentSchedule/RepaymentScheduleScreen.kt (1)

437-455: Magic indices in DisbursementRow reduce maintainability.

Indices 2 and 4 (lines 448–449) silently couple to the column order. If columns are reordered, this will break without any compiler warning.

Consider extracting column index constants shared with the header definition, or using a named mapping approach.

Comment on lines +423 to +424
org.jetbrains.kotlinx:kotlinx-html-jvm:0.11.0
org.jetbrains.kotlinx:kotlinx-html:0.11.0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify kotlinx-html version alignment across the repo

echo "=== All kotlinx-html references (excluding build artifacts) ==="
rg -n 'kotlinx-html' --max-count=30 -g '!**/build/**' -g '!**/dist/**'

echo -e "\n=== Version catalog files ==="
fd -e 'toml' -e 'properties' | head -20 | xargs -I {} sh -c 'echo "File: {}"; cat {} 2>/dev/null | grep -i kotlin | head -5'

echo -e "\n=== Build gradle files containing kotlinx-html ==="
find . -name '*.gradle.kts' -o -name 'build.gradle' | xargs grep -l 'kotlinx-html' 2>/dev/null

echo -e "\n=== Checking for kotlin version ==="
rg -n 'kotlin.*version|org\.jetbrains\.kotlin' -g '*.toml' -g '*.gradle*' -g '*.properties' | head -20

Repository: openMF/mifos-mobile

Length of output: 4709


🏁 Script executed:

#!/bin/bash
# Get the exact kotlinxHtml version definition and surrounding context
cat gradle/libs.versions.toml | grep -A 2 -B 2 "kotlinxHtml"

Repository: openMF/mifos-mobile

Length of output: 470


🏁 Script executed:

#!/bin/bash
# Check if there are any issues or conflicts related to kotlinx-html in build files
echo "=== Checking for any explicit version constraints or exclusions ==="
rg -n 'exclude|exclude\(|conflict|resolution' gradle/libs.versions.toml

echo -e "\n=== Checking HtmlTemplateGenerator.kt usage ==="
cat core/ui/src/commonMain/kotlin/org/mifos/mobile/core/ui/utils/pdf/HtmlTemplateGenerator.kt | head -50

echo -e "\n=== Checking if kotlinx-html is used elsewhere ==="
rg -l 'import.*kotlinx.*html' -g '*.kt'

Repository: openMF/mifos-mobile

Length of output: 2008


🌐 Web query:

kotlinx-html 0.11.0 Kotlin 2.1.20 compatibility

💡 Result:

kotlinx-html 0.11.0 was built against the Kotlin 1.9.x line (for example, the kotlinx-html-jvm:0.11.0 artifact has a runtime dependency on kotlin-stdlib:1.9.22). [1]

Kotlin 2.1.20 was released on March 20, 2025. [2]

What this means in practice:

  • JVM-only projects: it will usually work (Kotlin is generally backward/binary compatible with libraries compiled with older Kotlin), but 0.11.0 is not a “Kotlin 2.1.x-targeted” build.
  • Recommended: upgrade to kotlinx-html 0.12.0, which is built against the Kotlin 2.0.x line (for example, it depends on kotlin-stdlib:2.0.21), making it a much closer match for Kotlin 2.1.20. [3]

Bottom line: there’s no clear published guarantee that kotlinx-html:0.11.0 is “Kotlin 2.1.20 compatible”, but it’s a Kotlin 1.9-era build; for Kotlin 2.1.20, prefer 0.12.0 (or newer). [1][3]

Sources

  1. MvnRepository – kotlinx-html-jvm:0.11.0 dependencies (kotlin-stdlib:1.9.22). [1]
  2. Kotlin docs – Kotlin 2.1.20 release date (March 20, 2025). [2]
  3. MvnRepository – kotlinx-html:0.12.0 runtime dependency (kotlin-stdlib:2.0.21). [3]

🏁 Script executed:

#!/bin/bash
# Check if there's a newer version of kotlinx-html available and check build constraints
echo "=== Check for other kotlinx versions in the catalog ==="
rg -n "kotlinx" gradle/libs.versions.toml | head -30

echo -e "\n=== Check gradle files for any version constraints ==="
find . -name "build.gradle.kts" -o -name "build.gradle" | xargs grep -l "kotlinx-html" 2>/dev/null | head -5 | xargs cat

echo -e "\n=== Check if there's a CHANGELOG or version notes ==="
fd -e "CHANGELOG*" -e "CHANGES*" -e "RELEASE*" | head -5

Repository: openMF/mifos-mobile

Length of output: 1731


Upgrade kotlinx-html to 0.12.0 for Kotlin 2.1.20 compatibility.

The project uses Kotlin 2.1.20 but kotlinx-html 0.11.0 (built against Kotlin 1.9.x). While backward compatibility may allow it to work, kotlinx-html 0.12.0 is built against Kotlin 2.0.x and is the recommended version for Kotlin 2.1.20. Update the version reference in gradle/libs.versions.toml from 0.11.0 to 0.12.0.

🤖 Prompt for AI Agents
In `@cmp-android/dependencies/prodDebugRuntimeClasspath.txt` around lines 423 -
424, Update the kotlinx-html dependency version from 0.11.0 to 0.12.0 in the
project's versions catalog: find the version entry for kotlinx-html (the
libs.versions.toml key that controls org.jetbrains.kotlinx:kotlinx-html and
org.jetbrains.kotlinx:kotlinx-html-jvm) and change its value to "0.12.0" so both
coordinates (kotlinx-html and kotlinx-html-jvm) use the 0.12.0 artifact built
for Kotlin 2.x compatibility.

Comment on lines +19 to +23
actual class PdfGenerator {
actual suspend fun generateAndSharePdf(htmlContent: String, fileName: String, pageConfig: PageConfig) {
// TODO
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Silent no-op will hide failures on iOS — use TODO() to fail explicitly.

The empty body means callers on iOS will believe PDF export succeeded (no exception thrown), but nothing actually happens. Replace the comment with a TODO() call so any accidental invocation surfaces immediately rather than silently swallowing the request.

Proposed fix
 actual class PdfGenerator {
     actual suspend fun generateAndSharePdf(htmlContent: String, fileName: String, pageConfig: PageConfig) {
-        // TODO
+        TODO("iOS PDF generation not yet implemented")
     }
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
actual class PdfGenerator {
actual suspend fun generateAndSharePdf(htmlContent: String, fileName: String, pageConfig: PageConfig) {
// TODO
}
}
actual class PdfGenerator {
actual suspend fun generateAndSharePdf(htmlContent: String, fileName: String, pageConfig: PageConfig) {
TODO("iOS PDF generation not yet implemented")
}
}
🤖 Prompt for AI Agents
In
`@core/ui/src/nativeMain/kotlin/org/mifos/mobile/core/ui/utils/pdf/PdfGenerator.native.kt`
around lines 19 - 23, The iOS actual implementation of
PdfGenerator.generateAndSharePdf is a silent no-op; replace the // TODO comment
body with a call to TODO("...") so the method immediately throws when invoked on
iOS. Locate the actual class PdfGenerator and its suspend function
generateAndSharePdf(htmlContent: String, fileName: String, pageConfig:
PageConfig) and change the empty implementation to call TODO with a clear
message (e.g., that PDF generation is not implemented on iOS) so callers get a
visible failure instead of a silently swallowed call.

Comment on lines +61 to +75
iframe.onload = {
try {
window.setTimeout(
{
iframe.contentWindow?.print()
completionSignal.complete(Unit)
null
},
0,
)
} catch (e: Exception) {
completionSignal.completeExceptionally(e)
}
}
frameDoc.close()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

onload may never fire if iframe isn't attached to the DOM, causing indefinite hang.

If document.body is null on Line 55, the iframe isn't appended to the DOM, yet the code proceeds to write content and await the completionSignal. The onload event won't fire for a detached iframe, so completionSignal.await() on Line 80 will hang forever. Consider adding a guard or a timeout on the await.

Additionally, iframe.contentWindow?.print() (Line 65) uses a null-safe call — if contentWindow becomes null, print() is skipped but completionSignal.complete(Unit) still fires on Line 66, signaling success when no print actually occurred.

Proposed fix — guard against null body and null contentWindow
-            document.body?.appendChild(iframe)
+            val body = document.body
+                ?: throw Exception("Document body is not available")
+            body.appendChild(iframe)
-                                iframe.contentWindow?.print()
-                                completionSignal.complete(Unit)
+                                val win = iframe.contentWindow
+                                    ?: throw Exception("Lost access to iframe content window")
+                                win.print()
+                                completionSignal.complete(Unit)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
iframe.onload = {
try {
window.setTimeout(
{
iframe.contentWindow?.print()
completionSignal.complete(Unit)
null
},
0,
)
} catch (e: Exception) {
completionSignal.completeExceptionally(e)
}
}
frameDoc.close()
iframe.onload = {
try {
window.setTimeout(
{
val win = iframe.contentWindow
?: throw Exception("Lost access to iframe content window")
win.print()
completionSignal.complete(Unit)
null
},
0,
)
} catch (e: Exception) {
completionSignal.completeExceptionally(e)
}
}
frameDoc.close()
🤖 Prompt for AI Agents
In
`@core/ui/src/wasmJsMain/kotlin/org/mifos/mobile/core/ui/utils/pdf/PdfGenerator.wasmJs.kt`
around lines 61 - 75, The onload handler can never fire if the iframe isn't
attached and currently the code uses a null-safe contentWindow?.print() then
completes success even if print didn't run; update PdfGenerator.wasmJs.kt to (1)
verify document.body before writing/appending — if document.body is null, fail
fast or throw rather than proceeding; (2) ensure the iframe is appended to the
DOM (appendChild) before writing so onload can fire; (3) replace the
contentWindow?.print() + unconditional completionSignal.complete(Unit) with an
explicit check of iframe.contentWindow (fail or completeExceptionally if null)
and only complete successfully after invoking print; and (4) add a
timeout/failsafe around completionSignal.await() (e.g., completeExceptionally
after N ms) so the caller won’t hang indefinitely.

@Composable
fun MifosTableRow(
cells: List<@Composable () -> Unit>,
widths: List<Dp>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might not be safe i think.
what if we pass 4 cells and only 3 width. what about making a data class where the cell content and it's width defined together ?

}
Row(
modifier = Modifier
.horizontalScroll(scrollState)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doing this will affect a lot on performance every time the scrollState change the app will recompose all the rows at once and this might lag on old devices please move the horizontal scroll to a parent container not the individual rows

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants