fix - Introduce LocalizedDateFormatter for multiplatform date localization#3110
fix - Introduce LocalizedDateFormatter for multiplatform date localization#3110Aryan-Baglane wants to merge 11 commits intoopenMF:developmentfrom
Conversation
|
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:
📝 WalkthroughWalkthroughAdds a multiplatform LocalizedDateFormatter (expect/actual) with platform-specific formatting and relative-prefix logic; refactors DateHelper to use it, removes legacy month/name helpers, adds backend/ui date handling, and updates transfer feature to surface a uiTransferDate. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant LocalizedDateFormatter
participant PlatformFormatter
Caller->>LocalizedDateFormatter: formatFullDate(year, month, day)
LocalizedDateFormatter->>PlatformFormatter: build date & format (locale-aware)
PlatformFormatter-->>LocalizedDateFormatter: formatted text
LocalizedDateFormatter->>LocalizedDateFormatter: localizeDateDigits(formatted text)
LocalizedDateFormatter-->>Caller: localized full date string
Caller->>LocalizedDateFormatter: getRelativePrefix(year, month, day)
LocalizedDateFormatter->>PlatformFormatter: determine today/yesterday & compute relative text
PlatformFormatter-->>LocalizedDateFormatter: relative text or null
LocalizedDateFormatter->>LocalizedDateFormatter: localizeDateDigits(relative text)
LocalizedDateFormatter-->>Caller: localized relative prefix or null
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
core/common/src/commonMain/kotlin/org/mifos/mobile/core/common/DateHelper.kt (1)
71-76:⚠️ Potential issue | 🔴 CriticalCritical:
getDateAsString(List<Long>, pattern)is broken — format mismatch ingetFormatConverter.This calls
getDateAsString(integersOfDate.map { it.toInt() })which now returns a locale-formatted string (e.g.,"14 January 2026"or"١٤ يناير ٢٠٢٦"). The result is then passed togetFormatConverter, which parses it withFULL_MONTH("dd MM yyyy"— numeric month). This parse will throw because a locale-formatted date string doesn't match the"dd MM yyyy"pattern.
🤖 Fix all issues with AI agents
In
`@core/common/src/androidMain/kotlin/org/mifos/mobile/core/common/LocalizedDateFormatter.android.kt`:
- Around line 21-29: The formatFullDate function calls LocalDate.of(year, month,
day) which can throw DateTimeException for invalid inputs; wrap the
creation/formatting (LocalDate.of(...) and date.format(...)) in a try/catch for
DateTimeException (like getRelativePrefix does) and handle it consistently—e.g.,
fall back to a safe string (such as "$day/$month/$year" optionally passed
through localizeDateDigits) or return an empty string, and ensure the catch
returns that fallback instead of letting the exception propagate; update
references to LocalDate.of, DateTimeFormatter.ofPattern, date.format, and
localizeDateDigits accordingly.
- Around line 46-66: The Android getRelativePrefix implementation currently
returns relative strings for any date because it always uses
DateUtils.getRelativeTimeSpanString; change it to only return values for today
or yesterday: inside getRelativePrefix (and before calling
DateUtils.getRelativeTimeSpanString/localizeDateDigits) compute
LocalDate.now(ZoneId.systemDefault()) and compare targetDate (from
LocalDate.of(year, month, day)) to today and today.minusDays(1); if targetDate
is neither today nor yesterday, return null, otherwise proceed to compute
millis, call DateUtils.getRelativeTimeSpanString and pass the result to
localizeDateDigits as before; keep the existing try/catch for DateTimeException.
In
`@core/common/src/commonMain/kotlin/org/mifos/mobile/core/common/DateHelper.kt`:
- Around line 118-125: getDateAsList currently attempts to parse a localized
formatted date string using LocalDateTime.Format with the pattern "d MMMM yyyy",
which fails on JS/Native/WASM and when digits are localized; instead refactor to
avoid parsing formatted strings: change getDateAsList(signature) to accept raw
numeric components (year, month, day) or provide a new helper that takes
(year:Int, month:Int, day:Int) and returns List<Int>, and update callers such as
getDateAsLongFromList to pass those numeric components directly (remove reliance
on LocalDateTime.Format/formatter.parse and any platform-localized formatting).
- Around line 140-148: getFormatConverter currently assumes the input dateString
uses FULL_MONTH ("dd MM yyyy") which no longer matches values produced by
LocalizedDateFormatter.formatFullDate; update getFormatConverter to parse using
the same formatter/pattern used by LocalizedDateFormatter (instead of
LocalDateTime.Format { byUnicodePattern(FULL_MONTH) }) so parsing succeeds:
either obtain or reuse the exact source formatter/pattern from
LocalizedDateFormatter.formatFullDate (or expose a source pattern constant) and
replace pickerFormat with that parser, then format with finalFormat as before.
In
`@core/common/src/commonMain/kotlin/org/mifos/mobile/core/common/LocalizedDateFormatter.kt`:
- Around line 12-23: The two API methods use inconsistent parameter ordering
which is error-prone: change getRelativePrefix(day: Int, month: Int, year: Int)
to getRelativePrefix(year: Int, month: Int, day: Int) so both formatFullDate and
getRelativePrefix use (year, month, day); update all expect/actual declarations
and every call site that invokes getRelativePrefix to the new parameter order,
update KDoc for getRelativePrefix to state (year, month, day), and ensure all
platform-specific actual implementations (the LocalizedDateFormatter actuals)
construct platform date objects using the same (year, month, day) order to avoid
runtime bugs.
In
`@core/common/src/jsMain/kotlin/org/mifos/mobile/core/common/LocalizedDateFormatter.kt`:
- Around line 54-58: The top-level helper function dateLocaleOptions is
unintentionally public; change its visibility from public to internal (or
private if only used in this file) by updating its declaration to internal fun
dateLocaleOptions(...) to avoid leaking it into the public API, then update any
callers (if you made it private and callers are outside this file change back to
internal) and run the build/tests to ensure no usages break; reference function
name dateLocaleOptions and file LocalizedDateFormatter.kt to locate the
declaration.
- Around line 42-44: The code constructs rtf via js("new
Intl.RelativeTimeFormat(...)") which will throw if Intl.RelativeTimeFormat is
unavailable; update LocalizedDateFormatter to guard and provide fallback: before
creating rtf check (via js interop) that Intl and Intl.RelativeTimeFormat exist
and only instantiate rtf when supported, otherwise set rtf to null/undefined and
implement a fallback path inside getRelativePrefix (or the method that uses rtf)
to return sensible strings when rtf is missing; refer to the rtf variable and
getRelativePrefix to locate where to add the runtime check and fallback
behavior.
In
`@core/common/src/wasmJsMain/kotlin/org/mifos/mobile/core/common/LocalizedDateFormatter.kt`:
- Around line 30-51: The JS lambda for relativePrefixJs has the wrong parameter
list and wrong behavior: update the `@JsFun` body signature to accept (year,
month, day) and implement real relative-date logic (compare the input date to
today and yesterday) and return localized strings (use Intl.RelativeTimeFormat
or a manual comparison returning "Today"/"Yesterday" localized) so that the
external fun relativePrefixJs(year:Int, month:Int, day:Int) used by
getRelativePrefix(day, month, year) returns correct relative prefixes; ensure
the JS returns null/empty when the date is not today/yesterday and keep the
Kotlin call site (getRelativePrefix -> relativePrefixJs) unchanged.
🧹 Nitpick comments (4)
core/common/src/nativeMain/kotlin/org/mifos/mobile/core/common/LocalizedDateFormatter.kt (1)
29-54:getCurrentLocale()is called repeatedly — consider caching or passing as parameter.Every call to
formatFullDate,getRelativePrefix, andlocalizeDateDigitsinvokesgetCurrentLocale(), which readsNSUserDefaultsand constructs a newNSLocaleeach time. For a single formatting call, this results in multiple redundant locale constructions.core/common/src/desktopMain/kotlin/org/mifos/mobile/core/common/LocalizedDateFormatter.kt (2)
29-31: DesktopgetRelativePrefixalways returnsnull— no "Today"/"Yesterday" support.Android, JS, and Native all provide relative date strings. Desktop could easily do the same using
java.time.LocalDatecomparisons (similar to the Android implementation usingDateUtils). This creates an inconsistent UX across platforms.
19-47:localizeDateDigitsandformatFullDateare identical to the Android implementation.This is expected in KMP where
androidMainanddesktopMaincan't easily share JVM code, but worth noting if you later introduce ajvmMainshared source set.core/common/src/commonMain/kotlin/org/mifos/mobile/core/common/DateHelper.kt (1)
61-68: Note the parameter order swap when callinggetRelativePrefix.
dateis destructured as(year, month, day)butgetRelativePrefixis called as(day, month, year). This matches the (inconsistent) API signature, but is easy to get wrong during maintenance. See the earlier comment on theexpectdeclaration about standardizing parameter order.
Aryan-Baglane
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@core/common/src/commonMain/kotlin/org/mifos/mobile/core/common/DateHelper.kt`:
- Around line 61-69: The call to LocalizedDateFormatter.getRelativePrefix in
getFormattedDateWithPrefix is using the wrong argument order (day, month, year)
and must be changed to (year, month, day) to match the API; update the
invocation in the getFormattedDateWithPrefix function so it passes year, month,
day (same order as LocalizedDateFormatter.formatFullDate) to avoid
DateTimeException and incorrect results.
- Around line 176-185: The KDoc for getDateMonthYearString is incorrect about
the list order; update the comment to state the list is [year, month, day] (e.g.
[2025, 3, 8]) to match the destructuring val (year, month, day) = integersOfDate
and the behavior of getDateAsList / getDateAsLongFromList, and adjust the
descriptive sentence and example to reflect the actual order and output ("08
March 2025").
🧹 Nitpick comments (1)
core/common/src/wasmJsMain/kotlin/org/mifos/mobile/core/common/LocalizedDateFormatter.kt (1)
12-28:formatFullDateJsmonth validation is present, butnew Date(year, month - 1, day)silently rolls over invalid days.For example,
new Date(2026, 0, 32)becomes February 1, 2026 instead of erroring. This is consistent with JSDatebehavior, so it's a minor edge case — flagging for awareness rather than requiring a fix.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
feature/transfer-process/src/commonMain/kotlin/org/mifos/mobile/feature/transfer/process/transferProcess/TransferProcessScreen.kt (1)
231-257: Preview state is stale —uiTransferDateunset anddateFormatoutdated.The
@Previewdoesn't setuiTransferDate, so the date row renders"null". The payload also still carriesdateFormat = "dd MMMM yyyy", which no longer matches the format used at runtime ("dd MM yyyy").♻️ Proposed fix
TransferProcessScreen( state = TransferProcessState( transferType = TransferType.SELF, + uiTransferDate = "19 February 2026", transferPayload = TransferPayload( ... transferDate = "19 02 2026", - dateFormat = "dd MMMM yyyy", + dateFormat = "dd MM yyyy", locale = "en", ), ),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/transfer-process/src/commonMain/kotlin/org/mifos/mobile/feature/transfer/process/transferProcess/TransferProcessScreen.kt` around lines 231 - 257, The preview state is missing uiTransferDate and uses an outdated dateFormat; update TransferProcessScreenPreview to populate the state's uiTransferDate with a matching formatted string and change TransferPayload.dateFormat to the runtime format "dd MM yyyy". Specifically, when constructing TransferProcessState/TransferPayload in TransferProcessScreenPreview, set uiTransferDate (on TransferProcessState) to the formatted value for the preview (e.g., "27 09 2025") and update dateFormat in TransferPayload from "dd MMMM yyyy" to "dd MM yyyy" so the date row renders correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@feature/transfer-process/src/commonMain/kotlin/org/mifos/mobile/feature/transfer/process/transferProcess/TransferProcessScreen.kt`:
- Line 197: In TransferProcessScreen where the details map contains
Res.string.date to state.uiTransferDate.toString(), avoid calling toString() on
the nullable uiTransferDate; instead use a null-safe fallback (e.g.,
uiTransferDate.orEmpty() or uiTransferDate ?: "") so a null date renders as an
empty string or desired placeholder rather than the literal "null". Update the
mapping for Res.string.date to use that null-safe expression.
In
`@feature/transfer-process/src/commonMain/kotlin/org/mifos/mobile/feature/transfer/process/transferProcess/TransferProcessViewModel.kt`:
- Line 74: Remove the debug println in TransferProcessViewModel that prints
route.transferSuccessDestination; replace it by either removing the line
entirely or using the project's logging mechanism (e.g., processLogger or
Timber) if persistent logging is needed, ensuring no println statements remain
in production code.
- Around line 65-69: The backendDate computation is doing a UTC round-trip via
DateHelper.getDateAsLongFromList(...) (which anchors at UTC midnight) and then
DateHelper.getBackendDateFromLong(...), causing off-by-one days for UTC-negative
users; replace that sequence by taking currentDate (already local) and computing
its start-of-day in the system default timezone, e.g. get the epoch millis from
currentDate.atStartOfDayIn(TimeZone.currentSystemDefault()) and pass that millis
into DateHelper.getBackendDateFromLong instead of using getDateAsLongFromList
and the forced !! — update the TransferProcessViewModel backendDate assignment
to use the local start-of-day millis derived from currentDate so the date
remains correct for all timezones.
---
Duplicate comments:
In
`@core/common/src/commonMain/kotlin/org/mifos/mobile/core/common/DateHelper.kt`:
- Around line 132-139: getDateAsList currently uses a hardcoded pattern ("d MMMM
yyyy") with LocalDateTime.Format which will fail on JS/WASM/Native because
formatFullDate emits locale-native full-date strings; change the parser to use a
localized full-date formatter instead (e.g.,
DateTimeFormatter.ofLocalizedDate(FormatStyle.FULL).withLocale(Locale.getDefault())
or the Kotlin/Multiplatform equivalent) and parse into a LocalDate (e.g.,
LocalDate.from(parser.parse(date)) or LocalDate.parse(date, parser)) before
returning listOf(localDate.year, localDate.monthNumber, localDate.dayOfMonth) to
ensure portability across platforms and avoid formatter.parse(date) throwing.
- Around line 187-196: Standardize the list ordering to [year, month, day] to
remove the silent footgun: change getDateMonthYearString to expect and
destructure integersOfDate as (year, month, day) and pass them into
LocalizedDateFormatter.formatFullDate in that order (matching getDateAsString),
and update any callers (notably the TransferProcessViewModel call that builds
the date list) to provide the date as [year, month, day] instead of [day, month,
year].
---
Nitpick comments:
In
`@feature/transfer-process/src/commonMain/kotlin/org/mifos/mobile/feature/transfer/process/transferProcess/TransferProcessScreen.kt`:
- Around line 231-257: The preview state is missing uiTransferDate and uses an
outdated dateFormat; update TransferProcessScreenPreview to populate the state's
uiTransferDate with a matching formatted string and change
TransferPayload.dateFormat to the runtime format "dd MM yyyy". Specifically,
when constructing TransferProcessState/TransferPayload in
TransferProcessScreenPreview, set uiTransferDate (on TransferProcessState) to
the formatted value for the preview (e.g., "27 09 2025") and update dateFormat
in TransferPayload from "dd MMMM yyyy" to "dd MM yyyy" so the date row renders
correctly.
There was a problem hiding this comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@feature/transfer-process/src/commonMain/kotlin/org/mifos/mobile/feature/transfer/process/transferProcess/TransferProcessViewModel.kt`:
- Line 72: Remove the debug println in TransferProcessViewModel (the
println("TransferProcessViewModel: route = ${route.transferSuccessDestination}")
statement); delete that line (or replace it with a proper logger call if
persistent logging is required) so no debug prints remain before merging and
ensure any necessary logging uses the project's logger wrapper rather than
println.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
feature/transfer-process/src/commonMain/kotlin/org/mifos/mobile/feature/transfer/process/transferProcess/TransferProcessViewModel.kt (1)
75-75:⚠️ Potential issue | 🟡 MinorRemove debug
printlnbefore merging.- println("TransferProcessViewModel: route = ${route.transferSuccessDestination}")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/transfer-process/src/commonMain/kotlin/org/mifos/mobile/feature/transfer/process/transferProcess/TransferProcessViewModel.kt` at line 75, Remove the leftover debug println in TransferProcessViewModel (the line printing route.transferSuccessDestination); replace it with no-op or proper logging if needed (use existing logging facility instead of println) inside the TransferProcessViewModel initialization or method where it appears so no console debug remains before merge.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@feature/transfer-process/src/commonMain/kotlin/org/mifos/mobile/feature/transfer/process/transferProcess/TransferProcessViewModel.kt`:
- Line 75: Remove the leftover debug println in TransferProcessViewModel (the
line printing route.transferSuccessDestination); replace it with no-op or proper
logging if needed (use existing logging facility instead of println) inside the
TransferProcessViewModel initialization or method where it appears so no console
debug remains before merge.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
feature/transfer-process/src/commonMain/kotlin/org/mifos/mobile/feature/transfer/process/transferProcess/TransferProcessViewModel.kt
have you mistakenly mentioned some other ticket? |
|
@sahilshivekar no no i have metion the correct ticket but the dates were not localised so i have add localization to dates for whole app |
therajanmaurya
left a comment
therajanmaurya
left a comment
There was a problem hiding this comment.
Nice work on the multiplatform date localization! The LocalizedDateFormatter approach with expect/actual is clean and handles the platform differences well.
What I noticed:
- Good cleanup of
DateHelper.kt- removed ~370 lines of hardcoded English month names and duplicated formatting logic - Each platform uses native APIs for locale-aware formatting (Android
DateTimeFormatter, iOSNSDateFormatter, JSIntl) - The digit localization for RTL languages (Arabic, Hindi, etc.) is a nice touch
A couple of things:
-
Desktop
getRelativePrefixreturns null - see inline comment. This means desktop users won't see "Today" or "Yesterday" prefixes. -
Transfer date backend format - The change from
dd MMMM yyyytodd MM yyyyinTransferProcessViewModellooks intentional for API compatibility, but verify the backend accepts this format.
The refactoring simplifies the codebase significantly. The separation of UI date (uiTransferDate) from backend date (transferDate) in the ViewModel is a good pattern.
| .append(' ') | ||
| .append(integersOfDate[0]) | ||
| return stringBuilder.toString() | ||
| require(integersOfDate.size == 3) |
There was a problem hiding this comment.
Please add some message to be thrown when the condition fails. And this should be logged.
| else -> formatted | ||
| } | ||
| fun getFormattedDateWithPrefix(date: List<Int>): String { | ||
| require(date.size == 3) |
There was a problem hiding this comment.
This belongs to core module and I am seeing a lot of changes in this file. We need to make sure that this doesn't break the other screens wherever DateHelper is used.
Please upload screen recordings of other screens where DateHelper is used to ensure there are no regressions.
There was a problem hiding this comment.
ok sir but rajan sir also review this pr once. i will look into it if it doesnt break
|
Too many changes in the |
5 participants
Fixes - MM-514
fix - the payment calender screen is not displayed in the selected language
Screen.Recording.2026-02-11.at.11.17.19.PM.mov
Please make sure these boxes are checked before submitting your pull request - thanks!
Run the static analysis check
./gradlew checkorci-prepush.shto make sure you didn't break anythingIf you have multiple commits please combine them into one commit by squashing them.
Summary by CodeRabbit
New Features
Refactor