Skip to content

Commit d345de9

Browse files
gpuntogpunto
authored
Upsert new reactions in a sorted position in ActivityReactionList (#132)
* Remove unneeded tree update functions * Upsert new reactions in a sorted position in ActivityReactionList
1 parent c38ae51 commit d345de9

File tree

7 files changed

+75
-459
lines changed

7 files changed

+75
-459
lines changed

‎stream-feeds-android-client/src/main/kotlin/io/getstream/feeds/android/client/internal/model/ThreadedCommentOperations.kt‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,3 +200,38 @@ private fun ThreadedCommentData.upsertReply(
200200
}
201201
return copy(replies = updatedReplies, replyCount = updatedCount)
202202
}
203+
204+
/**
205+
* Removes a comment with the given [commentId] from the list.
206+
*
207+
* @param commentId The ID of the comment to remove.
208+
* @return A new list of [ThreadedCommentData] with the specified comment removed.
209+
*/
210+
internal fun List<ThreadedCommentData>.removeComment(commentId: String): List<ThreadedCommentData> {
211+
val indexToRemove = indexOfFirst { it.id == commentId }
212+
return if (indexToRemove >= 0) {
213+
// A top-level comment was removed, update the state
214+
toMutableList().apply { removeAt(indexToRemove) }
215+
} else {
216+
// It might be a nested reply, search and remove recursively
217+
map { parent -> parent.removeReply(commentId) }
218+
}
219+
}
220+
221+
private fun ThreadedCommentData.removeReply(commentIdToRemove: String): ThreadedCommentData {
222+
// If this comment has no replies, nothing to remove
223+
if (replies.isNullOrEmpty()) {
224+
return this
225+
}
226+
// Check if the comment to remove is a direct reply
227+
val indexToRemove = replies.indexOfFirst { it.id == commentIdToRemove }
228+
if (indexToRemove >= 0) {
229+
// Found and removed a direct child, update reply count
230+
return copy(
231+
replies = replies.toMutableList().apply { removeAt(indexToRemove) },
232+
replyCount = replyCount - 1,
233+
)
234+
}
235+
// If not found, recursively check each reply
236+
return copy(replies = replies.map { child -> child.removeReply(commentIdToRemove) })
237+
}

‎stream-feeds-android-client/src/main/kotlin/io/getstream/feeds/android/client/internal/state/ActivityCommentListStateImpl.kt‎

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,13 @@ import io.getstream.feeds.android.client.api.model.ThreadedCommentData
2222
import io.getstream.feeds.android.client.api.state.ActivityCommentListState
2323
import io.getstream.feeds.android.client.api.state.query.ActivityCommentsQuery
2424
import io.getstream.feeds.android.client.internal.model.PaginationResult
25+
import io.getstream.feeds.android.client.internal.model.removeComment
2526
import io.getstream.feeds.android.client.internal.model.removeReaction
2627
import io.getstream.feeds.android.client.internal.model.update
2728
import io.getstream.feeds.android.client.internal.model.upsertNestedReply
2829
import io.getstream.feeds.android.client.internal.model.upsertReaction
2930
import io.getstream.feeds.android.client.internal.state.query.toComparator
3031
import io.getstream.feeds.android.client.internal.utils.mergeSorted
31-
import io.getstream.feeds.android.client.internal.utils.treeRemoveFirst
3232
import io.getstream.feeds.android.client.internal.utils.upsertSorted
3333
import kotlinx.coroutines.flow.MutableStateFlow
3434
import kotlinx.coroutines.flow.StateFlow
@@ -92,15 +92,7 @@ internal class ActivityCommentListStateImpl(
9292
}
9393

9494
override fun onCommentRemoved(commentId: String) {
95-
_comments.update { current ->
96-
current.treeRemoveFirst(
97-
matcher = { it.id == commentId },
98-
childrenSelector = { it.replies.orEmpty() },
99-
updateChildren = { parent, children ->
100-
parent.copy(replies = children, replyCount = parent.replyCount - 1)
101-
},
102-
)
103-
}
95+
_comments.update { current -> current.removeComment(commentId) }
10496
}
10597

