From 5a4e6eb5957c7b862e922686d6582a3564d0db0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Saleniuk?= <30429749+saleniuk@users.noreply.github.com> Date: Wed, 23 Aug 2023 11:45:00 +0200 Subject: [PATCH] fix: missing username and avatar for first message after creating goup [WPB-3557] (#2125) --- .../home/conversations/AuthorHeaderHelper.kt | 47 ++++ .../home/conversations/ConversationScreen.kt | 20 +- .../conversations/AuthorHeaderHelperTest.kt | 225 ++++++++++++++++++ 3 files changed, 273 insertions(+), 19 deletions(-) create mode 100644 app/src/main/kotlin/com/wire/android/ui/home/conversations/AuthorHeaderHelper.kt create mode 100644 app/src/test/kotlin/com/wire/android/ui/home/conversations/AuthorHeaderHelperTest.kt diff --git a/app/src/main/kotlin/com/wire/android/ui/home/conversations/AuthorHeaderHelper.kt b/app/src/main/kotlin/com/wire/android/ui/home/conversations/AuthorHeaderHelper.kt new file mode 100644 index 00000000000..d4fe6dcbe30 --- /dev/null +++ b/app/src/main/kotlin/com/wire/android/ui/home/conversations/AuthorHeaderHelper.kt @@ -0,0 +1,47 @@ +/* + * Wire + * Copyright (C) 2023 Wire Swiss GmbH + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see http://www.gnu.org/licenses/. + */ +package com.wire.android.ui.home.conversations + +import androidx.annotation.VisibleForTesting +import com.wire.android.ui.home.conversations.model.UIMessage +import com.wire.kalium.util.DateTimeUtil + +object AuthorHeaderHelper { + + @VisibleForTesting + internal const val AGGREGATION_TIME_WINDOW: Int = 30_000 // millis + + internal fun shouldShowHeader(index: Int, messages: List, currentMessage: UIMessage): Boolean { + var showHeader = currentMessage is UIMessage.Regular + val nextIndex = index + 1 + if (nextIndex < messages.size) { + val nextUiMessage = messages[nextIndex] + if (currentMessage.header.userId == nextUiMessage.header.userId + && currentMessage is UIMessage.Regular + && nextUiMessage is UIMessage.Regular + ) { + val difference = DateTimeUtil.calculateMillisDifference( + nextUiMessage.header.messageTime.utcISO, + currentMessage.header.messageTime.utcISO, + ) + showHeader = difference > AGGREGATION_TIME_WINDOW + } + } + return showHeader + } +} diff --git a/app/src/main/kotlin/com/wire/android/ui/home/conversations/ConversationScreen.kt b/app/src/main/kotlin/com/wire/android/ui/home/conversations/ConversationScreen.kt index 400125b2c84..6b758030127 100644 --- a/app/src/main/kotlin/com/wire/android/ui/home/conversations/ConversationScreen.kt +++ b/app/src/main/kotlin/com/wire/android/ui/home/conversations/ConversationScreen.kt @@ -94,7 +94,6 @@ import com.wire.kalium.logic.data.user.UserId import com.wire.kalium.logic.feature.call.usecase.ConferenceCallingResult import com.wire.kalium.logic.feature.conversation.InteractionAvailability import com.wire.kalium.logic.feature.selfDeletingMessages.SelfDeletionTimer -import com.wire.kalium.util.DateTimeUtil import kotlinx.collections.immutable.ImmutableMap import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.delay @@ -111,7 +110,6 @@ import kotlin.time.Duration.Companion.milliseconds * Once the user scrolls further into older messages, we stop autoscroll. */ private const val MAXIMUM_SCROLLED_MESSAGES_UNTIL_AUTOSCROLL_STOPS = 5 -private const val AGGREGATION_TIME_WINDOW: Int = 30000 // TODO: !! this screen definitely needs a refactor and some cleanup !! @Composable @@ -648,7 +646,7 @@ fun MessageList( MessageItem( message = message, conversationDetailsData = conversationDetailsData, - showAuthor = shouldShowHeader(index, lazyPagingMessages.itemSnapshotList.items, message), + showAuthor = AuthorHeaderHelper.shouldShowHeader(index, lazyPagingMessages.itemSnapshotList.items, message), audioMessagesState = audioMessagesState, onAudioClick = onAudioItemClicked, onChangeAudioPosition = onChangeAudioPosition, @@ -676,22 +674,6 @@ fun MessageList( } } -private fun shouldShowHeader(index: Int, messages: List, currentMessage: UIMessage): Boolean { - var showHeader = true - val nextIndex = index + 1 - if (nextIndex < messages.size) { - val nextUiMessage = messages[nextIndex] - if (currentMessage.header.userId == nextUiMessage.header.userId) { - val difference = DateTimeUtil.calculateMillisDifference( - nextUiMessage.header.messageTime.utcISO, - currentMessage.header.messageTime.utcISO, - ) - showHeader = difference > AGGREGATION_TIME_WINDOW - } - } - return showHeader -} - private fun CoroutineScope.withSmoothScreenLoad(block: () -> Unit) = launch { val smoothAnimationDuration = 200.milliseconds delay(smoothAnimationDuration) // we wait a bit until the whole screen is loaded to show the animation properly diff --git a/app/src/test/kotlin/com/wire/android/ui/home/conversations/AuthorHeaderHelperTest.kt b/app/src/test/kotlin/com/wire/android/ui/home/conversations/AuthorHeaderHelperTest.kt new file mode 100644 index 00000000000..2a47d570b0a --- /dev/null +++ b/app/src/test/kotlin/com/wire/android/ui/home/conversations/AuthorHeaderHelperTest.kt @@ -0,0 +1,225 @@ +/* + * Wire + * Copyright (C) 2023 Wire Swiss GmbH + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see http://www.gnu.org/licenses/. + */ +package com.wire.android.ui.home.conversations + +import com.wire.android.framework.TestMessage +import com.wire.android.model.UserAvatarData +import com.wire.android.ui.home.conversations.model.MessageBody +import com.wire.android.ui.home.conversations.model.MessageSource +import com.wire.android.ui.home.conversations.model.UIMessage +import com.wire.android.ui.home.conversations.model.UIMessageContent +import com.wire.android.util.ui.UIText +import com.wire.kalium.logic.data.user.UserAvailabilityStatus +import com.wire.kalium.logic.data.user.UserId +import com.wire.kalium.util.DateTimeUtil +import org.amshove.kluent.internal.assertEquals +import org.junit.jupiter.api.Test +import java.util.UUID + +class AuthorHeaderHelperTest { + + @Test + fun givenOneRegularMessage_thenShouldShowHeaderForRecentMessage() { + // given + val messages = listOf(testRegularMessage()) + // when + val result = AuthorHeaderHelper.shouldShowHeader(0, messages, messages[0]) + // then + assertEquals(true, result) + } + + @Test + fun givenOneSystemMessage_thenShouldNotShowHeaderForRecentMessage() { + // given + val messages = listOf(testSystemMessage()) + // when + val result = AuthorHeaderHelper.shouldShowHeader(0, messages, messages[0]) + // then + assertEquals(false, result) + } + + @Test + fun givenOnePingMessage_thenShouldNotShowHeaderForRecentMessage() { + // given + val messages = listOf(testPingMessage()) + // when + val result = AuthorHeaderHelper.shouldShowHeader(0, messages, messages[0]) + // then + assertEquals(false, result) + } + + @Test + fun givenTwoRegularMessagesFromSameUser_thenShouldNotShowHeaderForRecentMessage() { + // given + val messages = listOf( // more recent message is first on list + testRegularMessage(userId = SELF_USER_ID, timestamp = "2021-01-01T00:00:01.000Z"), + testRegularMessage(userId = SELF_USER_ID, timestamp = "2021-01-01T00:00:00.000Z") + ) + // when + val result = AuthorHeaderHelper.shouldShowHeader(0, messages, messages[0]) + // then + assertEquals(false, result) + } + + @Test + fun givenTwoRegularMessagesFromDifferentUser_thenShouldShowHeaderForRecentMessage() { + // given + val messages = listOf( // more recent message is first on list + testRegularMessage(userId = SELF_USER_ID, timestamp = "2021-01-01T00:00:01.000Z"), + testRegularMessage(userId = OTHER_USER_ID, timestamp = "2021-01-01T00:00:00.000Z") + ) + // when + val result = AuthorHeaderHelper.shouldShowHeader(0, messages, messages[0]) + // then + assertEquals(true, result) + } + + @Test + fun givenSystemAndThenRegularMessageFromSameUser_thenShouldShowHeaderForRecentMessage() { + // given + val messages = listOf( // more recent message is first on list + testRegularMessage(userId = SELF_USER_ID, timestamp = "2021-01-01T00:00:01.000Z"), + testSystemMessage(userId = SELF_USER_ID, timestamp = "2021-01-01T00:00:00.000Z") + ) + // when + val result = AuthorHeaderHelper.shouldShowHeader(0, messages, messages[0]) + // then + assertEquals(true, result) + } + + @Test + fun givenRegularAndThenSystemMessagFromSameUsere_thenShouldNotShowHeaderForRecentMessage() { + // given + val messages = listOf( // more recent message is first on list + testSystemMessage(userId = SELF_USER_ID, timestamp = "2021-01-01T00:00:01.000Z"), + testRegularMessage(userId = SELF_USER_ID, timestamp = "2021-01-01T00:00:00.000Z") + ) + // when + val result = AuthorHeaderHelper.shouldShowHeader(0, messages, messages[0]) + // then + assertEquals(false, result) + } + + @Test + fun givenPingAndThenRegularMessageFromSameUser_thenShouldShowHeaderForRecentMessage() { + // given + val messages = listOf( // more recent message is first on list + testRegularMessage(userId = SELF_USER_ID, timestamp = "2021-01-01T00:00:01.000Z"), + testPingMessage(userId = SELF_USER_ID, timestamp = "2021-01-01T00:00:00.000Z") + ) + // when + val result = AuthorHeaderHelper.shouldShowHeader(0, messages, messages[0]) + // then + assertEquals(true, result) + } + + @Test + fun givenRegularAndThenPingMessageFromSameUser_thenShouldNotShowHeaderForRecentMessage() { + // given + val messages = listOf( // more recent message is first on list + testPingMessage(userId = SELF_USER_ID, timestamp = "2021-01-01T00:00:01.000Z"), + testRegularMessage(userId = SELF_USER_ID, timestamp = "2021-01-01T00:00:00.000Z") + ) + // when + val result = AuthorHeaderHelper.shouldShowHeader(0, messages, messages[0]) + // then + assertEquals(false, result) + } + + @Test + fun givenTwoRegularMessagesFromSameUserAndTimestampsWithinThreshold_thenShouldNotShowHeaderForRecentMessage() { + // given + val timestamp = "2021-01-01T00:00:00.000Z" + val timestampMinusLessThanThreshold = DateTimeUtil.minusMilliseconds(timestamp, AuthorHeaderHelper.AGGREGATION_TIME_WINDOW - 1L) + val messages = listOf( + testRegularMessage(userId = SELF_USER_ID, timestamp = timestamp), + testRegularMessage(userId = SELF_USER_ID, timestamp = timestampMinusLessThanThreshold) + ) + // when + val result = AuthorHeaderHelper.shouldShowHeader(0, messages, messages[0]) + // then + assertEquals(false, result) + } + + @Test + fun givenTwoRegularMessagesFromSameUserAndTimestampsBeyondThreshold_thenShouldShowHeaderForRecentMessage() { + // given + val timestamp = "2021-01-01T00:00:00.000Z" + val timestampMinusMoreThanThreshold = DateTimeUtil.minusMilliseconds(timestamp, AuthorHeaderHelper.AGGREGATION_TIME_WINDOW + 1L) + val messages = listOf( + testRegularMessage(userId = SELF_USER_ID, timestamp = timestamp), + testRegularMessage(userId = SELF_USER_ID, timestamp = timestampMinusMoreThanThreshold) + ) + // when + val result = AuthorHeaderHelper.shouldShowHeader(0, messages, messages[0]) + // then + assertEquals(true, result) + } + + companion object { + private val SELF_USER_ID = UserId("self", "domain") + private val OTHER_USER_ID = UserId("other", "domain") + + private fun testSystemMessage( + userId: UserId? = null, + timestamp: String = "2021-01-01T00:00:00.000Z" + ) = UIMessage.System( + header = TestMessage.UI_MESSAGE_HEADER.copy( + messageTime = TestMessage.UI_MESSAGE_HEADER.messageTime.copy( + utcISO = timestamp + ), + messageId = UUID.randomUUID().toString(), + userId = userId + ), + source = MessageSource.OtherUser, + messageContent = UIMessageContent.SystemMessage.HistoryLost() + ) + + private fun testPingMessage( + userId: UserId? = null, + timestamp: String = "2021-01-01T00:00:00.000Z" + ) = UIMessage.System( + header = TestMessage.UI_MESSAGE_HEADER.copy( + messageTime = TestMessage.UI_MESSAGE_HEADER.messageTime.copy( + utcISO = timestamp + ), + messageId = UUID.randomUUID().toString(), + userId = userId + ), + source = MessageSource.OtherUser, + messageContent = UIMessageContent.SystemMessage.Knock(UIText.DynamicString("pinged")), + ) + + private fun testRegularMessage( + userId: UserId? = null, + timestamp: String = "2021-01-01T00:00:00.000Z" + ) = UIMessage.Regular( + userAvatarData = UserAvatarData(asset = null, availabilityStatus = UserAvailabilityStatus.NONE), + source = if (userId == SELF_USER_ID) MessageSource.Self else MessageSource.OtherUser, + header = TestMessage.UI_MESSAGE_HEADER.copy( + messageTime = TestMessage.UI_MESSAGE_HEADER.messageTime.copy( + utcISO = timestamp + ), + messageId = UUID.randomUUID().toString(), + userId = userId + ), + messageContent = UIMessageContent.TextMessage(MessageBody(UIText.DynamicString("Some Text Message"))), + messageFooter = com.wire.android.ui.home.conversations.model.MessageFooter(TestMessage.UI_MESSAGE_HEADER.messageId) + ) + } +}