Skip to content

Commit 1374082

Browse files
AndyScherzingerAndyScherzinger
committed
feat(conversations): make archiving optimistic and guard it like the other list actions
Archiving from the conversation list was the last list action without an optimistic write: it awaited the server call and a full room list sync before the row moved, and a concurrent sync whose response was computed before the archive reached the server could still flip the row back until the next refresh. Move the action into ConversationsListViewModel following the favorite pattern: the archived flag is written locally first (the reactive list moves the row instantly, no extra sync round-trip), the server call is retried once and reverted on failure, and the change is registered as a pending archived flag in ConversationListUpdater so stale server responses cannot revert it before the server confirms. Assisted-by: Claude Code:claude-fable-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
1 parent f51c90c commit 1374082

4 files changed

Lines changed: 127 additions & 26 deletions

File tree

‎app/src/main/java/com/nextcloud/talk/conversationlist/ConversationsListActivity.kt‎

Lines changed: 22 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -109,12 +109,10 @@ import com.nextcloud.talk.utils.bundle.BundleKeys.KEY_SHARED_TEXT
109109
import com.nextcloud.talk.utils.permissions.PlatformPermissionUtil
110110
import com.nextcloud.talk.utils.power.PowerManagerUtils
111111
import com.nextcloud.talk.utils.singletons.ApplicationWideCurrentRoomHolder
112-
import kotlinx.coroutines.Dispatchers
113112
import kotlinx.coroutines.flow.MutableStateFlow
114113
import kotlinx.coroutines.flow.collect
115114
import kotlinx.coroutines.flow.onEach
116115
import kotlinx.coroutines.launch
117-
import kotlinx.coroutines.withContext
118116
import org.greenrobot.eventbus.Subscribe
119117
import org.greenrobot.eventbus.ThreadMode
120118
import retrofit2.HttpException
@@ -559,6 +557,27 @@ class ConversationsListActivity : BaseActivity() {
559557
}
560558
}
561559
}
560+
561+
lifecycleScope.launch {
562+
conversationsListViewModel.archiveState.collect { state ->
563+
when (state) {
564+
is ConversationsListViewModel.ArchiveUiState.Success -> {
565+
val messageRes = if (state.archived) {
566+
R.string.archived_conversation
567+
} else {
568+
R.string.unarchived_conversation
569+
}
570+
showSnackbar(String.format(resources.getString(messageRes), state.conversationDisplayName))
571+
conversationsListViewModel.resetArchiveState()
572+
}
573+
is ConversationsListViewModel.ArchiveUiState.Error -> {
574+
showSnackbar(resources.getString(R.string.nc_common_error_sorry))
575+
conversationsListViewModel.resetArchiveState()
576+
}
577+
ConversationsListViewModel.ArchiveUiState.None -> { /* no-op */ }
578+
}
579+
}
580+
}
562581
}
563582