10698
override fun onCommentReactionUpserted(

‎stream-feeds-android-client/src/main/kotlin/io/getstream/feeds/android/client/internal/state/ActivityReactionListStateImpl.kt‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import io.getstream.feeds.android.client.api.state.query.ActivityReactionsSort
2323
import io.getstream.feeds.android.client.internal.model.PaginationResult
2424
import io.getstream.feeds.android.client.internal.state.query.ActivityReactionsQueryConfig
2525
import io.getstream.feeds.android.client.internal.utils.mergeSorted
26-
import io.getstream.feeds.android.client.internal.utils.upsert
26+
import io.getstream.feeds.android.client.internal.utils.upsertSorted
2727
import kotlinx.coroutines.flow.MutableStateFlow
2828
import kotlinx.coroutines.flow.StateFlow
2929
import kotlinx.coroutines.flow.asStateFlow
@@ -76,7 +76,9 @@ internal class ActivityReactionListStateImpl(override val query: ActivityReactio
7676
}
7777

7878
override fun onReactionUpserted(reaction: FeedsReactionData) {
79-
_reactions.update { current -> current.upsert(reaction, FeedsReactionData::id) }
79+
_reactions.update { current ->
80+
current.upsertSorted(reaction, FeedsReactionData::id, reactionsSorting)
81+
}
8082
}
8183

8284
override fun onReactionRemoved(reaction: FeedsReactionData) {

‎stream-feeds-android-client/src/main/kotlin/io/getstream/feeds/android/client/internal/state/CommentReplyListStateImpl.kt‎

Lines changed: 2 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,8 @@ import io.getstream.feeds.android.client.api.model.ThreadedCommentData
2222
import io.getstream.feeds.android.client.api.state.CommentReplyListState
2323
import io.getstream.feeds.android.client.api.state.query.CommentRepliesQuery
2424
import io.getstream.feeds.android.client.internal.model.PaginationResult
25+
import io.getstream.feeds.android.client.internal.model.removeComment
2526
import io.getstream.feeds.android.client.internal.model.removeReaction
26-
import io.getstream.feeds.android.client.internal.model.update
2727
import io.getstream.feeds.android.client.internal.model.upsertNestedReply
2828
import io.getstream.feeds.android.client.internal.model.upsertReaction
2929
import io.getstream.feeds.android.client.internal.state.query.toComparator
@@ -76,16 +76,7 @@ internal class CommentReplyListStateImpl(
7676
}
7777

7878
private fun onReplyRemoved(commentId: String) {
79-
_replies.update { current ->
80-
val filteredTopLevel = current.filter { it.id != commentId }
81-
if (filteredTopLevel.size != current.size) {
82-
// A top-level comment was removed, update the state
83-
filteredTopLevel
84-
} else {
85-
// It might be a nested reply, search and remove recursively
86-
current.map { parent -> removeNestedReply(parent, commentId) }
87-
}
88-
}
79+
_replies.update { current -> current.removeComment(commentId) }
8980
}
9081

9182
override fun onCommentUpserted(comment: CommentData) {
@@ -114,26 +105,6 @@ internal class CommentReplyListStateImpl(
114105
}
115106
}
116107

117-
private fun removeNestedReply(
118-
comment: ThreadedCommentData,
119-
commentIdToRemove: String,
120-
): ThreadedCommentData {
121-
// If this comment has no replies, nothing to remove
122-
if (comment.replies.isNullOrEmpty()) {
123-
return comment
124-
}
125-
// Check if the comment to remove is a direct reply
126-
val filteredReplies = comment.replies.filter { it.id != commentIdToRemove }
127-
if (filteredReplies.size != comment.replies.size) {
128-
// Found and removed a direct child, update reply count
129-
return comment.copy(replies = filteredReplies, replyCount = comment.replyCount - 1)
130-
}
131-
// If not found, recursively check each reply
132-
return comment.copy(
133-
replies = comment.replies.map { child -> removeNestedReply(child, commentIdToRemove) }
134-
)
135-
}
136-
137108
private fun addNestedReplyReaction(
138109
parent: ThreadedCommentData,
139110
comment: CommentData,

‎stream-feeds-android-client/src/main/kotlin/io/getstream/feeds/android/client/internal/utils/List.kt‎

Lines changed: 0 additions & 119 deletions
Original file line numberDiff line numberDiff line change
@@ -311,122 +311,3 @@ internal fun <T> List<T>.mergeSorted(
311311
idSelector: (T) -> String,
312312
sort: List<Sort<T>>,
313313
): List<T> = mergeSorted(other, idSelector, CompositeComparator(sort))
314-
315-
/**
316-
* Updates an existing element in a tree-like list, and optionally sorts the result.
317-
*
318-
* This function traverses the tree, finds the first element for which [matcher] returns true and
319-
* replaces it with the updated element provided by the [updateElement] function. If no matching
320-
* element is found, the list remains unchanged. If a [comparator] is provided, the updated list
321-
* will be sorted according to it.
322-
*
323-
* @param matcher A function that determines whether an element should be updated.
324-
* @param childrenSelector A function that extracts the children of an element. This is used to
325-
* recursively update nested elements.
326-
* @param updateElement A function that takes the existing element and returns the updated element.
327-
* @param updateChildren A function that takes the existing element and the updated children, and
328-
* returns the updated element with the new children.
329-
* @param comparator The comparator used to sort the list after the update.
330-
* @return A list containing the updated element. If an existing element was found, it will be
331-
* replaced and repositioned; otherwise, the list remains unchanged.
332-
*/
333-
internal fun <T> List<T>.treeUpdateFirst(
334-
matcher: (T) -> Boolean,
335-
childrenSelector: (T) -> List<T>,
336-
updateElement: (T) -> T,
337-
updateChildren: (T, List<T>) -> T,
338-
comparator: Comparator<in T>? = null,
339-
): List<T> {
340-
341-
return internalTreeUpdate(matcher, childrenSelector, updateElement, updateChildren, comparator)
342-
?: this
343-
}
344-
345-
/**
346-
* Removes the first element matching the [matcher] from a tree-like list.
347-
*
348-
* This function traverses the tree, finds the first element for which [matcher] returns true and
349-
* removes it from the list.
350-
*
351-
* @param matcher A function that determines whether an element should be removed.
352-
* @param childrenSelector A function that extracts the children of an element. This is used to
353-
* recursively remove nested elements.
354-
* @param updateChildren A function that takes the existing element and the updated children, and
355-
* returns the updated element with the new children.
356-
* @return A list with the first matching element removed. If no matching element was found, the
357-
* original list is returned.
358-
*/
359-
internal fun <T> List<T>.treeRemoveFirst(
360-
matcher: (T) -> Boolean,
361-
childrenSelector: (T) -> List<T>,
362-
updateChildren: (T, List<T>) -> T,
363-
): List<T> {
364-
return internalTreeUpdate(matcher, childrenSelector, null, updateChildren, null) ?: this
365-
}
366-
367-
/**
368-
* Internal helper to update an element in a tree-like list.
369-
*
370-
* @return A new list with the updated element, or null if no update was made.
371-
*/
372-
private fun <T> List<T>.internalTreeUpdate(
373-
matcher: (T) -> Boolean,
374-
childrenSelector: (T) -> List<T>,
375-
updateElement: ((T) -> T)?,
376-
updateChildren: (T, List<T>) -> T,
377-
comparator: Comparator<in T>?,
378-
): List<T>? {
379-
if (isEmpty()) {
380-
return null
381-
}
382-
383-
// 1. Check for a match at the current level
384-
val index = indexOfFirst(matcher)
385-
if (index >= 0) {
386-
return toMutableList().apply {
387-
if (updateElement == null) {
388-
removeAt(index)
389-
} else {
390-
this[index] = updateElement(this[index])
391-
comparator?.let(::sortWith)
392-
}
393-
}
394-
}
395-
396-
// 2. If no match, recurse into children
397-
var wasUpdated = false
398-
val resultList =
399-
buildList(this.size) {
400-
for (item in this@internalTreeUpdate) {
401-
// If a sibling was updated, add the remaining items as-is
402-
if (wasUpdated) {
403-
add(item)
404-
continue
405-
}
406-
407-
val newChildren =
408-
childrenSelector(item)
409-
.internalTreeUpdate(
410-
matcher,
411-
childrenSelector,
412-
updateElement,
413-
updateChildren,
414-
comparator,
415-
)
416-
417-
// If the result is not null, it means the children were updated
418-
if (newChildren != null) {
419-
wasUpdated = true
420-
add(updateChildren(item, newChildren))
421-
} else {
422-
add(item)
423-
}
424-
}
425-
}
426-
427-
return if (wasUpdated) {
428-
comparator?.let(resultList::sortedWith) ?: resultList
429-
} else {
430-
null
431-
}
432-
}

‎stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/ActivityReactionListStateImplTest.kt‎

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import io.getstream.feeds.android.client.api.state.query.ActivityReactionsSort
2121
import io.getstream.feeds.android.client.internal.state.query.ActivityReactionsQueryConfig
2222
import io.getstream.feeds.android.client.internal.test.TestData.defaultPaginationResult
2323
import io.getstream.feeds.android.client.internal.test.TestData.feedsReactionData
24+
import java.util.Date
2425
import kotlinx.coroutines.test.runTest
2526
import org.junit.Assert.assertEquals
2627
import org.junit.Assert.assertNull
@@ -39,7 +40,10 @@ internal class ActivityReactionListStateImplTest {
3940
@Test
4041
fun `on queryMoreActivityReactions, then update reactions and pagination`() = runTest {
4142
val reactions =
42-
listOf(feedsReactionData(), feedsReactionData("reaction-2", "activity-1", "user-2"))
43+
listOf(
44+
feedsReactionData(activityId = "activity-1", userId = "user-1"),
45+
feedsReactionData(activityId = "activity-1", userId = "user-2"),
46+
)
4347
val paginationResult = defaultPaginationResult(reactions)
4448

4549
activityReactionListState.onQueryMoreActivityReactions(paginationResult, queryConfig)
@@ -52,7 +56,10 @@ internal class ActivityReactionListStateImplTest {
5256
@Test
5357
fun `on reactionRemoved, then remove specific reaction`() = runTest {
5458
val initialReactions =
55-
listOf(feedsReactionData(), feedsReactionData("reaction-2", "activity-1", "user-2"))
59+
listOf(
60+
feedsReactionData(activityId = "activity-1", userId = "user-1"),
61+
feedsReactionData(activityId = "activity-1", userId = "user-2"),
62+
)
5663
val paginationResult = defaultPaginationResult(initialReactions)
5764
activityReactionListState.onQueryMoreActivityReactions(paginationResult, queryConfig)
5865

@@ -62,10 +69,32 @@ internal class ActivityReactionListStateImplTest {
6269
assertEquals(listOf(initialReactions[1]), remainingReactions)
6370
}
6471

72+
@Test
73+
fun `on reactionUpserted with new reaction, then insert in sorted order`() = runTest {
74+
val olderReaction =
75+
feedsReactionData(activityId = "activity-1", userId = "user-1", createdAt = Date(1000))
76+
val newerReaction =
77+
feedsReactionData(activityId = "activity-1", userId = "user-3", createdAt = Date(3000))
78+
val initialReactions = listOf(newerReaction, olderReaction) // newest first
79+
val paginationResult = defaultPaginationResult(initialReactions)
80+
activityReactionListState.onQueryMoreActivityReactions(paginationResult, queryConfig)
81+
82+
val middleReaction =
83+
feedsReactionData(activityId = "activity-1", userId = "user-2", createdAt = Date(2000))
84+
activityReactionListState.onReactionUpserted(middleReaction)
85+
86+
// The reaction is inserted in sorted position (newest first)
87+
val expectedOrder = listOf(newerReaction, middleReaction, olderReaction)
88+
assertEquals(expectedOrder, activityReactionListState.reactions.value)
89+
}
90+
6591
@Test
6692
fun `on onActivityRemoved, clear all reactions`() = runTest {
6793
val reactions =
68-
listOf(feedsReactionData(), feedsReactionData("reaction-2", "activity-1", "user-2"))
94+
listOf(
95+
feedsReactionData(activityId = "activity-1", userId = "user-1"),
96+
feedsReactionData(activityId = "activity-1", userId = "user-2"),
97+
)
6998
val paginationResult = defaultPaginationResult(reactions)
7099

71100
activityReactionListState.onQueryMoreActivityReactions(paginationResult, queryConfig)

0 commit comments

Comments
 (0)