Skip to content

Commit d9c5297

Browse files
ohassineohassine
authored
feat: Suggested tag list does not update while typing (WPB-21037) (#4409)
1 parent 615a400 commit d9c5297

File tree

3 files changed

+62
-60
lines changed

3 files changed

+62
-60
lines changed

‎features/cells/src/main/java/com/wire/android/feature/cells/ui/tags/AddRemoveTagsScreen.kt‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -94,17 +94,17 @@ fun AddRemoveTagsScreen(
9494
},
9595
bottomBar = {
9696
val isLoading = addRemoveTagsViewModel.isLoading.collectAsState().value
97-
val tags = addRemoveTagsViewModel.suggestedTags.collectAsState()
97+
val tags = addRemoveTagsViewModel.suggestedTags
9898
Column(
9999
modifier = Modifier.background(colorsScheme().background)
100100
) {
101-
if (tags.value.isNotEmpty()) {
101+
if (tags.isNotEmpty()) {
102102
LazyRow(
103103
modifier = Modifier
104104
.fillMaxWidth()
105105
.padding(start = dimensions().spacing16x, end = dimensions().spacing16x)
106106
) {
107-
tags.value.forEach { tag ->
107+
tags.forEach { tag ->
108108
item {
109109
WireFilterChip(
110110
modifier = Modifier.padding(

‎features/cells/src/main/java/com/wire/android/feature/cells/ui/tags/AddRemoveTagsViewModel.kt‎

Lines changed: 41 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@ package com.wire.android.feature.cells.ui.tags
1919

2020
import androidx.compose.foundation.text.input.TextFieldState
2121
import androidx.compose.foundation.text.input.clearText
22+
import androidx.compose.runtime.getValue
23+
import androidx.compose.runtime.mutableStateOf
24+
import androidx.compose.runtime.setValue
2225
import androidx.compose.runtime.snapshotFlow
2326
import androidx.lifecycle.SavedStateHandle
2427
import androidx.lifecycle.viewModelScope
@@ -27,16 +30,13 @@ import com.wire.android.ui.common.ActionsViewModel
2730
import com.wire.kalium.cells.domain.usecase.GetAllTagsUseCase
2831
import com.wire.kalium.cells.domain.usecase.RemoveNodeTagsUseCase
2932
import com.wire.kalium.cells.domain.usecase.UpdateNodeTagsUseCase
33+
import com.wire.kalium.common.functional.getOrElse
3034
import com.wire.kalium.common.functional.onFailure
3135
import com.wire.kalium.common.functional.onSuccess
3236
import dagger.hilt.android.lifecycle.HiltViewModel
3337
import kotlinx.coroutines.flow.MutableStateFlow
34-
import kotlinx.coroutines.flow.SharingStarted
35-
import kotlinx.coroutines.flow.asStateFlow
3638
import kotlinx.coroutines.flow.collectLatest
37-
import kotlinx.coroutines.flow.combine
3839
import kotlinx.coroutines.flow.debounce
39-
import kotlinx.coroutines.flow.stateIn
4040
import kotlinx.coroutines.flow.update
4141
import kotlinx.coroutines.launch
4242
import javax.inject.Inject
@@ -55,70 +55,77 @@ class AddRemoveTagsViewModel @Inject constructor(
5555

5656
val tagsTextState = TextFieldState()
5757

58-
private val allTags: MutableStateFlow<Set<String>> = MutableStateFlow(emptySet())
59-
6058
val initialTags: Set<String> = navArgs.tags.toSet()
61-
private val _addedTags: MutableStateFlow<Set<String>> = MutableStateFlow(navArgs.tags.toSet())
62-
internal val addedTags = _addedTags.asStateFlow()
6359

6460
val disallowedChars = listOf(",", ";", "/", "\\", "\"", "\'", "<", ">")
6561

66-
private val _suggestedTags = MutableStateFlow<Set<String>>(emptySet())
67-
internal val suggestedTags =
68-
allTags.combine(addedTags) { all, added ->
69-
all.filter { it !in added }.toSet().sorted()
70-
}.stateIn(viewModelScope, SharingStarted.WhileSubscribed(), emptySet())
62+
var allTags: Set<String> = emptySet()
63+
private set
64+
65+
val addedTags: MutableStateFlow<Set<String>> = MutableStateFlow(navArgs.tags.toSet())
66+
67+
var suggestedTags by mutableStateOf<Set<String>>(emptySet())
68+
private set
7169

7270
init {
7371
viewModelScope.launch {
74-
getAllTagsUseCase().onSuccess { tags ->
75-
allTags.update { tags }
76-
}
77-
launch {
78-
snapshotFlow { tagsTextState.text.toString() }
79-
.debounce(TYPING_DEBOUNCE_TIME)
80-
.collectLatest { query ->
81-
val filtered = if (query.isBlank()) {
82-
allTags.value
83-
} else {
84-
allTags.value.filter { it.contains(query, ignoreCase = true) }.toSet()
85-
}
86-
_suggestedTags.value = filtered
87-
}
88-
}
72+
allTags = getAllTagsUseCase().getOrElse { emptySet() }
73+
updateSuggestions("") // initial state
74+
}
75+
76+
viewModelScope.launch {
77+
snapshotFlow { tagsTextState.text.toString() }
78+
.debounce(TYPING_DEBOUNCE_TIME)
79+
.collectLatest {
80+
onQueryChanged(it)
81+
}
8982
}
9083
}
9184

85+
fun onQueryChanged(query: String) {
86+
updateSuggestions(query)
87+
}
88+
89+
private fun updateSuggestions(query: String) {
90+
suggestedTags = allTags
91+
.asSequence()
92+
.filter { query.isBlank() || it.contains(query, ignoreCase = true) }
93+
.filter { it !in addedTags.value }
94+
.toSet()
95+
}
96+
9297
fun isValidTag(): Boolean = disallowedChars.none {
9398
it in tagsTextState.text
9499
} && tagsTextState.text.length in ALLOWED_LENGTH
95100

96101
fun addTag(tag: String) {
97102
tag.trim().let { newTag ->
98-
if (newTag.isNotBlank() && newTag !in _addedTags.value) {
99-
_addedTags.update { it + newTag }
103+
if (newTag.isNotBlank() && newTag !in addedTags.value) {
104+
addedTags.update { it + newTag }
105+
updateSuggestions("")
100106
tagsTextState.clearText()
101107
}
102108
}
103109
}
104110

105111
fun removeTag(tag: String) {
106-
_addedTags.update { it - tag }
112+
addedTags.update { it - tag }
113+
updateSuggestions("")
107114
}
108115

109116
fun removeLastTag() {
110-
_addedTags.value.lastOrNull()?.let { lastTag ->
117+
addedTags.value.lastOrNull()?.let { lastTag ->
111118
removeTag(lastTag)
112119
}
113120
}
114121

115122
fun updateTags() {
116123
viewModelScope.launch {
117124
isLoading.value = true
118-
val result = if (_addedTags.value.isEmpty()) {
125+
val result = if (addedTags.value.isEmpty()) {
119126
removeNodeTagsUseCase(navArgs.uuid)
120127
} else {
121-
updateNodeTagsUseCase(navArgs.uuid, _addedTags.value)
128+
updateNodeTagsUseCase(navArgs.uuid, addedTags.value)
122129
}
123130

124131
result

‎features/cells/src/test/kotlin/com/wire/android/feature/cells/ui/tags/AddRemoveTagsViewModelTest.kt‎

Lines changed: 18 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ import io.mockk.every
3333
import io.mockk.impl.annotations.MockK
3434
import kotlinx.coroutines.Dispatchers
3535
import kotlinx.coroutines.flow.first
36-
import kotlinx.coroutines.test.UnconfinedTestDispatcher
36+
import kotlinx.coroutines.test.StandardTestDispatcher
3737
import kotlinx.coroutines.test.advanceUntilIdle
3838
import kotlinx.coroutines.test.resetMain
3939
import kotlinx.coroutines.test.runTest
@@ -47,7 +47,7 @@ import org.junit.jupiter.api.Test
4747

4848
class AddRemoveTagsViewModelTest {
4949

50-
private val dispatcher = UnconfinedTestDispatcher()
50+
private val dispatcher = StandardTestDispatcher()
5151

5252
@BeforeEach
5353
fun beforeEach() {
@@ -60,7 +60,7 @@ class AddRemoveTagsViewModelTest {
6060
}
6161

6262
@Test
63-
fun `given a new valid tag, when addTag is called, then tag is added and suggestions updated`() = runTest {
63+
fun `given a new valid tag, when addTag is called, then tag is added`() = runTest {
6464
val (_, viewModel) = Arrangement()
6565
.withGetAllTagsUseCaseReturning(Either.Right(setOf()))
6666
.arrange()
@@ -70,11 +70,6 @@ class AddRemoveTagsViewModelTest {
7070

7171
val addedTags = viewModel.addedTags.first()
7272
assertTrue(addedTags.contains(newTag))
73-
74-
val suggestedTags = viewModel.suggestedTags.first()
75-
assertFalse(suggestedTags.contains(newTag))
76-
77-
assertEquals("", viewModel.tagsTextState.text)
7873
}
7974

8075
@Test
@@ -113,11 +108,10 @@ class AddRemoveTagsViewModelTest {
113108
.withGetAllTagsUseCaseReturning(Either.Right(setOf(tagInSuggestions)))
114109
.arrange()
115110

116-
viewModel.suggestedTags.test {
117-
assertTrue(expectMostRecentItem().contains(tagInSuggestions))
118-
viewModel.addTag(tagInSuggestions)
119-
assertFalse(expectMostRecentItem().contains(tagInSuggestions))
120-
}
111+
advanceUntilIdle()
112+
assertTrue(viewModel.suggestedTags.contains(tagInSuggestions))
113+
viewModel.addTag(tagInSuggestions)
114+
assertFalse(viewModel.suggestedTags.contains(tagInSuggestions))
121115
}
122116

123117
@Test
@@ -299,15 +293,6 @@ class AddRemoveTagsViewModelTest {
299293
every { savedStateHandle.get<ArrayList<String>>("tags") } returns ArrayList()
300294
}
301295

302-
private val viewModel by lazy {
303-
AddRemoveTagsViewModel(
304-
savedStateHandle = savedStateHandle,
305-
getAllTagsUseCase = getAllTagsUseCase,
306-
updateNodeTagsUseCase = updateNodeTagsUseCase,
307-
removeNodeTagsUseCase = removeNodeTagsUseCase,
308-
)
309-
}
310-
311296
fun withGetAllTagsUseCaseReturning(result: Either<CoreFailure, Set<String>>) = apply {
312297
coEvery { getAllTagsUseCase() } returns result
313298
}
@@ -320,6 +305,16 @@ class AddRemoveTagsViewModelTest {
320305
coEvery { removeNodeTagsUseCase(any()) } returns result
321306
}
322307

323-
fun arrange() = this to viewModel
308+
fun arrange(): Pair<Arrangement, AddRemoveTagsViewModel> {
309+
// Create a new ViewModel instance every time arrange() is called.
310+
// This prevents state from leaking between tests.
311+
val viewModel = AddRemoveTagsViewModel(
312+
savedStateHandle = savedStateHandle,
313+
getAllTagsUseCase = getAllTagsUseCase,
314+
updateNodeTagsUseCase = updateNodeTagsUseCase,
315+
removeNodeTagsUseCase = removeNodeTagsUseCase,
316+
)
317+
return this to viewModel
318+
}
324319
}
325320
}

0 commit comments

Comments
 (0)