Skip to content

Refactor rates data layer and update dependencies#170

Open
mdrlzy wants to merge 12 commits into
mainfrom
refactor/misc-improvements
Open

Refactor rates data layer and update dependencies#170
mdrlzy wants to merge 12 commits into
mainfrom
refactor/misc-improvements

Conversation

@mdrlzy

@mdrlzy mdrlzy commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

📌 Description

  • Added a forced rates refresh on the splash screen so the Quick screen opens with up to date rates
  • Added listeners for rates update flow on screens to refresh data when rates change
  • Added throttling for remote rates updated_at checks to avoid unnecessary API checks happening many times per minute
  • Added thousands separators for amount input fields
  • Replaced Retrofit with Ktor because Ktor is Kotlin-first and better fits future support for multiple API variants, such as premium subscription API
  • Updated Kotlin, Gradle, and project dependencies

@mdrlzy
mdrlzy force-pushed the refactor/misc-improvements branch from eb53965 to 208e90c Compare July 10, 2026 03:54
@mdrlzy
mdrlzy force-pushed the refactor/misc-improvements branch from 208e90c to cce4c46 Compare July 10, 2026 04:30
@mdrlzy
mdrlzy force-pushed the refactor/misc-improvements branch from cce4c46 to 758dadf Compare July 10, 2026 21:45
@mdrlzy
mdrlzy force-pushed the refactor/misc-improvements branch from 758dadf to 0325588 Compare July 11, 2026 03:20
@mdrlzy
mdrlzy force-pushed the refactor/misc-improvements branch from 0325588 to 640aa0d Compare July 11, 2026 14:02

@shubertm shubertm left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@kirillt, @mdrlzy Most of currency icons I had have gone missing after updating to this branch. Is it a design choice or something is not working on my side?

Code review findings from Codex:
1.
High: core/data/src/main/java/dev/arkbuilders/rate/core/data/repo/currency/CurrencyRepoImpl.kt:99 RatesUpdatedAtCheck is persisted before the crypto/fiat fetches and DB write complete. If either remote fetch fails, updateRates() returns failure, but subsequent calls are skipped for 10 minutes because the throttle check at CurrencyRepoImpl.kt:80 now short-circuits. A transient network or endpoint failure therefore causes a forced stale-data window.
2.
Medium: core/data/src/main/java/dev/arkbuilders/rate/core/data/repo/currency/CurrencyRepoImpl.kt:99 The same throttle also applies after a “remote data not newer” result at CurrencyRepoImpl.kt:101. If upstream publishes new rates shortly after that check, the app will not pick them up until the 10-minute interval expires. This is more visible because splash now waits on updateRates() during startup in app/src/main/java/dev/arkbuilders/rate/presentation/splash/SplashViewModel.kt:37 and SplashViewModel.kt:59.

@mdrlzy

mdrlzy commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator Author

@shubertm

Most of currency icons I had have gone missing after updating to this branch. Is it a design choice or something is not working on my side?

This is by design. Crypto icons are now dynamic and updated for each apk based on the top N currencies returned by the API. Here’s the command for populating the icons.

@mdrlzy

mdrlzy commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator Author

High: core/data/src/main/java/dev/arkbuilders/rate/core/data/repo/currency/CurrencyRepoImpl.kt:99 RatesUpdatedAtCheck is persisted before the crypto/fiat fetches and DB write complete. If either remote fetch fails, updateRates() returns failure, but subsequent calls are skipped for 10 minutes because the throttle check at CurrencyRepoImpl.kt:80 now short-circuits. A transient network or endpoint failure therefore causes a forced stale-data window.
2.
Medium: core/data/src/main/java/dev/arkbuilders/rate/core/data/repo/currency/CurrencyRepoImpl.kt:99 The same throttle also applies after a “remote data not newer” result at CurrencyRepoImpl.kt:101. If upstream publishes new rates shortly after that check, the app will not pick them up until the 10-minute interval expires. This is more visible because splash now waits on updateRates() during startup in app/src/main/java/dev/arkbuilders/rate/presentation/splash/SplashViewModel.kt:37 and SplashViewModel.kt:59.

I don’t see this as an issue. Rates are updated every 8 hours, so a potential 10-minute delay is negligible. The interval also prevents excessive retries when a request fails. I’d keep it as is

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