Skip to content

Commit

Permalink
[AND-65] Fix crash and improve error handling in poll creation. (#5630)
Browse files Browse the repository at this point in the history
Co-authored-by: PetarVelikov <[email protected]>
  • Loading branch information
VelikovPetar and PetarVelikov authored Feb 12, 2025
1 parent e536e13 commit 44f8f49
Show file tree
Hide file tree
Showing 7 changed files with 66 additions and 28 deletions.
12 changes: 7 additions & 5 deletions stream-chat-android-compose/api/stream-chat-android-compose.api
Original file line number Diff line number Diff line change
Expand Up @@ -1774,18 +1774,20 @@ public final class io/getstream/chat/android/compose/ui/messages/attachments/pol

public final class io/getstream/chat/android/compose/ui/messages/attachments/poll/PollSwitchInput {
public static final field $stable I
public synthetic fun <init> (Ljava/lang/Object;Ljava/lang/String;Ljava/lang/Object;IILkotlin/jvm/internal/DefaultConstructorMarker;)V
public synthetic fun <init> (Ljava/lang/Object;Ljava/lang/String;Ljava/lang/Object;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
public synthetic fun <init> (Ljava/lang/Object;Ljava/lang/String;Ljava/lang/Object;Ljava/lang/Object;IILkotlin/jvm/internal/DefaultConstructorMarker;)V
public synthetic fun <init> (Ljava/lang/Object;Ljava/lang/String;Ljava/lang/Object;Ljava/lang/Object;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
public final fun component1 ()Ljava/lang/Object;
public final fun component2 ()Ljava/lang/String;
public final fun component3 ()Ljava/lang/Object;
public final fun component4-PjHm6EE ()I
public final fun copy-YyDlPXQ (Ljava/lang/Object;Ljava/lang/String;Ljava/lang/Object;I)Lio/getstream/chat/android/compose/ui/messages/attachments/poll/PollSwitchInput;
public static synthetic fun copy-YyDlPXQ$default (Lio/getstream/chat/android/compose/ui/messages/attachments/poll/PollSwitchInput;Ljava/lang/Object;Ljava/lang/String;Ljava/lang/Object;IILjava/lang/Object;)Lio/getstream/chat/android/compose/ui/messages/attachments/poll/PollSwitchInput;
public final fun component4 ()Ljava/lang/Object;
public final fun component5-PjHm6EE ()I
public final fun copy-l6dddJE (Ljava/lang/Object;Ljava/lang/String;Ljava/lang/Object;Ljava/lang/Object;I)Lio/getstream/chat/android/compose/ui/messages/attachments/poll/PollSwitchInput;
public static synthetic fun copy-l6dddJE$default (Lio/getstream/chat/android/compose/ui/messages/attachments/poll/PollSwitchInput;Ljava/lang/Object;Ljava/lang/String;Ljava/lang/Object;Ljava/lang/Object;IILjava/lang/Object;)Lio/getstream/chat/android/compose/ui/messages/attachments/poll/PollSwitchInput;
public fun equals (Ljava/lang/Object;)Z
public final fun getDescription ()Ljava/lang/String;
public final fun getKeyboardType-PjHm6EE ()I
public final fun getMaxValue ()Ljava/lang/Object;
public final fun getMinValue ()Ljava/lang/Object;
public final fun getValue ()Ljava/lang/Object;
public fun hashCode ()I
public final fun setValue (Ljava/lang/Object;)V
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ public class AttachmentsPickerPollTabFactory : AttachmentsPickerTabFactory {
val pollSwitchItemFactory = ChatTheme.pollSwitchitemFactory
var optionItemList by remember { mutableStateOf(emptyList<PollOptionItem>()) }
var switchItemList: List<PollSwitchItem> by remember { mutableStateOf(pollSwitchItemFactory.providePollSwitchItemList()) }
var hasErrorOnOptions by remember { mutableStateOf(false) }
var hasError by remember { mutableStateOf(false) }
val nestedScrollConnection = remember {
object : NestedScrollConnection {
override fun onPreScroll(available: Offset, source: NestedScrollSource): Offset {
Expand All @@ -139,7 +139,7 @@ public class AttachmentsPickerPollTabFactory : AttachmentsPickerTabFactory {
.background(ChatTheme.colors.appBackground),
) {
val (question, onQuestionChanged) = rememberSaveable { mutableStateOf("") }
val isEnabled = question.isNotBlank() && optionItemList.any { it.title.isNotBlank() } && !hasErrorOnOptions
val isEnabled = question.isNotBlank() && optionItemList.any { it.title.isNotBlank() } && !hasError
val hasChanges = question.isNotBlank() || optionItemList.any { it.title.isNotBlank() }
var isShowingDiscardDialog by remember { mutableStateOf(false) }

Expand Down Expand Up @@ -175,7 +175,7 @@ public class AttachmentsPickerPollTabFactory : AttachmentsPickerTabFactory {
onQuestionsChanged = {
optionItemList = it
switchItemList = updateMaxVotesAllowedSwitch(optionItemList, switchItemList)
hasErrorOnOptions = it.fastAny { item -> item.pollOptionError != null }
hasError = hasError(optionItemList, switchItemList)
},
)

Expand All @@ -185,7 +185,7 @@ public class AttachmentsPickerPollTabFactory : AttachmentsPickerTabFactory {
pollSwitchItems = switchItemList,
onSwitchesChanged = {
switchItemList = it
hasErrorOnOptions = it.fastAny { item -> item.pollOptionError != null }
hasError = hasError(optionItemList, switchItemList)
},
)

Expand All @@ -202,6 +202,28 @@ public class AttachmentsPickerPollTabFactory : AttachmentsPickerTabFactory {
}
}

/**
* Checks if there are any errors in the 'options' list, or any errors or missing fields in the 'switches' list.
*/
private fun hasError(
options: List<PollOptionItem>,
switches: List<PollSwitchItem>,
): Boolean {
// Check errors in options
val hasErrorInOptions = options.fastAny { item ->
item.pollOptionError != null
}
// Check errors or missing fields in switches
val hasErrorInSwitches = switches.fastAny { item ->
val hasError = item.pollOptionError != null
val isMissingMandatoryInput = item.enabled &&
item.pollSwitchInput != null &&
item.pollSwitchInput.value.toString().isEmpty()
hasError || isMissingMandatoryInput
}
return hasErrorInOptions || hasErrorInSwitches
}

/**
* Updates the max votes allowed switch based on the number of options available.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import java.util.UUID
* @property title The title of this poll item.
* @property enabled Indicates if this switch is enabled or not.
* @property key The key that identifies this poll item.
* @property pollSwitchInput Optional input field to be presented when the switch is enabled.
* @property pollOptionError Indicates this option has an error.
*/
@Immutable
Expand All @@ -42,12 +43,14 @@ public data class PollSwitchItem(
*
* @property value The default value of the switch.
* @property description The description of the input in the switch (shown as hint/contentDescription).
* @property maxValue The maximum vale of the switch. Normally, you can use the limit of the decimal format of the [value].
* @property minValue The minimum value of the switch. Normally, you can use the limit of the decimal format of the [value].
* @property maxValue The maximum value of the switch. Normally, you can use the limit of the decimal format of the [value].
* @property keyboardType The type of the input of the switch and decide the keyboard type of the input.
*/
public data class PollSwitchInput(
public var value: Any,
public val description: String = "",
public val minValue: Any? = null,
public val maxValue: Any? = null,
public val keyboardType: KeyboardType = KeyboardType.Text,
)
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ import io.getstream.chat.android.compose.ui.theme.ChatTheme
* @param onSwitchesChanged A lambda that will be executed when a switch on the list is changed.
* @param itemHeightSize The height size of the question item.
* @param itemInnerPadding The inner padding size of the question item.
* It provides the index information [from] and [to] as a receiver, so you must swap the item of the [questions] list.
*/
@Composable
public fun PollSwitchList(
Expand Down Expand Up @@ -189,24 +188,34 @@ public fun PollSwitchList(
if (switchInput.keyboardType == KeyboardType.Number ||
switchInput.keyboardType == KeyboardType.Decimal
) {
val newInt = if (newValue.isBlank()) 0 else newValue.toInt()
val maxInt = switchInput.maxValue?.toString()?.toInt() ?: 0

if (newInt > maxInt) {
this[index] = item.copy(
pollSwitchInput = item.pollSwitchInput.copy(value = newValue),
pollOptionError = PollOptionNumberExceed(
context.getString(
R.string.stream_compose_poll_option_error_exceed,
maxInt,
),
),
)
} else {
if (newValue.isBlank()) {
// If newValue is empty, don't validate
this[index] = item.copy(
pollSwitchInput = item.pollSwitchInput.copy(value = newValue),
pollOptionError = null,
)
} else {
// Validate min/max range
val min = switchInput.minValue?.toString()?.toIntOrNull() ?: 0
val max = switchInput.maxValue?.toString()?.toIntOrNull() ?: 0
val value = newValue.toInt() // assume it is always numeric
if (value < min || value > max) {
this[index] = item.copy(
pollSwitchInput = item.pollSwitchInput.copy(value = newValue),
pollOptionError = PollOptionNumberExceed(
context.getString(
R.string.stream_compose_poll_option_error_exceed,
min,
max,
),
),
)
} else {
this[index] = item.copy(
pollSwitchInput = item.pollSwitchInput.copy(value = newValue),
pollOptionError = null,
)
}
}
} else {
this[index] = item.copy(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,9 @@ public class DefaultPollSwitchItemFactory(
PollSwitchItem(
title = context.getString(R.string.stream_compose_poll_option_switch_multiple_answers),
pollSwitchInput = PollSwitchInput(
value = 0,
value = "",
description = context.getString(R.string.stream_compose_poll_option_max_number_of_answers_hint),
minValue = 1,
maxValue = 2,
keyboardType = KeyboardType.Decimal,
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@
<string name="stream_compose_poll_option_description" translatable="false">Add an option</string>
<string name="stream_compose_poll_option_hint" translatable="false">Add an option</string>
<string name="stream_compose_poll_option_error_duplicated" translatable="false">This is already an option</string>
<string name="stream_compose_poll_option_error_exceed" translatable="false">Type a number under %d</string>
<string name="stream_compose_poll_option_error_exceed" translatable="false">Type a number between %d and %d</string>
<string name="stream_compose_poll_option_discard_dialog_title" translatable="false">Discard poll</string>
<string name="stream_compose_poll_option_discard_dialog_description" translatable="false">Are you sure want to discard your poll?</string>
<string name="stream_compose_poll_option_discard_dialog_cancel" translatable="false">Keep Editing</string>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ internal class DefaultPollSwitchItemFactoryTest {
Assertions.assertEquals("Multiple answers", items[0].title)
Assertions.assertEquals("maxVotesAllowed", items[0].key)
Assertions.assertFalse(items[0].enabled)
Assertions.assertEquals(0, items[0].pollSwitchInput?.value)
Assertions.assertEquals("", items[0].pollSwitchInput?.value)
Assertions.assertEquals(1, items[0].pollSwitchInput?.minValue)
Assertions.assertEquals(2, items[0].pollSwitchInput?.maxValue)
Assertions.assertEquals("Max number of answers", items[0].pollSwitchInput?.description)
Assertions.assertEquals(KeyboardType.Decimal, items[0].pollSwitchInput?.keyboardType)
Expand Down

0 comments on commit 44f8f49

Please sign in to comment.