feat: enhance sync functionality with network backoff handling and improve item sorting in ListDetailScreen

2025-11-08 20:34:19 +01:00
parent d7a2befc8b
commit 0bc3b3b06f
3 changed files with 114 additions and 46 deletions
@@ -8,6 +8,7 @@ import com.collabtable.app.data.database.CollabTableDatabase
 import com.collabtable.app.data.preferences.PreferencesManager
 import com.collabtable.app.utils.Logger
 import kotlinx.coroutines.Dispatchers
+import java.net.UnknownHostException
 import kotlinx.coroutines.sync.Mutex
 import kotlinx.coroutines.sync.withLock
 import kotlinx.coroutines.withContext
@@ -18,13 +19,19 @@ class SyncRepository(context: Context) {
    private val api = ApiClient.api
    private val prefs = appContext.getSharedPreferences("collab_table_prefs", Context.MODE_PRIVATE)

-        @Volatile private var authBackoffUntil: Long = 0L
-        @Volatile private var authBackoffMs: Long = 0L
+    @Volatile private var authBackoffUntil: Long = 0L
+    @Volatile private var authBackoffMs: Long = 0L
+    @Volatile private var networkBackoffUntil: Long = 0L
+    @Volatile private var networkBackoffMs: Long = 0L

        fun resetAuthBackoff() {
            authBackoffMs = 0L
            authBackoffUntil = 0L
        }
+        private fun resetNetworkBackoff() {
+            networkBackoffMs = 0L
+            networkBackoffUntil = 0L
+        }
    // We keep separate watermarks to avoid clock-skew issues:
    // - server watermark: server-assigned timestamp for fetching server changes
    // - local watermark: device local time for selecting local changes to upload
@@ -73,6 +80,10 @@ class SyncRepository(context: Context) {
                if (authBackoffUntil > now) {
                    return@withContext Result.success(Unit)
                }
+                // Respect network backoff for transient DNS failures / unreachable host
+                if (networkBackoffUntil > now) {
+                    return@withContext Result.success(Unit)
+                }
                val lastServerTs = getLastServerSyncTs()
                val lastLocalTs = getLastLocalSyncTs()
                // Capture a snapshot timestamp BEFORE reading local changes to avoid missing
@@ -84,6 +95,16 @@ class SyncRepository(context: Context) {
                    Logger.i("Sync", "[SYNC] Starting initial sync with server")
                }

+                // If our local DB appears empty but we have a nonzero server watermark (e.g., after reinstall
+                // or clearing local data), request a one-time full down-sync by resetting the effective
+                // lastSyncTimestamp to 0 for this request. This avoids permanently missing historical rows.
+                val hasAnyLists = database.listDao().getAllListIds().isNotEmpty()
+                val hasAnyItems = database.itemDao().getAllItemIds().isNotEmpty()
+                val effectiveLastServerTs = if (lastServerTs > 0 && (!hasAnyLists || !hasAnyItems)) 0L else lastServerTs
+                if (effectiveLastServerTs == 0L && lastServerTs > 0L) {
+                    Logger.w("Sync", "Local store empty but watermark set; performing one-time full down-sync")
+                }
+
                // Gather local changes since last sync
                val localLists = database.listDao().getListsUpdatedSince(lastLocalTs)
                    .filter { it.updatedAt in (lastLocalTs + 1)..localSnapshotTs }
@@ -108,7 +129,7 @@ class SyncRepository(context: Context) {
                // Send to server and get updates
                val syncRequest =
                    SyncRequest(
-                        lastSyncTimestamp = lastServerTs,
+                        lastSyncTimestamp = effectiveLastServerTs,
                        lists = localLists,
                        fields = localFields,
                        items = localItems,
@@ -116,7 +137,23 @@ class SyncRepository(context: Context) {
                    )

                // HTTP-only sync
-                val response = api.sync(syncRequest)
+                val response = try {
+                    api.sync(syncRequest)
+                } catch (e: UnknownHostException) {
+                    // Host could not be resolved; apply exponential backoff and surface a warning once.
+                    networkBackoffMs = if (networkBackoffMs == 0L) 5_000L else (networkBackoffMs * 2).coerceAtMost(180_000L)
+                    networkBackoffUntil = System.currentTimeMillis() + networkBackoffMs
+                    Logger.w("Sync", "Network host unresolved (DNS). Backing off for ${networkBackoffMs/1000}s: ${e.message}")
+                    return@withContext Result.failure(Exception("Network unreachable: unable to resolve host"))
+                } catch (e: Exception) {
+                    if (e.message?.contains("Unable to resolve host", ignoreCase = true) == true) {
+                        networkBackoffMs = if (networkBackoffMs == 0L) 5_000L else (networkBackoffMs * 2).coerceAtMost(180_000L)
+                        networkBackoffUntil = System.currentTimeMillis() + networkBackoffMs
+                        Logger.w("Sync", "DNS failure. Backing off for ${networkBackoffMs/1000}s: ${e.message}")
+                        return@withContext Result.failure(Exception("Network unreachable: unable to resolve host"))
+                    }
+                    throw e
+                }
                if (!response.isSuccessful) {
                    if (response.code() == 401) {
                        // Unauthorized: likely bad/missing password. Reset sync baseline and surface a clear error.
@@ -218,6 +255,7 @@ class SyncRepository(context: Context) {

                // Successful sync clears any previous auth backoff
                resetAuthBackoff()
+                resetNetworkBackoff()

                return@withContext Result.success(Unit)
            } catch (e: Exception) {
@@ -226,6 +264,7 @@ class SyncRepository(context: Context) {
                    setLastServerSyncTs(0)
                    setLastLocalSyncTs(0)
                }
+                // Do not reset network backoff here; it's managed where thrown.
                return@withContext Result.failure(e)
            }
            }
@@ -27,6 +27,7 @@ import androidx.compose.foundation.layout.size
 import androidx.compose.foundation.layout.width
 import androidx.compose.foundation.layout.wrapContentHeight
 import androidx.compose.foundation.lazy.LazyColumn
+import androidx.compose.foundation.lazy.rememberLazyListState
 import androidx.compose.foundation.lazy.items
 import androidx.compose.foundation.lazy.itemsIndexed
 import androidx.compose.foundation.rememberScrollState
@@ -126,6 +127,7 @@ fun ListDetailScreen(

    var showManageColumnsDialog by remember { mutableStateOf(false) }
    var showAddItemDialog by remember { mutableStateOf(false) }
+    var pendingScrollToBottom by remember { mutableStateOf(false) }
    var itemToEdit by remember { mutableStateOf<ItemWithValues?>(null) }
    var showSortDialog by remember { mutableStateOf(false) }
    var showGroupDialog by remember { mutableStateOf(false) }
@@ -218,12 +220,15 @@ fun ListDetailScreen(
                stableItems
            }

+        // Default ordering: append new items at bottom by createdAt ascending.
+        // If an explicit sortField is chosen, use that instead.
        val sortedItems: List<ItemWithValues> =
            if (sortField != null) {
                val base = filteredItems.sortedWith(compareBy { item -> valueFor(item, sortField) })
                if (sortAscending) base else base.asReversed()
            } else {
-                filteredItems
+                // Items from DB may not be ordered; enforce createdAt ascending
+                filteredItems.sortedBy { it.item.createdAt }
            }

        val processedItems: List<ItemWithValues> = sortedItems
@@ -270,6 +275,7 @@ fun ListDetailScreen(
                    modifier =
                        Modifier
                            .fillMaxWidth()
+                            .horizontalScroll(rememberScrollState())
                            .background(MaterialTheme.colorScheme.surfaceVariant)
                            .padding(horizontal = 8.dp, vertical = 4.dp),
                    horizontalArrangement = Arrangement.spacedBy(8.dp),
@@ -508,8 +514,23 @@ fun ListDetailScreen(
                        }
                    }
                } else {
+                    val listState = rememberLazyListState()
+                    // If a new item was just added, scroll to bottom after composition
+                    LaunchedEffect(processedItems.size, pendingScrollToBottom) {
+                        if (pendingScrollToBottom) {
+                            // totalItemsCount includes the trailing filler; scroll to the last real item index
+                            val lastIndex = (listState.layoutInfo.totalItemsCount - 2).coerceAtLeast(0)
+                            try {
+                                listState.animateScrollToItem(lastIndex)
+                            } catch (_: Exception) {
+                                listState.scrollToItem(lastIndex)
+                            }
+                            pendingScrollToBottom = false
+                        }
+                    }
                    LazyColumn(
                        modifier = Modifier.fillMaxSize(),
+                        state = listState,
                    ) {
                        groupedItems.entries.forEach { (groupName, groupItems) ->
                            // Show group header if grouping is enabled
@@ -597,6 +618,7 @@ fun ListDetailScreen(
            onAdd = { fieldValues ->
                viewModel.addItemWithValues(fieldValues)
                showAddItemDialog = false
+                pendingScrollToBottom = true
            },
        )
    }
@@ -318,52 +318,59 @@ fun ListItem(
    onDeleteClick: () -> Unit,
    dragHandle: (@Composable () -> Unit)? = null,
 ) {
-    Row(
-        modifier =
-            Modifier
-                .fillMaxWidth()
-                .padding(horizontal = 12.dp, vertical = 8.dp),
-        horizontalArrangement = Arrangement.SpaceBetween,
-        verticalAlignment = Alignment.CenterVertically,
+    // Expand the touch target to include the vertical spacing between rows without visually changing layout.
+    // We wrap the original row in a full-width Box with the same outer padding, then add a clickable
+    // modifier that uses additional vertical padding but negative padding inside to keep visual spacing.
+    Box(
+        modifier = Modifier
+            .fillMaxWidth()
+            .padding(horizontal = 12.dp) // keep original horizontal spacing
+            .clickable(onClick = onListClick) // entire row (including margins) is tappable
+            .padding(vertical = 4.dp) // expand touch area (original visual was 8.dp inside Row)
    ) {
-        // Drag handle on the far left
-        if (dragHandle != null) {
-            dragHandle()
-            Spacer(modifier = Modifier.width(8.dp))
-        }
-
-        // Table name centered/left and clickable
-        Column(
-            modifier =
-                Modifier
-                    .weight(1f)
-                    .clickable(onClick = onListClick),
+        Row(
+            modifier = Modifier
+                .fillMaxWidth()
+                .padding(vertical = 4.dp), // actual visual spacing remains ~8dp total (4 + 4)
+            horizontalArrangement = Arrangement.SpaceBetween,
+            verticalAlignment = Alignment.CenterVertically,
        ) {
-            Text(text = list.name, style = MaterialTheme.typography.titleMedium)
-            Spacer(modifier = Modifier.height(2.dp))
-            val subtitle = remember(list.updatedAt, nowMs) { formatUpdatedAgo(list.updatedAt, nowMs) }
-            Text(
-                text = subtitle,
-                style = MaterialTheme.typography.bodySmall,
-                color = MaterialTheme.colorScheme.onSurfaceVariant,
-            )
-        }
+            // Drag handle on the far left
+            if (dragHandle != null) {
+                dragHandle()
+                Spacer(modifier = Modifier.width(8.dp))
+            }

-        // Actions on the right
-        Row(verticalAlignment = Alignment.CenterVertically) {
-            IconButton(onClick = onEditClick) {
-                Icon(
-                    Icons.Default.Edit,
-                    contentDescription = "Edit",
-                    tint = MaterialTheme.colorScheme.primary,
+            // Table name and subtitle
+            Column(
+                modifier = Modifier.weight(1f),
+            ) {
+                Text(text = list.name, style = MaterialTheme.typography.titleMedium)
+                Spacer(modifier = Modifier.height(2.dp))
+                val subtitle = remember(list.updatedAt, nowMs) { formatUpdatedAgo(list.updatedAt, nowMs) }
+                Text(
+                    text = subtitle,
+                    style = MaterialTheme.typography.bodySmall,
+                    color = MaterialTheme.colorScheme.onSurfaceVariant,
                )
            }
-            IconButton(onClick = onDeleteClick) {
-                Icon(
-                    Icons.Default.Delete,
-                    contentDescription = "Delete",
-                    tint = MaterialTheme.colorScheme.error,
-                )
+
+            // Actions on the right (retain individual click targets)
+            Row(verticalAlignment = Alignment.CenterVertically) {
+                IconButton(onClick = onEditClick) {
+                    Icon(
+                        Icons.Default.Edit,
+                        contentDescription = "Edit",
+                        tint = MaterialTheme.colorScheme.primary,
+                    )
+                }
+                IconButton(onClick = onDeleteClick) {
+                    Icon(
+                        Icons.Default.Delete,
+                        contentDescription = "Delete",
+                        tint = MaterialTheme.colorScheme.error,
+                    )
+                }
            }
        }
    }