From 60a9e35a0e80d41a1eb9438c97db0c8b3345af7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Saleniuk?= <30429749+saleniuk@users.noreply.github.com> Date: Thu, 25 Jul 2024 12:33:17 +0000 Subject: [PATCH] fix: block sending empty messages, also empty markdowns [WPB-10179] (#3231) --- .../MessageCompositionInputStateHolder.kt | 5 +- .../android/ui/markdown/MarkdownHelper.kt | 31 ++++ .../com/wire/android/util/StringUtil.kt | 6 +- .../MessageCompositionInputStateHolderTest.kt | 162 ++++++++++++++++-- .../com/wire/android/util/StringUtilTest.kt | 81 +++++++++ 5 files changed, 270 insertions(+), 15 deletions(-) diff --git a/app/src/main/kotlin/com/wire/android/ui/home/messagecomposer/state/MessageCompositionInputStateHolder.kt b/app/src/main/kotlin/com/wire/android/ui/home/messagecomposer/state/MessageCompositionInputStateHolder.kt index 21514fb3099..f7042cf42b8 100644 --- a/app/src/main/kotlin/com/wire/android/ui/home/messagecomposer/state/MessageCompositionInputStateHolder.kt +++ b/app/src/main/kotlin/com/wire/android/ui/home/messagecomposer/state/MessageCompositionInputStateHolder.kt @@ -36,6 +36,7 @@ import com.wire.android.R import com.wire.android.ui.common.colorsScheme import com.wire.android.ui.common.textfield.WireTextFieldColors import com.wire.android.ui.common.textfield.wireTextFieldColors +import com.wire.android.util.isNotMarkdownBlank import com.wire.android.util.ui.KeyboardHeight @Stable @@ -69,11 +70,11 @@ class MessageCompositionInputStateHolder(val messageTextState: TextFieldState) { val inputType: InputType by derivedStateOf { when (val state = compositionState) { is CompositionState.Composing -> InputType.Composing( - isSendButtonEnabled = messageTextState.text.isNotEmpty() + isSendButtonEnabled = messageTextState.text.isNotMarkdownBlank() ) is CompositionState.Editing -> InputType.Editing( - isEditButtonEnabled = messageTextState.text.toString() != state.originalMessageText + isEditButtonEnabled = messageTextState.text != state.originalMessageText && messageTextState.text.isNotMarkdownBlank() ) } } diff --git a/app/src/main/kotlin/com/wire/android/ui/markdown/MarkdownHelper.kt b/app/src/main/kotlin/com/wire/android/ui/markdown/MarkdownHelper.kt index f45a04418cd..01b836dc4ca 100644 --- a/app/src/main/kotlin/com/wire/android/ui/markdown/MarkdownHelper.kt +++ b/app/src/main/kotlin/com/wire/android/ui/markdown/MarkdownHelper.kt @@ -209,6 +209,37 @@ fun MarkdownNode.getFirstInlines(): MarkdownPreview? { } } +private fun List.isNotBlank(): Boolean = this.any { + when (it) { + is MarkdownNode.Document -> it.children.isNotBlank() + is MarkdownNode.Block.BlockQuote -> it.children.isNotBlank() + is MarkdownNode.Block.FencedCode -> it.literal.isNotBlank() + is MarkdownNode.Block.Heading -> it.children.isNotBlank() + is MarkdownNode.Block.IntendedCode -> it.literal.isNotBlank() + is MarkdownNode.Block.ListBlock.Bullet -> it.children.isNotBlank() + is MarkdownNode.Block.ListBlock.Ordered -> it.children.isNotBlank() + is MarkdownNode.Block.ListItem -> it.children.isNotBlank() + is MarkdownNode.Block.Paragraph -> it.children.isNotBlank() + is MarkdownNode.Block.Table -> it.children.isNotBlank() + is MarkdownNode.Block.TableContent.Body -> it.children.isNotBlank() + is MarkdownNode.Block.TableContent.Head -> it.children.isNotBlank() + is MarkdownNode.Block.ThematicBreak -> true + is MarkdownNode.Inline.Break -> true + is MarkdownNode.Inline.Code -> it.literal.isNotBlank() + is MarkdownNode.Inline.Emphasis -> it.children.isNotBlank() + is MarkdownNode.Inline.Image -> it.destination.isNotBlank() + is MarkdownNode.Inline.Link -> it.destination.isNotBlank() + is MarkdownNode.Inline.Strikethrough -> it.children.isNotBlank() + is MarkdownNode.Inline.StrongEmphasis -> it.children.isNotBlank() + is MarkdownNode.Inline.Text -> it.literal.isNotBlank() + is MarkdownNode.TableCell -> it.children.isNotBlank() + is MarkdownNode.TableRow -> it.children.isNotBlank() + is MarkdownNode.Unsupported -> false + } +} + +fun MarkdownNode.Document.isNotBlank(): Boolean = children.isNotBlank() + private fun List.toPreview(): MarkdownPreview { return MarkdownPreview(this.toPersistentList()) } diff --git a/app/src/main/kotlin/com/wire/android/util/StringUtil.kt b/app/src/main/kotlin/com/wire/android/util/StringUtil.kt index 7b4a8925208..14dee701d41 100644 --- a/app/src/main/kotlin/com/wire/android/util/StringUtil.kt +++ b/app/src/main/kotlin/com/wire/android/util/StringUtil.kt @@ -18,6 +18,8 @@ package com.wire.android.util +import com.wire.android.ui.markdown.isNotBlank +import com.wire.android.ui.markdown.toMarkdownDocument import java.math.BigInteger import java.security.MessageDigest @@ -31,7 +33,7 @@ val String.Companion.NEW_LINE_SYMBOL get() = "\n" fun String?.orDefault(default: String) = this ?: default -public inline fun String.ifNotEmpty(transform: () -> String): String = if (!isEmpty()) transform() else this +inline fun String.ifNotEmpty(transform: () -> String): String = if (!isEmpty()) transform() else this @Suppress("MagicNumber") fun String.sha256(): String { @@ -47,3 +49,5 @@ fun String.toTitleCase(delimiter: String = " ", separator: String = " "): String fun String.capitalizeFirstLetter(): String = lowercase().replaceFirstChar(Char::titlecaseChar) fun String.normalizeFileName(): String = this.replace("/", "") + +fun CharSequence.isNotMarkdownBlank(): Boolean = this.isNotBlank() && this.toString().toMarkdownDocument().isNotBlank() diff --git a/app/src/test/kotlin/com/wire/android/ui/home/messagecomposer/state/MessageCompositionInputStateHolderTest.kt b/app/src/test/kotlin/com/wire/android/ui/home/messagecomposer/state/MessageCompositionInputStateHolderTest.kt index d74a68dae2f..40a8b329c7f 100644 --- a/app/src/test/kotlin/com/wire/android/ui/home/messagecomposer/state/MessageCompositionInputStateHolderTest.kt +++ b/app/src/test/kotlin/com/wire/android/ui/home/messagecomposer/state/MessageCompositionInputStateHolderTest.kt @@ -18,13 +18,15 @@ package com.wire.android.ui.home.messagecomposer.state import androidx.compose.foundation.text.input.TextFieldState +import androidx.compose.foundation.text.input.setTextAndPlaceCursorAtEnd import androidx.compose.ui.unit.dp import com.wire.android.config.CoroutineTestExtension import com.wire.android.config.SnapshotExtension +import com.wire.android.util.EMPTY import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.runTest import org.amshove.kluent.shouldBeEqualTo -import org.junit.jupiter.api.BeforeEach +import org.amshove.kluent.shouldBeInstanceOf import org.junit.jupiter.api.Test import org.junit.jupiter.api.extension.ExtendWith @@ -32,19 +34,9 @@ import org.junit.jupiter.api.extension.ExtendWith @ExtendWith(CoroutineTestExtension::class, SnapshotExtension::class) class MessageCompositionInputStateHolderTest { - private lateinit var messageTextState: TextFieldState - - private lateinit var state: MessageCompositionInputStateHolder - - @BeforeEach - fun before() { - messageTextState = TextFieldState() - state = MessageCompositionInputStateHolder(messageTextState = messageTextState) - } - @Test fun `when offset increases and is bigger than previous and options height, options height is updated`() = runTest { - + val (state, _) = Arrangement().arrange() // When state.handleImeOffsetChange( 50.dp, @@ -61,6 +53,7 @@ class MessageCompositionInputStateHolderTest { @Test fun `when offset decreases and showSubOptions is false, options height is updated`() = runTest { // Given + val (state, _) = Arrangement().arrange() state.updateValuesForTesting(previousOffset = 50.dp) // When @@ -78,6 +71,7 @@ class MessageCompositionInputStateHolderTest { @Test fun `when offset decreases to zero, showOptions and isTextExpanded are set to false`() = runTest { // Given + val (state, _) = Arrangement().arrange() state.updateValuesForTesting(previousOffset = 50.dp) // When @@ -96,6 +90,7 @@ class MessageCompositionInputStateHolderTest { @Test fun `when offset equals keyboard height, showSubOptions is set to false`() = runTest { // Given + val (state, _) = Arrangement().arrange() state.updateValuesForTesting(keyboardHeight = 30.dp) // When @@ -113,6 +108,7 @@ class MessageCompositionInputStateHolderTest { @Test fun `when offset is greater than keyboard height, keyboardHeight is updated`() = runTest { // Given + val (state, _) = Arrangement().arrange() state.updateValuesForTesting(keyboardHeight = 20.dp) // When @@ -130,6 +126,7 @@ class MessageCompositionInputStateHolderTest { @Test fun `when offset increases and is greater than keyboardHeight but is less than previousOffset, keyboardHeight is updated`() = runTest { // Given + val (state, _) = Arrangement().arrange() state.updateValuesForTesting(previousOffset = 50.dp, keyboardHeight = 20.dp) // When @@ -149,6 +146,7 @@ class MessageCompositionInputStateHolderTest { fun `when offset decreases, showSubOptions is true, and actualOffset is greater than optionsHeight, values remain unchanged`() = runTest { // Given + val (state, _) = Arrangement().arrange() state.updateValuesForTesting( previousOffset = 50.dp, keyboardHeight = 20.dp, @@ -172,6 +170,7 @@ class MessageCompositionInputStateHolderTest { fun `when offset decreases, showSubOptions is false, and actualOffset is greater than optionsHeight, optionsHeight is updated`() = runTest { // Given + val (state, _) = Arrangement().arrange() state.updateValuesForTesting( previousOffset = 50.dp, keyboardHeight = 20.dp, @@ -194,6 +193,7 @@ class MessageCompositionInputStateHolderTest { @Test fun `when offset is the same as previousOffset and greater than current keyboardHeight, keyboardHeight is updated`() = runTest { // Given + val (state, _) = Arrangement().arrange() state.updateValuesForTesting(previousOffset = 40.dp, keyboardHeight = 20.dp) // When @@ -212,6 +212,7 @@ class MessageCompositionInputStateHolderTest { @Test fun `given first keyboard appear when source equals target, then initialKeyboardHeight is set`() = runTest { // Given + val (state, _) = Arrangement().arrange() val imeValue = 50.dp state.updateValuesForTesting(initialKeyboardHeight = 0.dp) @@ -226,6 +227,7 @@ class MessageCompositionInputStateHolderTest { fun `given extended keyboard height when attachment button is clicked, then keyboardHeight is set to initialKeyboardHeight`() = runTest { // Given + val (state, _) = Arrangement().arrange() val initialKeyboardHeight = 10.dp state.updateValuesForTesting(previousOffset = 40.dp, keyboardHeight = 20.dp, initialKeyboardHeight = initialKeyboardHeight) @@ -241,6 +243,7 @@ class MessageCompositionInputStateHolderTest { @Test fun `when offset decreases but is not zero, only optionsHeight is updated`() = runTest { // Given + val (state, _) = Arrangement().arrange() state.updateValuesForTesting(previousOffset = 50.dp) // When @@ -261,6 +264,7 @@ class MessageCompositionInputStateHolderTest { fun `when keyboard is still in a process of hiding from the previous screen after navigating, options should not be visible`() = runTest { // Given + val (state, _) = Arrangement().arrange() state.updateValuesForTesting(previousOffset = 0.dp) // When @@ -277,6 +281,140 @@ class MessageCompositionInputStateHolderTest { state.isTextExpanded shouldBeEqualTo false } + @Test + fun `given empty text, when composing, then send button should be disabled`() = runTest { + // Given + val messageText = String.EMPTY + val (state, _) = Arrangement() + .withText(messageText) + .arrange() + + // When + state.toComposing() + + // Then + state.inputType.shouldBeInstanceOf().let { + it.isSendButtonEnabled shouldBeEqualTo false + } + } + + @Test + fun `given non-empty text but with only empty markdown, when composing, then send button should be disabled`() = runTest { + // Given + val messageText = "# " // just an example, more combinations are tested in StringUtilTest + val (state, _) = Arrangement() + .withText(messageText) + .arrange() + + // When + state.toComposing() + + // Then + state.inputType.shouldBeInstanceOf().let { + it.isSendButtonEnabled shouldBeEqualTo false + } + } + + @Test + fun `given non-empty text, when composing, then send button should be enabled`() = runTest { + // Given + val messageText = "text" + val (state, _) = Arrangement() + .withText(messageText) + .arrange() + + // When + state.toComposing() + + // Then + state.inputType.shouldBeInstanceOf().let { + it.isSendButtonEnabled shouldBeEqualTo true + } + } + + @Test + fun `given empty text, when editing, then send button should be disabled`() = runTest { + // Given + val editMessageText = "edit" + val messageText = String.EMPTY + val (state, _) = Arrangement() + .withText(messageText) + .arrange() + + // When + state.toEdit(editMessageText) + + // Then + state.inputType.shouldBeInstanceOf().let { + it.isEditButtonEnabled shouldBeEqualTo false + } + } + + @Test + fun `given non-empty text bit with only empty markdown, when editing, then send button should be disabled`() = runTest { + // Given + val editMessageText = "edit" + val messageText = "# " // just an example, more combinations are tested in StringUtilTest + val (state, _) = Arrangement() + .withText(messageText) + .arrange() + + // When + state.toEdit(editMessageText) + + // Then + state.inputType.shouldBeInstanceOf().let { + it.isEditButtonEnabled shouldBeEqualTo false + } + } + + @Test + fun `given the same text as edit message text, when editing, then send button should be disabled`() = runTest { + // Given + val editMessageText = "edit" + val (state, _) = Arrangement() + .withText(editMessageText) + .arrange() + + // When + state.toEdit(editMessageText) + + // Then + state.inputType.shouldBeInstanceOf().let { + it.isEditButtonEnabled shouldBeEqualTo false + } + } + + @Test + fun `given different text than edit message text, when editing, then send button should be enabled`() = runTest { + // Given + val editMessageText = "edit" + val messageText = "$editMessageText new" + val (state, _) = Arrangement() + .withText(messageText) + .arrange() + + // When + state.toEdit(editMessageText) + + // Then + state.inputType.shouldBeInstanceOf().let { + it.isEditButtonEnabled shouldBeEqualTo true + } + } + + class Arrangement { + + private val textFieldState = TextFieldState() + private val state = MessageCompositionInputStateHolder(textFieldState) + + fun withText(text: String) = apply { + textFieldState.setTextAndPlaceCursorAtEnd(text) + } + + fun arrange() = state to this + } + companion object { // I set it 0 to make tests more straight forward val NAVIGATION_BAR_HEIGHT = 0.dp diff --git a/app/src/test/kotlin/com/wire/android/util/StringUtilTest.kt b/app/src/test/kotlin/com/wire/android/util/StringUtilTest.kt index 0a8238c4dd9..f7a33180f22 100644 --- a/app/src/test/kotlin/com/wire/android/util/StringUtilTest.kt +++ b/app/src/test/kotlin/com/wire/android/util/StringUtilTest.kt @@ -17,6 +17,7 @@ */ package com.wire.android.util +import org.amshove.kluent.internal.assertEquals import org.junit.jupiter.api.Test class StringUtilTest { @@ -44,4 +45,84 @@ class StringUtilTest { val actual = input.normalizeFileName() assert(expected == actual) } + + @Suppress("LongMethod") + @Test + fun givenDifferentMarkdownsWithOnlyWhitespaces_whenCheckingIfNotBlank_thenReturnProperValues() { + + fun testIsNotMarkdownBlank(currentInput: String, expected: Boolean) { + val result = currentInput.isNotMarkdownBlank() + val errorMessage = "Expected $expected for input \"${currentInput}\".isNotMarkdownBlank() but got $result" + assertEquals(errorMessage, expected, result) + } + + testIsNotMarkdownBlank(" ", false) + testIsNotMarkdownBlank(" ", false) + testIsNotMarkdownBlank("\n", false) + testIsNotMarkdownBlank("a", true) + testIsNotMarkdownBlank(" a ", true) + + testIsNotMarkdownBlank("*", false) // empty bullet + testIsNotMarkdownBlank("* ", false) // empty bullet + testIsNotMarkdownBlank("*a", true) // just a text with one asterisk and "a" + testIsNotMarkdownBlank("* a", true) // bullet with "a" + testIsNotMarkdownBlank("* *", false) // two empty bullets + testIsNotMarkdownBlank("* * a", true) // two bullets with "a" + testIsNotMarkdownBlank("* * *", true) // thematic break + testIsNotMarkdownBlank("* * * a", true) // three bullets with "a" + testIsNotMarkdownBlank("**", true) // just a text with two asterisks + testIsNotMarkdownBlank("** **", true) // thematic break + testIsNotMarkdownBlank("**a**", true) // bold "a" text + testIsNotMarkdownBlank("** a **", true) // bold " a " text + testIsNotMarkdownBlank("**** ****", true) // thematic break + testIsNotMarkdownBlank("****a****", true) // bold "a" text + testIsNotMarkdownBlank("**\n**", true) // just two asterisks and another two asterisks in new line + + testIsNotMarkdownBlank("_", true) // just a text with one underscore + testIsNotMarkdownBlank("__", true) // just a text with two underscores + testIsNotMarkdownBlank("_ _", true) // just a text with two underscores and space between + testIsNotMarkdownBlank("_a_", true) // italic "a" text + testIsNotMarkdownBlank("_ a _", true) // italic " a " text + testIsNotMarkdownBlank("__ __", true) // thematic break + testIsNotMarkdownBlank("__a__", true) // bold "a" text + testIsNotMarkdownBlank("_\n_", true) // just underline and another underline in new line + + testIsNotMarkdownBlank("#", false) // empty header + testIsNotMarkdownBlank("##", false) // empty header + testIsNotMarkdownBlank("#a", true) // just a text with one hash and "a" + testIsNotMarkdownBlank("##a", true) // just a text with two hashes and "a" + testIsNotMarkdownBlank("# ", false) // empty header + testIsNotMarkdownBlank("# a", true) // header "a" text + testIsNotMarkdownBlank("## a", true) // header "a" text + + testIsNotMarkdownBlank("#_a_", true) // just a text with one hash and italic "a" + testIsNotMarkdownBlank("# _a_", true) // header italic "a" text + testIsNotMarkdownBlank("#__", true) // just a text with one hash and two underscores + testIsNotMarkdownBlank("#_ _", true) // just a text with one hash, two underscores and space between + testIsNotMarkdownBlank("# __", true) // header with two underscores + testIsNotMarkdownBlank("# _ _", true) // header with two underscores and space between + + testIsNotMarkdownBlank("_#_", true) // italic hash + testIsNotMarkdownBlank("_# _", true) // just a text with underscores and hash with space between + testIsNotMarkdownBlank("_#a_", true) // italic text with hash and "a" + testIsNotMarkdownBlank("_# a_", true) // italic text with underscores and hash, space and "a" between + + testIsNotMarkdownBlank("#**a**", true) // just a text with one hash and bold "a" + testIsNotMarkdownBlank("# **a**", true) // header bold "a" text + testIsNotMarkdownBlank("#****", true) // just a text with one hash and four asterisks + testIsNotMarkdownBlank("#** **", true) // just a text with one hash, four asterisks and space between + testIsNotMarkdownBlank("# ** **", true) // header with four asterisks and space between + + testIsNotMarkdownBlank("**#**", true) // bold hash + testIsNotMarkdownBlank("**# **", true) // just a text with asterisks and hash with space between + testIsNotMarkdownBlank("**#a**", true) // bold text with hash and "a" + testIsNotMarkdownBlank("**# a**", true) // bold text with underscores and hash, space and "a" between + + testIsNotMarkdownBlank("_****_", true) // italic four asterisks + testIsNotMarkdownBlank("_** **_", true) // italic four asterisks with space between + testIsNotMarkdownBlank("_**a**_", true) // bold italic "a" text + testIsNotMarkdownBlank("**__**", true) // bold two underscores + testIsNotMarkdownBlank("**_ _**", true) // bold two underscores with space between + testIsNotMarkdownBlank("**_a_**", true) // bold italic "a" text + } }