fix: wire up 'Check for Updates' to fetch tile version by torlando-tech · Pull Request #602 · torlando-tech/columba

torlando-tech · 2026-03-06T01:13:47Z

Summary

Closes #596

ViewModel: checkForUpdates() was a stub — now calls TileDownloadManager.fetchCurrentTileVersion() to compare the region's stored version against OpenFreeMap's latest TileJSON
UI: Added inline error state with warning icon and "Retry" button when the version check fails (network error, server unreachable)
Tests: Replaced 3 stub tests with 6 ViewModel tests (update available, up to date, server unreachable, exception, no stored version, multi-region) and added 2 UI tests (error display, retry callback)

User-visible behavior after fix

Scenario	Before	After
Tap "Check for Updates"	Nothing happens	Shows "Checking..." spinner (~1-2s)
Map is up to date	Nothing	Shows ✓ "Up to date"
Update available	Nothing	Shows "Update available" + "Update Now"
Network error	Nothing	Shows ⚠ error message + "Retry" button

Test plan

All 97 existing + new tests pass (OfflineMapsViewModelTest + OfflineMapsScreenTest)
Manual: tap "Check for Updates" on a downloaded region with internet → see "Checking..." then "Up to date" or "Update available"
Manual: tap "Check for Updates" with airplane mode → see error message with retry button

🤖 Generated with Claude Code

The button was a stub that set isChecking=true then immediately isChecking=false with no network call, so users saw no feedback. Now calls TileDownloadManager.fetchCurrentTileVersion() to compare the region's stored version against OpenFreeMap's latest TileJSON. Adds inline error state with retry button when the check fails. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

app/src/main/java/com/lxmf/messenger/viewmodel/OfflineMapsViewModel.kt

greptile-apps · 2026-03-06T01:18:40Z

Greptile Summary

This PR successfully implements the "Check for Updates" feature end-to-end: OfflineMapsViewModel.checkForUpdates() now calls TileDownloadManager.fetchCurrentTileVersion() to fetch the latest tile version from the server, the UI displays inline error/retry states on network failures, and "Update Now" navigates into the download wizard pre-filled with the existing region's parameters.

Key improvements:

Real network call to check for updates in OfflineMapsViewModel.checkForUpdates(), with null returns treated as server-unreachable errors
New initForUpdate() wizard pre-fill in OfflineMapDownloadViewModel with tile-version snapshot taken before download to avoid TOCTOU races
UI adds warning icon and "Retry" button for error states; update-check section hidden for regions without tileVersion
Defensive try/catch blocks around deleteOldRegion() to handle partial cleanup failures gracefully
6 new ViewModel tests covering update-available, up-to-date, network error, exception, and no-stored-version scenarios; 2 new UI tests for error display and retry callbacks
97 tests pass (existing + new)

Confidence Score: 5/5

Safe to merge. All core feature logic is correct, well-tested, and defensive error handling is in place.
The PR successfully wires up the "Check for Updates" feature with proper network error handling, defensive state updates, and TOCTOU-race prevention via pre-download tile-version snapshots. All 97 tests pass. File I/O is correctly placed on the IO dispatcher. Error states are clearly communicated to the user. The feature is complete and ready.
No files require special attention.

_{Last reviewed commit: 3183263}

app/src/main/java/com/lxmf/messenger/viewmodel/OfflineMapsViewModel.kt

greptile-apps · 2026-03-06T01:18:45Z

app/src/test/java/com/lxmf/messenger/viewmodel/OfflineMapsViewModelTest.kt

+    @Test
+    fun `checkForUpdates for region without tileVersion shows up to date`() =
+        runTest {
+            mockkObject(TileDownloadManager)
+            try {
+                coEvery { TileDownloadManager.fetchCurrentTileVersion() } returns "20260305_001001_pt"
+
+                // Region has no stored tileVersion (e.g., imported MBTiles)
+                val testRegion = createTestRegion(TestRegionConfig(id = 42L, tileVersion = null))
+                viewModel = createViewModel()
+
+                viewModel.state.test {
+                    awaitItem() // Initial state
+
+                    viewModel.checkForUpdates(testRegion)
+
+                    val state = expectMostRecentItem()
+                    val result = state.updateCheckResults[42L]
+                    assertNotNull(result)
+                    // latestVersion is set, currentVersion is null → hasUpdate = false
+                    assertNull(result!!.currentVersion)
+                    assertEquals("20260305_001001_pt", result.latestVersion)
+                    assertFalse(result.hasUpdate)
+                    assertNull(result.error)
+
+                    cancelAndConsumeRemainingEvents()
+                }
+            } finally {
+                unmockkObject(TileDownloadManager)
            }
        }