564583
private fun handleNoteToSelfShortcut(noteToSelfAvailable: Boolean, noteToSelfToken: String) {
@@ -1173,29 +1192,8 @@ class ConversationsListActivity : BaseActivity() {
11731192

11741193
@Suppress("Detekt.TooGenericExceptionCaught", "TooGenericExceptionCaught")
11751194
private fun handleArchiving(conversation: ConversationModel) {
1176-
val apiVersion = ApiUtils.getConversationApiVersion(currentUser!!, intArrayOf(ApiUtils.API_V4, ApiUtils.API_V1))
1177-
val url = ApiUtils.getUrlForArchive(apiVersion, currentUser?.baseUrl, conversation.token)
1178-
lifecycleScope.launch {
1179-
try {
1180-
if (conversation.hasArchived) {
1181-
withContext(Dispatchers.IO) { ncApiCoroutines.unarchiveConversation(credentials!!, url) }
1182-
fetchRooms()
1183-
showSnackbar(
1184-
String.format(resources.getString(R.string.unarchived_conversation), conversation.displayName)
1185-
)
1186-
} else {
1187-
withContext(Dispatchers.IO) { ncApiCoroutines.archiveConversation(credentials!!, url) }
1188-
fetchRooms()
1189-
showSnackbar(
1190-
String.format(resources.getString(R.string.archived_conversation), conversation.displayName)
1191-
)
1192-
}
1193-
} catch (e: Exception) {
1194-
showSnackbar(resources.getString(R.string.nc_common_error_sorry))
1195-
}
1196-
}
1195+
conversationsListViewModel.toggleConversationArchive(conversation)
11971196
}
1198-
11991197
private fun addConversationToFavorites(conversation: ConversationModel) {
12001198
conversationsListViewModel.addConversationToFavorites(conversation)
12011199
}

‎app/src/main/java/com/nextcloud/talk/conversationlist/data/network/ConversationListUpdater.kt‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,11 @@ class ConversationListUpdater @Inject constructor(
5757
*/
5858
private val pendingUnreadFlags = ConcurrentHashMap<String, Boolean>()
5959

60+
/**
61+
* Archived flags applied locally but not yet confirmed by a server response.
62+
*/
63+
private val pendingArchivedFlags = ConcurrentHashMap<String, Boolean>()
64+
6065
/**
6166
* Tag assignments applied locally but not yet confirmed by a server response.
6267
*/
@@ -91,6 +96,14 @@ class ConversationListUpdater @Inject constructor(
9196
pendingUnreadFlags.remove(internalConversationId)
9297
}
9398

99+
fun markPendingArchived(internalConversationId: String, archived: Boolean) {
100+
pendingArchivedFlags[internalConversationId] = archived
101+
}
102+
103+
fun clearPendingArchived(internalConversationId: String, archived: Boolean) {
104+
pendingArchivedFlags.remove(internalConversationId, archived)
105+
}
106+
94107
fun markPendingTags(internalConversationId: String, tagIds: List<String>) {
95108
pendingTagIds[internalConversationId] = tagIds
96109
}
@@ -148,6 +161,7 @@ class ConversationListUpdater @Inject constructor(
148161
var guarded = guardPendingReadMarker(serverItem, previous)
149162
guarded = guardPendingUnread(guarded, previous)
150163
guarded = guardPendingFavorite(guarded, previous)
164+
guarded = guardPendingArchived(guarded, previous)
151165
guarded = guardPendingTags(guarded, previous)
152166
guarded
153167
}
@@ -201,6 +215,17 @@ class ConversationListUpdater @Inject constructor(
201215
}
202216
}
203217

218+
private fun guardPendingArchived(serverItem: ConversationEntity, previous: ConversationEntity): ConversationEntity {
219+
val desired = pendingArchivedFlags[serverItem.internalId] ?: return serverItem
220+
221+
return if (serverItem.hasArchived == desired) {
222+
clearPendingArchived(serverItem.internalId, desired)
223+
serverItem
224+
} else {
225+
serverItem.copy(hasArchived = previous.hasArchived)
226+
}
227+
}
228+
204229
private fun guardPendingTags(serverItem: ConversationEntity, previous: ConversationEntity): ConversationEntity {
205230
val desired = pendingTagIds[serverItem.internalId] ?: return serverItem
206231

‎app/src/main/java/com/nextcloud/talk/conversationlist/viewmodels/ConversationsListViewModel.kt‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,15 @@ class ConversationsListViewModel @Inject constructor(
126126
private val _favoriteState = MutableStateFlow<FavoriteUiState>(FavoriteUiState.None)
127127
val favoriteState: StateFlow<FavoriteUiState> = _favoriteState.asStateFlow()
128128

129+
sealed class ArchiveUiState {
130+
data object None : ArchiveUiState()
131+
data class Success(val archived: Boolean, val conversationDisplayName: String) : ArchiveUiState()
132+
data object Error : ArchiveUiState()
133+
}
134+
135+
private val _archiveState = MutableStateFlow<ArchiveUiState>(ArchiveUiState.None)
136+
val archiveState: StateFlow<ArchiveUiState> = _archiveState.asStateFlow()
137+
129138
object GetRoomsStartState : ViewState
130139
class GetRoomsErrorState(val throwable: Throwable) : ViewState
131140
open class GetRoomsSuccessState(val listIsNotEmpty: Boolean) : ViewState
@@ -768,6 +777,43 @@ class ConversationsListViewModel @Inject constructor(
768777
_favoriteState.value = FavoriteUiState.None
769778
}
770779

780+
fun resetArchiveState() {
781+
_archiveState.value = ArchiveUiState.None
782+
}
783+
784+
@Suppress("Detekt.TooGenericExceptionCaught")
785+
fun toggleConversationArchive(conversation: ConversationModel) {
786+
val original = conversation.copy()
787+
val desiredArchived = !conversation.hasArchived
788+
val optimistic = conversation.copy(hasArchived = desiredArchived)
789+
val apiVersion = ApiUtils.getConversationApiVersion(currentUser, intArrayOf(ApiUtils.API_V4, ApiUtils.API_V1))
790+
val url = ApiUtils.getUrlForArchive(apiVersion, currentUser.baseUrl, conversation.token)
791+
viewModelScope.launch {
792+
conversationListUpdater.markPendingArchived(conversation.internalId, desiredArchived)
793+
withContext(Dispatchers.IO) {
794+
repository.updateConversation(optimistic)
795+
}
796+
try {
797+
withContext(Dispatchers.IO) {
798+
withRetry(1) {
799+
if (desiredArchived) {
800+
conversationsRepository.archiveConversation(credentials, url)
801+
} else {
802+
conversationsRepository.unarchiveConversation(credentials, url)
803+
}
804+
}
805+
}
806+
_archiveState.value = ArchiveUiState.Success(desiredArchived, conversation.displayName)
807+
} catch (e: Exception) {
808+
conversationListUpdater.clearPendingArchived(conversation.internalId, desiredArchived)
809+
withContext(Dispatchers.IO) {
810+
repository.updateConversation(original)
811+
}
812+
_archiveState.value = ArchiveUiState.Error
813+
}
814+
}
815+
}
816+
771817
@Suppress("Detekt.TooGenericExceptionCaught")
772818
fun addConversationToFavorites(conversation: ConversationModel) {
773819
val original = conversation.copy()

‎app/src/test/java/com/nextcloud/talk/conversationlist/data/network/ConversationListFreshnessIntegrationTest.kt‎

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,32 @@ class ConversationListFreshnessIntegrationTest {
289289
}
290290
}
291291

292+
@Test
293+
fun `a sync cannot revert a pending archive until the server confirms it`() {
294+
val user = user(withKeepNotificationsCapability = false)
295+
val repository = repository()
296+
whenever(conversationsNetwork.getRooms(any(), any(), any())).thenReturn(
297+
Observable.just(listOf(staleServerRoom(lastReadMessage = 10, unreadMessages = 0))),
298+
Observable.just(listOf(staleServerRoom(lastReadMessage = 10, unreadMessages = 0, hasArchived = true))),
299+
Observable.just(listOf(staleServerRoom(lastReadMessage = 10, unreadMessages = 0)))
300+
)
301+
302+
runBlocking {
303+
seedConversation(lastActivity = 12, lastReadMessage = 10, unreadMessages = 0)
304+
conversationListUpdater.markPendingArchived(INTERNAL_CONVERSATION_ID, archived = true)
305+
db.conversationsDao().updateConversation(conversationEntity().copy(hasArchived = true))
306+
307+
repository.getRooms(user).join()
308+
assertTrue("stale sync must not revert the archive", conversationEntity().hasArchived)
309+
310+
repository.getRooms(user).join()
311+
assertTrue("confirming sync must keep the archive", conversationEntity().hasArchived)
312+
313+
repository.getRooms(user).join()
314+
assertFalse("server authority must be restored", conversationEntity().hasArchived)
315+
}
316+
}
317+
292318
@Test
293319
fun `room list flow reflects database writes reactively`() {
294320
val user = user(withKeepNotificationsCapability = false)
@@ -332,13 +358,19 @@ class ConversationListFreshnessIntegrationTest {
332358
ApplicationProvider.getApplicationContext()
333359
)
334360

335-
private fun staleServerRoom(lastReadMessage: Int, unreadMessages: Int, favorite: Boolean = false): Conversation =
361+
private fun staleServerRoom(
362+
lastReadMessage: Int,
363+
unreadMessages: Int,
364+
favorite: Boolean = false,
365+
hasArchived: Boolean = false
366+
): Conversation =
336367
Conversation(
337368
token = ROOM_TOKEN,
338369
lastActivity = 12,
339370
lastReadMessage = lastReadMessage,
340371
unreadMessages = unreadMessages,
341-
favorite = favorite
372+
favorite = favorite,
373+
hasArchived = hasArchived
342374
)
343375

344376
private suspend fun seedConversation(lastActivity: Long, lastReadMessage: Int, unreadMessages: Int) {

0 commit comments

Comments
 (0)