From 8d2e110cfd48e05a1cf1221bd5103e721bae2168 Mon Sep 17 00:00:00 2001 From: gabriel20xx Date: Sat, 25 Oct 2025 15:40:58 +0200 Subject: [PATCH] feat: Refactor DAO insert methods to use Upsert and add transaction support for field reordering --- .../com/collabtable/app/data/dao/FieldDao.kt | 17 ++- .../com/collabtable/app/data/dao/ItemDao.kt | 4 +- .../collabtable/app/data/dao/ItemValueDao.kt | 4 +- .../com/collabtable/app/data/dao/ListDao.kt | 4 +- .../app/ui/screens/ListDetailScreen.kt | 97 +++++++++++------- .../app/ui/screens/ListDetailViewModel.kt | 88 +++++++++------- CollabTableAndroid/build-output.txt | Bin 2436 -> 5698 bytes 7 files changed, 130 insertions(+), 84 deletions(-) diff --git a/CollabTableAndroid/app/src/main/java/com/collabtable/app/data/dao/FieldDao.kt b/CollabTableAndroid/app/src/main/java/com/collabtable/app/data/dao/FieldDao.kt index a469cdf..02b57cd 100644 --- a/CollabTableAndroid/app/src/main/java/com/collabtable/app/data/dao/FieldDao.kt +++ b/CollabTableAndroid/app/src/main/java/com/collabtable/app/data/dao/FieldDao.kt @@ -12,10 +12,10 @@ interface FieldDao { @Query("SELECT * FROM fields WHERE id = :fieldId") suspend fun getFieldById(fieldId: String): Field? - @Insert(onConflict = OnConflictStrategy.REPLACE) + @Upsert suspend fun insertField(field: Field) - @Insert(onConflict = OnConflictStrategy.REPLACE) + @Upsert suspend fun insertFields(fields: List) @Update @@ -30,4 +30,17 @@ interface FieldDao { // Get all fields updated since timestamp, including deleted ones for sync @Query("SELECT * FROM fields WHERE updatedAt >= :since") suspend fun getFieldsUpdatedSince(since: Long): List + + // Reorder fields in a single transaction for clarity and consistency + @Transaction + suspend fun reorderFieldsInTransaction(reorderedFields: List, timestamp: Long) { + reorderedFields.forEachIndexed { index, field -> + updateField( + field.copy( + order = index, + updatedAt = timestamp + ) + ) + } + } } diff --git a/CollabTableAndroid/app/src/main/java/com/collabtable/app/data/dao/ItemDao.kt b/CollabTableAndroid/app/src/main/java/com/collabtable/app/data/dao/ItemDao.kt index a9c7c9d..18408d2 100644 --- a/CollabTableAndroid/app/src/main/java/com/collabtable/app/data/dao/ItemDao.kt +++ b/CollabTableAndroid/app/src/main/java/com/collabtable/app/data/dao/ItemDao.kt @@ -17,10 +17,10 @@ interface ItemDao { @Query("SELECT * FROM items WHERE id = :itemId") suspend fun getItemById(itemId: String): Item? - @Insert(onConflict = OnConflictStrategy.REPLACE) + @Upsert suspend fun insertItem(item: Item) - @Insert(onConflict = OnConflictStrategy.REPLACE) + @Upsert suspend fun insertItems(items: List) @Update diff --git a/CollabTableAndroid/app/src/main/java/com/collabtable/app/data/dao/ItemValueDao.kt b/CollabTableAndroid/app/src/main/java/com/collabtable/app/data/dao/ItemValueDao.kt index e70aacb..20d084d 100644 --- a/CollabTableAndroid/app/src/main/java/com/collabtable/app/data/dao/ItemValueDao.kt +++ b/CollabTableAndroid/app/src/main/java/com/collabtable/app/data/dao/ItemValueDao.kt @@ -12,10 +12,10 @@ interface ItemValueDao { @Query("SELECT * FROM item_values WHERE id = :valueId") suspend fun getValueById(valueId: String): ItemValue? - @Insert(onConflict = OnConflictStrategy.REPLACE) + @Upsert suspend fun insertValue(value: ItemValue) - @Insert(onConflict = OnConflictStrategy.REPLACE) + @Upsert suspend fun insertValues(values: List) @Update diff --git a/CollabTableAndroid/app/src/main/java/com/collabtable/app/data/dao/ListDao.kt b/CollabTableAndroid/app/src/main/java/com/collabtable/app/data/dao/ListDao.kt index 463f22b..7501714 100644 --- a/CollabTableAndroid/app/src/main/java/com/collabtable/app/data/dao/ListDao.kt +++ b/CollabTableAndroid/app/src/main/java/com/collabtable/app/data/dao/ListDao.kt @@ -17,10 +17,10 @@ interface ListDao { @Query("SELECT * FROM lists WHERE id = :listId") suspend fun getListById(listId: String): CollabList? - @Insert(onConflict = OnConflictStrategy.REPLACE) + @Upsert suspend fun insertList(list: CollabList) - @Insert(onConflict = OnConflictStrategy.REPLACE) + @Upsert suspend fun insertLists(lists: List) @Update diff --git a/CollabTableAndroid/app/src/main/java/com/collabtable/app/ui/screens/ListDetailScreen.kt b/CollabTableAndroid/app/src/main/java/com/collabtable/app/ui/screens/ListDetailScreen.kt index 9b45ee0..2d5f31a 100644 --- a/CollabTableAndroid/app/src/main/java/com/collabtable/app/ui/screens/ListDetailScreen.kt +++ b/CollabTableAndroid/app/src/main/java/com/collabtable/app/ui/screens/ListDetailScreen.kt @@ -66,6 +66,10 @@ fun ListDetailScreen( val fields by viewModel.fields.collectAsState() val items by viewModel.items.collectAsState() + // Use derivedStateOf to create stable references + val stableFields by remember { derivedStateOf { fields } } + val stableItems by remember { derivedStateOf { items } } + var showManageColumnsDialog by remember { mutableStateOf(false) } var showAddItemDialog by remember { mutableStateOf(false) } var itemToEdit by remember { mutableStateOf(null) } @@ -88,8 +92,8 @@ fun ListDetailScreen( val fieldWidths = remember { mutableStateMapOf() } // Initialize field widths - LaunchedEffect(fields) { - fields.forEach { field -> + LaunchedEffect(stableFields) { + stableFields.forEach { field -> if (!fieldWidths.containsKey(field.id)) { fieldWidths[field.id] = 150.dp } @@ -97,8 +101,8 @@ fun ListDetailScreen( } // Apply filtering, sorting, and grouping - val processedItems = remember(items, filterField, filterValue, sortField, sortAscending, groupByField) { - var result = items + val processedItems = remember(stableItems, stableFields, filterField, filterValue, sortField, sortAscending, groupByField) { + var result = stableItems // Apply filter if (filterField != null && filterValue.isNotBlank()) { @@ -174,7 +178,7 @@ fun ListDetailScreen( } } ) { padding -> - if (fields.isEmpty()) { + if (stableFields.isEmpty()) { Box( modifier = Modifier .fillMaxSize() @@ -329,21 +333,23 @@ fun ListDetailScreen( .horizontalScroll(horizontalScrollState), verticalAlignment = Alignment.CenterVertically ) { - fields.forEach { field -> - FieldHeader( - field = field, - width = fieldWidths[field.id] ?: 150.dp, - onWidthChange = { delta -> - val currentWidth = fieldWidths[field.id] ?: 150.dp - val newWidth = (currentWidth.value + delta).coerceIn(100f, 400f) - fieldWidths[field.id] = newWidth.dp - } - ) + stableFields.forEach { field -> + key(field.id) { + FieldHeader( + field = field, + width = fieldWidths[field.id] ?: 150.dp, + onWidthChange = { delta -> + val currentWidth = fieldWidths[field.id] ?: 150.dp + val newWidth = (currentWidth.value + delta).coerceIn(100f, 400f) + fieldWidths[field.id] = newWidth.dp + } + ) + } } } // Items list with synchronized scrolling - if (items.isEmpty()) { + if (stableItems.isEmpty()) { Box( modifier = Modifier.fillMaxSize(), contentAlignment = Alignment.Center @@ -378,7 +384,7 @@ fun ListDetailScreen( LazyColumn( modifier = Modifier.fillMaxSize() ) { - groupedItems.forEach { (groupName, groupItems) -> + groupedItems.entries.forEach { (groupName, groupItems) -> // Show group header if grouping is enabled if (groupByField != null) { item(key = "group_$groupName") { @@ -397,9 +403,12 @@ fun ListDetailScreen( } // Show items in the group - items(groupItems, key = { it.item.id }) { itemWithValues -> + items( + items = groupItems, + key = { it.item.id } + ) { itemWithValues -> ItemRow( - fields = fields, + fields = stableFields, fieldWidths = fieldWidths, itemWithValues = itemWithValues, scrollState = horizontalScrollState, @@ -415,7 +424,7 @@ fun ListDetailScreen( if (showManageColumnsDialog) { ManageColumnsDialog( - fields = fields, + fields = stableFields, onDismiss = { showManageColumnsDialog = false }, onAddField = { name, fieldType, fieldOptions -> viewModel.addField(name, fieldType, fieldOptions) @@ -434,7 +443,7 @@ fun ListDetailScreen( if (showAddItemDialog) { AddItemDialog( - fields = fields, + fields = stableFields, onDismiss = { showAddItemDialog = false }, onAdd = { fieldValues -> viewModel.addItemWithValues(fieldValues) @@ -446,7 +455,7 @@ fun ListDetailScreen( // Sort Dialog if (showSortDialog) { SortDialog( - fields = fields, + fields = stableFields, currentSortField = sortField, currentSortAscending = sortAscending, onDismiss = { showSortDialog = false }, @@ -461,7 +470,7 @@ fun ListDetailScreen( // Group Dialog if (showGroupDialog) { GroupDialog( - fields = fields, + fields = stableFields, currentGroupByField = groupByField, onDismiss = { showGroupDialog = false }, onApply = { newGroupByField -> @@ -474,7 +483,7 @@ fun ListDetailScreen( // Filter Dialog if (showFilterDialog) { FilterDialog( - fields = fields, + fields = stableFields, currentFilterField = filterField, currentFilterValue = filterValue, onDismiss = { showFilterDialog = false }, @@ -502,7 +511,7 @@ fun ListDetailScreen( itemToEdit?.let { itemWithValues -> EditItemDialog( - fields = fields, + fields = stableFields, itemWithValues = itemWithValues, onDismiss = { itemToEdit = null }, onUpdate = { fieldValues -> @@ -586,6 +595,10 @@ fun ItemRow( scrollState: androidx.compose.foundation.ScrollState, onClick: () -> Unit ) { + // Map values by fieldId to ensure correct value-column alignment regardless of original order + val valuesByFieldId = remember(itemWithValues.values) { + itemWithValues.values.associateBy { it.fieldId } + } Row( modifier = Modifier .fillMaxWidth() @@ -593,19 +606,20 @@ fun ItemRow( .horizontalScroll(scrollState), verticalAlignment = Alignment.CenterVertically ) { - fields.forEachIndexed { _, field -> - val value = itemWithValues.values.find { it.fieldId == field.id } - val fieldWidth = fieldWidths[field.id] ?: 150.dp - - Box( - modifier = Modifier - .width(fieldWidth) - .border( - width = 1.dp, - color = MaterialTheme.colorScheme.outline - ) - .padding(8.dp) - ) { + fields.forEach { field -> + key(field.id) { + val value = valuesByFieldId[field.id] + val fieldWidth = fieldWidths[field.id] ?: 150.dp + + Box( + modifier = Modifier + .width(fieldWidth) + .border( + width = 1.dp, + color = MaterialTheme.colorScheme.outline + ) + .padding(8.dp) + ) { when (field.getType()) { com.collabtable.app.data.model.FieldType.TEXT -> { Text( @@ -1001,6 +1015,7 @@ fun ItemRow( } } } + } } @OptIn(ExperimentalMaterial3Api::class) @@ -2596,6 +2611,10 @@ fun EditItemDialog( ) { val fieldValues = remember { mutableStateMapOf() } var showDeleteConfirmation by remember { mutableStateOf(false) } + // Map values by field to avoid linear lookups and ensure consistent mapping when fields reorder + val valuesByFieldId = remember(itemWithValues.values) { + itemWithValues.values.associateBy { it.fieldId } + } // Initialize field values from existing item LaunchedEffect(itemWithValues) { @@ -2647,7 +2666,7 @@ fun EditItemDialog( verticalArrangement = Arrangement.spacedBy(12.dp) ) { items(fields, key = { it.id }) { field -> - val itemValue = itemWithValues.values.find { it.fieldId == field.id } + val itemValue = valuesByFieldId[field.id] if (itemValue != null) { FieldInput( field = field, diff --git a/CollabTableAndroid/app/src/main/java/com/collabtable/app/ui/screens/ListDetailViewModel.kt b/CollabTableAndroid/app/src/main/java/com/collabtable/app/ui/screens/ListDetailViewModel.kt index c07f2a7..c2f4a5e 100644 --- a/CollabTableAndroid/app/src/main/java/com/collabtable/app/ui/screens/ListDetailViewModel.kt +++ b/CollabTableAndroid/app/src/main/java/com/collabtable/app/ui/screens/ListDetailViewModel.kt @@ -3,12 +3,14 @@ package com.collabtable.app.ui.screens import android.content.Context import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope +import androidx.room.withTransaction import com.collabtable.app.data.database.CollabTableDatabase import com.collabtable.app.data.model.* import com.collabtable.app.data.repository.SyncRepository import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.flow.debounce import kotlinx.coroutines.launch import java.util.UUID @@ -35,7 +37,9 @@ class ListDetailViewModel( private fun loadListData() { viewModelScope.launch { - database.listDao().getListWithFields(listId).collect { listWithFields -> + database.listDao().getListWithFields(listId) + .debounce(75) + .collect { listWithFields -> _list.value = listWithFields?.list _fields.value = listWithFields?.fields ?: emptyList() } @@ -90,12 +94,11 @@ class ListDetailViewModel( createdAt = timestamp, updatedAt = timestamp ) - database.fieldDao().insertField(newField) // Create empty ItemValue entries for this new field for all existing items val existingItems = _items.value - if (existingItems.isNotEmpty()) { - val newValues = existingItems.map { itemWithValues -> + val newValues = if (existingItems.isNotEmpty()) { + existingItems.map { itemWithValues -> ItemValue( id = UUID.randomUUID().toString(), itemId = itemWithValues.item.id, @@ -104,7 +107,16 @@ class ListDetailViewModel( updatedAt = timestamp ) } - database.itemValueDao().insertValues(newValues) + } else { + emptyList() + } + + // Insert atomically to avoid intermediate inconsistent states + database.withTransaction { + database.fieldDao().insertField(newField) + if (newValues.isNotEmpty()) { + database.itemValueDao().insertValues(newValues) + } } performSync() @@ -143,19 +155,23 @@ class ListDetailViewModel( createdAt = timestamp, updatedAt = timestamp ) - database.itemDao().insertItem(newItem) - - // Create empty values for each field - val values = _fields.value.map { field -> - ItemValue( - id = UUID.randomUUID().toString(), - itemId = newItem.id, - fieldId = field.id, - value = "", - updatedAt = timestamp - ) + // Insert item and its values atomically + database.withTransaction { + database.itemDao().insertItem(newItem) + // Create empty values for each field + val values = _fields.value.map { field -> + ItemValue( + id = UUID.randomUUID().toString(), + itemId = newItem.id, + fieldId = field.id, + value = "", + updatedAt = timestamp + ) + } + if (values.isNotEmpty()) { + database.itemValueDao().insertValues(values) + } } - database.itemValueDao().insertValues(values) performSync() } } @@ -169,19 +185,23 @@ class ListDetailViewModel( createdAt = timestamp, updatedAt = timestamp ) - database.itemDao().insertItem(newItem) - - // Create values for each field with the provided values - val values = _fields.value.map { field -> - ItemValue( - id = UUID.randomUUID().toString(), - itemId = newItem.id, - fieldId = field.id, - value = fieldValues[field.id] ?: "", - updatedAt = timestamp - ) + // Insert item and provided values atomically + database.withTransaction { + database.itemDao().insertItem(newItem) + // Create values for each field with the provided values + val values = _fields.value.map { field -> + ItemValue( + id = UUID.randomUUID().toString(), + itemId = newItem.id, + fieldId = field.id, + value = fieldValues[field.id] ?: "", + updatedAt = timestamp + ) + } + if (values.isNotEmpty()) { + database.itemValueDao().insertValues(values) + } } - database.itemValueDao().insertValues(values) performSync() } } @@ -211,14 +231,8 @@ class ListDetailViewModel( fun reorderFields(reorderedFields: List) { viewModelScope.launch { val timestamp = System.currentTimeMillis() - reorderedFields.forEachIndexed { index, field -> - database.fieldDao().updateField( - field.copy( - order = index, - updatedAt = timestamp - ) - ) - } + // Use DAO-level transaction for clarity + database.fieldDao().reorderFieldsInTransaction(reorderedFields, timestamp) performSync() } } diff --git a/CollabTableAndroid/build-output.txt b/CollabTableAndroid/build-output.txt index 2b6f476b8b4b139738272b226da0577c9276aab3..81dfa0f17f0c158a2d4f8675aca6756b07c02ed3 100644 GIT binary patch literal 5698 zcmeI0TTfe85QXQtQvbt|s;aG2a05-7^3aNKiIS2O#UWLnBJc$W=GgKjz@JaLz8QA* z!5Bx!cHb(?=eqZr*=w%*eERsi752uCZDbd^+S1iPzb*ZT*0HBHv2*KLTknqS(AG5e zFCBMnTUUGbwS8st5q5=gB9zmmur7r%6h=pq6W%N7I+k`H?mT^|yRqK1{q3>ze&rfB zbv&}yw(|w8TXrUdme}~y`ac)p`CrFC_Es%MNrBcAQ)Zm6rF zn4QN`mX}wyEu5jmy4uItD$#^PTu^`?~9cU^<=Le}oswLAqsZRt!TbR_Y>W9&kRIX3G1DVYqG z;HQd&-o)^<{;QQxW#+#_-q#9;9&g$ERwr#0zV%*gf6)KX{U-SowWaE7^>v&m*8K_vH` zIM%8VGv6;fUxx%T2mRfmv64B+_|?uVq`+>yXc>7uIKR_jWC=~XQB68=-RonMhwy{w z5$RUw-^kRZ&RI%*`(4Se&)v5&%C^=pSV1>83;V9-WYyG-zP+_*bfb%SH+8q#-&Olz zI@z)k+KPDtfA-%q=9J zTp7JR)xWUs?GKIoL09|L)QG+ExKV(#N?Y>TTnS-&rLmws4Z?SJ#h~K zi$vZFc>{UxZ((Bpbl;K=b@!D--gRX^j~C{AH&44+)#90atFO4hXUXfO>j#Te>P0fw zLmhuwbM&+I*F8)7yO??ZwG28va?WQkTkQen@%zyJbKacUyFRMc#pgT~@tyiK_-o5L zp~eT;)NJ0b&t_0Q4ykL))Q61pQjv?=U6 zWz#j!%tJj7R@bCMU)rEGG0@W}^zG|7lGc5V5B(Xz9Jr?nw1vxz!e>|6O8J8tkR=gI z=mV3;qX!;JI+%e?){p9NqpRS6H(iaR-W7_^@NsIymFA%%9z>@W`{Q{MabQzpvVE`#bFnNIEPND}rosg)XF88HYUF)FLu+AwkCao5O5f?$UBaoV?GwFu zB>OaO?ZB5rd;ktoCC3k7=AC2*AMgAe9Ch`UF9_?h$5^`e>>JmvB@Ke5S@Z9+4{~C7 z+J|Bl6QQ#MqAN(>60=`DRjz)nsN5AwQ`bBC?dhs%`?@>O(~$63tBvoDoA!&I#&?zQ piqxZ^2OQJq(S`^l4&4I!Sl9j9(#J-s9Hx`b-A!jSxVt<(c?KBKtSJto zDzXJd+&A>{q5XMH9&_{v-)=hD&GdHoEVU>}EWR4Ar2b?vS2R&gA@$JcxhxvXiHyFq z)>yYsDaOTO1GG;?QWq#l6X$9v-IDu`sv(!WZ-sSCotef8%E(X9mudL(>@g}0-|ECn zj$msP3tX%E60~*g9mhtXIvyY_ZB!YWs%EukiZcE$>1)(-PR^(AK`U=j_z_V(mU`i$ zWJyhHYXrF#bdB;U{|f_JGXDj>GIr9+_oA@lYW>0ooLCA% z)0#x`mn)vF)#0g%z_FaY3>b5L(UJqMj5^qrt$eMSafnNKnDbV)h7k*7kJZemB>DlB zWJjb-7Fz1!@counu3c+T*GFw`i(V&hZS*^jAKPeJ?uBdVw?6#LXm`f**!dS;kRQ^Q z;jc)pMoz2vkWf2E`{lN}e^O?KDtpwvBripp{jRfw%Dl+Q+S~MNWc1&|@QF3QDEM4E zi~D?c4SN4%$6E*WVl18KZil70rCeH0n5-IhqhceDqj7q2dNLy3(6Bc-w2vgr*|!_s zT3C`LXdItN4Zf5PX5lG6hYA;552WXVu;Q(^!;;&xN%FpX(GBT;ng#^wkOs~=oh8~Z zn4QYx1gO**cmu99OU_T6Etd~6Z%{$Lrp2=b>mUcHb3X87E2$Yh4?&N>3<&1dRPe}( zgIQ{Dg13eZ%vh-7o9`kSc5SE(Uy754`<(MBAv3cyCmX1g-q)_*>*YqcZV^8ky2cj0 zciq<)J-&l28ktj%OIA$gp1&;UUTS7EM?u}vSL&4`(ce3Nq90YHjY3?asYv_y84~Gz zoPHjUAe#<{msxf-9S7N8U+~Ato43G^AxnYWIq>{(<@)dzXhovo#+}2P3ux+&H=VHj Hrzigavsf{=