"Up to date" silently shown when version comparison is impossible

When a region has no stored tileVersion (e.g., downloaded before version tracking was introduced, or imported as MBTiles), currentVersion == null, so hasUpdate is always false per the property definition:

val hasUpdate: Boolean get() = latestVersion != null && currentVersion != null && latestVersion != currentVersion

The test correctly captures this with the comment // latestVersion is set, currentVersion is null → hasUpdate = false, and it asserts assertFalse(result.hasUpdate) with no error. This means the UI will display a green checkmark and "Up to date" for any region where version tracking was never recorded — even if the region is months old.

Consider a distinct UI state for this case (e.g., "Version unknown – re-download to track updates") so users are not misled into thinking an untracked map is current. This would require a small addition to UpdateCheckResult, for example:

data class UpdateCheckResult( // ... val canCompareVersions: Boolean = currentVersion != null, ) { val hasUpdate: Boolean get() = canCompareVersions && latestVersion != null && latestVersion != currentVersion }

And a new branch in the when block in OfflineMapsScreen.kt for !canCompareVersions && latestVersion != null.

Prompt To Fix With AI

This is a comment left during a code review. Path: app/src/test/java/com/lxmf/messenger/viewmodel/OfflineMapsViewModelTest.kt Line: 1017-1047 Comment: **"Up to date" silently shown when version comparison is impossible** When a region has no stored `tileVersion` (e.g., downloaded before version tracking was introduced, or imported as MBTiles), `currentVersion == null`, so `hasUpdate` is always `false` per the property definition: ```kotlin val hasUpdate: Boolean get() = latestVersion != null && currentVersion != null && latestVersion != currentVersion ``` The test correctly captures this with the comment `// latestVersion is set, currentVersion is null → hasUpdate = false`, and it asserts `assertFalse(result.hasUpdate)` with no error. This means the UI will display a green checkmark and **"Up to date"** for any region where version tracking was never recorded — even if the region is months old. Consider a distinct UI state for this case (e.g., `"Version unknown – re-download to track updates"`) so users are not misled into thinking an untracked map is current. This would require a small addition to `UpdateCheckResult`, for example: ```kotlin data class UpdateCheckResult( // ... val canCompareVersions: Boolean = currentVersion != null, ) { val hasUpdate: Boolean get() = canCompareVersions && latestVersion != null && latestVersion != currentVersion } ``` And a new branch in the `when` block in `OfflineMapsScreen.kt` for `!canCompareVersions && latestVersion != null`. How can I resolve this? If you propose a fix, please make it concise.

sentry · 2026-03-06T01:34:40Z

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Tests that call checkForUpdates() must mock fetchCurrentTileVersion() to prevent real IO dispatch that leaks across test boundaries. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

- Gate update check UI on region.tileVersion != null to avoid misleading "Check for Updates" button on imported/RMSP regions without versioning - Add defensive comment explaining why the catch block in checkForUpdates is intentional (guards against future contract changes) - Add test for regions without tileVersion not showing update button Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

torlando-tech · 2026-03-06T04:48:41Z

@greptileai

The Update Now button previously just re-triggered checkForUpdates(). Now it navigates to the download wizard pre-filled with the existing region's parameters (center, radius, zoom, name), skipping straight to the CONFIRM step. After the new download completes, the old region (MapLibre data, MBTiles file, cached style, DB record) is cleaned up. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

markCompleteWithMaplibreId() never set tileVersion, so all regions downloaded via MapLibre's OfflineManager had tileVersion=null. The UI gates "Check for Updates" on tileVersion != null, making the button invisible for every downloaded region. Now calls TileDownloadManager.fetchCurrentTileVersion() right after marking complete and persists it via updateTileVersion(). Failure is non-fatal — the region is still usable, just without update checking. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

The Next/Back/Download buttons at the bottom of the Location, Radius, and Confirm steps were hidden behind the system navigation bar. Added navigationBarsPadding() to all three step columns. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

The parent Scaffold in MainActivity consumes navigation bar insets but discards its paddingValues, so child Scaffolds see them as already consumed. Use raw WindowInsets.navigationBars to get the actual nav bar height and apply it at the Box level in OfflineMapDownloadScreen. Also move "offline_map_download" from exact-match to prefix-match in hideBottomNav so the parameterized update route also hides the bottom navigation bar. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

torlando-tech · 2026-03-07T00:55:01Z

@greptileai

app/src/main/java/com/lxmf/messenger/viewmodel/OfflineMapDownloadViewModel.kt

- Await async MapLibre region deletion via suspendCancellableCoroutine before deleting the DB record, preventing orphaned offline data - Wrap file I/O (MBTiles + cached style deletion) in Dispatchers.IO to avoid blocking the main thread - Guard against null e.message in checkForUpdates error path Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

torlando-tech · 2026-03-07T01:29:51Z

@greptileai

app/src/main/java/com/lxmf/messenger/viewmodel/OfflineMapDownloadViewModel.kt

app/src/main/java/com/lxmf/messenger/ui/screens/offlinemaps/OfflineMapsScreen.kt

- Snapshot tile version before download starts to avoid TOCTOU race where a server-side version rollout during download would record the wrong version string - Always delete DB record in deleteOldRegion even if MapLibre/file cleanup fails, preventing duplicate region entries visible to users - Snap to nearest RadiusOption instead of defaulting to MEDIUM when the stored radiusKm doesn't match an enum value exactly - Reorder when-branches in OfflineMapsScreen so error takes priority over "Up to date", preventing a future invariant violation from silently swallowing error messages Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

torlando-tech · 2026-03-07T02:57:03Z

@greptileai

app/src/main/java/com/lxmf/messenger/viewmodel/OfflineMapDownloadViewModel.kt

- Wrap deleteRegion() in deleteOldRegion() in its own try/catch so a Room exception doesn't propagate to the onComplete handler and leave the download wizard stuck on an error screen - When the pre-download tile version snapshot is null, retry the fetch in onComplete as a fallback so the region doesn't permanently lose update-checking capability Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

torlando-tech · 2026-03-07T03:07:14Z

@greptileai

sentry bot reviewed Mar 6, 2026

View reviewed changes

app/src/main/java/com/lxmf/messenger/viewmodel/OfflineMapsViewModel.kt Show resolved Hide resolved

greptile-apps bot reviewed Mar 6, 2026

View reviewed changes

torlando-tech and others added 2 commits March 5, 2026 21:48

fix: mock TileDownloadManager in clearUpdateCheckResult tests

181a9f4

Tests that call checkForUpdates() must mock fetchCurrentTileVersion() to prevent real IO dispatch that leaks across test boundaries. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

torlando-tech and others added 4 commits March 6, 2026 19:19

sentry bot reviewed Mar 7, 2026

View reviewed changes

app/src/main/java/com/lxmf/messenger/viewmodel/OfflineMapDownloadViewModel.kt Show resolved Hide resolved

greptile-apps bot reviewed Mar 7, 2026

View reviewed changes

app/src/main/java/com/lxmf/messenger/viewmodel/OfflineMapDownloadViewModel.kt Show resolved Hide resolved

greptile-apps bot reviewed Mar 7, 2026

View reviewed changes

app/src/main/java/com/lxmf/messenger/viewmodel/OfflineMapDownloadViewModel.kt Outdated Show resolved Hide resolved

app/src/main/java/com/lxmf/messenger/viewmodel/OfflineMapDownloadViewModel.kt Outdated Show resolved Hide resolved

torlando-tech merged commit 87aacbb into main Mar 7, 2026
14 checks passed

torlando-tech deleted the fix/596-check-for-updates-feedback branch March 7, 2026 03:46

Uh oh!

Conversation

torlando-tech commented Mar 6, 2026

Summary

User-visible behavior after fix

Test plan

Uh oh!

Uh oh!

greptile-apps bot commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Uh oh!

Uh oh!

greptile-apps bot Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

sentry bot commented Mar 6, 2026

Codecov Report

Uh oh!

torlando-tech commented Mar 6, 2026

Uh oh!

torlando-tech commented Mar 7, 2026

Uh oh!

Uh oh!

Uh oh!

torlando-tech commented Mar 7, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

torlando-tech commented Mar 7, 2026

Uh oh!

Uh oh!

Uh oh!

torlando-tech commented Mar 7, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

greptile-apps bot commented Mar 6, 2026 •

edited

Loading